Refactor packet capture to use new logging library.

Use the acts.libs.proc.process library to create a process that logs tcpdump output to local files.

Bug: 120086380

Test: act.py -c <config> -tc WifiNetworkSelectorTest -tb chromeos1-dev-test-station-3
Change-Id: I36927147e1e15b4c1fd577396832d67a6a7e29aa
diff --git a/acts/framework/acts/controllers/packet_capture.py b/acts/framework/acts/controllers/packet_capture.py
index 3947bda..c05cc3a 100755
--- a/acts/framework/acts/controllers/packet_capture.py
+++ b/acts/framework/acts/controllers/packet_capture.py
@@ -18,8 +18,12 @@
 from acts.controllers.ap_lib.hostapd_constants import AP_DEFAULT_CHANNEL_2G
 from acts.controllers.ap_lib.hostapd_constants import AP_DEFAULT_CHANNEL_5G
 from acts.controllers.utils_lib.ssh import connection
+from acts.controllers.utils_lib.ssh import formatter
 from acts.controllers.utils_lib.ssh import settings
+from acts.libs.logging import log_stream
+from acts.libs.proc.process import Process
 
+import logging
 import os
 import threading
 import time
@@ -44,31 +48,16 @@
 def create(configs):
     return [PacketCapture(c) for c in configs]
 
+
 def destroy(pcaps):
     for pcap in pcaps:
         pcap.close()
 
+
 def get_info(pcaps):
     return [pcap.ssh_settings.hostname for pcap in pcaps]
 
 
-class PcapProperties(object):
-    """Class to maintain packet capture properties after starting tcpdump.
-
-    Attributes:
-        pid: proccess id of tcpdump
-        pcap_dir: tmp dir location where pcap files are saved
-        pcap_file: pcap file name
-        pcap_thread: thread used to push files to logpath
-    """
-    def __init__(self, pid, pcap_dir, pcap_file, pcap_thread):
-        """Initialize object."""
-        self.pid = pid
-        self.pcap_dir = pcap_dir
-        self.pcap_file = pcap_file
-        self.pcap_thread = pcap_thread
-
-
 class PacketCaptureError(Exception):
     """Error related to Packet capture."""
 
@@ -81,7 +70,7 @@
     wifi networks; 'wlan2' which is a dual band interface.
 
     Attributes:
-        pcap: dict that specifies packet capture properties for a band.
+        pcap: dict that specifies packet capture processes for a band.
         tmp_dirs: list of tmp directories created for pcap files.
     """
     def __init__(self, configs):
@@ -99,8 +88,7 @@
         self._create_interface(MON_5G, 'monitor')
         self._create_interface(SCAN_IFACE, 'managed')
 
-        self.pcap_properties = dict()
-        self._pcap_stop_lock = threading.Lock()
+        self.pcap_processes = dict()
         self.tmp_dirs = []
 
     def _create_interface(self, iface, mode):
@@ -157,62 +145,6 @@
                 network = {}
         return scan_networks
 
-    def _check_if_tcpdump_started(self, pcap_log):
-        """Check if tcpdump started.
-
-        This method ensures that tcpdump has started successfully.
-        We look for 'listening on' from the stdout indicating that tcpdump
-        is started.
-
-        Args:
-            pcap_log: log file that has redirected output of starting tcpdump.
-
-        Returns:
-            True/False if tcpdump is started or not.
-        """
-        curr_time = time.time()
-        timeout = 3
-        find_str = 'listening on'
-        while time.time() < curr_time + timeout:
-            result = self.ssh.run('grep "%s" %s' % (find_str, pcap_log),
-                                  ignore_status=True)
-            if result.stdout and find_str in result.stdout:
-                return True
-            time.sleep(1)
-        return False
-
-    def _pull_pcap(self, band, pcap_file, log_path):
-        """Pulls pcap files to test log path from onhub.
-
-        Called by start_packet_capture(). This method moves a pcap file to log
-        path once it has reached 50MB.
-
-        Args:
-            index: param that indicates if the tcpdump is stopped.
-            pcap_file: pcap file to move.
-            log_path: log path to move the pcap file to.
-        """
-        curr_no = 0
-        while True:
-            next_no = curr_no + 1
-            curr_fno = '%02i' % curr_no
-            next_fno = '%02i' % next_no
-            curr_file = '%s%s' % (pcap_file, curr_fno)
-            next_file = '%s%s' % (pcap_file, next_fno)
-
-            result = self.ssh.run('ls %s' % next_file, ignore_status=True)
-            if not result.stderr and next_file in result.stdout:
-                self.ssh.pull_file(log_path, curr_file)
-                self.ssh.run('rm -rf %s' % curr_file, ignore_status=True)
-                curr_no += 1
-                continue
-
-            with self._pcap_stop_lock:
-                if band not in self.pcap_properties:
-                    self.ssh.pull_file(log_path, curr_file)
-                    break
-            time.sleep(2) # wait before looking for file again
-
     def get_wifi_scan_results(self):
         """Starts a wifi scan on wlan2 interface.
 
