[Breaking Change] Initial implementation of Collector Refactor

devlib/__init__.py

@@ -45,10 +45,11 @@
 from devlib.derived.energy import DerivedEnergyMeasurements
 from devlib.derived.fps import DerivedGfxInfoStats, DerivedSurfaceFlingerStats
 
-from devlib.trace.ftrace import FtraceCollector
-from devlib.trace.perf import PerfCollector
-from devlib.trace.serial_trace import SerialTraceCollector
-from devlib.trace.dmesg import DmesgCollector
+from devlib.collector.ftrace import FtraceCollector
+from devlib.collector.perf import PerfCollector
+from devlib.collector.serial_trace import SerialTraceCollector
+from devlib.collector.dmesg import DmesgCollector
+from devlib.collector.logcat import LogcatCollector
 
 from devlib.host import LocalConnection
 from devlib.utils.android import AdbConnection

devlib/trace/__init__.py → devlib/collector/__init__.py

@@ -15,12 +15,14 @@
 
 import logging
 
+from devlib.utils.types import caseless_string
 
-class TraceCollector(object):
+class CollectorBase(object):
 
     def __init__(self, target):
         self.target = target
         self.logger = logging.getLogger(self.__class__.__name__)
+        self.output_path = None
 
     def reset(self):
         pass
@@ -31,6 +33,12 @@ def start(self):
     def stop(self):
         pass
 
+    def set_output(self, output_path):
+        self.output_path = output_path
+
+    def get_data(self):
+        return CollectorOutput()
+
     def __enter__(self):
         self.reset()
         self.start()
@@ -39,5 +47,29 @@ def __enter__(self):
     def __exit__(self, exc_type, exc_value, traceback):
         self.stop()
 
-    def get_trace(self, outfile):
-        pass
+class CollectorOutputEntry(object):
+
+    path_kinds = ['file', 'directory']
+
+    def __init__(self, path, path_kind):
+        self.path = path
+
+        path_kind = caseless_string(path_kind)
+        if path_kind not in self.path_kinds:
+            msg = '{} is not a valid path_kind [{}]'
+            raise ValueError(msg.format(path_kind, ' '.join(self.path_kinds)))
+        self.path_kind = path_kind
+
+    def __str__(self):
+        return self.path
+
+    def __repr__(self):
+        return '<{} ({})>'.format(self.path, self.path_kind)
+
+    def __fspath__(self):
+        """Allow using with os.path operations"""
+        return self.path
+
+
+class CollectorOutput(list):
+    pass

devlib/trace/dmesg.py → devlib/collector/dmesg.py

@@ -18,7 +18,8 @@
 from itertools import takewhile
 from datetime import timedelta
 
-from devlib.trace import TraceCollector
+from devlib.collector import (CollectorBase, CollectorOutput,
+                              CollectorOutputEntry)
 
 
 class KernelLogEntry(object):
@@ -121,7 +122,7 @@ def __str__(self):
         )
 
 
