From 1513db0951e51642fb62b890c903c810a61df010 Mon Sep 17 00:00:00 2001
From: Brendan Jackman <brendan.jackman@arm.com>
Date: Wed, 10 May 2017 11:18:21 +0100
Subject: [PATCH] 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()