@@ -269,72 +201,40 @@
             return False
         return True
 
-    def start_packet_capture(self, band, log_path, pcap_file):
+    def start_packet_capture(self, band, log_path):
         """Start packet capture for band.
 
         band = 2G starts tcpdump on 'mon0' interface.
         band = 5G starts tcpdump on 'mon1' interface.
 
-        This method splits the pcap file every 50MB for 100 files.
-        Since, the size of the pcap file could become large, each split file
-        is moved to log_path once a new file is generated. This ensures that
-        there is no crash on the onhub router due to lack of space.
-
         Args:
             band: '2g' or '2G' and '5g' or '5G'.
-            log_path: test log path to save the pcap file.
-            pcap_file: name of the pcap file.
-
-        Returns:
-            pid: process id of the tcpdump.
+            log_path: base logging directory
         """
         band = band.upper()
-        if band not in BAND_IFACE.keys() or band in self.pcap_properties:
+        if band not in BAND_IFACE.keys() or band in self.pcap_processes:
             self.log.error("Invalid band or packet capture already running")
-            return None
-
-        pcap_dir = self.ssh.run('mktemp -d', ignore_status=True).stdout.rstrip()
-        self.tmp_dirs.append(pcap_dir)
-        pcap_file = os.path.join(pcap_dir, "%s_%s.pcap" % (pcap_file, band))
-        pcap_log = os.path.join(pcap_dir, "%s.log" % pcap_file)
-
-        cmd = 'tcpdump -i %s -W 100 -C 50 -w %s > %s 2>&1 & echo $!' % (
-            BAND_IFACE[band], pcap_file, pcap_log)
-        result = self.ssh.run(cmd, ignore_status=True)
-        if not self._check_if_tcpdump_started(pcap_log):
-            self.log.error("Failed to start packet capture")
-            return None
-
-        pcap_thread = threading.Thread(target=self._pull_pcap,
-                                       args=(band, pcap_file, log_path))
-        pcap_thread.start()
-
-        pid = int(result.stdout)
-        self.pcap_properties[band] = PcapProperties(
-            pid, pcap_dir, pcap_file, pcap_thread)
-        return pid
-
-    def stop_packet_capture(self, pid):
-        """Stop the packet capture.
-
-        Args:
-            pid: process id of tcpdump to kill.
-        """
-        for key, val in self.pcap_properties.items():
-            if val.pid == pid:
-                break
-        else:
-            self.log.error("Failed to stop tcpdump. Invalid PID %s" % pid)
             return
 
-        pcap_dir = val.pcap_dir
-        pcap_thread = val.pcap_thread
-        self.ssh.run('kill %s' % pid, ignore_status=True)
-        with self._pcap_stop_lock:
-            del self.pcap_properties[key]
-        pcap_thread.join()
-        self.ssh.run('rm -rf %s' % pcap_dir, ignore_status=True)
-        self.tmp_dirs.remove(pcap_dir)
+        pcap_logger = log_stream.create_logger(
+            band, base_path=log_path,
+            log_styles=(log_stream.LogStyles.LOG_DEBUG +
+                        log_stream.LogStyles.TESTCASE_LOG))
+        pcap_logger.setLevel(logging.DEBUG)
+        cmd = formatter.SshFormatter().format_command(
+            'tcpdump -i %s -l' %
+            (BAND_IFACE[band]), None, self.ssh_settings)
+        pcap_proc = Process(cmd)
+        pcap_proc.set_on_output_callback(lambda msg: pcap_logger.debug(msg))
+        pcap_proc.start()
+
+        self.pcap_processes[band] = pcap_proc
+
+    def stop_packet_capture(self):
+        """Stop the packet capture."""
+        for band in list(self.pcap_processes.keys()):
+            self.pcap_processes[band].stop()
+            del self.pcap_processes[band]
 
     def close(self):
         """Cleanup.
@@ -343,6 +243,4 @@
         """
         self._cleanup_interface(MON_2G)
         self._cleanup_interface(MON_5G)