-class DmesgCollector(TraceCollector):
+class DmesgCollector(CollectorBase):
     """
     Dmesg output collector.
 
@@ -151,6 +152,7 @@ class DmesgCollector(TraceCollector):
 
     def __init__(self, target, level=LOG_LEVELS[-1], facility='kern'):
         super(DmesgCollector, self).__init__(target)
+        self.output_path = None
 
         if level not in self.LOG_LEVELS:
             raise ValueError('level needs to be one of: {}'.format(
@@ -195,6 +197,12 @@ def stop(self):
 
         self.dmesg_out = self.target.execute(cmd)
 
-    def get_trace(self, outfile):
-        with open(outfile, 'wt') as f:
+    def set_output(self, output_path):
+        self.output_path = output_path
+
+    def get_data(self):
+        if self.output_path is None:
+            raise RuntimeError("Output path was not set.")
+        with open(self.output_path, 'wt') as f:
             f.write(self.dmesg_out + '\n')
+        return CollectorOutput([CollectorOutputEntry(self.output_path, 'file')])

devlib/trace/ftrace.py → devlib/collector/ftrace.py

@@ -23,7 +23,8 @@
 import contextlib
 from pipes import quote
 
-from devlib.trace import TraceCollector
+from devlib.collector import (CollectorBase, CollectorOutput,
+                              CollectorOutputEntry)
 from devlib.host import PACKAGE_BIN_DIRECTORY
 from devlib.exception import TargetStableError, HostError
 from devlib.utils.misc import check_output, which, memoized
@@ -50,7 +51,7 @@
 CPU_RE = re.compile(r'  Function \(CPU([0-9]+)\)')
 STATS_RE = re.compile(r'([^ ]*) +([0-9]+) +([0-9.]+) us +([0-9.]+) us +([0-9.]+) us')
 
-class FtraceCollector(TraceCollector):
+class FtraceCollector(CollectorBase):
 
     # pylint: disable=too-many-locals,too-many-branches,too-many-statements
     def __init__(self, target,
@@ -86,6 +87,7 @@ def __init__(self, target,
         self.target_output_file = target.path.join(self.target.working_directory, OUTPUT_TRACE_FILE)
         text_file_name = target.path.splitext(OUTPUT_TRACE_FILE)[0] + '.txt'
         self.target_text_file = target.path.join(self.target.working_directory, text_file_name)
+        self.output_path = None
         self.target_binary = None
         self.host_binary = None
         self.start_time = None
@@ -300,9 +302,14 @@ def stop(self):
                             timeout=TIMEOUT, as_root=True)
         self._reset_needed = True
 
-    def get_trace(self, outfile):
-        if os.path.isdir(outfile):
-            outfile = os.path.join(outfile, os.path.basename(self.target_output_file))
+    def set_output(self, output_path):
+        if os.path.isdir(output_path):
+            output_path = os.path.join(output_path, os.path.basename(self.target_output_file))
+        self.output_path = output_path
+
+    def get_data(self):
+        if self.output_path is None:
+            raise RuntimeError("Output path was not set.")
         self.target.execute('{0} extract -o {1}; chmod 666 {1}'.format(self.target_binary,
                                                                        self.target_output_file),
                             timeout=TIMEOUT, as_root=True)
@@ -311,20 +318,24 @@ def get_trace(self, outfile):
         # Therefore timout for the pull command must also be adjusted
         # accordingly.
         pull_timeout = 10 * (self.stop_time - self.start_time)
-        self.target.pull(self.target_output_file, outfile, timeout=pull_timeout)
-        if not os.path.isfile(outfile):
+        self.target.pull(self.target_output_file, self.output_path, timeout=pull_timeout)
+        output = CollectorOutput()
+        if not os.path.isfile(self.output_path):
             self.logger.warning('Binary trace not pulled from device.')
         else:
+            output.append(CollectorOutputEntry(self.output_path, 'file'))
             if self.autoreport:
-                textfile = os.path.splitext(outfile)[0] + '.txt'
+                textfile = os.path.splitext(self.output_path)[0] + '.txt'
                 if self.report_on_target:
                     self.generate_report_on_target()
                     self.target.pull(self.target_text_file,
                                      textfile, timeout=pull_timeout)
                 else:
-                    self.report(outfile, textfile)
+                    self.report(self.output_path, textfile)
+                output.append(CollectorOutputEntry(textfile, 'file'))
             if self.autoview:
-                self.view(outfile)
+                self.view(self.output_path)
+        return output
 
     def get_stats(self, outfile):
         if not (self.functions and self.tracer is None):

devlib/trace/logcat.py → devlib/collector/logcat.py

@@ -16,14 +16,16 @@
 import os
 import shutil
 
-from devlib.trace import TraceCollector
+from devlib.collector import (CollectorBase, CollectorOutput,
+                              CollectorOutputEntry)
 from devlib.utils.android import LogcatMonitor
 
-class LogcatCollector(TraceCollector):
+class LogcatCollector(CollectorBase):
 
     def __init__(self, target, regexps=None):
         super(LogcatCollector, self).__init__(target)
         self.regexps = regexps
+        self.output_path = None
         self._collecting = False
         self._prev_log = None
         self._monitor = None
@@ -45,12 +47,14 @@ def start(self):
         """
         Start collecting logcat lines
         """
+        if self.output_path is None:
+            raise RuntimeError("Output path was not set.")
         self._monitor = LogcatMonitor(self.target, self.regexps)
         if self._prev_log:
             # Append new data collection to previous collection
             self._monitor.start(self._prev_log)
         else:
-            self._monitor.start()
+            self._monitor.start(self.output_path)
 
         self._collecting = True
 
@@ -65,9 +69,10 @@ def stop(self):
         self._collecting = False
         self._prev_log = self._monitor.logfile
 
-    def get_trace(self, outfile):
-        """
-        Output collected logcat lines to designated file
-        """
-        # copy self._monitor.logfile to outfile
-        shutil.copy(self._monitor.logfile, outfile)
+    def set_output(self, output_path):
+        self.output_path = output_path
+
+    def get_data(self):
+        if self.output_path is None:
+            raise RuntimeError("No data collected.")
+        return CollectorOutput([CollectorOutputEntry(self.output_path, 'file')])

devlib/trace/perf.py → devlib/collector/perf.py

@@ -19,13 +19,14 @@
 from past.builtins import basestring, zip
 
 from devlib.host import PACKAGE_BIN_DIRECTORY
-from devlib.trace import TraceCollector
+from devlib.collector import (CollectorBase, CollectorOutput,
+                              CollectorOutputEntry)
 from devlib.utils.misc import ensure_file_directory_exists as _f
 
 
 PERF_COMMAND_TEMPLATE = '{binary} {command} {options} {events} sleep 1000 > {outfile} 2>&1 '
 PERF_REPORT_COMMAND_TEMPLATE= '{binary} report {options} -i {datafile} > {outfile} 2>&1 '
-PERF_RECORD_COMMAND_TEMPLATE= '{binary} record {options} {events} -o {outfile}' 
+PERF_RECORD_COMMAND_TEMPLATE= '{binary} record {options} {events} -o {outfile}'
 
 PERF_DEFAULT_EVENTS = [
     'cpu-migrations',
@@ -42,7 +43,7 @@
 
 DEFAULT_EVENTS = {'perf':PERF_DEFAULT_EVENTS, 'simpleperf':SIMPLEPERF_DEFAULT_EVENTS}
 
-class PerfCollector(TraceCollector):
+class PerfCollector(CollectorBase):
     """
     Perf is a Linux profiling with performance counters.
     Simpleperf is an Android profiling tool with performance counters.
