From c837a29299562f434bf18253cc60750de216dfd1 Mon Sep 17 00:00:00 2001 From: Anthony Barbier Date: Wed, 28 Jun 2017 15:08:35 +0100 Subject: Let the user customize the way to execute superuser commands --- devlib/utils/ssh.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/devlib/utils/ssh.py b/devlib/utils/ssh.py index 8704008..89613bd 100644 --- a/devlib/utils/ssh.py +++ b/devlib/utils/ssh.py @@ -160,7 +160,8 @@ class SshConnection(object): telnet=False, password_prompt=None, original_prompt=None, - platform=None + platform=None, + sudo_cmd="sudo -- sh -c '{}'" ): self.host = host self.username = username @@ -169,6 +170,7 @@ 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 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) @@ -212,7 +214,7 @@ class SshConnection(object): port_string = '-p {}'.format(self.port) if self.port else '' keyfile_string = '-i {}'.format(self.keyfile) if self.keyfile else '' if as_root: - command = "sudo -- sh -c '{}'".format(command) + command = self.sudo_cmd.format(command) command = '{} {} {} {}@{} {}'.format(ssh, keyfile_string, port_string, self.username, self.host, command) logger.debug(command) if self.password: @@ -240,7 +242,7 @@ class SshConnection(object): # As we're already root, there is no need to use sudo. as_root = False if as_root: - command = "sudo -- sh -c '{}'".format(escape_single_quotes(command)) + command = self.sudo_cmd.format(escape_single_quotes(command)) if log: logger.debug(command) self.conn.sendline(command) -- cgit v1.2.3 From 1199f2512be2367ba1c97eafdbb56ffee30cb3e7 Mon Sep 17 00:00:00 2001 From: Anthony Barbier Date: Wed, 28 Jun 2017 17:15:05 +0100 Subject: Check if any big/LITTLE core is online before dereferencing an potentially empty list --- devlib/module/biglittle.py | 104 +++++++++++++++++++++++++++++++++------------ 1 file changed, 78 insertions(+), 26 deletions(-) diff --git a/devlib/module/biglittle.py b/devlib/module/biglittle.py index eafcb0a..d9ed2bf 100644 --- a/devlib/module/biglittle.py +++ b/devlib/module/biglittle.py @@ -44,79 +44,131 @@ class BigLittleModule(Module): # cpufreq def list_bigs_frequencies(self): - return self.target.cpufreq.list_frequencies(self.bigs_online[0]) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + return self.target.cpufreq.list_frequencies(bigs_online[0]) def list_bigs_governors(self): - return self.target.cpufreq.list_governors(self.bigs_online[0]) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + return self.target.cpufreq.list_governors(bigs_online[0]) def list_bigs_governor_tunables(self): - return self.target.cpufreq.list_governor_tunables(self.bigs_online[0]) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + return self.target.cpufreq.list_governor_tunables(bigs_online[0]) def list_littles_frequencies(self): - return self.target.cpufreq.list_frequencies(self.littles_online[0]) + littles_online = self.littles_online + if len(littles_online) > 0: + return self.target.cpufreq.list_frequencies(littles_online[0]) def list_littles_governors(self): - return self.target.cpufreq.list_governors(self.littles_online[0]) + littles_online = self.littles_online + if len(littles_online) > 0: + return self.target.cpufreq.list_governors(littles_online[0]) def list_littles_governor_tunables(self): - return self.target.cpufreq.list_governor_tunables(self.littles_online[0]) + littles_online = self.littles_online + if len(littles_online) > 0: + return self.target.cpufreq.list_governor_tunables(littles_online[0]) def get_bigs_governor(self): - return self.target.cpufreq.get_governor(self.bigs_online[0]) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + return self.target.cpufreq.get_governor(bigs_online[0]) def get_bigs_governor_tunables(self): - return self.target.cpufreq.get_governor_tunables(self.bigs_online[0]) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + return self.target.cpufreq.get_governor_tunables(bigs_online[0]) def get_bigs_frequency(self): - return self.target.cpufreq.get_frequency(self.bigs_online[0]) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + return self.target.cpufreq.get_frequency(bigs_online[0]) def get_bigs_min_frequency(self): - return self.target.cpufreq.get_min_frequency(self.bigs_online[0]) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + return self.target.cpufreq.get_min_frequency(bigs_online[0]) def get_bigs_max_frequency(self): - return self.target.cpufreq.get_max_frequency(self.bigs_online[0]) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + return self.target.cpufreq.get_max_frequency(bigs_online[0]) def get_littles_governor(self): - return self.target.cpufreq.get_governor(self.littles_online[0]) + littles_online = self.littles_online + if len(littles_online) > 0: + return self.target.cpufreq.get_governor(littles_online[0]) def get_littles_governor_tunables(self): - return self.target.cpufreq.get_governor_tunables(self.littles_online[0]) + littles_online = self.littles_online + if len(littles_online) > 0: + return self.target.cpufreq.get_governor_tunables(littles_online[0]) def get_littles_frequency(self): - return self.target.cpufreq.get_frequency(self.littles_online[0]) + littles_online = self.littles_online + if len(littles_online) > 0: + return self.target.cpufreq.get_frequency(littles_online[0]) def get_littles_min_frequency(self): - return self.target.cpufreq.get_min_frequency(self.littles_online[0]) + littles_online = self.littles_online + if len(littles_online) > 0: + return self.target.cpufreq.get_min_frequency(littles_online[0]) def get_littles_max_frequency(self): - return self.target.cpufreq.get_max_frequency(self.littles_online[0]) + littles_online = self.littles_online + if len(littles_online) > 0: + return self.target.cpufreq.get_max_frequency(littles_online[0]) def set_bigs_governor(self, governor, **kwargs): - self.target.cpufreq.set_governor(self.bigs_online[0], governor, **kwargs) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + self.target.cpufreq.set_governor(bigs_online[0], governor, **kwargs) def set_bigs_governor_tunables(self, governor, **kwargs): - self.target.cpufreq.set_governor_tunables(self.bigs_online[0], governor, **kwargs) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + self.target.cpufreq.set_governor_tunables(bigs_online[0], governor, **kwargs) def set_bigs_frequency(self, frequency, exact=True): - self.target.cpufreq.set_frequency(self.bigs_online[0], frequency, exact) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + self.target.cpufreq.set_frequency(bigs_online[0], frequency, exact) def set_bigs_min_frequency(self, frequency, exact=True): - self.target.cpufreq.set_min_frequency(self.bigs_online[0], frequency, exact) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + self.target.cpufreq.set_min_frequency(bigs_online[0], frequency, exact) def set_bigs_max_frequency(self, frequency, exact=True): - self.target.cpufreq.set_max_frequency(self.bigs_online[0], frequency, exact) + bigs_online = self.bigs_online + if len(bigs_online) > 0: + self.target.cpufreq.set_max_frequency(bigs_online[0], frequency, exact) def set_littles_governor(self, governor, **kwargs): - self.target.cpufreq.set_governor(self.littles_online[0], governor, **kwargs) + littles_online = self.littles_online + if len(littles_online) > 0: + self.target.cpufreq.set_governor(littles_online[0], governor, **kwargs) def set_littles_governor_tunables(self, governor, **kwargs): - self.target.cpufreq.set_governor_tunables(self.littles_online[0], governor, **kwargs) + littles_online = self.littles_online + if len(littles_online) > 0: + self.target.cpufreq.set_governor_tunables(littles_online[0], governor, **kwargs) def set_littles_frequency(self, frequency, exact=True): - self.target.cpufreq.set_frequency(self.littles_online[0], frequency, exact) + littles_online = self.littles_online + if len(littles_online) > 0: + self.target.cpufreq.set_frequency(littles_online[0], frequency, exact) def set_littles_min_frequency(self, frequency, exact=True): - self.target.cpufreq.set_min_frequency(self.littles_online[0], frequency, exact) + littles_online = self.littles_online + if len(littles_online) > 0: + self.target.cpufreq.set_min_frequency(littles_online[0], frequency, exact) def set_littles_max_frequency(self, frequency, exact=True): - self.target.cpufreq.set_max_frequency(self.littles_online[0], frequency, exact) + littles_online = self.littles_online + if len(littles_online) > 0: + self.target.cpufreq.set_max_frequency(littles_online[0], frequency, exact) -- cgit v1.2.3 From 3660361df094dceefea3205b2da385dc964b0a38 Mon Sep 17 00:00:00 2001 From: Anthony Barbier Date: Fri, 30 Jun 2017 13:15:23 +0100 Subject: Raise a ValueError when trying to set a property of a type of core which is offline --- devlib/module/biglittle.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/devlib/module/biglittle.py b/devlib/module/biglittle.py index d9ed2bf..e353229 100644 --- a/devlib/module/biglittle.py +++ b/devlib/module/biglittle.py @@ -127,48 +127,68 @@ class BigLittleModule(Module): bigs_online = self.bigs_online if len(bigs_online) > 0: 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: 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: 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: 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: 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: 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: 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: 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: 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: self.target.cpufreq.set_max_frequency(littles_online[0], frequency, exact) + else: + raise ValueError("All littles appear to be offline") -- cgit v1.2.3 From 92b0c25ed3b1bd0c68cf1004544936e5074ca82d Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Thu, 6 Jul 2017 16:01:15 +0100 Subject: target: Add background_invoke() method --- devlib/target.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/devlib/target.py b/devlib/target.py index f3aaea0..65d5600 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -351,6 +351,38 @@ class Target(object): command = 'cd {} && {}'.format(in_directory, command) return self.execute(command, as_root=as_root, timeout=timeout) + def background_invoke(self, binary, args=None, in_directory=None, + on_cpus=None, as_root=False): + """ + Executes the specified binary as a background task under the + specified conditions. + + :binary: binary to execute. Must be present and executable on the device. + :args: arguments to be passed to the binary. The can be either a list or + a string. + :in_directory: execute the binary in the specified directory. This must + be an absolute path. + :on_cpus: taskset the binary to these CPUs. This may be a single ``int`` (in which + case, it will be interpreted as the mask), a list of ``ints``, in which + case this will be interpreted as the list of cpus, or string, which + will be interpreted as a comma-separated list of cpu ranges, e.g. + ``"0,4-7"``. + :as_root: Specify whether the command should be run as root + + :returns: the subprocess instance handling that command + """ + command = binary + if args: + if isiterable(args): + args = ' '.join(args) + command = '{} {}'.format(command, args) + if on_cpus: + on_cpus = bitmask(on_cpus) + command = '{} taskset 0x{:x} {}'.format(self.busybox, on_cpus, command) + if in_directory: + command = 'cd {} && {}'.format(in_directory, command) + return self.background(command, as_root=as_root) + def kick_off(self, command, as_root=False): raise NotImplementedError() -- cgit v1.2.3 From c8af995392a8867c5663d2855b08595a6b98d78f Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Thu, 6 Jul 2017 16:01:30 +0100 Subject: doc/target: Add background_invoke() documentation --- doc/target.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/doc/target.rst b/doc/target.rst index d14942e..5339a72 100644 --- a/doc/target.rst +++ b/doc/target.rst @@ -265,6 +265,24 @@ Target :param timeout: If this is specified and invocation does not terminate within this number of seconds, an exception will be raised. +.. method:: Target.background_invoke(binary [, args [, in_directory [, on_cpus [, as_root ]]]]) + + Execute the specified binary on target (must already be installed) as a background + task, under the specified conditions and return the :class:`subprocess.Popen` + instance for the command. + + :param binary: binary to execute. Must be present and executable on the device. + :param args: arguments to be passed to the binary. The can be either a list or + a string. + :param in_directory: execute the binary in the specified directory. This must + be an absolute path. + :param on_cpus: taskset the binary to these CPUs. This may be a single ``int`` (in which + case, it will be interpreted as the mask), a list of ``ints``, in which + case this will be interpreted as the list of cpus, or string, which + will be interpreted as a comma-separated list of cpu ranges, e.g. + ``"0,4-7"``. + :param as_root: Specify whether the command should be run as root + .. method:: Target.kick_off(command [, as_root]) Kick off the specified command on the target and return immediately. Unlike -- cgit v1.2.3 From baedd676a94c6f912d52f4d5fad5fe5eff724616 Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Tue, 4 Jul 2017 17:57:16 +0100 Subject: module: Control and parse gem5's statistics log file Gem5's statistics log file contains plenty of interesting information that are not exposed so far. This module enables control and parsing of the statistics file by: - configuring periodic dumps of statistics; - marking Regions of Interest (ROIs); - and extracting values of specific fields during the ROIs. --- devlib/module/gem5stats.py | 148 +++++++++++++++++++++++++++++++++++++++++++++ devlib/utils/gem5.py | 43 +++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 devlib/module/gem5stats.py create mode 100644 devlib/utils/gem5.py diff --git a/devlib/module/gem5stats.py b/devlib/module/gem5stats.py new file mode 100644 index 0000000..3d109aa --- /dev/null +++ b/devlib/module/gem5stats.py @@ -0,0 +1,148 @@ +# Copyright 2017 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 os.path +from collections import defaultdict + +import devlib +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 + + +class Gem5ROI: + def __init__(self, number, target): + self.target = target + self.number = number + self.running = False + + def start(self): + if self.running: + return False + self.target.execute('m5 roistart {}'.format(self.number)) + self.running = True + return True + + def stop(self): + if not self.running: + return False + self.target.execute('m5 roiend {}'.format(self.number)) + self.running = False + return True + +class Gem5StatsModule(Module): + ''' + 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 + use. The translation of labels into gem5 ROI numbers will be performed + internally in order to avoid conflicts between multiple clients. + ''' + name = 'gem5stats' + + @staticmethod + def probe(target): + return isinstance(target.platform, Gem5SimulationPlatform) + + def __init__(self, target): + super(Gem5StatsModule, self).__init__(target) + self._current_origin = 0 + self._stats_file_path = os.path.join(target.platform.gem5_out_dir, + 'stats.txt') + self.rois = {} + + def book_roi(self, label): + if label in self.rois: + raise KeyError('ROI label {} already used'.format(label)) + if len(self.rois) >= GEM5STATS_ROI_NUMBER: + raise RuntimeError('Too many ROIs reserved') + all_rois = set(xrange(GEM5STATS_ROI_NUMBER)) + used_rois = set([roi.number for roi in self.rois.values()]) + avail_rois = all_rois - used_rois + self.rois[label] = Gem5ROI(list(avail_rois)[0], self.target) + + def free_roi(self, label): + if label not in self.rois: + raise KeyError('ROI label {} not reserved yet'.format(label)) + self.rois[label].stop() + del self.rois[label] + + def roi_start(self, label): + 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)) + + 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)) + + def start_periodic_dump(self, delay_ns=0, period_ns=10000000): + # Default period is 10ms because it's roughly what's needed to have + # accurate power estimations + if delay_ns < 0 or period_ns < 0: + 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): + ''' + Tries to match the list of keys passed as parameter over the statistics + dumps covered by selected ROIs since origin. 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. + + Keys must match fields in gem5's statistics log file. Key example: + system.cluster0.cores0.power_model.static_power + ''' + for label in rois_labels: + if label not in self.rois: + raise KeyError('Impossible to match ROI label {}'.format(label)) + if self.rois[label].running: + self.logger.warning('Trying to match records in statistics file' + ' while ROI {} is running'.format(label)) + + records = {} + for key in keys: + records[key] = defaultdict(list) + with open(self._stats_file_path, 'r') as stats_file: + stats_file.seek(self._current_origin) + for dump in iter_statistics_dump(stats_file): + for label in rois_labels: + # Save records only when ROIs are ON + roi_field = 'ROI::{}'.format(self.rois[label].number) + if (roi_field in dump) and (int(dump[roi_field]) == 1): + for key in keys: + records[key][label].append(dump[key]) + return records + + def reset_origin(self): + ''' + Place origin right after the last full dump in the file + ''' + last_dump_tail = self._current_origin + # Dump & reset stats to start from a fresh state + self.target.execute('m5 dumpresetstats') + with open(self._stats_file_path, 'r') as stats_file: + for line in stats_file: + if GEM5STATS_DUMP_TAIL in line: + last_dump_tail = stats_file.tell() + self._current_origin = last_dump_tail + diff --git a/devlib/utils/gem5.py b/devlib/utils/gem5.py new file mode 100644 index 0000000..c609d70 --- /dev/null +++ b/devlib/utils/gem5.py @@ -0,0 +1,43 @@ +# Copyright 2017 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 re + + +GEM5STATS_FIELD_REGEX = re.compile("^(?P[^- ]\S*) +(?P[^#]+).+$") +GEM5STATS_DUMP_HEAD = '---------- Begin Simulation Statistics ----------' +GEM5STATS_DUMP_TAIL = '---------- End Simulation Statistics ----------' +GEM5STATS_ROI_NUMBER = 8 + + +def iter_statistics_dump(stats_file): + ''' + Yields statistics dumps as dicts. The parameter is assumed to be a stream + reading from the statistics log file. + ''' + cur_dump = {} + while True: + line = stats_file.readline() + if not line: + break + if GEM5STATS_DUMP_TAIL in line: + yield cur_dump + cur_dump = {} + else: + res = GEM5STATS_FIELD_REGEX.match(line) + if res: + k = res.group("key") + v = res.group("value").split() + cur_dump[k] = v[0] if len(v)==1 else set(v) + -- cgit v1.2.3 From ce48ad217d7de1b268ccc08515e4a44429220e7b Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Fri, 7 Jul 2017 18:12:51 +0100 Subject: module: gem5stats: add an iterator to match records --- devlib/module/gem5stats.py | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/devlib/module/gem5stats.py b/devlib/module/gem5stats.py index 3d109aa..ba24a43 100644 --- a/devlib/module/gem5stats.py +++ b/devlib/module/gem5stats.py @@ -28,6 +28,7 @@ class Gem5ROI: self.target = target self.number = number self.running = False + self.field = 'ROI::{}'.format(number) def start(self): if self.running: @@ -42,7 +43,7 @@ class Gem5ROI: self.target.execute('m5 roiend {}'.format(self.number)) self.running = False return True - + class Gem5StatsModule(Module): ''' Module controlling Region of Interest (ROIs) markers, satistics dump @@ -56,7 +57,7 @@ class Gem5StatsModule(Module): @staticmethod def probe(target): - return isinstance(target.platform, Gem5SimulationPlatform) + return isinstance(target.platform, Gem5SimulationPlatform) def __init__(self, target): super(Gem5StatsModule, self).__init__(target) @@ -109,6 +110,22 @@ class Gem5StatsModule(Module): in-order list of records for the key under consideration during the active intervals of the ROI. + Keys must match fields in gem5's statistics log file. Key example: + system.cluster0.cores0.power_model.static_power + ''' + records = defaultdict(lambda : defaultdict(list)) + for record, active_rois in self.match_iter(keys, rois_labels): + for key in record: + for roi_label in active_rois: + records[key][roi_label].append(record[key]) + return records + + def match_iter(self, keys, rois_labels): + ''' + Yields for each dump since origin a pair containing: + 1. a dict storing the values corresponding to each of the specified keys + 2. the list of currently active ROIs among those passed as parameters. + Keys must match fields in gem5's statistics log file. Key example: system.cluster0.cores0.power_model.static_power ''' @@ -118,20 +135,19 @@ class Gem5StatsModule(Module): if self.rois[label].running: self.logger.warning('Trying to match records in statistics file' ' while ROI {} is running'.format(label)) + + def roi_active(roi_label, dump): + roi = self.rois[roi_label] + return (roi.field in dump) and (int(dump[roi.field]) == 1) - records = {} - for key in keys: - records[key] = defaultdict(list) with open(self._stats_file_path, 'r') as stats_file: stats_file.seek(self._current_origin) for dump in iter_statistics_dump(stats_file): - for label in rois_labels: - # Save records only when ROIs are ON - roi_field = 'ROI::{}'.format(self.rois[label].number) - if (roi_field in dump) and (int(dump[roi_field]) == 1): - for key in keys: - records[key][label].append(dump[key]) - return records + active_rois = [l for l in rois_labels if roi_active(l, dump)] + if active_rois: + record = {k: dump[k] for k in keys} + yield (record, active_rois) + def reset_origin(self): ''' -- cgit v1.2.3 From 7a827e2b116c4fed24ec9076d7b0689cb1f55a96 Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Wed, 17 May 2017 18:17:29 +0100 Subject: instrument: Add power monitoring support on Gem5 platforms --- devlib/__init__.py | 1 + devlib/instrument/gem5power.py | 74 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 devlib/instrument/gem5power.py diff --git a/devlib/__init__.py b/devlib/__init__.py index 911c50d..2f50632 100644 --- a/devlib/__init__.py +++ b/devlib/__init__.py @@ -17,6 +17,7 @@ from devlib.instrument.frames import GfxInfoFramesInstrument 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.trace.ftrace import FtraceCollector diff --git a/devlib/instrument/gem5power.py b/devlib/instrument/gem5power.py new file mode 100644 index 0000000..6411b3f --- /dev/null +++ b/devlib/instrument/gem5power.py @@ -0,0 +1,74 @@ +# Copyright 2017 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. + +from __future__ import division +import csv +import re + +from devlib.platform.gem5 import Gem5SimulationPlatform +from devlib.instrument import Instrument, CONTINUOUS, MeasurementsCsv +from devlib.exception import TargetError, HostError + + +class Gem5PowerInstrument(Instrument): + ''' + Instrument enabling power monitoring in gem5 + ''' + + mode = CONTINUOUS + roi_label = 'power_instrument' + + def __init__(self, target, power_sites): + ''' + Parameter power_sites is a list of gem5 identifiers for power values. + One example of such a field: + system.cluster0.cores0.power_model.static_power + ''' + if not isinstance(target.platform, Gem5SimulationPlatform): + raise TargetError('Gem5PowerInstrument requires a gem5 platform') + if not target.has('gem5stats'): + raise TargetError('Gem5StatsModule is not loaded') + super(Gem5PowerInstrument, self).__init__(target) + + # power_sites is assumed to be a list later + if isinstance(power_sites, list): + self.power_sites = power_sites + else: + self.power_sites = [power_sites] + self.add_channel('sim_seconds', 'time') + for field in self.power_sites: + self.add_channel(field, 'power') + self.target.gem5stats.book_roi(self.roi_label) + self.sample_period_ns = 10000000 + self.target.gem5stats.start_periodic_dump(0, self.sample_period_ns) + + def start(self): + self.target.gem5stats.roi_start(self.roi_label) + + def stop(self): + self.target.gem5stats.roi_end(self.roi_label) + + def get_data(self, outfile): + active_sites = [c.site for c in self.active_channels] + with open(outfile, 'wb') as wfh: + writer = csv.writer(wfh) + writer.writerow([c.label for c in self.active_channels]) # headers + for rec, rois in self.target.gem5stats.match_iter(active_sites, [self.roi_label]): + writer.writerow([float(rec[s]) for s in active_sites]) + return MeasurementsCsv(outfile, self.active_channels) + + def reset(self, sites=None, kinds=None, channels=None): + super(Gem5PowerInstrument, self).reset(sites, kinds, channels) + self.target.gem5stats.reset_origin() + -- cgit v1.2.3 From 86c9b6a1c7ef593dacede2c622cbaaefcad6b0dd Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Mon, 10 Jul 2017 14:54:14 +0100 Subject: utils/misc: update MIDR table Update the MIDR CPU part number table with values from https://github.com/torvalds/linux/blob/master/arch/arm64/include/asm/cputype.h --- devlib/utils/misc.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/devlib/utils/misc.py b/devlib/utils/misc.py index b8626aa..b11b657 100644 --- a/devlib/utils/misc.py +++ b/devlib/utils/misc.py @@ -79,9 +79,18 @@ CPU_PART_MAP = { 0xd08: {None: 'A72'}, 0xd09: {None: 'A73'}, }, + 0x42: { # Broadcom + 0x516: {None: 'Vulcan'}, + 0x43: { # Cavium + 0x0a1: {None: 'Thunderx'}, + 0x0a2: {None: 'Thunderx81xx'}, + }, 0x4e: { # Nvidia 0x0: {None: 'Denver'}, }, + 0x50: { # AppliedMicro + 0x0: {None: 'xgene'}, + }, 0x51: { # Qualcomm 0x02d: {None: 'Scorpion'}, 0x04d: {None: 'MSM8960'}, @@ -91,6 +100,7 @@ CPU_PART_MAP = { }, 0x205: {0x1: 'KryoSilver'}, 0x211: {0x1: 'KryoGold'}, + 0x800: {None: 'Falkor'}, }, 0x56: { # Marvell 0x131: { -- cgit v1.2.3 From 0a95bbed879f8f350325f2a2d3c7056f2a531955 Mon Sep 17 00:00:00 2001 From: Ionela Voinescu Date: Mon, 10 Jul 2017 16:06:24 +0100 Subject: misc.py: fix syntax error introduced by 86c9b6a1c7e Signed-off-by: Ionela Voinescu --- devlib/utils/misc.py | 1 + 1 file changed, 1 insertion(+) diff --git a/devlib/utils/misc.py b/devlib/utils/misc.py index b11b657..d6a5093 100644 --- a/devlib/utils/misc.py +++ b/devlib/utils/misc.py @@ -81,6 +81,7 @@ CPU_PART_MAP = { }, 0x42: { # Broadcom 0x516: {None: 'Vulcan'}, + }, 0x43: { # Cavium 0x0a1: {None: 'Thunderx'}, 0x0a2: {None: 'Thunderx81xx'}, -- cgit v1.2.3 From f26f9427231a16652da2ba9064ea9ba89ff6701a Mon Sep 17 00:00:00 2001 From: Patrick Bellasi Date: Wed, 12 Jul 2017 12:27:28 +0100 Subject: tagert: factor out the wait_boot_complete code The connect() method embeds some code to wait for a target to be completely booted. Let's move this code into a dedicated function. Signed-off-by: Patrick Bellasi --- devlib/target.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index 35473b4..8a38a8c 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -877,8 +877,16 @@ class AndroidTarget(Target): pass self._connected_as_root = None - def connect(self, timeout=10, check_boot_completed=True): # pylint: disable=arguments-differ + def wait_boot_complete(self, timeout=10): start = time.time() + boot_completed = boolean(self.getprop('sys.boot_completed')) + while not boot_completed and timeout >= time.time() - start: + time.sleep(5) + boot_completed = boolean(self.getprop('sys.boot_completed')) + if not boot_completed: + raise TargetError('Connected but Android did not fully boot.') + + def connect(self, timeout=10, check_boot_completed=True): # pylint: disable=arguments-differ device = self.connection_settings.get('device') if device and ':' in device: # ADB does not automatically remove a network device from it's @@ -890,12 +898,7 @@ class AndroidTarget(Target): super(AndroidTarget, self).connect(timeout=timeout) if check_boot_completed: - boot_completed = boolean(self.getprop('sys.boot_completed')) - while not boot_completed and timeout >= time.time() - start: - time.sleep(5) - boot_completed = boolean(self.getprop('sys.boot_completed')) - if not boot_completed: - raise TargetError('Connected but Android did not fully boot.') + self.wait_boot_complete(timeout) def setup(self, executables=None): super(AndroidTarget, self).setup(executables) -- cgit v1.2.3 From 0c7eb9e91e46fd551d9986afebe10670b1f160aa Mon Sep 17 00:00:00 2001 From: Patrick Bellasi Date: Wed, 12 Jul 2017 12:30:30 +0100 Subject: target: add support to kill/restart and ADB connection Sometimes it could be useful to disconnect from the target, for example when a "pass thought" energy monitor is used to control the USB connection to the device. This patch adds a couple of utility methods which allows to kill the ADB server and restart it while waiting for the device to be ready to accept new connections. Signed-off-by: Patrick Bellasi --- devlib/target.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/devlib/target.py b/devlib/target.py index 8a38a8c..e9b24df 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1077,6 +1077,12 @@ class AndroidTarget(Target): def clear_logcat(self): adb_command(self.adb_name, 'logcat -c', timeout=30) + def adb_kill_server(self, timeout=30): + adb_command(self.adb_name, 'kill-server', timeout) + + def adb_wait_for_device(self, timeout=30): + adb_command(self.adb_name, 'wait-for-device', timeout) + def adb_reboot_bootloader(self, timeout=30): adb_command(self.adb_name, 'reboot-bootloader', timeout) -- cgit v1.2.3 From 85036fbb303681cc09bbf7800aac1a9e4648a247 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 12 Jul 2017 13:44:47 +0100 Subject: utils/android: better error message when exit code not detected Include the stdout/stderr output in the message of the exception raised when the exit code of the adb command cannot be detected. --- devlib/utils/android.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index f683190..e8ca78d 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -381,8 +381,9 @@ def adb_shell(device, command, timeout=None, check_exit_code=False, raise TargetError(message.format(re_search[0])) else: message = 'adb has returned early; did not get an exit code. '\ - 'Was kill-server invoked?' - raise TargetError(message) + 'Was kill-server invoked?\nOUTPUT:\n-----\n{}\n'\ + '-----\nERROR:\n-----\n{}\n-----' + raise TargetError(message.format(raw_output, error)) return output -- cgit v1.2.3 From 1fd5636217e6dd59a0e9914a0f0ec31bfdae4b95 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Tue, 10 Jan 2017 15:35:21 +0000 Subject: Documentation: Corrected typos --- doc/connection.rst | 10 +++++----- doc/instrumentation.rst | 26 +++++++++++++------------- doc/modules.rst | 12 ++++++------ doc/overview.rst | 6 +++--- doc/platform.rst | 10 +++++----- doc/target.rst | 10 +++++----- 6 files changed, 37 insertions(+), 37 deletions(-) diff --git a/doc/connection.rst b/doc/connection.rst index 1d8f098..70d9e25 100644 --- a/doc/connection.rst +++ b/doc/connection.rst @@ -99,7 +99,7 @@ Connection Types ``adb`` is part of the Android SDK (though stand-alone versions are also available). - :param device: The name of the adb divice. This is usually a unique hex + :param device: The name of the adb device. This is usually a unique hex string for USB-connected devices, or an ip address/port combination. To see connected devices, you can run ``adb devices`` on the host. @@ -126,21 +126,21 @@ Connection Types .. note:: ``keyfile`` and ``password`` can't be specified at the same time. - :param port: TCP port on which SSH server is litening on the remoted device. + :param port: TCP port on which SSH server is listening on the remote device. Omit to use the default port. :param timeout: Timeout for the connection in seconds. If a connection cannot be established within this time, an error will be raised. :param password_prompt: A string with the password prompt used by ``sshpass``. Set this if your version of ``sshpass`` - uses somethin other than ``"[sudo] password"``. + uses something other than ``"[sudo] password"``. .. class:: TelnetConnection(host, username, password=None, port=None,\ timeout=None, password_prompt=None,\ original_prompt=None) - A connectioned to a device on the network over Telenet. + A connection to a device on the network over Telenet. .. note:: Since Telenet protocol is does not support file transfer, scp is used for that purpose. @@ -153,7 +153,7 @@ Connection Types ``sshpass`` utility must be installed on the system. - :param port: TCP port on which SSH server is litening on the remoted device. + :param port: TCP port on which SSH server is listening on the remote device. Omit to use the default port. :param timeout: Timeout for the connection in seconds. If a connection cannot be established within this time, an error will be diff --git a/doc/instrumentation.rst b/doc/instrumentation.rst index 7beec79..3c777ac 100644 --- a/doc/instrumentation.rst +++ b/doc/instrumentation.rst @@ -114,14 +114,14 @@ Instrument :class:`Measurement` objects (one for each active channel). .. note:: This method is only implemented by :class:`Instrument`\ s that - support ``INSTANTANEOUS`` measurment. + support ``INSTANTANEOUS`` measurement. .. method:: Instrument.start() Starts collecting measurements from ``active_channels``. .. note:: This method is only implemented by :class:`Instrument`\ s that - support ``CONTINUOUS`` measurment. + support ``CONTINUOUS`` measurement. .. method:: Instrument.stop() @@ -129,14 +129,14 @@ Instrument :func:`start()`. .. note:: This method is only implemented by :class:`Instrument`\ s that - support ``CONTINUOUS`` measurment. + support ``CONTINUOUS`` measurement. .. method:: Instrument.get_data(outfile) Write collected data into ``outfile``. Must be called after :func:`stop()`. Data will be written in CSV format with a column for each channel and a row for each sample. Column heading will be channel, labels in the form - ``_`` (see :class:`InstrumentChannel`). The order of the coluns + ``_`` (see :class:`InstrumentChannel`). The order of the columns will be the same as the order of channels in ``Instrument.active_channels``. This returns a :class:`MeasurementCsv` instance associated with the outfile @@ -144,7 +144,7 @@ Instrument returned by ``take_measurement()``. .. note:: This method is only implemented by :class:`Instrument`\ s that - support ``CONTINUOUS`` measurment. + support ``CONTINUOUS`` measurement. .. attribute:: Instrument.sample_rate_hz @@ -163,16 +163,16 @@ Instrument Channel ``site`` and a ``measurement_type``. A ``site`` indicates where on the target a measurement is collected from - (e.g. a volage rail or location of a sensor). + (e.g. a voltage rail or location of a sensor). A ``measurement_type`` is an instance of :class:`MeasurmentType` that - describes what sort of measurment this is (power, temperature, etc). Each - mesurement type has a standard unit it is reported in, regardless of an + describes what sort of measurement this is (power, temperature, etc). Each + measurement type has a standard unit it is reported in, regardless of an instrument used to collect it. A channel (i.e. site/measurement_type combination) is unique per instrument, however there may be more than one channel associated with one site (e.g. for - both volatage and power). + both voltage and power). It should not be assumed that any site/measurement_type combination is valid. The list of available channels can queried with @@ -180,22 +180,22 @@ Instrument Channel .. attribute:: InstrumentChannel.site - The name of the "site" from which the measurments are collected (e.g. voltage + The name of the "site" from which the measurements are collected (e.g. voltage rail, sensor, etc). .. attribute:: InstrumentChannel.kind - A string indingcating the type of measrument that will be collted. This is + A string indicating the type of measurement that will be collected. This is the ``name`` of the :class:`MeasurmentType` associated with this channel. .. attribute:: InstrumentChannel.units - Units in which measurment will be reported. this is determined by the + Units in which measurement will be reported. this is determined by the underlying :class:`MeasurmentType`. .. attribute:: InstrumentChannel.label - A label that can be attached to measurments associated with with channel. + A label that can be attached to measurements associated with with channel. This is constructed with :: '{}_{}'.format(self.site, self.kind) diff --git a/doc/modules.rst b/doc/modules.rst index ac75f99..b89b488 100644 --- a/doc/modules.rst +++ b/doc/modules.rst @@ -72,7 +72,7 @@ policies (governors). The ``devlib`` module exposes the following interface :param cpu: The cpu; could be a numeric or the corresponding string (e.g. ``1`` or ``"cpu1"``). - :param governor: The name of the governor. This must be one of the governors + :param governor: The name of the governor. This must be one of the governors supported by the CPU (as returned by ``list_governors()``. Keyword arguments may be used to specify governor tunable values. @@ -126,7 +126,7 @@ policies (governors). The ``devlib`` module exposes the following interface cpuidle ------- -``cpufreq`` is the kernel subsystem for managing CPU low power (idle) states. +``cpuidle`` is the kernel subsystem for managing CPU low power (idle) states. .. method:: target.cpuidle.get_driver() @@ -155,7 +155,7 @@ cpuidle Enable or disable the specified or all states (optionally on the specified CPU. -You can also call ``enable()`` or ``disable()`` on :class:`CpuidleState` objects +You can also call ``enable()`` or ``disable()`` on :class:`CpuidleState` objects returned by get_state(s). cgroups @@ -182,7 +182,7 @@ Every module (ultimately) derives from :class:`Module` class. A module must define the following class attributes: :name: A unique name for the module. This cannot clash with any of the existing - names and must be a valid Python identifier, but is otherwise free-from. + names and must be a valid Python identifier, but is otherwise free-form. :kind: This identifies the type of functionality a module implements, which in turn determines the interface implemented by the module (all modules of the same kind must expose a consistent interface). This must be a valid @@ -271,7 +271,7 @@ HardResetModule .. method:: HardResetModule.__call__() Must be implemented by derived classes. - + Implements hard reset for a target devices. The equivalent of physically power cycling the device. This may be used by client code in situations where the target becomes unresponsive and/or a regular reboot is not @@ -355,7 +355,7 @@ for an "Acme" device. name = 'acme_hard_reset' def __call__(self): - # Assuming Acme board comes with a "reset-acme-board" utility + # Assuming Acme board comes with a "reset-acme-board" utility os.system('reset-acme-board {}'.format(self.target.name)) register_module(AcmeHardReset) diff --git a/doc/overview.rst b/doc/overview.rst index 421f053..7a60fb8 100644 --- a/doc/overview.rst +++ b/doc/overview.rst @@ -74,13 +74,13 @@ This sets up the target for ``devlib`` interaction. This includes creating working directories, deploying busybox, etc. It's usually enough to do this once for a new device, as the changes this makes will persist across reboots. However, there is no issue with calling this multiple times, so, to be on the -safe site, it's a good idea to call this once at the beginning of your scripts. +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 very case, it is +:class:`TargetError` will be raised if something goes wrong. In each case, it is also possible to specify ``as_root=True`` if the specified command should be executed as root. @@ -154,7 +154,7 @@ Process Control # kill all running instances of a process. t.killall('badexe', signal=signal.SIGKILL) - # List processes running on the target. This retruns a list of parsed + # List processes running on the target. This returns a list of parsed # PsEntry records. entries = t.ps() # e.g. print virtual memory sizes of all running sshd processes: diff --git a/doc/platform.rst b/doc/platform.rst index 5270c09..3449db3 100644 --- a/doc/platform.rst +++ b/doc/platform.rst @@ -18,7 +18,7 @@ it was not specified explicitly by the user. :param core_names: A list of CPU core names in the order they appear registered with the OS. If they are not specified, they will be queried at run time. - :param core_clusters: Alist with cluster ids of each core (starting with + :param core_clusters: A list with cluster ids of each core (starting with 0). If this is not specified, clusters will be inferred from core names (cores with the same name are assumed to be in a cluster). @@ -38,13 +38,13 @@ Versatile Express The generic platform may be extended to support hardware- or infrastructure-specific functionality. Platforms exist for ARM VersatileExpress-based :class:`Juno` and :class:`TC2` development boards. In -addition to the standard :class:`Platform` parameters above, these platfroms +addition to the standard :class:`Platform` parameters above, these platforms support additional configuration: .. class:: VersatileExpressPlatform - Normally, this would be instatiated via one of its derived classes + Normally, this would be instantiated via one of its derived classes (:class:`Juno` or :class:`TC2`) that set appropriate defaults for some of the parameters. @@ -63,7 +63,7 @@ support additional configuration: mounted on the host system. :param hard_reset_method: Specifies the method for hard-resetting the devices (e.g. if it becomes unresponsive and normal reboot - method doesn not work). Currently supported methods + method doesn't not work). Currently supported methods are: :dtr: reboot by toggling DTR line on the serial @@ -80,7 +80,7 @@ support additional configuration: The following values are currently supported: :uefi: Boot via UEFI menu, by selecting the entry - specified by ``uefi_entry`` paramter. If this + specified by ``uefi_entry`` parameter. If this entry does not exist, it will be automatically created based on values provided for ``image``, ``initrd``, ``fdt``, and ``bootargs`` parameters. diff --git a/doc/target.rst b/doc/target.rst index 5339a72..b64246a 100644 --- a/doc/target.rst +++ b/doc/target.rst @@ -38,7 +38,7 @@ Target by the connection's account). This location will be created, if necessary, during ``setup()``. - This location does *not* to be same as the system's executables + This location does *not* need to be same as the system's executables location. In fact, to prevent devlib from overwriting system's defaults, it better if this is a separate location, if possible. @@ -83,12 +83,12 @@ Target .. attribute:: Target.big_core - This is the name of the cores that the "big"s in an ARM big.LITTLE + This is the name of the cores that are the "big"s in an ARM big.LITTLE configuration. This is obtained via the underlying :class:`Platform`. .. attribute:: Target.little_core - This is the name of the cores that the "little"s in an ARM big.LITTLE + This is the name of the cores that are the "little"s in an ARM big.LITTLE configuration. This is obtained via the underlying :class:`Platform`. .. attribute:: Target.is_connected @@ -440,12 +440,12 @@ Target .. method:: Target.extract(path, dest=None) - Extracts the specified archive/file and returns the path to the extrated + Extracts the specified archive/file and returns the path to the extracted contents. The extraction method is determined based on the file extension. ``zip``, ``tar``, ``gzip``, and ``bzip2`` are supported. :param dest: Specified an on-target destination directory (which must exist) - for the extrated contents. + for the extracted contents. Returns the path to the extracted contents. In case of files (gzip and bzip2), the path to the decompressed file is returned; for archives, the -- cgit v1.2.3 From 68be9d8acc7d6c00b212d42b27c2575dab826062 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Fri, 14 Jul 2017 16:59:52 +0100 Subject: utils/android: Added native code extraction of apks from WA2 --- devlib/utils/android.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index e8ca78d..22aab95 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -27,7 +27,7 @@ import re from collections import defaultdict from devlib.exception import TargetError, HostError, DevlibError -from devlib.utils.misc import check_output, which, memoized +from devlib.utils.misc import check_output, which, memoized, ABI_MAP from devlib.utils.misc import escape_single_quotes, escape_double_quotes @@ -124,6 +124,7 @@ class ApkInfo(object): self.label = None self.version_name = None self.version_code = None + self.native_code = None self.parse(path) def parse(self, apk_path): @@ -143,6 +144,19 @@ class ApkInfo(object): elif line.startswith('launchable-activity:'): match = self.name_regex.search(line) self.activity = match.group('name') + elif line.startswith('native-code'): + apk_abis = [entry.strip() for entry in line.split(':')[1].split("'") if entry.strip()] + mapped_abis = [] + for apk_abi in apk_abis: + found = False + for abi, architectures in ABI_MAP.iteritems(): + if apk_abi in architectures: + mapped_abis.append(abi) + found = True + break + if not found: + mapped_abis.append(apk_abi) + self.native_code = mapped_abis else: pass # not interested -- cgit v1.2.3 From 98fb2e2306587ebd4bcfc3eed03f304525a212f5 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Fri, 14 Jul 2017 17:41:16 +0100 Subject: Target: Ported supported_abi property from WA2 --- devlib/target.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/devlib/target.py b/devlib/target.py index a920655..27b1c4b 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -101,6 +101,10 @@ class Target(object): def abi(self): # pylint: disable=no-self-use return None + @property + def supported_abi(self): + return [self.abi] + @property @memoized def cpuinfo(self): @@ -828,6 +832,30 @@ class AndroidTarget(Target): def abi(self): return self.getprop()['ro.product.cpu.abi'].split('-')[0] + @property + @memoized + def supported_abi(self): + props = self.getprop() + result = [props['ro.product.cpu.abi']] + if 'ro.product.cpu.abi2' in props: + result.append(props['ro.product.cpu.abi2']) + if 'ro.product.cpu.abilist' in props: + for abi in props['ro.product.cpu.abilist'].split(','): + if abi not in result: + result.append(abi) + + mapped_result = [] + for supported_abi in result: + for abi, architectures in ABI_MAP.iteritems(): + found = False + if supported_abi in architectures and abi not in mapped_result: + mapped_result.append(abi) + found = True + break + if not found and supported_abi not in mapped_result: + mapped_result.append(supported_abi) + return mapped_result + @property @memoized def os_version(self): -- cgit v1.2.3 From 02c93b48abaa5e321cbdbfbe78bb1d37481eacbd Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Mon, 17 Jul 2017 15:18:39 +0100 Subject: ABI_MAP: Adds 'armeabi-v7a' to mapping --- devlib/utils/misc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/utils/misc.py b/devlib/utils/misc.py index d6a5093..f601686 100644 --- a/devlib/utils/misc.py +++ b/devlib/utils/misc.py @@ -41,7 +41,7 @@ from devlib.exception import HostError, TimeoutError # ABI --> architectures list ABI_MAP = { - 'armeabi': ['armeabi', 'armv7', 'armv7l', 'armv7el', 'armv7lh'], + 'armeabi': ['armeabi', 'armv7', 'armv7l', 'armv7el', 'armv7lh', 'armeabi-v7a'], 'arm64': ['arm64', 'armv8', 'arm64-v8a', 'aarch64'], } -- cgit v1.2.3 From 7919a5643c9e2178a96e8a7292c953ad16081dc8 Mon Sep 17 00:00:00 2001 From: Anthony Barbier Date: Tue, 25 Jul 2017 13:48:33 +0100 Subject: Add support for arbitrary ADB servers --- devlib/utils/android.py | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index f683190..d0aa652 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -163,7 +163,7 @@ class AdbConnection(object): @memoized def newline_separator(self): output = adb_command(self.device, - "shell '({}); echo \"\n$?\"'".format(self.ls_command)) + "shell '({}); echo \"\n$?\"'".format(self.ls_command), adb_server=self.adb_server) if output.endswith('\r\n'): return '\r\n' elif output.endswith('\n'): @@ -178,7 +178,7 @@ class AdbConnection(object): def _setup_ls(self): command = "shell '(ls -1); echo \"\n$?\"'" try: - output = adb_command(self.device, command, timeout=self.timeout) + output = adb_command(self.device, command, timeout=self.timeout, adb_server=self.adb_server) except subprocess.CalledProcessError as e: raise HostError( 'Failed to set up ls command on Android device. Output:\n' @@ -191,11 +191,12 @@ class AdbConnection(object): self.ls_command = 'ls' logger.debug("ls command is set to {}".format(self.ls_command)) - def __init__(self, device=None, timeout=None, platform=None): + 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: - device = adb_get_device(timeout=timeout) + device = adb_get_device(timeout=timeout, adb_server=adb_server) self.device = device + self.adb_server = adb_server adb_connect(self.device) AdbConnection.active_connections[self.device] += 1 self._setup_ls() @@ -206,7 +207,7 @@ class AdbConnection(object): command = "push '{}' '{}'".format(source, dest) if not os.path.exists(source): raise HostError('No such file "{}"'.format(source)) - return adb_command(self.device, command, timeout=timeout) + return adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server) def pull(self, source, dest, timeout=None): if timeout is None: @@ -215,18 +216,18 @@ class AdbConnection(object): if os.path.isdir(dest) and \ ('*' in source or '?' in source): command = 'shell {} {}'.format(self.ls_command, source) - output = adb_command(self.device, command, timeout=timeout) + output = adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server) for line in output.splitlines(): command = "pull '{}' '{}'".format(line.strip(), dest) - adb_command(self.device, command, timeout=timeout) + adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server) return command = "pull '{}' '{}'".format(source, dest) - return adb_command(self.device, command, timeout=timeout) + return adb_command(self.device, command, timeout=timeout, adb_server=self.adb_server) 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, self.newline_separator) + as_root, self.newline_separator,adb_server=self.adb_server) def background(self, command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, as_root=False): return adb_background_shell(self.device, command, stdout, stderr, as_root) @@ -258,7 +259,7 @@ def fastboot_flash_partition(partition, path_to_image): fastboot_command(command) -def adb_get_device(timeout=None): +def adb_get_device(timeout=None, adb_server=None): """ Returns the serial number of a connected android device. @@ -273,7 +274,7 @@ def adb_get_device(timeout=None): # then the output length is 2 + (1 for each device) start = time.time() while True: - output = adb_command(None, "devices").splitlines() # pylint: disable=E1103 + output = adb_command(None, "devices", adb_server=adb_server).splitlines() # pylint: disable=E1103 output_length = len(output) if output_length == 3: # output[1] is the 2nd line in the output which has the device name @@ -339,11 +340,14 @@ def _ping(device): def adb_shell(device, command, timeout=None, check_exit_code=False, - as_root=False, newline_separator='\r\n'): # NOQA + as_root=False, newline_separator='\r\n', adb_server=None): # NOQA _check_env() if as_root: command = 'echo \'{}\' | su'.format(escape_single_quotes(command)) - device_part = ['-s', device] if device else [] + device_part = [] + if adb_server: + device_part = ['-H', adb_server] + device_part += ['-s', device] if device else [] # On older combinations of ADB/Android versions, the adb host command always # exits with 0 if it was able to run the command on the target, even if the @@ -401,8 +405,8 @@ def adb_background_shell(device, command, return subprocess.Popen(full_command, stdout=stdout, stderr=stderr, shell=True) -def adb_list_devices(): - output = adb_command(None, 'devices') +def adb_list_devices(adb_server=None): + output = adb_command(None, 'devices',adb_server=adb_server) devices = [] for line in output.splitlines(): parts = [p.strip() for p in line.split()] @@ -411,9 +415,12 @@ def adb_list_devices(): return devices -def adb_command(device, command, timeout=None): +def adb_command(device, command, timeout=None,adb_server=None): _check_env() - device_string = ' -s {}'.format(device) if device else '' + device_string = "" + if adb_server != None: + device_string = ' -H {}'.format(adb_server) + device_string += ' -s {}'.format(device) if device else '' full_command = "adb{} {}".format(device_string, command) logger.debug(full_command) output, _ = check_output(full_command, timeout, shell=True) -- cgit v1.2.3 From b54dc19b81d7586110a5ff7b0c4a5e04bc9f2bc6 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Wed, 26 Jul 2017 11:39:49 +0100 Subject: devlib/module/thermal: Fix thermal zone disabling Calling thermal.disable_all_zones() would raise an exception. I've changed a few things: * use self.zones.itervalues() in disable_all_zones to fix that exception * renamed zone.set_mode() to zone.set_enabled() --- devlib/module/thermal.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/devlib/module/thermal.py b/devlib/module/thermal.py index 4fa8e15..fa13fbb 100644 --- a/devlib/module/thermal.py +++ b/devlib/module/thermal.py @@ -61,8 +61,8 @@ class ThermalZone(object): value = self.target.read_value(self.target.path.join(self.path, 'mode')) return value == 'enabled' - def set_mode(self, enable): - value = 'enabled' if enable else 'disabled' + def set_enabled(self, enabled=True): + value = 'enabled' if enabled else 'disabled' self.target.write_value(self.target.path.join(self.path, 'mode'), value) def get_temperature(self): @@ -100,5 +100,5 @@ class ThermalModule(Module): def disable_all_zones(self): """Disables all the thermal zones in the target""" - for zone in self.zones: - zone.set_mode('disabled') + for zone in self.zones.itervalues(): + zone.set_enabled(False) -- cgit v1.2.3 From b392a0a1b48e65a703df26ceb49f9f48795e125b Mon Sep 17 00:00:00 2001 From: Anthony Barbier Date: Wed, 26 Jul 2017 14:00:38 +0100 Subject: Use related_cpus instead of affected_cpus related_cpus doesn't rely on the related cores to be online to work cf https://www.kernel.org/doc/Documentation/cpu-freq/user-guide.txt --- devlib/module/cpufreq.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/module/cpufreq.py b/devlib/module/cpufreq.py index e18d95b..46873af 100644 --- a/devlib/module/cpufreq.py +++ b/devlib/module/cpufreq.py @@ -420,7 +420,7 @@ class CpufreqModule(Module): if isinstance(cpu, int): cpu = 'cpu{}'.format(cpu) - sysfile = '/sys/devices/system/cpu/{}/cpufreq/affected_cpus'.format(cpu) + sysfile = '/sys/devices/system/cpu/{}/cpufreq/related_cpus'.format(cpu) return [int(c) for c in self.target.read_value(sysfile).split()] -- cgit v1.2.3 From 36aa3af66d21f7fdd4fad3f3709622dfbf992645 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 26 Jul 2017 13:59:55 +0100 Subject: module/cpufreq: fix domain cpus. - Use related_cpus rather than affected_cpus inside get_domain_cpus to return all cpus in the domain (including those online). This changes the behavior, but old behavior was almost certainly wrong as the method is memoized, and so the result should not be affected by hotplug. - Add a non-memoized get_affected_cpus() which implements the old behavior. --- devlib/module/cpufreq.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/devlib/module/cpufreq.py b/devlib/module/cpufreq.py index e18d95b..c4bc14f 100644 --- a/devlib/module/cpufreq.py +++ b/devlib/module/cpufreq.py @@ -412,6 +412,17 @@ class CpufreqModule(Module): """ return self.target._execute_util('cpufreq_trace_all_frequencies', as_root=True) + def get_affected_cpus(self, cpu): + """ + Get the CPUs that share a frequency domain with the given CPU + """ + if isinstance(cpu, int): + cpu = 'cpu{}'.format(cpu) + + sysfile = '/sys/devices/system/cpu/{}/cpufreq/affected_cpus'.format(cpu) + + return [int(c) for c in self.target.read_value(sysfile).split()] + @memoized def get_domain_cpus(self, cpu): """ @@ -420,7 +431,7 @@ class CpufreqModule(Module): if isinstance(cpu, int): cpu = 'cpu{}'.format(cpu) - sysfile = '/sys/devices/system/cpu/{}/cpufreq/affected_cpus'.format(cpu) + sysfile = '/sys/devices/system/cpu/{}/cpufreq/related_cpus'.format(cpu) return [int(c) for c in self.target.read_value(sysfile).split()] -- cgit v1.2.3 From e206e9b24a416d5f0965df0d0e7db53214a7869a Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 26 Jul 2017 14:20:58 +0100 Subject: module/cpufreq: fix get_affected_cpus docstring The docstring was duplicated from get_domain_cpus. This updates the docstring to reflect the difference from get_domain_cpus. --- devlib/module/cpufreq.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/module/cpufreq.py b/devlib/module/cpufreq.py index c4bc14f..2addfed 100644 --- a/devlib/module/cpufreq.py +++ b/devlib/module/cpufreq.py @@ -414,7 +414,7 @@ class CpufreqModule(Module): def get_affected_cpus(self, cpu): """ - Get the CPUs that share a frequency domain with the given CPU + Get the online CPUs that share a frequency domain with the given CPU """ if isinstance(cpu, int): cpu = 'cpu{}'.format(cpu) -- cgit v1.2.3 From 63e60401d5780217df64f75f3f6a1f8e7c0719b9 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 26 Jul 2017 14:21:55 +0100 Subject: module/cpufreq: rename get_domain_cpus Rename get_domain_cpus to get_related_cpus to match cpufreq nomenclature. --- devlib/module/cpufreq.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devlib/module/cpufreq.py b/devlib/module/cpufreq.py index 2addfed..a8028f3 100644 --- a/devlib/module/cpufreq.py +++ b/devlib/module/cpufreq.py @@ -424,7 +424,7 @@ class CpufreqModule(Module): return [int(c) for c in self.target.read_value(sysfile).split()] @memoized - def get_domain_cpus(self, cpu): + def get_related_cpus(self, cpu): """ Get the CPUs that share a frequency domain with the given CPU """ @@ -442,6 +442,6 @@ class CpufreqModule(Module): cpus = set(range(self.target.number_of_cpus)) while cpus: cpu = iter(cpus).next() - domain = self.target.cpufreq.get_domain_cpus(cpu) + domain = self.target.cpufreq.get_related_cpus(cpu) yield domain cpus = cpus.difference(domain) -- cgit v1.2.3 From d25beb5c8b0d8289de5ab52c1974e63c987f58ed Mon Sep 17 00:00:00 2001 From: Ionela Voinescu Date: Thu, 27 Jul 2017 11:07:05 +0100 Subject: gem5stats: fix missing import for TargetError Signed-off-by: Ionela Voinescu --- devlib/module/gem5stats.py | 1 + 1 file changed, 1 insertion(+) diff --git a/devlib/module/gem5stats.py b/devlib/module/gem5stats.py index ba24a43..f919905 100644 --- a/devlib/module/gem5stats.py +++ b/devlib/module/gem5stats.py @@ -17,6 +17,7 @@ import os.path from collections import defaultdict import devlib +from devlib.exception import TargetError from devlib.module import Module from devlib.platform import Platform from devlib.platform.gem5 import Gem5SimulationPlatform -- cgit v1.2.3 From 70d755d75b6939caa799bfa30142304b2d83ae60 Mon Sep 17 00:00:00 2001 From: Basil Eljuse Date: Fri, 28 Jul 2017 12:23:06 +0100 Subject: Mark inline function to be static to avoid link issues with GCC compiler --- src/readenergy/readenergy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/readenergy/readenergy.c b/src/readenergy/readenergy.c index 6e4f35f..890a454 100644 --- a/src/readenergy/readenergy.c +++ b/src/readenergy/readenergy.c @@ -114,7 +114,7 @@ struct reading double sys_enm_ch0_gpu; }; -inline uint64_t join_64bit_register(uint32_t *buffer, int index) +static inline uint64_t join_64bit_register(uint32_t *buffer, int index) { uint64_t result = 0; result |= buffer[index]; -- cgit v1.2.3 From 93b39a7f47a64217724260881cccc8bc7f60cf74 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 9 Aug 2017 15:59:24 +0100 Subject: ssh: Fix redacting SSH password in errors IIUC this is supposed to redact the password from exception messages, but 'pass_string' is not set to the password so nothing is altered. Fix that. --- devlib/utils/ssh.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/devlib/utils/ssh.py b/devlib/utils/ssh.py index 89613bd..5470367 100644 --- a/devlib/utils/ssh.py +++ b/devlib/utils/ssh.py @@ -284,16 +284,17 @@ class SshConnection(object): 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) - pass_string = '' + command_redacted = command logger.debug(command) if self.password: command = _give_password(self.password, command) + command_redacted = command.replace(self.password, '') try: check_output(command, timeout=timeout, shell=True) except subprocess.CalledProcessError as e: - raise subprocess.CalledProcessError(e.returncode, e.cmd.replace(pass_string, ''), e.output) + raise subprocess.CalledProcessError(e.returncode, command_redacted e.output) except TimeoutError as e: - raise TimeoutError(e.command.replace(pass_string, ''), e.output) + raise TimeoutError(command_redacted, e.output) class TelnetConnection(SshConnection): -- cgit v1.2.3 From 380ad0515d2911e2d08a6f60edfa92568a7d71f2 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 9 Aug 2017 16:00:46 +0100 Subject: ssh: Improve error when failing to push file CalledProcessError doesn't include the output of the failed command in the message it prints, so use a HostError isntead. --- devlib/utils/ssh.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/devlib/utils/ssh.py b/devlib/utils/ssh.py index 5470367..aaab2dd 100644 --- a/devlib/utils/ssh.py +++ b/devlib/utils/ssh.py @@ -292,7 +292,8 @@ class SshConnection(object): try: check_output(command, timeout=timeout, shell=True) except subprocess.CalledProcessError as e: - raise subprocess.CalledProcessError(e.returncode, command_redacted e.output) + raise HostError("Failed to copy file with '{}'. Output:\n{}".format( + command_redacted, e.output)) except TimeoutError as e: raise TimeoutError(command_redacted, e.output) -- cgit v1.2.3 From ddd2e29b87fa5daa12d88b9c6f7ea3b6e7c8ca9c Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Thu, 6 Jul 2017 17:30:12 +0100 Subject: AndroidTarget: Adds methods to get/set screen brightness --- devlib/target.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/devlib/target.py b/devlib/target.py index 27b1c4b..af49989 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1186,6 +1186,26 @@ class AndroidTarget(Target): if self.is_screen_on(): self.execute('input keyevent 26') + def set_auto_brightness(self, auto_brightness): + cmd = 'settings put system screen_brightness_mode {}' + self.execute(cmd.format(int(boolean(auto_brightness)))) + + def get_auto_brightness(self): + cmd = 'settings get system screen_brightness_mode' + return boolean(self.execute(cmd).strip()) + + def set_brightness(self, value): + if not 0 <= value <= 255: + msg = 'Invalid brightness "{}"; Must be between 0 and 255' + raise ValueError(msg.format(value)) + self.set_auto_brightness(False) + cmd = 'settings put system screen_brightness {}' + self.execute(cmd.format(int(value))) + + def get_brightness(self): + cmd = 'settings get system screen_brightness' + return integer(self.execute(cmd).strip()) + def homescreen(self): self.execute('am start -a android.intent.action.MAIN -c android.intent.category.HOME') -- cgit v1.2.3 From 3e751746d6c07a1643cdfb7f383dbc4a17765d88 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Thu, 6 Jul 2017 17:31:02 +0100 Subject: AndroidTarget: Adds methods to get/set airplane mode In order to change the state of airplane mode, the setting needs to be configured before broadcasting an intent to update the system. As of Android N, root is required to send the broadcast, therefore this method will raise an error if the requirements are not satisfied. --- devlib/target.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/devlib/target.py b/devlib/target.py index af49989..925f770 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1206,6 +1206,18 @@ class AndroidTarget(Target): cmd = 'settings get system screen_brightness' return integer(self.execute(cmd).strip()) + def get_airplane_mode(self): + cmd = 'settings get global airplane_mode_on' + return boolean(self.execute(cmd).strip()) + + def set_airplane_mode(self, mode): + root_required = self.get_sdk_version() > 23 + if root_required and not self.is_rooted: + raise TargetError('Root is required to toggle airplane mode on Android 7+') + cmd = 'settings put global airplane_mode_on {}' + self.execute(cmd.format(int(boolean(mode)))) + self.execute('am broadcast -a android.intent.action.AIRPLANE_MODE', as_root=root_required) + def homescreen(self): self.execute('am start -a android.intent.action.MAIN -c android.intent.category.HOME') -- cgit v1.2.3 From 003785dde127c4d60a1e4ce4f558226c91298817 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Mon, 24 Jul 2017 17:44:33 +0100 Subject: AndroidTarget: Adds methods to get/set screen rotation --- devlib/target.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/devlib/target.py b/devlib/target.py index 925f770..c8b15d8 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1218,6 +1218,37 @@ class AndroidTarget(Target): self.execute(cmd.format(int(boolean(mode)))) self.execute('am broadcast -a android.intent.action.AIRPLANE_MODE', as_root=root_required) + def get_auto_rotation(self): + cmd = 'settings get system accelerometer_rotation' + return boolean(self.execute(cmd).strip()) + + def set_auto_rotation(self, autorotate): + cmd = 'settings put system accelerometer_rotation {}' + self.execute(cmd.format(int(boolean(autorotate)))) + + def set_natural_rotation(self): + self.set_rotation(0) + + def set_left_rotation(self): + self.set_rotation(1) + + def set_inverted_rotation(self): + self.set_rotation(2) + + def set_right_rotation(self): + self.set_rotation(3) + + def get_rotation(self): + cmd = 'settings get system user_rotation' + return self.execute(cmd).strip() + + def set_rotation(self, rotation): + if not 0 <= rotation <= 3: + raise ValueError('Rotation value must be between 0 and 3') + self.set_auto_rotation(False) + cmd = 'settings put system user_rotation {}' + self.execute(cmd.format(rotation)) + def homescreen(self): self.execute('am start -a android.intent.action.MAIN -c android.intent.category.HOME') -- cgit v1.2.3 From 1229af089509d099fcb71009c90b87969814a70a Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Wed, 26 Jul 2017 11:29:10 +0100 Subject: AndroidTarget: Fixes 'swipe_to_unlock' method. --- devlib/target.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index c8b15d8..4892951 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1065,16 +1065,16 @@ class AndroidTarget(Target): width, height = self.screen_resolution command = 'input swipe {} {} {} {}' if direction == "horizontal": - swipe_heigh = height * 2 // 3 + swipe_height = height * 2 // 3 start = 100 stop = width - start - self.execute(command.format(start, swipe_heigh, stop, swipe_heigh)) - if direction == "vertical": - swipe_middle = height / 2 - swipe_heigh = height * 2 // 3 - self.execute(command.format(swipe_middle, swipe_heigh, swipe_middle, 0)) + self.execute(command.format(start, swipe_height, stop, swipe_height)) + elif direction == "vertical": + swipe_middle = width / 2 + swipe_height = height * 2 // 3 + self.execute(command.format(swipe_middle, swipe_height, swipe_middle, 0)) else: - raise DeviceError("Invalid swipe direction: {}".format(self.swipe_to_unlock)) + raise TargetError("Invalid swipe direction: {}".format(direction)) def getprop(self, prop=None): props = AndroidProperties(self.execute('getprop')) -- cgit v1.2.3 From 8839ed01bad79fbd110bcd7d47c620c211627d60 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Thu, 10 Aug 2017 17:19:13 +0100 Subject: AndroidTarget: Changes default unlocking method Instead of using a default 'horizontal' unlocking method, now will try a diagonal swipe up as this should work most modern devices with vertical or horizontal unlocking methods. --- devlib/target.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index 4892951..b96bbdc 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1061,10 +1061,15 @@ class AndroidTarget(Target): # Android-specific - def swipe_to_unlock(self, direction="horizontal"): + def swipe_to_unlock(self, direction="diagonal"): width, height = self.screen_resolution command = 'input swipe {} {} {} {}' - if direction == "horizontal": + if direction == "diagonal": + start = 100 + stop = width - start + swipe_height = height * 2 // 3 + self.execute(command.format(start, swipe_height, stop, 0)) + elif direction == "horizontal": swipe_height = height * 2 // 3 start = 100 stop = width - start -- cgit v1.2.3 From 38258eb74cb01fe16022a08403a6399331e26f16 Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Fri, 21 Jul 2017 16:13:31 +0100 Subject: module: gem5stats: enable asynchronous origins in gem5 stats file Currently, reset_origin() in the gem5stats module enables to virtually truncate the existing dumps from the gem5 statistics file so that any subsequent match() calls will apply only on new dumps. However, the current implementation resets the origin of the stats file unilaterally, without other clients knowing about it, hence leading to data being lost. This commits removes the reset_origin() method and provides a different API to handle virtual truncatures in the stats file. Namely, the match() method now takes a base_dump parameter letting clients specifiying from which dump they want match() to apply. This feature could also be usefull if someone wanted to add off-line processing features for statistics files. --- devlib/instrument/gem5power.py | 6 ++-- devlib/module/gem5stats.py | 66 ++++++++++++++++++++++++++++++------------ 2 files changed, 51 insertions(+), 21 deletions(-) diff --git a/devlib/instrument/gem5power.py b/devlib/instrument/gem5power.py index 6411b3f..7715ec1 100644 --- a/devlib/instrument/gem5power.py +++ b/devlib/instrument/gem5power.py @@ -52,6 +52,7 @@ class Gem5PowerInstrument(Instrument): self.target.gem5stats.book_roi(self.roi_label) self.sample_period_ns = 10000000 self.target.gem5stats.start_periodic_dump(0, self.sample_period_ns) + self._base_stats_dump = 0 def start(self): self.target.gem5stats.roi_start(self.roi_label) @@ -64,11 +65,12 @@ class Gem5PowerInstrument(Instrument): with open(outfile, 'wb') as wfh: writer = csv.writer(wfh) writer.writerow([c.label for c in self.active_channels]) # headers - for rec, rois in self.target.gem5stats.match_iter(active_sites, [self.roi_label]): + for rec, rois in self.target.gem5stats.match_iter(active_sites, + [self.roi_label], self._base_stats_dump): writer.writerow([float(rec[s]) for s in active_sites]) return MeasurementsCsv(outfile, self.active_channels) def reset(self, sites=None, kinds=None, channels=None): super(Gem5PowerInstrument, self).reset(sites, kinds, channels) - self.target.gem5stats.reset_origin() + self._base_stats_dump = self.target.gem5stats.next_dump_no() diff --git a/devlib/module/gem5stats.py b/devlib/module/gem5stats.py index f919905..9109751 100644 --- a/devlib/module/gem5stats.py +++ b/devlib/module/gem5stats.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import sys import logging import os.path from collections import defaultdict @@ -66,6 +67,7 @@ class Gem5StatsModule(Module): self._stats_file_path = os.path.join(target.platform.gem5_out_dir, 'stats.txt') self.rois = {} + self._dump_pos_cache = {0: 0} def book_roi(self, label): if label in self.rois: @@ -103,27 +105,27 @@ class Gem5StatsModule(Module): raise ValueError(msg.format(delay_ns, period_ns)) self.target.execute('m5 dumpresetstats {} {}'.format(delay_ns, period_ns)) - def match(self, keys, rois_labels): + def match(self, keys, rois_labels, base_dump=0): ''' Tries to match the list of keys passed as parameter over the statistics - dumps covered by selected ROIs since origin. 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. + dumps covered by selected ROIs since ``base_dump``. 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. Keys must match fields in gem5's statistics log file. Key example: system.cluster0.cores0.power_model.static_power ''' records = defaultdict(lambda : defaultdict(list)) - for record, active_rois in self.match_iter(keys, rois_labels): + for record, active_rois in self.match_iter(keys, rois_labels, base_dump): for key in record: for roi_label in active_rois: records[key][roi_label].append(record[key]) return records - def match_iter(self, keys, rois_labels): + def match_iter(self, keys, rois_labels, base_dump=0): ''' - Yields for each dump since origin a pair containing: + Yields for each dump since ``base_dump`` a pair containing: 1. a dict storing the values corresponding to each of the specified keys 2. the list of currently active ROIs among those passed as parameters. @@ -142,24 +144,50 @@ class Gem5StatsModule(Module): return (roi.field in dump) and (int(dump[roi.field]) == 1) with open(self._stats_file_path, 'r') as stats_file: - stats_file.seek(self._current_origin) + self._goto_dump(stats_file, base_dump) for dump in iter_statistics_dump(stats_file): active_rois = [l for l in rois_labels if roi_active(l, dump)] if active_rois: record = {k: dump[k] for k in keys} yield (record, active_rois) - - def reset_origin(self): + def next_dump_no(self): ''' - Place origin right after the last full dump in the file + 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 + 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. ''' - last_dump_tail = self._current_origin - # Dump & reset stats to start from a fresh state - self.target.execute('m5 dumpresetstats') with open(self._stats_file_path, 'r') as stats_file: - for line in stats_file: - if GEM5STATS_DUMP_TAIL in line: - last_dump_tail = stats_file.tell() - self._current_origin = last_dump_tail + # _goto_dump reach EOF and returns the total number of dumps + 1 + return self._goto_dump(stats_file, sys.maxint) + + def _goto_dump(self, stats_file, target_dump): + if target_dump < 0: + raise HostError('Cannot go to dump {}'.format(target_dump)) + + # Go to required dump quickly if it was visited before + if target_dump in self._dump_pos_cache: + stats_file.seek(self._dump_pos_cache[target_dump]) + return target_dump + # Or start from the closest dump already visited before the required one + prev_dumps = filter(lambda x: x < target_dump, self._dump_pos_cache.keys()) + 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 = dump_iterator.next() + except StopIteration: + break + # End of passed dump is beginning og next one + curr_pos = stats_file.tell() + curr_dump += 1 + self._dump_pos_cache[curr_dump] = curr_pos + return curr_dump -- cgit v1.2.3 From 3d10e3eae9bad78b6602e68364536362d178a239 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 16 Aug 2017 15:58:10 +0100 Subject: utils/types: numeric expeanded to handle percentages numeric() conversion function is expanded to handle percentage values in the form "10.123%". The result is the numeric part converted to a float and divided by 100, e.g. 0.10123. --- devlib/utils/types.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/devlib/utils/types.py b/devlib/utils/types.py index be30bfc..645328d 100644 --- a/devlib/utils/types.py +++ b/devlib/utils/types.py @@ -68,6 +68,15 @@ def numeric(value): """ if isinstance(value, int): return value + + if isinstance(value, basestring): + value = value.strip() + if value.endswith('%'): + try: + return float(value.rstrip('%')) / 100 + except ValueError: + raise ValueError('Not numeric: {}'.format(value)) + try: fvalue = float(value) except ValueError: -- cgit v1.2.3 From 5b99c1613b1f040d1f2b8fdeb3fe0fd680a05b81 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Wed, 9 Aug 2017 10:56:01 +0100 Subject: Documentation: Adds documentation for Android Target Adds documentation for the `AndroidTarget` class including the new android methods. --- doc/target.rst | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 124 insertions(+), 14 deletions(-) diff --git a/doc/target.rst b/doc/target.rst index b64246a..08472e2 100644 --- a/doc/target.rst +++ b/doc/target.rst @@ -2,18 +2,18 @@ Target ====== -.. class:: Target(connection_settings=None, platform=None, working_directory=None, executables_directory=None, connect=True, modules=None, load_default_modules=True, shell_prompt=DEFAULT_SHELL_PROMPT) - +.. class:: Target(connection_settings=None, platform=None, working_directory=None, executables_directory=None, connect=True, modules=None, load_default_modules=True, shell_prompt=DEFAULT_SHELL_PROMPT, conn_cls=None) + :class:`Target` is the primary interface to the remote device. All interactions with the device are performed via a :class:`Target` instance, either directly, or via its modules or a wrapper interface (such as an :class:`Instrument`). - :param connection_settings: A ``dict`` that specifies how to connect to the remote + :param connection_settings: A ``dict`` that specifies how to connect to the remote device. Its contents depend on the specific :class:`Target` type (used see :ref:`connection-types`\ ). - :param platform: A :class:`Target` defines interactions at Operating System level. A + :param platform: A :class:`Target` defines interactions at Operating System level. A :class:`Platform` describes the underlying hardware (such as CPUs available). If a :class:`Platform` instance is not specified on :class:`Target` creation, one will be created automatically and it will @@ -22,8 +22,8 @@ Target :param working_directory: This is primary location for on-target file system interactions performed by ``devlib``. This location *must* be readable and - writable directly (i.e. without sudo) by the connection's user account. - It may or may not allow execution. This location will be created, + writable directly (i.e. without sudo) by the connection's user account. + It may or may not allow execution. This location will be created, if necessary, during ``setup()``. If not explicitly specified, this will be set to a default value @@ -35,7 +35,7 @@ Target (obviously). It should also be possible to write to this location, possibly with elevated privileges (i.e. on a rooted Linux target, it should be possible to write here with sudo, but not necessarily directly - by the connection's account). This location will be created, + by the connection's account). This location will be created, if necessary, during ``setup()``. This location does *not* need to be same as the system's executables @@ -52,7 +52,7 @@ Target :param modules: a list of additional modules to be installed. Some modules will try to install by default (if supported by the underlying target). - Current default modules are ``hotplug``, ``cpufreq``, ``cpuidle``, + Current default modules are ``hotplug``, ``cpufreq``, ``cpuidle``, ``cgroups``, and ``hwmon`` (See :ref:`modules`\ ). See modules documentation for more detail. @@ -68,6 +68,9 @@ Target prompted on the target. This may be used by some modules that establish auxiliary connections to a target over UART. + :param conn_cls: This is the type of connection that will be used to communicate + with the device. + .. attribute:: Target.core_names This is a list containing names of CPU cores on the target, in the order in @@ -94,7 +97,7 @@ Target .. attribute:: Target.is_connected A boolean value that indicates whether an active connection exists to the - target device. + target device. .. attribute:: Target.connected_as_root @@ -146,7 +149,7 @@ Target thread. .. method:: Target.connect([timeout]) - + Establish a connection to the target. It is usually not necessary to call this explicitly, as a connection gets automatically established on instantiation. @@ -225,7 +228,7 @@ Target :param timeout: Timeout (in seconds) for the execution of the command. If specified, an exception will be raised if execution does not complete with the specified period. - :param check_exit_code: If ``True`` (the default) the exit code (on target) + :param check_exit_code: If ``True`` (the default) the exit code (on target) from execution of the command will be checked, and an exception will be raised if it is not ``0``. :param as_root: The command will be executed as root. This will fail on @@ -262,7 +265,7 @@ Target will be interpreted as a comma-separated list of cpu ranges, e.g. ``"0,4-7"``. :param as_root: Specify whether the command should be run as root - :param timeout: If this is specified and invocation does not terminate within this number + :param timeout: If this is specified and invocation does not terminate within this number of seconds, an exception will be raised. .. method:: Target.background_invoke(binary [, args [, in_directory [, on_cpus [, as_root ]]]]) @@ -314,13 +317,13 @@ Target .. method:: Target.write_value(path, value [, verify]) - Write the value to the specified path on the target. This is primarily + Write the value to the specified path on the target. This is primarily intended for sysfs/procfs/debugfs etc. :param path: file to write into :param value: value to be written :param verify: If ``True`` (the default) the value will be read back after - it is written to make sure it has been written successfully. This due to + it is written to make sure it has been written successfully. This due to some sysfs entries silently failing to set the written value without returning an error code. @@ -450,3 +453,110 @@ Target Returns the path to the extracted contents. In case of files (gzip and bzip2), the path to the decompressed file is returned; for archives, the path to the directory with the archive's contents is returned. + + +Android Target +--------------- + +.. class:: AndroidTarget(connection_settings=None, platform=None, working_directory=None, executables_directory=None, connect=True, modules=None, load_default_modules=True, shell_prompt=DEFAULT_SHELL_PROMPT, conn_cls=AdbConnection, package_data_directory="/data/data") + + :class:`AndroidTarget` is a subclass of :class:`Target` with additional features specific to a device running Android. + + :param package_data_directory: This is the location of the data stored + for installed Android packages on the device. + +.. method:: AndroidTarget.set_rotation(rotation) + + Specify an integer representing the desired screen rotation with the + following mappings: Natural: ``0``, Rotated Left: ``1``, Inverted : ``2`` + and Rotated Right : ``3``. + +.. method:: AndroidTarget.get_rotation(rotation) + + Returns an integer value representing the orientation of the devices + screen. ``0`` : Natural, ``1`` : Rotated Left, ``2`` : Inverted + and ``3`` : Rotated Right. + +.. method:: AndroidTarget.set_natural_rotation() + + Sets the screen orientation of the device to its natural (0 degrees) + orientation. + +.. method:: AndroidTarget.set_left_rotation() + + Sets the screen orientation of the device to 90 degrees. + +.. method:: AndroidTarget.set_inverted_rotation() + + Sets the screen orientation of the device to its inverted (180 degrees) + orientation. + +.. method:: AndroidTarget.set_right_rotation() + + Sets the screen orientation of the device to 270 degrees. + +.. method:: AndroidTarget.set_auto_rotation(autorotate) + + Specify a boolean value for whether the devices auto-rotation should + be enabled. + +.. method:: AndroidTarget.get_auto_rotation() + + Returns ``True`` if the targets auto rotation is currently enabled and + ``False`` otherwise. + +.. method:: AndroidTarget.set_airplane_mode(mode) + + Specify a boolean value for whether the device should be in airplane mode. + + .. note:: Requires the device to be rooted if the device is running Android 7+. + +.. method:: AndroidTarget.get_airplane_mode() + + Returns ``True`` if the target is currently in airplane mode and + ``False`` otherwise. + +.. method:: AndroidTarget.set_brightness(value) + + Sets the devices screen brightness to a specified integer between ``0`` and + ``255``. + +.. method:: AndroidTarget.get_brightness() + + Returns an integer between ``0`` and ``255`` representing the devices + current screen brightness. + +.. method:: AndroidTarget.set_auto_brightness(auto_brightness) + + Specify a boolean value for whether the devices auto brightness + should be enabled. + +.. method:: AndroidTarget.get_auto_brightness() + + Returns ``True`` if the targets auto brightness is currently + enabled and ``False`` otherwise. + +.. method:: AndroidTarget.ensure_screen_is_off() + + Checks if the devices screen is on and if so turns it off. + +.. method:: AndroidTarget.ensure_screen_is_on() + + Checks if the devices screen is off and if so turns it on. + +.. method:: AndroidTarget.is_screen_on() + + Returns ``True`` if the targets screen is currently on and ``False`` + otherwise. + +.. method:: AndroidTarget.homescreen() + + Returns the device to its home screen. + +.. method:: AndroidTarget.swipe_to_unlock(direction="diagonal") + + Performs a swipe input on the device to try and unlock the device. + A direction of ``"horizontal"``, ``"vertical"`` or ``"diagonal"`` + can be supplied to specify in which direction the swipe should be + performed. By default ``"diagonal"`` will be used to try and + support the majority of newer devices. -- cgit v1.2.3 From 2de2b36387ed55ec620767c6fd1583a4baa43d50 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Tue, 25 Jul 2017 16:19:08 +0100 Subject: Instrumentation: Fix conversion between microseconds and seconds --- devlib/instrument/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 044c7d4..dd5b1f5 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -75,12 +75,12 @@ _measurement_types = [ MeasurementType('unknown', None), MeasurementType('time', 'seconds', conversions={ - 'time_us': lambda x: x * 1000, + 'time_us': lambda x: x * 1000000, } ), MeasurementType('time_us', 'microseconds', conversions={ - 'time': lambda x: x / 1000, + 'time': lambda x: x / 1000000, } ), MeasurementType('temperature', 'degrees'), -- cgit v1.2.3 From 9b465c27662a8cdf68a52d02b823fc7d51cae051 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Fri, 18 Aug 2017 09:59:23 +0100 Subject: Instruments: Add millisecond MeasurementType and conversion Allows for reporting times in milliseconds as used with the acmecape instrument. --- devlib/instrument/__init__.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index dd5b1f5..f1a89e0 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -76,11 +76,19 @@ _measurement_types = [ MeasurementType('time', 'seconds', conversions={ 'time_us': lambda x: x * 1000000, + 'time_ms': lambda x: x * 1000, } ), MeasurementType('time_us', 'microseconds', conversions={ 'time': lambda x: x / 1000000, + 'time_ms': lambda x: x / 1000, + } + ), + MeasurementType('time_ms', 'milliseconds', + conversions={ + 'time': lambda x: x / 1000, + 'time_us': lambda x: x * 1000, } ), MeasurementType('temperature', 'degrees'), -- cgit v1.2.3 From 5ef99f2cff14f00da46d9ca011acdee8b5596779 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Wed, 2 Aug 2017 16:59:10 +0100 Subject: Instrument/MeasurementType: Allow for converting to the same type When trying to convert measurments to a standarised type some inputs may already be of the correct type and will now return the same value unchanged. --- devlib/instrument/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index f1a89e0..46f9c3e 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -48,6 +48,8 @@ class MeasurementType(object): if not isinstance(to, MeasurementType): msg = 'Unexpected conversion target: "{}"' raise ValueError(msg.format(to)) + if to.name == self.name: + return value if not to.name in self.conversions: msg = 'No conversion from {} to {} available' raise ValueError(msg.format(self.name, to.name)) -- cgit v1.2.3 From d3c3015fc82efe11521e41dc4b696ae396474e1d Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Thu, 3 Aug 2017 12:13:48 +0100 Subject: Instrument/MeasurementCSV: Add support for recording sample rate. If performing post processing on a MeasurementCsv file, if a timestamp is not available then the recorded sample rate can be used as a substitute. --- devlib/instrument/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 46f9c3e..9f8ac00 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -143,9 +143,10 @@ class Measurement(object): class MeasurementsCsv(object): - def __init__(self, path, channels=None): + def __init__(self, path, channels=None, sample_rate_hz=None): self.path = path self.channels = channels + self.sample_rate_hz = sample_rate_hz self._fh = open(path, 'rb') if self.channels is None: self._load_channels() -- cgit v1.2.3 From 30fdfc23d37d136ce66cea427f396653a340743f Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Mon, 24 Jul 2017 15:52:34 +0100 Subject: Instrument/Acmecape: Add support for acmecape --- devlib/instrument/acmecape.py | 122 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 devlib/instrument/acmecape.py diff --git a/devlib/instrument/acmecape.py b/devlib/instrument/acmecape.py new file mode 100644 index 0000000..abf6a19 --- /dev/null +++ b/devlib/instrument/acmecape.py @@ -0,0 +1,122 @@ +#pylint: disable=attribute-defined-outside-init +from __future__ import division +import csv +import os +import time +import tempfile +from fcntl import fcntl, F_GETFL, F_SETFL +from string import Template +from subprocess import Popen, PIPE, STDOUT + +from devlib import Instrument, CONTINUOUS, MeasurementsCsv +from devlib.exception import HostError +from devlib.utils.misc import which + +OUTPUT_CAPTURE_FILE = 'acme-cape.csv' +IIOCAP_CMD_TEMPLATE = Template(""" +${iio_capture} -n ${host} -b ${buffer_size} -c -f ${outfile} ${iio_device} +""") + +def _read_nonblock(pipe, size=1024): + fd = pipe.fileno() + flags = fcntl(fd, F_GETFL) + flags |= os.O_NONBLOCK + fcntl(fd, F_SETFL, flags) + + output = '' + try: + while True: + output += pipe.read(size) + except IOError: + pass + return output + + +class AcmeCapeInstrument(Instrument): + + mode = CONTINUOUS + + def __init__(self, target, + iio_capture=which('iio_capture'), + host='baylibre-acme.local', + iio_device='iio:device0', + buffer_size=256): + super(AcmeCapeInstrument, self).__init__(target) + self.iio_capture = iio_capture + self.host = host + self.iio_device = iio_device + self.buffer_size = buffer_size + if self.iio_capture is None: + raise HostError('Missing iio-capture binary') + self.command = None + self.process = None + + self.add_channel('shunt', 'voltage') + self.add_channel('bus', 'voltage') + self.add_channel('device', 'power') + self.add_channel('device', 'current') + self.add_channel('timestamp', 'time_ms') + + def reset(self, sites=None, kinds=None, channels=None): + super(AcmeCapeInstrument, self).reset(sites, kinds, channels) + self.raw_data_file = tempfile.mkstemp('.csv')[1] + params = dict( + iio_capture=self.iio_capture, + host=self.host, + buffer_size=self.buffer_size, + iio_device=self.iio_device, + outfile=self.raw_data_file + ) + 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) + + def stop(self): + self.process.terminate() + timeout_secs = 10 + for _ in xrange(timeout_secs): + if self.process.poll() is not None: + break + time.sleep(1) + else: + output = _read_nonblock(self.process.stdout) + self.process.kill() + self.logger.error('iio-capture did not terminate gracefully') + if self.process.poll() is None: + msg = 'Could not terminate iio-capture:\n{}' + raise HostError(msg.format(output)) + if not os.path.isfile(self.raw_data_file): + raise HostError('Output CSV not generated.') + + def get_data(self, outfile): + if os.stat(self.raw_data_file).st_size == 0: + self.logger.warning('"{}" appears to be empty'.format(self.raw_data_file)) + return + + all_channels = [c.label for c in self.list_channels()] + active_channels = [c.label for c in self.active_channels] + active_indexes = [all_channels.index(ac) for ac in active_channels] + + with open(self.raw_data_file, 'rb') as fh: + with open(outfile, 'wb') as wfh: + writer = csv.writer(wfh) + writer.writerow(active_channels) + + reader = csv.reader(fh, skipinitialspace=True) + header = reader.next() + ts_index = header.index('timestamp ms') + + + for row in reader: + output_row = [] + for i in active_indexes: + if i == ts_index: + # Leave time in ms + output_row.append(float(row[i])) + else: + # Convert rest into standard units. + output_row.append(float(row[i])/1000) + writer.writerow(output_row) + return MeasurementsCsv(outfile, self.active_channels) -- cgit v1.2.3 From 7dd934a5d8a9902d54385ea775558e2e8551570d Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Thu, 3 Aug 2017 16:41:10 +0100 Subject: Instrumentation: Update to populate missing 'sample_rate_hz` attributes To conform with the Instrumentation API each instrument should populate the `sample_rate_hz` attribute with the relevant information. --- devlib/instrument/acmecape.py | 1 + devlib/instrument/frames.py | 1 + devlib/instrument/gem5power.py | 13 ++++++++----- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/devlib/instrument/acmecape.py b/devlib/instrument/acmecape.py index abf6a19..103a9f8 100644 --- a/devlib/instrument/acmecape.py +++ b/devlib/instrument/acmecape.py @@ -46,6 +46,7 @@ class AcmeCapeInstrument(Instrument): self.host = host self.iio_device = iio_device self.buffer_size = buffer_size + self.sample_rate_hz = 100 if self.iio_capture is None: raise HostError('Missing iio-capture binary') self.command = None diff --git a/devlib/instrument/frames.py b/devlib/instrument/frames.py index d5a2147..d43977f 100644 --- a/devlib/instrument/frames.py +++ b/devlib/instrument/frames.py @@ -16,6 +16,7 @@ class FramesInstrument(Instrument): self.collector_target = collector_target self.period = period self.keep_raw = keep_raw + self.sample_rate_hz = 1 / self.period self.collector = None self.header = None self._need_reset = True diff --git a/devlib/instrument/gem5power.py b/devlib/instrument/gem5power.py index 7715ec1..534046e 100644 --- a/devlib/instrument/gem5power.py +++ b/devlib/instrument/gem5power.py @@ -28,10 +28,10 @@ class Gem5PowerInstrument(Instrument): mode = CONTINUOUS roi_label = 'power_instrument' - + def __init__(self, target, power_sites): ''' - Parameter power_sites is a list of gem5 identifiers for power values. + Parameter power_sites is a list of gem5 identifiers for power values. One example of such a field: system.cluster0.cores0.power_model.static_power ''' @@ -51,6 +51,9 @@ class Gem5PowerInstrument(Instrument): self.add_channel(field, 'power') self.target.gem5stats.book_roi(self.roi_label) self.sample_period_ns = 10000000 + # Sample rate must remain unset as gem5 does not provide samples + # at regular intervals therefore the reported timestamp should be used. + self.sample_rate_hz = None self.target.gem5stats.start_periodic_dump(0, self.sample_period_ns) self._base_stats_dump = 0 @@ -59,17 +62,17 @@ class Gem5PowerInstrument(Instrument): def stop(self): self.target.gem5stats.roi_end(self.roi_label) - + def get_data(self, outfile): active_sites = [c.site for c in self.active_channels] with open(outfile, 'wb') as wfh: writer = csv.writer(wfh) writer.writerow([c.label for c in self.active_channels]) # headers - for rec, rois in self.target.gem5stats.match_iter(active_sites, + for rec, rois in self.target.gem5stats.match_iter(active_sites, [self.roi_label], self._base_stats_dump): writer.writerow([float(rec[s]) for s in active_sites]) return MeasurementsCsv(outfile, self.active_channels) - + 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() -- cgit v1.2.3 From 411719d58d14250ef7fd9a992a1b3602b3856c88 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Fri, 18 Aug 2017 17:06:28 +0100 Subject: Instrument/gem5power: Rename timestamp channel to match new API To conform with the new DerivedMeasuements API the "sim_seconds" channel has been renamed to "timestamp" so that it can be identified in post processing. As "sim_seconds" needs to be extracted from the gem5 statistics file, an addional mapping has been added to support this. --- devlib/instrument/gem5power.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/devlib/instrument/gem5power.py b/devlib/instrument/gem5power.py index 534046e..1e392a8 100644 --- a/devlib/instrument/gem5power.py +++ b/devlib/instrument/gem5power.py @@ -28,6 +28,7 @@ class Gem5PowerInstrument(Instrument): mode = CONTINUOUS roi_label = 'power_instrument' + site_mapping = {'timestamp': 'sim_seconds'} def __init__(self, target, power_sites): ''' @@ -46,7 +47,7 @@ class Gem5PowerInstrument(Instrument): self.power_sites = power_sites else: self.power_sites = [power_sites] - self.add_channel('sim_seconds', 'time') + self.add_channel('timestamp', 'time') for field in self.power_sites: self.add_channel(field, 'power') self.target.gem5stats.book_roi(self.roi_label) @@ -68,7 +69,8 @@ class Gem5PowerInstrument(Instrument): with open(outfile, 'wb') as wfh: writer = csv.writer(wfh) writer.writerow([c.label for c in self.active_channels]) # headers - for rec, rois in self.target.gem5stats.match_iter(active_sites, + 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, [self.roi_label], self._base_stats_dump): writer.writerow([float(rec[s]) for s in active_sites]) return MeasurementsCsv(outfile, self.active_channels) -- cgit v1.2.3 From 049b275665e961f8d27c3f54347842e4560f8f68 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Thu, 3 Aug 2017 16:43:32 +0100 Subject: Instrumentation: Update to store sample rate in MeasurementCSV file. This commit updates existing instruments to store their sample rates when creating the respective MeasurementCsv files. --- devlib/instrument/acmecape.py | 2 +- devlib/instrument/daq.py | 2 +- devlib/instrument/energy_probe.py | 2 +- devlib/instrument/frames.py | 2 +- devlib/instrument/gem5power.py | 2 +- devlib/instrument/monsoon.py | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/devlib/instrument/acmecape.py b/devlib/instrument/acmecape.py index 103a9f8..e1bb6c1 100644 --- a/devlib/instrument/acmecape.py +++ b/devlib/instrument/acmecape.py @@ -120,4 +120,4 @@ class AcmeCapeInstrument(Instrument): # Convert rest into standard units. output_row.append(float(row[i])/1000) writer.writerow(output_row) - return MeasurementsCsv(outfile, self.active_channels) + return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) diff --git a/devlib/instrument/daq.py b/devlib/instrument/daq.py index 58d2f3e..75e854d 100644 --- a/devlib/instrument/daq.py +++ b/devlib/instrument/daq.py @@ -126,7 +126,7 @@ class DaqInstrument(Instrument): writer.writerow(row) raw_row = _read_next_rows() - return MeasurementsCsv(outfile, self.active_channels) + return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) finally: for fh in file_handles: fh.close() diff --git a/devlib/instrument/energy_probe.py b/devlib/instrument/energy_probe.py index ed502f5..5f47430 100644 --- a/devlib/instrument/energy_probe.py +++ b/devlib/instrument/energy_probe.py @@ -113,4 +113,4 @@ class EnergyProbeInstrument(Instrument): continue else: not_a_full_row_seen = True - return MeasurementsCsv(outfile, self.active_channels) + return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) diff --git a/devlib/instrument/frames.py b/devlib/instrument/frames.py index d43977f..d1899fb 100644 --- a/devlib/instrument/frames.py +++ b/devlib/instrument/frames.py @@ -44,7 +44,7 @@ class FramesInstrument(Instrument): self.collector.process_frames(raw_outfile) active_sites = [chan.label for chan in self.active_channels] self.collector.write_frames(outfile, columns=active_sites) - return MeasurementsCsv(outfile, self.active_channels) + return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) def _init_channels(self): raise NotImplementedError() diff --git a/devlib/instrument/gem5power.py b/devlib/instrument/gem5power.py index 1e392a8..d265440 100644 --- a/devlib/instrument/gem5power.py +++ b/devlib/instrument/gem5power.py @@ -73,7 +73,7 @@ class Gem5PowerInstrument(Instrument): for rec, rois in self.target.gem5stats.match_iter(sites_to_match, [self.roi_label], self._base_stats_dump): writer.writerow([float(rec[s]) for s in active_sites]) - return MeasurementsCsv(outfile, self.active_channels) + return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) def reset(self, sites=None, kinds=None, channels=None): super(Gem5PowerInstrument, self).reset(sites, kinds, channels) diff --git a/devlib/instrument/monsoon.py b/devlib/instrument/monsoon.py index e373d68..3103618 100644 --- a/devlib/instrument/monsoon.py +++ b/devlib/instrument/monsoon.py @@ -129,4 +129,4 @@ class MonsoonInstrument(Instrument): row.append(usb) writer.writerow(row) - return MeasurementsCsv(outfile, self.active_channels) + return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) -- cgit v1.2.3 From c093d567542460e398662b76e0577fbb49ae89f7 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Mon, 7 Aug 2017 14:29:40 +0100 Subject: DerivedMeasurements: Add DerivedEnergyMeasurments Adds `DerivedMeasurements` which are designed to perform post processing on a provided MeasurementCsv. Currently only a `DerivedEnergyMeasurements` class has been added which has 2 purposes: - Calculate energy from power results if not present using recorded timestamps, falling back to a provided sample rate - Calculate cumulative energy and average power from a specified MeasurementCSV file. --- devlib/__init__.py | 3 ++ devlib/derived/__init__.py | 19 +++++++ devlib/derived/derived_measurements.py | 97 ++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 devlib/derived/__init__.py create mode 100644 devlib/derived/derived_measurements.py diff --git a/devlib/__init__.py b/devlib/__init__.py index 2f50632..b1b4fa3 100644 --- a/devlib/__init__.py +++ b/devlib/__init__.py @@ -19,6 +19,9 @@ from devlib.instrument.monsoon import MonsoonInstrument from devlib.instrument.netstats import NetstatsInstrument from devlib.instrument.gem5power import Gem5PowerInstrument +from devlib.derived import DerivedMeasurements +from devlib.derived.derived_measurements import DerivedEnergyMeasurements + from devlib.trace.ftrace import FtraceCollector from devlib.host import LocalConnection diff --git a/devlib/derived/__init__.py b/devlib/derived/__init__.py new file mode 100644 index 0000000..5689a58 --- /dev/null +++ b/devlib/derived/__init__.py @@ -0,0 +1,19 @@ +# Copyright 2015 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. +# +class DerivedMeasurements(object): + + @staticmethod + def process(measurements_csv): + raise NotImplementedError() diff --git a/devlib/derived/derived_measurements.py b/devlib/derived/derived_measurements.py new file mode 100644 index 0000000..770db88 --- /dev/null +++ b/devlib/derived/derived_measurements.py @@ -0,0 +1,97 @@ +# Copyright 2013-2015 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. +# +from __future__ import division +from collections import defaultdict + +from devlib import DerivedMeasurements +from devlib.instrument import Measurement, MEASUREMENT_TYPES, InstrumentChannel + + +class DerivedEnergyMeasurements(DerivedMeasurements): + + @staticmethod + def process(measurements_csv): + + should_calculate_energy = [] + use_timestamp = False + + # Determine sites to calculate energy for + channel_map = defaultdict(list) + for channel in measurements_csv.channels: + channel_map[channel].append(channel.kind) + for channel, kinds in channel_map.iteritems(): + if 'power' in kinds and not 'energy' in kinds: + should_calculate_energy.append(channel.site) + if channel.site == 'timestamp': + use_timestamp = True + time_measurment = channel.measurement_type + + if measurements_csv.sample_rate_hz is None and not use_timestamp: + msg = 'Timestamp data is unavailable, please provide a sample rate' + raise ValueError(msg) + + if use_timestamp: + # Find index of timestamp column + ts_index = [i for i, chan in enumerate(measurements_csv.channels) + if chan.site == 'timestamp'] + if len(ts_index) > 1: + raise ValueError('Multiple timestamps detected') + ts_index = ts_index[0] + + row_ts = 0 + last_ts = 0 + energy_results = defaultdict(dict) + power_results = defaultdict(float) + + # Process data + for count, row in enumerate(measurements_csv.itermeasurements()): + if use_timestamp: + last_ts = row_ts + row_ts = time_measurment.convert(float(row[ts_index].value), 'time') + for entry in row: + channel = entry.channel + site = channel.site + if channel.kind == 'energy': + if count == 0: + energy_results[site]['start'] = entry.value + else: + energy_results[site]['end'] = entry.value + + if channel.kind == 'power': + power_results[site] += entry.value + + if site in should_calculate_energy: + if count == 0: + energy_results[site]['start'] = 0 + energy_results[site]['end'] = 0 + elif use_timestamp: + energy_results[site]['end'] += entry.value * (row_ts - last_ts) + else: + energy_results[site]['end'] += entry.value * (1 / + measurements_csv.sample_rate_hz) + + # Calculate final measurements + derived_measurements = [] + for site in energy_results: + total_energy = energy_results[site]['end'] - energy_results[site]['start'] + instChannel = InstrumentChannel('cum_energy', site, MEASUREMENT_TYPES['energy']) + derived_measurements.append(Measurement(total_energy, instChannel)) + + for site in power_results: + power = power_results[site] / (count + 1) #pylint: disable=undefined-loop-variable + instChannel = InstrumentChannel('avg_power', site, MEASUREMENT_TYPES['power']) + derived_measurements.append(Measurement(power, instChannel)) + + return derived_measurements -- cgit v1.2.3 From eeb5e93e6f30887e6e0835f9d033e64c233ff885 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Mon, 7 Aug 2017 15:39:38 +0100 Subject: Documentation/Instrumentation: Fix typos --- doc/instrumentation.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/instrumentation.rst b/doc/instrumentation.rst index 3c777ac..91a2e64 100644 --- a/doc/instrumentation.rst +++ b/doc/instrumentation.rst @@ -151,7 +151,7 @@ Instrument Sample rate of the instrument in Hz. Assumed to be the same for all channels. .. note:: This attribute is only provided by :class:`Instrument`\ s that - support ``CONTINUOUS`` measurment. + support ``CONTINUOUS`` measurement. Instrument Channel ~~~~~~~~~~~~~~~~~~ -- cgit v1.2.3 From c62905cfdc03938803e3a2dd261953599fb0416f Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Fri, 18 Aug 2017 17:22:47 +0100 Subject: Instrumentation/Instrumentation: Update `timestamp` documentation --- doc/instrumentation.rst | 54 ++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/doc/instrumentation.rst b/doc/instrumentation.rst index 91a2e64..76adf39 100644 --- a/doc/instrumentation.rst +++ b/doc/instrumentation.rst @@ -139,6 +139,14 @@ Instrument ``_`` (see :class:`InstrumentChannel`). The order of the columns will be the same as the order of channels in ``Instrument.active_channels``. + If reporting timestamps, one channel must have a ``site`` named ``"timestamp"`` + and a ``kind`` of a :class:`MeasurmentType` of an appropriate time unit which will + be used, if appropriate, during any post processing. + + .. note:: Currently supported time units are seconds, milliseconds and + microseconds, other units can also be used if an appropriate + conversion is provided. + This returns a :class:`MeasurementCsv` instance associated with the outfile that can be used to stream :class:`Measurement`\ s lists (similar to what is returned by ``take_measurement()``. @@ -211,27 +219,31 @@ be reported as "power" in Watts, and never as "pwr" in milliWatts. Currently defined measurement types are -+-------------+---------+---------------+ -| name | units | category | -+=============+=========+===============+ -| time | seconds | | -+-------------+---------+---------------+ -| temperature | degrees | | -+-------------+---------+---------------+ -| power | watts | power/energy | -+-------------+---------+---------------+ -| voltage | volts | power/energy | -+-------------+---------+---------------+ -| current | amps | power/energy | -+-------------+---------+---------------+ -| energy | joules | power/energy | -+-------------+---------+---------------+ -| tx | bytes | data transfer | -+-------------+---------+---------------+ -| rx | bytes | data transfer | -+-------------+---------+---------------+ -| tx/rx | bytes | data transfer | -+-------------+---------+---------------+ ++-------------+-------------+---------------+ +| name | units | category | ++=============+=============+===============+ +| time | seconds | | ++-------------+-------------+---------------+ +| time | microseconds| | ++-------------+-------------+---------------+ +| time | milliseconds| | ++-------------+-------------+---------------+ +| temperature | degrees | | ++-------------+-------------+---------------+ +| power | watts | power/energy | ++-------------+-------------+---------------+ +| voltage | volts | power/energy | ++-------------+-------------+---------------+ +| current | amps | power/energy | ++-------------+-------------+---------------+ +| energy | joules | power/energy | ++-------------+-------------+---------------+ +| tx | bytes | data transfer | ++-------------+-------------+---------------+ +| rx | bytes | data transfer | ++-------------+-------------+---------------+ +| tx/rx | bytes | data transfer | ++-------------+-------------+---------------+ .. instruments: -- cgit v1.2.3 From 60e69fc4e89cdf17649848729fd8b9add0e21a11 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Fri, 18 Aug 2017 17:23:18 +0100 Subject: Documentation/DerivedMeasurements: Added documentation for new API --- doc/derived_measurements.rst | 69 ++++++++++++++++++++++++++++++++++++++++++++ doc/index.rst | 1 + 2 files changed, 70 insertions(+) create mode 100644 doc/derived_measurements.rst diff --git a/doc/derived_measurements.rst b/doc/derived_measurements.rst new file mode 100644 index 0000000..fcd497c --- /dev/null +++ b/doc/derived_measurements.rst @@ -0,0 +1,69 @@ +Derived Measurements +===================== + + +The ``DerivedMeasurements`` API provides a consistent way of performing post +processing on a provided :class:`MeasurementCsv` file. + +Example +------- + +The following example shows how to use an implementation of a +:class:`DerivedMeasurement` to obtain a list of calculated ``Measurements``. + +.. code-block:: ipython + + # Import the relevant derived measurement module + # in this example the derived energy module is used. + In [1]: from devlib import DerivedEnergyMeasurements + + # Obtain a MeasurementCsv file from an instrument or create from + # existing .csv file. In this example an existing csv file is used which was + # created with a sampling rate of 100Hz + In [2]: from devlib import MeasurementsCsv + In [3]: measurement_csv = MeasurementsCsv('/example/measurements.csv', sample_rate_hz=100) + + # Process the file and obtain a list of the derived measurements + In [4]: derived_measurements = DerivedEnergyMeasurements.process(measurement_csv) + + In [5]: derived_measurements + Out[5]: [device_energy: 239.1854075 joules, device_power: 5.5494089227 watts] + +API +--- + +Derived Measurements +~~~~~~~~~~~~~~~~~~~~ + +.. class:: DerivedMeasurements() + + The ``DerivedMeasurements`` class is an abstract base for implementing + additional classes to calculate various metrics. + +.. method:: DerivedMeasurements.process(measurement_csv) + + Returns a list of :class:`Measurement` objects that have been calculated. + + + +Available Derived Measurements +------------------------------- +.. class:: DerivedEnergyMeasurements() + + The ``DerivedEnergyMeasurements`` class is used to calculate average power and + cumulative energy for each site if the required data is present. + + The calculation of cumulative energy can occur in 3 ways. If a + ``site`` contains ``energy`` results, the first and last measurements are extracted + and the delta calculated. If not, a ``timestamp`` channel will be used to calculate + the energy from the power channel, failing back to using the sample rate attribute + of the :class:`MeasurementCsv` file if timestamps are not available. If neither + timestamps or a sample rate are available then an error will be raised. + + +.. method:: DerivedEnergyMeasurements.process(measurement_csv) + + Returns a list of :class:`Measurement` objects that have been calculated for + the average power and cumulative energy for each site. + + diff --git a/doc/index.rst b/doc/index.rst index 2c6d72f..5f4dda5 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -19,6 +19,7 @@ Contents: target modules instrumentation + derived_measurements platform connection -- cgit v1.2.3 From 64c865de59db951dfe142d62ac55b6c6ce276012 Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Wed, 16 Aug 2017 12:40:16 +0100 Subject: module: gem5stats: enhance match() with regex support The current implementation of match() in the gem5stats module returns records matching exactly the specified keys. This commit changes this behaviour by matching keys over regular expressions, hence resulting in a much more powerful match() implementation. --- devlib/module/gem5stats.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/devlib/module/gem5stats.py b/devlib/module/gem5stats.py index 9109751..8fea339 100644 --- a/devlib/module/gem5stats.py +++ b/devlib/module/gem5stats.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import re import sys import logging import os.path @@ -139,6 +140,10 @@ class Gem5StatsModule(Module): 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) @@ -148,8 +153,8 @@ class Gem5StatsModule(Module): for dump in iter_statistics_dump(stats_file): active_rois = [l for l in rois_labels if roi_active(l, dump)] if active_rois: - record = {k: dump[k] for k in keys} - yield (record, active_rois) + rec = {k: dump[k] for k in dump if all_keys_re.search(k)} + yield (rec, active_rois) def next_dump_no(self): ''' -- cgit v1.2.3 From 3c8294c6eb1e6a466780153d7e323973c6b5a2c8 Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Wed, 16 Aug 2017 14:39:55 +0100 Subject: module/gem5stats: better document match() and match_iter() behaviours --- devlib/module/gem5stats.py | 84 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 70 insertions(+), 14 deletions(-) diff --git a/devlib/module/gem5stats.py b/devlib/module/gem5stats.py index 8fea339..0f0fbd7 100644 --- a/devlib/module/gem5stats.py +++ b/devlib/module/gem5stats.py @@ -108,14 +108,55 @@ class Gem5StatsModule(Module): def match(self, keys, rois_labels, base_dump=0): ''' - Tries to match the list of keys passed as parameter over the statistics - dumps covered by selected ROIs since ``base_dump``. 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. - - Keys must match fields in gem5's statistics log file. Key example: - system.cluster0.cores0.power_model.static_power + Extract specific values from the statistics log file of gem5 + + :param keys: a list of key name or regular expression patterns that + will be matched in the fields of the statistics file. ``match()`` + returns only the values of fields matching at least one these + keys. + :type keys: list + + :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 + the head of the stats file and ignore all dumps before a specific + 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. + + Example of return value: + * Result of match(['sim_'],['roi_1']): + { + 'sim_inst': + { + 'roi_1': [265300176, 267975881] + } + 'sim_ops': + { + 'roi_1': [324395787, 327699419] + } + 'sim_seconds': + { + 'roi_1': [0.199960, 0.199897] + } + 'sim_freq': + { + 'roi_1': [1000000000000, 1000000000000] + } + 'sim_ticks': + { + 'roi_1': [199960234227, 199896897330] + } + } ''' records = defaultdict(lambda : defaultdict(list)) for record, active_rois in self.match_iter(keys, rois_labels, base_dump): @@ -126,12 +167,27 @@ class Gem5StatsModule(Module): def match_iter(self, keys, rois_labels, base_dump=0): ''' - Yields for each dump since ``base_dump`` a pair containing: - 1. a dict storing the values corresponding to each of the specified keys - 2. the list of currently active ROIs among those passed as parameters. - - Keys must match fields in gem5's statistics log file. Key example: - system.cluster0.cores0.power_model.static_power + Yield specific values dump-by-dump from the statistics log file of gem5 + + :param keys: same as ``match()`` + :param rois_labels: same as ``match()`` + :param base_dump: same as ``match()`` + :returns: a pair containing: + 1. a dict storing the values corresponding to each of the found keys + 2. the list of currently active ROIs among those passed as parameters + + 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_freq': 1000000000000, + 'sim_ticks': 199960234227, + }, + [ 'roi_1 ' ] + ) ''' for label in rois_labels: if label not in self.rois: -- cgit v1.2.3 From 4b36439de84fae46c3eb0dcbc90c79583d6bc09a Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Mon, 21 Aug 2017 18:16:10 +0100 Subject: module/cgroups: robustify task freezer The set() method of the CGroup class used to freeze tasks relies on target's write_value(). Sometimes, the freezing procedure takes some time and the call to write_value() in set() fails by reading "FREEZING" while it expected "FROZEN". To avoid this issue, this commits introduces a shutil call dedicated to changing the state of the freezer controller. --- devlib/bin/scripts/shutils.in | 20 ++++++++++++++++++++ devlib/module/cgroups.py | 6 +++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/devlib/bin/scripts/shutils.in b/devlib/bin/scripts/shutils.in index d44e444..6d78be7 100755 --- a/devlib/bin/scripts/shutils.in +++ b/devlib/bin/scripts/shutils.in @@ -177,6 +177,23 @@ cgroups_tasks_in() { exit 0 } +cgroups_freezer_set_state() { + STATE=${1} + SYSFS_ENTRY=${2}/freezer.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 + sleep 1 + done + + # We have an issue + echo "ERROR: Freezer stalled while changing state to \"$STATE\"." >&2 + exit 1 +} ################################################################################ # Main Function Dispatcher @@ -213,6 +230,9 @@ cgroups_tasks_move) cgroups_tasks_in) cgroups_tasks_in $* ;; +cgroups_freezer_set_state) + cgroups_freezer_set_state $* + ;; ftrace_get_function_stats) ftrace_get_function_stats ;; diff --git a/devlib/module/cgroups.py b/devlib/module/cgroups.py index bfe2785..8987899 100644 --- a/devlib/module/cgroups.py +++ b/devlib/module/cgroups.py @@ -466,11 +466,11 @@ class CgroupsModule(Module): if freezer is None: raise RuntimeError('freezer cgroup controller not present') freezer_cg = freezer.cgroup('/DEVLIB_FREEZER') - thawed_cg = freezer.cgroup('/') + cmd = 'cgroups_freezer_set_state {{}} {}'.format(freezer_cg.directory) if thaw: # Restart froozen tasks - freezer_cg.set(state='THAWED') + freezer.target._execute_util(cmd.format('THAWED'), as_root=True) # Remove all tasks from freezer freezer.move_all_tasks_to('/') return @@ -482,7 +482,7 @@ class CgroupsModule(Module): tasks = freezer.tasks('/') # Freeze all tasks - freezer_cg.set(state='FROZEN') + freezer.target._execute_util(cmd.format('FROZEN'), as_root=True) return tasks -- cgit v1.2.3 From 34d73e6af1b170178f9b9923637ce789d7985161 Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Thu, 17 Aug 2017 10:40:10 +0100 Subject: utils/gem5: try to cast statistics dump values All values in the gem5 statistics log file are numeric. This commit adds a cast on the strings read from the stats file to native numeric values when and logs a warning in case of a malformed entry. --- devlib/instrument/gem5power.py | 2 +- devlib/utils/gem5.py | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/devlib/instrument/gem5power.py b/devlib/instrument/gem5power.py index d265440..4b145d9 100644 --- a/devlib/instrument/gem5power.py +++ b/devlib/instrument/gem5power.py @@ -72,7 +72,7 @@ class Gem5PowerInstrument(Instrument): 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, [self.roi_label], self._base_stats_dump): - writer.writerow([float(rec[s]) for s in active_sites]) + writer.writerow([rec[s] for s in active_sites]) return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) def reset(self, sites=None, kinds=None, channels=None): diff --git a/devlib/utils/gem5.py b/devlib/utils/gem5.py index c609d70..0ca42ec 100644 --- a/devlib/utils/gem5.py +++ b/devlib/utils/gem5.py @@ -13,6 +13,9 @@ # limitations under the License. import re +import logging + +from devlib.utils.types import numeric GEM5STATS_FIELD_REGEX = re.compile("^(?P[^- ]\S*) +(?P[^#]+).+$") @@ -20,6 +23,8 @@ GEM5STATS_DUMP_HEAD = '---------- Begin Simulation Statistics ----------' GEM5STATS_DUMP_TAIL = '---------- End Simulation Statistics ----------' GEM5STATS_ROI_NUMBER = 8 +logger = logging.getLogger('gem5') + def iter_statistics_dump(stats_file): ''' @@ -38,6 +43,11 @@ def iter_statistics_dump(stats_file): res = GEM5STATS_FIELD_REGEX.match(line) if res: k = res.group("key") - v = res.group("value").split() - cur_dump[k] = v[0] if len(v)==1 else set(v) + vtext = res.group("value") + try: + v = map(numeric, vtext.split()) + 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)) -- cgit v1.2.3 From 42fa0e2a83260d73cd8ac70a623e3eea7196c352 Mon Sep 17 00:00:00 2001 From: Suren Baghdasaryan Date: Mon, 28 Aug 2017 14:39:54 -0700 Subject: utils/android_build: Add Build class for Android-specific build tasks Add Build class that allows to build Android modules and kernel. Change-Id: Ia0d985170ad40b44e7b85e0fbf3b89f891867ef5 Signed-off-by: Suren Baghdasaryan --- devlib/utils/android_build.py | 110 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 devlib/utils/android_build.py diff --git a/devlib/utils/android_build.py b/devlib/utils/android_build.py new file mode 100644 index 0000000..da89dbb --- /dev/null +++ b/devlib/utils/android_build.py @@ -0,0 +1,110 @@ +# SPDX-License-Identifier: Apache-2.0 +# +# Copyright (C) 2015, ARM Limited and contributors. +# +# 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 os +from collections import namedtuple + +class Build(object): + BuildParams = namedtuple("BuildParams", "arch defconfig cross_compile") + + device_kernel_build_params = { + 'hikey960': BuildParams('arm64', 'hikey960_defconfig', 'aarch64-linux-android-'), + 'hikey': BuildParams('arm64', 'hikey_defconfig', 'aarch64-linux-android-') + } + + """ + Collection of Android related build actions + """ + def __init__(self, te): + if (te.ANDROID_BUILD_TOP and te.TARGET_PRODUCT and te.TARGET_BUILD_VARIANT): + self._te = te + else: + te._log.warning('Build initialization failed: invalid paramterers') + raise + + def build_module(self, module_path): + """ + Build a module and its dependencies. + + :param module_path: module path + :type module_path: str + + """ + self._te._log.info('BUILDING module %s', module_path) + cur_dir = os.getcwd() + os.chdir(self._te.ANDROID_BUILD_TOP) + lunch_target = self._te.TARGET_PRODUCT + '-' + self._te.TARGET_BUILD_VARIANT + os.system('source build/envsetup.sh && lunch ' + + lunch_target + ' && mmma -j16 ' + module_path) + os.chdir(cur_dir) + + + def build_kernel(self, kernel_path, arch, defconfig, cross_compile, clean=False): + """ + Build kernel. + + :param kernel_path: path to kernel sources + :type kernel_path: str + + :param arch: device architecture string used in ARCH command line parameter + :type arch: str + + :param defconfig: defconfig file name + :type defconfig: str + + :param cross_compile: compiler string used in CROSS_COMPILE command line parameter + :type cross_compile: str + + :param clean: flag to clean the previous build + :type clean: boolean + + """ + self._te._log.info('BUILDING kernel @ %s', kernel_path) + cur_dir = os.getcwd() + os.chdir(kernel_path) + os.system('. ./build.config') + if clean: + os.system('make clean') + os.system('make ARCH=' + arch + ' ' + defconfig) + os.system('make -j24 ARCH=' + arch + ' CROSS_COMPILE=' + cross_compile) + os.chdir(cur_dir) + + def build_kernel_for_device(self, kernel_path, device_name, clean): + """ + Build kernel for specified device. + + :param kernel_path: path to kernel sources + :type kernel_path: str + + :param device_name: device name + :type device_name: str + + :param clean: flag to clean the previous build + :type clean: boolean + + """ + if not device_name in self.device_kernel_build_params: + return False + + params = self.device_kernel_build_params[device_name] + self.build_kernel(kernel_path, params.arch, + params.defconfig, params.cross_compile, clean) + return True + + +# vim :set tabstop=4 shiftwidth=4 expandtab -- cgit v1.2.3 From 07048eeb8bf34603db2670cb40630df2d5e59d49 Mon Sep 17 00:00:00 2001 From: Marissa Wall Date: Thu, 31 Aug 2017 16:26:35 -0700 Subject: cpufreq/time_in_state: add time_in_state support Get the time_in_state for all cpus by cluster. Add option to compute the delta between two time_in_states. This can be used in determining how much power the cpu used over a period of time. Test: call cpufreq.get_time_in_state(clusters) Change-Id: I173d269c486b476005563d3ffa4af29024b91a15 --- devlib/module/cpufreq.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/devlib/module/cpufreq.py b/devlib/module/cpufreq.py index e18d95b..5e8c585 100644 --- a/devlib/module/cpufreq.py +++ b/devlib/module/cpufreq.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # +import json from devlib.module import Module from devlib.exception import TargetError from devlib.utils.misc import memoized @@ -434,3 +435,55 @@ class CpufreqModule(Module): domain = self.target.cpufreq.get_domain_cpus(cpu) yield domain cpus = cpus.difference(domain) + + def get_time_in_state(self, clusters): + """ + Gets the time at each frequency on each cluster + :param clusters: A list of clusters on the device. Each cluster is a + list of cpus on that cluster. + """ + time_in_state_by_cluster = {} + + for i, cluster in enumerate(clusters): + frequencies = self.list_frequencies(cluster[0]) + time_in_state = dict((str(freq), 0) for freq in frequencies) + + for cpu in cluster: + stats = self.target.execute('cat '\ + '/sys/devices/system/cpu/cpu{}/cpufreq/stats' + '/time_in_state'.format(cpu)) + for entry in stats.split('\n'): + if len(entry) > 0: + freq, time = entry.split(' ') + time_in_state[freq] += int(time) + + time_in_state_by_cluster[str(i)] = time_in_state + + return time_in_state_by_cluster + + def dump_time_in_state_delta(self, start, clusters, dump_file): + """ + Dumps the time between the stop and start by cluster by frequency + :param start: The inital output from a call to cpufreq.get_time_in_state + :param clusters: A list of clusters on the device. Each cluster is a + list of cpus on that cluster. + :param dump_file: A file to dump the delta time_in_state_delta to. + """ + stop = self.get_time_in_state(clusters) + + time_in_state_delta = {} + + for cl in start: + time_in_state_delta[cl] = {} + + for freq in start[cl].keys(): + time_in_start = start[cl][freq] + time_in_stop = stop[cl][freq] + time_in_state_delta[cl][freq] = time_in_stop - time_in_start + + output = {'time_delta' : time_in_state_delta, + 'clusters' : {str(i) : [str(c) for c in cl] + for i, cl in enumerate(clusters)}} + + with open(dump_file, 'w') as dfile: + json.dump(output, dfile, indent=4, sort_keys=True) -- cgit v1.2.3 From 035181a8f17adfb92c2be90776867fba908dc754 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Mon, 11 Sep 2017 16:55:02 +0100 Subject: utils/android: Add LogcatMonitor --- devlib/utils/android.py | 129 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 45bff20..48d6cf5 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -24,6 +24,9 @@ import time import subprocess import logging import re +import threading +import tempfile +import Queue from collections import defaultdict from devlib.exception import TargetError, HostError, DevlibError @@ -508,3 +511,129 @@ def _check_env(): platform_tools = _env.platform_tools adb = _env.adb aapt = _env.aapt + +class LogcatMonitor(threading.Thread): + + FLUSH_SIZE = 1000 + + @property + def logfile(self): + return self._logfile + + def __init__(self, target, regexps=None): + super(LogcatMonitor, self).__init__() + + self.target = target + + self._stopped = threading.Event() + self._match_found = threading.Event() + + self._sought = None + self._found = None + + self._lines = Queue.Queue() + self._datalock = threading.Lock() + self._regexps = regexps + + def start(self, outfile=None): + if outfile: + self._logfile = outfile + else: + fd, self._logfile = tempfile.mkstemp() + os.close(fd) + logger.debug('Logging to {}'.format(self._logfile)) + + super(LogcatMonitor, self).start() + + def run(self): + self.target.clear_logcat() + + logcat_cmd = 'logcat' + + # Join all requested regexps with an 'or' + if self._regexps: + regexp = '{}'.format('|'.join(self._regexps)) + if len(self._regexps) > 1: + regexp = '({})'.format(regexp) + logcat_cmd = '{} -e {}'.format(logcat_cmd, regexp) + + logger.debug('logcat command ="{}"'.format(logcat_cmd)) + self._logcat = self.target.background(logcat_cmd) + + while not self._stopped.is_set(): + line = self._logcat.stdout.readline(1024) + self._add_line(line) + + def stop(self): + # Popen can be stuck on readline() so send it a SIGKILL + self._logcat.terminate() + + self._stopped.set() + self.join() + + self._flush_lines() + + def _add_line(self, line): + self._lines.put(line) + + if self._sought and re.match(self._sought, line): + self._found = line + self._match_found.set() + + if self._lines.qsize() >= self.FLUSH_SIZE: + self._flush_lines() + + def _flush_lines(self): + with self._datalock: + with open(self._logfile, 'a') as fh: + while not self._lines.empty(): + fh.write(self._lines.get()) + + def clear_log(self): + with self._datalock: + while not self._lines.empty(): + self._lines.get() + + with open(self._logfile, 'w') as fh: + pass + + def get_log(self): + """ + Return the list of lines found by the monitor + """ + self._flush_lines() + + with self._datalock: + with open(self._logfile, 'r') as fh: + res = [line for line in fh] + + return res + + def search(self, regexp, timeout=30): + """ + Search a line that matches a regexp in the logcat log + """ + res = [] + + self._flush_lines() + + with self._datalock: + with open(self._logfile, 'r') as fh: + for line in fh: + if re.match(regexp, line): + res.append(line) + + # Found some matches, return them + if len(res) > 0: + return res + + # Did not find any match, wait for one to pop up + self._sought = regexp + found = self._match_found.wait(timeout) + self._match_found.clear() + self._sought = None + + if found: + return [self._found] + else: + raise RuntimeError('Logcat monitor timeout ({}s)'.format(timeout)) -- cgit v1.2.3 From 7c2fd87a3b04df99c6c6ad612d017b8303f0ba10 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Mon, 11 Sep 2017 16:58:24 +0100 Subject: target: Add LogcatMonitor getter for android target --- devlib/target.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/devlib/target.py b/devlib/target.py index b96bbdc..51826fb 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -14,7 +14,7 @@ from devlib.module import get_module from devlib.platform import Platform from devlib.exception import TargetError, TargetNotRespondingError, TimeoutError from devlib.utils.ssh import SshConnection -from devlib.utils.android import AdbConnection, AndroidProperties, adb_command, adb_disconnect +from devlib.utils.android import AdbConnection, AndroidProperties, LogcatMonitor, adb_command, adb_disconnect from devlib.utils.misc import memoized, isiterable, convert_new_lines, merge_lists from devlib.utils.misc import ABI_MAP, get_cpu_name, ranges_to_list, escape_double_quotes from devlib.utils.types import integer, boolean, bitmask, identifier, caseless_string @@ -1156,6 +1156,9 @@ class AndroidTarget(Target): def clear_logcat(self): adb_command(self.adb_name, 'logcat -c', timeout=30) + def get_logcat_monitor(self, regexps=None): + return LogcatMonitor(self, regexps) + def adb_kill_server(self, timeout=30): adb_command(self.adb_name, 'kill-server', timeout) -- cgit v1.2.3 From 0d3a0223b38aa5b9e59d2af716db3d3d9622c256 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Mon, 11 Sep 2017 16:51:33 +0100 Subject: trace: Add logcat trace collector --- devlib/trace/logcat.py | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 devlib/trace/logcat.py diff --git a/devlib/trace/logcat.py b/devlib/trace/logcat.py new file mode 100644 index 0000000..1372d7a --- /dev/null +++ b/devlib/trace/logcat.py @@ -0,0 +1,58 @@ +import os +import re +import shutil + +from devlib.trace import TraceCollector +from devlib.utils.android import LogcatMonitor + +class LogcatCollector(TraceCollector): + + def __init__(self, target, regexps=None): + super(LogcatCollector, self).__init__(target) + self.regexps = regexps + self._collecting = False + self._prev_log = None + + def reset(self): + """ + Clear Collector data but do not interrupt collection + """ + if not self._monitor: + return + + if self._collecting: + self._monitor.clear_log() + elif self._prev_log: + os.remove(self._prev_log) + self._prev_log = None + + def start(self): + """ + Start collecting logcat lines + """ + self._monitor = LogcatMonitor(self.target, self.regexps) + if self._prev_log: + # Append new data collection to previous collection + self._monitor.start(self._prev_log) + else: + self._monitor.start() + + self._collecting = True + + def stop(self): + """ + Stop collecting logcat lines + """ + if not self._collecting: + raise RuntimeError('Logcat monitor not running, nothing to stop') + + self._monitor.stop() + self._collecting = False + self._prev_log = self._monitor.logfile + + def get_trace(self, outfile): + """ + Output collected logcat lines to designated file + """ + # copy self._monitor.logfile to outfile + shutil.copy(self._monitor.logfile, outfile) -- cgit v1.2.3 From 4de973483e155a3cc51d184218bca399ab5460b0 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Tue, 12 Sep 2017 14:52:39 +0100 Subject: host: Add kill_children utility method This method is useful for killing the children spawned by Popen() calls with shell=True. For instance: proc = Popen('sleep 100', shell=True) proc.kill() This would spawn a shell task and that shell would spawn the sleep task. Issuing a kill to the Popen handle will only kill the shell task, and the sleep task will keep running. Using host.kill_children(proc.pid) will ensure all child tasks are killed. --- devlib/host.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/devlib/host.py b/devlib/host.py index 8c8a069..8062d24 100644 --- a/devlib/host.py +++ b/devlib/host.py @@ -14,6 +14,7 @@ # from glob import iglob import os +import signal import shutil import subprocess import logging @@ -24,6 +25,11 @@ from devlib.utils.misc import check_output PACKAGE_BIN_DIRECTORY = os.path.join(os.path.dirname(__file__), 'bin') +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()): + kill_children(cpid, signal) + os.kill(cpid, signal) class LocalConnection(object): -- cgit v1.2.3 From 6bda0934ad97a48236c0d9545b4d17f5a2571547 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Tue, 12 Sep 2017 15:44:25 +0100 Subject: utils/android: LogcatMonitor fixes host.kill_children() is used to properly kill the logcat process when it is IO blocked. The logcat regexp argument is now within double quotes, as having parenthesis within the regexp could break the command. LogcatMonitor.search() has been renamed to wait_for() to make the behaviour of the method more explicit. A non-blocking version of this method has been added and is named search(). --- devlib/utils/android.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 48d6cf5..4916246 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -32,6 +32,7 @@ from collections import defaultdict 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 logger = logging.getLogger('android') @@ -555,18 +556,21 @@ class LogcatMonitor(threading.Thread): regexp = '{}'.format('|'.join(self._regexps)) if len(self._regexps) > 1: regexp = '({})'.format(regexp) - logcat_cmd = '{} -e {}'.format(logcat_cmd, regexp) + logcat_cmd = '{} -e "{}"'.format(logcat_cmd, regexp) logger.debug('logcat command ="{}"'.format(logcat_cmd)) self._logcat = self.target.background(logcat_cmd) while not self._stopped.is_set(): line = self._logcat.stdout.readline(1024) - self._add_line(line) + if line: + self._add_line(line) def stop(self): - # Popen can be stuck on readline() so send it a SIGKILL - self._logcat.terminate() + # Kill the underlying logcat process + # This will unblock self._logcat.stdout.readline() + host.kill_children(self._logcat.pid) + self._logcat.kill() self._stopped.set() self.join() @@ -609,9 +613,10 @@ class LogcatMonitor(threading.Thread): return res - def search(self, regexp, timeout=30): + def search(self, regexp): """ Search a line that matches a regexp in the logcat log + Return immediatly """ res = [] @@ -623,8 +628,18 @@ class LogcatMonitor(threading.Thread): if re.match(regexp, line): res.append(line) + return res + + def wait_for(self, regexp, timeout=30): + """ + Search a line that matches a regexp in the logcat log + Wait for it to appear if it's not found + """ + res = self.search(regexp) + # Found some matches, return them - if len(res) > 0: + # Also return if thread not running + if len(res) > 0 or not self.is_alive(): return res # Did not find any match, wait for one to pop up -- cgit v1.2.3 From 24d5630e54cb6af1915175e63f53a2d9b03f2b11 Mon Sep 17 00:00:00 2001 From: Valentin Schneider Date: Wed, 13 Sep 2017 11:47:51 +0100 Subject: utils/android: Add grant_app_permissions This is mostly useful to avoid having to manually click/tap on the permission requests that may pop up when opening apps, which would ruin automation --- devlib/utils/android.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 4916246..c5a79ad 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -445,6 +445,23 @@ def adb_command(device, command, timeout=None,adb_server=None): output, _ = check_output(full_command, timeout, shell=True) return output +def grant_app_permissions(target, package): + """ + Grant an app all the permissions it may ask for + """ + dumpsys = target.execute('dumpsys package {}'.format(package)) + + permissions = re.search( + 'requested permissions:\s*(?P(android.permission.+\s*)+)', dumpsys + ) + permissions = permissions.group('permissions').replace(" ", "").splitlines() + + for permission in permissions: + try: + target.execute('pm grant {} {}'.format(package, permission)) + except TargetError: + logger.debug('Cannot grant {}'.format(permission)) + # Messy environment initialisation stuff... -- cgit v1.2.3 From 90040e8b5877646ce111107f63b315a24768ae48 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 13 Sep 2017 14:03:59 +0100 Subject: utils/android: grant_app_permissions fix. Handle the case where an app does not specify any permissions. --- devlib/utils/android.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index c5a79ad..170fda0 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -454,6 +454,8 @@ def grant_app_permissions(target, package): permissions = re.search( 'requested permissions:\s*(?P(android.permission.+\s*)+)', dumpsys ) + if permissions is None: + return permissions = permissions.group('permissions').replace(" ", "").splitlines() for permission in permissions: -- cgit v1.2.3 From 1513db0951e51642fb62b890c903c810a61df010 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 10 May 2017 11:18:21 +0100 Subject: instrument: Clear up Instrument.reset semantics - Fix missing parameter in the documentation - Clarify meaning of `sites` and `kinds` in the documentation. - With the current implementation the `channels` argument is useless: if `sites` and `kinds` are not also specified then all channels are enabled anyway. Fix that by making those parameters ignored when `channels` is provided. --- devlib/instrument/__init__.py | 27 ++++++++++++++++----------- doc/instrumentation.rst | 19 +++++++++++++------ 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 9f8ac00..77ba1d3 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -258,24 +258,29 @@ class Instrument(object): pass def reset(self, sites=None, kinds=None, channels=None): + self.active_channels = [] if kinds is None and sites is None and channels is None: 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] - self.active_channels = [] - for chan_name in (channels or []): + elif channels is not None: + if sites is not None or kinds is not None: + raise ValueError( + 'sites and kinds should not be set if channels is set') + for chan_name in channels: try: self.active_channels.append(self.channels[chan_name]) except KeyError: msg = 'Unexpected channel "{}"; must be in {}' raise ValueError(msg.format(chan_name, self.channels.keys())) - for chan in self.channels.values(): - if (kinds is None or chan.kind in kinds) and \ - (sites is None or chan.site in sites): - self.active_channels.append(chan) + else: + if isinstance(sites, basestring): + sites = [sites] + if isinstance(kinds, basestring): + kinds = [kinds] + else: + for chan in self.channels.values(): + if (kinds is None or chan.kind in kinds) and \ + (sites is None or chan.site in sites): + self.active_channels.append(chan) # instantaneous diff --git a/doc/instrumentation.rst b/doc/instrumentation.rst index 76adf39..8aee1ce 100644 --- a/doc/instrumentation.rst +++ b/doc/instrumentation.rst @@ -99,14 +99,21 @@ Instrument ``teardown()`` has been called), but see documentation for the instrument you're interested in. -.. method:: Instrument.reset([sites, [kinds]]) +.. method:: Instrument.reset(sites=None, kinds=None, channels=None) This is used to configure an instrument for collection. This must be invoked - before ``start()`` is called to begin collection. ``sites`` and ``kinds`` - parameters may be used to specify which channels measurements should be - collected from (if omitted, then measurements will be collected for all - available sites/kinds). This methods sets the ``active_channels`` attribute - of the ``Instrument``. + before ``start()`` is called to begin collection. This methods sets the + ``active_channels`` attribute of the ``Instrument``. + + If ``channels`` is provided, it is a list of names of channels to enable and + ``sites`` and ``kinds`` must both be ``None``. + + Otherwise, if one of ``sites`` or ``kinds`` is provided, all channels + matching the given sites or kinds are enabled. If both are provided then all + channels of the given kinds at the given sites are enabled. + + If none of ``sites``, ``kinds`` or ``channels`` are provided then all + available channels are enabled. .. method:: Instrument.take_measurment() -- cgit v1.2.3 From ff366b3fd9d1a1d71186df78fc0ed8044a65d37c Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Mon, 21 Aug 2017 09:42:52 +0100 Subject: derived: renamed derived_measurments --> energy Renamed to reduce redandancy in import path. --- devlib/__init__.py | 2 +- devlib/derived/derived_measurements.py | 97 ---------------------------------- devlib/derived/energy.py | 97 ++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 98 deletions(-) delete mode 100644 devlib/derived/derived_measurements.py create mode 100644 devlib/derived/energy.py diff --git a/devlib/__init__.py b/devlib/__init__.py index b1b4fa3..19232c7 100644 --- a/devlib/__init__.py +++ b/devlib/__init__.py @@ -20,7 +20,7 @@ from devlib.instrument.netstats import NetstatsInstrument from devlib.instrument.gem5power import Gem5PowerInstrument from devlib.derived import DerivedMeasurements -from devlib.derived.derived_measurements import DerivedEnergyMeasurements +from devlib.derived.energy import DerivedEnergyMeasurements from devlib.trace.ftrace import FtraceCollector diff --git a/devlib/derived/derived_measurements.py b/devlib/derived/derived_measurements.py deleted file mode 100644 index 770db88..0000000 --- a/devlib/derived/derived_measurements.py +++ /dev/null @@ -1,97 +0,0 @@ -# Copyright 2013-2015 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. -# -from __future__ import division -from collections import defaultdict - -from devlib import DerivedMeasurements -from devlib.instrument import Measurement, MEASUREMENT_TYPES, InstrumentChannel - - -class DerivedEnergyMeasurements(DerivedMeasurements): - - @staticmethod - def process(measurements_csv): - - should_calculate_energy = [] - use_timestamp = False - - # Determine sites to calculate energy for - channel_map = defaultdict(list) - for channel in measurements_csv.channels: - channel_map[channel].append(channel.kind) - for channel, kinds in channel_map.iteritems(): - if 'power' in kinds and not 'energy' in kinds: - should_calculate_energy.append(channel.site) - if channel.site == 'timestamp': - use_timestamp = True - time_measurment = channel.measurement_type - - if measurements_csv.sample_rate_hz is None and not use_timestamp: - msg = 'Timestamp data is unavailable, please provide a sample rate' - raise ValueError(msg) - - if use_timestamp: - # Find index of timestamp column - ts_index = [i for i, chan in enumerate(measurements_csv.channels) - if chan.site == 'timestamp'] - if len(ts_index) > 1: - raise ValueError('Multiple timestamps detected') - ts_index = ts_index[0] - - row_ts = 0 - last_ts = 0 - energy_results = defaultdict(dict) - power_results = defaultdict(float) - - # Process data - for count, row in enumerate(measurements_csv.itermeasurements()): - if use_timestamp: - last_ts = row_ts - row_ts = time_measurment.convert(float(row[ts_index].value), 'time') - for entry in row: - channel = entry.channel - site = channel.site - if channel.kind == 'energy': - if count == 0: - energy_results[site]['start'] = entry.value - else: - energy_results[site]['end'] = entry.value - - if channel.kind == 'power': - power_results[site] += entry.value - - if site in should_calculate_energy: - if count == 0: - energy_results[site]['start'] = 0 - energy_results[site]['end'] = 0 - elif use_timestamp: - energy_results[site]['end'] += entry.value * (row_ts - last_ts) - else: - energy_results[site]['end'] += entry.value * (1 / - measurements_csv.sample_rate_hz) - - # Calculate final measurements - derived_measurements = [] - for site in energy_results: - total_energy = energy_results[site]['end'] - energy_results[site]['start'] - instChannel = InstrumentChannel('cum_energy', site, MEASUREMENT_TYPES['energy']) - derived_measurements.append(Measurement(total_energy, instChannel)) - - for site in power_results: - power = power_results[site] / (count + 1) #pylint: disable=undefined-loop-variable - instChannel = InstrumentChannel('avg_power', site, MEASUREMENT_TYPES['power']) - derived_measurements.append(Measurement(power, instChannel)) - - return derived_measurements diff --git a/devlib/derived/energy.py b/devlib/derived/energy.py new file mode 100644 index 0000000..770db88 --- /dev/null +++ b/devlib/derived/energy.py @@ -0,0 +1,97 @@ +# Copyright 2013-2015 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. +# +from __future__ import division +from collections import defaultdict + +from devlib import DerivedMeasurements +from devlib.instrument import Measurement, MEASUREMENT_TYPES, InstrumentChannel + + +class DerivedEnergyMeasurements(DerivedMeasurements): + + @staticmethod + def process(measurements_csv): + + should_calculate_energy = [] + use_timestamp = False + + # Determine sites to calculate energy for + channel_map = defaultdict(list) + for channel in measurements_csv.channels: + channel_map[channel].append(channel.kind) + for channel, kinds in channel_map.iteritems(): + if 'power' in kinds and not 'energy' in kinds: + should_calculate_energy.append(channel.site) + if channel.site == 'timestamp': + use_timestamp = True + time_measurment = channel.measurement_type + + if measurements_csv.sample_rate_hz is None and not use_timestamp: + msg = 'Timestamp data is unavailable, please provide a sample rate' + raise ValueError(msg) + + if use_timestamp: + # Find index of timestamp column + ts_index = [i for i, chan in enumerate(measurements_csv.channels) + if chan.site == 'timestamp'] + if len(ts_index) > 1: + raise ValueError('Multiple timestamps detected') + ts_index = ts_index[0] + + row_ts = 0 + last_ts = 0 + energy_results = defaultdict(dict) + power_results = defaultdict(float) + + # Process data + for count, row in enumerate(measurements_csv.itermeasurements()): + if use_timestamp: + last_ts = row_ts + row_ts = time_measurment.convert(float(row[ts_index].value), 'time') + for entry in row: + channel = entry.channel + site = channel.site + if channel.kind == 'energy': + if count == 0: + energy_results[site]['start'] = entry.value + else: + energy_results[site]['end'] = entry.value + + if channel.kind == 'power': + power_results[site] += entry.value + + if site in should_calculate_energy: + if count == 0: + energy_results[site]['start'] = 0 + energy_results[site]['end'] = 0 + elif use_timestamp: + energy_results[site]['end'] += entry.value * (row_ts - last_ts) + else: + energy_results[site]['end'] += entry.value * (1 / + measurements_csv.sample_rate_hz) + + # Calculate final measurements + derived_measurements = [] + for site in energy_results: + total_energy = energy_results[site]['end'] - energy_results[site]['start'] + instChannel = InstrumentChannel('cum_energy', site, MEASUREMENT_TYPES['energy']) + derived_measurements.append(Measurement(total_energy, instChannel)) + + for site in power_results: + power = power_results[site] / (count + 1) #pylint: disable=undefined-loop-variable + instChannel = InstrumentChannel('avg_power', site, MEASUREMENT_TYPES['power']) + derived_measurements.append(Measurement(power, instChannel)) + + return derived_measurements -- cgit v1.2.3 From dfd0b8ebd9a5f57aec6480ca6bd1d4d25763aedc Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Mon, 21 Aug 2017 11:01:42 +0100 Subject: MeasurementsCsv: rename itermeasurments Renamed to iter_measurments for readability. --- devlib/derived/energy.py | 2 +- devlib/instrument/__init__.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/devlib/derived/energy.py b/devlib/derived/energy.py index 770db88..b3f9c82 100644 --- a/devlib/derived/energy.py +++ b/devlib/derived/energy.py @@ -56,7 +56,7 @@ class DerivedEnergyMeasurements(DerivedMeasurements): power_results = defaultdict(float) # Process data - for count, row in enumerate(measurements_csv.itermeasurements()): + for count, row in enumerate(measurements_csv.iter_measurements()): if use_timestamp: last_ts = row_ts row_ts = time_measurment.convert(float(row[ts_index].value), 'time') diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 77ba1d3..cceada6 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -152,9 +152,9 @@ class MeasurementsCsv(object): self._load_channels() def measurements(self): - return list(self.itermeasurements()) + return list(self.iter_measurements()) - def itermeasurements(self): + def iter_measurements(self): self._fh.seek(0) reader = csv.reader(self._fh) reader.next() # headings -- cgit v1.2.3 From 15333eb09c3d74491fd37b54fd53552ff0fc17b8 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 22 Aug 2017 09:27:24 +0100 Subject: utils/misc: update CPU_PART_MAP with Mongoose Add Samsung's Mongoose M1 core part identifiers to the map. --- devlib/utils/misc.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/devlib/utils/misc.py b/devlib/utils/misc.py index f601686..8cfd59f 100644 --- a/devlib/utils/misc.py +++ b/devlib/utils/misc.py @@ -103,6 +103,9 @@ CPU_PART_MAP = { 0x211: {0x1: 'KryoGold'}, 0x800: {None: 'Falkor'}, }, + 0x53: { # Samsung LSI + 0x001: {0x1: 'MongooseM1'}, + }, 0x56: { # Marvell 0x131: { 0x2: 'Feroceon 88F6281', -- cgit v1.2.3 From 2afa8f86a4faca3d4c7a5d80eae3b46086c5fc6f Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 30 Aug 2017 14:27:03 +0100 Subject: insturment: add catergory for time + doc fix - Add a category name for time MeasurmentType's, as there are now multiple. - Fix the names of time_ms and time_us in the documentation. --- devlib/instrument/__init__.py | 6 +++--- doc/instrumentation.rst | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index cceada6..6d11eda 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -75,19 +75,19 @@ class MeasurementType(object): # Standard measures _measurement_types = [ MeasurementType('unknown', None), - MeasurementType('time', 'seconds', + MeasurementType('time', 'seconds', 'time', conversions={ 'time_us': lambda x: x * 1000000, 'time_ms': lambda x: x * 1000, } ), - MeasurementType('time_us', 'microseconds', + MeasurementType('time_us', 'microseconds', 'time', conversions={ 'time': lambda x: x / 1000000, 'time_ms': lambda x: x / 1000, } ), - MeasurementType('time_ms', 'milliseconds', + MeasurementType('time_ms', 'milliseconds', 'time', conversions={ 'time': lambda x: x / 1000, 'time_us': lambda x: x * 1000, diff --git a/doc/instrumentation.rst b/doc/instrumentation.rst index 8aee1ce..6f381b4 100644 --- a/doc/instrumentation.rst +++ b/doc/instrumentation.rst @@ -229,11 +229,11 @@ defined measurement types are +-------------+-------------+---------------+ | name | units | category | +=============+=============+===============+ -| time | seconds | | +| time | seconds | time | +-------------+-------------+---------------+ -| time | microseconds| | +| time_us | microseconds| time | +-------------+-------------+---------------+ -| time | milliseconds| | +| time_ms | milliseconds| time | +-------------+-------------+---------------+ | temperature | degrees | | +-------------+-------------+---------------+ -- cgit v1.2.3 From 823ce718bf0edc6baa99c2184fe479186771988f Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 30 Aug 2017 14:55:42 +0100 Subject: instrument: add get_raw() API Derived metrics may be calculated form data in raw output that is not present in the resulting MeasurementCSV. This adds a method to provide uniform access to raw artifacts generated by an instrument. --- devlib/instrument/__init__.py | 3 +++ devlib/instrument/acmecape.py | 3 +++ devlib/instrument/daq.py | 6 ++++++ devlib/instrument/energy_probe.py | 11 ++++++++--- devlib/instrument/frames.py | 10 +++++++--- doc/instrumentation.rst | 11 +++++++++-- 6 files changed, 36 insertions(+), 8 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 6d11eda..74113a3 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -297,3 +297,6 @@ class Instrument(object): def get_data(self, outfile): pass + + def get_raw(self): + return [] diff --git a/devlib/instrument/acmecape.py b/devlib/instrument/acmecape.py index e1bb6c1..1053c9d 100644 --- a/devlib/instrument/acmecape.py +++ b/devlib/instrument/acmecape.py @@ -121,3 +121,6 @@ class AcmeCapeInstrument(Instrument): output_row.append(float(row[i])/1000) writer.writerow(output_row) return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) + + def get_raw(self): + return [self.raw_data_file] diff --git a/devlib/instrument/daq.py b/devlib/instrument/daq.py index 75e854d..d497151 100644 --- a/devlib/instrument/daq.py +++ b/devlib/instrument/daq.py @@ -33,6 +33,7 @@ class DaqInstrument(Instrument): # pylint: disable=no-member super(DaqInstrument, self).__init__(target) self._need_reset = True + self._raw_files = [] if execute_command is None: raise HostError('Could not import "daqpower": {}'.format(import_error_mesg)) if labels is None: @@ -68,6 +69,7 @@ class DaqInstrument(Instrument): if not result.status == Status.OK: # pylint: disable=no-member raise HostError(result.message) self._need_reset = False + self._raw_files = [] def start(self): if self._need_reset: @@ -86,6 +88,7 @@ class DaqInstrument(Instrument): site = os.path.splitext(entry)[0] path = os.path.join(tempdir, entry) raw_file_map[site] = path + self._raw_files.append(path) active_sites = unique([c.site for c in self.active_channels]) file_handles = [] @@ -131,6 +134,9 @@ class DaqInstrument(Instrument): for fh in file_handles: fh.close() + def get_raw(self): + return self._raw_files + def teardown(self): self.execute('close') diff --git a/devlib/instrument/energy_probe.py b/devlib/instrument/energy_probe.py index 5f47430..c8f179e 100644 --- a/devlib/instrument/energy_probe.py +++ b/devlib/instrument/energy_probe.py @@ -52,6 +52,7 @@ class EnergyProbeInstrument(Instrument): self.raw_output_directory = None self.process = None self.sample_rate_hz = 10000 # Determined empirically + self.raw_data_file = None for label in self.labels: for kind in self.attributes: @@ -64,6 +65,7 @@ class EnergyProbeInstrument(Instrument): 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.raw_data_file = None def start(self): self.logger.debug(self.command) @@ -92,10 +94,10 @@ class EnergyProbeInstrument(Instrument): num_of_ports = len(self.resistor_values) struct_format = '{}I'.format(num_of_ports * self.attributes_per_sample) not_a_full_row_seen = False - raw_data_file = os.path.join(self.raw_output_directory, '0000000000') + self.raw_data_file = os.path.join(self.raw_output_directory, '0000000000') - self.logger.debug('Parsing raw data file: {}'.format(raw_data_file)) - with open(raw_data_file, 'rb') as bfile: + self.logger.debug('Parsing raw data file: {}'.format(self.raw_data_file)) + with open(self.raw_data_file, 'rb') as bfile: with open(outfile, 'wb') as wfh: writer = csv.writer(wfh) writer.writerow(active_channels) @@ -114,3 +116,6 @@ class EnergyProbeInstrument(Instrument): else: not_a_full_row_seen = True return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) + + def get_raw(self): + return [self.raw_data_file] diff --git a/devlib/instrument/frames.py b/devlib/instrument/frames.py index d1899fb..54869c1 100644 --- a/devlib/instrument/frames.py +++ b/devlib/instrument/frames.py @@ -20,6 +20,7 @@ class FramesInstrument(Instrument): self.collector = None self.header = None self._need_reset = True + self._raw_file = None self._init_channels() def reset(self, sites=None, kinds=None, channels=None): @@ -27,6 +28,7 @@ class FramesInstrument(Instrument): self.collector = self.collector_cls(self.target, self.period, self.collector_target, self.header) self._need_reset = False + self._raw_file = None def start(self): if self._need_reset: @@ -38,14 +40,16 @@ class FramesInstrument(Instrument): self._need_reset = True def get_data(self, outfile): - raw_outfile = None if self.keep_raw: - raw_outfile = outfile + '.raw' - self.collector.process_frames(raw_outfile) + self._raw_file = outfile + '.raw' + self.collector.process_frames(self._raw_file) active_sites = [chan.label for chan in self.active_channels] self.collector.write_frames(outfile, columns=active_sites) return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) + def get_raw(self): + return [self._raw_file] if self._raw_file else [] + def _init_channels(self): raise NotImplementedError() diff --git a/doc/instrumentation.rst b/doc/instrumentation.rst index 6f381b4..f23fc3e 100644 --- a/doc/instrumentation.rst +++ b/doc/instrumentation.rst @@ -65,8 +65,8 @@ Instrument :INSTANTANEOUS: The instrument supports taking a single sample via ``take_measurement()``. :CONTINUOUS: The instrument supports collecting measurements over a - period of time via ``start()``, ``stop()``, and - ``get_data()`` methods. + period of time via ``start()``, ``stop()``, ``get_data()``, + and (optionally) ``get_raw`` methods. .. note:: It's possible for one instrument to support more than a single mode. @@ -161,6 +161,13 @@ Instrument .. note:: This method is only implemented by :class:`Instrument`\ s that support ``CONTINUOUS`` measurement. +.. method:: Instrument.get_raw() + + Returns a list of paths to files containing raw output from the underlying + source(s) that is used to produce the data CSV. If now raw output is + generated or saved, an empty list will be returned. The format of the + contents of the raw files is entirely source-dependent. + .. attribute:: Instrument.sample_rate_hz Sample rate of the instrument in Hz. Assumed to be the same for all channels. -- cgit v1.2.3 From 9192deb8ee13a8105742631cece3b4be095c077f Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 31 Aug 2017 17:38:33 +0100 Subject: InstrumentChannel: name is now an alias for label In addition to a label constructed form the combination of site and measurment type, channels had a name that was specified on creation. This proven to be not particularly useful (there only being one instance of the name being set to something materially different from the label); and this has lead to channels being inconsistenly referenced (some times a channel is identified by its label, and sometimes by the name). This commit removes the name from __init__ arguments, and InstrumentChannel.name is now an alias for InstrumentChannel.label. --- devlib/instrument/__init__.py | 15 ++++++--------- devlib/instrument/hwmon.py | 2 +- devlib/platform/arm.py | 32 ++++++++++++++++---------------- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 74113a3..36bd4ed 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -175,14 +175,12 @@ class MeasurementsCsv(object): if entry.endswith(suffix): site = entry[:-len(suffix)] measure = mt - name = '{}_{}'.format(site, measure) break else: site = entry measure = 'unknown' - name = entry - chan = InstrumentChannel(name, site, measure) + chan = InstrumentChannel(site, measure) self.channels.append(chan) @@ -192,6 +190,8 @@ class InstrumentChannel(object): def label(self): return '{}_{}'.format(self.site, self.kind) + name = label + @property def kind(self): return self.measurement_type.name @@ -200,8 +200,7 @@ class InstrumentChannel(object): def units(self): return self.measurement_type.units - def __init__(self, name, site, measurement_type, **attrs): - self.name = name + def __init__(self, site, measurement_type, **attrs): self.site = site if isinstance(measurement_type, MeasurementType): self.measurement_type = measurement_type @@ -243,10 +242,8 @@ class Instrument(object): measure = measure.name return [c for c in self.list_channels() if c.kind == measure] - def add_channel(self, site, measure, name=None, **attrs): - if name is None: - name = '{}_{}'.format(site, measure) - chan = InstrumentChannel(name, site, measure, **attrs) + def add_channel(self, site, measure, **attrs): + chan = InstrumentChannel(site, measure, **attrs) self.channels[chan.label] = chan # initialization and teardown diff --git a/devlib/instrument/hwmon.py b/devlib/instrument/hwmon.py index ae49f40..5a9d8af 100644 --- a/devlib/instrument/hwmon.py +++ b/devlib/instrument/hwmon.py @@ -45,7 +45,7 @@ class HwmonInstrument(Instrument): measure = self.measure_map.get(ts.kind)[0] if measure: self.logger.debug('\tAdding sensor {}'.format(ts.name)) - self.add_channel(_guess_site(ts), measure, name=ts.name, sensor=ts) + self.add_channel(_guess_site(ts), measure, sensor=ts) else: self.logger.debug('\tSkipping sensor {} (unknown kind "{}")'.format(ts.name, ts.kind)) except ValueError: diff --git a/devlib/platform/arm.py b/devlib/platform/arm.py index e760eaf..76b58a4 100644 --- a/devlib/platform/arm.py +++ b/devlib/platform/arm.py @@ -210,22 +210,22 @@ class JunoEnergyInstrument(Instrument): mode = CONTINUOUS | INSTANTANEOUS _channels = [ - InstrumentChannel('sys_curr', 'sys', 'current'), - InstrumentChannel('a57_curr', 'a57', 'current'), - InstrumentChannel('a53_curr', 'a53', 'current'), - InstrumentChannel('gpu_curr', 'gpu', 'current'), - InstrumentChannel('sys_volt', 'sys', 'voltage'), - InstrumentChannel('a57_volt', 'a57', 'voltage'), - InstrumentChannel('a53_volt', 'a53', 'voltage'), - InstrumentChannel('gpu_volt', 'gpu', 'voltage'), - InstrumentChannel('sys_pow', 'sys', 'power'), - InstrumentChannel('a57_pow', 'a57', 'power'), - InstrumentChannel('a53_pow', 'a53', 'power'), - InstrumentChannel('gpu_pow', 'gpu', 'power'), - InstrumentChannel('sys_cenr', 'sys', 'energy'), - InstrumentChannel('a57_cenr', 'a57', 'energy'), - InstrumentChannel('a53_cenr', 'a53', 'energy'), - InstrumentChannel('gpu_cenr', 'gpu', 'energy'), + InstrumentChannel('sys', 'current'), + InstrumentChannel('a57', 'current'), + InstrumentChannel('a53', 'current'), + InstrumentChannel('gpu', 'current'), + InstrumentChannel('sys', 'voltage'), + InstrumentChannel('a57', 'voltage'), + InstrumentChannel('a53', 'voltage'), + InstrumentChannel('gpu', 'voltage'), + InstrumentChannel('sys', 'power'), + InstrumentChannel('a57', 'power'), + InstrumentChannel('a53', 'power'), + InstrumentChannel('gpu', 'power'), + InstrumentChannel('sys', 'energy'), + InstrumentChannel('a57', 'energy'), + InstrumentChannel('a53', 'energy'), + InstrumentChannel('gpu', 'energy'), ] def __init__(self, target): -- cgit v1.2.3 From 07ba177e58bfe5cd7e9a2788c519d0a173653fc9 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 09:52:20 +0100 Subject: InstrumentChannel: allow None sites Allow site to be None (still must be explicitly specified). If the site is None, the label if created using only the measurement type. --- devlib/instrument/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 36bd4ed..856a3af 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -188,7 +188,9 @@ class InstrumentChannel(object): @property def label(self): - return '{}_{}'.format(self.site, self.kind) + if self.site is not None: + return '{}_{}'.format(self.site, self.kind) + return self.kind name = label -- cgit v1.2.3 From 8479af48c46f76607ad6080daea7bc2b2e1863da Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 10:13:59 +0100 Subject: MeasurementCsv: various enhancements - Added values() and iter_values() methods. These return each row as a named tuple, with channel labels as the field names. - __cmp__ has been made more generic by checking wether other has "value" attribute, rather than wether it is an instance of Measurment. - MeasurementCsv no longer keeps an open handle to the file, and instead re-opens the file each time it needs it. This removes the need for managing the open handle, and alows parallel iterations over the values (each iteration will have it's own read handle into the files). --- devlib/instrument/__init__.py | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 856a3af..99f4e5a 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -127,7 +127,7 @@ class Measurement(object): self.channel = channel def __cmp__(self, other): - if isinstance(other, Measurement): + if hasattr(other, 'value'): return cmp(self.value, other.value) else: return cmp(self.value, other) @@ -147,26 +147,32 @@ class MeasurementsCsv(object): self.path = path self.channels = channels self.sample_rate_hz = sample_rate_hz - self._fh = open(path, 'rb') if self.channels is None: self._load_channels() + headings = [chan.label for chan in self.channels] + self.data_tuple = collections.namedtuple('csv_entry', headings) def measurements(self): return list(self.iter_measurements()) def iter_measurements(self): - self._fh.seek(0) - reader = csv.reader(self._fh) - reader.next() # headings - for row in reader: + for row in self._iter_rows(): values = map(numeric, row) yield [Measurement(v, c) for (v, c) in zip(values, self.channels)] + def values(self): + return list(self.iter_values()) + + def iter_values(self): + for row in self._iter_rows(): + values = map(numeric, row) + yield self.data_tuple(*values) + def _load_channels(self): - self._fh.seek(0) - reader = csv.reader(self._fh) - header = reader.next() - self._fh.seek(0) + header = [] + with open(self.path, 'rb') as fh: + reader = csv.reader(fh) + header = reader.next() self.channels = [] for entry in header: @@ -177,12 +183,23 @@ class MeasurementsCsv(object): measure = mt break else: - site = entry - measure = 'unknown' + if entry in MEASUREMENT_TYPES: + site = None + measure = entry + else: + site = entry + measure = 'unknown' chan = InstrumentChannel(site, measure) self.channels.append(chan) + def _iter_rows(self): + with open(self.path, 'rb') as fh: + reader = csv.reader(fh) + reader.next() # headings + for row in reader: + yield row + class InstrumentChannel(object): -- cgit v1.2.3 From dd26b43ac5237d967dca702deb5274a566a8e885 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 11:13:34 +0100 Subject: derived: DerivedMeasurments now return DerivedMetrics DerivedMeasurments processors now return DerviedMetrics rather than measurments. The notion of an InstrumentChannel doesn't really make sense in the context of DerivedMeasurments, which are not directly measured on the target. Since Measurement's require a channel, a simpler DerviedMetric is added that only requires a name and a type. --- devlib/__init__.py | 2 +- devlib/derived/__init__.py | 39 +++++++++++++++++++++++++++++++++++++++ devlib/derived/energy.py | 12 ++++++------ doc/derived_measurements.rst | 36 +++++++++++++++++++++++++++++++++--- 4 files changed, 79 insertions(+), 10 deletions(-) diff --git a/devlib/__init__.py b/devlib/__init__.py index 19232c7..42509f9 100644 --- a/devlib/__init__.py +++ b/devlib/__init__.py @@ -19,7 +19,7 @@ from devlib.instrument.monsoon import MonsoonInstrument from devlib.instrument.netstats import NetstatsInstrument from devlib.instrument.gem5power import Gem5PowerInstrument -from devlib.derived import DerivedMeasurements +from devlib.derived import DerivedMeasurements, DerivedMetric from devlib.derived.energy import DerivedEnergyMeasurements from devlib.trace.ftrace import FtraceCollector diff --git a/devlib/derived/__init__.py b/devlib/derived/__init__.py index 5689a58..5f7dc6e 100644 --- a/devlib/derived/__init__.py +++ b/devlib/derived/__init__.py @@ -12,6 +12,45 @@ # See the License for the specific language governing permissions and # limitations under the License. # + +from devlib.instrument import MeasurementType, MEASUREMENT_TYPES + + +class DerivedMetric(object): + + __slots__ = ['name', 'value', 'measurement_type'] + + @property + def units(self): + return self.measurement_type.units + + def __init__(self, name, value, measurement_type): + self.name = name + self.value = value + if isinstance(measurement_type, MeasurementType): + self.measurement_type = measurement_type + else: + try: + self.measurement_type = MEASUREMENT_TYPES[measurement_type] + except KeyError: + 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) + + __repr__ = __str__ + + class DerivedMeasurements(object): @staticmethod diff --git a/devlib/derived/energy.py b/devlib/derived/energy.py index b3f9c82..84d3d7c 100644 --- a/devlib/derived/energy.py +++ b/devlib/derived/energy.py @@ -15,8 +15,8 @@ from __future__ import division from collections import defaultdict -from devlib import DerivedMeasurements -from devlib.instrument import Measurement, MEASUREMENT_TYPES, InstrumentChannel +from devlib import DerivedMeasurements, DerivedMetric +from devlib.instrument import MEASUREMENT_TYPES, InstrumentChannel class DerivedEnergyMeasurements(DerivedMeasurements): @@ -86,12 +86,12 @@ class DerivedEnergyMeasurements(DerivedMeasurements): derived_measurements = [] for site in energy_results: total_energy = energy_results[site]['end'] - energy_results[site]['start'] - instChannel = InstrumentChannel('cum_energy', site, MEASUREMENT_TYPES['energy']) - derived_measurements.append(Measurement(total_energy, instChannel)) + name = '{}_total_energy'.format(site) + derived_measurements.append(DerivedMetric(name, total_energy, MEASUREMENT_TYPES['energy'])) for site in power_results: power = power_results[site] / (count + 1) #pylint: disable=undefined-loop-variable - instChannel = InstrumentChannel('avg_power', site, MEASUREMENT_TYPES['power']) - derived_measurements.append(Measurement(power, instChannel)) + name = '{}_average_power'.format(site) + derived_measurements.append(DerivedMetric(name, power, MEASUREMENT_TYPES['power'])) return derived_measurements diff --git a/doc/derived_measurements.rst b/doc/derived_measurements.rst index fcd497c..02a6daa 100644 --- a/doc/derived_measurements.rst +++ b/doc/derived_measurements.rst @@ -9,7 +9,7 @@ Example ------- The following example shows how to use an implementation of a -:class:`DerivedMeasurement` to obtain a list of calculated ``Measurements``. +:class:`DerivedMeasurement` to obtain a list of calculated ``DerivedMetric``'s. .. code-block:: ipython @@ -42,9 +42,39 @@ Derived Measurements .. method:: DerivedMeasurements.process(measurement_csv) - Returns a list of :class:`Measurement` objects that have been calculated. + Returns a list of :class:`DerivedMetric` objects that have been calculated. +Derived Metric +~~~~~~~~~~~~~~ + +.. class:: DerivedMetric + + Represents a metric derived from previously collected ``Measurement``s. + Unlike, a ``Measurement``, this was not measured directly from the target. + + +.. attribute:: DerivedMetric.name + + The name of the derived metric. This uniquely defines a metric -- two + ``DerivedMetric`` objects with the same ``name`` represent to instances of + the same metric (e.g. computed from two different inputs). + +.. attribute:: DerivedMetric.value + + The ``numeric`` value of the metric that has been computed for a particular + input. + +.. attribute:: DerivedMetric.measurement_type + + The ``MeasurementType`` of the metric. This indicates which conceptual + category the metric falls into, its units, and conversions to other + measurement types. + +.. attribute:: DerivedMetric.units + + The units in which the metric's value is expressed. + Available Derived Measurements ------------------------------- @@ -63,7 +93,7 @@ Available Derived Measurements .. method:: DerivedEnergyMeasurements.process(measurement_csv) - Returns a list of :class:`Measurement` objects that have been calculated for + Returns a list of :class:`DerivedMetric` objects that have been calculated for the average power and cumulative energy for each site. -- cgit v1.2.3 From adf25f93bb34eb8c293f80dc1ce3f82022dccfca Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 14:19:24 +0100 Subject: DerivedMeasurements: add process_raw() + doc updates - Add process_raw() method to the API. This is inteneded to be invoked on any raw output (i.e. not MeasurmentCsv) generated by an Instrument. - Both process() process_raw() are portional to be overriden by impolementation; the default behavior is to return an empty list. - The output specification for both is extened to allow MeasurementCsv's, as well as DerivedMetric's. - Documentation has been reworded for clarity. --- devlib/derived/__init__.py | 8 +++--- doc/derived_measurements.rst | 59 ++++++++++++++++++++++++++++++++------------ 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/devlib/derived/__init__.py b/devlib/derived/__init__.py index 5f7dc6e..24ac060 100644 --- a/devlib/derived/__init__.py +++ b/devlib/derived/__init__.py @@ -53,6 +53,8 @@ class DerivedMetric(object): class DerivedMeasurements(object): - @staticmethod - def process(measurements_csv): - raise NotImplementedError() + def process(self, measurements_csv): + return [] + + def process_raw(self, *args): + return [] diff --git a/doc/derived_measurements.rst b/doc/derived_measurements.rst index 02a6daa..285bce6 100644 --- a/doc/derived_measurements.rst +++ b/doc/derived_measurements.rst @@ -35,14 +35,29 @@ API Derived Measurements ~~~~~~~~~~~~~~~~~~~~ -.. class:: DerivedMeasurements() +.. class:: DerivedMeasurements - The ``DerivedMeasurements`` class is an abstract base for implementing - additional classes to calculate various metrics. + The ``DerivedMeasurements`` class provides an API for post-processing + instrument output offline (i.e. without a connection to the target device) to + generate additional metrics. .. method:: DerivedMeasurements.process(measurement_csv) - Returns a list of :class:`DerivedMetric` objects that have been calculated. + Process a :class:`MeasurementsCsv`, returning a list of + :class:`DerivedMetric` and/or :class:`MeasurementsCsv` objects that have been + derived from the input. The exact nature and ordering of the list memebers + is specific to indivial 'class'`DerivedMeasurements` implementations. + +.. method:: DerivedMeasurements.process_raw(\*args) + + Process raw output from an instrument, returnin a list :class:`DerivedMetric` + and/or :class:`MeasurementsCsv` objects that have been derived from the + input. The exact nature and ordering of the list memebers is specific to + indivial 'class'`DerivedMeasurements` implewmentations. + + The arguents to this method should be paths to raw output files generated by + an instrument. The number and order of expected arguments is specific to + particular implmentations. Derived Metric @@ -78,22 +93,34 @@ Derived Metric Available Derived Measurements ------------------------------- -.. class:: DerivedEnergyMeasurements() - The ``DerivedEnergyMeasurements`` class is used to calculate average power and - cumulative energy for each site if the required data is present. +.. note:: If a method of the API is not documented for a particular + implementation, that means that it s not overriden by that + implementation. It is still safe to call it -- an empty list will be + returned. - The calculation of cumulative energy can occur in 3 ways. If a - ``site`` contains ``energy`` results, the first and last measurements are extracted - and the delta calculated. If not, a ``timestamp`` channel will be used to calculate - the energy from the power channel, failing back to using the sample rate attribute - of the :class:`MeasurementCsv` file if timestamps are not available. If neither - timestamps or a sample rate are available then an error will be raised. +Energy +~~~~~~ +.. class:: DerivedEnergyMeasurements -.. method:: DerivedEnergyMeasurements.process(measurement_csv) + The ``DerivedEnergyMeasurements`` class is used to calculate average power and + cumulative energy for each site if the required data is present. - Returns a list of :class:`DerivedMetric` objects that have been calculated for - the average power and cumulative energy for each site. + The calculation of cumulative energy can occur in 3 ways. If a + ``site`` contains ``energy`` results, the first and last measurements are extracted + and the delta calculated. If not, a ``timestamp`` channel will be used to calculate + the energy from the power channel, failing back to using the sample rate attribute + of the :class:`MeasurementCsv` file if timestamps are not available. If neither + timestamps or a sample rate are available then an error will be raised. + + +.. method:: DerivedEnergyMeasurements.process(measurement_csv) + This will return total cumulative energy for each energy channel, and the + average power for each power channel in the input CSV. The output will contain + all energy metrics followed by power metrics. The ordering of both will match + the ordering of channels in the input. The metrics will by named based on the + sites of the coresponding channels according to the following patters: + ``"_total_energy"`` and ``"_average_power"``. -- cgit v1.2.3 From 01b5cffe03b68bb5cb74c4f344b0552e7cb75309 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 14:26:04 +0100 Subject: instrument: Update MeasurementType table - Add generic "count" and "percent" MeasurementType's. - Add "fps" MeasurementType. - Add "thermal" category for "termperature" - Add a comment describing each category --- devlib/instrument/__init__.py | 26 ++++++++++++++++++++++++-- doc/instrumentation.rst | 6 ++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 99f4e5a..0d2c1ed 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -72,9 +72,25 @@ class MeasurementType(object): return text.format(self.name, self.units) -# Standard measures +# Standard measures. In order to make sure that downstream data processing is not tied +# to particular insturments (e.g. a particular method of mearuing power), instruments +# must, where possible, resport their measurments formatted as on of the standard types +# defined here. _measurement_types = [ + # For whatever reason, the type of measurement could not be established. MeasurementType('unknown', None), + + # Generic measurements + MeasurementType('count', 'count'), + MeasurementType('percent', 'percent'), + + # Time measurement. While there is typically a single "canonical" unit + # used for each type of measurmenent, time may be measured to a wide variety + # of events occuring at a wide range of scales. Forcing everying into a + # single scale will lead to inefficient and awkward to work with result tables. + # Coversion functions between the formats are specified, so that downstream + # processors that expect all times time be at a particular scale can automatically + # covert without being familar with individual instruments. MeasurementType('time', 'seconds', 'time', conversions={ 'time_us': lambda x: x * 1000000, @@ -93,17 +109,23 @@ _measurement_types = [ 'time_us': lambda x: x * 1000, } ), - MeasurementType('temperature', 'degrees'), + # Measurements related to thermals. + MeasurementType('temperature', 'degrees', 'thermal'), + + # Measurements related to power end energy consumption. MeasurementType('power', 'watts', 'power/energy'), MeasurementType('voltage', 'volts', 'power/energy'), MeasurementType('current', 'amps', 'power/energy'), MeasurementType('energy', 'joules', 'power/energy'), + # Measurments realted to data transfer, e.g. neworking, + # memory, or backing storage. MeasurementType('tx', 'bytes', 'data transfer'), MeasurementType('rx', 'bytes', 'data transfer'), MeasurementType('tx/rx', 'bytes', 'data transfer'), + MeasurementType('fps', 'fps', 'ui render'), MeasurementType('frames', 'frames', 'ui render'), ] for m in _measurement_types: diff --git a/doc/instrumentation.rst b/doc/instrumentation.rst index f23fc3e..0d4a6ce 100644 --- a/doc/instrumentation.rst +++ b/doc/instrumentation.rst @@ -236,13 +236,15 @@ defined measurement types are +-------------+-------------+---------------+ | name | units | category | +=============+=============+===============+ -| time | seconds | time | +| count | count | | ++-------------+-------------+---------------+ +| percent | percent | | +-------------+-------------+---------------+ | time_us | microseconds| time | +-------------+-------------+---------------+ | time_ms | milliseconds| time | +-------------+-------------+---------------+ -| temperature | degrees | | +| temperature | degrees | thermal | +-------------+-------------+---------------+ | power | watts | power/energy | +-------------+-------------+---------------+ -- cgit v1.2.3 From a8ca0fc6c834a2e3e145e6d102fff9133cee2db6 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 14:34:55 +0100 Subject: util/rendering: add gfxinfo_get_last_dump Add gfxinfo_get_last_dump utility function to get the last gfxinfo dump from a (potentially large) file containing a concatenation of such dumps (as in the raw output of the GfxinfoFrames instrument). --- devlib/utils/rendering.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/devlib/utils/rendering.py b/devlib/utils/rendering.py index 3b7b6c4..9ab1e00 100644 --- a/devlib/utils/rendering.py +++ b/devlib/utils/rendering.py @@ -203,3 +203,43 @@ class GfxinfoFrameCollector(FrameCollector): if not found: logger.warning('Could not find frames data in gfxinfo output') return + + +def _file_reverse_iter(fh, buf_size=1024): + fh.seek(0, os.SEEK_END) + offset = 0 + file_size = remaining_size = fh.tell() + while remaining_size > 0: + offset = min(file_size, offset + buf_size) + fh.seek(file_size - offset) + buf = fh.read(min(remaining_size, buf_size)) + remaining_size -= buf_size + yield buf + + +def gfxinfo_get_last_dump(filepath): + """ + Return the last gfxinfo dump from the frame collector's raw output. + + """ + record = '' + with open(filepath, 'r') as fh: + fh_iter = _file_reverse_iter(fh) + try: + while True: + buf = fh_iter.next() + ix = buf.find('** Graphics') + if ix >= 0: + return buf[ix:] + record + + ix = buf.find(' **\n') + if ix >= 0: + buf = fh_iter.next() + buf + ix = buf.find('** Graphics') + if ix < 0: + msg = '"{}" appears to be corrupted' + raise RuntimeError(msg.format(filepath)) + return buf[ix:] + record + record = buf + record + except StopIteration: + pass -- cgit v1.2.3 From a290d2883512d50829afc38eec1ccd734ecee883 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 20 Sep 2017 13:02:44 +0100 Subject: utils/android: Start ADB server before listing devices Otherwise if the server isn't started we fail to parse the output of 'adb devices' --- devlib/utils/android.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 170fda0..286c3fe 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -286,6 +286,10 @@ def adb_get_device(timeout=None, adb_server=None): """ # TODO this is a hacky way to issue a adb command to all listed devices + # Ensure server is started so the 'daemon started successfully' message + # doesn't confuse the parsing below + adb_command(None, 'start-server', adb_server=adb_server) + # The output of calling adb devices consists of a heading line then # a list of the devices sperated by new line # The last line is a blank new line. in otherwords, if there is a device found -- cgit v1.2.3 From b3242a1ee4d16eb05b303f49538748f66e1b8dac Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 20 Sep 2017 13:40:36 +0100 Subject: utils/android: whitespace --- devlib/utils/android.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 170fda0..0123792 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -616,7 +616,7 @@ class LogcatMonitor(threading.Thread): with self._datalock: while not self._lines.empty(): self._lines.get() - + with open(self._logfile, 'w') as fh: pass -- cgit v1.2.3 From 8cf4a44bd74fefa78b25bc01a508482517adf2d1 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 20 Sep 2017 13:40:43 +0100 Subject: utils/android: Fix race condition in LogcatMonitor If you call .start then immediately call .stop, the thread may not yet have set ._logcat, resulting in an AttributeError. I initially fixed this by setting _logcat = None in __init__, then putting the `kill` calls inside `if self._logcat`. The problem with this, as pointed out by @valschneider, is that we can then have this sequence: main thread: monitor thread stop() run() if self._logcat: . # False, don't kill process . join() . self._logcat = <...> Therefore, just have the stop() method wait until the process is started before unconditionally killing it. --- devlib/utils/android.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 0123792..fd4c42e 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -545,6 +545,7 @@ class LogcatMonitor(threading.Thread): self.target = target + self._started = threading.Event() self._stopped = threading.Event() self._match_found = threading.Event() @@ -580,12 +581,17 @@ class LogcatMonitor(threading.Thread): logger.debug('logcat command ="{}"'.format(logcat_cmd)) self._logcat = self.target.background(logcat_cmd) + self._started.set() + while not self._stopped.is_set(): line = self._logcat.stdout.readline(1024) if line: self._add_line(line) def stop(self): + # Make sure we've started before we try to kill anything + self._started.wait() + # Kill the underlying logcat process # This will unblock self._logcat.stdout.readline() host.kill_children(self._logcat.pid) -- cgit v1.2.3 From 22c1f5e911356867c4feb419379b50a83e5317b6 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 20 Sep 2017 15:04:15 +0100 Subject: utils/android: Don't lock up if LogcatMonitor stopped before start Calling stop before start will result in hanging in self._started.wait(), because that event will never get set. Although stop before start is an illegal usage pattern, let's try not to fail annoyingly. --- devlib/utils/android.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index fd4c42e..bbc4b9d 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -589,6 +589,10 @@ class LogcatMonitor(threading.Thread): self._add_line(line) def stop(self): + if not self.is_alive(): + logger.warning('LogcatMonitor.stop called before start') + return + # Make sure we've started before we try to kill anything self._started.wait() -- cgit v1.2.3 From fb5a260f4b4879503bd5385f96a97612b0e49d13 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Thu, 21 Sep 2017 13:20:46 +0100 Subject: utils/anroid: Documentation for LogcatMonitor --- devlib/utils/android.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 170fda0..1f1f762 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -533,6 +533,16 @@ def _check_env(): aapt = _env.aapt class LogcatMonitor(threading.Thread): + """ + Helper class for monitoring Anroid's logcat + + :param target: Android target to monitor + :type target: :class:`AndroidTarget` + + device. Logcat entries that don't match any will not be + seen. If omitted, all entries will be sent to host. + :type regexps: list(str) + """ FLUSH_SIZE = 1000 @@ -556,6 +566,12 @@ class LogcatMonitor(threading.Thread): self._regexps = regexps def start(self, outfile=None): + """ + Start logcat and begin monitoring + + :param outfile: Optional path to file to store all logcat entries + :type outfile: str + """ if outfile: self._logfile = outfile else: @@ -653,6 +669,15 @@ class LogcatMonitor(threading.Thread): """ Search a line that matches a regexp in the logcat log Wait for it to appear if it's not found + + :param regexp: regexp to search + :type regexp: str + + :param timeout: Timeout in seconds, before rasing RuntimeError. + ``None`` means wait indefinitely + :type timeout: number + + :returns: List of matched strings """ res = self.search(regexp) -- cgit v1.2.3 From 6bb24aa12a8ca20c2803a8c3fb3c7fc2c3bd79e7 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Thu, 21 Sep 2017 13:30:33 +0100 Subject: hwmon: Disable if no permissions If we don't have permissions, scan() currently raises a TargetError. Instead we should return False from probe() so the module is disabled --- devlib/module/hwmon.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/devlib/module/hwmon.py b/devlib/module/hwmon.py index dc00442..06fa550 100644 --- a/devlib/module/hwmon.py +++ b/devlib/module/hwmon.py @@ -15,6 +15,7 @@ import re from collections import defaultdict +from devlib import TargetError from devlib.module import Module from devlib.utils.types import integer @@ -116,7 +117,15 @@ class HwmonModule(Module): @staticmethod def probe(target): - return target.file_exists(HWMON_ROOT) + if not target.file_exists(HWMON_ROOT): + return False + try: + target.list_directory(HWMON_ROOT, as_root=target.is_rooted) + except TargetError: + # Probably no permissions + return False + + return True @property def sensors(self): -- cgit v1.2.3 From 50dfb297cd1dfc2082a60a25e422796869b97186 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 13 Sep 2017 11:36:41 +0100 Subject: utils/rendering: fix surfaceflinger list SurfaceFlingerFrameCollector.list now converts line endings before splitting, so it now works when the endings are something other than "\r\n". --- devlib/utils/rendering.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/devlib/utils/rendering.py b/devlib/utils/rendering.py index 9ab1e00..a273cc7 100644 --- a/devlib/utils/rendering.py +++ b/devlib/utils/rendering.py @@ -122,7 +122,8 @@ class SurfaceFlingerFrameCollector(FrameCollector): return self.target.execute(cmd.format(activity)) def list(self): - return self.target.execute('dumpsys SurfaceFlinger --list').split('\r\n') + text = self.target.execute('dumpsys SurfaceFlinger --list') + return text.replace('\r\n', '\n').replace('\r', '\n').split('\n') def _process_raw_file(self, fh): text = fh.read().replace('\r\n', '\n').replace('\r', '\n') -- cgit v1.2.3 From d952abf52e4fbe1688b279a69ba3dee43897ed67 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 19 Sep 2017 13:48:08 +0100 Subject: utils/rendering: frame collectors should respect column order Previously FrameCollector.write_frames used "columns" argument only as a filter for which columns to write, but the order would always be the same as in raw output. The Instrument API requires that the column ordering in the resulting MeasurementsCsv matches the ordering of channels specified in reset() (if any). This means the collectors should respect the ordering specified in the "columns" parameter (which gets populated based on active channels). --- devlib/utils/rendering.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/devlib/utils/rendering.py b/devlib/utils/rendering.py index a273cc7..665135a 100644 --- a/devlib/utils/rendering.py +++ b/devlib/utils/rendering.py @@ -83,9 +83,14 @@ class FrameCollector(threading.Thread): header = self.header frames = self.frames else: - header = [c for c in self.header if c in columns] - indexes = [self.header.index(c) for c in header] + indexes = [] + for c in columns: + if c not in self.header: + msg = 'Invalid column "{}"; must be in {}' + raise ValueError(msg.format(c, self.header)) + indexes.append(self.header.index(c)) frames = [[f[i] for i in indexes] for f in self.frames] + header = columns with open(outfile, 'w') as wfh: writer = csv.writer(wfh) if header: -- cgit v1.2.3 From 96693a30354955b0037ba128bb0cf152e71020dd Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Fri, 22 Sep 2017 17:39:17 +0100 Subject: AndroidTarget: fix get_pid_of for recent Androids ps on recent Androids no longer takes an optional comm name; use Target.ps() instead, and filter by process name on the host. --- devlib/target.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index 51826fb..fca798c 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1011,11 +1011,12 @@ class AndroidTarget(Target): self.uninstall_executable(name) def get_pids_of(self, process_name): - result = self.execute('ps {}'.format(process_name[-15:]), check_exit_code=False).strip() - if result and 'not found' not in result: - return [int(x.split()[1]) for x in result.split('\n')[1:]] - else: - return [] + result = [] + search_term = process_name[-15:] + for entry in self.ps(): + if search_term in entry.name: + result.append(entry.pid) + return result def ps(self, **kwargs): lines = iter(convert_new_lines(self.execute('ps')).split('\n')) -- cgit v1.2.3 From 109fcc6deb79606faf197b58a131e977355afec6 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 26 Sep 2017 13:30:15 +0100 Subject: AndroidTarget: fix ps() ps() splits the output on whiestspace into fields; always expecting nine. In some cases, wchan field may be blank, resulting in only eight chunks after the split. Detect that case and insert and empty entry at the appropriate index. --- devlib/target.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index fca798c..4b2da42 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1024,8 +1024,12 @@ class AndroidTarget(Target): result = [] for line in lines: parts = line.split(None, 8) - if parts: - result.append(PsEntry(*(parts[0:1] + map(int, parts[1:5]) + parts[5:]))) + if not parts: + continue + if len(parts) == 8: + # wchan was blank; insert an empty field where it should be. + parts.insert(5, '') + result.append(PsEntry(*(parts[0:1] + map(int, parts[1:5]) + parts[5:]))) if not kwargs: return result else: -- cgit v1.2.3 From f692315d9c4de4e9d9693acf3304e5ed6d69efa8 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 17:36:18 +0100 Subject: derived: add DerivedGfxInfoStats Add DerivedGfxInfoStats that parse output from GfxInfoFramesInstrument to produce FPS data and rendering statistics. --- devlib/__init__.py | 1 + devlib/derived/fps.py | 137 +++++++++++++++++++++++++++++++++++++++++++ devlib/utils/rendering.py | 2 + doc/derived_measurements.rst | 55 +++++++++++++++++ 4 files changed, 195 insertions(+) create mode 100644 devlib/derived/fps.py diff --git a/devlib/__init__.py b/devlib/__init__.py index 42509f9..efcf0bc 100644 --- a/devlib/__init__.py +++ b/devlib/__init__.py @@ -21,6 +21,7 @@ from devlib.instrument.gem5power import Gem5PowerInstrument from devlib.derived import DerivedMeasurements, DerivedMetric from devlib.derived.energy import DerivedEnergyMeasurements +from devlib.derived.fps import DerivedGfxInfoStats from devlib.trace.ftrace import FtraceCollector diff --git a/devlib/derived/fps.py b/devlib/derived/fps.py new file mode 100644 index 0000000..31a5399 --- /dev/null +++ b/devlib/derived/fps.py @@ -0,0 +1,137 @@ +from __future__ import division +import csv +import os +import re + +try: + import pandas as pd +except ImportError: + pd = None + +from devlib import DerivedMeasurements, DerivedMetric, MeasurementsCsv, InstrumentChannel +from devlib.utils.rendering import gfxinfo_get_last_dump, VSYNC_INTERVAL +from devlib.utils.types import numeric + + +class DerivedFpsStats(DerivedMeasurements): + + def __init__(self, drop_threshold=5, suffix=None, filename=None, outdir=None): + self.drop_threshold = drop_threshold + self.suffix = suffix + self.filename = filename + self.outdir = outdir + if (filename is None) and (suffix is None): + self.suffix = '-fps' + elif (filename is not None) and (suffix is not None): + raise ValueError('suffix and filename cannot be specified at the same time.') + if filename is not None and os.sep in filename: + raise ValueError('filename cannot be a path (cannot countain "{}"'.format(os.sep)) + + def process(self, measurements_csv): + if isinstance(measurements_csv, basestring): + measurements_csv = MeasurementsCsv(measurements_csv) + if pd is not None: + return self._process_with_pandas(measurements_csv) + return self._process_without_pandas(measurements_csv) + + def _get_csv_file_name(self, frames_file): + outdir = self.outdir or os.path.dirname(frames_file) + if self.filename: + return os.path.join(outdir, self.filename) + + frames_basename = os.path.basename(frames_file) + rest, ext = os.path.splitext(frames_basename) + csv_basename = rest + self.suffix + ext + return os.path.join(outdir, csv_basename) + + +class DerivedGfxInfoStats(DerivedFpsStats): + + @staticmethod + def process_raw(filepath, *args): + metrics = [] + dump = gfxinfo_get_last_dump(filepath) + seen_stats = False + for line in dump.split('\n'): + if seen_stats and not line.strip(): + break + elif line.startswith('Janky frames:'): + text = line.split(': ')[-1] + val_text, pc_text = text.split('(') + metrics.append(DerivedMetric('janks', numeric(val_text.strip()), 'count')) + metrics.append(DerivedMetric('janks_pc', numeric(pc_text[:-3]), 'percent')) + elif ' percentile: ' in line: + ptile, val_text = line.split(' percentile: ') + name = 'render_time_{}_ptile'.format(ptile) + value = numeric(val_text.strip()[:-2]) + metrics.append(DerivedMetric(name, value, 'time_ms')) + elif line.startswith('Number '): + name_text, val_text = line.strip().split(': ') + name = name_text[7:].lower().replace(' ', '_') + value = numeric(val_text) + metrics.append(DerivedMetric(name, value, 'count')) + else: + continue + seen_stats = True + return metrics + + def _process_without_pandas(self, measurements_csv): + per_frame_fps = [] + start_vsync, end_vsync = None, None + frame_count = 0 + + for frame_data in measurements_csv.iter_values(): + if frame_data.Flags_flags != 0: + continue + frame_count += 1 + + if start_vsync is None: + start_vsync = frame_data.Vsync_time_us + end_vsync = frame_data.Vsync_time_us + + frame_time = frame_data.FrameCompleted_time_us - frame_data.IntendedVsync_time_us + pff = 1e9 / frame_time + if pff > self.drop_threshold: + per_frame_fps.append([pff]) + + if frame_count: + duration = end_vsync - start_vsync + fps = (1e9 * frame_count) / float(duration) + else: + duration = 0 + fps = 0 + + csv_file = self._get_csv_file_name(measurements_csv.path) + with open(csv_file, 'wb') as wfh: + writer = csv.writer(wfh) + writer.writerow(['fps']) + writer.writerows(per_frame_fps) + + return [DerivedMetric('fps', fps, 'fps'), + DerivedMetric('total_frames', frame_count, 'frames'), + MeasurementsCsv(csv_file)] + + 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 = (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] + fps = (1e9 * frame_count) / float(duration) + else: + duration = 0 + fps = 0 + + csv_file = self._get_csv_file_name(measurements_csv.path) + per_frame_fps.to_csv(csv_file, index=False, header=True) + + return [DerivedMetric('fps', fps, 'fps'), + DerivedMetric('total_frames', frame_count, 'frames'), + MeasurementsCsv(csv_file)] + diff --git a/devlib/utils/rendering.py b/devlib/utils/rendering.py index 665135a..7bb6f3a 100644 --- a/devlib/utils/rendering.py +++ b/devlib/utils/rendering.py @@ -18,6 +18,8 @@ logger = logging.getLogger('rendering') SurfaceFlingerFrame = namedtuple('SurfaceFlingerFrame', 'desired_present_time actual_present_time frame_ready_time') +VSYNC_INTERVAL = 16666667 + class FrameCollector(threading.Thread): diff --git a/doc/derived_measurements.rst b/doc/derived_measurements.rst index 285bce6..0a678bb 100644 --- a/doc/derived_measurements.rst +++ b/doc/derived_measurements.rst @@ -124,3 +124,58 @@ Energy sites of the coresponding channels according to the following patters: ``"_total_energy"`` and ``"_average_power"``. + +FPS / Rendering +~~~~~~~~~~~~~~~ + +.. class:: DerivedGfxInfoStats(drop_threshold=5, suffix='-fps', filename=None, outdir=None) + + Produces FPS (frames-per-second) and other dervied statistics from + :class:`GfxInfoFramesInstrument` output. This takes several optional + parameters in creation: + + :param drop_threshold: FPS in an application, such as a game, which this + processor is primarily targeted at, cannot reasonably + drop to a very low value. This is specified to this + threhold. If an FPS for a frame is computed to be + lower than this treshold, it will be dropped on the + assumption that frame rednering was suspended by the + system (e.g. when idling), or there was some sort of + error, and therefore this should be used in + performance calculations. defaults to ``5``. + :param suffix: The name of the gerated per-frame FPS csv file will be + derived from the input frames csv file by appending this + suffix. This cannot be specified at the same time as + a ``filename``. + :param filename: As an alternative to the suffix, a complete file name for + FPS csv can be specified. This cannot be used at the same + time as the ``suffix``. + :param outdir: By default, the FPS csv file will be placed in the same + directory as the input frames csv file. This can be changed + by specifying an alternate directory here + + .. warning:: Specifying both ``filename`` and ``oudir`` will mean that exactly + the same file will be used for FPS output on each invocation of + ``process()`` (even for different inputs) resulting in previous + results being overwritten. + +.. method:: DerivedGfxInfoStats.process(measurement_csv) + + Process the fames csv generated by :class:`GfxInfoFramesInstrument` and + returns a list containing exactly three entries: :class:`DerivedMetric`\ s + ``fps`` and ``total_frames``, followed by a :class:`MeasurentCsv` containing + per-frame FPSs values. + +.. method:: DerivedGfxInfoStats.process_raw(gfxinfo_frame_raw_file) + + As input, this takes a single argument, which should be the path to the raw + output file of :class:`GfxInfoFramesInstrument`. The returns stats + accumulated by gfxinfo. At the time of wrinting, the stats (in order) are: + ``janks``, ``janks_pc`` (percentage of all frames), + ``render_time_50th_ptile`` (50th percentile, or median, for time to render a + frame), ``render_time_90th_ptile``, ``render_time_95th_ptile``, + ``render_time_99th_ptile``, ``missed_vsync``, ``hight_input_latency``, + ``slow_ui_thread``, ``slow_bitmap_uploads``, ``slow_issue_draw_commands``. + Please see the `gfxinfo documentation`_ for details. + +.. _gfxinfo documentation: https://developer.android.com/training/testing/performance.html -- cgit v1.2.3 From 9f666320f38db60fdadb096b656076208faa915a Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 13 Sep 2017 11:45:55 +0100 Subject: derived: add DerivedSurfaceFlingerStats Add DerivedSurfaceFlingerStats that parse output from SurfaceFlingerFramesInstrument to produce FPS data and rendering statistics. --- devlib/__init__.py | 4 +-- devlib/derived/fps.py | 77 ++++++++++++++++++++++++++++++++++++++++++++ doc/derived_measurements.rst | 40 +++++++++++++++++++++++ 3 files changed, 119 insertions(+), 2 deletions(-) diff --git a/devlib/__init__.py b/devlib/__init__.py index efcf0bc..2b3f3b6 100644 --- a/devlib/__init__.py +++ b/devlib/__init__.py @@ -13,7 +13,7 @@ from devlib.instrument import Instrument, InstrumentChannel, Measurement, Measur from devlib.instrument import MEASUREMENT_TYPES, INSTANTANEOUS, CONTINUOUS from devlib.instrument.daq import DaqInstrument from devlib.instrument.energy_probe import EnergyProbeInstrument -from devlib.instrument.frames import GfxInfoFramesInstrument +from devlib.instrument.frames import GfxInfoFramesInstrument, SurfaceFlingerFramesInstrument from devlib.instrument.hwmon import HwmonInstrument from devlib.instrument.monsoon import MonsoonInstrument from devlib.instrument.netstats import NetstatsInstrument @@ -21,7 +21,7 @@ from devlib.instrument.gem5power import Gem5PowerInstrument from devlib.derived import DerivedMeasurements, DerivedMetric from devlib.derived.energy import DerivedEnergyMeasurements -from devlib.derived.fps import DerivedGfxInfoStats +from devlib.derived.fps import DerivedGfxInfoStats, DerivedSurfaceFlingerStats from devlib.trace.ftrace import FtraceCollector diff --git a/devlib/derived/fps.py b/devlib/derived/fps.py index 31a5399..e595926 100644 --- a/devlib/derived/fps.py +++ b/devlib/derived/fps.py @@ -9,6 +9,7 @@ except ImportError: pd = None from devlib import DerivedMeasurements, DerivedMetric, MeasurementsCsv, InstrumentChannel +from devlib.exception import HostError from devlib.utils.rendering import gfxinfo_get_last_dump, VSYNC_INTERVAL from devlib.utils.types import numeric @@ -135,3 +136,79 @@ class DerivedGfxInfoStats(DerivedFpsStats): DerivedMetric('total_frames', frame_count, 'frames'), MeasurementsCsv(csv_file)] + +class DerivedSurfaceFlingerStats(DerivedFpsStats): + + def _process_with_pandas(self, measurements_csv): + data = pd.read_csv(measurements_csv.path) + + # fiter out bogus frames. + bogus_frames_filter = data.actual_present_time_us != 0x7fffffffffffffff + actual_present_times = data.actual_present_time_us[bogus_frames_filter] + actual_present_time_deltas = actual_present_times.diff().dropna() + + vsyncs_to_compose = actual_present_time_deltas.div(VSYNC_INTERVAL) + vsyncs_to_compose.apply(lambda x: int(round(x, 0))) + + # drop values lower than drop_threshold FPS as real in-game frame + # rate is unlikely to drop below that (except on loading screens + # etc, which should not be factored in frame rate calculation). + per_frame_fps = (1.0 / (vsyncs_to_compose.multiply(VSYNC_INTERVAL / 1e9))) + keep_filter = per_frame_fps > self.drop_threshold + filtered_vsyncs_to_compose = vsyncs_to_compose[keep_filter] + per_frame_fps.name = 'fps' + + csv_file = self._get_csv_file_name(measurements_csv.path) + per_frame_fps.to_csv(csv_file, index=False, header=True) + + if not filtered_vsyncs_to_compose.empty: + fps = 0 + total_vsyncs = filtered_vsyncs_to_compose.sum() + frame_count = filtered_vsyncs_to_compose.size + + if total_vsyncs: + fps = 1e9 * frame_count / (VSYNC_INTERVAL * total_vsyncs) + + janks = self._calc_janks(filtered_vsyncs_to_compose) + not_at_vsync = self._calc_not_at_vsync(vsyncs_to_compose) + else: + fps = 0 + frame_count = 0 + janks = 0 + not_at_vsync = 0 + + return [DerivedMetric('fps', fps, 'fps'), + DerivedMetric('total_frames', frame_count, 'frames'), + MeasurementsCsv(csv_file), + DerivedMetric('janks', janks, 'count'), + DerivedMetric('janks_pc', janks * 100 / frame_count, 'percent'), + DerivedMetric('missed_vsync', not_at_vsync, 'count')] + + 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. + raise HostError('Please install "pandas" Python package to process SurfaceFlinger frames') + + @staticmethod + def _calc_janks(filtered_vsyncs_to_compose): + """ + Internal method for calculating jank frames. + """ + pause_latency = 20 + vtc_deltas = filtered_vsyncs_to_compose.diff().dropna() + vtc_deltas = vtc_deltas.abs() + janks = vtc_deltas.apply(lambda x: (pause_latency > x > 1.5) and 1 or 0).sum() + + return janks + + @staticmethod + def _calc_not_at_vsync(vsyncs_to_compose): + """ + Internal method for calculating the number of frames that did not + render in a single vsync cycle. + """ + epsilon = 0.0001 + func = lambda x: (abs(x - 1.0) > epsilon) and 1 or 0 + not_at_vsync = vsyncs_to_compose.apply(func).sum() + + return not_at_vsync diff --git a/doc/derived_measurements.rst b/doc/derived_measurements.rst index 0a678bb..d56f94b 100644 --- a/doc/derived_measurements.rst +++ b/doc/derived_measurements.rst @@ -179,3 +179,43 @@ FPS / Rendering Please see the `gfxinfo documentation`_ for details. .. _gfxinfo documentation: https://developer.android.com/training/testing/performance.html + + +.. class:: DerivedSurfaceFlingerStats(drop_threshold=5, suffix='-fps', filename=None, outdir=None) + + Produces FPS (frames-per-second) and other dervied statistics from + :class:`SurfaceFlingerFramesInstrument` output. This takes several optional + parameters in creation: + + :param drop_threshold: FPS in an application, such as a game, which this + processor is primarily targeted at, cannot reasonably + drop to a very low value. This is specified to this + threhold. If an FPS for a frame is computed to be + lower than this treshold, it will be dropped on the + assumption that frame rednering was suspended by the + system (e.g. when idling), or there was some sort of + error, and therefore this should be used in + performance calculations. defaults to ``5``. + :param suffix: The name of the gerated per-frame FPS csv file will be + derived from the input frames csv file by appending this + suffix. This cannot be specified at the same time as + a ``filename``. + :param filename: As an alternative to the suffix, a complete file name for + FPS csv can be specified. This cannot be used at the same + time as the ``suffix``. + :param outdir: By default, the FPS csv file will be placed in the same + directory as the input frames csv file. This can be changed + by specifying an alternate directory here + + .. warning:: Specifying both ``filename`` and ``oudir`` will mean that exactly + the same file will be used for FPS output on each invocation of + ``process()`` (even for different inputs) resulting in previous + results being overwritten. + +.. method:: DerivedSurfaceFlingerStats.process(measurement_csv) + + Process the fames csv generated by :class:`SurfaceFlingerFramesInstrument` and + returns a list containing exactly three entries: :class:`DerivedMetric`\ s + ``fps`` and ``total_frames``, followed by a :class:`MeasurentCsv` containing + per-frame FPSs values, followed by ``janks`` ``janks_pc``, and + ``missed_vsync`` metrics. -- cgit v1.2.3 From 4593d8605de3edaf5210c18a4b3d41f5884a0267 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Fri, 22 Sep 2017 13:46:32 +0100 Subject: gfxinfo fixes - Make sure timestamps are actually reported in microseconds. - Eliminate duplicate entries from successive dumps --- devlib/derived/fps.py | 4 ++-- devlib/instrument/frames.py | 1 + devlib/utils/rendering.py | 7 ++++++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/devlib/derived/fps.py b/devlib/derived/fps.py index e595926..2695c4c 100644 --- a/devlib/derived/fps.py +++ b/devlib/derived/fps.py @@ -97,7 +97,7 @@ class DerivedGfxInfoStats(DerivedFpsStats): if frame_count: duration = end_vsync - start_vsync - fps = (1e9 * frame_count) / float(duration) + fps = (1e6 * frame_count) / float(duration) else: duration = 0 fps = 0 @@ -116,7 +116,7 @@ class DerivedGfxInfoStats(DerivedFpsStats): 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 = (1e9 / frame_time) + per_frame_fps = (1e6 / frame_time) keep_filter = per_frame_fps > self.drop_threshold per_frame_fps = per_frame_fps[keep_filter] per_frame_fps.name = 'fps' diff --git a/devlib/instrument/frames.py b/devlib/instrument/frames.py index 54869c1..a2e06b3 100644 --- a/devlib/instrument/frames.py +++ b/devlib/instrument/frames.py @@ -1,3 +1,4 @@ +from __future__ import division from devlib.instrument import (Instrument, CONTINUOUS, MeasurementsCsv, MeasurementType) from devlib.utils.rendering import (GfxinfoFrameCollector, diff --git a/devlib/utils/rendering.py b/devlib/utils/rendering.py index 7bb6f3a..6c3909d 100644 --- a/devlib/utils/rendering.py +++ b/devlib/utils/rendering.py @@ -195,6 +195,7 @@ class GfxinfoFrameCollector(FrameCollector): def _process_raw_file(self, fh): found = False try: + last_vsync = 0 while True: for line in fh: if line.startswith('---PROFILEDATA---'): @@ -205,7 +206,11 @@ class GfxinfoFrameCollector(FrameCollector): for line in fh: if line.startswith('---PROFILEDATA---'): break - self.frames.append(map(int, line.strip().split(',')[:-1])) # has a trailing ',' + entries = map(int, line.strip().split(',')[:-1]) # has a trailing ',' + if entries[1] <= last_vsync: + continue # repeat frame + last_vsync = entries[1] + self.frames.append(entries) except StopIteration: pass if not found: -- cgit v1.2.3 From 865cf9598537f5a405e8ada5ffed91daae9ac414 Mon Sep 17 00:00:00 2001 From: Suren Baghdasaryan Date: Thu, 31 Aug 2017 13:28:47 -0700 Subject: utils/android_build: Remove methods to build kernel Kernel build process will be described in an external script referenced from board config file. Change-Id: Ied74824996a4121cdfded55b283e196e8193c6bc Signed-off-by: Suren Baghdasaryan --- devlib/utils/android_build.py | 70 +++++-------------------------------------- 1 file changed, 8 insertions(+), 62 deletions(-) diff --git a/devlib/utils/android_build.py b/devlib/utils/android_build.py index da89dbb..de638ba 100644 --- a/devlib/utils/android_build.py +++ b/devlib/utils/android_build.py @@ -17,16 +17,11 @@ import logging import os +import shutil +import subprocess from collections import namedtuple class Build(object): - BuildParams = namedtuple("BuildParams", "arch defconfig cross_compile") - - device_kernel_build_params = { - 'hikey960': BuildParams('arm64', 'hikey960_defconfig', 'aarch64-linux-android-'), - 'hikey': BuildParams('arm64', 'hikey_defconfig', 'aarch64-linux-android-') - } - """ Collection of Android related build actions """ @@ -37,6 +32,11 @@ class Build(object): te._log.warning('Build initialization failed: invalid paramterers') raise + def exec_cmd(self, cmd): + ret = subprocess.call(cmd, shell=True) + if ret != 0: + raise RuntimeError('Command \'{}\' returned error code {}'.format(cmd, ret)) + def build_module(self, module_path): """ Build a module and its dependencies. @@ -45,66 +45,12 @@ class Build(object): :type module_path: str """ - self._te._log.info('BUILDING module %s', module_path) cur_dir = os.getcwd() os.chdir(self._te.ANDROID_BUILD_TOP) lunch_target = self._te.TARGET_PRODUCT + '-' + self._te.TARGET_BUILD_VARIANT - os.system('source build/envsetup.sh && lunch ' + + self.exec_cmd('source build/envsetup.sh && lunch ' + lunch_target + ' && mmma -j16 ' + module_path) os.chdir(cur_dir) - def build_kernel(self, kernel_path, arch, defconfig, cross_compile, clean=False): - """ - Build kernel. - - :param kernel_path: path to kernel sources - :type kernel_path: str - - :param arch: device architecture string used in ARCH command line parameter - :type arch: str - - :param defconfig: defconfig file name - :type defconfig: str - - :param cross_compile: compiler string used in CROSS_COMPILE command line parameter - :type cross_compile: str - - :param clean: flag to clean the previous build - :type clean: boolean - - """ - self._te._log.info('BUILDING kernel @ %s', kernel_path) - cur_dir = os.getcwd() - os.chdir(kernel_path) - os.system('. ./build.config') - if clean: - os.system('make clean') - os.system('make ARCH=' + arch + ' ' + defconfig) - os.system('make -j24 ARCH=' + arch + ' CROSS_COMPILE=' + cross_compile) - os.chdir(cur_dir) - - def build_kernel_for_device(self, kernel_path, device_name, clean): - """ - Build kernel for specified device. - - :param kernel_path: path to kernel sources - :type kernel_path: str - - :param device_name: device name - :type device_name: str - - :param clean: flag to clean the previous build - :type clean: boolean - - """ - if not device_name in self.device_kernel_build_params: - return False - - params = self.device_kernel_build_params[device_name] - self.build_kernel(kernel_path, params.arch, - params.defconfig, params.cross_compile, clean) - return True - - # vim :set tabstop=4 shiftwidth=4 expandtab -- cgit v1.2.3 From bfb472171560f7bbdad0bfbafb6da7afeaedacb1 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 3 Oct 2017 16:47:11 +0100 Subject: Target: ensure shutils is always set Make sure shutils is always set, even if setup() has not been called by initializing it on first access if necessary. --- devlib/target.py | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index 4b2da42..f84c6a1 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -149,6 +149,12 @@ class Target(object): else: return None + @property + def shutils(self): + if self._shutils is None: + self._setup_shutils() + return self._shutils + def __init__(self, connection_settings=None, platform=None, @@ -189,6 +195,7 @@ class Target(object): self._installed_modules = {} self._cache = {} self._connections = {} + self._shutils = None self.busybox = None if load_default_modules: @@ -229,20 +236,7 @@ class Target(object): self.execute('mkdir -p {}'.format(self.executables_directory)) self.busybox = self.install(os.path.join(PACKAGE_BIN_DIRECTORY, self.abi, 'busybox')) - # Setup shutils script for the target - shutils_ifile = os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils.in') - shutils_ofile = os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils') - shell_path = '/bin/sh' - if self.os == 'android': - shell_path = '/system/bin/sh' - with open(shutils_ifile) as fh: - lines = fh.readlines() - with open(shutils_ofile, 'w') as ofile: - for line in lines: - line = line.replace("__DEVLIB_SHELL__", shell_path) - line = line.replace("__DEVLIB_BUSYBOX__", self.busybox) - ofile.write(line) - self.shutils = self.install(os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils')) + self._setup_shutils() for host_exe in (executables or []): # pylint: disable=superfluous-parens self.install(host_exe) @@ -622,6 +616,21 @@ class Target(object): # internal methods + def _setup_shutils(self): + shutils_ifile = os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils.in') + shutils_ofile = os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils') + shell_path = '/bin/sh' + if self.os == 'android': + shell_path = '/system/bin/sh' + with open(shutils_ifile) as fh: + lines = fh.readlines() + with open(shutils_ofile, 'w') as ofile: + for line in lines: + line = line.replace("__DEVLIB_SHELL__", shell_path) + line = line.replace("__DEVLIB_BUSYBOX__", self.busybox) + ofile.write(line) + self._shutils = self.install(os.path.join(PACKAGE_BIN_DIRECTORY, 'scripts', 'shutils')) + def _execute_util(self, command, timeout=None, check_exit_code=True, as_root=False): command = '{} {}'.format(self.shutils, command) return self.conn.execute(command, timeout, check_exit_code, as_root) -- cgit v1.2.3 From 92fb54d57b1c0d221101fae4c6aeb56a5948b3e2 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 3 Oct 2017 16:50:23 +0100 Subject: module: nicer logger name Use Module.name rather than Module.__name__ to name the logger for slightly more readable log output. --- devlib/module/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/module/__init__.py b/devlib/module/__init__.py index 5cf0a43..38a2315 100644 --- a/devlib/module/__init__.py +++ b/devlib/module/__init__.py @@ -56,7 +56,7 @@ class Module(object): def __init__(self, target): self.target = target - self.logger = logging.getLogger(self.__class__.__name__) + self.logger = logging.getLogger(self.name) class HardRestModule(Module): # pylint: disable=R0921 -- cgit v1.2.3 From 181bc180c4f9f13208e4e19e3f2ed2d1013b7db1 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 3 Oct 2017 16:28:09 +0100 Subject: Target: add read_tree_values and read_tree_values_flat Add two new methods to target that allow querying values of all sysfs nodes in a sub-directory structure all at once. The difference is that read_tree_values_flat returns a flat dict of path->value mappings; read_tree_values returns a nested dict structure that mimics the scanned sub-directories tree. --- devlib/bin/scripts/shutils.in | 19 +++++++++++++++++++ devlib/target.py | 43 +++++++++++++++++++++++++++++++++++++++++++ doc/target.rst | 26 ++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/devlib/bin/scripts/shutils.in b/devlib/bin/scripts/shutils.in index 6d78be7..004030d 100755 --- a/devlib/bin/scripts/shutils.in +++ b/devlib/bin/scripts/shutils.in @@ -195,6 +195,22 @@ cgroups_freezer_set_state() { exit 1 } +################################################################################ +# Misc +################################################################################ + +read_tree_values() { + PATH=$1 + MAXDEPTH=$2 + + PATHS=$($BUSYBOX find $PATH -follow -maxdepth $MAXDEPTH) + if [ ${#PATHS[@]} -eq 0 ]; then + echo "ERROR: '$1' does not exist" + else + $BUSYBOX grep -s '' $PATHS + fi +} + ################################################################################ # Main Function Dispatcher ################################################################################ @@ -236,6 +252,9 @@ cgroups_freezer_set_state) ftrace_get_function_stats) ftrace_get_function_stats ;; +read_tree_values) + read_tree_values $* + ;; *) echo "Command [$CMD] not supported" exit -1 diff --git a/devlib/target.py b/devlib/target.py index f84c6a1..8609fec 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -614,6 +614,20 @@ class Target(object): timeout = duration + 10 self.execute('sleep {}'.format(duration), timeout=timeout) + def read_tree_values_flat(self, path, depth=1, check_exit_code=True): + command = 'read_tree_values {} {}'.format(path, depth) + output = self._execute_util(command, as_root=self.is_rooted, + check_exit_code=check_exit_code) + result = {} + for entry in output.strip().split('\n'): + path, value = entry.strip().split(':', 1) + result[path] = value + return result + + def read_tree_values(self, path, depth=1, dictcls=dict, check_exit_code=True): + value_map = self.read_tree_values_flat(path, depth, check_exit_code) + return _build_path_tree(value_map, path, self.path.sep, dictcls) + # internal methods def _setup_shutils(self): @@ -1558,3 +1572,32 @@ def _get_part_name(section): if name is None: name = '{}/{}/{}'.format(implementer, part, variant) return name + + +def _build_path_tree(path_map, basepath, sep=os.path.sep, dictcls=dict): + """ + Convert a flat mapping of paths to values into a nested structure of + dict-line object (``dict``'s by default), mirroring the directory hierarchy + represented by the paths relative to ``basepath``. + + """ + def process_node(node, path, value): + parts = path.split(sep, 1) + if len(parts) == 1: # leaf + node[parts[0]] = value + else: # branch + if parts[0] not in node: + node[parts[0]] = dictcls() + process_node(node[parts[0]], parts[1], value) + + relpath_map = {os.path.relpath(p, basepath): v + for p, v in path_map.iteritems()} + + if len(relpath_map) == 1 and relpath_map.keys()[0] == '.': + result = relpath_map.values()[0] + else: + result = dictcls() + for path, value in relpath_map.iteritems(): + process_node(result, path, value) + + return result diff --git a/doc/target.rst b/doc/target.rst index 08472e2..ae6ddb0 100644 --- a/doc/target.rst +++ b/doc/target.rst @@ -327,6 +327,32 @@ 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): + + Read values of all sysfs (or similar) file nodes under ``path``, traversing + up to the maximum depth ``depth``. + + Returns a nested structure of dict-like objects (``dict``\ s by default) that + follows the structure of the scanned sub-directory tree. The top-level entry + has a single item who's key is ``path``. If ``path`` points to a single file, + the value of the entry is the value ready from that file node. Otherwise, the + 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. + + :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. + +.. method:: Target.read_tree_values_flat(path, depth=1): + + Read values of all sysfs (or similar) file nodes under ``path``, traversing + up to the maximum depth ``depth``. + + Returns a dict mapping paths of file nodes to corresponding values. + + :param path: sysfs path to scan + :param depth: maximum depth to descend + .. method:: Target.reset() Soft reset the target. Typically, this means executing ``reboot`` on the -- cgit v1.2.3 From 5a599f91db437d3a95f3ae15e1113704418fa9c1 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 3 Oct 2017 16:52:40 +0100 Subject: module/hwmon: optimize initialization. Optimize hwmon module initialization by using the new Target.grep_values call to get information about all HWMON devices in a single call to the target. --- devlib/module/hwmon.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/devlib/module/hwmon.py b/devlib/module/hwmon.py index 06fa550..d04bce7 100644 --- a/devlib/module/hwmon.py +++ b/devlib/module/hwmon.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # +import os import re from collections import defaultdict @@ -78,16 +79,15 @@ class HwmonDevice(object): all_sensors.extend(sensors_of_kind.values()) return all_sensors - def __init__(self, target, path): + def __init__(self, target, path, name, fields): self.target = target self.path = path - self.name = self.target.read_value(self.target.path.join(self.path, 'name')) + self.name = name self._sensors = defaultdict(dict) path = self.path if not path.endswith(self.target.path.sep): path += self.target.path.sep - for entry in self.target.list_directory(path, - as_root=self.target.is_rooted): + for entry in fields: match = HWMON_FILE_REGEX.search(entry) if match: kind = match.group('kind') @@ -117,14 +117,11 @@ class HwmonModule(Module): @staticmethod def probe(target): - if not target.file_exists(HWMON_ROOT): - return False try: target.list_directory(HWMON_ROOT, as_root=target.is_rooted) except TargetError: - # Probably no permissions + # Doesn't exist or no permissions return False - return True @property @@ -141,11 +138,13 @@ class HwmonModule(Module): self.scan() def scan(self): - for entry in self.target.list_directory(self.root, - as_root=self.target.is_rooted): - if entry.startswith('hwmon'): - entry_path = self.target.path.join(self.root, entry) - if self.target.file_exists(self.target.path.join(entry_path, 'name')): - device = HwmonDevice(self.target, entry_path) - self.devices.append(device) + values_tree = self.target.read_tree_values(self.root, depth=3) + for entry_id, fields in values_tree.iteritems(): + path = self.target.path.join(self.root, entry_id) + name = fields.pop('name', None) + if name is None: + continue + self.logger.debug('Adding device {}'.format(name)) + device = HwmonDevice(self.target, path, name, fields) + self.devices.append(device) -- cgit v1.2.3 From d7ca39e4d13ea1bebb12cd27ae09835f683ed327 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 4 Oct 2017 09:28:09 +0100 Subject: module/cpuidle: optimize initialization. Optimize cpuidle module initialization by using the new Target.grep_values call to get information about all idle states in a single call to the target during module intialization, rather lazily fetching them from the target afterwards. --- devlib/module/cpuidle.py | 100 +++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 52 deletions(-) diff --git a/devlib/module/cpuidle.py b/devlib/module/cpuidle.py index fd986c0..b17f1f5 100644 --- a/devlib/module/cpuidle.py +++ b/devlib/module/cpuidle.py @@ -41,51 +41,17 @@ class CpuidleState(object): raise ValueError('invalid idle state name: "{}"'.format(self.id)) return int(self.id[i:]) - def __init__(self, target, index, path): + def __init__(self, target, index, path, name, desc, power, latency, residency): self.target = target self.index = index self.path = path + self.name = name + self.desc = desc + self.power = power + self.latency = latency self.id = self.target.path.basename(self.path) self.cpu = self.target.path.basename(self.target.path.dirname(path)) - @property - @memoized - def desc(self): - return self.get('desc') - - @property - @memoized - def name(self): - return self.get('name') - - @property - @memoized - def latency(self): - """Exit latency in uS""" - return self.get('latency') - - @property - @memoized - def power(self): - """Power usage in mW - - ..note:: - - This value is not always populated by the kernel and may be garbage. - """ - return self.get('power') - - @property - @memoized - def target_residency(self): - """Target residency in uS - - This is the amount of time in the state required to 'break even' on - power - the system should avoid entering the state for less time than - this. - """ - return self.get('residency') - def enable(self): self.set('disable', 0) @@ -126,23 +92,47 @@ class Cpuidle(Module): def probe(target): return target.file_exists(Cpuidle.root_path) - def get_driver(self): - return self.target.read_value(self.target.path.join(self.root_path, 'current_driver')) - - def get_governor(self): - return self.target.read_value(self.target.path.join(self.root_path, 'current_governor_ro')) + def __init__(self, target): + super(Cpuidle, self).__init__(target) + self._states = {} + + basepath = '/sys/devices/system/cpu/' + values_tree = self.target.read_tree_values(basepath, depth=4, check_exit_code=False) + i = 0 + cpu_id = 'cpu{}'.format(i) + while cpu_id in values_tree: + cpu_node = values_tree[cpu_id] + + if 'cpuidle' in cpu_node: + idle_node = cpu_node['cpuidle'] + self._states[cpu_id] = [] + j = 0 + state_id = 'state{}'.format(j) + while state_id in idle_node: + state_node = idle_node[state_id] + state = CpuidleState( + self.target, + index=j, + path=self.target.path.join(basepath, cpu_id, 'cpuidle', state_id), + name=state_node['name'], + desc=state_node['desc'], + power=int(state_node['power']), + latency=int(state_node['latency']), + residency=int(state_node['residency']) if 'residency' in state_node else None, + ) + msg = 'Adding {} state {}: {} {}' + self.logger.debug(msg.format(cpu_id, j, state.name, state.desc)) + self._states[cpu_id].append(state) + j += 1 + state_id = 'state{}'.format(j) + + i += 1 + cpu_id = 'cpu{}'.format(i) - @memoized def get_states(self, cpu=0): if isinstance(cpu, int): cpu = 'cpu{}'.format(cpu) - states_dir = self.target.path.join(self.target.path.dirname(self.root_path), cpu, 'cpuidle') - idle_states = [] - for state in self.target.list_directory(states_dir): - if state.startswith('state'): - index = int(state[5:]) - idle_states.append(CpuidleState(self.target, index, self.target.path.join(states_dir, state))) - return idle_states + return self._states.get(cpu) def get_state(self, state, cpu=0): if isinstance(state, int): @@ -176,3 +166,9 @@ class Cpuidle(Module): """ output = self.target._execute_util('cpuidle_wake_all_cpus') print(output) + + def get_driver(self): + return self.target.read_value(self.target.path.join(self.root_path, 'current_driver')) + + def get_governor(self): + return self.target.read_value(self.target.path.join(self.root_path, 'current_governor_ro')) -- cgit v1.2.3 From f0426467922bb8cb195de0f73c10ddf9bcf5d835 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 4 Oct 2017 13:19:26 +0100 Subject: module/hotplug: optimize online_all Optimize online_all by offloading it to a shutils function which only requres a single call to the target. --- devlib/bin/scripts/shutils.in | 16 ++++++++++++++++ devlib/module/hotplug.py | 3 ++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/devlib/bin/scripts/shutils.in b/devlib/bin/scripts/shutils.in index 004030d..a678a65 100755 --- a/devlib/bin/scripts/shutils.in +++ b/devlib/bin/scripts/shutils.in @@ -195,6 +195,19 @@ cgroups_freezer_set_state() { exit 1 } +################################################################################ +# Hotplug +################################################################################ + +hotplug_online_all() { + PATHS=(/sys/devices/system/cpu/cpu[0-9]*) + for path in "${PATHS[@]}"; do + if [ $(cat $path/online) -eq 0 ]; then + echo 1 > $path/online + fi + done +} + ################################################################################ # Misc ################################################################################ @@ -252,6 +265,9 @@ cgroups_freezer_set_state) ftrace_get_function_stats) ftrace_get_function_stats ;; +hotplug_online_all) + hotplug_online_all + ;; read_tree_values) read_tree_values $* ;; diff --git a/devlib/module/hotplug.py b/devlib/module/hotplug.py index 8ae238e..cfce2e5 100644 --- a/devlib/module/hotplug.py +++ b/devlib/module/hotplug.py @@ -21,7 +21,8 @@ class HotplugModule(Module): return target.path.join(cls.base_path, cpu, 'online') def online_all(self): - self.online(*range(self.target.number_of_cpus)) + self.target._execute_util('hotplug_online_all', + as_root=self.target.is_rooted) def online(self, *args): for cpu in args: -- cgit v1.2.3 From d560aea660690598eebc7f77efee51efcc8ad2a6 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 5 Oct 2017 09:35:11 +0100 Subject: Target: fix read_tree_values for empty dir grep was existing with 1 when passed an empty directory, resulting in TargetError being raised unless explicitly suppressed with check_exit_code=False. This case is now fixed to correctly return an empty dict without error. read_tree_values bash function has also been optimized to not unnecessarily run find in a subshell if the path passed to the function does not exist. --- devlib/bin/scripts/shutils.in | 9 ++++++--- devlib/target.py | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/devlib/bin/scripts/shutils.in b/devlib/bin/scripts/shutils.in index a678a65..401e51e 100755 --- a/devlib/bin/scripts/shutils.in +++ b/devlib/bin/scripts/shutils.in @@ -216,10 +216,13 @@ read_tree_values() { PATH=$1 MAXDEPTH=$2 + if [ ! -e $PATH ]; then + echo "ERROR: $PATH does not exist" + exit 1 + fi + PATHS=$($BUSYBOX find $PATH -follow -maxdepth $MAXDEPTH) - if [ ${#PATHS[@]} -eq 0 ]; then - echo "ERROR: '$1' does not exist" - else + if [ ${#PATHS[@]} -gt 1 ]; then $BUSYBOX grep -s '' $PATHS fi } diff --git a/devlib/target.py b/devlib/target.py index 8609fec..ec24765 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -620,6 +620,8 @@ class Target(object): check_exit_code=check_exit_code) result = {} for entry in output.strip().split('\n'): + if not entry.strip(): + continue path, value = entry.strip().split(':', 1) result[path] = value return result -- cgit v1.2.3 From 5c28e4167757eeaef722b1e2a88797266c013259 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 5 Oct 2017 10:59:22 +0100 Subject: shutils: fix read_tree_values My bash fu was off on my previous list. The output of find was being treated as a single string, rather than an array of paths; which ment that the test that there was more than one path returned failed, resulting in null output not just for empty directories but for everyting. This fixes the issue by converting find output to an array. --- devlib/bin/scripts/shutils.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devlib/bin/scripts/shutils.in b/devlib/bin/scripts/shutils.in index 401e51e..024e891 100755 --- a/devlib/bin/scripts/shutils.in +++ b/devlib/bin/scripts/shutils.in @@ -221,9 +221,9 @@ read_tree_values() { exit 1 fi - PATHS=$($BUSYBOX find $PATH -follow -maxdepth $MAXDEPTH) + PATHS=($($BUSYBOX find $PATH -follow -maxdepth $MAXDEPTH)) if [ ${#PATHS[@]} -gt 1 ]; then - $BUSYBOX grep -s '' $PATHS + $BUSYBOX grep -s '' ${PATHS[@]} fi } -- cgit v1.2.3 From a0b273b0317bbfc748435a1ebc80d1d589f9d479 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 5 Oct 2017 13:58:46 +0100 Subject: shutil: fix read_tree_values and hotplug_online_all for sh read_tree_values and hotplug_online_all relied on () array evaluation which is a Bash thing and is broken in sh; this fixes things for sh. --- devlib/bin/scripts/shutils.in | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/devlib/bin/scripts/shutils.in b/devlib/bin/scripts/shutils.in index 024e891..007d0a5 100755 --- a/devlib/bin/scripts/shutils.in +++ b/devlib/bin/scripts/shutils.in @@ -200,8 +200,7 @@ cgroups_freezer_set_state() { ################################################################################ hotplug_online_all() { - PATHS=(/sys/devices/system/cpu/cpu[0-9]*) - for path in "${PATHS[@]}"; do + for path in /sys/devices/system/cpu/cpu[0-9]*; do if [ $(cat $path/online) -eq 0 ]; then echo 1 > $path/online fi @@ -213,17 +212,21 @@ hotplug_online_all() { ################################################################################ read_tree_values() { - PATH=$1 + BASEPATH=$1 MAXDEPTH=$2 - if [ ! -e $PATH ]; then - echo "ERROR: $PATH does not exist" + if [ ! -e $BASEPATH ]; then + echo "ERROR: $BASEPATH does not exist" exit 1 fi - PATHS=($($BUSYBOX find $PATH -follow -maxdepth $MAXDEPTH)) - if [ ${#PATHS[@]} -gt 1 ]; then - $BUSYBOX grep -s '' ${PATHS[@]} + PATHS=$($BUSYBOX find $BASEPATH -follow -maxdepth $MAXDEPTH) + i=0 + for path in $PATHS; do + i=$(expr $i + 1) + done + if [ $i -gt 1 ]; then + $BUSYBOX grep -s '' $PATHS fi } -- cgit v1.2.3 From 3e3f964e43d6fec8fe0db536a9054567239547c7 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 5 Oct 2017 16:37:30 +0100 Subject: shuilt: re-introduce speedup The previous fix for read_tree_values fixed the issue with sh not supporting arrays, but looping over paths and counting them. Hover each count increment requires spawning a subshell. For a large number of paths, this can eat away any performance benefits of using read_tree_values. Since we only care whether the count is greater than one, detect that and break out of the loop early to re-introduce the performance improvement. --- devlib/bin/scripts/shutils.in | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/devlib/bin/scripts/shutils.in b/devlib/bin/scripts/shutils.in index 007d0a5..49d7407 100755 --- a/devlib/bin/scripts/shutils.in +++ b/devlib/bin/scripts/shutils.in @@ -223,7 +223,10 @@ read_tree_values() { PATHS=$($BUSYBOX find $BASEPATH -follow -maxdepth $MAXDEPTH) i=0 for path in $PATHS; do - i=$(expr $i + 1) + i=$(expr $i + 1) + if [ $i -gt 1 ]; then + break; + fi done if [ $i -gt 1 ]; then $BUSYBOX grep -s '' $PATHS -- cgit v1.2.3 From 7e073c1fce9304cdf7b60f507208f15a66939bc0 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Fri, 6 Oct 2017 13:37:00 +0100 Subject: read_tree_values: more robust parsing. Skip entries not containing a ":". --- devlib/target.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/target.py b/devlib/target.py index ec24765..6d2a12d 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -620,7 +620,7 @@ class Target(object): check_exit_code=check_exit_code) result = {} for entry in output.strip().split('\n'): - if not entry.strip(): + if ':' not in entry: continue path, value = entry.strip().split(':', 1) result[path] = value -- cgit v1.2.3 From 661ba191147b453f3e1c9eb006b4fe1b2455b928 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Fri, 6 Oct 2017 16:17:56 +0100 Subject: utils/misc: strip more with strip_bash_colors Update regex used by strip_bash_colors to match more ANSI escape sequencies. --- devlib/utils/misc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devlib/utils/misc.py b/devlib/utils/misc.py index 8cfd59f..9565f47 100644 --- a/devlib/utils/misc.py +++ b/devlib/utils/misc.py @@ -474,8 +474,8 @@ def which(name): return None -_bash_color_regex = re.compile('\x1b\\[[0-9;]+m') - +# This matches most ANSI escape sequences, not just colors +_bash_color_regex = re.compile(r'\x1b\[[0-9;]*[a-zA-Z]') def strip_bash_colors(text): return _bash_color_regex.sub('', text) -- cgit v1.2.3 From 1072a1a9f0fd9594e2259ea37b52fe07f6d81f8f Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Fri, 6 Oct 2017 16:20:33 +0100 Subject: utils/ssh: fix Gem5Connection.pull Gem5Connection lists the path to be pulled, however it was not stripping ANSI escape sequences from resulting output, which would corrupt the path. --- devlib/utils/ssh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/utils/ssh.py b/devlib/utils/ssh.py index aaab2dd..27c52d2 100644 --- a/devlib/utils/ssh.py +++ b/devlib/utils/ssh.py @@ -444,7 +444,7 @@ class Gem5Connection(TelnetConnection): self._check_ready() result = self._gem5_shell("ls {}".format(source)) - files = result.split() + files = strip_bash_colors(result).split() for filename in files: dest_file = os.path.basename(filename) -- cgit v1.2.3 From 17bcabd461b22434db00552056e7de995fc7f498 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Mon, 9 Oct 2017 15:15:24 +0100 Subject: target: Install busybox before updating modules Due to the new read_tree_values API, some devlib modules now use shutils in their __init__, which happens from Target::connect(). Therefore ensure we have busybox on the target before we get to that stage. --- devlib/target.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index 6d2a12d..81629b1 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -215,7 +215,9 @@ class Target(object): tid = id(threading.current_thread()) self._connections[tid] = self.get_connection(timeout=timeout) self._resolve_paths() - self.busybox = self.get_installed('busybox') + self.execute('mkdir -p {}'.format(self.working_directory)) + self.execute('mkdir -p {}'.format(self.executables_directory)) + self.busybox = self.install(os.path.join(PACKAGE_BIN_DIRECTORY, self.abi, 'busybox')) self.platform.update_from_target(self) self._update_modules('connected') if self.platform.big_core and self.load_default_modules: @@ -232,10 +234,6 @@ class Target(object): return self.conn_cls(timeout=timeout, **self.connection_settings) # pylint: disable=not-callable def setup(self, executables=None): - self.execute('mkdir -p {}'.format(self.working_directory)) - self.execute('mkdir -p {}'.format(self.executables_directory)) - self.busybox = self.install(os.path.join(PACKAGE_BIN_DIRECTORY, self.abi, 'busybox')) - self._setup_shutils() for host_exe in (executables or []): # pylint: disable=superfluous-parens -- cgit v1.2.3 From 8a0554faab92b6d1ff30a124a24312a8e126b134 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Mon, 9 Oct 2017 17:08:38 +0100 Subject: AndroidTarget: prevent concurrent invocations of 'logcat -c' 'adb logcat -c' has been observed to fail when called twice concurrently. Rather than requiring all devlib users to fix their usage patterns, let's just delay whenever clear_logcat is called twice. --- devlib/target.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/devlib/target.py b/devlib/target.py index 6d2a12d..2fc64dc 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -952,6 +952,7 @@ class AndroidTarget(Target): shell_prompt=shell_prompt, conn_cls=conn_cls) self.package_data_directory = package_data_directory + self.clear_logcat_lock = threading.Lock() def reset(self, fastboot=False): # pylint: disable=arguments-differ try: @@ -1184,7 +1185,8 @@ class AndroidTarget(Target): adb_command(self.adb_name, command, timeout=timeout) def clear_logcat(self): - adb_command(self.adb_name, 'logcat -c', timeout=30) + with self.clear_logcat_lock: + adb_command(self.adb_name, 'logcat -c', timeout=30) def get_logcat_monitor(self, regexps=None): return LogcatMonitor(self, regexps) -- cgit v1.2.3 From 0b04ffcc443f27424146905a0525491f90cd084c Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 4 Oct 2017 17:12:27 +0100 Subject: acmecape: Fix default iio-capture binary name --- devlib/instrument/acmecape.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/instrument/acmecape.py b/devlib/instrument/acmecape.py index 1053c9d..5300343 100644 --- a/devlib/instrument/acmecape.py +++ b/devlib/instrument/acmecape.py @@ -37,7 +37,7 @@ class AcmeCapeInstrument(Instrument): mode = CONTINUOUS def __init__(self, target, - iio_capture=which('iio_capture'), + iio_capture=which('iio-capture'), host='baylibre-acme.local', iio_device='iio:device0', buffer_size=256): -- cgit v1.2.3 From dbe568f51be54c4cb2db9540489c8a0de5c6a19b Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 4 Oct 2017 17:32:32 +0100 Subject: acmecape: Add check for nonzero return code from iio-capture --- devlib/instrument/acmecape.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/devlib/instrument/acmecape.py b/devlib/instrument/acmecape.py index 5300343..0b10d08 100644 --- a/devlib/instrument/acmecape.py +++ b/devlib/instrument/acmecape.py @@ -77,17 +77,22 @@ class AcmeCapeInstrument(Instrument): def stop(self): self.process.terminate() timeout_secs = 10 + output = '' for _ in xrange(timeout_secs): if self.process.poll() is not None: break time.sleep(1) else: - output = _read_nonblock(self.process.stdout) + output += _read_nonblock(self.process.stdout) self.process.kill() self.logger.error('iio-capture did not terminate gracefully') if self.process.poll() is None: msg = 'Could not terminate iio-capture:\n{}' raise HostError(msg.format(output)) + if self.process.returncode != 15: # iio-capture exits with 15 when killed + output += self.process.stdout.read() + raise HostError('iio-capture exited with an error ({}), output:\n{}' + .format(self.process.returncode, output)) if not os.path.isfile(self.raw_data_file): raise HostError('Output CSV not generated.') -- cgit v1.2.3 From 7dd781135546cc081f84bc2acd6d3c44afd5c785 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Mon, 9 Oct 2017 15:28:57 +0100 Subject: acmecape: Add note on how to reboot ACME The ACME firmware sometimes benefits from turning-it-off-and-on-again. --- devlib/instrument/acmecape.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/devlib/instrument/acmecape.py b/devlib/instrument/acmecape.py index 0b10d08..818094f 100644 --- a/devlib/instrument/acmecape.py +++ b/devlib/instrument/acmecape.py @@ -91,6 +91,9 @@ class AcmeCapeInstrument(Instrument): raise HostError(msg.format(output)) if self.process.returncode != 15: # iio-capture exits with 15 when killed output += self.process.stdout.read() + self.logger.info('ACME instrument encountered an error, ' + 'you may want to try rebooting the ACME device:\n' + ' ssh root@{} reboot'.format(self.host)) raise HostError('iio-capture exited with an error ({}), output:\n{}' .format(self.process.returncode, output)) if not os.path.isfile(self.raw_data_file): -- cgit v1.2.3 From 99aca2543835d56001eec63ce60e6c7dc73a4570 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Mon, 9 Oct 2017 18:30:06 +0100 Subject: ApkInfo: Improve error for bad .apk files Provide a more specific error, including the output from aapt. --- devlib/utils/android.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index ce1ab0b..0cdd2b0 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -135,7 +135,11 @@ class ApkInfo(object): _check_env() command = [aapt, 'dump', 'badging', apk_path] logger.debug(' '.join(command)) - output = subprocess.check_output(command) + try: + output = subprocess.check_output(command, stderr=subprocess.STDOUT) + except subprocess.CalledProcessError as e: + raise HostError('Error parsing APK file {}. `aapt` says:\n{}' + .format(apk_path, e.output)) for line in output.split('\n'): if line.startswith('application-label:'): self.label = line.split(':')[1].strip().replace('\'', '') -- cgit v1.2.3 From 0bfb6e4e544e1221220a77c19f83cca7de9dfca2 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 11 Oct 2017 13:09:15 +0100 Subject: AndroidTarget: Add boolean 'state' to airplane mode intent broadcast Recently I've been seeing some errors with enabling airplane mode on hikey960 - firstly I get a 'broken pipe' error followed by a 'cmd: Can't find service: settings'. This missing boolean was the first thing I found that had changed wrt. the code I used to use, and adding it back appears to fix the issue. --- devlib/target.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index b2cfd2f..6132f2b 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1252,9 +1252,11 @@ class AndroidTarget(Target): root_required = self.get_sdk_version() > 23 if root_required and not self.is_rooted: raise TargetError('Root is required to toggle airplane mode on Android 7+') + mode = int(boolean(mode)) cmd = 'settings put global airplane_mode_on {}' - self.execute(cmd.format(int(boolean(mode)))) - self.execute('am broadcast -a android.intent.action.AIRPLANE_MODE', as_root=root_required) + self.execute(cmd.format(mode)) + self.execute('am broadcast -a android.intent.action.AIRPLANE_MODE ' + '--ez state {}'.format(mode), as_root=root_required) def get_auto_rotation(self): cmd = 'settings get system accelerometer_rotation' -- cgit v1.2.3 From da22befd8006e4927a039c503fb688f3643f07a5 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Thu, 21 Sep 2017 12:32:18 +0100 Subject: libs/android: Add get_adb_command function This is just like adb_command but instead of executing the command it just returns it. --- devlib/utils/android.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 0cdd2b0..b5dabaf 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -442,13 +442,16 @@ def adb_list_devices(adb_server=None): return devices -def adb_command(device, command, timeout=None,adb_server=None): +def get_adb_command(device, command, timeout=None,adb_server=None): _check_env() device_string = "" if adb_server != None: device_string = ' -H {}'.format(adb_server) device_string += ' -s {}'.format(device) if device else '' - full_command = "adb{} {}".format(device_string, command) + 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) logger.debug(full_command) output, _ = check_output(full_command, timeout, shell=True) return output -- cgit v1.2.3 From a01418b075ad0a6f627352e8275f04e322bf9c6a Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 11 Oct 2017 17:08:24 +0100 Subject: platform/arm: propagate model parameter VExpress platforms were not updated to handle "model" parameter when it was added to Platfrom. --- devlib/platform/arm.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/devlib/platform/arm.py b/devlib/platform/arm.py index 76b58a4..7d3ced2 100644 --- a/devlib/platform/arm.py +++ b/devlib/platform/arm.py @@ -33,6 +33,7 @@ class VersatileExpressPlatform(Platform): core_names=None, core_clusters=None, big_core=None, + model=None, modules=None, # serial settings @@ -61,6 +62,7 @@ class VersatileExpressPlatform(Platform): core_names, core_clusters, big_core, + model, modules) self.serial_port = serial_port self.baudrate = baudrate -- cgit v1.2.3 From 1d85501181fd205f4c670dd4e7a2d4b1778459ed Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Thu, 21 Sep 2017 12:30:13 +0100 Subject: utils/android: Use pexpect for LogcatMonitor Using a pexpect.spawn object simplifies the LogcatMonitor by removing the need for a separate thread along with the synchronization that brings. Since pexpect.spawn creates a logfile that is flushed with each write, it also removes the need for code to handle flushing. I originally wrote this to allow more complex features that are made possible by the pexpect API, however I then discovered those features are not necessary and did not submit this for merging. However I then discovered that in Python 2.7, threading.Event.wait (used in the `wait_for` method) makes the task uninterriptible (i.e. can't be killed with Ctrl+C/SIGINT), which is rather frustrating. This issue doesn't arise when using pexpect's expect method, so that's why I'm submitting this now. --- devlib/utils/android.py | 124 +++++++++++------------------------------------- 1 file changed, 28 insertions(+), 96 deletions(-) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index b5dabaf..b7c73e6 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -20,6 +20,7 @@ Utility functions for working with Android devices through adb. """ # pylint: disable=E1103 import os +import pexpect import time import subprocess import logging @@ -543,20 +544,19 @@ def _check_env(): adb = _env.adb aapt = _env.aapt -class LogcatMonitor(threading.Thread): +class LogcatMonitor(object): """ Helper class for monitoring Anroid's logcat :param target: Android target to monitor :type target: :class:`AndroidTarget` + :param regexps: List of uncompiled regular expressions to filter on the device. Logcat entries that don't match any will not be seen. If omitted, all entries will be sent to host. :type regexps: list(str) """ - FLUSH_SIZE = 1000 - @property def logfile(self): return self._logfile @@ -565,16 +565,6 @@ class LogcatMonitor(threading.Thread): super(LogcatMonitor, self).__init__() self.target = target - - self._started = threading.Event() - self._stopped = threading.Event() - self._match_found = threading.Event() - - self._sought = None - self._found = None - - self._lines = Queue.Queue() - self._datalock = threading.Lock() self._regexps = regexps def start(self, outfile=None): @@ -585,15 +575,10 @@ class LogcatMonitor(threading.Thread): :type outfile: str """ if outfile: - self._logfile = outfile + self._logfile = open(outfile) else: - fd, self._logfile = tempfile.mkstemp() - os.close(fd) - logger.debug('Logging to {}'.format(self._logfile)) + self._logfile = tempfile.NamedTemporaryFile() - super(LogcatMonitor, self).start() - - def run(self): self.target.clear_logcat() logcat_cmd = 'logcat' @@ -605,86 +590,32 @@ class LogcatMonitor(threading.Thread): regexp = '({})'.format(regexp) logcat_cmd = '{} -e "{}"'.format(logcat_cmd, regexp) - logger.debug('logcat command ="{}"'.format(logcat_cmd)) - self._logcat = self.target.background(logcat_cmd) - - self._started.set() + logcat_cmd = get_adb_command(self.target.conn.device, logcat_cmd) - while not self._stopped.is_set(): - line = self._logcat.stdout.readline(1024) - if line: - self._add_line(line) + logger.debug('logcat command ="{}"'.format(logcat_cmd)) + self._logcat = pexpect.spawn(logcat_cmd, logfile=self._logfile) def stop(self): - if not self.is_alive(): - logger.warning('LogcatMonitor.stop called before start') - return - - # Make sure we've started before we try to kill anything - self._started.wait() - - # Kill the underlying logcat process - # This will unblock self._logcat.stdout.readline() - host.kill_children(self._logcat.pid) - self._logcat.kill() - - self._stopped.set() - self.join() - - self._flush_lines() - - def _add_line(self, line): - self._lines.put(line) - - if self._sought and re.match(self._sought, line): - self._found = line - self._match_found.set() - - if self._lines.qsize() >= self.FLUSH_SIZE: - self._flush_lines() - - def _flush_lines(self): - with self._datalock: - with open(self._logfile, 'a') as fh: - while not self._lines.empty(): - fh.write(self._lines.get()) - - def clear_log(self): - with self._datalock: - while not self._lines.empty(): - self._lines.get() - - with open(self._logfile, 'w') as fh: - pass + self._logcat.terminate() + self._logfile.close() def get_log(self): """ Return the list of lines found by the monitor """ - self._flush_lines() - - with self._datalock: - with open(self._logfile, 'r') as fh: - res = [line for line in fh] + with open(self._logfile.name) as fh: + return [line for line in fh] - return res + def clear_log(self): + with open(self._logfile, 'w') as fh: + pass def search(self, regexp): """ Search a line that matches a regexp in the logcat log Return immediatly """ - res = [] - - self._flush_lines() - - with self._datalock: - with open(self._logfile, 'r') as fh: - for line in fh: - if re.match(regexp, line): - res.append(line) - - return res + return [line for line in self.get_log() if re.match(regexp, line)] def wait_for(self, regexp, timeout=30): """ @@ -700,20 +631,21 @@ class LogcatMonitor(threading.Thread): :returns: List of matched strings """ - res = self.search(regexp) + log = self.get_log() + res = [line for line in log if re.match(regexp, line)] # Found some matches, return them - # Also return if thread not running - if len(res) > 0 or not self.is_alive(): + if len(res) > 0: return res - # Did not find any match, wait for one to pop up - self._sought = regexp - found = self._match_found.wait(timeout) - self._match_found.clear() - self._sought = None + # Store the number of lines we've searched already, so we don't have to + # re-grep them after 'expect' returns + next_line_num = len(log) - if found: - return [self._found] - else: + try: + self._logcat.expect(regexp, timeout=timeout) + except pexpect.TIMEOUT: raise RuntimeError('Logcat monitor timeout ({}s)'.format(timeout)) + + return [line for line in self.get_log()[next_line_num:] + if re.match(regexp, line)] -- cgit v1.2.3 From 71d5b8bc79b4c2a89422836e6f7169f16a2d3640 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Thu, 12 Oct 2017 14:39:27 +0100 Subject: LogcatMonitor: Fix clear_log --- devlib/utils/android.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index b7c73e6..318b4d0 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -607,7 +607,7 @@ class LogcatMonitor(object): return [line for line in fh] def clear_log(self): - with open(self._logfile, 'w') as fh: + with open(self._logfile.name, 'w') as fh: pass def search(self, regexp): -- cgit v1.2.3 From 4a6aacef992e1d5b78f9d80004936fb540740bf5 Mon Sep 17 00:00:00 2001 From: Patrick Bellasi Date: Thu, 12 Oct 2017 14:51:19 +0100 Subject: Instrument/Acmecape: ensure iio-capture termination Once an ACME cape instrument is released, if the stop() method has not been called by the client code, let's ensure to release the channels by killing the corresponding iio-caputure process. Signed-off-by: Patrick Bellasi --- devlib/instrument/acmecape.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/devlib/instrument/acmecape.py b/devlib/instrument/acmecape.py index 818094f..5c10598 100644 --- a/devlib/instrument/acmecape.py +++ b/devlib/instrument/acmecape.py @@ -58,6 +58,12 @@ class AcmeCapeInstrument(Instrument): self.add_channel('device', 'current') self.add_channel('timestamp', 'time_ms') + def __del__(self): + if self.process and self.process.pid: + self.logger.warning('killing iio-capture process [%d]...', + self.process.pid) + self.process.kill() + def reset(self, sites=None, kinds=None, channels=None): super(AcmeCapeInstrument, self).reset(sites, kinds, channels) self.raw_data_file = tempfile.mkstemp('.csv')[1] @@ -98,6 +104,7 @@ class AcmeCapeInstrument(Instrument): .format(self.process.returncode, output)) if not os.path.isfile(self.raw_data_file): raise HostError('Output CSV not generated.') + self.process = None def get_data(self, outfile): if os.stat(self.raw_data_file).st_size == 0: -- cgit v1.2.3 From fe403b629ec42bdd7decca3c02a92abe7f6d7f7c Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 18 Oct 2017 14:26:09 +0100 Subject: AndroidTarget: Make get_rotation return an int That means you can pass the result back to set_rotation without having to change its type. --- devlib/target.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/target.py b/devlib/target.py index 6132f2b..c8d95ad 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1280,7 +1280,7 @@ class AndroidTarget(Target): def get_rotation(self): cmd = 'settings get system user_rotation' - return self.execute(cmd).strip() + return int(self.execute(cmd).strip()) def set_rotation(self, rotation): if not 0 <= rotation <= 3: -- cgit v1.2.3 From a679d579fd7bf4f7fbcf42b74334ee9b95db9931 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Fri, 20 Oct 2017 16:08:32 +0100 Subject: LogcatMonitor: Fix opening logfile for write --- devlib/utils/android.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 318b4d0..14cb253 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -575,7 +575,7 @@ class LogcatMonitor(object): :type outfile: str """ if outfile: - self._logfile = open(outfile) + self._logfile = open(outfile, 'w') else: self._logfile = tempfile.NamedTemporaryFile() -- cgit v1.2.3 From 486b3f524e6b2e7baf3e12446dfe9125d69c88b7 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 18 Oct 2017 11:28:08 +0100 Subject: target: Add is_network_connected method --- devlib/target.py | 35 +++++++++++++++++++++++++++++++++++ doc/target.rst | 7 +++++++ 2 files changed, 42 insertions(+) diff --git a/devlib/target.py b/devlib/target.py index 6132f2b..1d2754f 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -31,6 +31,8 @@ KVERSION_REGEX =re.compile( r'(?P\d+)(\.(?P\d+)(\.(?P\d+)(-rc(?P\d+))?)?)?(.*-g(?P[0-9a-fA-F]{7,}))?' ) +GOOGLE_DNS_SERVER_ADDRESS = '8.8.8.8' + class Target(object): @@ -701,6 +703,39 @@ class Target(object): def _resolve_paths(self): raise NotImplementedError() + def is_network_connected(self): + self.logger.debug('Checking for internet connectivity...') + + timeout_s = 5 + # It would be nice to use busybox for this, but that means we'd need + # root (ping is usually setuid so it can open raw sockets to send ICMP) + command = 'ping -q -c 1 -w {} {} 2>&1'.format(timeout_s, + GOOGLE_DNS_SERVER_ADDRESS) + + # We'll use our own retrying mechanism (rather than just using ping's -c + # to send multiple packets) so that we don't slow things down in the + # 'good' case where the first packet gets echoed really quickly. + for _ in range(5): + try: + self.execute(command) + return True + except TargetError as e: + err = str(e).lower() + if '100% packet loss' in err: + # We sent a packet but got no response. + # Try again - we don't want this to fail just because of a + # transient drop in connection quality. + self.logger.debug('No ping response from {} after {}s' + .format(GOOGLE_DNS_SERVER_ADDRESS, timeout_s)) + continue + elif 'network is unreachable' in err: + # No internet connection at all, we can fail straight away + self.logger.debug('Network unreachable') + return False + else: + # Something else went wrong, we don't know what, raise an + # error. + raise class LinuxTarget(Target): diff --git a/doc/target.rst b/doc/target.rst index ae6ddb0..5a6583e 100644 --- a/doc/target.rst +++ b/doc/target.rst @@ -480,6 +480,13 @@ Target bzip2), the path to the decompressed file is returned; for archives, the path to the directory with the archive's contents is returned. +.. method:: Target.is_network_connected() + Checks for internet connectivity on the device. This doesn't actually + guarantee that the internet connection is "working" (which is rather + nebulous), it's intended just for failing early when definitively _not_ + connected to the internet. + + :returns: ``True`` if internet seems available, ``False`` otherwise. Android Target --------------- -- cgit v1.2.3 From 417ab3df3e292393bd6c75046091815ade575f79 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Tue, 24 Oct 2017 16:03:10 +0100 Subject: target: Ensure returning False when is_network_connected fails --- devlib/target.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/devlib/target.py b/devlib/target.py index 1d2754f..aea3331 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -715,7 +715,8 @@ class Target(object): # We'll use our own retrying mechanism (rather than just using ping's -c # to send multiple packets) so that we don't slow things down in the # 'good' case where the first packet gets echoed really quickly. - for _ in range(5): + attempts = 5 + for _ in range(attempts): try: self.execute(command) return True @@ -737,6 +738,10 @@ class Target(object): # error. raise + self.logger.debug('Failed to ping {} after {} attempts'.format( + GOOGLE_DNS_SERVER_ADDRESS, attempts)) + return False + class LinuxTarget(Target): path = posixpath -- cgit v1.2.3