-        for tmp_dir in self.tmp_dirs:
-            self.ssh.run('rm -rf %s' % tmp_dir, ignore_status=True)
         self.ssh.close()
diff --git a/acts/framework/acts/libs/proc/process.py b/acts/framework/acts/libs/proc/process.py
index 477cfc6..f0d279f 100644
--- a/acts/framework/acts/libs/proc/process.py
+++ b/acts/framework/acts/libs/proc/process.py
@@ -125,24 +125,18 @@
         Args:
             kill_timeout: The amount of time to wait until killing the process.
         """
-        start_time = time.time()
-
         try:
             self._process.wait(kill_timeout)
         except subprocess.TimeoutExpired:
             self._stopped = True
             self._process.kill()
 
-        time_left = self._get_timeout_left(kill_timeout, start_time)
-
         if self._listening_thread is not None:
-            self._listening_thread.join(timeout=time_left)
+            self._listening_thread.join()
             self._listening_thread = None
 
-        time_left = self._get_timeout_left(kill_timeout, start_time)
-
         if self._redirection_thread is not None:
-            self._redirection_thread.join(timeout=time_left)
+            self._redirection_thread.join()
             self._redirection_thread = None
 
     def stop(self, timeout=60.0):
diff --git a/acts/framework/acts/test_utils/wifi/wifi_test_utils.py b/acts/framework/acts/test_utils/wifi/wifi_test_utils.py
index 67bac00..dc11c5a 100755
--- a/acts/framework/acts/test_utils/wifi/wifi_test_utils.py
+++ b/acts/framework/acts/test_utils/wifi/wifi_test_utils.py
@@ -16,7 +16,7 @@
 
 import logging
 import os
-import pprint
+import shutil
 import time
 
 from enum import IntEnum
@@ -1700,48 +1700,37 @@
     }
     return config
 
-def start_pcap(pcap, wifi_band, log_path, test_name):
+def start_pcap(pcap, wifi_band, log_path):
     """Start packet capture in monitor mode.
 
     Args:
         pcap: packet capture object
         wifi_band: '2g' or '5g' or 'dual'
-        log_path: current test log path
-        test_name: test name to be used for pcap file name
-
-    Returns:
-        Dictionary with pid of the tcpdump process as key and log path
-        of the file name as the value
+        log_path: base logging directory
     """
-    log_dir = os.path.join(log_path, test_name)
-    utils.create_dir(log_dir)
     if wifi_band == 'dual':
         bands = [BAND_2G, BAND_5G]
     else:
         bands = [wifi_band]
-    pids = {}
     for band in bands:
-        pid = pcap.start_packet_capture(band, log_dir, test_name)
-        pids[pid] = os.path.join(log_dir, test_name)
-    return pids
+        pcap.start_packet_capture(band, log_path)
 
-def stop_pcap(pcap, pids, test_status=None):
-    """Stop packet capture in monitor mode.
-
-    Since, the pcap logs in monitor mode can be very large, we will
-    delete them if they are not required. 'test_status' if True, will delete
-    the pcap files. If False, we will keep them.
+def stop_pcap(pcap, log_path, results):
+    """Stop packet capture in monitor mode. Deletes pcap files from passing
+    test cases.
 
     Args:
         pcap: packet capture object
-        pids: dictionary returned by start_pcap
-        test_status: status of the test case
+        log_path: base logging directory
+        results: test records for the test class
     """
-    for pid, fname in pids.items():
-        pcap.stop_packet_capture(pid)
-
-    if test_status:
-        os.system('rm -rf %s' % os.path.dirname(fname))
+    pcap.stop_packet_capture()
+    # Since the pcap logs in monitor mode can be very large, we will
+    # delete them if they are not required, i.e. if the test case passes
+    for record in results.passed:
+        pcap_dir = os.path.join(log_path, record.test_class,
+                                record.test_name)
+        shutil.rmtree(pcap_dir)
 
 def start_cnss_diags(ads):
     for ad in ads:
diff --git a/acts/tests/google/wifi/WifiChaosTest.py b/acts/tests/google/wifi/WifiChaosTest.py
index 8357d74..b7f30e9 100755
--- a/acts/tests/google/wifi/WifiChaosTest.py
+++ b/acts/tests/google/wifi/WifiChaosTest.py
@@ -14,6 +14,7 @@
 #   See the License for the specific language governing permissions and
 #   limitations under the License.
 
+import os
 import re
 import sys
 import time
@@ -85,6 +86,8 @@
         asserts.assert_true(
             self.lock_pcap(),
             "Could not lock a Packet Capture. Aborting Interop test.")
+        self.pcap_log_path = os.path.join(self.log_path, 'PacketCapture')
+        wutils.start_pcap(self.pcap, self.band.lower(), self.pcap_log_path)
 
         wutils.wifi_toggle_state(self.dut, True)
 
@@ -121,11 +124,7 @@
         self.dut.droid.wakeLockAcquireBright()
         self.dut.droid.wakeUpNow()
 
-    def on_pass(self, test_name, begin_time):
-        wutils.stop_pcap(self.pcap, self.pcap_pid, True)
-
     def on_fail(self, test_name, begin_time):
-        wutils.stop_pcap(self.pcap, self.pcap_pid, False)
         self.dut.take_bug_report(test_name, begin_time)
         self.dut.cat_adb_log(test_name, begin_time)
 
@@ -134,6 +133,9 @@
         self.dut.droid.goToSleepNow()
         wutils.reset_wifi(self.dut)
 
+    def teardown_class(self):
+        wutils.stop_pcap(self.pcap, self.pcap_log_path, self.results)
+
 
     """Helper Functions"""
 
@@ -279,8 +281,6 @@
 
         self.get_band_and_chan(ssid)
         self.pcap.configure_monitor_mode(self.band, self.chan)
-        self.pcap_pid = wutils.start_pcap(
-                self.pcap, self.band.lower(), self.log_path, self.test_name)
         self.run_connect_disconnect(network, hostname, rpm_port, rpm_ip,
                                     release_ap)
 
diff --git a/acts/tests/google/wifi/WifiNetworkSelectorTest.py b/acts/tests/google/wifi/WifiNetworkSelectorTest.py
index ffeb6b5..6d5baa7 100644
--- a/acts/tests/google/wifi/WifiNetworkSelectorTest.py
+++ b/acts/tests/google/wifi/WifiNetworkSelectorTest.py
@@ -15,6 +15,7 @@
 #   limitations under the License.
 
 import logging
+import os
 import time
 
 import acts.signals as signals
@@ -47,6 +48,7 @@
 
     def __init__(self, controllers):
         WifiBaseTest.__init__(self, controllers)
+        self.pcap_log_path = None
 
     def setup_class(self):
         self.dut = self.android_devices[0]
@@ -61,6 +63,8 @@
 
         if hasattr(self, 'packet_capture'):
             self.configure_packet_capture()
+            self.pcap_log_path = os.path.join(self.log_path, 'PacketCapture')
+            wutils.start_pcap(self.packet_capture, 'dual', self.pcap_log_path)
 
     def setup_test(self):
         #reset and clear all saved networks on the DUT
@@ -73,26 +77,20 @@
         self.dut.droid.wakeUpNow()
         self.dut.ed.clear_all_events()
 
-        if hasattr(self, 'packet_capture'):
-            self.pcap_pids = wutils.start_pcap(
-                self.packet_capture, 'dual', self.log_path, self.test_name)
-
     def teardown_test(self):
         #turn off the screen
         self.dut.droid.wakeLockRelease()
         self.dut.droid.goToSleepNow()
 
-    def on_pass(self, test_name, begin_time):
-        if hasattr(self, 'packet_capture'):
-            wutils.stop_pcap(self.packet_capture, self.pcap_pids, True)
-
     def on_fail(self, test_name, begin_time):
-        if hasattr(self, 'packet_capture'):
-            wutils.stop_pcap(self.packet_capture, self.pcap_pids, False)
         self.dut.take_bug_report(test_name, begin_time)
         self.dut.cat_adb_log(test_name, begin_time)
 
     def teardown_class(self):
+        if hasattr(self, 'packet_capture'):
+            wutils.stop_pcap(self.packet_capture, self.pcap_log_path,
+                             self.results)
+
         if "AccessPoint" in self.user_params:
             del self.user_params["reference_networks"]
             del self.user_params["open_network"]