From b9dca9db08ff7a675a3bc33d4cfaba1040e0e648 Mon Sep 17 00:00:00 2001
From: Austin Bozowski <bozowski@google.com>
Date: Fri, 3 Nov 2023 01:16:11 -0700
Subject: [PATCH] Refactor

---
 src/tools/interop/idt/README.md               | 16 +++++++++----
 src/tools/interop/idt/capture/__init__.py     |  2 +-
 .../idt/capture/{factory.py => controller.py} | 11 +++++----
 .../ecosystem/play_services/play_services.py  |  3 +--
 .../play_services/play_services_analysis.py   |  5 ++--
 .../capture/ecosystem/play_services/prober.py |  3 +--
 .../play_services_user/play_services_user.py  |  6 ++---
 src/tools/interop/idt/capture/loader.py       |  2 +-
 src/tools/interop/idt/capture/pcap/pcap.py    |  5 ++--
 .../idt/capture/platform/android/android.py   |  3 +--
 .../capture/platform/android/capabilities.py  |  6 ++---
 .../platform/android/streams/logcat/logcat.py |  3 +--
 .../platform/android/streams/pcap/pcap.py     |  8 +++----
 .../platform/android/streams/screen/screen.py |  4 +---
 .../idt/capture/utils/async_control.py        | 24 -------------------
 src/tools/interop/idt/discovery/ble.py        |  2 +-
 src/tools/interop/idt/discovery/dnssd.py      |  4 +---
 src/tools/interop/idt/idt.py                  |  6 ++---
 src/tools/interop/idt/probe/linux.py          |  4 ++--
 src/tools/interop/idt/probe/mac.py            |  4 ++--
 src/tools/interop/idt/probe/probe.py          | 10 +++-----
 src/tools/interop/idt/probe/runner.py         |  2 +-
 .../idt/{capture => }/utils/__init__.py       |  5 ++--
 .../idt/{capture => }/utils/artifact.py       |  4 +---
 .../idt/{capture => }/utils/host_platform.py  |  0
 src/tools/interop/idt/{ => utils}/log.py      |  0
 .../interop/idt/{capture => }/utils/shell.py  |  9 ++-----
 27 files changed, 56 insertions(+), 95 deletions(-)
 rename src/tools/interop/idt/capture/{factory.py => controller.py} (97%)
 delete mode 100644 src/tools/interop/idt/capture/utils/async_control.py
 rename src/tools/interop/idt/{capture => }/utils/__init__.py (89%)
 rename src/tools/interop/idt/{capture => }/utils/artifact.py (94%)
 rename src/tools/interop/idt/{capture => }/utils/host_platform.py (100%)
 rename src/tools/interop/idt/{ => utils}/log.py (100%)
 rename src/tools/interop/idt/{capture => }/utils/shell.py (92%)

diff --git a/src/tools/interop/idt/README.md b/src/tools/interop/idt/README.md
index 0ab28fb46e8db9..da754b551052b1 100644
--- a/src/tools/interop/idt/README.md
+++ b/src/tools/interop/idt/README.md
@@ -260,17 +260,15 @@ Collect contextually relevant networking info from the local environment and pro
 
 -   The entry point is in `idt.py` which contains simple CLI parsing with
     `argparse`.
--   `log` contains logging utilities used by everything in the project.
+
+
 
 ### `capture`
 
 -   `base` contains the base classes for ecosystems and platforms.
--   `factory` contains the ecosystem and platform producer and controller
+-   `controller` contains the ecosystem and platform producer and controller
 -   `loader` is a generic class loader that dynamically imports classes
     matching a given super class from a given directory.
--   `utils/shell` contains a simple helper class for background and foreground
-    Bash commands.
--   `utils/artifact` contains helper functions for managing artifacts.
 -   `/platform` and `/ecosystem` contain one package for each platform and ecosystem, which should each contain
     one implementation of the respective base classs.
 
@@ -289,6 +287,14 @@ Collect contextually relevant networking info from the local environment and pro
 - `linux` and `mac` contain `probe` implementations for each host platform.
 - - The package contains a simple dataclass to represent probe targets.
 
