sascha/devlib
1
0
Fork 0
mirror of https://github.com/ARM-software/devlib.git synced 2025-01-31 02:00:45 +00:00
Code Issues Releases Wiki Activity

Instrument: Fix & restructure reset()

Browse Source 
Calling `Instrument.reset(kinds='some string')` leaves
`self.active_channels` as `[]` which is probably not the expected
behaviour. This is caused by the last nested `else` which refers to
the condition `if isinstance(kinds, basestring)` and might have been
overlooked because of having been confused with the top-level `else`.
Anyhow, an `else` does not seem to be needed there.

This bug illustrates the risk of having too many nested levels and
execution paths which also impact the readability of the code. We
modify the implementation to solve the bug on top of which we:

  - Reduce the maximum order of nested levels from 4 to 3;

  - Express more clearly the potential paths of execution
    (less nested conditions);

  - Replace unnecessary `for`-loops by list comprehensions,
    removing the need for an initialisation of `active_channels`
    and making clearer what each path of execution ends up with;

  - Removed unnecessary `List` copies of `self.channels.values()`;

  - Used the fact that the message of a `KeyError` is the unknown
    key.
This commit is contained in:
Pierre-Clement Tosi 2018-05-30 14:15:52 +01:00 committed by Marc Bonnici
parent fe0d6eda2a
commit c4f6a1a85f
1 changed files with 15 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
devlib/instrument/__init__.py
View File

@ -1,4 +1,4 @@
# Copyright 2015 ARM Limited
# Copyright 2018 ARM Limited
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -298,29 +298,26 @@ 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)
elif channels is not None:
if 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()))
raise ValueError('sites and kinds should not be set if channels is set')
try:
self.active_channels = [self.channels[ch] for ch in channels]
except KeyError as e:
msg = 'Unexpected channel "{}"; must be in {}'
raise ValueError(msg.format(e, self.channels.keys()))
elif sites is None and kinds is None:
self.active_channels = sorted(self.channels.itervalues(), key=lambda x: x.label)
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)
wanted = lambda ch : ((kinds is None or ch.kind in kinds) and
(sites is None or ch.site in sites))
self.active_channels = filter(wanted, self.channels.itervalues())
# instantaneous