pw_system: Style fixes to Python scripts

- Move time_offset from Device to DeviceWithTracing.
- Include modules not classes. While PEP8 allows including classes, it
  is easier to know where a class or free function comes from when
  reading the code -- this is also what the Google style suggest, which
  we don't follow but PEP8 allows it.
- Fix multiline and single line strings.
- Pass kwargs as kwargs in DeviceWithTracing.
- Use only one type of quotes (single).

Test: Ran pw_system demo on host and pw console
Change-Id: I06d897283a3536e63b6ceee1485cef5b06e1c6a5
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/182661
Reviewed-by: Ted Pudlik <tpudlik@google.com>
Reviewed-by: Dave Roth <davidroth@google.com>
Commit-Queue: Carlos Chinchilla <cachinchilla@google.com>

pw_system/py/pw_system/console.py

@@ -55,17 +55,17 @@
 import IPython  # type: ignore
 
 from pw_cli import log as pw_cli_log
-from pw_console.embed import PwConsoleEmbed
-from pw_console.log_store import LogStore
-from pw_console.plugins.bandwidth_toolbar import BandwidthToolbar
-from pw_console.pyserial_wrapper import SerialWithLogging
-from pw_console.python_logging import create_temp_log_file, JsonLogFormatter
-from pw_console.socket_client import SocketClient, SocketClientWithLogging
+from pw_console import embed
+from pw_console import log_store
+from pw_console.plugins import bandwidth_toolbar
+from pw_console import pyserial_wrapper
+from pw_console import python_logging
+from pw_console import socket_client
 from pw_hdlc import rpc
 from pw_rpc.console_tools.console import flattened_rpc_completions
-from pw_system.device import Device
-from pw_system.device_tracing import DeviceWithTracing
-from pw_tokenizer.detokenize import AutoUpdatingDetokenizer
+from pw_system import device as pw_device
+from pw_system import device_tracing
+from pw_tokenizer import detokenize
 
 # Default proto imports:
 from pw_log.proto import log_pb2
@@ -165,8 +165,8 @@ def get_parser() -> argparse.ArgumentParser:
             'Socket address used to connect to server. Type "default" to use '
             'localhost:33000, pass the server address and port as '
             'address:port, or prefix the path to a forwarded socket with '
