utils/version: Version bump

Instead of parsing the text of the file to extract the current version
use the version_helper to access the newly added version tuple.

devlib/__init__.py

@@ -15,7 +15,7 @@
 
 from devlib.target import Target, LinuxTarget, AndroidTarget, LocalLinuxTarget, ChromeOsTarget
 from devlib.host import PACKAGE_BIN_DIRECTORY
-from devlib.exception import DevlibError, TargetError, HostError, TargetNotRespondingError
+from devlib.exception import DevlibError, DevlibTransientError, DevlibStableError, TargetError, TargetTransientError, TargetStableError, TargetNotRespondingError, HostError
 
 from devlib.module import Module, HardRestModule, BootModule, FlashModule
 from devlib.module import get_module, register_module
@@ -34,25 +34,33 @@ from devlib.instrument.hwmon import HwmonInstrument
 from devlib.instrument.monsoon import MonsoonInstrument
 from devlib.instrument.netstats import NetstatsInstrument
 from devlib.instrument.gem5power import Gem5PowerInstrument
+from devlib.instrument.baylibre_acme import (
+        BaylibreAcmeNetworkInstrument,
+        BaylibreAcmeXMLInstrument,
+        BaylibreAcmeLocalInstrument,
+        BaylibreAcmeInstrument,
+)
 
 from devlib.derived import DerivedMeasurements, DerivedMetric
 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.host import LocalConnection
 from devlib.utils.android import AdbConnection
 from devlib.utils.ssh import SshConnection, TelnetConnection, Gem5Connection
 
-from devlib.utils.version import get_commit as __get_commit
+from devlib.utils.version import (get_devlib_version as __get_devlib_version,
+                                  get_commit as __get_commit)
 
 
-__version__ = '1.0.0'
+__version__ = __get_devlib_version()
 
 __commit = __get_commit()
 if __commit:
-    __full_version__ = '{}-{}'.format(__version__, __commit)
+    __full_version__ = '{}+{}'.format(__version__, __commit)
 else:
     __full_version__ = __version__

devlib/bin/scripts/shutils.in

@@ -214,7 +214,7 @@ cgroups_freezer_set_state() {
 
     # Set the state of the freezer
     echo $STATE > $SYSFS_ENTRY
-    
+
     # And check it applied cleanly
     for i in `seq 1 10`; do
         [ $($CAT $SYSFS_ENTRY) = $STATE ] && exit 0
@@ -238,6 +238,19 @@ hotplug_online_all() {
     done
 }
 
+
+################################################################################
+# Scheduler
+################################################################################
+
+sched_get_kernel_attributes() {
+	MATCH=${1:-'.*'}
+	[ -d /proc/sys/kernel/ ] || exit 1
+	$GREP '' /proc/sys/kernel/sched_* | \
+		$SED -e 's|/proc/sys/kernel/sched_||' | \
+		$GREP -e "$MATCH"
+}
+
 ################################################################################
 # Misc
 ################################################################################
@@ -264,6 +277,48 @@ read_tree_values() {
     fi
 }
 
+read_tree_tgz_b64() {
+    BASEPATH=$1
+    MAXDEPTH=$2
+    TMPBASE=$3
+
+    if [ ! -e $BASEPATH ]; then
+        echo "ERROR: $BASEPATH does not exist"
+        exit 1
+    fi
+
+    cd $TMPBASE
+    TMP_FOLDER=$($BUSYBOX realpath $($BUSYBOX mktemp -d XXXXXX))
+
+    # 'tar' doesn't work as expected on debugfs, so copy the tree first to
+    # workaround the issue
+    cd $BASEPATH
+    for CUR_FILE in $($BUSYBOX find . -follow -type f -maxdepth $MAXDEPTH); do
+        $BUSYBOX cp --parents $CUR_FILE $TMP_FOLDER/ 2> /dev/null
+    done
+
+    cd $TMP_FOLDER
+    $BUSYBOX tar cz * 2>/dev/null | $BUSYBOX base64
+
+    # Clean-up the tmp folder since we won't need it any more
+    cd $TMPBASE
+    rm -rf $TMP_FOLDER
+}
+
+get_linux_system_id() {
+	kernel=$($BUSYBOX uname -r)
+	hardware=$($BUSYBOX ip a | $BUSYBOX grep 'link/ether' | $BUSYBOX sed 's/://g' | $BUSYBOX awk '{print $2}' | $BUSYBOX tr -d '\n')
+	filesystem=$(ls /dev/disk/by-uuid | $BUSYBOX tr '\n' '-' | $BUSYBOX sed 's/-$//')
+	echo "$hardware/$kernel/$filesystem"
+}
+
+get_android_system_id() {
+	kernel=$($BUSYBOX uname -r)
+	hardware=$($BUSYBOX ip a | $BUSYBOX grep 'link/ether' | $BUSYBOX sed 's/://g' | $BUSYBOX awk '{print $2}' | $BUSYBOX tr -d '\n')
+	filesystem=$(content query --uri content://settings/secure --projection value --where "name='android_id'" | $BUSYBOX cut -f2 -d=)
+	echo "$hardware/$kernel/$filesystem"
+}
+
 ################################################################################
 # Main Function Dispatcher
 ################################################################################
@@ -323,6 +378,18 @@ hotplug_online_all)
 read_tree_values)
 	read_tree_values $*
     ;;
+read_tree_tgz_b64)
+	read_tree_tgz_b64 $*
+    ;;
+get_linux_system_id)
+	get_linux_system_id $*
+    ;;
+get_android_system_id)
+	get_android_system_id $*
+    ;;
+sched_get_kernel_attributes)
+	sched_get_kernel_attributes $*
+	;;
 *)
     echo "Command [$CMD] not supported"
     exit -1

devlib/derived/__init__.py

@@ -36,25 +36,28 @@ class DerivedMetric(object):
                 msg = 'Unknown measurement type:  {}'
                 raise ValueError(msg.format(measurement_type))
 
-    def __cmp__(self, other):
-        if hasattr(other, 'value'):
-            return cmp(self.value, other.value)
-        else:
-            return cmp(self.value, other)
-
     def __str__(self):
         if self.units:
             return '{}: {} {}'.format(self.name, self.value, self.units)
         else:
             return '{}: {}'.format(self.name, self.value)
 
+    # pylint: disable=undefined-variable
+    def __cmp__(self, other):
+        if hasattr(other, 'value'):
+            return cmp(self.value, other.value)
+        else:
+            return cmp(self.value, other)
+
     __repr__ = __str__
 
 
 class DerivedMeasurements(object):
 
+    # pylint: disable=no-self-use,unused-argument
     def process(self, measurements_csv):
         return []
 
+    # pylint: disable=no-self-use
     def process_raw(self, *args):
         return []

devlib/derived/energy.py

@@ -15,12 +15,13 @@
 from __future__ import division
 from collections import defaultdict
 
-from devlib import DerivedMeasurements, DerivedMetric
-from devlib.instrument import  MEASUREMENT_TYPES, InstrumentChannel
+from devlib.derived import DerivedMeasurements, DerivedMetric
+from devlib.instrument import  MEASUREMENT_TYPES
 
 
 class DerivedEnergyMeasurements(DerivedMeasurements):
 
+    # pylint: disable=too-many-locals,too-many-branches
     @staticmethod
     def process(measurements_csv):
 

devlib/derived/fps.py

@@ -15,7 +15,6 @@
 
 from __future__ import division
 import os
-import re
 
 try:
     import pandas as pd
@@ -24,8 +23,9 @@ except ImportError:
 
 from past.builtins import basestring
 
-from devlib import DerivedMeasurements, DerivedMetric, MeasurementsCsv, InstrumentChannel
+from devlib.derived import DerivedMeasurements, DerivedMetric
 from devlib.exception import HostError
+from devlib.instrument import MeasurementsCsv
 from devlib.utils.csvutil import csvwriter
 from devlib.utils.rendering import gfxinfo_get_last_dump, VSYNC_INTERVAL
 from devlib.utils.types import numeric
@@ -45,6 +45,7 @@ class DerivedFpsStats(DerivedMeasurements):
         if filename is not None and os.sep in filename:
             raise ValueError('filename cannot be a path (cannot countain "{}"'.format(os.sep))
 
+    # pylint: disable=no-member
     def process(self, measurements_csv):
         if isinstance(measurements_csv, basestring):
             measurements_csv = MeasurementsCsv(measurements_csv)
@@ -65,6 +66,7 @@ class DerivedFpsStats(DerivedMeasurements):
 
 class DerivedGfxInfoStats(DerivedFpsStats):
 
+    #pylint: disable=arguments-differ
     @staticmethod
     def process_raw(filepath, *args):
         metrics = []
@@ -104,17 +106,17 @@ class DerivedGfxInfoStats(DerivedFpsStats):
             frame_count += 1
 
             if start_vsync is None:
-                start_vsync = frame_data.Vsync_time_us
-            end_vsync = frame_data.Vsync_time_us
+                start_vsync = frame_data.Vsync_time_ns
+            end_vsync = frame_data.Vsync_time_ns
 
-            frame_time = frame_data.FrameCompleted_time_us - frame_data.IntendedVsync_time_us
+            frame_time = frame_data.FrameCompleted_time_ns - frame_data.IntendedVsync_time_ns
             pff = 1e9 / frame_time
             if pff > self.drop_threshold:
                 per_frame_fps.append([pff])
 
         if frame_count:
             duration = end_vsync - start_vsync
-            fps = (1e6 * frame_count) / float(duration)
+            fps = (1e9 * frame_count) / float(duration)
         else:
             duration = 0
             fps = 0
@@ -131,15 +133,15 @@ class DerivedGfxInfoStats(DerivedFpsStats):
     def _process_with_pandas(self, measurements_csv):
         data = pd.read_csv(measurements_csv.path)
         data = data[data.Flags_flags == 0]
-        frame_time = data.FrameCompleted_time_us - data.IntendedVsync_time_us
-        per_frame_fps = (1e6 / frame_time)
+        frame_time = data.FrameCompleted_time_ns - data.IntendedVsync_time_ns
+        per_frame_fps = (1e9 / frame_time)
         keep_filter = per_frame_fps > self.drop_threshold
         per_frame_fps = per_frame_fps[keep_filter]
         per_frame_fps.name = 'fps'
 
         frame_count = data.index.size
         if frame_count > 1:
-            duration = data.Vsync_time_us.iloc[-1] - data.Vsync_time_us.iloc[0]
+            duration = data.Vsync_time_ns.iloc[-1] - data.Vsync_time_ns.iloc[0]
             fps = (1e9 * frame_count) / float(duration)
         else:
             duration = 0
@@ -155,6 +157,7 @@ class DerivedGfxInfoStats(DerivedFpsStats):
 
 class DerivedSurfaceFlingerStats(DerivedFpsStats):
 
+    # pylint: disable=too-many-locals
     def _process_with_pandas(self, measurements_csv):
         data = pd.read_csv(measurements_csv.path)
 
@@ -193,7 +196,7 @@ class DerivedSurfaceFlingerStats(DerivedFpsStats):
             janks = 0
             not_at_vsync = 0
 
-        janks_pc =  0 if frame_count == 0 else janks * 100 / frame_count
+        janks_pc = 0 if frame_count == 0 else janks * 100 / frame_count
 
         return [DerivedMetric('fps', fps, 'fps'),
                 DerivedMetric('total_frames', frame_count, 'frames'),
@@ -202,6 +205,7 @@ class DerivedSurfaceFlingerStats(DerivedFpsStats):
                 DerivedMetric('janks_pc', janks_pc, 'percent'),
                 DerivedMetric('missed_vsync', not_at_vsync, 'count')]
 
+    # pylint: disable=unused-argument,no-self-use
     def _process_without_pandas(self, measurements_csv):
         # Given that SurfaceFlinger has been deprecated in favor of GfxInfo,
         # it does not seem worth it implementing this.

devlib/exception.py

@@ -22,12 +22,43 @@ class DevlibError(Exception):
         return str(self)
 
 
+class DevlibStableError(DevlibError):
+    """Non transient target errors, that are not subject to random variations
+    in the environment and can be reliably linked to for example a missing
+    feature on a target."""
+    pass
+
+
+class DevlibTransientError(DevlibError):
+    """Exceptions inheriting from ``DevlibTransientError`` represent random
+    transient events that are usually related to issues in the environment, as
+    opposed to programming errors, for example network failures or
+    timeout-related exceptions. When the error could come from
+    indistinguishable transient or non-transient issue, it can generally be
+    assumed that the configuration is correct and therefore, a transient
+    exception is raised."""
+    pass
+
+
 class TargetError(DevlibError):
     """An error has occured on the target"""
     pass
 
 
-class TargetNotRespondingError(DevlibError):
+class TargetTransientError(TargetError, DevlibTransientError):
+    """Transient target errors that can happen randomly when everything is
+    properly configured."""
+    pass
+
+
+class TargetStableError(TargetError, DevlibStableError):
+    """Non-transient target errors that can be linked to a programming error or
+    a configuration issue, and is not influenced by non-controllable parameters
+    such as network issues."""
+    pass
+
+
+class TargetNotRespondingError(TargetTransientError):
     """The target is unresponsive."""
     pass
 
@@ -37,7 +68,8 @@ class HostError(DevlibError):
     pass
 
 