@@ -82,7 +83,7 @@ class PerfCollector(TraceCollector):
         man perf-stat
     """
 
-    def __init__(self, 
+    def __init__(self,
                  target,
                  perf_type='perf',
                  command='stat',
@@ -95,6 +96,7 @@ def __init__(self,
         self.force_install = force_install
         self.labels = labels
         self.report_options = report_options
+        self.output_path = None
 
         # Validate parameters
         if isinstance(optionstring, list):
@@ -148,14 +150,24 @@ def stop(self):
         self.target.killall('sleep', as_root=self.target.is_rooted)
         # NB: we hope that no other "important" sleep is on-going
 
-    # pylint: disable=arguments-differ
-    def get_trace(self, outdir):
+    def set_output(self, output_path):
+        self.output_path = output_path
+
+    def get_data(self):
+        if self.output_path is None:
+            raise RuntimeError("Output path was not set.")
+
+        output = CollectorOutput()
+
         for label in self.labels:
             if self.command == 'record':
-                self._wait_for_data_file_write(label, outdir)
-                self._pull_target_file_to_host(label, 'rpt', outdir)
+                self._wait_for_data_file_write(label, self.output_path)
+                path = self._pull_target_file_to_host(label, 'rpt', self.output_path)
+                output.append(CollectorOutputEntry(path, 'file'))
             else:
-                self._pull_target_file_to_host(label, 'out', outdir)
+                path = self._pull_target_file_to_host(label, 'out', self.output_path)
+                output.append(CollectorOutputEntry(path, 'file'))
+        return output
 
     def _deploy_perf(self):
         host_executable = os.path.join(PACKAGE_BIN_DIRECTORY,
@@ -198,13 +210,14 @@ def _build_perf_record_command(self, options, label):
                                                       outfile=self._get_target_file(label, 'data'))
         return command
 
-    def _pull_target_file_to_host(self, label, extension, outdir):
+    def _pull_target_file_to_host(self, label, extension, output_path):
         target_file = self._get_target_file(label, extension)
         host_relpath = os.path.basename(target_file)
-        host_file = _f(os.path.join(outdir, host_relpath))
+        host_file = _f(os.path.join(output_path, host_relpath))
         self.target.pull(target_file, host_file)
+        return host_file
 
-    def _wait_for_data_file_write(self, label, outdir):
+    def _wait_for_data_file_write(self, label, output_path):
         data_file_finished_writing = False
         max_tries = 80
         current_tries = 0
@@ -216,7 +229,7 @@ def _wait_for_data_file_write(self, label, outdir):
                 current_tries += 1
             else:
                 if current_tries >= max_tries:
-                    self.logger.warning('''writing {}.data file took longer than expected, 
+                    self.logger.warning('''writing {}.data file took longer than expected,
                                         file may not have written correctly'''.format(label))
                 data_file_finished_writing = True
         report_command = self._build_perf_report_command(self.report_options, label)
@@ -229,7 +242,7 @@ def _validate_events(self, events):
             if available_event == '':
                 continue
             if 'OR' in available_event:
-                available_events.append(available_event.split('OR')[1]) 
+                available_events.append(available_event.split('OR')[1])
             available_events[available_events.index(available_event)] = available_event.split()[0].strip()
         # Raw hex event codes can also be passed in that do not appear on perf/simpleperf list, prefixed with 'r'
         raw_event_code_regex = re.compile(r"^r(0x|0X)?[A-Fa-f0-9]+$")