-            f'"{SocketClient.FILE_SOCKET_SERVER}:" as '
-            f'{SocketClient.FILE_SOCKET_SERVER}:path_to_file.'
+            f'"{socket_client.SocketClient.FILE_SOCKET_SERVER}:" as '
+            f'{socket_client.SocketClient.FILE_SOCKET_SERVER}:path_to_file.'
         ),
     )
     parser.add_argument(
@@ -265,10 +265,10 @@ def _expand_globs(globs: Iterable[str]) -> Iterator[Path]:
 
 
 def _start_python_terminal(  # pylint: disable=too-many-arguments
-    device: Device,
-    device_log_store: LogStore,
-    root_log_store: LogStore,
-    serial_debug_log_store: LogStore,
+    device: pw_device.Device,
+    device_log_store: log_store.LogStore,
+    root_log_store: log_store.LogStore,
+    serial_debug_log_store: log_store.LogStore,
     log_file: str,
     host_logfile: str,
     device_logfile: str,
@@ -327,14 +327,14 @@ def _start_python_terminal(  # pylint: disable=too-many-arguments
     client_info = device.info()
     completions = flattened_rpc_completions([client_info])
 
-    log_windows: Dict[str, Union[List[logging.Logger], LogStore]] = {
+    log_windows: Dict[str, Union[List[logging.Logger], log_store.LogStore]] = {
         'Device Logs': device_log_store,
         'Host Logs': root_log_store,
     }
     if serial_debug:
         log_windows['Serial Debug'] = serial_debug_log_store
 
-    interactive_console = PwConsoleEmbed(
+    interactive_console = embed.PwConsoleEmbed(
         global_vars=local_variables,
         local_vars=None,
         loggers=log_windows,
@@ -344,7 +344,9 @@ def _start_python_terminal(  # pylint: disable=too-many-arguments
     )
     interactive_console.add_sentence_completer(completions)
     if serial_debug:
-        interactive_console.add_bottom_toolbar(BandwidthToolbar())
+        interactive_console.add_bottom_toolbar(
+            bandwidth_toolbar.BandwidthToolbar()
+        )
 
     # Setup Python logger propagation
     interactive_console.setup_python_logging(
@@ -387,13 +389,13 @@ def console(
 
     # Don't send device logs to the root logger.
     _DEVICE_LOG.propagate = False
-    # Create pw_console LogStore handlers. These are the data source for log
-    # messages to be displayed in the UI.
-    device_log_store = LogStore()
-    root_log_store = LogStore()
-    serial_debug_log_store = LogStore()
-    # Attach the LogStores as handlers for each log window we want to show.
-    # This should be done before device initialization to capture early
+    # Create pw_console log_store.LogStore handlers. These are the data source
+    # for log messages to be displayed in the UI.
+    device_log_store = log_store.LogStore()
+    root_log_store = log_store.LogStore()
+    serial_debug_log_store = log_store.LogStore()
+    # Attach the log_store.LogStores as handlers for each log window we want to
+    # show. This should be done before device initialization to capture early
     # messages.
     _DEVICE_LOG.addHandler(device_log_store)
     _ROOT_LOG.addHandler(root_log_store)
@@ -402,7 +404,7 @@ def console(
     if not logfile:
         # Create a temp logfile to prevent logs from appearing over stdout. This
         # would corrupt the prompt toolkit UI.
-        logfile = create_temp_log_file()
+        logfile = python_logging.create_temp_log_file()
 
     log_level = logging.DEBUG if verbose else logging.INFO
 
@@ -445,7 +447,7 @@ def console(
     if json_logfile:
         json_filehandler = logging.FileHandler(json_logfile, encoding='utf-8')
         json_filehandler.setLevel(log_level)
-        json_filehandler.setFormatter(JsonLogFormatter())
+        json_filehandler.setFormatter(python_logging.JsonLogFormatter())
         _DEVICE_LOG.addHandler(json_filehandler)
 
     detokenizer = None
@@ -454,7 +456,9 @@ def console(
         for token_database in token_databases:
             token_databases_with_domains.append(str(token_database) + "#trace")
 
-        detokenizer = AutoUpdatingDetokenizer(*token_databases_with_domains)
+        detokenizer = detokenize.AutoUpdatingDetokenizer(
+            *token_databases_with_domains
+        )
         detokenizer.show_errors = True
 
     protos: List[Union[ModuleType, Path]] = list(_expand_globs(proto_globs))
@@ -490,7 +494,11 @@ def console(
 
     timestamp_decoder = None
     if socket_addr is None:
-        serial_impl = SerialWithLogging if serial_debug else serial.Serial
+        serial_impl = (
+            pyserial_wrapper.SerialWithLogging
+            if serial_debug
+            else serial.Serial
+        )
         serial_device = serial_impl(
             device,
             baudrate,
@@ -510,9 +518,15 @@ def milliseconds_to_string(timestamp):
 
         timestamp_decoder = milliseconds_to_string
     else:
-        socket_impl = SocketClientWithLogging if serial_debug else SocketClient
+        socket_impl = (
+            socket_client.SocketClientWithLogging
+            if serial_debug
+            else socket_client.SocketClient
+        )
 
-        def disconnect_handler(socket_device: SocketClient) -> None:
+        def disconnect_handler(
+            socket_device: socket_client.SocketClient,
+        ) -> None:
             """Attempts to reconnect on disconnected socket."""
             _LOG.error('Socket disconnected. Will retry to connect.')
             while True:
@@ -535,17 +549,17 @@ def disconnect_handler(socket_device: SocketClient) -> None:
             return 1
 
     with reader:
-        device_client = DeviceWithTracing(
-            ticks_per_second,
+        device_client = device_tracing.DeviceWithTracing(
             channel_id,
             reader,
             write,
-            protos,
+            proto_library=protos,
             detokenizer=detokenizer,
             timestamp_decoder=timestamp_decoder,
             rpc_timeout_s=5,
             use_rpc_logging=rpc_logging,
             use_hdlc_encoding=hdlc_encoding,
+            ticks_per_second=ticks_per_second,
         )
         with device_client:
             _start_python_terminal(

pw_system/py/pw_system/device.py

@@ -19,23 +19,13 @@
 from typing import Any, Callable, List, Union, Optional
 
 from pw_thread_protos import thread_pb2
-from pw_hdlc.rpc import (
-    HdlcRpcClient,
-    channel_output,
-    NoEncodingSingleChannelRpcClient,
-    RpcClient,
-    CancellableReader,
-)
-from pw_log.log_decoder import (
-    Log,
-    LogStreamDecoder,
-    log_decoded_log,
-    timestamp_parser_ns_since_boot,
-)
-from pw_log_rpc.rpc_log_stream import LogStreamHandler
+from pw_hdlc import rpc
+from pw_log import log_decoder
+from pw_log_rpc import rpc_log_stream
 from pw_metric import metric_parser
-from pw_rpc import callback_client, Channel, console_tools
-from pw_thread.thread_analyzer import ThreadSnapshotAnalyzer
+import pw_rpc
+from pw_rpc import callback_client, console_tools
+from pw_thread import thread_analyzer
 from pw_tokenizer import detokenize
 from pw_tokenizer.proto import decode_optionally_tokenized
 from pw_unit_test.rpc import run_tests as pw_unit_test_run_tests, TestRecord
@@ -53,18 +43,15 @@ class Device:
     Note: use this class as a base for specialized device representations.
     """
 
-    # pylint: disable=too-many-instance-attributes
     def __init__(
-        # pylint: disable=too-many-arguments
         self,
         channel_id: int,
-        reader: CancellableReader,
+        reader: rpc.CancellableReader,
         write,
         proto_library: List[Union[ModuleType, Path]],
         detokenizer: Optional[detokenize.Detokenizer] = None,
         timestamp_decoder: Optional[Callable[[int], str]] = None,
         rpc_timeout_s: float = 5,
-        time_offset: int = 0,
         use_rpc_logging: bool = True,
         use_hdlc_encoding: bool = True,
         logger: logging.Logger = DEFAULT_DEVICE_LOGGER,
@@ -73,7 +60,6 @@ def __init__(
         self.protos = proto_library
         self.detokenizer = detokenizer
         self.rpc_timeout_s = rpc_timeout_s
-        self.time_offset = time_offset
 
         self.logger = logger
         self.logger.setLevel(logging.DEBUG)  # Allow all device logs through.
@@ -96,19 +82,21 @@ def detokenize_and_log_output(data: bytes, _detokenizer=None):
             for line in log_messages.splitlines():
                 self.logger.info(line)
 
-        self.client: RpcClient
+        self.client: rpc.RpcClient
         if use_hdlc_encoding:
-            channels = [Channel(self.channel_id, channel_output(write))]
-            self.client = HdlcRpcClient(
+            channels = [
+                pw_rpc.Channel(self.channel_id, rpc.channel_output(write))
+            ]
+            self.client = rpc.HdlcRpcClient(
                 reader,
                 self.protos,
                 channels,
                 detokenize_and_log_output,
                 client_impl=callback_client_impl,
             )
         else:
-            channel = Channel(self.channel_id, write)
-            self.client = NoEncodingSingleChannelRpcClient(
+            channel = pw_rpc.Channel(self.channel_id, write)
+            self.client = rpc.NoEncodingSingleChannelRpcClient(
                 reader,
                 self.protos,
                 channel,
@@ -118,22 +106,22 @@ def detokenize_and_log_output(data: bytes, _detokenizer=None):
         if use_rpc_logging:
             # Create the log decoder used by the LogStreamHandler.
 
-            def decoded_log_handler(log: Log) -> None:
-                log_decoded_log(log, self.logger)
+            def decoded_log_handler(log: log_decoder.Log) -> None:
+                log_decoder.log_decoded_log(log, self.logger)
 
-            self._log_decoder = LogStreamDecoder(
+            self._log_decoder = log_decoder.LogStreamDecoder(
                 decoded_log_handler=decoded_log_handler,
                 detokenizer=self.detokenizer,
                 source_name='RpcDevice',
                 timestamp_parser=(
                     timestamp_decoder
                     if timestamp_decoder
-                    else timestamp_parser_ns_since_boot
+                    else log_decoder.timestamp_parser_ns_since_boot
                 ),
             )
 
             # Start listening to logs as soon as possible.
-            self.log_stream_handler = LogStreamHandler(
+            self.log_stream_handler = rpc_log_stream.LogStreamHandler(
                 self.rpcs, self._log_decoder
             )
             self.log_stream_handler.listen_to_logs()
@@ -184,5 +172,7 @@ def snapshot_peak_stack_usage(self, thread_name: Optional[str] = None):
         for thread_info_block in rsp:
             for thread in thread_info_block.threads:
                 thread_info.threads.append(thread)
-        for line in str(ThreadSnapshotAnalyzer(thread_info)).splitlines():
+        for line in str(
+            thread_analyzer.ThreadSnapshotAnalyzer(thread_info)
+        ).splitlines():
             _LOG.info('%s', line)

pw_system/py/pw_system/device_tracing.py

@@ -37,16 +37,22 @@ class DeviceWithTracing(Device):
     """Represents an RPC Client for a device running a Pigweed target with
     tracing.
 
-    The target must have and RPC support for the following services:
+    The target must have RPC support for the following services:
      - logging
      - file
      - transfer
      - tracing
     Note: use this class as a base for specialized device representations.
     """
 
-    def __init__(self, ticks_per_second: Optional[int], *argv, **kargv):
-        super().__init__(*argv, **kargv)
+    def __init__(
+        self,
+        *device_args,
+        ticks_per_second: Optional[int] = None,
+        time_offset: int = 0,
+        **device_kwargs,
+    ):
+        super().__init__(*device_args, **device_kwargs)
 
         # Create the transfer manager
         self.transfer_service = self.rpcs.pw.transfer.Transfer
@@ -56,6 +62,7 @@ def __init__(self, ticks_per_second: Optional[int], *argv, **kargv):
             initial_response_timeout_s=self.rpc_timeout_s,
             default_protocol_version=pw_transfer.ProtocolVersion.LATEST,
         )
+        self.time_offset = time_offset
 
         if ticks_per_second:
             self.ticks_per_second = ticks_per_second
@@ -69,8 +76,7 @@ def get_ticks_per_second(self) -> int:
             resp = trace_service.GetClockParameters()
             if not resp.status.ok():
                 _LOG.error(
-                    'Failed to get clock parameters: %s. Using default \
-                    value',
+                    'Failed to get clock parameters: %s. Using default value',
                     resp.status,
                 )
                 return DEFAULT_TICKS_PER_SECOND