-class TimeoutError(DevlibError):
+# pylint: disable=redefined-builtin
+class TimeoutError(DevlibTransientError):
     """Raised when a subprocess command times out. This is basically a ``DevlibError``-derived version
     of ``subprocess.CalledProcessError``, the thinking being that while a timeout could be due to
     programming error (e.g. not setting long enough timers), it is often due to some failure in the
@@ -73,13 +105,23 @@ class WorkerThreadError(DevlibError):
         super(WorkerThreadError, self).__init__(message)
 
 
+class KernelConfigKeyError(KeyError, IndexError, DevlibError):
+    """
+    Exception raised when a kernel config option cannot be found.
+
+    It inherits from :exc:`IndexError` for backward compatibility, and
+    :exc:`KeyError` to behave like a regular mapping.
+    """
+    pass
+
+
 def get_traceback(exc=None):
     """
     Returns the string with the traceback for the specifiec exc
     object, or for the current exception exc is not specified.
 
     """
-    import io, traceback, sys
+    import io, traceback, sys  # pylint: disable=multiple-imports
     if exc is None:
         exc = sys.exc_info()
     if not exc:

devlib/host.py

@@ -18,13 +18,16 @@ import signal
 import shutil
 import subprocess
 import logging
+from distutils.dir_util import copy_tree
 from getpass import getpass
+from pipes import quote
 
-from devlib.exception import TargetError
+from devlib.exception import TargetTransientError, TargetStableError
 from devlib.utils.misc import check_output
 
 PACKAGE_BIN_DIRECTORY = os.path.join(os.path.dirname(__file__), 'bin')
 
+# pylint: disable=redefined-outer-name
 def kill_children(pid, signal=signal.SIGKILL):
     with open('/proc/{0}/task/{0}/children'.format(pid), 'r') as fd:
         for cpid in map(int, fd.read().strip().split()):
@@ -35,6 +38,7 @@ class LocalConnection(object):
 
     name = 'local'
 
+    # pylint: disable=unused-argument
     def __init__(self, platform=None, keep_password=True, unrooted=False,
                  password=None, timeout=None):
         self.logger = logging.getLogger('local_connection')
@@ -53,30 +57,38 @@ class LocalConnection(object):
             for each_source in iglob(source):
                 shutil.copy(each_source, dest)
         else:
-            shutil.copy(source, dest)
+            if os.path.isdir(source):
+                # Use distutils to allow copying into an existing directory structure.
+                copy_tree(source, dest)
+            else:
+                shutil.copy(source, dest)
 
+    # pylint: disable=unused-argument
     def execute(self, command, timeout=None, check_exit_code=True,
-                as_root=False, strip_colors=True):
+                as_root=False, strip_colors=True, will_succeed=False):
         self.logger.debug(command)
         if as_root:
             if self.unrooted:
-                raise TargetError('unrooted')
+                raise TargetStableError('unrooted')
             password = self._get_password()
-            command = 'echo \'{}\' | sudo -S '.format(password) + command
+            command = 'echo {} | sudo -S '.format(quote(password)) + command
         ignore = None if check_exit_code else 'all'
         try:
             return check_output(command, shell=True, timeout=timeout, ignore=ignore)[0]
         except subprocess.CalledProcessError as e:
             message = 'Got exit code {}\nfrom: {}\nOUTPUT: {}'.format(
                 e.returncode, command, e.output)
-            raise TargetError(message)
+            if will_succeed:
+                raise TargetTransientError(message)
+            else:
+                raise TargetStableError(message)
 
     def background(self, command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, as_root=False):
         if as_root:
             if self.unrooted:
-                raise TargetError('unrooted')
+                raise TargetStableError('unrooted')
             password = self._get_password()
-            command = 'echo \'{}\' | sudo -S '.format(password) + command
+            command = 'echo {} | sudo -S '.format(quote(password)) + command
         return subprocess.Popen(command, stdout=stdout, stderr=stderr, shell=True)
 
     def close(self):

devlib/instrument/__init__.py

@@ -58,6 +58,7 @@ class MeasurementType(object):
             raise ValueError(msg.format(self.name, to.name))
         return self.conversions[to.name](value)
 
+    # pylint: disable=undefined-variable
     def __cmp__(self, other):
         if isinstance(other, MeasurementType):
             other = other.name
@@ -96,20 +97,30 @@ _measurement_types = [
     # covert without being familar with individual instruments.
     MeasurementType('time', 'seconds', 'time',
         conversions={
-            'time_us': lambda x: x * 1000000,
-            'time_ms': lambda x: x * 1000,
+            'time_us': lambda x: x * 1e6,
+            'time_ms': lambda x: x * 1e3,
+            'time_ns': lambda x: x * 1e9,
         }
     ),
     MeasurementType('time_us', 'microseconds', 'time',
         conversions={
-            'time': lambda x: x / 1000000,
-            'time_ms': lambda x: x / 1000,
+            'time': lambda x: x / 1e6,
+            'time_ms': lambda x: x / 1e3,
+            'time_ns': lambda x: x * 1e3,
         }
     ),
     MeasurementType('time_ms', 'milliseconds', 'time',
         conversions={
-            'time': lambda x: x / 1000,
-            'time_us': lambda x: x * 1000,
+            'time': lambda x: x / 1e3,
+            'time_us': lambda x: x * 1e3,
+            'time_ns': lambda x: x * 1e6,
+        }
+    ),
+    MeasurementType('time_ns', 'nanoseconds', 'time',
+    conversions={
+        'time': lambda x: x / 1e9,
+        'time_ms': lambda x: x / 1e6,
+        'time_us': lambda x: x / 1e3,
         }
     ),
 
@@ -151,6 +162,7 @@ class Measurement(object):
         self.value = value
         self.channel = channel
 
+    # pylint: disable=undefined-variable
     def __cmp__(self, other):
         if hasattr(other, 'value'):
             return cmp(self.value, other.value)
@@ -204,7 +216,7 @@ class MeasurementsCsv(object):
             for mt in MEASUREMENT_TYPES:
                 suffix = '_{}'.format(mt)
                 if entry.endswith(suffix):
-                    site =  entry[:-len(suffix)]
+                    site = entry[:-len(suffix)]
                     measure = mt
                     break
             else:
@@ -218,6 +230,7 @@ class MeasurementsCsv(object):
             chan = InstrumentChannel(site, measure)
             self.channels.append(chan)
 
+    # pylint: disable=stop-iteration-return
     def _iter_rows(self):
         with csvreader(self.path) as reader:
             next(reader)  # headings
@@ -308,16 +321,16 @@ class Instrument(object):
                 msg = 'Unexpected channel "{}"; must be in {}'
                 raise ValueError(msg.format(e, self.channels.keys()))
         elif sites is None and kinds is None:
-            self.active_channels = sorted(self.channels.itervalues(), key=lambda x: x.label)
+            self.active_channels = sorted(self.channels.values(), key=lambda x: x.label)
         else:
             if isinstance(sites, basestring):
                 sites = [sites]
             if isinstance(kinds, basestring):
                 kinds = [kinds]
 
-            wanted = lambda ch : ((kinds is None or ch.kind in kinds) and
+            wanted = lambda ch: ((kinds is None or ch.kind in kinds) and
                                   (sites is None or ch.site in sites))
-            self.active_channels = filter(wanted, self.channels.itervalues())
+            self.active_channels = list(filter(wanted, self.channels.values()))
 
     # instantaneous
 
@@ -332,6 +345,7 @@ class Instrument(object):
     def stop(self):
         pass
 
+    # pylint: disable=no-self-use
     def get_data(self, outfile):
         pass
 

devlib/instrument/acmecape.py

@@ -19,9 +19,11 @@ import os
 import sys
 import time
 import tempfile
+import shlex
 from fcntl import fcntl, F_GETFL, F_SETFL
 from string import Template
 from subprocess import Popen, PIPE, STDOUT
+from pipes import quote
 
 from devlib import Instrument, CONTINUOUS, MeasurementsCsv
 from devlib.exception import HostError
@@ -85,15 +87,17 @@ class AcmeCapeInstrument(Instrument):
         params = dict(
             iio_capture=self.iio_capture,
             host=self.host,
-            buffer_size=self.buffer_size,
+            # This must be a string for quote()
+            buffer_size=str(self.buffer_size),
             iio_device=self.iio_device,
             outfile=self.raw_data_file
         )
+        params = {k: quote(v) for k, v in params.items()}
         self.command = IIOCAP_CMD_TEMPLATE.substitute(**params)
         self.logger.debug('ACME cape command: {}'.format(self.command))
 
     def start(self):
-        self.process = Popen(self.command.split(), stdout=PIPE, stderr=STDOUT)
+        self.process = Popen(shlex.split(self.command), stdout=PIPE, stderr=STDOUT)
 
     def stop(self):
         self.process.terminate()
@@ -112,7 +116,7 @@ class AcmeCapeInstrument(Instrument):
                 raise HostError(msg.format(output))
         if self.process.returncode != 15: # iio-capture exits with 15 when killed
             if sys.version_info[0] == 3:
-                output += self.process.stdout.read().decode(sys.stdout.encoding, 'replace')
+                output += self.process.stdout.read().decode(sys.stdout.encoding or 'utf-8', 'replace')
             else:
                 output += self.process.stdout.read()
             self.logger.info('ACME instrument encountered an error, '

devlib/instrument/arm_energy_probe.py

@@ -34,8 +34,7 @@ from __future__ import division
 import os
 import subprocess
 import signal
-import struct
-import sys
+from pipes import quote
 
 import tempfile
 import shutil
@@ -99,7 +98,7 @@ class ArmEnergyProbeInstrument(Instrument):
         self.output_file_figure = os.path.join(self.output_directory, 'summary.txt')
         self.output_file_error = os.path.join(self.output_directory, 'error.log')
         self.output_fd_error = open(self.output_file_error, 'w')
-        self.command = 'arm-probe --config {} > {}'.format(self.config_file, self.output_file_raw)
+        self.command = 'arm-probe --config {} > {}'.format(quote(self.config_file), quote(self.output_file_raw))
 
     def start(self):
         self.logger.debug(self.command)
@@ -109,8 +108,8 @@ class ArmEnergyProbeInstrument(Instrument):
                                        shell=True)
 
     def stop(self):
-       self.logger.debug("kill running arm-probe")
-       os.killpg(self.armprobe.pid, signal.SIGTERM)
+        self.logger.debug("kill running arm-probe")
+        os.killpg(self.armprobe.pid, signal.SIGTERM)
 
     def get_data(self, outfile):  # pylint: disable=R0914
         self.logger.debug("Parse data and compute consumed energy")
@@ -133,7 +132,7 @@ class ArmEnergyProbeInstrument(Instrument):
                     if len(row) < len(active_channels):
                         continue
                     # all data are in micro (seconds/watt)
-                    new = [ float(row[i])/1000000 for i in active_indexes ]
+                    new = [float(row[i])/1000000 for i in active_indexes]
                     writer.writerow(new)
 
         self.output_fd_error.close()

devlib/instrument/baylibre_acme.py

@@ -0,0 +1,557 @@
+#pylint: disable=attribute-defined-outside-init
+
+import collections
+import functools
+import re
+import threading
+
+from past.builtins import basestring
+
+try:
+    import iio
+except ImportError as e:
+    iio_import_failed = True
+    iio_import_error  = e
+else:
+    iio_import_failed = False
+import numpy as np
+import pandas as pd
+
+from devlib import CONTINUOUS, Instrument, HostError, MeasurementsCsv, TargetError
+from devlib.utils.ssh import SshConnection
+
+class IIOINA226Channel(object):
+
+    def __init__(self, iio_channel):
+
+        channel_id   = iio_channel.id
+        channel_type = iio_channel.attrs['type'].value
+
+        re_measure = r'(?P<measure>\w+)(?P<index>\d*)$'
+        re_dtype = r'le:(?P<sign>\w)(?P<width>\d+)/(?P<size>\d+)>>(?P<align>\d+)'
+
+        match_measure = re.search(re_measure, channel_id)
+        match_dtype = re.search(re_dtype, channel_type)
+
+        if not match_measure:
+            msg = "IIO channel ID '{}' does not match expected RE '{}'"
+            raise ValueError(msg.format(channel_id, re_measure))
+
+        if not match_dtype:
+            msg = "'IIO channel type '{}' does not match expected RE '{}'"
+            raise ValueError(msg.format(channel_type, re_dtype))
+
+        self.measure = match_measure.group('measure')
+        self.iio_dtype = 'int{}'.format(match_dtype.group('width'))
+        self.iio_channel = iio_channel
+        # Data is reported in amps, volts, watts and microseconds:
+        self.iio_scale   = (1. if 'scale' not in iio_channel.attrs
+                            else float(iio_channel.attrs['scale'].value))
+        self.iio_scale /= 1000
+        # As calls to iio_store_buffer will be blocking and probably coming
+        # from a loop retrieving samples from the ACME, we want to provide
+        # consistency in processing timing between iterations i.e. we want
+        # iio_store_buffer to be o(1) for every call (can't have that with []):
+        self.sample_buffers = collections.deque()
+
+    def iio_store_buffer_samples(self, iio_buffer):
+        # IIO buffers receive and store their data as an interlaced array of
+        # samples from all the IIO channels of the IIO device. The IIO library
+        # provides a reliable function to extract the samples (bytes, actually)
+        # corresponding to a channel from the received buffer; in Python, it is
+        # iio.Channel.read(iio.Buffer).
+        #
+        # NB: As this is called in a potentially tightly timed loop, we do as
+        #     little work as possible:
+        self.sample_buffers.append(self.iio_channel.read(iio_buffer))
+
+    def iio_get_samples(self, absolute_timestamps=False):
+        # Up to this point, the data is not interpreted yet i.e. these are
+        # bytearrays. Hence the use of np.dtypes.
+        buffers = [np.frombuffer(b, dtype=self.iio_dtype)
+                   for b in self.sample_buffers]
+
+        must_shift = (self.measure == 'timestamp' and not absolute_timestamps)
+        samples = np.concatenate(buffers)
+        return (samples - samples[0] if must_shift else samples) * self.iio_scale
+
+    def iio_forget_samples(self):
+        self.sample_buffers.clear()
+
+
+# Decorators for the attributes of IIOINA226Instrument:
+
+def only_set_to(valid_values, dynamic=False):
+    def validating_wrapper(func):
+        @functools.wraps(func)
+        def wrapper(self, value):
+            values = (valid_values if not dynamic
+                      else getattr(self, valid_values))
+            if value not in values: 
+                msg = '{} is invalid; expected values are {}'
+                raise ValueError(msg.format(value, valid_values))
+            return func(self, value)
+        return wrapper
+    return validating_wrapper
+
+
+def with_input_as(wanted_type):
+    def typecasting_wrapper(func):
+        @functools.wraps(func)
+        def wrapper(self, value):
+            return func(self, wanted_type(value))
+        return wrapper
+    return typecasting_wrapper
+
+
+def _IIODeviceAttr(attr_name, attr_type, writable=False, dyn_vals=None, stat_vals=None):
+
+    def getter(self):
+        return attr_type(self.iio_device.attrs[attr_name].value)
+
+    def setter(self, value):
+        self.iio_device.attrs[attr_name].value = str(attr_type(value))
+
+    if writable and (dyn_vals or stat_vals):
+        vals, dyn = dyn_vals or stat_vals, dyn_vals is not None
+        setter = with_input_as(attr_type)(only_set_to(vals, dyn)(setter))
+
+    return property(getter, setter if writable else None)
+
+
+def _IIOChannelIntTime(chan_name):
+
+    attr_name, attr_type = 'integration_time', float
+
+    def getter(self):
+        ch = self.iio_device.find_channel(chan_name)
+        return attr_type(ch.attrs[attr_name].value)
+
+    @only_set_to('INTEGRATION_TIMES_AVAILABLE', dynamic=True)
+    @with_input_as(attr_type)
+    def setter(self, value):
+        ch = self.iio_device.find_channel(chan_name)
+        ch.attrs[attr_name].value = str(value)
+
+    return property(getter, setter)
+
+
+def _setify(x):
+    return {x} if isinstance(x, basestring) else set(x) #Py3: basestring->str
+
+
+class IIOINA226Instrument(object):
+
+    IIO_DEVICE_NAME = 'ina226'
+
+    def __init__(self, iio_device):
+
+        if iio_device.name != self.IIO_DEVICE_NAME:
+            msg = 'IIO device is {}; expected {}'
+            raise TargetError(msg.format(iio_device.name, self.IIO_DEVICE_NAME))
+
+        self.iio_device           = iio_device
+        self.absolute_timestamps  = False
+        self.high_resolution      = True
+        self.buffer_samples_count = None
+        self.buffer_is_circular   = False
+
+        self.collector = None
+        self.work_done = threading.Event()
+        self.collector_exception = None
+
+        self.data = collections.OrderedDict()
+
+        channels = {
+                'timestamp': 'timestamp',
+                'shunt'    : 'voltage0',
+                'voltage'  : 'voltage1', # bus
+                'power'    : 'power2',
+                'current'  : 'current3',
+        }
+        self.computable_channels = {'current' : {'shunt'},
+                                    'power'   : {'shunt', 'voltage'}}
+        self.uncomputable_channels = set(channels) - set(self.computable_channels)
+        self.channels = {k: IIOINA226Channel(self.iio_device.find_channel(v))
+                         for k, v in channels.items()}
+        # We distinguish between "output" channels (as seen by the user of this
+        # class) and "hardware" channels (as requested from the INA226).
+        # This is necessary because of the 'high_resolution' feature which
+        # requires outputting computed channels:
+        self.active_channels = set()  # "hardware" channels
+        self.wanted_channels = set()  # "output" channels
+
+
+    # Properties
+
+    OVERSAMPLING_RATIOS_AVAILABLE = (1, 4, 16, 64, 128, 256, 512, 1024)
+    INTEGRATION_TIMES_AVAILABLE = _IIODeviceAttr('integration_time_available',
+                                                 lambda x: tuple(map(float, x.split())))
+
+    sample_rate_hz     = _IIODeviceAttr('in_sampling_frequency', int)
+    shunt_resistor     = _IIODeviceAttr('in_shunt_resistor'    , int, True)
+    oversampling_ratio = _IIODeviceAttr('in_oversampling_ratio', int, True,
+                                        dyn_vals='OVERSAMPLING_RATIOS_AVAILABLE')
+
+    integration_time_shunt = _IIOChannelIntTime('voltage0')
+    integration_time_bus   = _IIOChannelIntTime('voltage1')
+
+    def list_channels(self):
+        return self.channels.keys()
+
+    def activate(self, channels=None):
+        all_channels = set(self.channels)
+        requested_channels = (all_channels if channels is None
+                              else _setify(channels))
+
+        unknown = ', '.join(requested_channels - all_channels)
+        if unknown:
+            raise ValueError('Unknown channel(s): {}'.format(unknown))
+
+        self.wanted_channels |= requested_channels
+
+    def deactivate(self, channels=None):
+        unwanted_channels = (self.wanted_channels if channels is None
+                             else _setify(channels))
+
+        unknown = ', '.join(unwanted_channels - set(self.channels))
+        if unknown:
+            raise ValueError('Unknown channel(s): {}'.format(unknown))
+
+        unactive = ', '.join(unwanted_channels - self.wanted_channels)
+        if unactive:
+            raise ValueError('Already unactive channel(s): {}'.format(unactive))
+
+        self.wanted_channels -= unwanted_channels
+
+    def sample_collector(self):
+        class Collector(threading.Thread):
+            def run(collector_self):
+                for name, ch in self.channels.items():
+                    ch.iio_channel.enabled = (name in self.active_channels)
+
+                samples_count = self.buffer_samples_count or self.sample_rate_hz
+
+                iio_buffer = iio.Buffer(self.iio_device, samples_count,
+                                        self.buffer_is_circular)
+                # NB: This buffer creates a communication pipe to the
+                #     BeagleBone (or is it between the BBB and the ACME?)
+                #     that locks down any configuration. The IIO drivers
+                #     do not limit access when a buffer exists so that
+                #     configuring the INA226 (i.e. accessing iio.Device.attrs
+                #     or iio.Channel.attrs from iio.Device.channels i.e.
+                #     assigning to or reading from any property of this class
+                #     or calling its setup or reset methods) will screw up the
+                #     whole system and will require rebooting the BBB-ACME board!
+
+                self.collector_exception = None
+                try:
+                    refilled_once = False
+                    while not (refilled_once and self.work_done.is_set()):
+                        refilled_once = True
+                        iio_buffer.refill()
+                        for name in self.active_channels:
+                            self.channels[name].iio_store_buffer_samples(iio_buffer)
+                except Exception as e:
+                    self.collector_exception = e
+                finally:
+                    del iio_buffer
+                    for ch in self.channels.values():
+                        ch.enabled = False
+
+        return Collector()
+
+    def start_capturing(self):
+        if not self.wanted_channels:
+            raise TargetError('No active channel: aborting.')
+
+        self.active_channels = self.wanted_channels.copy()
+        if self.high_resolution:
+            self.active_channels &= self.uncomputable_channels
+            for channel, dependencies in self.computable_channels.items():
+                if channel in self.wanted_channels:
+                    self.active_channels |= dependencies
+
+        self.work_done.clear()
+        self.collector = self.sample_collector()
+        self.collector.daemon = True
+        self.collector.start()
+
+    def stop_capturing(self):
+        self.work_done.set()
+        self.collector.join()
+
+        if self.collector_exception:
+            raise self.collector_exception
+
+        self.data.clear()
+        for channel in self.active_channels:
+            ch = self.channels[channel]
+            self.data[channel] = ch.iio_get_samples(self.absolute_timestamps)
+            ch.iio_forget_samples()
+
+        if self.high_resolution:
+            res_ohm = 1e-6 * self.shunt_resistor
+            current = self.data['shunt'] / res_ohm
+            if 'current' in self.wanted_channels:
+                self.data['current'] = current
+            if 'power' in self.wanted_channels:
+                self.data['power'] = current * self.data['voltage']
+            for channel in set(self.data) - self.wanted_channels:
+                del self.data[channel]
+
+        self.active_channels.clear()
+
+    def get_data(self):
+        return self.data
+
+
+class BaylibreAcmeInstrument(Instrument):
+
+    mode = CONTINUOUS
+
+    MINIMAL_ACME_SD_IMAGE_VERSION    = (2, 1, 3)
+    MINIMAL_ACME_IIO_DRIVERS_VERSION = (0, 6)
+    MINIMAL_HOST_IIO_DRIVERS_VERSION = (0, 15)
+
+    def __init__(self, target=None, iio_context=None,
+                 use_base_iio_context=False, probe_names=None):
+
+        if iio_import_failed:
+            raise HostError('Could not import "iio": {}'.format(iio_import_error))
+
+        super(BaylibreAcmeInstrument, self).__init__(target)
+
+        if isinstance(probe_names, basestring):
+            probe_names = [probe_names]
+
+        self.iio_context = (iio_context if not use_base_iio_context
+                            else iio.Context(iio_context))
+
+        self.check_version()
+
+        if probe_names is not None:
+            if len(probe_names) != len(set(probe_names)):
+                msg = 'Probe names should be unique: {}'
+                raise ValueError(msg.format(probe_names))
+
+            if len(probe_names) != len(self.iio_context.devices):
+                msg = ('There should be as many probe_names ({}) '
+                       'as detected probes ({}).')
+                raise ValueError(msg.format(len(probe_names),
+                                            len(self.iio_context.devices)))
+
+        probes = [IIOINA226Instrument(d) for d in self.iio_context.devices]
+
+        self.probes = (dict(zip(probe_names, probes)) if probe_names
+                       else {p.iio_device.id : p for p in probes})
+        self.active_probes = set()
+
+        for probe in self.probes:
+            for measure in ['voltage', 'power', 'current']:
+                self.add_channel(site=probe, measure=measure)
+        self.add_channel('timestamp', 'time_us')
+
+        self.data = pd.DataFrame()
+
+    def check_version(self):
+        msg = ('The IIO drivers running on {} ({}) are out-of-date; '
+               'devlib requires {} or later.')
+
+        if iio.version[:2] < self.MINIMAL_HOST_IIO_DRIVERS_VERSION:
+            ver_str = '.'.join(map(str, iio.version[:2]))
+            min_str = '.'.join(map(str, self.MINIMAL_HOST_IIO_DRIVERS_VERSION))
+            raise HostError(msg.format('this host', ver_str, min_str))
+
+        if self.version[:2] < self.MINIMAL_ACME_IIO_DRIVERS_VERSION:
+            ver_str = '.'.join(map(str, self.version[:2]))
+            min_str = '.'.join(map(str, self.MINIMAL_ACME_IIO_DRIVERS_VERSION))
+            raise TargetError(msg.format('the BBB', ver_str, min_str))
+
+    # properties
+
+    def probes_unique_property(self, property_name):
+        probes = self.active_probes or self.probes
+        try:
+            # This will fail if there is not exactly one single value:
+            (value,) = {getattr(self.probes[p], property_name) for p in probes}
+        except ValueError:
+            msg = 'Probes have different values for {}.'
+            raise ValueError(msg.format(property_name) if probes else 'No probe')
+        return value
+
+    @property
+    def version(self):
+        return self.iio_context.version
+
+    @property
+    def OVERSAMPLING_RATIOS_AVAILABLE(self):
+        return self.probes_unique_property('OVERSAMPLING_RATIOS_AVAILABLE')
+
+    @property
+    def INTEGRATION_TIMES_AVAILABLE(self):
+        return self.probes_unique_property('INTEGRATION_TIMES_AVAILABLE')
+
+    @property
+    def sample_rate_hz(self):
+        return self.probes_unique_property('sample_rate_hz')
+
+    @sample_rate_hz.setter
+    # This setter is required for compliance with the inherited methods
+    def sample_rate_hz(self, value):
+        if value is not None:
+            raise AttributeError("can't set attribute")
+
+    # initialization and teardown
+
+    def setup(self, shunt_resistor,
+              integration_time_bus,
+              integration_time_shunt,
+              oversampling_ratio,
+              buffer_samples_count=None,
+              buffer_is_circular=False,
+              absolute_timestamps=False,
+              high_resolution=True):
+
+        def pseudo_list(v, i):
+            try:
+                return v[i]
+            except TypeError:
+                return v
+
+        for i, p in enumerate(self.probes.values()):
+            for attr, val in locals().items():
+                if attr != 'self':
+                    setattr(p, attr, pseudo_list(val, i))
+
+        self.absolute_timestamps = all(pseudo_list(absolute_timestamps, i)
+                                       for i in range(len(self.probes)))
+
+    def reset(self, sites=None, kinds=None, channels=None):
+
+        # populate self.active_channels:
+        super(BaylibreAcmeInstrument, self).reset(sites, kinds, channels)
+
+        for ch in self.active_channels:
+            if ch.site != 'timestamp':
+                self.probes[ch.site].activate(['timestamp', ch.kind])
+                self.active_probes.add(ch.site)
+
+    def teardown(self):
+        del self.active_channels[:]
+        self.active_probes.clear()
+
+    def start(self):
+        for p in self.active_probes:
+            self.probes[p].start_capturing()
+
+    def stop(self):
+        for p in self.active_probes:
+            self.probes[p].stop_capturing()
+
+        max_rate_probe = max(self.active_probes,
+                             key=lambda p: self.probes[p].sample_rate_hz)
+
+        probes_dataframes = {
+                probe: pd.DataFrame.from_dict(self.probes[probe].get_data())
+                                   .set_index('timestamp')
+                for probe in self.active_probes
+        }
+
+        for df in probes_dataframes.values():
+            df.set_index(pd.to_datetime(df.index, unit='us'), inplace=True)
+
+        final_index = probes_dataframes[max_rate_probe].index
+
+        df = pd.concat(probes_dataframes, axis=1).sort_index()
+        df.columns = ['_'.join(c).strip() for c in df.columns.values]
+
+        self.data = df.interpolate('time').reindex(final_index)
+
+        if not self.absolute_timestamps:
+            epoch_index = self.data.index.astype(np.int64) // 1000
+            self.data.set_index(epoch_index, inplace=True)
+            # self.data.index is in [us]
+            # columns are in volts, amps and watts
+
+    def get_data(self, outfile=None, **to_csv_kwargs):
+        if outfile is None:
+            return self.data
+
+        self.data.to_csv(outfile, **to_csv_kwargs)
+        return MeasurementsCsv(outfile, self.active_channels)
+
+class BaylibreAcmeLocalInstrument(BaylibreAcmeInstrument):
+
+    def __init__(self, target=None, probe_names=None):
+
+        if iio_import_failed:
+            raise HostError('Could not import "iio": {}'.format(iio_import_error))
+
+        super(BaylibreAcmeLocalInstrument, self).__init__(
+                        target=target,
+                        iio_context=iio.LocalContext(),
+                        probe_names=probe_names
+        )
+
+class BaylibreAcmeXMLInstrument(BaylibreAcmeInstrument):
+
+    def __init__(self, target=None, xmlfile=None, probe_names=None):
+
+        if iio_import_failed:
+            raise HostError('Could not import "iio": {}'.format(iio_import_error))
+
+        super(BaylibreAcmeXMLInstrument, self).__init__(
+                        target=target,
+                        iio_context=iio.XMLContext(xmlfile),
+                        probe_names=probe_names
+        )
+
+class BaylibreAcmeNetworkInstrument(BaylibreAcmeInstrument):
+
+    def __init__(self, target=None, hostname=None, probe_names=None):
+
+        if iio_import_failed:
+            raise HostError('Could not import "iio": {}'.format(iio_import_error))
+
+        super(BaylibreAcmeNetworkInstrument, self).__init__(
+                        target=target,
+                        iio_context=iio.NetworkContext(hostname),
+                        probe_names=probe_names
+        )
+
+        try:
+            self.ssh_connection = SshConnection(hostname, username='root', password=None)
+        except TargetError as e:
+            msg = 'No SSH connexion could be established to {}: {}'
+            self.logger.debug(msg.format(hostname, e))
+            self.ssh_connection = None
+
+    def check_version(self):
+        super(BaylibreAcmeNetworkInstrument, self).check_version()
+
+        cmd = r"""sed -nr 's/^VERSION_ID="(.+)"$/\1/p' < /etc/os-release"""
+        try:
+            ver_str = self._ssh(cmd).rstrip()
+            ver = tuple(map(int, ver_str.split('.')))
+        except Exception as e:
+            self.logger.debug('Unable to verify ACME SD image version through SSH: {}'.format(e))
+        else:
+            if ver < self.MINIMAL_ACME_SD_IMAGE_VERSION:
+                min_str = '.'.join(map(str, self.MINIMAL_ACME_SD_IMAGE_VERSION))
+                msg = ('The ACME SD image for the BBB (ver. {}) is out-of-date; '
+                       'devlib requires {} or later.')
+                raise TargetError(msg.format(ver_str, min_str))
+
+    def _ssh(self, cmd=''):
+        """Connections are assumed to be rare."""
+        if self.ssh_connection is None:
+            raise TargetError('No SSH connection; see log.')
+        return self.ssh_connection.execute(cmd)
+
+    def _reboot(self):
+        """Always delete the object after calling its _reboot method"""
+        try:
+            self._ssh('reboot')
+        except:
+            pass

devlib/instrument/daq.py

@@ -60,7 +60,8 @@ class DaqInstrument(Instrument):
         result = self.execute('list_devices')
         if result.status == Status.OK:
             if device_id not in result.data:
-                raise ValueError('Device "{}" is not found on the DAQ server.'.format(device_id))
+                msg = 'Device "{}" is not found on the DAQ server. Available devices are: "{}"'
+                raise ValueError(msg.format(device_id, ', '.join(result.data)))
         elif result.status != Status.OKISH:
             raise HostError('Problem querying DAQ server: {}'.format(result.message))
 
@@ -156,4 +157,3 @@ class DaqInstrument(Instrument):
 
     def execute(self, command, **kwargs):
         return execute_command(self.server_config, command, **kwargs)
-

devlib/instrument/energy_probe.py

@@ -19,6 +19,7 @@ import tempfile
 import struct
 import subprocess
 import sys
+from pipes import quote
 
 from devlib.instrument import Instrument, CONTINUOUS, MeasurementsCsv
 from devlib.exception import HostError
@@ -65,7 +66,10 @@ class EnergyProbeInstrument(Instrument):
         parts = ['-r {}:{} '.format(i, int(1000 * rval))
                  for i, rval in enumerate(self.resistor_values)]
         rstring = ''.join(parts)
-        self.command = '{} -d {} -l {} {}'.format(self.caiman, self.device_entry, rstring, self.raw_output_directory)
+        self.command = '{} -d {} -l {} {}'.format(
+            quote(self.caiman), quote(self.device_entry),
+            rstring, quote(self.raw_output_directory)
+        )
         self.raw_data_file = None
 
     def start(self):
@@ -82,8 +86,8 @@ class EnergyProbeInstrument(Instrument):
         if self.process.returncode is not None:
             stdout, stderr = self.process.communicate()
             if sys.version_info[0] == 3:
-                stdout = stdout.decode(sys.stdout.encoding, 'replace')
-                stderr = stderr.decode(sys.stdout.encoding, 'replace')
+                stdout = stdout.decode(sys.stdout.encoding or 'utf-8', 'replace')
+                stderr = stderr.decode(sys.stdout.encoding or 'utf-8', 'replace')
             raise HostError(
                 'Energy Probe: Caiman exited unexpectedly with exit code {}.\n'
                 'stdout:\n{}\nstderr:\n{}'.format(self.process.returncode,
@@ -114,7 +118,7 @@ class EnergyProbeInstrument(Instrument):
                         writer.writerow(row)
                     except struct.error:
                         if not_a_full_row_seen:
-                            self.logger.warn('possibly missaligned caiman raw data, row contained {} bytes'.format(len(data)))
+                            self.logger.warning('possibly missaligned caiman raw data, row contained {} bytes'.format(len(data)))
                             continue
                         else:
                             not_a_full_row_seen = True

devlib/instrument/frames.py

@@ -41,6 +41,7 @@ class FramesInstrument(Instrument):
 
     def reset(self, sites=None, kinds=None, channels=None):
         super(FramesInstrument, self).reset(sites, kinds, channels)
+        # pylint: disable=not-callable
         self.collector = self.collector_cls(self.target, self.period,
                                             self.collector_target, self.header)
         self._need_reset = False
@@ -81,7 +82,7 @@ class GfxInfoFramesInstrument(FramesInstrument):
             if entry == 'Flags':
                 self.add_channel('Flags', MeasurementType('flags', 'flags'))
             else:
-                self.add_channel(entry, 'time_us')
+                self.add_channel(entry, 'time_ns')
         self.header = [chan.label for chan in self.channels.values()]
 
 

devlib/instrument/gem5power.py

@@ -13,11 +13,10 @@
 # limitations under the License.
 
 from __future__ import division
-import re
 
 from devlib.platform.gem5 import Gem5SimulationPlatform
 from devlib.instrument import Instrument, CONTINUOUS, MeasurementsCsv
-from devlib.exception import TargetError, HostError
+from devlib.exception import TargetStableError
 from devlib.utils.csvutil import csvwriter
 
 
@@ -37,9 +36,9 @@ class Gem5PowerInstrument(Instrument):
             system.cluster0.cores0.power_model.static_power
         '''
         if not isinstance(target.platform, Gem5SimulationPlatform):
-            raise TargetError('Gem5PowerInstrument requires a gem5 platform')
+            raise TargetStableError('Gem5PowerInstrument requires a gem5 platform')
         if not target.has('gem5stats'):
-            raise TargetError('Gem5StatsModule is not loaded')
+            raise TargetStableError('Gem5StatsModule is not loaded')
         super(Gem5PowerInstrument, self).__init__(target)
 
         # power_sites is assumed to be a list later
@@ -69,7 +68,7 @@ class Gem5PowerInstrument(Instrument):
         with csvwriter(outfile) as writer:
             writer.writerow([c.label for c in self.active_channels]) # headers
             sites_to_match = [self.site_mapping.get(s, s) for s in active_sites]
-            for rec, rois in self.target.gem5stats.match_iter(sites_to_match,
+            for rec, _ in self.target.gem5stats.match_iter(sites_to_match,
                     [self.roi_label], self._base_stats_dump):
                 writer.writerow([rec[s] for s in sites_to_match])
         return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz)
@@ -77,4 +76,3 @@ class Gem5PowerInstrument(Instrument):
     def reset(self, sites=None, kinds=None, channels=None):
         super(Gem5PowerInstrument, self).reset(sites, kinds, channels)
         self._base_stats_dump = self.target.gem5stats.next_dump_no()
-

devlib/instrument/hwmon.py

@@ -16,7 +16,7 @@ from __future__ import division
 import re
 
 from devlib.instrument import Instrument, Measurement, INSTANTANEOUS
-from devlib.exception import TargetError
+from devlib.exception import TargetStableError
 
 
 class HwmonInstrument(Instrument):
@@ -35,7 +35,7 @@ class HwmonInstrument(Instrument):
 
     def __init__(self, target):
         if not hasattr(target, 'hwmon'):
-            raise TargetError('Target does not support HWMON')
+            raise TargetStableError('Target does not support HWMON')
         super(HwmonInstrument, self).__init__(target)
 
         self.logger.debug('Discovering available HWMON sensors...')

devlib/instrument/monsoon.py

@@ -21,12 +21,11 @@ from tempfile import NamedTemporaryFile
 
 from devlib.instrument import Instrument, CONTINUOUS, MeasurementsCsv
 from devlib.exception import HostError
-from devlib.host import PACKAGE_BIN_DIRECTORY
 from devlib.utils.csvutil import csvwriter
 from devlib.utils.misc import which
 
 
-INSTALL_INSTRUCTIONS="""
+INSTALL_INSTRUCTIONS = """
 MonsoonInstrument requires the monsoon.py tool, available from AOSP:
 
 https://android.googlesource.com/platform/cts/+/master/tools/utils/monsoon.py
@@ -68,6 +67,7 @@ class MonsoonInstrument(Instrument):
 
         self.process = None
         self.output = None
+        self.buffer_file = None
 
         self.sample_rate_hz = 500
         self.add_channel('output', 'power')
@@ -101,8 +101,8 @@ class MonsoonInstrument(Instrument):
         if process.returncode is not None:
             stdout, stderr = process.communicate()
             if sys.version_info[0] == 3:
-                stdout = stdout.encode(sys.stdout.encoding)
-                stderr = stderr.encode(sys.stdout.encoding)
+                stdout = stdout.encode(sys.stdout.encoding or 'utf-8')
+                stderr = stderr.encode(sys.stdout.encoding or 'utf-8')
             raise HostError(
                 'Monsoon script exited unexpectedly with exit code {}.\n'
                 'stdout:\n{}\nstderr:\n{}'.format(process.returncode,
@@ -110,7 +110,7 @@ class MonsoonInstrument(Instrument):
 
         process.send_signal(signal.SIGINT)
 
-        stderr =  process.stderr.read()
+        stderr = process.stderr.read()
 
         self.buffer_file.close()
         with open(self.buffer_file.name) as f:
@@ -124,7 +124,7 @@ class MonsoonInstrument(Instrument):
         if self.process:
             raise RuntimeError('`get_data` called before `stop`')
 
-        stdout, stderr = self.output
+        stdout, _ = self.output
 
         with csvwriter(outfile) as writer:
             active_sites = [c.site for c in self.active_channels]

devlib/instrument/netstats/__init__.py

@@ -22,7 +22,7 @@ from collections import defaultdict
 from future.moves.itertools import zip_longest
 
 from devlib.instrument import Instrument, MeasurementsCsv, CONTINUOUS
-from devlib.exception import TargetError, HostError
+from devlib.exception import TargetStableError, HostError
 from devlib.utils.android import ApkInfo
 from devlib.utils.csvutil import csvwriter
 
@@ -84,7 +84,7 @@ class NetstatsInstrument(Instrument):
 
         """
         if target.os != 'android':
-            raise TargetError('netstats insturment only supports Android targets')
+            raise TargetStableError('netstats instrument only supports Android targets')
         if apk is None:
             apk = os.path.join(THIS_DIR, 'netstats.apk')
         if not os.path.isfile(apk):
@@ -101,6 +101,7 @@ class NetstatsInstrument(Instrument):
             self.add_channel(package, 'tx')
             self.add_channel(package, 'rx')
 
+    # pylint: disable=keyword-arg-before-vararg,arguments-differ
     def setup(self, force=False, *args, **kwargs):
         if self.target.package_is_installed(self.package):
             if force:

devlib/module/__init__.py

@@ -37,6 +37,9 @@ class Module(object):
     #             serial).
     #  'connected' -- installed when a connection to to the target has been
     #                 established. This is the default.
+    #   'setup' -- installed after initial setup of the device has been performed.
+    #              This allows the module to utilize assets deployed during the
+    #              setup stage for example 'Busybox'.
     stage = 'connected'
 
     @staticmethod
@@ -61,7 +64,7 @@ class Module(object):
         self.logger = logging.getLogger(self.name)
 
 
-class HardRestModule(Module):  # pylint: disable=R0921
+class HardRestModule(Module):
 
     kind = 'hard_reset'
 
@@ -69,7 +72,7 @@ class HardRestModule(Module):  # pylint: disable=R0921
         raise NotImplementedError()
 
 
-class BootModule(Module):  # pylint: disable=R0921
+class BootModule(Module):
 
     kind = 'boot'
 

devlib/module/android.py

@@ -125,4 +125,3 @@ def get_mapping(base_dir, partition_file):
                 HostError('file {} was not found in the bundle or was misplaced'.format(pair[1]))
             mapping[pair[0]] = image_path
     return mapping
-

devlib/module/biglittle.py

@@ -60,150 +60,150 @@ class BigLittleModule(Module):
 
     def list_bigs_frequencies(self):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             return self.target.cpufreq.list_frequencies(bigs_online[0])
 
     def list_bigs_governors(self):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             return self.target.cpufreq.list_governors(bigs_online[0])
 
     def list_bigs_governor_tunables(self):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             return self.target.cpufreq.list_governor_tunables(bigs_online[0])
 
     def list_littles_frequencies(self):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             return self.target.cpufreq.list_frequencies(littles_online[0])
 
     def list_littles_governors(self):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             return self.target.cpufreq.list_governors(littles_online[0])
 
     def list_littles_governor_tunables(self):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             return self.target.cpufreq.list_governor_tunables(littles_online[0])
 
     def get_bigs_governor(self):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             return self.target.cpufreq.get_governor(bigs_online[0])
 
     def get_bigs_governor_tunables(self):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             return self.target.cpufreq.get_governor_tunables(bigs_online[0])
 
     def get_bigs_frequency(self):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             return self.target.cpufreq.get_frequency(bigs_online[0])
 
     def get_bigs_min_frequency(self):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             return self.target.cpufreq.get_min_frequency(bigs_online[0])
 
     def get_bigs_max_frequency(self):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             return self.target.cpufreq.get_max_frequency(bigs_online[0])
 
     def get_littles_governor(self):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             return self.target.cpufreq.get_governor(littles_online[0])
 
     def get_littles_governor_tunables(self):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             return self.target.cpufreq.get_governor_tunables(littles_online[0])
 
     def get_littles_frequency(self):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             return self.target.cpufreq.get_frequency(littles_online[0])
 
     def get_littles_min_frequency(self):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             return self.target.cpufreq.get_min_frequency(littles_online[0])
 
     def get_littles_max_frequency(self):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             return self.target.cpufreq.get_max_frequency(littles_online[0])
 
     def set_bigs_governor(self, governor, **kwargs):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             self.target.cpufreq.set_governor(bigs_online[0], governor, **kwargs)
         else:
             raise ValueError("All bigs appear to be offline")
 
     def set_bigs_governor_tunables(self, governor, **kwargs):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             self.target.cpufreq.set_governor_tunables(bigs_online[0], governor, **kwargs)
         else:
             raise ValueError("All bigs appear to be offline")
 
     def set_bigs_frequency(self, frequency, exact=True):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             self.target.cpufreq.set_frequency(bigs_online[0], frequency, exact)
         else:
             raise ValueError("All bigs appear to be offline")
 
     def set_bigs_min_frequency(self, frequency, exact=True):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             self.target.cpufreq.set_min_frequency(bigs_online[0], frequency, exact)
         else:
             raise ValueError("All bigs appear to be offline")
 
     def set_bigs_max_frequency(self, frequency, exact=True):
         bigs_online = self.bigs_online
-        if len(bigs_online) > 0:
+        if bigs_online:
             self.target.cpufreq.set_max_frequency(bigs_online[0], frequency, exact)
         else:
             raise ValueError("All bigs appear to be offline")
 
     def set_littles_governor(self, governor, **kwargs):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             self.target.cpufreq.set_governor(littles_online[0], governor, **kwargs)
         else:
             raise ValueError("All littles appear to be offline")
 
     def set_littles_governor_tunables(self, governor, **kwargs):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             self.target.cpufreq.set_governor_tunables(littles_online[0], governor, **kwargs)
         else:
             raise ValueError("All littles appear to be offline")
 
     def set_littles_frequency(self, frequency, exact=True):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             self.target.cpufreq.set_frequency(littles_online[0], frequency, exact)
         else:
             raise ValueError("All littles appear to be offline")
 
     def set_littles_min_frequency(self, frequency, exact=True):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             self.target.cpufreq.set_min_frequency(littles_online[0], frequency, exact)
         else:
             raise ValueError("All littles appear to be offline")
 
     def set_littles_max_frequency(self, frequency, exact=True):
         littles_online = self.littles_online
-        if len(littles_online) > 0:
+        if littles_online:
             self.target.cpufreq.set_max_frequency(littles_online[0], frequency, exact)
         else:
             raise ValueError("All littles appear to be offline")

devlib/module/cgroups.py

@@ -18,7 +18,7 @@ import re
 from collections import namedtuple
 
 from devlib.module import Module
-from devlib.exception import TargetError
+from devlib.exception import TargetStableError
 from devlib.utils.misc import list_to_ranges, isiterable
 from devlib.utils.types import boolean
 
@@ -121,18 +121,20 @@ class Controller(object):
             cgroups.append(cg)
         return cgroups
 
-    def move_tasks(self, source, dest, exclude=[]):
+    def move_tasks(self, source, dest, exclude=None):
+        if exclude is None:
+            exclude = []
         try:
             srcg = self._cgroups[source]
             dstg = self._cgroups[dest]
         except KeyError as e:
-            raise ValueError('Unkown group: {}'.format(e))
-        output = self.target._execute_util(
+            raise ValueError('Unknown group: {}'.format(e))
+        self.target._execute_util(  # pylint: disable=protected-access
                     'cgroups_tasks_move {} {} \'{}\''.format(
                     srcg.directory, dstg.directory, exclude),
                     as_root=True)
 
-    def move_all_tasks_to(self, dest, exclude=[]):
+    def move_all_tasks_to(self, dest, exclude=None):
         """
         Move all the tasks to the specified CGroup
 
@@ -145,8 +147,10 @@ class Controller(object):
         tasks.
 
         :param exclude: list of commands to keep in the root CGroup
-        :type exlude: list(str)
+        :type exclude: list(str)
         """
+        if exclude is None:
+            exclude = []
 
         if isinstance(exclude, str):
             exclude = [exclude]
@@ -169,6 +173,7 @@ class Controller(object):
             if cgroup != dest:
                 self.move_tasks(cgroup, dest, grep_filters)
 
+    # pylint: disable=too-many-locals
     def tasks(self, cgroup,
               filter_tid='',
               filter_tname='',
@@ -203,8 +208,8 @@ class Controller(object):
         try:
             cg = self._cgroups[cgroup]
         except KeyError as e:
-            raise ValueError('Unkown group: {}'.format(e))
-        output = self.target._execute_util(
+            raise ValueError('Unknown group: {}'.format(e))
+        output = self.target._execute_util(  # pylint: disable=protected-access
                     'cgroups_tasks_in {}'.format(cg.directory),
                     as_root=True)
         entries = output.splitlines()
@@ -234,7 +239,7 @@ class Controller(object):
         try:
             cg = self._cgroups[cgroup]
         except KeyError as e:
-            raise ValueError('Unkown group: {}'.format(e))
+            raise ValueError('Unknown group: {}'.format(e))
         output = self.target.execute(
                     '{} wc -l {}/tasks'.format(
                     self.target.busybox, cg.directory),
@@ -257,8 +262,9 @@ class CGroup(object):
 
         # Control cgroup path
         self.directory = controller.mount_point
+
         if name != '/':
-            self.directory = self.target.path.join(controller.mount_point, name[1:])
+            self.directory = self.target.path.join(controller.mount_point, name.strip('/'))
 
         # Setup path for tasks file
         self.tasks_file = self.target.path.join(self.directory, 'tasks')
@@ -276,7 +282,7 @@ class CGroup(object):
             self.target.execute('[ -d {0} ]'\
                 .format(self.directory), as_root=True)
             return True
-        except TargetError:
+        except TargetStableError:
             return False
 
     def get(self):
@@ -286,7 +292,7 @@ class CGroup(object):
                 self.controller.kind)
         logging.debug('  %s',
                 self.directory)
-        output = self.target._execute_util(
+        output = self.target._execute_util(  # pylint: disable=protected-access
                     'cgroups_get_attributes {} {}'.format(
                     self.directory, self.controller.kind),
                     as_root=True)
@@ -302,7 +308,7 @@ class CGroup(object):
             if isiterable(attrs[idx]):
                 attrs[idx] = list_to_ranges(attrs[idx])
             # Build attribute path
-            if self.controller._noprefix:
+            if self.controller._noprefix:  # pylint: disable=protected-access
                 attr_name = '{}'.format(idx)
             else:
                 attr_name = '{}.{}'.format(self.controller.kind, idx)
@@ -314,7 +320,7 @@ class CGroup(object):
             # Set the attribute value
             try:
                 self.target.write_value(path, attrs[idx])
-            except TargetError:
+            except TargetStableError:
                 # Check if the error is due to a non-existing attribute
                 attrs = self.get()
                 if idx not in attrs:
@@ -363,7 +369,7 @@ class CgroupsModule(Module):
 
         # Get the list of the available controllers
         subsys = self.list_subsystems()
-        if len(subsys) == 0:
+        if not subsys:
             self.logger.warning('No CGroups controller available')
             return
 
@@ -384,9 +390,9 @@ class CgroupsModule(Module):
             controller = Controller(ss.name, hid, hierarchy[hid])
             try:
                 controller.mount(self.target, self.cgroup_root)
-            except TargetError:
+            except TargetStableError:
                 message = 'Failed to mount "{}" controller'
-                raise TargetError(message.format(controller.kind))
+                raise TargetStableError(message.format(controller.kind))
             self.logger.info('  %-12s : %s', controller.kind,
                              controller.mount_point)
             self.controllers[ss.name] = controller
@@ -420,20 +426,27 @@ class CgroupsModule(Module):
         :param cgroup: Name of cgroup to run command into
         :returns: A command to run `cmdline` into `cgroup`
         """
+        if not cgroup.startswith('/'):
+            message = 'cgroup name "{}" must start with "/"'.format(cgroup)
+            raise ValueError(message)
         return 'CGMOUNT={} {} cgroups_run_into {} {}'\
                 .format(self.cgroup_root, self.target.shutils,
                         cgroup, cmdline)
 
-    def run_into(self, cgroup, cmdline):
+    def run_into(self, cgroup, cmdline, as_root=None):
         """
         Run the specified command into the specified CGroup
 
         :param cmdline: Command to be run into cgroup
         :param cgroup: Name of cgroup to run command into
+        :param as_root: Specify whether to run the command as root, if not
+                        specified will default to whether the target is rooted.
         :returns: Output of command.
         """
+        if as_root is None:
+            as_root = self.target.is_rooted
         cmd = self.run_into_cmd(cgroup, cmdline)
-        raw_output = self.target.execute(cmd)
+        raw_output = self.target.execute(cmd, as_root=as_root)
 
         # First line of output comes from shutils; strip it out.
         return raw_output.split('\n', 1)[1]
@@ -444,11 +457,11 @@ class CgroupsModule(Module):
         A regexps of tasks names can be used to defined tasks which should not
         be moved.
         """
-        return self.target._execute_util(
+        return self.target._execute_util(  # pylint: disable=protected-access
             'cgroups_tasks_move {} {} {}'.format(srcg, dstg, exclude),
             as_root=True)
 
-    def isolate(self, cpus, exclude=[]):
+    def isolate(self, cpus, exclude=None):
         """
         Remove all userspace tasks from specified CPUs.
 
@@ -465,6 +478,8 @@ class CgroupsModule(Module):
                  sandbox is the CGroup of sandboxed CPUs
                  isolated is the CGroup of isolated CPUs
         """
+        if exclude is None:
+            exclude = []
         all_cpus = set(range(self.target.number_of_cpus))
         sbox_cpus = list(all_cpus - set(cpus))
         isol_cpus = list(all_cpus - set(sbox_cpus))
@@ -483,7 +498,7 @@ class CgroupsModule(Module):
 
         return sbox_cg, isol_cg
 
-    def freeze(self, exclude=[], thaw=False):
+    def freeze(self, exclude=None, thaw=False):
         """
         Freeze all user-space tasks but the specified ones
 
@@ -501,6 +516,9 @@ class CgroupsModule(Module):
         :type thaw: bool
         """
 
+        if exclude is None:
+            exclude = []
+
         # Create Freezer CGroup
         freezer = self.controller('freezer')
         if freezer is None:
@@ -509,7 +527,8 @@ class CgroupsModule(Module):
         cmd = 'cgroups_freezer_set_state {{}} {}'.format(freezer_cg.directory)
 
         if thaw:
-            # Restart froozen tasks
+            # Restart frozen tasks
+            # pylint: disable=protected-access
             freezer.target._execute_util(cmd.format('THAWED'), as_root=True)
             # Remove all tasks from freezer
             freezer.move_all_tasks_to('/')
@@ -522,7 +541,7 @@ class CgroupsModule(Module):
         tasks = freezer.tasks('/')
 
         # Freeze all tasks
+        # pylint: disable=protected-access
         freezer.target._execute_util(cmd.format('FROZEN'), as_root=True)
 
         return tasks
-

devlib/module/cooling.py

@@ -37,12 +37,14 @@ class MbedFanActiveCoolingModule(Module):
         with open_serial_connection(timeout=self.timeout,
                                     port=self.port,
                                     baudrate=self.baud) as target:
+            # pylint: disable=no-member
             target.sendline('motor_{}_1'.format(self.fan_pin))
 
     def stop(self):
         with open_serial_connection(timeout=self.timeout,
                                     port=self.port,
                                     baudrate=self.baud) as target:
+            # pylint: disable=no-member
             target.sendline('motor_{}_0'.format(self.fan_pin))
 
 

devlib/module/cpufreq.py

@@ -12,8 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
+from contextlib import contextmanager
+
 from devlib.module import Module
-from devlib.exception import TargetError
+from devlib.exception import TargetStableError
 from devlib.utils.misc import memoized
 
 
@@ -82,7 +84,7 @@ class CpufreqModule(Module):
                Setting the governor on any core in a cluster will also set it on all
                other cores in that cluster.
 
-        :raises: TargetError if governor is not supported by the CPU, or if,
+        :raises: TargetStableError if governor is not supported by the CPU, or if,
                  for some reason, the governor could not be set.
 
         """
@@ -90,11 +92,52 @@ class CpufreqModule(Module):
             cpu = 'cpu{}'.format(cpu)
         supported = self.list_governors(cpu)
         if governor not in supported:
-            raise TargetError('Governor {} not supported for cpu {}'.format(governor, cpu))
+            raise TargetStableError('Governor {} not supported for cpu {}'.format(governor, cpu))
         sysfile = '/sys/devices/system/cpu/{}/cpufreq/scaling_governor'.format(cpu)
         self.target.write_value(sysfile, governor)
         self.set_governor_tunables(cpu, governor, **kwargs)
 
+    @contextmanager
+    def use_governor(self, governor, cpus=None, **kwargs):
+        """
+        Use a given governor, then restore previous governor(s)
+
+        :param governor: Governor to use on all targeted CPUs (see :meth:`set_governor`)
+        :type governor: str
+
+        :param cpus: CPUs affected by the governor change (all by default)
+        :type cpus: list
+
+        :Keyword Arguments: Governor tunables, See :meth:`set_governor_tunables`
+        """
+        if not cpus:
+            cpus = range(self.target.number_of_cpus)
+
+        # Setting a governor & tunables for a cpu will set them for all cpus
+        # in the same clock domain, so only manipulating one cpu per domain
+        # is enough
+        domains = set(self.get_affected_cpus(cpu)[0] for cpu in cpus)
+        prev_governors = {cpu : (self.get_governor(cpu), self.get_governor_tunables(cpu))
+                          for cpu in domains}
+
+        # Special case for userspace, frequency is not seen as a tunable
+        userspace_freqs = {}
+        for cpu, (prev_gov, _) in prev_governors.items():
+            if prev_gov == "userspace":
+                userspace_freqs[cpu] = self.get_frequency(cpu)
+
+        for cpu in domains:
+            self.set_governor(cpu, governor, **kwargs)
+
+        try:
+            yield
+
+        finally:
+            for cpu, (prev_gov, tunables) in prev_governors.items():
+                self.set_governor(cpu, prev_gov, **tunables)
+                if prev_gov == "userspace":
+                    self.set_frequency(cpu, userspace_freqs[cpu])
+
     def list_governor_tunables(self, cpu):
         """Returns a list of tunables available for the governor on the specified CPU."""
         if isinstance(cpu, int):
@@ -104,11 +147,11 @@ class CpufreqModule(Module):
             try:
                 tunables_path = '/sys/devices/system/cpu/{}/cpufreq/{}'.format(cpu, governor)
                 self._governor_tunables[governor] = self.target.list_directory(tunables_path)
-            except TargetError:  # probably an older kernel
+            except TargetStableError:  # probably an older kernel
                 try:
                     tunables_path = '/sys/devices/system/cpu/cpufreq/{}'.format(governor)
                     self._governor_tunables[governor] = self.target.list_directory(tunables_path)
-                except TargetError:  # governor does not support tunables
+                except TargetStableError:  # governor does not support tunables
                     self._governor_tunables[governor] = []
         return self._governor_tunables[governor]
 
@@ -122,7 +165,7 @@ class CpufreqModule(Module):
                 try:
                     path = '/sys/devices/system/cpu/{}/cpufreq/{}/{}'.format(cpu, governor, tunable)
                     tunables[tunable] = self.target.read_value(path)
-                except TargetError:  # May be an older kernel
+                except TargetStableError:  # May be an older kernel
                     path = '/sys/devices/system/cpu/cpufreq/{}/{}'.format(governor, tunable)
                     tunables[tunable] = self.target.read_value(path)
         return tunables
@@ -140,7 +183,7 @@ class CpufreqModule(Module):
         The rest should be keyword parameters mapping tunable name onto the value to
         be set for it.
 
-        :raises: TargetError if governor specified is not a valid governor name, or if
+        :raises: TargetStableError if governor specified is not a valid governor name, or if
                  a tunable specified is not valid for the governor, or if could not set
                  tunable.
 
@@ -155,7 +198,7 @@ class CpufreqModule(Module):
                 path = '/sys/devices/system/cpu/{}/cpufreq/{}/{}'.format(cpu, governor, tunable)
                 try:
                     self.target.write_value(path, value)
-                except TargetError:
+                except TargetStableError:
                     if self.target.file_exists(path):
                         # File exists but we did something wrong
                         raise
@@ -165,7 +208,7 @@ class CpufreqModule(Module):
             else:
                 message = 'Unexpected tunable {} for governor {} on {}.\n'.format(tunable, governor, cpu)
                 message += 'Available tunables are: {}'.format(valid_tunables)
-                raise TargetError(message)
+                raise TargetStableError(message)
 
     @memoized
     def list_frequencies(self, cpu):
@@ -177,14 +220,14 @@ class CpufreqModule(Module):
             cmd = 'cat /sys/devices/system/cpu/{}/cpufreq/scaling_available_frequencies'.format(cpu)
             output = self.target.execute(cmd)
             available_frequencies = list(map(int, output.strip().split()))  # pylint: disable=E1103
-        except TargetError:
+        except TargetStableError:
             # On some devices scaling_frequencies  is not generated.
             # http://adrynalyne-teachtofish.blogspot.co.uk/2011/11/how-to-enable-scalingavailablefrequenci.html
             # Fall back to parsing stats/time_in_state
             path = '/sys/devices/system/cpu/{}/cpufreq/stats/time_in_state'.format(cpu)
             try:
                 out_iter = iter(self.target.read_value(path).split())
-            except TargetError:
+            except TargetStableError:
                 if not self.target.file_exists(path):
                     # Probably intel_pstate. Can't get available freqs.
                     return []
@@ -200,7 +243,7 @@ class CpufreqModule(Module):
         could not be found.
         """
         freqs = self.list_frequencies(cpu)
-        return freqs and max(freqs) or None
+        return max(freqs) if freqs else None
 
     @memoized
     def get_min_available_frequency(self, cpu):
@@ -209,7 +252,7 @@ class CpufreqModule(Module):
         could not be found.
         """
         freqs = self.list_frequencies(cpu)
-        return freqs and min(freqs) or None
+        return min(freqs) if freqs else None
 
     def get_min_frequency(self, cpu):
         """
@@ -219,7 +262,7 @@ class CpufreqModule(Module):
         try to read the minimum frequency and the following exception will be
         raised ::
 
-        :raises: TargetError if for some reason the frequency could not be read.
+        :raises: TargetStableError if for some reason the frequency could not be read.
 
         """
         if isinstance(cpu, int):
@@ -239,7 +282,7 @@ class CpufreqModule(Module):
 
         on the device.
 
-        :raises: TargetError if the frequency is not supported by the CPU, or if, for
+        :raises: TargetStableError if the frequency is not supported by the CPU, or if, for
                  some reason, frequency could not be set.
         :raises: ValueError if ``frequency`` is not an integer.
 
@@ -250,7 +293,7 @@ class CpufreqModule(Module):
         try:
             value = int(frequency)
             if exact and available_frequencies and value not in available_frequencies:
-                raise TargetError('Can\'t set {} frequency to {}\nmust be in {}'.format(cpu,
+                raise TargetStableError('Can\'t set {} frequency to {}\nmust be in {}'.format(cpu,
                                                                                         value,
                                                                                         available_frequencies))
             sysfile = '/sys/devices/system/cpu/{}/cpufreq/scaling_min_freq'.format(cpu)
@@ -266,7 +309,7 @@ class CpufreqModule(Module):
         try to read the current frequency and the following exception will be
         raised ::
 
-        :raises: TargetError if for some reason the frequency could not be read.
+        :raises: TargetStableError if for some reason the frequency could not be read.
 
         """
         if isinstance(cpu, int):
@@ -288,7 +331,7 @@ class CpufreqModule(Module):
 
         on the device (if it exists).
 
-        :raises: TargetError if the frequency is not supported by the CPU, or if, for
+        :raises: TargetStableError if the frequency is not supported by the CPU, or if, for
                  some reason, frequency could not be set.
         :raises: ValueError if ``frequency`` is not an integer.
 
@@ -300,11 +343,11 @@ class CpufreqModule(Module):
             if exact:
                 available_frequencies = self.list_frequencies(cpu)
                 if available_frequencies and value not in available_frequencies:
-                    raise TargetError('Can\'t set {} frequency to {}\nmust be in {}'.format(cpu,
+                    raise TargetStableError('Can\'t set {} frequency to {}\nmust be in {}'.format(cpu,
                                                                                             value,
                                                                                             available_frequencies))
             if self.get_governor(cpu) != 'userspace':
-                raise TargetError('Can\'t set {} frequency; governor must be "userspace"'.format(cpu))
+                raise TargetStableError('Can\'t set {} frequency; governor must be "userspace"'.format(cpu))
             sysfile = '/sys/devices/system/cpu/{}/cpufreq/scaling_setspeed'.format(cpu)
             self.target.write_value(sysfile, value, verify=False)
         except ValueError:
@@ -318,7 +361,7 @@ class CpufreqModule(Module):
         try to read the maximum frequency and the following exception will be
         raised ::
 
-        :raises: TargetError if for some reason the frequency could not be read.
+        :raises: TargetStableError if for some reason the frequency could not be read.
         """
         if isinstance(cpu, int):
             cpu = 'cpu{}'.format(cpu)
@@ -337,7 +380,7 @@ class CpufreqModule(Module):
 
         on the device.
 
-        :raises: TargetError if the frequency is not supported by the CPU, or if, for
+        :raises: TargetStableError if the frequency is not supported by the CPU, or if, for
                  some reason, frequency could not be set.
         :raises: ValueError if ``frequency`` is not an integer.
 
@@ -348,7 +391,7 @@ class CpufreqModule(Module):
         try:
             value = int(frequency)
             if exact and available_frequencies and value not in available_frequencies:
-                raise TargetError('Can\'t set {} frequency to {}\nmust be in {}'.format(cpu,
+                raise TargetStableError('Can\'t set {} frequency to {}\nmust be in {}'.format(cpu,
                                                                                         value,
                                                                                         available_frequencies))
             sysfile = '/sys/devices/system/cpu/{}/cpufreq/scaling_max_freq'.format(cpu)
@@ -380,6 +423,7 @@ class CpufreqModule(Module):
         """
         Set the specified (minimum) frequency for all the (online) CPUs
         """
+        # pylint: disable=protected-access
         return self.target._execute_util(
                 'cpufreq_set_all_frequencies {}'.format(freq),
                 as_root=True)
@@ -388,6 +432,7 @@ class CpufreqModule(Module):
         """
         Get the current frequency for all the (online) CPUs
         """
+        # pylint: disable=protected-access
         output = self.target._execute_util(
                 'cpufreq_get_all_frequencies', as_root=True)
         frequencies = {}
@@ -403,16 +448,17 @@ class CpufreqModule(Module):
         Set the specified governor for all the (online) CPUs
         """
         try:
+            # pylint: disable=protected-access
             return self.target._execute_util(
                 'cpufreq_set_all_governors {}'.format(governor),
                 as_root=True)
-        except TargetError as e:
+        except TargetStableError as e:
             if ("echo: I/O error" in str(e) or
                 "write error: Invalid argument" in str(e)):
 
                 cpus_unsupported = [c for c in self.target.list_online_cpus()
                                     if governor not in self.list_governors(c)]
-                raise TargetError("Governor {} unsupported for CPUs {}".format(
+                raise TargetStableError("Governor {} unsupported for CPUs {}".format(
                     governor, cpus_unsupported))
             else:
                 raise
@@ -421,6 +467,7 @@ class CpufreqModule(Module):
         """
         Get the current governor for all the (online) CPUs
         """
+        # pylint: disable=protected-access
         output = self.target._execute_util(
                 'cpufreq_get_all_governors', as_root=True)
         governors = {}
@@ -435,6 +482,7 @@ class CpufreqModule(Module):
         """
         Report current frequencies on trace file
         """
+        # pylint: disable=protected-access
         return self.target._execute_util('cpufreq_trace_all_frequencies', as_root=True)
 
     def get_affected_cpus(self, cpu):
@@ -478,7 +526,7 @@ class CpufreqModule(Module):
         """
         cpus = set(range(self.target.number_of_cpus))
         while cpus:
-            cpu = next(iter(cpus))
+            cpu = next(iter(cpus))  # pylint: disable=stop-iteration-return
             domain = self.target.cpufreq.get_related_cpus(cpu)
             yield domain
             cpus = cpus.difference(domain)

devlib/module/cpuidle.py

@@ -16,7 +16,6 @@
 from past.builtins import basestring
 
 from devlib.module import Module
-from devlib.utils.misc import memoized
 from devlib.utils.types import integer, boolean
 
 
@@ -51,6 +50,7 @@ class CpuidleState(object):
         self.desc = desc
         self.power = power
         self.latency = latency
+        self.residency = residency
         self.id = self.target.path.basename(self.path)
         self.cpu = self.target.path.basename(self.target.path.dirname(path))
 
@@ -166,7 +166,8 @@ class Cpuidle(Module):
         """
         Momentarily wake each CPU. Ensures cpu_idle events in trace file.
         """
-        output = self.target._execute_util('cpuidle_wake_all_cpus')
+        # pylint: disable=protected-access
+        self.target._execute_util('cpuidle_wake_all_cpus')
 
     def get_driver(self):
         return self.target.read_value(self.target.path.join(self.root_path, 'current_driver'))

devlib/module/devfreq.py

@@ -13,7 +13,7 @@
 # limitations under the License.
 #
 from devlib.module import Module
-from devlib.exception import TargetError
+from devlib.exception import TargetStableError
 from devlib.utils.misc import memoized
 
 class DevfreqModule(Module):
@@ -64,13 +64,13 @@ class DevfreqModule(Module):
         Additional keyword arguments can be used to specify governor tunables for
         governors that support them.
 
-        :raises: TargetError if governor is not supported by the device, or if,
+        :raises: TargetStableError if governor is not supported by the device, or if,
                  for some reason, the governor could not be set.
 
         """
         supported = self.list_governors(device)
         if governor not in supported:
-            raise TargetError('Governor {} not supported for device {}'.format(governor, device))
+            raise TargetStableError('Governor {} not supported for device {}'.format(governor, device))
         sysfile = '/sys/class/devfreq/{}/governor'.format(device)
         self.target.write_value(sysfile, governor)
 
@@ -94,7 +94,7 @@ class DevfreqModule(Module):
         will try to read the minimum frequency and the following exception will
         be raised ::
 
-        :raises: TargetError if for some reason the frequency could not be read.
+        :raises: TargetStableError if for some reason the frequency could not be read.
 
         """
         sysfile = '/sys/class/devfreq/{}/min_freq'.format(device)
@@ -112,7 +112,7 @@ class DevfreqModule(Module):
 
         on the device.
 
-        :raises: TargetError if the frequency is not supported by the device, or if, for
+        :raises: TargetStableError if the frequency is not supported by the device, or if, for
                  some reason, frequency could not be set.
         :raises: ValueError if ``frequency`` is not an integer.
 
@@ -121,7 +121,7 @@ class DevfreqModule(Module):
         try:
             value = int(frequency)
             if exact and available_frequencies and value not in available_frequencies:
-                raise TargetError('Can\'t set {} frequency to {}\nmust be in {}'.format(device,
+                raise TargetStableError('Can\'t set {} frequency to {}\nmust be in {}'.format(device,
                                                                                         value,
                                                                                         available_frequencies))
             sysfile = '/sys/class/devfreq/{}/min_freq'.format(device)
@@ -137,7 +137,7 @@ class DevfreqModule(Module):
         will try to read the current frequency and the following exception will
         be raised ::
 
-        :raises: TargetError if for some reason the frequency could not be read.
+        :raises: TargetStableError if for some reason the frequency could not be read.
 
         """
         sysfile = '/sys/class/devfreq/{}/cur_freq'.format(device)
@@ -151,7 +151,7 @@ class DevfreqModule(Module):
         try to read the maximum frequency and the following exception will be
         raised ::
 
-        :raises: TargetError if for some reason the frequency could not be read.
+        :raises: TargetStableError if for some reason the frequency could not be read.
         """
         sysfile = '/sys/class/devfreq/{}/max_freq'.format(device)
         return self.target.read_int(sysfile)
@@ -168,7 +168,7 @@ class DevfreqModule(Module):
 
         on the device.
 
-        :raises: TargetError if the frequency is not supported by the device, or
+        :raises: TargetStableError if the frequency is not supported by the device, or
                  if, for some reason, frequency could not be set.
         :raises: ValueError if ``frequency`` is not an integer.
 
@@ -180,7 +180,7 @@ class DevfreqModule(Module):
             raise ValueError('Frequency must be an integer; got: "{}"'.format(frequency))
 
         if exact and value not in available_frequencies:
-            raise TargetError('Can\'t set {} frequency to {}\nmust be in {}'.format(device,
+            raise TargetStableError('Can\'t set {} frequency to {}\nmust be in {}'.format(device,
                                                                                     value,
                                                                                     available_frequencies))
         sysfile = '/sys/class/devfreq/{}/max_freq'.format(device)
@@ -200,15 +200,15 @@ class DevfreqModule(Module):
         Set the specified governor for all the (available) devices
         """
         try:
-            return self.target._execute_util(
+            return self.target._execute_util(  # pylint: disable=protected-access
                 'devfreq_set_all_governors {}'.format(governor), as_root=True)
-        except TargetError as e:
+        except TargetStableError as e:
             if ("echo: I/O error" in str(e) or
                 "write error: Invalid argument" in str(e)):
 
                 devs_unsupported = [d for d in self.target.list_devices()
                                     if governor not in self.list_governors(d)]
-                raise TargetError("Governor {} unsupported for devices {}".format(
+                raise TargetStableError("Governor {} unsupported for devices {}".format(
                     governor, devs_unsupported))
             else:
                 raise
@@ -217,7 +217,7 @@ class DevfreqModule(Module):
         """
         Get the current governor for all the (online) CPUs
         """
-        output = self.target._execute_util(
+        output = self.target._execute_util(  # pylint: disable=protected-access
                 'devfreq_get_all_governors', as_root=True)
         governors = {}
         for x in output.splitlines():
@@ -241,7 +241,7 @@ class DevfreqModule(Module):
         """
         Set the specified (minimum) frequency for all the (available) devices
         """
-        return self.target._execute_util(
+        return self.target._execute_util(  # pylint: disable=protected-access
                 'devfreq_set_all_frequencies {}'.format(freq),
                 as_root=True)
 
@@ -249,7 +249,7 @@ class DevfreqModule(Module):
         """
         Get the current frequency for all the (available) devices
         """
-        output = self.target._execute_util(
+        output = self.target._execute_util(  # pylint: disable=protected-access
                 'devfreq_get_all_frequencies', as_root=True)
         frequencies = {}
         for x in output.splitlines():
@@ -258,4 +258,3 @@ class DevfreqModule(Module):
                 break
             frequencies[kv[0]] = kv[1]
         return frequencies
-

devlib/module/gem5stats.py

@@ -14,16 +14,13 @@
 
 import re
 import sys
-import logging
 import os.path
 from collections import defaultdict
 
-import devlib
-from devlib.exception import TargetError
+from devlib.exception import TargetStableError, HostError
 from devlib.module import Module
-from devlib.platform import Platform
 from devlib.platform.gem5 import Gem5SimulationPlatform
-from devlib.utils.gem5 import iter_statistics_dump, GEM5STATS_ROI_NUMBER, GEM5STATS_DUMP_TAIL
+from devlib.utils.gem5 import iter_statistics_dump, GEM5STATS_ROI_NUMBER
 
 
 class Gem5ROI:
@@ -39,7 +36,7 @@ class Gem5ROI:
         self.target.execute('m5 roistart {}'.format(self.number))
         self.running = True
         return True
-    
+
     def stop(self):
         if not self.running:
             return False
@@ -49,7 +46,7 @@ class Gem5ROI:
 
 class Gem5StatsModule(Module):
     '''
-    Module controlling Region of Interest (ROIs) markers, satistics dump 
+    Module controlling Region of Interest (ROIs) markers, satistics dump
     frequency and parsing statistics log file when using gem5 platforms.
 
     ROIs are identified by user-defined labels and need to be booked prior to
@@ -90,13 +87,13 @@ class Gem5StatsModule(Module):
         if label not in self.rois:
             raise KeyError('Incorrect ROI label: {}'.format(label))
         if not self.rois[label].start():
-            raise TargetError('ROI {} was already running'.format(label))
-    
+            raise TargetStableError('ROI {} was already running'.format(label))
+
     def roi_end(self, label):
         if label not in self.rois:
             raise KeyError('Incorrect ROI label: {}'.format(label))
         if not self.rois[label].stop():
-            raise TargetError('ROI {} was not running'.format(label))
+            raise TargetStableError('ROI {} was not running'.format(label))
 
     def start_periodic_dump(self, delay_ns=0, period_ns=10000000):
         # Default period is 10ms because it's roughly what's needed to have
@@ -105,7 +102,7 @@ class Gem5StatsModule(Module):
             msg = 'Delay ({}) and period ({}) for periodic dumps must be positive'
             raise ValueError(msg.format(delay_ns, period_ns))
         self.target.execute('m5 dumpresetstats {} {}'.format(delay_ns, period_ns))
-    
+
     def match(self, keys, rois_labels, base_dump=0):
         '''
         Extract specific values from the statistics log file of gem5
@@ -116,49 +113,49 @@ class Gem5StatsModule(Module):
             keys.
         :type keys: list
 
-        :param rois_labels: list of ROIs labels. ``match()`` returns the 
+        :param rois_labels: list of ROIs labels. ``match()`` returns the
             values of the specified fields only during dumps spanned by at
             least one of these ROIs.
         :type rois_label: list
 
-        :param base_dump: dump number from which ``match()`` should operate. By 
-            specifying a non-zero dump number, one can virtually truncate 
+        :param base_dump: dump number from which ``match()`` should operate. By
+            specifying a non-zero dump number, one can virtually truncate
             the head of the stats file and ignore all dumps before a specific
-            instant. The value of ``base_dump`` will typically (but not 
+            instant. The value of ``base_dump`` will typically (but not
             necessarily) be the result of a previous call to ``next_dump_no``.
             Default value is 0.
         :type base_dump: int
 
         :returns: a dict indexed by key parameters containing a dict indexed by
         ROI labels containing an in-order list of records for the key under
-        consideration during the active intervals of the ROI. 
-        
+        consideration during the active intervals of the ROI.
+
         Example of return value:
          * Result of match(['sim_'],['roi_1']):
             {
-                'sim_inst': 
+                'sim_inst':
                 {
                     'roi_1': [265300176, 267975881]
                 }
-                'sim_ops': 
+                'sim_ops':
                 {
                     'roi_1': [324395787, 327699419]
                 }
-                'sim_seconds': 
+                'sim_seconds':
                 {
                     'roi_1': [0.199960, 0.199897]
                 }
-                'sim_freq': 
+                'sim_freq':
                 {
                     'roi_1': [1000000000000, 1000000000000]
                 }
-                'sim_ticks': 
+                'sim_ticks':
                 {
                     'roi_1': [199960234227, 199896897330]
                 }
             }
         '''
-        records = defaultdict(lambda : defaultdict(list))
+        records = defaultdict(lambda: defaultdict(list))
         for record, active_rois in self.match_iter(keys, rois_labels, base_dump):
             for key in record:
                 for roi_label in active_rois:
@@ -178,15 +175,15 @@ class Gem5StatsModule(Module):
 
         Example of return value:
          * Result of match_iter(['sim_'],['roi_1', 'roi_2']).next()
-            ( 
-                { 
+            (
+                {
                     'sim_inst': 265300176,
                     'sim_ops': 324395787,
-                    'sim_seconds': 0.199960, 
+                    'sim_seconds': 0.199960,
                     'sim_freq': 1000000000000,
                     'sim_ticks': 199960234227,
                 },
-                [ 'roi_1 ' ] 
+                [ 'roi_1 ' ]
             )
         '''
         for label in rois_labels:
@@ -195,11 +192,11 @@ class Gem5StatsModule(Module):
             if self.rois[label].running:
                 self.logger.warning('Trying to match records in statistics file'
                         ' while ROI {} is running'.format(label))
-        
+
         # Construct one large regex that concatenates all keys because
         # matching one large expression is more efficient than several smaller
         all_keys_re = re.compile('|'.join(keys))
-        
+
         def roi_active(roi_label, dump):
             roi = self.rois[roi_label]
             return (roi.field in dump) and (int(dump[roi.field]) == 1)
@@ -215,8 +212,8 @@ class Gem5StatsModule(Module):
     def next_dump_no(self):
         '''
         Returns the number of the next dump to be written to the stats file.
-        
-        For example, if next_dump_no is called while there are 5 (0 to 4) full 
+
+        For example, if next_dump_no is called while there are 5 (0 to 4) full
         dumps in the stats file, it will return 5. This will be usefull to know
         from which dump one should match() in the future to get only data from
         now on.
@@ -224,7 +221,7 @@ class Gem5StatsModule(Module):
         with open(self._stats_file_path, 'r') as stats_file:
             # _goto_dump reach EOF and returns the total number of dumps + 1
             return self._goto_dump(stats_file, sys.maxsize)
-    
+
     def _goto_dump(self, stats_file, target_dump):
         if target_dump < 0:
             raise HostError('Cannot go to dump {}'.format(target_dump))
@@ -238,12 +235,12 @@ class Gem5StatsModule(Module):
         curr_dump = max(prev_dumps)
         curr_pos = self._dump_pos_cache[curr_dump]
         stats_file.seek(curr_pos)
-        
+
         # And iterate until target_dump
         dump_iterator = iter_statistics_dump(stats_file)
         while curr_dump < target_dump:
             try:
-                dump = next(dump_iterator)
+                next(dump_iterator)
             except StopIteration:
                 break
             # End of passed dump is beginning og next one
@@ -251,4 +248,3 @@ class Gem5StatsModule(Module):
             curr_dump += 1
         self._dump_pos_cache[curr_dump] = curr_pos
         return curr_dump
-

devlib/module/gpufreq.py

@@ -28,9 +28,8 @@
 # limitations under the License.
 
 import re
-import json
 from devlib.module import Module
-from devlib.exception import TargetError
+from devlib.exception import TargetStableError
 from devlib.utils.misc import memoized
 
 class GpufreqModule(Module):
@@ -57,7 +56,7 @@ class GpufreqModule(Module):
 
     def set_governor(self, governor):
         if governor not in self.governors:
-            raise TargetError('Governor {} not supported for gpu {}'.format(governor, cpu))
+            raise TargetStableError('Governor {} not supported for gpu'.format(governor))
         self.target.write_value("/sys/kernel/gpu/gpu_governor", governor)
 
     def get_frequencies(self):
@@ -74,7 +73,7 @@ class GpufreqModule(Module):
         try to read the current frequency and the following exception will be
         raised ::
 
-        :raises: TargetError if for some reason the frequency could not be read.
+        :raises: TargetStableError if for some reason the frequency could not be read.
 
         """
         return int(self.target.read_value("/sys/kernel/gpu/gpu_clock"))
@@ -85,6 +84,6 @@ class GpufreqModule(Module):
         Returns the model name reported by the GPU.
         """
         try:
-          return self.target.read_value("/sys/kernel/gpu/gpu_model")
-        except:
-          return "unknown"
+            return self.target.read_value("/sys/kernel/gpu/gpu_model")
+        except:  # pylint: disable=bare-except
+            return "unknown"

devlib/module/hotplug.py

@@ -35,8 +35,12 @@ class HotplugModule(Module):
             cpu = 'cpu{}'.format(cpu)
         return target.path.join(cls.base_path, cpu, 'online')
 
+    def list_hotpluggable_cpus(self):
+        return [cpu for cpu in range(self.target.number_of_cpus)
+                if self.target.file_exists(self._cpu_path(self.target, cpu))]
+
     def online_all(self):
-        self.target._execute_util('hotplug_online_all',
+        self.target._execute_util('hotplug_online_all',  # pylint: disable=protected-access
                                   as_root=self.target.is_rooted)
 
     def online(self, *args):
@@ -53,4 +57,3 @@ class HotplugModule(Module):
             return
         value = 1 if online else 0
         self.target.write_value(path, value)
-

devlib/module/hwmon.py

@@ -12,11 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-import os
 import re
 from collections import defaultdict
 
-from devlib import TargetError
+from devlib import TargetStableError
 from devlib.module import Module
 from devlib.utils.types import integer
 
@@ -119,7 +118,7 @@ class HwmonModule(Module):
     def probe(target):
         try:
             target.list_directory(HWMON_ROOT, as_root=target.is_rooted)
-        except TargetError:
+        except TargetStableError:
             # Doesn't exist or no permissions
             return False
         return True
@@ -138,7 +137,7 @@ class HwmonModule(Module):
         self.scan()
 
     def scan(self):
-        values_tree = self.target.read_tree_values(self.root, depth=3)
+        values_tree = self.target.read_tree_values(self.root, depth=3, tar=True)
         for entry_id, fields in values_tree.items():
             path = self.target.path.join(self.root, entry_id)
             name = fields.pop('name', None)
@@ -147,4 +146,3 @@ class HwmonModule(Module):
             self.logger.debug('Adding device {}'.format(name))
             device = HwmonDevice(self.target, path, name, fields)
             self.devices.append(device)
-

devlib/module/sched.py

@@ -13,27 +13,15 @@
 # limitations under the License.
 #
 
-#    Copyright 2018 Arm Limited
-#
-# 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 logging
 import re
+from enum import Enum
+
+from past.builtins import basestring
 
 from devlib.module import Module
 from devlib.utils.misc import memoized
-
-from past.builtins import basestring
+from devlib.utils.types import boolean
 
 
 class SchedProcFSNode(object):
@@ -62,7 +50,7 @@ class SchedProcFSNode(object):
     MC
     """
 
-    _re_procfs_node = re.compile(r"(?P<name>.*)(?P<digits>\d+)$")
+    _re_procfs_node = re.compile(r"(?P<name>.*\D)(?P<digits>\d+)$")
 
     @staticmethod
     def _ends_with_digits(node):
@@ -104,7 +92,7 @@ class SchedProcFSNode(object):
             return SchedProcFSNode(node_data)
 
     @staticmethod
-    def _build_entry(node_name, node_data):
+    def _build_entry(node_data):
         value = node_data
 
         # Most nodes just contain numerical data, try to convert
@@ -120,7 +108,7 @@ class SchedProcFSNode(object):
         if isinstance(node_data, dict):
             return SchedProcFSNode._build_directory(node_name, node_data)
         else:
-            return SchedProcFSNode._build_entry(node_name, node_data)
+            return SchedProcFSNode._build_entry(node_data)
 
     def __getattr__(self, name):
         return self._dyn_attrs[name]
@@ -152,51 +140,74 @@ class SchedProcFSNode(object):
             self._dyn_attrs[key] = self._build_node(key, nodes[key])
 
 
-class SchedDomain(SchedProcFSNode):
+class DocInt(int):
+
+    # See https://stackoverflow.com/a/50473952/5096023
+    def __new__(cls, value, doc):
+        new = super(DocInt, cls).__new__(cls, value)
+        new.__doc__ = doc
+        return new
+
+
+class SchedDomainFlag(DocInt, Enum):
     """
-    Represents a sched domain as seen through procfs
+    Represents a sched domain flag
     """
+    # pylint: disable=bad-whitespace
     # Domain flags obtained from include/linux/sched/topology.h on v4.17
     # https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux/+/v4.17/include/linux/sched/topology.h#20
-    SD_LOAD_BALANCE        = 0x0001  # Do load balancing on this domain.
-    SD_BALANCE_NEWIDLE     = 0x0002  # Balance when about to become idle
-    SD_BALANCE_EXEC        = 0x0004  # Balance on exec
-    SD_BALANCE_FORK        = 0x0008  # Balance on fork, clone
-    SD_BALANCE_WAKE        = 0x0010  # Balance on wakeup
-    SD_WAKE_AFFINE         = 0x0020  # Wake task to waking CPU
-    SD_ASYM_CPUCAPACITY    = 0x0040  # Groups have different max cpu capacities
-    SD_SHARE_CPUCAPACITY   = 0x0080  # Domain members share cpu capacity
-    SD_SHARE_POWERDOMAIN   = 0x0100  # Domain members share power domain
-    SD_SHARE_PKG_RESOURCES = 0x0200  # Domain members share cpu pkg resources
-    SD_SERIALIZE           = 0x0400  # Only a single load balancing instance
-    SD_ASYM_PACKING        = 0x0800  # Place busy groups earlier in the domain
-    SD_PREFER_SIBLING      = 0x1000  # Prefer to place tasks in a sibling domain
-    SD_OVERLAP             = 0x2000  # sched_domains of this level overlap
-    SD_NUMA                = 0x4000  # cross-node balancing
+    SD_LOAD_BALANCE =        0x0001, "Do load balancing on this domain"
+    SD_BALANCE_NEWIDLE =     0x0002, "Balance when about to become idle"
+    SD_BALANCE_EXEC =        0x0004, "Balance on exec"
+    SD_BALANCE_FORK =        0x0008, "Balance on fork, clone"
+    SD_BALANCE_WAKE =        0x0010, "Balance on wakeup"
+    SD_WAKE_AFFINE =         0x0020, "Wake task to waking CPU"
+    SD_ASYM_CPUCAPACITY =    0x0040, "Groups have different max cpu capacities"
+    SD_SHARE_CPUCAPACITY =   0x0080, "Domain members share cpu capacity"
+    SD_SHARE_POWERDOMAIN =   0x0100, "Domain members share power domain"
+    SD_SHARE_PKG_RESOURCES = 0x0200, "Domain members share cpu pkg resources"
+    SD_SERIALIZE =           0x0400, "Only a single load balancing instance"
+    SD_ASYM_PACKING =        0x0800, "Place busy groups earlier in the domain"
+    SD_PREFER_SIBLING =      0x1000, "Prefer to place tasks in a sibling domain"
+    SD_OVERLAP =             0x2000, "Sched_domains of this level overlap"
+    SD_NUMA =                0x4000, "Cross-node balancing"
     # Only defined in Android
     # https://android.googlesource.com/kernel/common/+/android-4.14/include/linux/sched/topology.h#29
-    SD_SHARE_CAP_STATES    = 0x8000  # Domain members share capacity state
+    SD_SHARE_CAP_STATES =    0x8000, "(Android only) Domain members share capacity state"
 
-    # Checked to be valid from v4.4
-    SD_FLAGS_REF_PARTS = (4, 4, 0)
-
-    @staticmethod
-    def check_version(target, logger):
+    @classmethod
+    def check_version(cls, target, logger):
         """
         Check the target and see if its kernel version matches our view of the world
         """
         parts = target.kernel_version.parts
-        if parts < SchedDomain.SD_FLAGS_REF_PARTS:
+        # Checked to be valid from v4.4
+        # Not saved as a class attribute else it'll be converted to an enum
+        ref_parts = (4, 4, 0)
+        if parts < ref_parts:
             logger.warn(
                 "Sched domain flags are defined for kernels v{} and up, "
-                "but target is running v{}".format(SchedDomain.SD_FLAGS_REF_PARTS, parts)
+                "but target is running v{}".format(ref_parts, parts)
             )
 
-    def has_flags(self, flags):
-        """
-        :returns: Whether 'flags' are set on this sched domain
-        """
-        return self.flags & flags == flags
+
+    def __str__(self):
+        return self.name
+
+
+class SchedDomain(SchedProcFSNode):
+    """
+    Represents a sched domain as seen through procfs
+    """
+    def __init__(self, nodes):
+        super(SchedDomain, self).__init__(nodes)
+
+        obj_flags = set()
+        for flag in list(SchedDomainFlag):
+            if self.flags & flag.value == flag.value:
+                obj_flags.add(flag)
+
+        self.flags = obj_flags
 
 
 class SchedProcFSData(SchedProcFSNode):
@@ -208,7 +219,19 @@ class SchedProcFSData(SchedProcFSNode):
 
     @staticmethod
     def available(target):
-        return target.directory_exists(SchedProcFSData.sched_domain_root)
+        path = SchedProcFSData.sched_domain_root
+        cpus = target.list_directory(path) if target.file_exists(path) else []
+
+        if not cpus:
+            return False
+
+        # Even if we have a CPU entry, it can be empty (e.g. hotplugged out)
+        # Make sure some data is there
+        for cpu in cpus:
+            if target.file_exists(target.path.join(path, cpu, "domain0", "name")):
+                return True
+
+        return False
 
     def __init__(self, target, path=None):
         if not path:
@@ -227,10 +250,113 @@ class SchedModule(Module):
     @staticmethod
     def probe(target):
         logger = logging.getLogger(SchedModule.name)
-        SchedDomain.check_version(target, logger)
+        SchedDomainFlag.check_version(target, logger)
 
         return SchedProcFSData.available(target)
 
+    def get_kernel_attributes(self, matching=None, check_exit_code=True):
+        """
+        Get the value of scheduler attributes.
+
+        :param matching: an (optional) substring to filter the scheduler
+        attributes to be returned.
+
+        The scheduler exposes a list of tunable attributes under:
+            /proc/sys/kernel
+        all starting with the "sched_" prefix.
+
+        This method returns a dictionary of all the "sched_" attributes exposed
+        by the target kernel, within the prefix removed.
+        It's possible to restrict the list of attributes by specifying a
+        substring to be matched.
+
+        returns: a dictionary of scheduler tunables
+        """
+        command = 'sched_get_kernel_attributes {}'.format(
+            matching if matching else ''
+        )
+        output = self.target._execute_util(command, as_root=self.target.is_rooted,
+                                           check_exit_code=check_exit_code)
+        result = {}
+        for entry in output.strip().split('\n'):
+            if ':' not in entry:
+                continue
+            path, value = entry.strip().split(':', 1)
+            if value in ['0', '1']:
+                value = bool(int(value))
+            elif value.isdigit():
+                value = int(value)
+            result[path] = value
+        return result
+
+    def set_kernel_attribute(self, attr, value, verify=True):
+        """
+        Set the value of a scheduler attribute.
+
+        :param attr: the attribute to set, without the "sched_" prefix
+        :param value: the value to set
+        :param verify: true to check that the requested value has been set
+
+        :raise TargetError: if the attribute cannot be set
+        """
+        if isinstance(value, bool):
+            value = '1' if value else '0'
+        elif isinstance(value, int):
+            value = str(value)
+        path = '/proc/sys/kernel/sched_' + attr
+        self.target.write_value(path, value, verify)
+
+    @property
+    @memoized
+    def has_debug(self):
+        if self.target.config.get('SCHED_DEBUG') != 'y':
+            return False;
+        return self.target.file_exists('/sys/kernel/debug/sched_features')
+
+    def get_features(self):
+        """
+        Get the status of each sched feature
+
+        :returns: a dictionary of features and their "is enabled" status
+        """
+        if not self.has_debug:
+            raise RuntimeError("sched_features not available")
+        feats = self.target.read_value('/sys/kernel/debug/sched_features')
+        features = {}
+        for feat in feats.split():
+            value = True
+            if feat.startswith('NO'):
+                feat = feat.replace('NO_', '', 1)
+                value = False
+            features[feat] = value
+        return features
+
+    def set_feature(self, feature, enable, verify=True):
+        """
+        Set the status of a specified scheduler feature
+
+        :param feature: the feature name to set
+        :param enable: true to enable the feature, false otherwise
+
+        :raise ValueError: if the specified enable value is not bool
+        :raise RuntimeError: if the specified feature cannot be set
+        """
+        if not self.has_debug:
+            raise RuntimeError("sched_features not available")
+        feature = feature.upper()
+        feat_value = feature
+        if not boolean(enable):
+            feat_value = 'NO_' + feat_value
+        self.target.write_value('/sys/kernel/debug/sched_features',
+                                feat_value, verify=False)
+        if not verify:
+            return
+        msg = 'Failed to set {}, feature not supported?'.format(feat_value)
+        features = self.get_features()
+        feat_value = features.get(feature, not enable)
+        if feat_value != enable:
+            raise RuntimeError(msg)
+
     def get_cpu_sd_info(self, cpu):
         """
         :returns: An object view of /proc/sys/kernel/sched_domain/cpu<cpu>/*

devlib/module/vexpress.py

@@ -20,7 +20,7 @@ import shutil
 from subprocess import CalledProcessError
 
 from devlib.module import HardRestModule, BootModule, FlashModule
-from devlib.exception import TargetError, HostError
+from devlib.exception import TargetError, TargetStableError, HostError
 from devlib.utils.serial_port import open_serial_connection, pulse_dtr, write_characters
 from devlib.utils.uefi import UefiMenu, UefiConfig
 from devlib.utils.uboot import UbootMenu
@@ -89,7 +89,7 @@ class VexpressReboottxtHardReset(HardRestModule):
         try:
             if self.target.is_connected:
                 self.target.execute('sync')
-        except TargetError:
+        except (TargetError, CalledProcessError):
             pass
 
         if not os.path.exists(self.path):
@@ -209,6 +209,7 @@ class VexpressUefiShellBoot(VexpressBootModule):
 
     name = 'vexpress-uefi-shell'
 
+    # pylint: disable=keyword-arg-before-vararg
     def __init__(self, target, uefi_entry='^Shell$',
                  efi_shell_prompt='Shell>',
                  image='kernel', bootargs=None,
@@ -224,7 +225,7 @@ class VexpressUefiShellBoot(VexpressBootModule):
         try:
             menu.select(self.uefi_entry)
         except LookupError:
-            raise TargetError('Did not see "{}" UEFI entry.'.format(self.uefi_entry))
+            raise TargetStableError('Did not see "{}" UEFI entry.'.format(self.uefi_entry))
         tty.expect(self.efi_shell_prompt, timeout=self.timeout)
         if self.bootargs:
             tty.sendline('')  # stop default boot
@@ -239,6 +240,7 @@ class VexpressUBoot(VexpressBootModule):
 
     name = 'vexpress-u-boot'
 
+    # pylint: disable=keyword-arg-before-vararg
     def __init__(self, target, env=None,
                  *args, **kwargs):
         super(VexpressUBoot, self).__init__(target, *args, **kwargs)
@@ -260,6 +262,7 @@ class VexpressBootmon(VexpressBootModule):
 
     name = 'vexpress-bootmon'
 
+    # pylint: disable=keyword-arg-before-vararg
     def __init__(self, target,
                  image, fdt, initrd, bootargs,
                  uses_bootscript=False,
@@ -282,11 +285,11 @@ class VexpressBootmon(VexpressBootModule):
         with open_serial_connection(port=self.port,
                                     baudrate=self.baudrate,
                                     timeout=self.timeout,
-                                    init_dtr=0) as tty:
-            write_characters(tty, 'fl linux fdt {}'.format(self.fdt))
-            write_characters(tty, 'fl linux initrd {}'.format(self.initrd))
-            write_characters(tty, 'fl linux boot {} {}'.format(self.image,
-                                                               self.bootargs))
+                                    init_dtr=0) as tty_conn:
+            write_characters(tty_conn, 'fl linux fdt {}'.format(self.fdt))
+            write_characters(tty_conn, 'fl linux initrd {}'.format(self.initrd))
+            write_characters(tty_conn, 'fl linux boot {} {}'.format(self.image,
+                                                                    self.bootargs))
 
 
 class VersatileExpressFlashModule(FlashModule):
@@ -328,9 +331,10 @@ class VersatileExpressFlashModule(FlashModule):
                                     baudrate=self.target.platform.baudrate,
                                     timeout=self.timeout,
                                     init_dtr=0) as tty:
+            # pylint: disable=no-member
             i = tty.expect([self.mcc_prompt, AUTOSTART_MESSAGE, OLD_AUTOSTART_MESSAGE])
             if i:
-                tty.sendline('')
+                tty.sendline('')  # pylint: disable=no-member
             wait_for_vemsd(self.vemsd_mount, tty, self.mcc_prompt, self.short_delay)
         try:
             if image_bundle:
@@ -340,7 +344,7 @@ class VersatileExpressFlashModule(FlashModule):
             os.system('sync')
         except (IOError, OSError) as e:
             msg = 'Could not deploy images to {}; got: {}'
-            raise TargetError(msg.format(self.vemsd_mount, e))
+            raise TargetStableError(msg.format(self.vemsd_mount, e))
         self.target.boot()
         self.target.connect(timeout=30)
 
@@ -386,5 +390,4 @@ def wait_for_vemsd(vemsd_mount, tty, mcc_prompt=DEFAULT_MCC_PROMPT, short_delay=
         time.sleep(short_delay * 3)
         if os.path.exists(path):
             return
-    raise TargetError('Could not mount {}'.format(vemsd_mount))
-
+    raise TargetStableError('Could not mount {}'.format(vemsd_mount))

devlib/platform/arm.py

@@ -19,10 +19,11 @@ import tempfile
 import time
 import pexpect
 
-from devlib.platform import Platform
-from devlib.instrument import Instrument, InstrumentChannel, MeasurementsCsv, Measurement,  CONTINUOUS,  INSTANTANEOUS
-from devlib.exception import TargetError, HostError
+from devlib.exception import HostError, TargetTransientError
 from devlib.host import PACKAGE_BIN_DIRECTORY
+from devlib.instrument import (Instrument, InstrumentChannel, MeasurementsCsv,
+                               Measurement, CONTINUOUS, INSTANTANEOUS)
+from devlib.platform import Platform
 from devlib.utils.csvutil import csvreader, csvwriter
 from devlib.utils.serial_port import open_serial_connection
 
@@ -99,6 +100,7 @@ class VersatileExpressPlatform(Platform):
             addr = self._get_target_ip_address(target)
             target.connection_settings['host'] = addr
 
+    # pylint: disable=no-member
     def _get_target_ip_address(self, target):
         with open_serial_connection(port=self.serial_port,
                                     baudrate=self.baudrate,
@@ -121,7 +123,7 @@ class VersatileExpressPlatform(Platform):
                     except pexpect.TIMEOUT:
                         pass  # We have our own timeout -- see below.
                     if (time.time() - wait_start_time) > self.ready_timeout:
-                        raise TargetError('Could not acquire IP address.')
+                        raise TargetTransientError('Could not acquire IP address.')
             finally:
                 tty.sendline('exit')  # exit shell created by "su" call at the start
 
@@ -250,7 +252,7 @@ class JunoEnergyInstrument(Instrument):
         self.command = '{} -o {}'.format(self.binary, self.on_target_file)
         self.command2 = '{}'.format(self.binary)
 
-    def setup(self):
+    def setup(self):  # pylint: disable=arguments-differ
         self.binary = self.target.install(os.path.join(PACKAGE_BIN_DIRECTORY,
                                                        self.target.abi, self.binname))
         self.command = '{} -o {}'.format(self.binary, self.on_target_file)
@@ -266,6 +268,7 @@ class JunoEnergyInstrument(Instrument):
     def stop(self):
         self.target.killall(self.binname, signal='TERM', as_root=True)
 
+    # pylint: disable=arguments-differ
     def get_data(self, output_file):
         temp_file = tempfile.mktemp()
         self.target.pull(self.on_target_file, temp_file)
@@ -296,10 +299,9 @@ class JunoEnergyInstrument(Instrument):
         result = []
         output = self.target.execute(self.command2).split()
         with csvreader(output) as reader:
-            headings=next(reader)
+            headings = next(reader)
             values = next(reader)
             for chan in self.active_channels:
                 value = values[headings.index(chan.name)]
                 result.append(Measurement(value, chan))
         return result
-

devlib/platform/gem5.py

@@ -15,12 +15,13 @@
 import os
 import re
 import subprocess
-import sys
 import shutil
 import time
 import types
+import shlex
+from pipes import quote
 
-from devlib.exception import TargetError
+from devlib.exception import TargetStableError
 from devlib.host import PACKAGE_BIN_DIRECTORY
 from devlib.platform import Platform
 from devlib.utils.ssh import AndroidGem5Connection, LinuxGem5Connection
@@ -55,7 +56,7 @@ class Gem5SimulationPlatform(Platform):
         self.stdout_file = None
         self.stderr_file = None
         self.stderr_filename = None
-        if self.gem5_port is None:
+        if self.gem5_port is None:  # pylint: disable=simplifiable-if-statement
             # Allows devlib to pick up already running simulations
             self.start_gem5_simulation = True
         else:
@@ -87,12 +88,12 @@ class Gem5SimulationPlatform(Platform):
         Check if the command to start gem5 makes sense
         """
         if self.gem5args_binary is None:
-            raise TargetError('Please specify a gem5 binary.')
+            raise TargetStableError('Please specify a gem5 binary.')
         if self.gem5args_args is None:
-            raise TargetError('Please specify the arguments passed on to gem5.')
+            raise TargetStableError('Please specify the arguments passed on to gem5.')
         self.gem5args_virtio = str(self.gem5args_virtio).format(self.gem5_interact_dir)
         if self.gem5args_virtio is None:
-            raise TargetError('Please specify arguments needed for virtIO.')
+            raise TargetStableError('Please specify arguments needed for virtIO.')
 
     def _start_interaction_gem5(self):
         """
@@ -111,7 +112,7 @@ class Gem5SimulationPlatform(Platform):
             if not os.path.exists(self.stats_directory):
                 os.mkdir(self.stats_directory)
             if os.path.exists(self.gem5_out_dir):
-                raise TargetError("The gem5 stats directory {} already "
+                raise TargetStableError("The gem5 stats directory {} already "
                                   "exists.".format(self.gem5_out_dir))
             else:
                 os.mkdir(self.gem5_out_dir)
@@ -130,11 +131,11 @@ class Gem5SimulationPlatform(Platform):
             self.logger.info("Starting the gem5 simulator")
 
             command_line = "{} --outdir={} {} {}".format(self.gem5args_binary,
-                                                         self.gem5_out_dir,
+                                                         quote(self.gem5_out_dir),
                                                          self.gem5args_args,
                                                          self.gem5args_virtio)
             self.logger.debug("gem5 command line: {}".format(command_line))
-            self.gem5 = subprocess.Popen(command_line.split(),
+            self.gem5 = subprocess.Popen(shlex.split(command_line),
                                          stdout=self.stdout_file,
                                          stderr=self.stderr_file)
 
@@ -154,7 +155,7 @@ class Gem5SimulationPlatform(Platform):
         e.g. pid, input directory etc
         """
         self.logger("This functionality is not yet implemented")
-        raise TargetError()
+        raise TargetStableError()
 
     def _intercept_telnet_port(self):
         """
@@ -162,13 +163,13 @@ class Gem5SimulationPlatform(Platform):
         """
 
         if self.gem5 is None:
-            raise TargetError('The platform has no gem5 simulation! '
+            raise TargetStableError('The platform has no gem5 simulation! '
                               'Something went wrong')
         while self.gem5_port is None:
             # Check that gem5 is running!
             if self.gem5.poll():
                 message = "The gem5 process has crashed with error code {}!\n\tPlease see {} for details."
-                raise TargetError(message.format(self.gem5.poll(), self.stderr_file.name))
+                raise TargetStableError(message.format(self.gem5.poll(), self.stderr_file.name))
 
             # Open the stderr file
             with open(self.stderr_filename, 'r') as f:
@@ -186,7 +187,7 @@ class Gem5SimulationPlatform(Platform):
                     # Check if the sockets are not disabled
                     m = re.search(r"Sockets disabled, not accepting terminal connections", line)
                     if m:
-                        raise TargetError("The sockets have been disabled!"
+                        raise TargetStableError("The sockets have been disabled!"
                                           "Pass --listener-mode=on to gem5")
                 else:
                     time.sleep(1)
@@ -234,6 +235,7 @@ class Gem5SimulationPlatform(Platform):
         # Call the general update_from_target implementation
         super(Gem5SimulationPlatform, self).update_from_target(target)
 
+
     def gem5_capture_screen(self, filepath):
         file_list = os.listdir(self.gem5_out_dir)
         screen_caps = []
@@ -243,6 +245,7 @@ class Gem5SimulationPlatform(Platform):
 
         if '{ts}' in filepath:
             cmd = '{} date -u -Iseconds'
+            # pylint: disable=no-member
             ts = self.target.execute(cmd.format(self.target.busybox)).strip()
             filepath = filepath.format(ts=ts)
 
@@ -258,6 +261,7 @@ class Gem5SimulationPlatform(Platform):
                 im.save(temp_image, "PNG")
                 shutil.copy(temp_image, filepath)
                 os.remove(temp_image)
+                # pylint: disable=undefined-variable
                 gem5_logger.info("capture_screen: using gem5 screencap")
                 successful_capture = True
 
@@ -266,12 +270,14 @@ class Gem5SimulationPlatform(Platform):
 
         return successful_capture
 
+    # pylint: disable=no-self-use
     def _deploy_m5(self, target):
         # m5 is not yet installed so install it
         host_executable = os.path.join(PACKAGE_BIN_DIRECTORY,
                                        target.abi, 'm5')
         return target.install(host_executable)
 
+    # pylint: disable=no-self-use
     def _resize_shell(self, target):
         """
         Resize the shell to avoid line wrapping issues.
@@ -282,18 +288,16 @@ class Gem5SimulationPlatform(Platform):
         target.execute('reset', check_exit_code=False)
 
 # Methods that will be monkey-patched onto the target
-def _overwritten_reset(self):
-    raise TargetError('Resetting is not allowed on gem5 platforms!')
+def _overwritten_reset(self):  # pylint: disable=unused-argument
+    raise TargetStableError('Resetting is not allowed on gem5 platforms!')
 
-def _overwritten_reboot(self):
-    raise TargetError('Rebooting is not allowed on gem5 platforms!')
+def _overwritten_reboot(self):  # pylint: disable=unused-argument
+    raise TargetStableError('Rebooting is not allowed on gem5 platforms!')
 
 def _overwritten_capture_screen(self, filepath):
     connection_screencapped = self.platform.gem5_capture_screen(filepath)
-    if connection_screencapped == False:
+    if not connection_screencapped:
         # The connection was not able to capture the screen so use the target
         # implementation
         self.logger.debug('{} was not able to screen cap, using the original target implementation'.format(self.platform.__class__.__name__))
         self.target_impl_capture_screen(filepath)
-
-

devlib/trace/__init__.py

@@ -31,5 +31,13 @@ class TraceCollector(object):
     def stop(self):
         pass
 
+    def __enter__(self):
+        self.reset()
+        self.start()
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        self.stop()
+
     def get_trace(self, outfile):
         pass

devlib/trace/ftrace.py

@@ -23,7 +23,7 @@ import sys
 
 from devlib.trace import TraceCollector
 from devlib.host import PACKAGE_BIN_DIRECTORY
-from devlib.exception import TargetError, HostError
+from devlib.exception import TargetStableError, HostError
 from devlib.utils.misc import check_output, which
 
 
@@ -50,6 +50,7 @@ STATS_RE = re.compile(r'([^ ]*) +([0-9]+) +([0-9.]+) us +([0-9.]+) us +([0-9.]+)
 
 class FtraceCollector(TraceCollector):
 
+    # pylint: disable=too-many-locals,too-many-branches,too-many-statements
     def __init__(self, target,
                  events=None,
                  functions=None,
@@ -84,6 +85,7 @@ class FtraceCollector(TraceCollector):
         self.function_string = None
         self._reset_needed = True
 
+        # pylint: disable=bad-whitespace
         # Setup tracing paths
         self.available_events_file    = self.target.path.join(self.tracing_path, 'available_events')
         self.available_functions_file = self.target.path.join(self.tracing_path, 'available_filter_functions')
@@ -97,7 +99,7 @@ class FtraceCollector(TraceCollector):
         self.kernelshark = which('kernelshark')
 
         if not self.target.is_rooted:
-            raise TargetError('trace-cmd instrument cannot be used on an unrooted device.')
+            raise TargetStableError('trace-cmd instrument cannot be used on an unrooted device.')
         if self.autoreport and not self.report_on_target and self.host_binary is None:
             raise HostError('trace-cmd binary must be installed on the host if autoreport=True.')
         if self.autoview and self.kernelshark is None:
@@ -107,7 +109,7 @@ class FtraceCollector(TraceCollector):
             self.target_binary = self.target.install(host_file)
         else:
             if not self.target.is_installed('trace-cmd'):
-                raise TargetError('No trace-cmd found on device and no_install=True is specified.')
+                raise TargetStableError('No trace-cmd found on device and no_install=True is specified.')
             self.target_binary = 'trace-cmd'
 
         # Validate required events to be traced
@@ -122,10 +124,10 @@ class FtraceCollector(TraceCollector):
                 _event = '*' + event
             event_re = re.compile(_event.replace('*', '.*'))
             # Select events matching the required ones
-            if len(list(filter(event_re.match, available_events))) == 0:
+            if not list(filter(event_re.match, available_events)):
                 message = 'Event [{}] not available for tracing'.format(event)
                 if strict:
-                    raise TargetError(message)
+                    raise TargetStableError(message)
                 self.target.logger.warning(message)
             else:
                 selected_events.append(event)
@@ -133,14 +135,14 @@ class FtraceCollector(TraceCollector):
         # Thus, if not other events have been specified, try to add at least
         # a tracepoint which is always available and possibly triggered few
         # times.
-        if self.functions and len(selected_events) == 0:
+        if self.functions and not selected_events:
             selected_events = ['sched_wakeup_new']
         self.event_string = _build_trace_events(selected_events)
 
         # Check for function tracing support
         if self.functions:
             if not self.target.file_exists(self.function_profile_file):
-                raise TargetError('Function profiling not supported. '\
+                raise TargetStableError('Function profiling not supported. '\
                         'A kernel build with CONFIG_FUNCTION_PROFILER enable is required')
             # Validate required functions to be traced
             available_functions = self.target.execute(
@@ -151,7 +153,7 @@ class FtraceCollector(TraceCollector):
                 if function not in available_functions:
                     message = 'Function [{}] not available for profiling'.format(function)
                     if strict:
-                        raise TargetError(message)
+                        raise TargetStableError(message)
                     self.target.logger.warning(message)
                 else:
                     selected_functions.append(function)
@@ -237,6 +239,7 @@ class FtraceCollector(TraceCollector):
 
         if os.path.isdir(outfile):
             outfile = os.path.join(outfile, OUTPUT_PROFILE_FILE)
+        # pylint: disable=protected-access
         output = self.target._execute_util('ftrace_get_function_stats',
                                             as_root=True)
 
@@ -264,7 +267,7 @@ class FtraceCollector(TraceCollector):
 
         self.logger.debug("FTrace stats output [%s]...", outfile)
         with open(outfile, 'w') as fh:
-           json.dump(function_stats, fh, indent=4)
+            json.dump(function_stats, fh, indent=4)
         self.logger.debug("FTrace function stats save in [%s]", outfile)
 
         return function_stats
@@ -278,9 +281,9 @@ class FtraceCollector(TraceCollector):
             process = subprocess.Popen(command, stderr=subprocess.PIPE, shell=True)
             _, error = process.communicate()
             if sys.version_info[0] == 3:
-                error = error.decode(sys.stdout.encoding, 'replace')
+                error = error.decode(sys.stdout.encoding or 'utf-8', 'replace')
             if process.returncode:
-                raise TargetError('trace-cmd returned non-zero exit code {}'.format(process.returncode))
+                raise TargetStableError('trace-cmd returned non-zero exit code {}'.format(process.returncode))
             if error:
                 # logged at debug level, as trace-cmd always outputs some
                 # errors that seem benign.

devlib/trace/logcat.py

@@ -14,7 +14,6 @@
 #
 
 import os
-import re
 import shutil
 
 from devlib.trace import TraceCollector
@@ -27,6 +26,7 @@ class LogcatCollector(TraceCollector):
         self.regexps = regexps
         self._collecting = False
         self._prev_log = None
+        self._monitor = None
 
     def reset(self):
         """

devlib/trace/perf.py

@@ -0,0 +1,137 @@
+#    Copyright 2018 ARM Limited
+#
+# 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 os
+import re
+from past.builtins import basestring, zip
+
+from devlib.host import PACKAGE_BIN_DIRECTORY
+from devlib.trace import TraceCollector
+from devlib.utils.misc import ensure_file_directory_exists as _f
+
+
+PERF_COMMAND_TEMPLATE = '{} stat {} {} sleep 1000 > {} 2>&1 '
+
+PERF_COUNT_REGEX = re.compile(r'^(CPU\d+)?\s*(\d+)\s*(.*?)\s*(\[\s*\d+\.\d+%\s*\])?\s*$')
+
+DEFAULT_EVENTS = [
+    'migrations',
+    'cs',
+]
+
+
+class PerfCollector(TraceCollector):
+    """
+    Perf is a Linux profiling with performance counters.
+
+    Performance counters are CPU hardware registers that count hardware events
+    such as instructions executed, cache-misses suffered, or branches
+    mispredicted. They form a basis for profiling applications to trace dynamic
+    control flow and identify hotspots.
+
+    pref accepts options and events. If no option is given the default '-a' is
+    used. For events, the default events are migrations and cs. They both can
+    be specified in the config file.
+
+    Events must be provided as a list that contains them and they will look like
+    this ::
+
+        perf_events = ['migrations', 'cs']
+
+    Events can be obtained by typing the following in the command line on the
+    device ::
+
+        perf list
+
+    Whereas options, they can be provided as a single string as following ::
+
+        perf_options = '-a -i'
+
+    Options can be obtained by running the following in the command line ::
+
+        man perf-stat
+    """
+
+    def __init__(self, target,
+                 events=None,
+                 optionstring=None,
+                 labels=None,
+                 force_install=False):
+        super(PerfCollector, self).__init__(target)
+        self.events = events if events else DEFAULT_EVENTS
+        self.force_install = force_install
+        self.labels = labels
+
+        # Validate parameters
+        if isinstance(optionstring, list):
+            self.optionstrings = optionstring
+        else:
+            self.optionstrings = [optionstring]
+        if self.events and isinstance(self.events, basestring):
+            self.events = [self.events]
+        if not self.labels:
+            self.labels = ['perf_{}'.format(i) for i in range(len(self.optionstrings))]
+        if len(self.labels) != len(self.optionstrings):
+            raise ValueError('The number of labels must match the number of optstrings provided for perf.')
+
+        self.binary = self.target.get_installed('perf')
+        if self.force_install or not self.binary:
+            self.binary = self._deploy_perf()
+
+        self.commands = self._build_commands()
+
+    def reset(self):
+        self.target.killall('perf', as_root=self.target.is_rooted)
+        for label in self.labels:
+            filepath = self._get_target_outfile(label)
+            self.target.remove(filepath)
+
+    def start(self):
+        for command in self.commands:
+            self.target.kick_off(command)
+
+    def stop(self):
+        self.target.killall('sleep', as_root=self.target.is_rooted)
+
+    # pylint: disable=arguments-differ
+    def get_trace(self, outdir):
+        for label in self.labels:
+            target_file = self._get_target_outfile(label)
+            host_relpath = os.path.basename(target_file)
+            host_file = _f(os.path.join(outdir, host_relpath))
+            self.target.pull(target_file, host_file)
+
+    def _deploy_perf(self):
+        host_executable = os.path.join(PACKAGE_BIN_DIRECTORY,
+                                       self.target.abi, 'perf')
+        return self.target.install(host_executable)
+
+    def _build_commands(self):
+        commands = []
+        for opts, label in zip(self.optionstrings, self.labels):
+            commands.append(self._build_perf_command(opts, self.events, label))
+        return commands
+
+    def _get_target_outfile(self, label):
+        return self.target.get_workpath('{}.out'.format(label))
+
+    def _build_perf_command(self, options, events, label):
+        event_string = ' '.join(['-e {}'.format(e) for e in events])
+        command = PERF_COMMAND_TEMPLATE.format(self.binary,
+                                               options or '',
+                                               event_string,
+                                               self._get_target_outfile(label))
+        return command

devlib/trace/serial_trace.py

@@ -13,9 +13,9 @@
 # limitations under the License.
 #
 
-from pexpect.exceptions import TIMEOUT
 import shutil
 from tempfile import NamedTemporaryFile
+from pexpect.exceptions import TIMEOUT
 
 from devlib.trace import TraceCollector
 from devlib.utils.serial_port import get_connection
@@ -52,7 +52,8 @@ class SerialTraceCollector(TraceCollector):
 
 
         self._tmpfile = NamedTemporaryFile()
-        self._tmpfile.write("-------- Starting serial logging --------\n")
+        start_marker = "-------- Starting serial logging --------\n"
+        self._tmpfile.write(start_marker.encode('utf-8'))
 
         self._serial_target, self._conn = get_connection(port=self.serial_port,
                                                          baudrate=self.baudrate,
@@ -76,7 +77,8 @@ class SerialTraceCollector(TraceCollector):
         self._serial_target.close()
         del self._conn
 
-        self._tmpfile.write("-------- Stopping serial logging --------\n")
+        stop_marker = "-------- Stopping serial logging --------\n"
+        self._tmpfile.write(stop_marker.encode('utf-8'))
 
         self._collecting = False
 

devlib/trace/systrace.py

@@ -13,30 +13,15 @@
 # limitations under the License.
 #
 
-#    Copyright 2018 Arm Limited
-#
-# 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 os
 import subprocess
 
 from shutil import copyfile
 from tempfile import NamedTemporaryFile
 
-from devlib.exception import TargetError, HostError
+from devlib.exception import TargetStableError, HostError
 from devlib.trace import TraceCollector
-from devlib.utils.android import platform_tools
+import devlib.utils.android
 from devlib.utils.misc import memoized
 
 
@@ -74,13 +59,11 @@ class SystraceCollector(TraceCollector):
     @property
     @memoized
     def available_categories(self):
-        lines = subprocess.check_output([self.systrace_binary, '-l']).splitlines()
+        lines = subprocess.check_output(
+            [self.systrace_binary, '-l'], universal_newlines=True
+        ).splitlines()
 
-        categories = []
-        for line in lines:
-            categories.append(line.split()[0])
-
-        return categories
+        return [line.split()[0] for line in lines if line]
 
     def __init__(self, target,
                  categories=None,
@@ -98,6 +81,7 @@ class SystraceCollector(TraceCollector):
         # Try to find a systrace binary
         self.systrace_binary = None
 
+        platform_tools = devlib.utils.android.platform_tools
         systrace_binary_path = os.path.join(platform_tools, 'systrace', 'systrace.py')
         if not os.path.isfile(systrace_binary_path):
             raise HostError('Could not find any systrace binary under {}'.format(platform_tools))
@@ -109,12 +93,12 @@ class SystraceCollector(TraceCollector):
             if category not in self.available_categories:
                 message = 'Category [{}] not available for tracing'.format(category)
                 if strict:
-                    raise TargetError(message)
+                    raise TargetStableError(message)
                 self.logger.warning(message)
 
         self.categories = list(set(self.categories) & set(self.available_categories))
         if not self.categories:
-            raise TargetError('None of the requested categories are available')
+            raise TargetStableError('None of the requested categories are available')
 
     def __del__(self):
         self.reset()
@@ -122,6 +106,7 @@ class SystraceCollector(TraceCollector):
     def _build_cmd(self):
         self._tmpfile = NamedTemporaryFile()
 
+        # pylint: disable=attribute-defined-outside-init
         self.systrace_cmd = '{} -o {} -e {}'.format(
             self.systrace_binary,
             self._tmpfile.name,
@@ -152,7 +137,8 @@ class SystraceCollector(TraceCollector):
         self._systrace_process = subprocess.Popen(
             self.systrace_cmd,
             stdin=subprocess.PIPE,
-            shell=True
+            shell=True,
+            universal_newlines=True
         )
 
     def stop(self):

devlib/utils/__init__.py

@@ -12,5 +12,3 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-
-

devlib/utils/android.py

@@ -20,21 +20,21 @@ Utility functions for working with Android devices through adb.
 """
 # pylint: disable=E1103
 import os
-import pexpect
-import time
-import subprocess
-import logging
 import re
-import threading
-import tempfile
-import queue
 import sys
+import time
+import logging
+import tempfile
+import subprocess
 from collections import defaultdict
+import pexpect
+import xml.etree.ElementTree
+import zipfile
 
-from devlib.exception import TargetError, HostError, DevlibError
-from devlib.utils.misc import check_output, which, memoized, ABI_MAP
-from devlib.utils.misc import escape_single_quotes, escape_double_quotes
-from devlib import host
+from pipes import quote
+
+from devlib.exception import TargetTransientError, TargetStableError, HostError
+from devlib.utils.misc import check_output, which, ABI_MAP
 
 
 logger = logging.getLogger('android')
@@ -117,6 +117,7 @@ class AdbDevice(object):
         self.name = name
         self.status = status
 
+    # pylint: disable=undefined-variable
     def __cmp__(self, other):
         if isinstance(other, AdbDevice):
             return cmp(self.name, other.name)
@@ -134,6 +135,7 @@ class ApkInfo(object):
     version_regex = re.compile(r"name='(?P<name>[^']+)' versionCode='(?P<vcode>[^']+)' versionName='(?P<vname>[^']+)'")
     name_regex = re.compile(r"name='(?P<name>[^']+)'")
     permission_regex = re.compile(r"name='(?P<permission>[^']+)'")
+    activity_regex = re.compile(r'\s*A:\s*android:name\(0x\d+\)=".(?P<name>\w+)"')
 
     def __init__(self, path=None):
         self.path = path
@@ -146,17 +148,10 @@ class ApkInfo(object):
         self.permissions = []
         self.parse(path)
 
+    # pylint: disable=too-many-branches
     def parse(self, apk_path):
         _check_env()
-        command = [aapt, 'dump', 'badging', apk_path]
-        logger.debug(' '.join(command))
-        try:
-            output = subprocess.check_output(command, stderr=subprocess.STDOUT)
-            if sys.version_info[0] == 3:
-                output = output.decode(sys.stdout.encoding, 'replace')
-        except subprocess.CalledProcessError as e:
-            raise HostError('Error parsing APK file {}. `aapt` says:\n{}'
-                            .format(apk_path, e.output))
+        output = self._run([aapt, 'dump', 'badging', apk_path])
         for line in output.split('\n'):
             if line.startswith('application-label:'):
                 self.label = line.split(':')[1].strip().replace('\'', '')
@@ -189,6 +184,50 @@ class ApkInfo(object):
             else:
                 pass  # not interested
 
+        self._apk_path = apk_path
+        self._activities = None
+        self._methods = None
+
+    @property
+    def activities(self):
+        if self._activities is None:
+            cmd = [aapt, 'dump', 'xmltree', self._apk_path,
+                   'AndroidManifest.xml']
+            matched_activities = self.activity_regex.finditer(self._run(cmd))
+            self._activities = [m.group('name') for m in matched_activities]
+        return self._activities
+
+    @property
+    def methods(self):
+        if self._methods is None:
+            with zipfile.ZipFile(self._apk_path, 'r') as z:
+                extracted = z.extract('classes.dex', tempfile.gettempdir())
+
+            dexdump = os.path.join(os.path.dirname(aapt), 'dexdump')
+            command = [dexdump, '-l', 'xml', extracted]
+            dump = self._run(command)
+
+            xml_tree = xml.etree.ElementTree.fromstring(dump)
+
+            package = next(i for i in xml_tree.iter('package')
+                           if i.attrib['name'] == self.package)
+
+            self._methods = [(meth.attrib['name'], klass.attrib['name'])
+                             for klass in package.iter('class')
+                             for meth in klass.iter('method')]
+        return self._methods
+
+    def _run(self, command):
+        logger.debug(' '.join(command))
+        try:
+            output = subprocess.check_output(command, stderr=subprocess.STDOUT)
+            if sys.version_info[0] == 3:
+                output = output.decode(sys.stdout.encoding or 'utf-8', 'replace')
+        except subprocess.CalledProcessError as e:
+            raise HostError('Error while running "{}":\n{}'
+                            .format(command, e.output))
+        return output
+
 
 class AdbConnection(object):
 
@@ -222,6 +261,7 @@ class AdbConnection(object):
             self.ls_command = 'ls'
         logger.debug("ls command is set to {}".format(self.ls_command))
 
+    # pylint: disable=unused-argument
     def __init__(self, device=None, timeout=None, platform=None, adb_server=None):
         self.timeout = timeout if timeout is not None else self.default_timeout
         if device is None:
@@ -235,7 +275,7 @@ class AdbConnection(object):
     def push(self, source, dest, timeout=None):
         if timeout is None:
             timeout = self.timeout
-        command = "push '{}' '{}'".format(source, dest)
+        command = "push {} {}".format(quote(source), quote(dest))
         if not os.path.exists(source):
             raise HostError('No such file "{}"'.format(source))
         return adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server)
@@ -249,19 +289,26 @@ class AdbConnection(object):
             command = 'shell {} {}'.format(self.ls_command, source)
             output = adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server)
             for line in output.splitlines():
-                command = "pull '{}' '{}'".format(line.strip(), dest)
+                command = "pull {} {}".format(quote(line.strip()), quote(dest))
                 adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server)
             return
-        command = "pull '{}' '{}'".format(source, dest)
+        command = "pull {} {}".format(quote(source), quote(dest))
         return adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server)
 
+    # pylint: disable=unused-argument
     def execute(self, command, timeout=None, check_exit_code=False,
-                as_root=False, strip_colors=True):
-        return adb_shell(self.device, command, timeout, check_exit_code,
-                         as_root, adb_server=self.adb_server)
+                as_root=False, strip_colors=True, will_succeed=False):
+        try:
+            return adb_shell(self.device, command, timeout, check_exit_code,
+                             as_root, adb_server=self.adb_server)
+        except TargetStableError as e:
+            if will_succeed:
+                raise TargetTransientError(e)
+            else:
+                raise
 
     def background(self, command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, as_root=False):
-        return adb_background_shell(self.device, command, stdout, stderr, as_root)
+        return adb_background_shell(self.device, command, stdout, stderr, as_root, adb_server=self.adb_server)
 
     def close(self):
         AdbConnection.active_connections[self.device] -= 1
@@ -278,7 +325,7 @@ class AdbConnection(object):
 
 def fastboot_command(command, timeout=None, device=None):
     _check_env()
-    target = '-s {}'.format(device) if device else ''
+    target = '-s {}'.format(quote(device)) if device else ''
     full_command = 'fastboot {} {}'.format(target, command)
     logger.debug(full_command)
     output, _ = check_output(full_command, timeout, shell=True)
@@ -286,7 +333,7 @@ def fastboot_command(command, timeout=None, device=None):
 
 
 def fastboot_flash_partition(partition, path_to_image):
-    command = 'flash {} {}'.format(partition, path_to_image)
+    command = 'flash {} {}'.format(quote(partition), quote(path_to_image))
     fastboot_command(command)
 
 
@@ -334,7 +381,7 @@ def adb_connect(device, timeout=None, attempts=MAX_ATTEMPTS):
         tries += 1
         if device:
             if "." in device: # Connect is required only for ADB-over-IP
-                command = 'adb connect {}'.format(device)
+                command = 'adb connect {}'.format(quote(device))
                 logger.debug(command)
                 output, _ = check_output(command, shell=True, timeout=timeout)
         if _ping(device):
@@ -356,26 +403,27 @@ def adb_disconnect(device):
         logger.debug(command)
         retval = subprocess.call(command, stdout=open(os.devnull, 'wb'), shell=True)
         if retval:
-            raise TargetError('"{}" returned {}'.format(command, retval))
+            raise TargetTransientError('"{}" returned {}'.format(command, retval))
 
 
 def _ping(device):
     _check_env()
-    device_string = ' -s {}'.format(device) if device else ''
+    device_string = ' -s {}'.format(quote(device)) if device else ''
     command = "adb{} shell \"ls /data/local/tmp > /dev/null\"".format(device_string)
     logger.debug(command)
     result = subprocess.call(command, stderr=subprocess.PIPE, shell=True)
-    if not result:
+    if not result:  # pylint: disable=simplifiable-if-statement
         return True
     else:
         return False
 
 
+# pylint: disable=too-many-locals
 def adb_shell(device, command, timeout=None, check_exit_code=False,
               as_root=False, adb_server=None):  # NOQA
     _check_env()
     if as_root:
-        command = 'echo \'{}\' | su'.format(escape_single_quotes(command))
+        command = 'echo {} | su'.format(quote(command))
     device_part = []
     if adb_server:
         device_part = ['-H', adb_server]
@@ -392,7 +440,7 @@ def adb_shell(device, command, timeout=None, check_exit_code=False,
     try:
         raw_output, _ = check_output(actual_command, timeout, shell=False, combined_output=True)
     except subprocess.CalledProcessError as e:
-        raise TargetError(str(e))
+        raise TargetStableError(str(e))
 
     if raw_output:
         try:
@@ -411,19 +459,19 @@ def adb_shell(device, command, timeout=None, check_exit_code=False,
             if int(exit_code):
                 message = ('Got exit code {}\nfrom target command: {}\n'
                            'OUTPUT: {}')
-                raise TargetError(message.format(exit_code, command, output))
+                raise TargetStableError(message.format(exit_code, command, output))
             elif re_search:
                 message = 'Could not start activity; got the following:\n{}'
-                raise TargetError(message.format(re_search[0]))
+                raise TargetStableError(message.format(re_search[0]))
         else:  # not all digits
             if re_search:
                 message = 'Could not start activity; got the following:\n{}'
-                raise TargetError(message.format(re_search[0]))
+                raise TargetStableError(message.format(re_search[0]))
             else:
                 message = 'adb has returned early; did not get an exit code. '\
                           'Was kill-server invoked?\nOUTPUT:\n-----\n{}\n'\
                           '-----'
-                raise TargetError(message.format(raw_output))
+                raise TargetTransientError(message.format(raw_output))
 
     return output
 
@@ -431,19 +479,22 @@ def adb_shell(device, command, timeout=None, check_exit_code=False,
 def adb_background_shell(device, command,
                          stdout=subprocess.PIPE,
                          stderr=subprocess.PIPE,
-                         as_root=False):
+                         as_root=False,
+                         adb_server=None):
     """Runs the sepcified command in a subprocess, returning the the Popen object."""
     _check_env()
     if as_root:
-        command = 'echo \'{}\' | su'.format(escape_single_quotes(command))
-    device_string = ' -s {}'.format(device) if device else ''
-    full_command = 'adb{} shell "{}"'.format(device_string, escape_double_quotes(command))
+        command = 'echo {} | su'.format(quote(command))
+
+    device_string = ' -H {}'.format(adb_server) if adb_server else ''
+    device_string += ' -s {}'.format(device) if device else ''
+    full_command = 'adb{} shell {}'.format(device_string, quote(command))
     logger.debug(full_command)
     return subprocess.Popen(full_command, stdout=stdout, stderr=stderr, shell=True)
 
 
 def adb_list_devices(adb_server=None):
-    output = adb_command(None, 'devices',adb_server=adb_server)
+    output = adb_command(None, 'devices', adb_server=adb_server)
     devices = []
     for line in output.splitlines():
         parts = [p.strip() for p in line.split()]
@@ -452,7 +503,7 @@ def adb_list_devices(adb_server=None):
     return devices
 
 
-def get_adb_command(device, command, timeout=None,adb_server=None):
+def get_adb_command(device, command, adb_server=None):
     _check_env()
     device_string = ""
     if adb_server != None:
@@ -460,8 +511,8 @@ def get_adb_command(device, command, timeout=None,adb_server=None):
     device_string += ' -s {}'.format(device) if device else ''
     return "adb{} {}".format(device_string, command)
 
-def adb_command(device, command, timeout=None,adb_server=None):
-    full_command = get_adb_command(device, command, timeout, adb_server)
+def adb_command(device, command, timeout=None, adb_server=None):
+    full_command = get_adb_command(device, command, adb_server)
     logger.debug(full_command)
     output, _ = check_output(full_command, timeout, shell=True)
     return output
@@ -482,7 +533,7 @@ def grant_app_permissions(target, package):
     for permission in permissions:
         try:
             target.execute('pm grant {} {}'.format(package, permission))
-        except TargetError:
+        except TargetStableError:
             logger.debug('Cannot grant {}'.format(permission))
 
 
@@ -575,6 +626,8 @@ class LogcatMonitor(object):
 
         self.target = target
         self._regexps = regexps
+        self._logcat = None
+        self._logfile = None
 
     def start(self, outfile=None):
         """
@@ -600,9 +653,9 @@ class LogcatMonitor(object):
             # Logcat on older version of android do not support the -e argument
             # so fall back to using grep.
             if self.target.get_sdk_version() > 23:
-                logcat_cmd = '{} -e "{}"'.format(logcat_cmd, regexp)
+                logcat_cmd = '{} -e {}'.format(logcat_cmd, quote(regexp))
             else:
-                logcat_cmd = '{} | grep "{}"'.format(logcat_cmd, regexp)
+                logcat_cmd = '{} | grep {}'.format(logcat_cmd, quote(regexp))
 
         logcat_cmd = get_adb_command(self.target.conn.device, logcat_cmd)
 
@@ -651,7 +704,7 @@ class LogcatMonitor(object):
             return [line for line in fh]
 
     def clear_log(self):
-        with open(self._logfile.name, 'w') as fh:
+        with open(self._logfile.name, 'w') as _:
             pass
 
     def search(self, regexp):
@@ -679,7 +732,7 @@ class LogcatMonitor(object):
         res = [line for line in log if re.match(regexp, line)]
 
         # Found some matches, return them
-        if len(res) > 0:
+        if res:
             return res
 
         # Store the number of lines we've searched already, so we don't have to

devlib/utils/gem5.py

@@ -28,7 +28,7 @@ logger = logging.getLogger('gem5')
 
 def iter_statistics_dump(stats_file):
     '''
-    Yields statistics dumps as dicts. The parameter is assumed to be a stream 
+    Yields statistics dumps as dicts. The parameter is assumed to be a stream
     reading from the statistics log file.
     '''
     cur_dump = {}
@@ -40,14 +40,13 @@ def iter_statistics_dump(stats_file):
             yield cur_dump
             cur_dump = {}
         else:
-            res = GEM5STATS_FIELD_REGEX.match(line) 
+            res = GEM5STATS_FIELD_REGEX.match(line)
             if res:
                 k = res.group("key")
                 vtext = res.group("value")
                 try:
                     v = list(map(numeric, vtext.split()))
-                    cur_dump[k] = v[0] if len(v)==1 else set(v)
+                    cur_dump[k] = v[0] if len(v) == 1 else set(v)
                 except ValueError:
                     msg = 'Found non-numeric entry in gem5 stats ({}: {})'
                     logger.warning(msg.format(k, vtext))
-

devlib/utils/misc.py

@@ -19,27 +19,29 @@ Miscellaneous functions that don't fit anywhere else.
 
 """
 from __future__ import division
-import os
-import sys
-import re
-import string
-import threading
-import signal
-import subprocess
-import pkgutil
-import logging
-import random
-import ctypes
-import threading
-from operator import itemgetter
+from functools import partial, reduce
 from itertools import groupby
-from functools import partial
+from operator import itemgetter
 
+import ctypes
+import logging
+import os
+import pkgutil
+import random
+import re
+import signal
+import string
+import subprocess
+import sys
+import threading
 import wrapt
+import warnings
+
+
 from past.builtins import basestring
 
+# pylint: disable=redefined-builtin
 from devlib.exception import HostError, TimeoutError
-from functools import reduce
 
 
 # ABI --> architectures list
@@ -182,9 +184,9 @@ def check_output(command, timeout=None, ignore=None, inputtext=None,
         output, error = process.communicate(inputtext)
         if sys.version_info[0] == 3:
             # Currently errors=replace is needed as 0x8c throws an error
-            output = output.decode(sys.stdout.encoding, "replace")
+            output = output.decode(sys.stdout.encoding or 'utf-8', "replace")
             if error:
-                error =  error.decode(sys.stderr.encoding, "replace")
+                error = error.decode(sys.stderr.encoding or 'utf-8', "replace")
     finally:
         if timeout:
             timer.cancel()
@@ -415,24 +417,50 @@ def convert_new_lines(text):
     """ Convert new lines to a common format.  """
     return text.replace('\r\n', '\n').replace('\r', '\n')
 
+def sanitize_cmd_template(cmd):
+    msg = (
+        '''Quoted placeholder should not be used, as it will result in quoting the text twice. {} should be used instead of '{}' or "{}" in the template: '''
+    )
+    for unwanted in ('"{}"', "'{}'"):
+        if unwanted in cmd:
+            warnings.warn(msg + cmd, stacklevel=2)
+            cmd = cmd.replace(unwanted, '{}')
+
+    return cmd
 
 def escape_quotes(text):
-    """Escape quotes, and escaped quotes, in the specified text."""
+    """
+    Escape quotes, and escaped quotes, in the specified text.
+
+    .. note:: :func:`pipes.quote` should be favored where possible.
+    """
     return re.sub(r'\\("|\')', r'\\\\\1', text).replace('\'', '\\\'').replace('\"', '\\\"')
 
 
 def escape_single_quotes(text):
-    """Escape single quotes, and escaped single quotes, in the specified text."""
+    """
+    Escape single quotes, and escaped single quotes, in the specified text.
+
+    .. note:: :func:`pipes.quote` should be favored where possible.
+    """
     return re.sub(r'\\("|\')', r'\\\\\1', text).replace('\'', '\'\\\'\'')
 
 
 def escape_double_quotes(text):
-    """Escape double quotes, and escaped double quotes, in the specified text."""
+    """
+    Escape double quotes, and escaped double quotes, in the specified text.
+
+    .. note:: :func:`pipes.quote` should be favored where possible.
+    """
     return re.sub(r'\\("|\')', r'\\\\\1', text).replace('\"', '\\\"')
 
 
 def escape_spaces(text):
-    """Escape spaces in the specified text"""
+    """
+    Escape spaces in the specified text
+
+    .. note:: :func:`pipes.quote` should be favored where possible.
+    """
     return text.replace(' ', '\ ')
 
 
@@ -523,6 +551,12 @@ def get_random_string(length):
 
 class LoadSyntaxError(Exception):
 
+    @property
+    def message(self):
+        if self.args:
+            return self.args[0]
+        return str(self)
+
     def __init__(self, message, filepath, lineno):
         super(LoadSyntaxError, self).__init__(message)
         self.filepath = filepath
@@ -535,6 +569,7 @@ class LoadSyntaxError(Exception):
 
 RAND_MOD_NAME_LEN = 30
 BAD_CHARS = string.punctuation + string.whitespace
+# pylint: disable=no-member
 if sys.version_info[0] == 3:
     TRANS_TABLE = str.maketrans(BAD_CHARS, '_' * len(BAD_CHARS))
 else:
@@ -639,17 +674,24 @@ def __get_memo_id(obj):
 
 
 @wrapt.decorator
-def memoized(wrapped, instance, args, kwargs):
-    """A decorator for memoizing functions and methods."""
+def memoized(wrapped, instance, args, kwargs):  # pylint: disable=unused-argument
+    """
+    A decorator for memoizing functions and methods.
+
+    .. warning:: this may not detect changes to mutable types. As long as the
+                 memoized function was used with an object as an argument
+                 before, the cached result will be returned, even if the
+                 structure of the object (e.g. a list) has changed in the mean time.
+
+    """
     func_id = repr(wrapped)
 
     def memoize_wrapper(*args, **kwargs):
         id_string = func_id + ','.join([__get_memo_id(a) for a in  args])
-        id_string += ','.join('{}={}'.format(k, v)
+        id_string += ','.join('{}={}'.format(k, __get_memo_id(v))
                               for k, v in kwargs.items())
         if id_string not in __memo_cache:
             __memo_cache[id_string] = wrapped(*args, **kwargs)
         return __memo_cache[id_string]
 
     return memoize_wrapper(*args, **kwargs)
-

devlib/utils/parse_aep.py

@@ -28,18 +28,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import os
-import sys
 import getopt
-import subprocess
 import logging
 import signal
-import serial
-import time
-import math
+import sys
 
 logger = logging.getLogger('aep-parser')
 
+# pylint: disable=attribute-defined-outside-init
 class AepParser(object):
     prepared = False
 
@@ -94,7 +90,7 @@ class AepParser(object):
                 continue
 
             if parent not in virtual:
-                virtual[parent] = { supply : index }
+                virtual[parent] = {supply : index}
 
             virtual[parent][supply] = index
 
@@ -102,7 +98,7 @@ class AepParser(object):
         # child
         for supply in list(virtual.keys()):
             if len(virtual[supply]) == 1:
-                del virtual[supply];
+                del virtual[supply]
 
         for supply in list(virtual.keys()):
             # Add label, hide and duplicate columns for virtual domains
@@ -121,7 +117,7 @@ class AepParser(object):
 
         label[0] = array[0]
         unit[0] = "(S)"
-        for i in range(1,len(array)):
+        for i in range(1, len(array)):
             label[i] = array[i][:-3]
             unit[i] = array[i][-3:]
 
@@ -138,7 +134,7 @@ class AepParser(object):
         # By default we assume that there is no child
         duplicate = [0] * len(label)
 
-        for i in range(len(label)):
+        for i in range(len(label)):  # pylint: disable=consider-using-enumerate
             # We only care about time and Watt
             if label[i] == 'time':
                 hide[i] = 0
@@ -167,7 +163,7 @@ class AepParser(object):
     @staticmethod
     def parse_text(array, hide):
         data = [0]*len(array)
-        for i in range(len(array)):
+        for i in range(len(array)):  # pylint: disable=consider-using-enumerate
             if hide[i]:
                 continue
 
@@ -193,18 +189,18 @@ class AepParser(object):
         return data
 
     @staticmethod
-    def delta_nrj(array, delta, min, max, hide):
+    def delta_nrj(array, delta, minimu, maximum, hide):
     # Compute the energy consumed in this time slice and add it
     # delta[0] is used to save the last time stamp
 
-        if (delta[0] < 0):
+        if delta[0] < 0:
             delta[0] = array[0]
 
         time = array[0] - delta[0]
-        if (time <= 0):
+        if time <= 0:
             return delta
 
-        for i in range(len(array)):
+        for i in range(len(array)):  # pylint: disable=consider-using-enumerate
             if hide[i]:
                 continue
 
@@ -213,10 +209,10 @@ class AepParser(object):
             except ValueError:
                 continue
 
-            if (data < min[i]):
-                min[i] = data
-            if (data > max[i]):
-                max[i] = data
+            if data < minimu[i]:
+                minimu[i] = data
+            if data > maximum[i]:
+                maximum[i] = data
             delta[i] += time * data
 
         # save last time stamp
@@ -225,11 +221,11 @@ class AepParser(object):
         return delta
 
     def output_label(self, label, hide):
-        self.fo.write(label[0]+"(uS)")
+        self.fo.write(label[0] + "(uS)")
         for i in range(1, len(label)):
             if hide[i]:
                 continue
-            self.fo.write(" "+label[i]+"(uW)")
+            self.fo.write(" " + label[i] + "(uW)")
 
         self.fo.write("\n")
 
@@ -248,34 +244,34 @@ class AepParser(object):
 
         self.fo.write("\n")
 
-    def prepare(self, infile, outfile, summaryfile):
-
+    # pylint: disable-redefined-outer-name,
+    def prepare(self, input_file, outfile, summaryfile):
         try:
-            self.fi = open(infile, "r")
+            self.fi = open(input_file, "r")
         except IOError:
-            logger.warn('Unable to open input file {}'.format(infile))
-            logger.warn('Usage: parse_arp.py -i <inputfile> [-o <outputfile>]')
+            logger.warning('Unable to open input file {}'.format(input_file))
+            logger.warning('Usage: parse_arp.py -i <inputfile> [-o <outputfile>]')
             sys.exit(2)
 
         self.parse = True
-        if len(outfile) > 0:
+        if outfile:
             try:
                 self.fo = open(outfile, "w")
             except IOError:
-                logger.warn('Unable to create {}'.format(outfile))
+                logger.warning('Unable to create {}'.format(outfile))
                 self.parse = False
         else:
-                self.parse = False
+            self.parse = False
 
         self.summary = True
-        if len(summaryfile) > 0:
+        if summaryfile:
             try:
                 self.fs = open(summaryfile, "w")
             except IOError:
-                logger.warn('Unable to create {}'.format(summaryfile))
+                logger.warning('Unable to create {}'.format(summaryfile))
                 self.fs = sys.stdout
         else:
-                self.fs = sys.stdout
+            self.fs = sys.stdout
 
         self.prepared = True
 
@@ -291,7 +287,8 @@ class AepParser(object):
 
         self.prepared = False
 
-    def parse_aep(self, start=0, lenght=-1):
+    # pylint: disable=too-many-branches,too-many-statements,redefined-outer-name,too-many-locals
+    def parse_aep(self, start=0, length=-1):
     # Parse aep data and calculate the energy consumed
         begin = 0
 
@@ -302,7 +299,7 @@ class AepParser(object):
         lines = self.fi.readlines()
 
         for myline in lines:
-            array  = myline.split()
+            array = myline.split()
 
             if "#" in myline:
                 # update power topology
@@ -331,8 +328,8 @@ class AepParser(object):
 
                 # Init arrays
                 nrj = [0]*len(label)
-                min = [100000000]*len(label)
-                max = [0]*len(label)
+                minimum = [100000000]*len(label)
+                maximum = [0]*len(label)
                 offset = [0]*len(label)
 
                 continue
@@ -342,21 +339,21 @@ class AepParser(object):
 
             # get 1st time stamp
             if begin <= 0:
-                being = data[0]
+                begin = data[0]
 
             # skip data before start
             if (data[0]-begin) < start:
                 continue
 
-            # stop after lenght
-            if lenght >= 0 and (data[0]-begin) > (start + lenght):
+            # stop after length
+            if length >= 0 and (data[0]-begin) > (start + length):
                 continue
 
             # add virtual domains
             data = self.add_virtual_data(data, virtual)
 
             # extract power figures
-            self.delta_nrj(data, nrj, min, max, hide)
+            self.delta_nrj(data, nrj, minimum, maximum, hide)
 
             # write data into new file
             if self.parse:
@@ -365,7 +362,6 @@ class AepParser(object):
         # if there is no data just return
         if label_line or len(nrj) == 1:
             raise ValueError('No data found in the data file. Please check the Arm Energy Probe')
-            return
 
         # display energy consumption of each channel and total energy consumption
         total = 0
@@ -377,27 +373,33 @@ class AepParser(object):
             nrj[i] -= offset[i] * nrj[0]
 
             total_nrj = nrj[i]/1000000000000.0
-            duration = (max[0]-min[0])/1000000.0
+            duration = (maximum[0]-minimum[0])/1000000.0
             channel_name = label[i]
             average_power = total_nrj/duration
 
-            self.fs.write("Total nrj: %8.3f J for %s -- duration %8.3f sec -- min %8.3f W -- max %8.3f W\n" % (nrj[i]/1000000000000.0, label[i], (max[0]-min[0])/1000000.0, min[i]/1000000.0, max[i]/1000000.0))
+            total = nrj[i]/1000000000000.0
+            duration = (maximum[0]-minimum[0])/1000000.0
+            min_power = minimum[i]/1000000.0
+            max_power = maximum[i]/1000000.0
+            output = "Total nrj: %8.3f J for %s -- duration %8.3f sec -- min %8.3f W -- max %8.3f W\n"
+            self.fs.write(output.format(total, label[i], duration, min_power, max_power))
 
             # store each AEP channel info  except Platform in the results table
             results_table[channel_name] = total_nrj, average_power
 
-            if (min[i] < offset[i]):
-                self.fs.write ("!!! Min below offset\n")
+            if minimum[i] < offset[i]:
+                self.fs.write("!!! Min below offset\n")
 
             if duplicate[i]:
                 continue
 
             total += nrj[i]
 
-        self.fs.write ("Total nrj: %8.3f J for %s -- duration %8.3f sec\n" % (total/1000000000000.0, "Platform ", (max[0]-min[0])/1000000.0))
+        output = "Total nrj: %8.3f J for Platform  -- duration %8.3f sec\n"
+        self.fs.write(output.format(total/1000000000000.0, (maximum[0]-minimum[0])/1000000.0))
 
         total_nrj = total/1000000000000.0
-        duration = (max[0]-min[0])/1000000.0
+        duration = (maximum[0]-minimum[0])/1000000.0
         average_power = total_nrj/duration
 
         # store AEP Platform channel info in the results table
@@ -405,11 +407,12 @@ class AepParser(object):
 
         return results_table
 
+    # pylint: disable=too-many-branches,no-self-use,too-many-locals
     def topology_from_config(self, topofile):
         try:
             ft = open(topofile, "r")
         except IOError:
-            logger.warn('Unable to open config file {}'.format(topofile))
+            logger.warning('Unable to open config file {}'.format(topofile))
             return
         lines = ft.readlines()
 
@@ -451,10 +454,11 @@ class AepParser(object):
             topo[items[0]] = info
 
             # Increase index
-            index +=1
+            index += 1
 
 
         # Create an entry for each virtual parent
+        # pylint: disable=consider-iterating-dictionary
         for supply in topo.keys():
             # Parent is in the topology
             parent = topo[supply]['parent']
@@ -462,23 +466,25 @@ class AepParser(object):
                 continue
 
             if parent not in virtual:
-                virtual[parent] = { supply : topo[supply]['index'] }
+                virtual[parent] = {supply : topo[supply]['index']}
 
             virtual[parent][supply] = topo[supply]['index']
 
 
         # Remove parent with 1 child as they don't give more information than their
         # child
+        # pylint: disable=consider-iterating-dictionary
         for supply in list(virtual.keys()):
             if len(virtual[supply]) == 1:
-                del virtual[supply];
+                del virtual[supply]
 
         topo_list = ['']*(1+len(topo)+len(virtual))
         topo_list[0] = 'time'
+        # pylint: disable=consider-iterating-dictionary
         for chnl in topo.keys():
             topo_list[topo[chnl]['index']] = chnl
         for chnl in virtual.keys():
-            index +=1
+            index += 1
             topo_list[index] = chnl
 
         ft.close()
@@ -490,6 +496,7 @@ class AepParser(object):
 
 if __name__ == '__main__':
 
+    # pylint: disable=unused-argument
     def handleSigTERM(signum, frame):
         sys.exit(2)
 
@@ -501,11 +508,11 @@ if __name__ == '__main__':
     ch.setLevel(logging.DEBUG)
     logger.addHandler(ch)
 
-    infile = ""
-    outfile = ""
+    in_file = ""
+    out_file = ""
     figurefile = ""
     start = 0
-    lenght = -1
+    length = -1
 
     try:
         opts, args = getopt.getopt(sys.argv[1:], "i:vo:s:l:t:")
@@ -515,22 +522,22 @@ if __name__ == '__main__':
 
     for o, a in opts:
         if o == "-i":
-            infile = a
+            in_file = a
         if o == "-v":
             logger.setLevel(logging.DEBUG)
         if o == "-o":
             parse = True
-            outfile = a
+            out_file = a
         if o == "-s":
             start = int(float(a)*1000000)
         if o == "-l":
-            lenght = int(float(a)*1000000)
+            length = int(float(a)*1000000)
         if o == "-t":
-            topofile = a
+            topfile = a
             parser = AepParser()
-            print(parser.topology_from_config(topofile))
+            print(parser.topology_from_config(topfile))
             exit(0)
 
     parser = AepParser()
-    parser.prepare(infile, outfile, figurefile)
-    parser.parse_aep(start, lenght)
+    parser.prepare(in_file, out_file, figurefile)
+    parser.parse_aep(start, length)

devlib/utils/rendering.py

@@ -15,15 +15,15 @@
 
 import logging
 import os
-import re
 import shutil
 import sys
 import tempfile
 import threading
 import time
-from collections import namedtuple, OrderedDict
-from distutils.version import LooseVersion
+from collections import namedtuple
+from pipes import quote
 
+# pylint: disable=redefined-builtin
 from devlib.exception  import WorkerThreadError, TargetNotRespondingError, TimeoutError
 from devlib.utils.csvutil import csvwriter
 
@@ -49,12 +49,12 @@ class FrameCollector(threading.Thread):
         self.refresh_period = None
         self.drop_threshold = None
         self.unresponsive_count = 0
-        self.last_ready_time = None
+        self.last_ready_time = 0
         self.exc = None
         self.header = None
 
     def run(self):
-        logger.debug('Surface flinger frame data collection started.')
+        logger.debug('Frame data collection started.')
         try:
             self.stop_signal.clear()
             fd, self.temp_file = tempfile.mkstemp()
@@ -71,7 +71,7 @@ class FrameCollector(threading.Thread):
         except Exception as e:  # pylint: disable=W0703
             logger.warning('Exception on collector thread: {}({})'.format(e.__class__.__name__, e))
             self.exc = WorkerThreadError(self.name, sys.exc_info())
-        logger.debug('Surface flinger frame data collection stopped.')
+        logger.debug('Frame data collection stopped.')
 
     def stop(self):
         self.stop_signal.set()
@@ -133,14 +133,14 @@ class SurfaceFlingerFrameCollector(FrameCollector):
     def collect_frames(self, wfh):
         for activity in self.list():
             if activity == self.view:
-                wfh.write(self.get_latencies(activity))
+                wfh.write(self.get_latencies(activity).encode('utf-8'))
 
     def clear(self):
         self.target.execute('dumpsys SurfaceFlinger --latency-clear ')
 
     def get_latencies(self, activity):
-        cmd = 'dumpsys SurfaceFlinger --latency "{}"'
-        return self.target.execute(cmd.format(activity))
+        cmd = 'dumpsys SurfaceFlinger --latency {}'
+        return self.target.execute(cmd.format(quote(activity)))
 
     def list(self):
         text = self.target.execute('dumpsys SurfaceFlinger --list')
@@ -190,12 +190,16 @@ class GfxinfoFrameCollector(FrameCollector):
     def __init__(self, target, period, package, header=None):
         super(GfxinfoFrameCollector, self).__init__(target, period)
         self.package = package
-        self.header =  None
+        self.header = None
         self._init_header(header)
 
     def collect_frames(self, wfh):
         cmd = 'dumpsys gfxinfo {} framestats'
-        wfh.write(self.target.execute(cmd.format(self.package)))
+        result = self.target.execute(cmd.format(self.package))
+        if sys.version_info[0] == 3:
+            wfh.write(result.encode('utf-8'))
+        else:
+            wfh.write(result)
 
     def clear(self):
         pass
@@ -261,7 +265,7 @@ def gfxinfo_get_last_dump(filepath):
 
                 ix = buf.find(' **\n')
                 if ix >= 0:
-                    buf =  next(fh_iter) + buf
+                    buf = next(fh_iter) + buf
                     ix = buf.find('** Graphics')
                     if ix < 0:
                         msg = '"{}" appears to be corrupted'

devlib/utils/serial_port.py

@@ -20,6 +20,7 @@ from logging import Logger
 
 import serial
 
+# pylint: disable=import-error,wrong-import-position,ungrouped-imports,wrong-import-order
 import pexpect
 from distutils.version import StrictVersion as V
 if V(pexpect.__version__) < V('4.0.0'):
@@ -48,6 +49,7 @@ def pulse_dtr(conn, state=True, duration=0.1):
     conn.setDTR(not state)
 
 
+# pylint: disable=keyword-arg-before-vararg
 def get_connection(timeout, init_dtr=None, logcls=SerialLogger,
                    logfile=None, *args, **kwargs):
     if init_dtr is not None:
@@ -89,6 +91,7 @@ def write_characters(conn, line, delay=0.05):
     conn.sendline('')
 
 
+# pylint: disable=keyword-arg-before-vararg
 @contextmanager
 def open_serial_connection(timeout, get_conn=False, init_dtr=None,
                            logcls=SerialLogger, *args, **kwargs):
@@ -111,11 +114,13 @@ def open_serial_connection(timeout, get_conn=False, init_dtr=None,
     """
     target, conn = get_connection(timeout, init_dtr=init_dtr,
                                   logcls=logcls, *args, **kwargs)
+
     if get_conn:
-        yield target, conn
+        target_and_conn = (target, conn)
     else:
-        yield target
-
-    target.close()  # Closes the file descriptor used by the conn.
-    del conn
+        target_and_conn = target
 
+    try:
+        yield target_and_conn
+    finally:
+        target.close()  # Closes the file descriptor used by the conn.

devlib/utils/ssh.py

@@ -25,7 +25,11 @@ import shutil
 import socket
 import sys
 import time
+import atexit
+from pipes import quote
+from future.utils import raise_from
 
+# pylint: disable=import-error,wrong-import-position,ungrouped-imports,wrong-import-order
 import pexpect
 from distutils.version import StrictVersion as V
 if V(pexpect.__version__) < V('4.0.0'):
@@ -34,10 +38,11 @@ else:
     from pexpect import pxssh
 from pexpect import EOF, TIMEOUT, spawn
 
-from devlib.exception import HostError, TargetError, TimeoutError
-from devlib.utils.misc import which, strip_bash_colors, check_output
-from devlib.utils.misc import (escape_single_quotes, escape_double_quotes,
-                               escape_spaces)
+# pylint: disable=redefined-builtin,wrong-import-position
+from devlib.exception import (HostError, TargetStableError, TargetNotRespondingError,
+                              TimeoutError, TargetTransientError)
+from devlib.utils.misc import (which, strip_bash_colors, check_output,
+                               sanitize_cmd_template, memoized)
 from devlib.utils.types import boolean
 
 
@@ -58,7 +63,7 @@ def ssh_get_shell(host, username, password=None, keyfile=None, port=None, timeou
                 raise ValueError('keyfile may not be used with a telnet connection.')
             conn = TelnetPxssh(original_prompt=original_prompt)
         else:  # ssh
-            conn = pxssh.pxssh()
+            conn = pxssh.pxssh(echo=False)
 
         try:
             if keyfile:
@@ -70,10 +75,10 @@ def ssh_get_shell(host, username, password=None, keyfile=None, port=None, timeou
             timeout -= time.time() - start_time
             if timeout <= 0:
                 message = 'Could not connect to {}; is the host name correct?'
-                raise TargetError(message.format(host))
+                raise TargetTransientError(message.format(host))
             time.sleep(5)
 
-    conn.setwinsize(500,200)
+    conn.setwinsize(500, 200)
     conn.sendline('')
     conn.prompt()
     conn.setecho(False)
@@ -147,12 +152,13 @@ class SshConnection(object):
 
     default_password_prompt = '[sudo] password'
     max_cancel_attempts = 5
-    default_timeout=10
+    default_timeout = 10
 
     @property
     def name(self):
         return self.host
 
+    # pylint: disable=unused-argument,super-init-not-called
     def __init__(self,
                  host,
                  username,
@@ -164,7 +170,7 @@ class SshConnection(object):
                  password_prompt=None,
                  original_prompt=None,
                  platform=None,
-                 sudo_cmd="sudo -- sh -c '{}'"
+                 sudo_cmd="sudo -- sh -c {}"
                  ):
         self.host = host
         self.username = username
@@ -173,25 +179,22 @@ class SshConnection(object):
         self.port = port
         self.lock = threading.Lock()
         self.password_prompt = password_prompt if password_prompt is not None else self.default_password_prompt
-        self.sudo_cmd = sudo_cmd
+        self.sudo_cmd = sanitize_cmd_template(sudo_cmd)
         logger.debug('Logging in {}@{}'.format(username, host))
         timeout = timeout if timeout is not None else self.default_timeout
         self.conn = ssh_get_shell(host, username, password, self.keyfile, port, timeout, False, None)
+        atexit.register(self.close)
 
     def push(self, source, dest, timeout=30):
-        dest = '"{}"@"{}":"{}"'.format(escape_double_quotes(self.username),
-                                       escape_spaces(escape_double_quotes(self.host)),
-                                       escape_spaces(escape_double_quotes(dest)))
+        dest = '{}@{}:{}'.format(self.username, self.host, dest)
         return self._scp(source, dest, timeout)
 
     def pull(self, source, dest, timeout=30):
-        source = '"{}"@"{}":"{}"'.format(escape_double_quotes(self.username),
-                                         escape_spaces(escape_double_quotes(self.host)),
-                                         escape_spaces(escape_double_quotes(source)))
+        source = '{}@{}:{}'.format(self.username, self.host, source)
         return self._scp(source, dest, timeout)
 
     def execute(self, command, timeout=None, check_exit_code=True,
-                as_root=False, strip_colors=True): #pylint: disable=unused-argument
+                as_root=False, strip_colors=True, will_succeed=False): #pylint: disable=unused-argument
         if command == '':
             # Empty command is valid but the __devlib_ec stuff below will
             # produce a syntax error with bash. Treat as a special case.
@@ -199,22 +202,31 @@ class SshConnection(object):
         try:
             with self.lock:
                 _command = '({}); __devlib_ec=$?; echo; echo $__devlib_ec'.format(command)
-                raw_output = self._execute_and_wait_for_prompt(
-                    _command, timeout, as_root, strip_colors)
-                output, exit_code_text, _ = raw_output.rsplit('\r\n', 2)
+                full_output = self._execute_and_wait_for_prompt(_command, timeout, as_root, strip_colors)
+                split_output = full_output.rsplit('\r\n', 2)
+                try:
+                    output, exit_code_text, _ = split_output
+                except ValueError as e:
+                    raise TargetStableError(
+                        "cannot split reply (target misconfiguration?):\n'{}'".format(full_output))
                 if check_exit_code:
                     try:
                         exit_code = int(exit_code_text)
                         if exit_code:
                             message = 'Got exit code {}\nfrom: {}\nOUTPUT: {}'
-                            raise TargetError(message.format(exit_code, command, output))
+                            raise TargetStableError(message.format(exit_code, command, output))
                     except (ValueError, IndexError):
                         logger.warning(
                             'Could not get exit code for "{}",\ngot: "{}"'\
                             .format(command, exit_code_text))
                 return output
         except EOF:
-            raise TargetError('Connection lost.')
+            raise TargetNotRespondingError('Connection lost.')
+        except TargetStableError as e:
+            if will_succeed:
+                raise TargetTransientError(e)
+            else:
+                raise
 
     def background(self, command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, as_root=False):
         try:
@@ -225,20 +237,24 @@ class SshConnection(object):
             command = '{} {} {} {}@{} {}'.format(ssh, keyfile_string, port_string, self.username, self.host, command)
             logger.debug(command)
             if self.password:
-                command = _give_password(self.password, command)
+                command, _ = _give_password(self.password, command)
             return subprocess.Popen(command, stdout=stdout, stderr=stderr, shell=True)
         except EOF:
-            raise TargetError('Connection lost.')
+            raise TargetNotRespondingError('Connection lost.')
 
     def close(self):
         logger.debug('Logging out {}@{}'.format(self.username, self.host))
-        self.conn.logout()
+        try:
+            self.conn.logout()
+        except:
+            logger.debug('Connection lost.')
+            self.conn.close(force=True)
 
     def cancel_running_command(self):
         # simulate impatiently hitting ^C until command prompt appears
         logger.debug('Sending ^C')
         for _ in range(self.max_cancel_attempts):
-            self.conn.sendline(chr(3))
+            self._sendline(chr(3))
             if self.conn.prompt(0.1):
                 return True
         return False
@@ -249,28 +265,24 @@ class SshConnection(object):
             # As we're already root, there is no need to use sudo.
             as_root = False
         if as_root:
-            command = self.sudo_cmd.format(escape_single_quotes(command))
+            command = self.sudo_cmd.format(quote(command))
             if log:
                 logger.debug(command)
-            self.conn.sendline(command)
+            self._sendline(command)
             if self.password:
                 index = self.conn.expect_exact([self.password_prompt, TIMEOUT], timeout=0.5)
                 if index == 0:
-                    self.conn.sendline(self.password)
+                    self._sendline(self.password)
         else:  # not as_root
             if log:
                 logger.debug(command)
-            self.conn.sendline(command)
+            self._sendline(command)
         timed_out = self._wait_for_prompt(timeout)
-        # the regex removes line breaks potential introduced when writing
-        # command to shell.
         if sys.version_info[0] == 3:
-            output = process_backspaces(self.conn.before.decode(sys.stdout.encoding, 'replace'))
+            output = process_backspaces(self.conn.before.decode(sys.stdout.encoding or 'utf-8', 'replace'))
         else:
             output = process_backspaces(self.conn.before)
-        output = re.sub(r'\r([^\n])', r'\1', output)
-        if '\r\n' in output: # strip the echoed command
-            output = output.split('\r\n', 1)[1]
+
         if timed_out:
             self.cancel_running_command()
             raise TimeoutError(command, output)
@@ -291,25 +303,40 @@ class SshConnection(object):
         # fails to connect to a device if port is explicitly specified using -P
         # option, even if it is the default port, 22. To minimize this problem,
         # only specify -P for scp if the port is *not* the default.
-        port_string = '-P {}'.format(self.port) if (self.port and self.port != 22) else ''
-        keyfile_string = '-i {}'.format(self.keyfile) if self.keyfile else ''
-        command = '{} -r {} {} {} {}'.format(scp, keyfile_string, port_string, source, dest)
+        port_string = '-P {}'.format(quote(str(self.port))) if (self.port and self.port != 22) else ''
+        keyfile_string = '-i {}'.format(quote(self.keyfile)) if self.keyfile else ''
+        command = '{} -r {} {} {} {}'.format(scp, keyfile_string, port_string, quote(source), quote(dest))
         command_redacted = command
         logger.debug(command)
         if self.password:
-            command = _give_password(self.password, command)
-            command_redacted = command.replace(self.password, '<redacted>')
+            command, command_redacted = _give_password(self.password, command)
         try:
             check_output(command, timeout=timeout, shell=True)
         except subprocess.CalledProcessError as e:
-            raise HostError("Failed to copy file with '{}'. Output:\n{}".format(
-                command_redacted, e.output))
+            raise_from(HostError("Failed to copy file with '{}'. Output:\n{}".format(
+                command_redacted, e.output)), None)
         except TimeoutError as e:
             raise TimeoutError(command_redacted, e.output)
 
+    def _sendline(self, command):
+        # Workaround for https://github.com/pexpect/pexpect/issues/552
+        if len(command) == self._get_window_size()[1] - self._get_prompt_length():
+            command += ' '
+        self.conn.sendline(command)
+
+    @memoized
+    def _get_prompt_length(self):
+        self.conn.sendline()
+        self.conn.prompt()
+        return len(self.conn.after)
+
+    @memoized
+    def _get_window_size(self):
+        return self.conn.getwinsize()
 
 class TelnetConnection(SshConnection):
 
+    # pylint: disable=super-init-not-called
     def __init__(self,
                  host,
                  username,
@@ -333,6 +360,7 @@ class TelnetConnection(SshConnection):
 
 class Gem5Connection(TelnetConnection):
 
+    # pylint: disable=super-init-not-called
     def __init__(self,
                  platform,
                  host=None,
@@ -347,10 +375,10 @@ class Gem5Connection(TelnetConnection):
         if host is not None:
             host_system = socket.gethostname()
             if host_system != host:
-                raise TargetError("Gem5Connection can only connect to gem5 "
-                                   "simulations on your current host, which "
-                                   "differs from the one given {}!"
-                                   .format(host_system, host))
+                raise TargetStableError("Gem5Connection can only connect to gem5 "
+                                  "simulations on your current host {}, which "
+                                  "differs from the one given {}!"
+                                  .format(host_system, host))
         if username is not None and username != 'root':
             raise ValueError('User should be root in gem5!')
         if password is not None and password != '':
@@ -435,13 +463,12 @@ class Gem5Connection(TelnetConnection):
         if os.path.basename(dest) != filename:
             dest = os.path.join(dest, filename)
         # Back to the gem5 world
-        self._gem5_shell("ls -al {}{}".format(self.gem5_input_dir, filename))
-        self._gem5_shell("cat '{}''{}' > '{}'".format(self.gem5_input_dir,
-                                                     filename,
-                                                     dest))
+        filename = quote(self.gem5_input_dir + filename)
+        self._gem5_shell("ls -al {}".format(filename))
+        self._gem5_shell("cat {} > {}".format(filename, quote(dest)))
         self._gem5_shell("sync")
-        self._gem5_shell("ls -al {}".format(dest))
-        self._gem5_shell("ls -al {}".format(self.gem5_input_dir))
+        self._gem5_shell("ls -al {}".format(quote(dest)))
+        self._gem5_shell("ls -al {}".format(quote(self.gem5_input_dir)))
         logger.debug("Push complete.")
 
     def pull(self, source, dest, timeout=0): #pylint: disable=unused-argument
@@ -470,8 +497,8 @@ class Gem5Connection(TelnetConnection):
             if os.path.isabs(source):
                 if os.path.dirname(source) != self.execute('pwd',
                                               check_exit_code=False):
-                    self._gem5_shell("cat '{}' > '{}'".format(filename,
-                                                              dest_file))
+                    self._gem5_shell("cat {} > {}".format(quote(filename),
+                                                              quote(dest_file)))
             self._gem5_shell("sync")
             self._gem5_shell("ls -la {}".format(dest_file))
             logger.debug('Finished the copy in the simulator')
@@ -492,16 +519,23 @@ class Gem5Connection(TelnetConnection):
             logger.debug("Pull complete.")
 
     def execute(self, command, timeout=1000, check_exit_code=True,
-                as_root=False, strip_colors=True):
+                as_root=False, strip_colors=True, will_succeed=False):
         """
         Execute a command on the gem5 platform
         """
         # First check if the connection is set up to interact with gem5
         self._check_ready()
 
-        output = self._gem5_shell(command,
-                                  check_exit_code=check_exit_code,
-                                  as_root=as_root)
+        try:
+            output = self._gem5_shell(command,
+                                      check_exit_code=check_exit_code,
+                                      as_root=as_root)
+        except TargetStableError as e:
+            if will_succeed:
+                raise TargetTransientError(e)
+            else:
+                raise
+
         if strip_colors:
             output = strip_bash_colors(output)
         return output
@@ -517,8 +551,8 @@ class Gem5Connection(TelnetConnection):
         trial = 0
         while os.path.isfile(redirection_file):
             # Log file already exists so add to name
-           redirection_file = 'BACKGROUND_{}{}.log'.format(command_name, trial)
-           trial += 1
+            redirection_file = 'BACKGROUND_{}{}.log'.format(command_name, trial)
+            trial += 1
 
         # Create the command to pass on to gem5 shell
         complete_command = '{} >> {} 2>&1 &'.format(command, redirection_file)
@@ -548,7 +582,7 @@ class Gem5Connection(TelnetConnection):
         try:
             shutil.rmtree(self.gem5_interact_dir)
         except OSError:
-            gem5_logger.warn("Failed to remove the temporary directory!")
+            gem5_logger.warning("Failed to remove the temporary directory!")
 
         # Delete the lock file
         os.remove(self.lock_file_name)
@@ -563,6 +597,7 @@ class Gem5Connection(TelnetConnection):
         self.connect_gem5(port, gem5_simulation, gem5_interact_dir, gem5_out_dir)
 
     # Handle the EOF exception raised by pexpect
+    # pylint: disable=no-self-use
     def _gem5_EOF_handler(self, gem5_simulation, gem5_out_dir, err):
         # If we have reached the "EOF", it typically means
         # that gem5 crashed and closed the connection. Let's
@@ -570,12 +605,13 @@ class Gem5Connection(TelnetConnection):
         # rather than spewing out pexpect errors.
         if gem5_simulation.poll():
             message = "The gem5 process has crashed with error code {}!\n\tPlease see {} for details."
-            raise TargetError(message.format(gem5_simulation.poll(), gem5_out_dir))
+            raise TargetNotRespondingError(message.format(gem5_simulation.poll(), gem5_out_dir))
         else:
             # Let's re-throw the exception in this case.
             raise err
 
     # This function connects to the gem5 simulation
+    # pylint: disable=too-many-statements
     def connect_gem5(self, port, gem5_simulation, gem5_interact_dir,
                       gem5_out_dir):
         """
@@ -597,7 +633,7 @@ class Gem5Connection(TelnetConnection):
         lock_file_name = '{}{}_{}.LOCK'.format(self.lock_directory, host, port)
         if os.path.isfile(lock_file_name):
             # There is already a connection to this gem5 simulation
-            raise TargetError('There is already a connection to the gem5 '
+            raise TargetStableError('There is already a connection to the gem5 '
                               'simulation using port {} on {}!'
                               .format(port, host))
 
@@ -616,7 +652,7 @@ class Gem5Connection(TelnetConnection):
                 self._gem5_EOF_handler(gem5_simulation, gem5_out_dir, err)
         else:
             gem5_simulation.kill()
-            raise TargetError("Failed to connect to the gem5 telnet session.")
+            raise TargetNotRespondingError("Failed to connect to the gem5 telnet session.")
 
         gem5_logger.info("Connected! Waiting for prompt...")
 
@@ -711,7 +747,7 @@ class Gem5Connection(TelnetConnection):
     def _gem5_util(self, command):
         """ Execute a gem5 utility command using the m5 binary on the device """
         if self.m5_path is None:
-            raise TargetError('Path to m5 binary on simulated system  is not set!')
+            raise TargetStableError('Path to m5 binary on simulated system  is not set!')
         self._gem5_shell('{} {}'.format(self.m5_path, command))
 
     def _gem5_shell(self, command, as_root=False, timeout=None, check_exit_code=True, sync=True):  # pylint: disable=R0912
@@ -725,7 +761,7 @@ class Gem5Connection(TelnetConnection):
         fails, warn, but continue with the potentially wrong output.
 
         The exit code is also checked by default, and non-zero exit codes will
-        raise a TargetError.
+        raise a TargetStableError.
         """
         if sync:
             self._sync_gem5_shell()
@@ -733,7 +769,7 @@ class Gem5Connection(TelnetConnection):
         gem5_logger.debug("gem5_shell command: {}".format(command))
 
         if as_root:
-            command = 'echo "{}" | su'.format(escape_double_quotes(command))
+            command = 'echo {} | su'.format(quote(command))
 
         # Send the actual command
         self.conn.send("{}\n".format(command))
@@ -754,7 +790,7 @@ class Gem5Connection(TelnetConnection):
                 # prompt has returned. Hence, we have a bit of an issue. We
                 # warn, and return the whole output.
                 if command_index == -1:
-                    gem5_logger.warn("gem5_shell: Unable to match command in "
+                    gem5_logger.warning("gem5_shell: Unable to match command in "
                                      "command output. Expect parsing errors!")
                     command_index = 0
 
@@ -781,7 +817,7 @@ class Gem5Connection(TelnetConnection):
                 exit_code = int(exit_code_text.split()[0])
                 if exit_code:
                     message = 'Got exit code {}\nfrom: {}\nOUTPUT: {}'
-                    raise TargetError(message.format(exit_code, command, output))
+                    raise TargetStableError(message.format(exit_code, command, output))
             except (ValueError, IndexError):
                 gem5_logger.warning('Could not get exit code for "{}",\ngot: "{}"'.format(command, exit_code_text))
 
@@ -836,7 +872,7 @@ class Gem5Connection(TelnetConnection):
         Check if the gem5 platform is ready
         """
         if not self.ready:
-            raise TargetError('Gem5 is not ready to interact yet')
+            raise TargetTransientError('Gem5 is not ready to interact yet')
 
     def _wait_for_boot(self):
         pass
@@ -845,7 +881,8 @@ class Gem5Connection(TelnetConnection):
         """
         Internal method to check if the target has a certain file
         """
-        command = 'if [ -e \'{}\' ]; then echo 1; else echo 0; fi'
+        filepath = quote(filepath)
+        command = 'if [ -e {} ]; then echo 1; else echo 0; fi'
         output = self.execute(command.format(filepath), as_root=self.is_rooted)
         return boolean(output.strip())
 
@@ -901,8 +938,10 @@ class AndroidGem5Connection(Gem5Connection):
 def _give_password(password, command):
     if not sshpass:
         raise HostError('Must have sshpass installed on the host in order to use password-based auth.')
-    pass_string = "sshpass -p '{}' ".format(password)
-    return pass_string + command
+    pass_template = "sshpass -p {} "
+    pass_string = pass_template.format(quote(password))
+    redacted_string = pass_template.format(quote('<redacted>'))
+    return (pass_string + command, redacted_string + command)
 
 
 def _check_env():

devlib/utils/types.py

@@ -153,11 +153,11 @@ if sys.version_info[0] == 3:
         if isinstance(value, regex_type):
             if isinstance(value.pattern, bytes):
                 return value
-            return re.compile(value.pattern.encode(sys.stdout.encoding),
+            return re.compile(value.pattern.encode(sys.stdout.encoding or 'utf-8'),
                               value.flags & ~re.UNICODE)
         else:
             if isinstance(value, str):
-                value = value.encode(sys.stdout.encoding)
+                value = value.encode(sys.stdout.encoding or 'utf-8')
             return re.compile(value)
 else:
     def regex(value):

devlib/utils/uboot.py

@@ -113,4 +113,3 @@ class UbootMenu(object):
         except TIMEOUT:
             pass
         self.conn.buffer = ''
-

devlib/utils/uefi.py

@@ -237,5 +237,3 @@ class UefiMenu(object):
         self.options = {}
         self.prompt = None
         self.empty_buffer()
-
-

devlib/utils/version.py

@@ -15,8 +15,23 @@
 
 import os
 import sys
+from collections import namedtuple
 from subprocess import Popen, PIPE
 
+
+VersionTuple = namedtuple('Version', ['major', 'minor', 'revision', 'dev'])
+
+version = VersionTuple(1, 1, 1, '')
+
+
+def get_devlib_version():
+    version_string = '{}.{}.{}'.format(
+        version.major, version.minor, version.revision)
+    if version.dev:
+        version_string += '.{}'.format(version.dev)
+    return version_string
+
+
 def get_commit():
     p = Popen(['git', 'rev-parse', 'HEAD'], cwd=os.path.dirname(__file__),
               stdout=PIPE, stderr=PIPE)
@@ -24,7 +39,7 @@ def get_commit():
     p.wait()
     if p.returncode:
         return None
-    if sys.version_info[0] == 3:
-        return std[:8].decode(sys.stdout.encoding, 'replace')
+    if sys.version_info[0] == 3 and isinstance(std, bytes):
+        return std[:8].decode(sys.stdout.encoding or 'utf-8', 'replace')
     else:
         return std[:8]

doc/conf.py

@@ -31,6 +31,9 @@ import shlex
 # ones.
 extensions = [
     'sphinx.ext.autodoc',
+    'sphinx.ext.graphviz',
+    'sphinx.ext.mathjax',
+    'sphinx.ext.todo',
     'sphinx.ext.viewcode',
 ]
 
@@ -58,9 +61,9 @@ author = u'ARM Limited'
 # built documents.
 #
 # The short X.Y version.
-version = '0.1'
+version = '1.0.0'
 # The full version, including alpha/beta/rc tags.
-release = '0.1'
+release = '1.0.0'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.
@@ -104,7 +107,7 @@ pygments_style = 'sphinx'
 #keep_warnings = False
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = False
+todo_include_todos = True
 
 
 # -- Options for HTML output ----------------------------------------------

doc/connection.rst

@@ -40,7 +40,7 @@ class that implements the following methods.
    :param timeout: timeout (in seconds) for the transfer; if the transfer does
        not  complete within this period, an exception will be raised.
 
-.. method:: execute(self, command, timeout=None, check_exit_code=False, as_root=False)
+.. method:: execute(self, command, timeout=None, check_exit_code=False, as_root=False, strip_colors=True, will_succeed=False)
 
    Execute the specified command on the connected device and return its output.
 
@@ -53,6 +53,13 @@ class that implements the following methods.
        raised if it is not ``0``.
    :param as_root: The command will be executed as root. This will fail on
        unrooted connected devices.
+   :param strip_colours: The command output will have colour encodings and
+       most ANSI escape sequences striped out before returning.
+   :param will_succeed: The command is assumed to always succeed, unless there is
+       an issue in the environment like the loss of network connectivity. That
+       will make the method always raise an instance of a subclass of
+       :class:`DevlibTransientError' when the command fails, instead of a
+       :class:`DevlibStableError`.
 
 .. method:: background(self, command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, as_root=False)
 
@@ -196,13 +203,12 @@ Connection Types
 
     :param host: Host on which the gem5 simulation is running
 
-                     .. note:: Even thought the input parameter for the ``host``
-                               will be ignored, the gem5 simulation needs to on
-                               the same host as the user as the user is
-                               currently on, so if the host given as input
-                               parameter is not the same as the actual host, a
-                               ``TargetError`` will be raised to prevent
-                               confusion.
+                     .. note:: Even though the input parameter for the ``host``
+                               will be ignored, the gem5 simulation needs to be
+                               on the same host the user is currently on, so if
+                               the host given as input parameter is not the
+                               same as the actual host, a ``TargetStableError``
+                               will be raised to prevent confusion.
 
     :param username: Username in the simulated system
     :param password: No password required in gem5 so does not need to be set

doc/instrumentation.rst

@@ -13,7 +13,7 @@ Example
 The following example shows how to use an instrument to read temperature from an
 Android target.
 
-.. code-block:: ipython
+.. code-block:: python
 
     # import and instantiate the Target and the instrument
     # (note: this assumes exactly one android target connected
@@ -51,7 +51,7 @@ API
 Instrument
 ~~~~~~~~~~
 
-.. class:: Instrument(target, **kwargs)
+.. class:: Instrument(target, \*\*kwargs)
 
    An ``Instrument`` allows collection of measurement from one or more
    channels. An ``Instrument`` may support ``INSTANTANEOUS`` or ``CONTINUOUS``
@@ -88,7 +88,7 @@ Instrument
    Returns channels for a particular ``measure`` type. A ``measure`` can be
    either a string (e.g. ``"power"``) or a :class:`MeasurmentType` instance.
 
-.. method::  Instrument.setup(*args, **kwargs)
+.. method::  Instrument.setup(\*args, \*\*kwargs)
 
    This will set up the instrument on the target. Parameters this method takes
    are particular to subclasses (see documentation for specific instruments
@@ -115,7 +115,7 @@ Instrument
    If none of ``sites``, ``kinds`` or ``channels`` are provided then all
    available channels are enabled.
 
-.. method:: Instrument.take_measurment()
+.. method:: Instrument.take_measurement()
 
    Take a single measurement from ``active_channels``. Returns a list of
    :class:`Measurement` objects (one for each active channel).
@@ -178,7 +178,7 @@ Instrument
 Instrument Channel
 ~~~~~~~~~~~~~~~~~~
 
-.. class:: InstrumentChannel(name, site, measurement_type, **attrs)
+.. class:: InstrumentChannel(name, site, measurement_type, \*\*attrs)
 
    An :class:`InstrumentChannel` describes a single type of measurement that may
    be collected by an :class:`Instrument`. A channel is primarily defined by a
@@ -228,9 +228,9 @@ Measurement Types
 
 In order to make instruments easer to use, and to make it easier to swap them
 out when necessary (e.g. change method of collecting power), a number of
-standard measurement types are defined. This way, for example, power will always
-be reported as "power" in Watts, and never as "pwr" in milliWatts. Currently
-defined measurement types are
+standard measurement types are defined. This way, for example, power will
+always be reported as "power" in Watts, and never as "pwr" in milliWatts.
+Currently defined measurement types are
 
 
 +-------------+-------------+---------------+
@@ -269,4 +269,644 @@ Available Instruments
 
 This section lists instruments that are currently part of devlib.
 
-TODO
+.. todo:: Add other instruments
+
+
+Baylibre ACME BeagleBone Black Cape
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. _official project page:            http://baylibre.com/acme/
+.. _image built for using the ACME:   https://gitlab.com/baylibre-acme/ACME-Software-Release/blob/master/README.md
+.. _libiio (the Linux IIO interface): https://github.com/analogdevicesinc/libiio
+.. _Linux Industrial I/O Subsystem:   https://wiki.analog.com/software/linux/docs/iio/iio
+.. _Texas Instruments INA226:         http://www.ti.com/lit/ds/symlink/ina226.pdf
+
+From the `official project page`_:
+
+  [The Baylibre Another Cute Measurement Equipment (ACME)] is an extension for
+  the BeagleBone Black (the ACME Cape), designed to provide multi-channel power
+  and temperature measurements capabilities to the BeagleBone Black (BBB). It
+  comes with power and temperature probes integrating a power switch (the ACME
+  Probes), turning it into an advanced all-in-one power/temperature measurement
+  solution.
+
+  The ACME initiative is completely open source, from HW to SW drivers and
+  applications.
+
+
+The Infrastructure
+^^^^^^^^^^^^^^^^^^
+
+Retrieving measurement from the ACME through devlib requires:
+
+  - a BBB running the `image built for using the ACME`_ (micro SD card required);
+
+  - an ACME cape on top of the BBB;
+
+  - at least one ACME probe [#acme_probe_variants]_ connected to the ACME cape;
+
+  - a BBB-host interface (typically USB or Ethernet) [#acme_name_conflicts]_;
+
+  - a host (the one running devlib) with `libiio (the Linux IIO interface)`_
+    installed, and a Python environment able to find the libiio Python wrapper
+    *i.e.* able to ``import iio`` as communications between the BBB and the
+    host rely on the `Linux Industrial I/O Subsystem`_ (IIO).
+
+The ACME probes are built on top of the `Texas Instruments INA226`_ and the
+data acquisition chain is as follows:
+
+.. graphviz::
+
+    digraph target {
+        rankdir = LR
+        bgcolor = transparent
+
+        subgraph cluster_target {
+
+            subgraph cluster_BBB {
+                node [style = filled, color = white];
+                style = filled;
+                color = lightgrey;
+                label = "BeagleBone Black";
+
+                drivers -> "IIO Daemon" [dir = both]
+            }
+
+            subgraph cluster_INA226 {
+                node [style = filled, color = white];
+                style = filled;
+                color = lightgrey;
+                label = INA226;
+
+                ADC -> Processing
+                Processing -> Registers
+            }
+
+            subgraph cluster_inputs {
+                node [style = filled, color = white];
+                style = filled;
+                color = lightgrey;
+                label = Inputs;
+
+                "Bus Voltage" -> ADC;
+                "Shunt Voltage" -> ADC;
+            }
+
+            Registers -> drivers [dir = both, label = I2C];
+        }
+
+        subgraph cluster_IIO {
+            style = none
+            "IIO Daemon" -> "IIO Interface" [dir = both, label = "Eth./USB"]
+        }
+    }
+
+For reference, the software stack on the host is roughly given by:
+
+.. graphviz::
+
+    digraph host {
+        rankdir = LR
+        bgcolor = transparent
+
+        subgraph cluster_host {
+
+            subgraph cluster_backend {
+                node [style = filled, color = white];
+                style = filled;
+                color = lightgrey;
+                label = Backend;
+
+                "IIO Daemon" -> "C API" [dir = both]
+            }
+
+            subgraph cluster_Python {
+                node [style = filled, color = white];
+                style = filled;
+                color = lightgrey;
+                label = Python;
+
+                "C API" -> "iio Wrapper" [dir = both]
+                "iio Wrapper" -> devlib [dir = both]
+                devlib -> "User" [dir = both]
+            }
+        }
+
+        subgraph cluster_IIO {
+            style = none
+            "IIO Interface" -> "IIO Daemon" [dir = both, label = "Eth./USB"]
+        }
+    }
+
+Ethernet was the only IIO Interface used and tested during the development of
+this instrument. However,
+`USB seems to be supported<https://gitlab.com/baylibre-acme/ACME/issues/2>`_.
+The IIO library also provides "Local" and "XML" connections but these are to be
+used when the IIO devices are directly connected to the host *i.e.* in our
+case, if we were to run Python and devlib on the BBB. These are also untested.
+
+Measuring Power
+^^^^^^^^^^^^^^^
+
+In IIO terminology, the ACME cape is an *IIO context* and ACME probes are *IIO
+devices* with *IIO channels*. An input *IIO channel* (the ACME has no *output
+IIO channel*) is a stream of samples and an ACME cape can be connected to up to
+8 probes *i.e.* have 8 *IIO devices*. The probes are discovered at startup by
+the IIO drivers on the BBB and are indexed according to the order in which they
+are connected to the ACME cape (with respect to the "Probe *X*" connectors on
+the cape).
+
+
+.. figure:: images/instrumentation/baylibre_acme/cape.png
+   :width: 50%
+   :alt: ACME Cape
+   :align: center
+
+   ACME Cape on top of a BBB: Notice the numbered probe connectors (
+   `source <https://baylibre.com/wp-content/uploads/2015/11/20150916_BayLibre_ACME_RevB-010-1030x599.png>`_)
+
+
+Please note that the numbers on the PCB do not represent the index of a probe
+in IIO; on top of being 1-based (as opposed to IIO device indexing being
+0-based), skipped connectors do not result in skipped indices *e.g.* if three
+probes are connected to the cape at ``Probe 1``, ``Probe 3`` and ``Probe 7``,
+IIO (and therefore the entire software stack, including devlib) will still
+refer to them as devices ``0``, ``1`` and ``2``, respectively. Furthermore,
+probe "hot swapping" does not seem to be supported.
+
+INA226: The probing spearhead
+"""""""""""""""""""""""""""""
+
+An ACME probe has 5 *IIO channels*, 4 of which being "IIO wrappers" around what
+the INA226 outputs (through its I2C registers): the bus voltage, the shunt
+voltage, the shunt current and the load power. The last channel gives the
+timestamps and is probably added further down the pipeline. A typical circuit
+configuration for the INA226 (useful when shunt-based ACME probes are used as
+their PCB does not contain the full circuit unlike the USB and jack variants)
+is given by its datasheet:
+
+.. figure:: images/instrumentation/baylibre_acme/ina226_circuit.png
+   :width: 90%
+   :alt: Typical circuit configuration, INA226
+   :align: center
+
+   Typical Circuit Configuration (source: `Texas Instruments INA226`_)
+
+
+The analog-to-digital converter (ADC)
+'''''''''''''''''''''''''''''''''''''
+
+The digital time-discrete sampled signal of the analog time-continuous input
+voltage signal is obtained through an analog-to-digital converter (ADC). To
+measure the "instantaneous input voltage", the ADC "charges up or down" a
+capacitor before measuring its charge.
+
+The *integration time* is the time spend by the ADC acquiring the input signal
+in its capacitor. The longer this time is, the more resilient the sampling
+process is to unwanted noise. The drawback is that, if the integration time is
+increased then the sampling rate decreases. This effect can be somewhat
+compared to a *low-pass filter*.
+
+As the INA226 alternatively connects its ADC to the bus voltage and shunt
+voltage (see previous figure), samples are retrieved at a frequency of
+
+.. math::
+    \frac{1}{T_{bus} + T_{shunt}}
+
+where :math:`T_X` is the integration time for the :math:`X` voltage.
+
+As described below (:meth:`BaylibreAcmeInstrument.reset`), the integration
+times for the bus and shunt voltage can be set separately which allows a
+tradeoff of accuracy between signals. This is particularly useful as the shunt
+voltage returned by the INA226 has a higher resolution than the bus voltage
+(2.5 μV and 1.25 mV LSB, respectively) and therefore would benefit more from a
+longer integration time.
+
+As an illustration, consider the following sampled sine wave and notice how
+increasing the integration time (of the bus voltage in this case) "smoothes"
+out the signal:
+
+.. figure:: images/instrumentation/baylibre_acme/int_time.png
+   :alt: Illustration of the impact of the integration time
+   :align: center
+
+   Increasing the integration time increases the resilience to noise
+
+
+Internal signal processing
+''''''''''''''''''''''''''
+
+The INA226 is able to accumulate samples acquired by its ADC and output to the
+ACME board (technically, to its I2C registers) the average value of :math:`N`
+samples. This is called *oversampling*. While the integration time somewhat
+behaves as an analog low-pass filter, the oversampling feature is a digital
+low-pass filter by definition. The former should be set to reduce sampling
+noise (*i.e.* noise on a single sample coming from the sampling process) while
+the latter should be used to filter out high-frequency noise present in the
+input signal and control the sampling frequency.
+
+Therefore, samples are available at the output of the INA226 at a frequency
+
+.. math::
+    \frac{1}{N(T_{bus} + T_{shunt})}
+
+and oversampling ratio provides a way to control the output sampling frequency
+(*i.e.* to limit the required output bandwidth) while making sure the signal
+fidelity is as desired.
+
+
+The 4 IIO channels coming from the INA226 can be grouped according to their
+respective origins: the bus and shunt voltages are measured (and, potentially
+filtered) while the shunt current and load power are computed. Indeed, the
+INA226 contains on-board fixed-point arithmetic units to compute the trivial
+expressions:
+
+.. math::
+
+   I_{shunt} = \frac{V_{shunt}}{R_{shunt}}
+   ,\ \
+   P_{load} = V_{load}\ I_{load}
+      \approx V_{bus} \ I_{shunt}
+
+A functional block diagram of this is also given by the datasheet:
+
+.. figure:: images/instrumentation/baylibre_acme/ina226_functional.png
+   :width: 60%
+   :alt: Functional block diagram, INA226
+   :align: center
+
+   Acquisition and Processing: Functional Block Diagram
+   (source: `Texas Instruments INA226`_)
+
+In the end, there are therefore 3 channels (bus voltage, shunt voltage and
+timestamps) that are necessary to figure out the load power consumption, while
+the others are being provided for convenience *e.g.* in case the rest of the
+hardware does not have the computing power to make the computation.
+
+
+Sampling Frequency Issues
+"""""""""""""""""""""""""
+
+It looks like the INA226-ACME-BBB setup has a bottleneck preventing the
+sampling frequency to go higher than ~1.4 kHz (the maximal theoretical sampling
+frequency is ~3.6 kHz). We know that this issue is not internal to the ADC
+itself (inside of the INA226) because modifying the integration time affects
+the output signal even when the sampling frequency is capped (as shown above)
+but it may come from anywhere after that.
+
+Because of this, there is no point in using a (theoretical) sampling frequency
+that is larger than 1.4 kHz. But it is important to note that the ACME will
+still report the theoretical sampling rate (probably computed with the formula
+given above) through :attr:`BaylibreAcmeInstrument.sample_rate_hz` and
+:attr:`IIOINA226Instrument.sample_rate_hz` even if it differs from the actual
+sampling rate.
+
+Note that, even though this is obvious for the theoretical sampling rate, the
+specific values of the bus and shunt integration times do not seem to have an
+influence on the measured sampling rate; only their sum matters. This further
+points toward a data-processing bottleneck rather than a hardware bug in the
+acquisition device.
+
+The following chart compares the evolution of the measured sampling rate with
+the expected one as we modify it through :math:`T_{shunt}`, :math:`T_{bus}` and
+:math:`N`:
+
+.. figure:: images/instrumentation/baylibre_acme/bottleneck.png
+   :alt: Sampling frequency does not go higher than 1.4 kHz
+   :align: center
+
+   Theoretical vs measured sampling rates
+
+
+Furthermore, because the transactions are done through a buffer (see next
+section), if the sampling frequency is too low, the connection may time-out
+before the buffer is full and ready to be sent. This may be fixed in an
+upcoming release.
+
+Buffer-based transactions
+"""""""""""""""""""""""""
+
+Samples made available by the INA226 are retrieved by the BBB and stored in a
+buffer which is sent back to the host once it is full (see
+``buffer_samples_count`` in :meth:`BaylibreAcmeInstrument.setup` for setting
+its size). Therefore, the larger the buffer is, the longer it takes to be
+transmitted back but the less often it has to be transmitted. To illustrate
+this, consider the following graphs showing the time difference between
+successive samples in a retrieved signal when the size of the buffer changes:
+
+.. figure:: images/instrumentation/baylibre_acme/buffer.png
+   :alt: Buffer size impact on the sampled signal
+   :align: center
+
+   Impact of the buffer size on the sampling regularity
+
+devlib API
+^^^^^^^^^^
+
+ACME Cape + BBB (IIO Context)
+"""""""""""""""""""""""""""""
+
+devlib provides wrapper classes for all the IIO connections to an IIO context
+given by `libiio (the Linux IIO interface)`_ however only the network-based one
+has been tested. For the other classes, please refer to the official IIO
+documentation for the meaning of their constructor parameters.
+
+.. class:: BaylibreAcmeInstrument(target=None, iio_context=None, use_base_iio_context=False, probe_names=None)
+
+   Base class wrapper for the ACME instrument which itself is a wrapper for the
+   IIO context base class. This class wraps around the passed ``iio_context``;
+   if ``use_base_iio_context`` is ``True``, ``iio_context`` is first passed to
+   the :class:`iio.Context` base class (see its documentation for how this
+   parameter is then used), else ``iio_context`` is expected to be a valid
+   instance of :class:`iio.Context`.
+
+   ``probe_names`` is expected to be a string or list of strings; if passed,
+   the probes in the instance are named according to it in the order in which
+   they are discovered (see previous comment about probe discovery and
+   :attr:`BaylibreAcmeInstrument.probes`). There should be as many
+   ``probe_names`` as there are probes connected to the ACME. By default, the
+   probes keep their IIO names.
+
+   To ensure that the setup is reliable, ``devlib`` requires minimal versions
+   for ``iio``, the IIO drivers and the ACME BBB SD image.
+
+.. class:: BaylibreAcmeNetworkInstrument(target=None, hostname=None, probe_names=None)
+
+   Child class of :class:`BaylibreAcmeInstrument` for Ethernet-based IIO
+   communication. The ``hostname`` should be the IP address or network name of
+   the BBB. If it is ``None``, the ``IIOD_REMOTE`` environment variable will be
+   used as the hostname. If that environment variable is empty, the server will
+   be discovered using ZeroConf. If that environment variable is not set, a
+   local context is created.
+
+.. class:: BaylibreAcmeXMLInstrument(target=None, xmlfile=None, probe_names=None)
+
+   Child class of :class:`BaylibreAcmeInstrument` using the XML backend of the
+   IIO library and building an IIO context from the provided ``xmlfile`` (a
+   string giving the path to the file is expected).
+
+.. class:: BaylibreAcmeLocalInstrument(target=None, probe_names=None)
+
+   Child class of :class:`BaylibreAcmeInstrument` using the Local IIO backend.
+
+.. attribute:: BaylibreAcmeInstrument.mode
+
+   The collection mode for the ACME is ``CONTINUOUS``.
+
+.. method:: BaylibreAcmeInstrument.setup(shunt_resistor, integration_time_bus, integration_time_shunt, oversampling_ratio, buffer_samples_count=None, buffer_is_circular=False, absolute_timestamps=False, high_resolution=True)
+
+   The ``shunt_resistor`` (:math:`R_{shunt}` [:math:`\mu\Omega`]),
+   ``integration_time_bus`` (:math:`T_{bus}` [s]), ``integration_time_shunt``
+   (:math:`T_{shunt}` [s]) and ``oversampling_ratio`` (:math:`N`) are copied
+   into on-board registers inside of the INA226 to be used as described above.
+   Please note that there exists a limited set of accepted values for these
+   parameters; for the integration times, refer to
+   ``IIOINA226Instrument.INTEGRATION_TIMES_AVAILABLE`` and for the
+   ``oversampling_ratio``, refer to
+   ``IIOINA226Instrument.OVERSAMPLING_RATIOS_AVAILABLE``. If all probes share
+   the same value for these attributes, this class provides
+   :attr:`BaylibreAcmeInstrument.OVERSAMPLING_RATIOS_AVAILABLE` and
+   :attr:`BaylibreAcmeInstrument.INTEGRATION_TIMES_AVAILABLE`.
+
+   The ``buffer_samples_count`` is the size of the IIO buffer expressed **in
+   samples**; this is independent of the number of active channels! By default,
+   if ``buffer_samples_count`` is not passed, the IIO buffer of size
+   :attr:`IIOINA226Instrument.sample_rate_hz` is created meaning that a buffer
+   transfer happens roughly every second.
+
+   If ``absolute_timestamps`` is ``False``, the first sample from the
+   ``timestamps`` channel is substracted from all the following samples of this
+   channel, effectively making its signal start at 0.
+
+   ``high_resolution`` is used to enable a mode where power and current are
+   computed offline on the host machine running ``devlib``: even if the user
+   asks for power or current channels, they are not enabled in hardware
+   (INA226) and instead the necessary voltage signal(s) are enabled to allow
+   the computation of the desired signals using the FPU of the host (which is
+   very likely to be much more accurate than the fixed-point 16-bit unit of the
+   INA226).
+
+   A circular buffer can be used by setting ``buffer_is_circular`` to ``True``
+   (directly passed to :class:`iio.Buffer`).
+
+   Each one of the arguments of this method can either be a single value which
+   will be used for all probes or a list of values giving the corresponding
+   setting for each probe (in the order of ``probe_names`` passed to the
+   constructor) with the exception of ``absolute_timestamps`` (as all signals
+   are resampled onto a common time signal) which, if passed as an array, will
+   be ``True`` only if all of its elements are ``True``.
+
+.. method:: BaylibreAcmeInstrument.reset(sites=None, kinds=None, channels=None)
+
+   :meth:`BaylibreAcmeInstrument.setup` should **always** be called before
+   calling this method so that the hardware is correctly configured. Once this
+   method has been called, :meth:`BaylibreAcmeInstrument.setup` can only be
+   called again once :meth:`BaylibreAcmeInstrument.teardown` has been called.
+
+   This method inherits from :meth:`Instrument.reset`; call
+   :meth:`list_channels` for a list of available channels from a given
+   instance.
+
+   Please note that the size of the transaction buffer is proportional to the
+   number of active channels (for a fixed ``buffer_samples_count``). Therefore,
+   limiting the number of active channels allows to limit the required
+   bandwidth. ``high_resolution`` in :meth:`BaylibreAcmeInstrument.setup`
+   limits the number of active channels to the minimum required.
+
+.. method:: BaylibreAcmeInstrument.start()
+
+   :meth:`BaylibreAcmeInstrument.reset` should **always** be called before
+   calling this method so that the right channels are active,
+   :meth:`BaylibreAcmeInstrument.stop` should **always** be called after
+   calling this method and no other method of the object should be called
+   in-between.
+
+   This method starts the sampling process of the active channels. The samples
+   are stored but are not available until :meth:`BaylibreAcmeInstrument.stop`
+   has been called.
+
+.. method:: BaylibreAcmeInstrument.stop()
+
+   :meth:`BaylibreAcmeInstrument.start` should **always** be called before
+   calling this method so that samples are being captured.
+
+   This method stops the sampling process of the active channels and retrieves
+   and pre-processes the samples. Once this function has been called, the
+   samples are made available through :meth:`BaylibreAcmeInstrument.get_data`.
+   Note that it is safe to call :meth:`BaylibreAcmeInstrument.start` after this
+   method returns but this will discard the data previously acquired.
+
+   When this method returns, It is guaranteed that the content of at least one
+   IIO buffer will have been captured.
+
+   If different sampling frequencies were used for the different probes, the
+   signals are resampled to share the time signal with the highest sampling
+   frequency.
+
+.. method:: BaylibreAcmeInstrument.teardown()
+
+   This method can be called at any point (unless otherwise specified *e.g.*
+   :meth:`BaylibreAcmeInstrument.start`) to deactive any active probe once
+   :meth:`BaylibreAcmeInstrument.reset` has been called. This method does not
+   affect already captured samples.
+
+The following graph gives a summary of the allowed calling sequence(s) where
+each edge means "can be called directly after":
+
+.. graphviz::
+
+   digraph acme_calls {
+       rankdir = LR
+       bgcolor = transparent
+
+       __init__ -> setup -> reset -> start -> stop -> teardown
+
+       teardown:sw -> setup [style=dashed]
+       teardown -> reset [style=dashed]
+
+       stop -> reset [style=dashed]
+       stop:nw -> start [style=dashed]
+
+       reset -> teardown [style=dashed]
+   }
+
+.. method:: BaylibreAcmeInstrument.get_data(outfile=None)
+
+   Inherited from :meth:`Instrument.get_data`. If ``outfile`` is ``None``
+   (default), the samples are returned as a `pandas.DataFrame` with the
+   channels as columns. Else, it behaves like the parent class, returning a
+   ``MeasurementCsv``.
+
+.. method:: BaylibreAcmeInstrument.add_channel()
+
+   Should not be used as new channels are discovered through the IIO context.
+
+.. method:: BaylibreAcmeInstrument.list_channels()
+
+    Inherited from :meth:`Instrument.list_channels`.
+
+.. attribute:: BaylibreAcmeInstrument.sample_rate_hz
+.. attribute:: BaylibreAcmeInstrument.OVERSAMPLING_RATIOS_AVAILABLE
+.. attribute:: BaylibreAcmeInstrument.INTEGRATION_TIMES_AVAILABLE
+
+   These attributes return the corresponding attributes of the probes if they
+   all share the same value (and are therefore provided to avoid reading from a
+   single probe and expecting the others to share this value). They should be
+   used whenever the assumption that all probes share the same value for the
+   accessed attribute is made. For this reason, an exception is raised if it is
+   not the case.
+
+   If probes are active (*i.e.* :meth:`BaylibreAcmeInstrument.reset` has been
+   called), only these are read for the value of the attribute (as others have
+   been tagged to be ignored). If not, all probes are used.
+
+.. attribute:: BaylibreAcmeInstrument.probes
+
+   Dictionary of :class:`IIOINA226Instrument` instances representing the probes
+   connected to the ACME. If provided to the constructor, the keys are the
+   ``probe_names`` that were passed.
+
+ACME Probes (IIO Devices)
+"""""""""""""""""""""""""
+
+The following class is not supposed to be instantiated by the user code: the
+API is provided as the ACME probes can be accessed through the
+:attr:`BaylibreAcmeInstrument.probes` attribute.
+
+.. class:: IIOINA226Instrument(iio_device)
+
+   This class is a wrapper for the :class:`iio.Device` class and takes a valid
+   instance as ``iio_device``. It is not supposed to be instantiated by the
+   user and its partial documentation is provided for read-access only.
+
+.. attribute:: IIOINA226Instrument.shunt_resistor
+.. attribute:: IIOINA226Instrument.sample_rate_hz
+.. attribute:: IIOINA226Instrument.oversampling_ratio
+.. attribute:: IIOINA226Instrument.integration_time_shunt
+.. attribute:: IIOINA226Instrument.integration_time_bus
+.. attribute:: IIOINA226Instrument.OVERSAMPLING_RATIOS_AVAILABLE
+.. attribute:: IIOINA226Instrument.INTEGRATION_TIMES_AVAILABLE
+
+   These attributes are provided *for reference* and should not be assigned to
+   but can be used to make the user code more readable, if needed. Please note
+   that, as reading these attributes reads the underlying value from the
+   hardware, they should not be read when the ACME is active *i.e* when
+   :meth:`BaylibreAcmeInstrument.setup` has been called without calling
+   :meth:`BaylibreAcmeInstrument.teardown`.
+
+
+Examples
+""""""""
+
+The following example shows a basic use of an ACME at IP address
+``ACME_IP_ADDR`` with 2 probes connected, capturing all the channels during
+(roughly) 10 seconds at a sampling rate of 613 Hz and outputing the
+measurements to the CSV file ``acme.csv``:
+
+.. code-block:: python
+
+   import time
+   import devlib
+
+   acme = devlib.BaylibreAcmeNetworkInstrument(hostname=ACME_IP_ADDR,
+                                               probe_names=['battery', 'usb'])
+
+   int_times = acme.INTEGRATION_TIMES_AVAILABLE
+   ratios    = acme.OVERSAMPLING_RATIOS_AVAILABLE
+
+   acme.setup(shunt_resistor=20000,
+              integration_time_bus=int_times[1],
+              integration_time_shunt=int_times[1],
+              oversampling_ratio=ratios[1])
+
+   acme.reset()
+   acme.start()
+   time.sleep(10)
+   acme.stop()
+   acme.get_data('acme.csv')
+   acme.teardown()
+
+It is common to have different resistances for different probe shunt resistors.
+Furthermore, we may want to have different sampling frequencies for different
+probes (*e.g.* if it is known that the USB voltage changes rather slowly).
+Finally, it is possible to set the integration times for the bus and shunt
+voltages of a same probe to different values. The following call to
+:meth:`BaylibreAcmeInstrument.setup` illustrates these:
+
+.. code-block:: python
+
+   acme.setup(shunt_resistor=[20000, 10000],
+              integration_time_bus=[int_times[2], int_times[3]],
+              integration_time_shunt=[int_times[3], int_times[4]],
+              oversampling_ratio=[ratios[0], ratios[1]])
+
+   for n, p in acme.probes.iteritems():
+      print('{}:'.format(n))
+      print('    T_bus = {} s'.format(p.integration_time_bus))
+      print('    T_shn = {} s'.format(p.integration_time_shunt))
+      print('    N     = {}'.format(p.oversampling_ratio))
+      print('    freq  = {} Hz'.format(p.sample_rate_hz))
+
+   # Output:
+   #
+   #   battery:
+   #       T_bus = 0.000332 s
+   #       T_shn = 0.000588 s
+   #       N     = 1
+   #       freq  = 1087 Hz
+   #   usb:
+   #       T_bus = 0.000588 s
+   #       T_shn = 0.0011 s
+   #       N     = 4
+   #       freq  = 148 Hz
+
+Please keep in mind that calling ``acme.get_data('acme.csv')`` after capturing
+samples with this setup will output signals with the same sampling frequency
+(the highest one among the sampling frequencies) as the signals are resampled
+to output a single time signal.
+
+.. rubric:: Footnotes
+
+.. [#acme_probe_variants] There exist different variants of the ACME probe (USB, Jack, shunt resistor) but they all use the same probing hardware (the TI INA226) and don't differ from the point of view of the software stack (at any level, including devlib, the highest one)
+
+.. [#acme_name_conflicts] Be careful that in cases where multiple ACME boards are being used, it may be required to manually handle name conflicts

doc/modules.rst

@@ -212,6 +212,9 @@ define the following class attributes:
         :early: The module will be installed when a :class:`Target` is first
                 created. This should be used for modules that do not rely on a
                 live connection to the target.
+        :setup: The module will be installed after initial setup of the device
+                has been performed. This allows the module to utilize assets
+                deployed during the setup stage for example 'Busybox'.
 
 Additionally, a module must implement a static (or class) method :func:`probe`:
 

doc/overview.rst

@@ -2,10 +2,12 @@ Overview
 ========
 
 A :class:`Target` instance serves as the main interface to the target device.
-There currently three target interfaces:
+There are currently four target interfaces:
 
 - :class:`LinuxTarget` for interacting with Linux devices over SSH.
-- :class:`AndroidTraget` for interacting with Android devices over adb.
+- :class:`AndroidTarget` for interacting with Android devices over adb.
+- :class:`ChromeOsTarget`: for interacting with ChromeOS devices over SSH, and
+                           their Android containers over adb.
 - :class:`LocalLinuxTarget`: for interacting with the local Linux host.
 
 They all work in more-or-less the same way, with the major difference being in
@@ -37,6 +39,7 @@ instantiating each of the three target types.
                                         'password': 'sekrit',
                                         # or
                                         'keyfile': '/home/me/.ssh/id_rsa'})
+   # ChromeOsTarget connection is performed in the same way as LinuxTarget
 
    # For an Android target, you will need to pass the device name as reported
    # by "adb devices". If there is only one device visible to adb, you can omit
@@ -79,8 +82,14 @@ safe side, it's a good idea to call this once at the beginning of your scripts.
 Command Execution
 ~~~~~~~~~~~~~~~~~
 
-There are several ways to execute a command on the target. In each case, a
-:class:`TargetError` will be raised if something goes wrong. In each case, it is
+There are several ways to execute a command on the target. In each case, an
+instance of a subclass of :class:`TargetError` will be raised if something goes
+wrong. When a transient error is encountered such as the loss of the network
+connectivity, it will raise a :class:`TargetTransientError`. When the command
+fails, it will raise a :class:`TargetStableError` unless the
+``will_succeed=True`` parameter is specified, in which case a
+:class:`TargetTransientError` will be raised since it is assumed that the
+command cannot fail unless there is an environment issue. In each case, it is
 also possible to specify ``as_root=True`` if the specified command should be
 executed as root.
 
@@ -213,6 +222,66 @@ executables_directory
    t.push('/local/path/to/assets.tar.gz', t.get_workpath('assets.tar.gz'))
 
 
+Exceptions Handling
+-------------------
+
+Devlib custom exceptions all derive from :class:`DevlibError`. Some exceptions
+are further categorized into :class:`DevlibTransientError` and
+:class:`DevlibStableError`. Transient errors are raised when there is an issue
+in the environment that can happen randomly such as the loss of network
+connectivity. Even a properly configured environment can be subject to such
+transient errors. Stable errors are related to either programming errors or
+configuration issues in the broad sense. This distinction allows quicker
+analysis of failures, since most transient errors can be ignored unless they
+happen at an alarming rate. :class:`DevlibTransientError` usually propagates up
+to the caller of devlib APIs, since it means that an operation could not
+complete. Retrying it or bailing out is therefore a responsability of the caller.
+
+The hierarchy is as follows:
+
+- :class:`DevlibError`
+   
+   - :class:`WorkerThreadError`
+   - :class:`HostError`
+   - :class:`TargetError`
+      
+      - :class:`TargetStableError`
+      - :class:`TargetTransientError`
+      - :class:`TargetNotRespondingError`
+   
+   - :class:`DevlibStableError`
+      
+      - :class:`TargetStableError`
+
+   - :class:`DevlibTransientError`
+
+      - :class:`TimeoutError`
+      - :class:`TargetTransientError`
+      - :class:`TargetNotRespondingError`
+
+
+Extending devlib
+~~~~~~~~~~~~~~~~
+
+New devlib code is likely to face the decision of raising a transient or stable
+error. When it is unclear which one should be used, it can generally be assumed
+that the system is properly configured and therefore, the error is linked to an
+environment transient failure. If a function is somehow probing a property of a
+system in the broad meaning, it can use a stable error as a way to signal a
+non-expected value of that property even if it can also face transient errors.
+An example are the various ``execute()`` methods where the command can generally
+not be assumed to be supposed to succeed by devlib. Their failure does not
+usually come from an environment random issue, but for example a permission
+error. The user can use such expected failure to probe the system. Another
+example is boot completion detection on Android: boot failure cannot be
+distinguished from a timeout which is too small. A non-transient exception is
+still raised, since assuming the timeout comes from a network failure would
+either make the function useless, or force the calling code to handle a
+transient exception under normal operation. The calling code would potentially
+wrongly catch transient exceptions raised by other functions as well and attach
+a wrong meaning to them.
+
+
 Modules
 -------
 
@@ -254,17 +323,14 @@ You can collected traces (currently, just ftrace) using
    # the buffer size to be used.
    trace = FtraceCollector(t, events=['power*'], buffer_size=40000)
 
-   # clear ftrace buffer
-   trace.reset()
-
-   # start trace collection
-   trace.start()
-
-   # Perform the operations you want to trace here...
-   import time; time.sleep(5)
-
-   # stop trace collection
-   trace.stop()
+   # As a context manager, clear ftrace buffer using trace.reset(),
+   # start trace collection using trace.start(), then stop it Using
+   # trace.stop(). Using a context manager brings the guarantee that
+   # tracing will stop even if an exception occurs, including 
+   # KeyboardInterrupt (ctr-C) and SystemExit (sys.exit)
+   with trace:
+      # Perform the operations you want to trace here...
+      import time; time.sleep(5)
 
    # extract the trace file from the target into a local file
    trace.get_trace('/tmp/trace.bin')

doc/target.rst

@@ -120,6 +120,16 @@ Target
    This is a dict that contains a mapping of OS version elements to their
    values. This mapping is OS-specific.
 
+.. attribute:: Target.system_id
+
+   A unique identifier for the system running on the target. This identifier is
+   intended to be uninque for the combination of hardware, kernel, and file
+   system.
+
+.. attribute:: Target.model
+
+   The model name/number of the target device.
+
 .. attribute:: Target.cpuinfo
 
    This is a :class:`Cpuinfo` instance which contains parsed contents of
@@ -222,7 +232,7 @@ Target
    :param timeout: timeout (in seconds) for the transfer; if the transfer does
        not  complete within this period, an exception will be raised.
 
-.. method:: Target.execute(command [, timeout [, check_exit_code [, as_root]]])
+.. method:: Target.execute(command [, timeout [, check_exit_code [, as_root [, strip_colors [, will_succeed]]]]])
 
    Execute the specified command on the target device and return its output.
 
@@ -235,6 +245,13 @@ Target
        raised if it is not ``0``.
    :param as_root: The command will be executed as root. This will fail on
        unrooted targets.
+   :param strip_colours: The command output will have colour encodings and
+       most ANSI escape sequences striped out before returning.
+   :param will_succeed: The command is assumed to always succeed, unless there is
+       an issue in the environment like the loss of network connectivity. That
+       will make the method always raise an instance of a subclass of
+       :class:`DevlibTransientError` when the command fails, instead of a
+       :class:`DevlibStableError`.
 
 .. method:: Target.background(command [, stdout [, stderr [, as_root]]])
 
@@ -329,7 +346,7 @@ Target
        some sysfs entries silently failing to set the written value without
        returning an error code.
 
-.. method:: Target.read_tree_values(path, depth=1, dictcls=dict):
+.. method:: Target.read_tree_values(path, depth=1, dictcls=dict, [, tar [, decode_unicode [, strip_null_char ]]]):
 
    Read values of all sysfs (or similar) file nodes under ``path``, traversing
    up to the maximum depth ``depth``.
@@ -341,9 +358,18 @@ Target
    value is a dict-line object  with a key for every entry under ``path``
    mapping onto its value or further dict-like objects as appropriate.
 
+   Although the default behaviour should suit most users, it is possible to
+   encounter issues when reading binary files, or files with colons in their
+   name for example. In such cases, the ``tar`` parameter can be set to force a
+   full archive of the tree using tar, hence providing a more robust behaviour.
+   This can, however, slow down the read process significantly.
+
    :param path: sysfs path to scan
    :param depth: maximum depth to descend
    :param dictcls: a dict-like type to be used for each level of the hierarchy.
+   :param tar: the files will be read using tar rather than grep
+   :param decode_unicode: decode the content of tar-ed files as utf-8
+   :param strip_null_char: remove null chars from utf-8 decoded files
 
 .. method:: Target.read_tree_values_flat(path, depth=1):
 

setup.py

@@ -41,23 +41,13 @@ except OSError:
     pass
 
 
-with open(os.path.join(devlib_dir, '__init__.py')) as fh:
-    # Extract the version by parsing the text of the file,
-    # as may not be able to load as a module yet.
-    for line in fh:
-        if '__version__' in line:
-            parts = line.split("'")
-            __version__ = parts[1]
-            break
-    else:
-        raise RuntimeError('Did not see __version__')
-
-    vh_path = os.path.join(devlib_dir, 'utils', 'version.py')
-    # can load this, as it does not have any devlib imports
-    version_helper = imp.load_source('version_helper', vh_path)
-    commit = version_helper.get_commit()
-    if commit:
-        __version__ = '{}+{}'.format(__version__, commit)
+vh_path = os.path.join(devlib_dir, 'utils', 'version.py')
+# can load this, as it does not have any devlib imports
+version_helper = imp.load_source('version_helper', vh_path)
+__version__ = version_helper.get_devlib_version()
+commit = version_helper.get_commit()
+if commit:
+    __version__ = '{}+{}'.format(__version__, commit)
 
 
 packages = []
@@ -94,11 +84,15 @@ params = dict(
         'pyserial',  # Serial port interface
         'wrapt',  # Basic for construction of decorator functions
         'future', # Python 2-3 compatibility
+        'enum34;python_version<"3.4"', # Enums for Python < 3.4
+        'pandas',
+        'numpy',
     ],
     extras_require={
         'daq': ['daqpower'],
         'doc': ['sphinx'],
         'monsoon': ['python-gflags'],
+        'acme': ['pandas', 'numpy'],
     },
     # https://pypi.python.org/pypi?%3Aaction=list_classifiers
     classifiers=[

tests/test_target.py

@@ -0,0 +1,32 @@
+import os
+import shutil
+import tempfile
+from unittest import TestCase
+
+from devlib import LocalLinuxTarget
+
+
+class TestReadTreeValues(TestCase):
+
+    def test_read_multiline_values(self):
+        data = {
+            'test1': '1',
+            'test2': '2\n\n',
+            'test3': '3\n\n4\n\n',
+        }
+
+        tempdir = tempfile.mkdtemp(prefix='devlib-test-')
+        for key, value in data.items():
+            path = os.path.join(tempdir, key)
+            with open(path, 'w') as wfh:
+                wfh.write(value)
+
+        t = LocalLinuxTarget(connection_settings={'unrooted': True})
+        raw_result = t.read_tree_values_flat(tempdir)
+        result = {os.path.basename(k): v for k, v in raw_result.items()}
+
+        shutil.rmtree(tempdir)
+
+        self.assertEqual({k: v.strip()
+                          for k, v in data.items()},
+                         result)