+### `utils`
+
+-   `log` contains logging utilities used by everything in the project.
+-   `artifact` contains helper functions for managing artifacts.
+-   `shell` contains a simple helper class for background and foreground
+    Bash commands.
+-   `host_platform` contains helpfer functions for the host running `idt`.
+
 ### Conventions
 
 -   `config.py` should be used to hold development configs within the directory
diff --git a/src/tools/interop/idt/capture/__init__.py b/src/tools/interop/idt/capture/__init__.py
index d4e9baedc1119d..1c8d4303e1b45e 100644
--- a/src/tools/interop/idt/capture/__init__.py
+++ b/src/tools/interop/idt/capture/__init__.py
@@ -17,7 +17,7 @@
 
 from capture import ecosystem, platform
 
-from .factory import EcosystemCapture, EcosystemController, EcosystemFactory, PlatformFactory, PlatformLogStreamer
+from .controller import EcosystemCapture, EcosystemController, EcosystemFactory, PlatformFactory, PlatformLogStreamer
 from .pcap import PacketCaptureRunner
 
 __all__ = [
diff --git a/src/tools/interop/idt/capture/factory.py b/src/tools/interop/idt/capture/controller.py
similarity index 97%
rename from src/tools/interop/idt/capture/factory.py
rename to src/tools/interop/idt/capture/controller.py
index aef6251e96747f..44d3d9d69a454c 100644
--- a/src/tools/interop/idt/capture/factory.py
+++ b/src/tools/interop/idt/capture/controller.py
@@ -23,11 +23,10 @@
 import typing
 
 import capture
-import log
 from capture.base import EcosystemCapture, PlatformLogStreamer, UnsupportedCapturePlatformException
-from capture.utils.artifact import create_standard_log_name, safe_mkdir
-from capture.utils.async_control import get_timeout
-from log import border_print
+from utils.artifact import create_standard_log_name, safe_mkdir, log
+from utils.log import border_print
+from . import config
 
 _PLATFORM_MAP: typing.Dict[str, PlatformLogStreamer] = {}
 _ECOSYSTEM_MAP: typing.Dict[str, PlatformLogStreamer] = {}
@@ -36,6 +35,10 @@
 logger = log.get_logger(__file__)
 
 
+def get_timeout():
+    return asyncio.get_running_loop().time() + config.async_timeout
+
+
 class PlatformFactory:
 
     @staticmethod
diff --git a/src/tools/interop/idt/capture/ecosystem/play_services/play_services.py b/src/tools/interop/idt/capture/ecosystem/play_services/play_services.py
index 1e87b99bafcfed..d2c83987f93b8b 100644
--- a/src/tools/interop/idt/capture/ecosystem/play_services/play_services.py
+++ b/src/tools/interop/idt/capture/ecosystem/play_services/play_services.py
@@ -20,10 +20,9 @@
 import os
 from typing import Dict
 
-import log
 from capture.base import EcosystemCapture, UnsupportedCapturePlatformException
 from capture.platform.android import Android
-from capture.utils.artifact import create_standard_log_name
+from utils.artifact import create_standard_log_name, log
 
 from . import config
 from .play_services_analysis import PlayServicesAnalysis
diff --git a/src/tools/interop/idt/capture/ecosystem/play_services/play_services_analysis.py b/src/tools/interop/idt/capture/ecosystem/play_services/play_services_analysis.py
index 7094b7edc5be6a..e6ebb8f13075d8 100644
--- a/src/tools/interop/idt/capture/ecosystem/play_services/play_services_analysis.py
+++ b/src/tools/interop/idt/capture/ecosystem/play_services/play_services_analysis.py
@@ -17,10 +17,9 @@
 
 import os
 
-import log
 from capture.platform.android import Android
-from capture.utils.artifact import create_standard_log_name
-from log import add_border, print_and_write
+from utils.artifact import create_standard_log_name, log
+from utils.log import add_border, print_and_write
 
 logger = log.get_logger(__file__)
 
diff --git a/src/tools/interop/idt/capture/ecosystem/play_services/prober.py b/src/tools/interop/idt/capture/ecosystem/play_services/prober.py
index 9c619947151663..ecdab93393d153 100644
--- a/src/tools/interop/idt/capture/ecosystem/play_services/prober.py
+++ b/src/tools/interop/idt/capture/ecosystem/play_services/prober.py
@@ -17,8 +17,7 @@
 
 import os
 
-import log
-from capture.utils.shell import Bash
+from utils.shell import Bash, log
 
 logger = log.get_logger(__file__)
 
diff --git a/src/tools/interop/idt/capture/ecosystem/play_services_user/play_services_user.py b/src/tools/interop/idt/capture/ecosystem/play_services_user/play_services_user.py
index fb73f9b9e5eac6..71752d2149f168 100644
--- a/src/tools/interop/idt/capture/ecosystem/play_services_user/play_services_user.py
+++ b/src/tools/interop/idt/capture/ecosystem/play_services_user/play_services_user.py
@@ -20,10 +20,10 @@
 
 from capture.base import EcosystemCapture, UnsupportedCapturePlatformException
 from capture.platform.android.android import Android
-from capture.utils.artifact import create_standard_log_name
-from log import print_and_write
+from utils.artifact import create_standard_log_name
+from utils.log import print_and_write
 
-from log import get_logger
+from utils.log import get_logger
 
 logger = get_logger(__file__)
 
diff --git a/src/tools/interop/idt/capture/loader.py b/src/tools/interop/idt/capture/loader.py
index 73c87382541142..8e02e9d492c2b5 100644
--- a/src/tools/interop/idt/capture/loader.py
+++ b/src/tools/interop/idt/capture/loader.py
@@ -21,7 +21,7 @@
 import traceback
 from typing import Any
 
-import log
+from utils import log
 
 logger = log.get_logger(__file__)
 
diff --git a/src/tools/interop/idt/capture/pcap/pcap.py b/src/tools/interop/idt/capture/pcap/pcap.py
index e87e07d124774f..2cb2285908b066 100644
--- a/src/tools/interop/idt/capture/pcap/pcap.py
+++ b/src/tools/interop/idt/capture/pcap/pcap.py
@@ -18,9 +18,8 @@
 import os
 import time
 
-import log
-from capture.utils.artifact import create_standard_log_name
-from capture.utils.shell import Bash
+from utils.artifact import create_standard_log_name, log
+from utils.shell import Bash
 
 logger = log.get_logger(__file__)
 
diff --git a/src/tools/interop/idt/capture/platform/android/android.py b/src/tools/interop/idt/capture/platform/android/android.py
index 81e6d0c11fde76..e9e0ba2345d31c 100644
--- a/src/tools/interop/idt/capture/platform/android/android.py
+++ b/src/tools/interop/idt/capture/platform/android/android.py
@@ -20,9 +20,8 @@
 import traceback
 import typing
 
-import log
 from capture.base import PlatformLogStreamer
-from capture.utils.shell import Bash
+from utils.shell import Bash, log
 
 from . import config, streams
 from .capabilities import Capabilities
diff --git a/src/tools/interop/idt/capture/platform/android/capabilities.py b/src/tools/interop/idt/capture/platform/android/capabilities.py
index 6f1e7a586e3051..535fb039aaeecd 100644
--- a/src/tools/interop/idt/capture/platform/android/capabilities.py
+++ b/src/tools/interop/idt/capture/platform/android/capabilities.py
@@ -1,13 +1,11 @@
 from typing import TYPE_CHECKING
 
-from capture.utils.artifact import create_standard_log_name
-from capture.utils.shell import Bash
+from utils.artifact import create_standard_log_name, log
+from utils.shell import Bash
 
 if TYPE_CHECKING:
     from capture.platform.android import Android
 
-import log
-
 from . import config
 
 logger = log.get_logger(__file__)
diff --git a/src/tools/interop/idt/capture/platform/android/streams/logcat/logcat.py b/src/tools/interop/idt/capture/platform/android/streams/logcat/logcat.py
index 5a07991270e7ea..78df8493b8d997 100644
--- a/src/tools/interop/idt/capture/platform/android/streams/logcat/logcat.py
+++ b/src/tools/interop/idt/capture/platform/android/streams/logcat/logcat.py
@@ -19,8 +19,7 @@
 import os
 from typing import TYPE_CHECKING
 
-import log
-from capture.utils.artifact import create_standard_log_name
+from utils.artifact import create_standard_log_name, log
 
 from ..base import AndroidStream
 
diff --git a/src/tools/interop/idt/capture/platform/android/streams/pcap/pcap.py b/src/tools/interop/idt/capture/platform/android/streams/pcap/pcap.py
index 4e8efdab24f9d8..1587f0c0f5a867 100644
--- a/src/tools/interop/idt/capture/platform/android/streams/pcap/pcap.py
+++ b/src/tools/interop/idt/capture/platform/android/streams/pcap/pcap.py
@@ -19,9 +19,9 @@
 import os
 from typing import TYPE_CHECKING
 
-from capture.utils.artifact import create_standard_log_name, safe_mkdir
-from capture.utils.shell import Bash
-from capture.utils.host_platform import is_mac
+from utils.artifact import create_standard_log_name, safe_mkdir, log
+from utils.shell import Bash
+from utils.host_platform import is_mac
 
 from ... import config
 from ..base import AndroidStream
@@ -29,8 +29,6 @@
 if TYPE_CHECKING:
     from capture.platform.android import Android
 
-import log
-
 logger = log.get_logger(__file__)
 
 
diff --git a/src/tools/interop/idt/capture/platform/android/streams/screen/screen.py b/src/tools/interop/idt/capture/platform/android/streams/screen/screen.py
index b36bba63b785d8..a18ca25f9d0c34 100644
--- a/src/tools/interop/idt/capture/platform/android/streams/screen/screen.py
+++ b/src/tools/interop/idt/capture/platform/android/streams/screen/screen.py
@@ -19,15 +19,13 @@
 import os
 from typing import TYPE_CHECKING
 
-from capture.utils.artifact import create_standard_log_name
+from utils.artifact import create_standard_log_name, log
 
 from ..base import AndroidStream
 
 if TYPE_CHECKING:
     from capture.platform.android import Android
 
-import log
-
 logger = log.get_logger(__file__)
 
 
diff --git a/src/tools/interop/idt/capture/utils/async_control.py b/src/tools/interop/idt/capture/utils/async_control.py
deleted file mode 100644
index d0e6f666b6a416..00000000000000
--- a/src/tools/interop/idt/capture/utils/async_control.py
+++ /dev/null
@@ -1,24 +0,0 @@
-#
-#    Copyright (c) 2023 Project CHIP Authors
-#    All rights reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License");
-#    you may not use this file except in compliance with the License.
-#    You may obtain a copy of the License at
-#
-#        http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS,
-#    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-#    See the License for the specific language governing permissions and
-#    limitations under the License.
-#
-
-import asyncio
-
-from capture import config
-
-
-def get_timeout():
-    return asyncio.get_running_loop().time() + config.async_timeout
diff --git a/src/tools/interop/idt/discovery/ble.py b/src/tools/interop/idt/discovery/ble.py
index a7618a55c0d9fb..4222ac7d4e9cb2 100644
--- a/src/tools/interop/idt/discovery/ble.py
+++ b/src/tools/interop/idt/discovery/ble.py
@@ -21,7 +21,7 @@
 import sys
 import time
 
-import log
+from utils import log
 from bleak import AdvertisementData, BleakScanner, BLEDevice
 from bleak.exc import BleakDBusError
 
diff --git a/src/tools/interop/idt/discovery/dnssd.py b/src/tools/interop/idt/discovery/dnssd.py
index 643bc201c53c49..34cfc7b2c7a113 100644
--- a/src/tools/interop/idt/discovery/dnssd.py
+++ b/src/tools/interop/idt/discovery/dnssd.py
@@ -18,10 +18,8 @@
 import asyncio
 import os
 
-import log
 from zeroconf import ServiceBrowser, ServiceInfo, ServiceListener, Zeroconf
-from capture.utils.artifact import create_standard_log_name
-
+from utils.artifact import create_standard_log_name, log
 
 logger = log.get_logger(__file__)
 
diff --git a/src/tools/interop/idt/idt.py b/src/tools/interop/idt/idt.py
index ef8957ff63085c..0af18addc58249 100644
--- a/src/tools/interop/idt/idt.py
+++ b/src/tools/interop/idt/idt.py
@@ -25,10 +25,10 @@
 from pathlib import Path
 
 from capture import EcosystemController, EcosystemFactory, PacketCaptureRunner, PlatformFactory
-from capture.utils.artifact import create_file_timestamp, safe_mkdir
-from capture.utils.shell import Bash
+from utils.artifact import create_file_timestamp, safe_mkdir
+from utils.shell import Bash
 from discovery import MatterBleScanner, MatterDnssdListener
-from log import border_print
+from utils.log import border_print
 
 import config
 
diff --git a/src/tools/interop/idt/probe/linux.py b/src/tools/interop/idt/probe/linux.py
index 5f021da7457f9d..17311f831d3aa9 100644
--- a/src/tools/interop/idt/probe/linux.py
+++ b/src/tools/interop/idt/probe/linux.py
@@ -16,9 +16,9 @@
 #
 
 import probe.probe as p
-from capture.utils.host_platform import get_ll_interface
+from utils.host_platform import get_ll_interface
 from . import config
-from log import get_logger
+from utils.log import get_logger
 
 logger = get_logger(__file__)
 
diff --git a/src/tools/interop/idt/probe/mac.py b/src/tools/interop/idt/probe/mac.py
index e6d4841c245098..d40ef4269b9013 100644
--- a/src/tools/interop/idt/probe/mac.py
+++ b/src/tools/interop/idt/probe/mac.py
@@ -16,8 +16,8 @@
 #
 
 import probe.probe as p
-from capture.utils.host_platform import get_ll_interface
-from log import get_logger
+from utils.host_platform import get_ll_interface
+from utils.log import get_logger
 from . import config, ProbeTarget
 
 logger = get_logger(__file__)
diff --git a/src/tools/interop/idt/probe/probe.py b/src/tools/interop/idt/probe/probe.py
index b3237cb3b5eda3..289279d4ecde69 100644
--- a/src/tools/interop/idt/probe/probe.py
+++ b/src/tools/interop/idt/probe/probe.py
@@ -20,11 +20,11 @@
 import os.path
 from abc import ABC, abstractmethod
 
-from capture.utils.artifact import create_standard_log_name
-from capture.utils.shell import Bash
+from utils.artifact import create_standard_log_name
+from utils.shell import Bash
 from discovery import MatterDnssdListener
 from discovery.dnssd import ServiceInfo
-from log import get_logger
+from utils.log import get_logger
 from . import config, ProbeTarget
 
 logger = get_logger(__file__)
@@ -33,10 +33,6 @@
 class GenericMatterProber(ABC):
 
     def __init__(self, artifact_dir: str, dnssd_artifact_dir: str) -> None:
-        """
-        Class that probes the local environment for all matter devices
-        Platform and ecosystem agnostic
-        """
         self.artifact_dir = artifact_dir
         self.dnssd_artifact_dir = dnssd_artifact_dir
         self.logger = logger
diff --git a/src/tools/interop/idt/probe/runner.py b/src/tools/interop/idt/probe/runner.py
index 01f8745b128cc2..ae48ca30e3a846 100644
--- a/src/tools/interop/idt/probe/runner.py
+++ b/src/tools/interop/idt/probe/runner.py
@@ -15,7 +15,7 @@
 #    limitations under the License.
 #
 
-from capture.utils.host_platform import is_mac
+from utils.host_platform import is_mac
 from .mac import ProberMacHost
 from .linux import ProberLinuxHost
 
diff --git a/src/tools/interop/idt/capture/utils/__init__.py b/src/tools/interop/idt/utils/__init__.py
similarity index 89%
rename from src/tools/interop/idt/capture/utils/__init__.py
rename to src/tools/interop/idt/utils/__init__.py
index f92780d4df0177..43ab9acb8a5558 100644
--- a/src/tools/interop/idt/capture/utils/__init__.py
+++ b/src/tools/interop/idt/utils/__init__.py
@@ -15,10 +15,11 @@
 #    limitations under the License.
 #
 
-from . import artifact, async_control, shell
+from . import artifact, host_platform, log, shell
 
 __all__ = [
     'artifact',
-    'async_control',
+    'host_platform',
+    'log',
     'shell',
 ]
diff --git a/src/tools/interop/idt/capture/utils/artifact.py b/src/tools/interop/idt/utils/artifact.py
similarity index 94%
rename from src/tools/interop/idt/capture/utils/artifact.py
rename to src/tools/interop/idt/utils/artifact.py
index a7b9cf202a157c..8749b1ab84880b 100644
--- a/src/tools/interop/idt/capture/utils/artifact.py
+++ b/src/tools/interop/idt/utils/artifact.py
@@ -15,13 +15,11 @@
 #    limitations under the License.
 #
 
-import multiprocessing
 import os
 import time
-from multiprocessing import Process
 from pathlib import Path
 
-import log
+from . import log
 
 logger = log.get_logger(__file__)
 
diff --git a/src/tools/interop/idt/capture/utils/host_platform.py b/src/tools/interop/idt/utils/host_platform.py
similarity index 100%
rename from src/tools/interop/idt/capture/utils/host_platform.py
rename to src/tools/interop/idt/utils/host_platform.py
diff --git a/src/tools/interop/idt/log.py b/src/tools/interop/idt/utils/log.py
similarity index 100%
rename from src/tools/interop/idt/log.py
rename to src/tools/interop/idt/utils/log.py
diff --git a/src/tools/interop/idt/capture/utils/shell.py b/src/tools/interop/idt/utils/shell.py
similarity index 92%
rename from src/tools/interop/idt/capture/utils/shell.py
rename to src/tools/interop/idt/utils/shell.py
index d8e5c8a26ba8c3..0d5beaa3c265dd 100644
--- a/src/tools/interop/idt/capture/utils/shell.py
+++ b/src/tools/interop/idt/utils/shell.py
@@ -20,7 +20,7 @@
 import subprocess
 import psutil
 
-import log
+from . import log
 
 logger = log.get_logger(__file__)
 
@@ -33,7 +33,7 @@ def __init__(self, command: str, sync: bool = False,
         """
         Run a bash command as a sub process
         :param command: Command to run
-        :param sync: If True, wait for command to terminate
+        :param sync: If True, wait for command to terminate upon start_command()
         :param capture_output: Only applies to sync; if True, store and suppress stdout and stderr
         :param cwd: Set working directory of command
         """
@@ -60,23 +60,19 @@ def get_captured_output(self) -> str:
 
     def start_command(self) -> None:
         if self.sync:
-            # Sync commands share stdin and out
             self.proc = subprocess.run(self.args, capture_output=self.capture_output, cwd=self.cwd)
             return
         if not self.command_is_running():
-            # Background commands cannot access stdin, but share stdout
             self.proc = subprocess.Popen(self.args, cwd=self.cwd, stdin=subprocess.PIPE)
         else:
             self.logger.warning(f'{self.command} start requested while running')
 
     def term_with_sudo(self, proc: multiprocessing.Process) -> None:
         self.logger.debug(f"SIGTERM {proc.pid} with sudo")
-        # These have to be sync in case another prompt for sudo is needed
         Bash(f"sudo kill {proc.pid}", sync=True).start_command()
 
     def kill_with_sudo(self, proc: multiprocessing.Process) -> None:
         self.logger.debug(f"SIGKILL {proc.pid} with sudo")
-        # These have to be sync in case another prompt for sudo is needed
         Bash(f"sudo kill -9 {proc.pid}", sync=True).start_command()
 
     def term(self, proc: multiprocessing.Process) -> None:
@@ -117,7 +113,6 @@ def stop_command(self) -> None:
             self.stop_single_proc(psutil_proc)
         else:
             self.logger.warning(f'{self.command} stop requested while not running')
-        self.proc = None
 
     def finished_success(self) -> bool:
         if not self.sync: