Merge 25a6d16 into a3560c7

docs/developer_guide/writing_devices.rst

@@ -216,19 +216,19 @@ Implementing the device interface
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 Device interfaces are implemented by sub-classing an appropriate
-pre-written communication adapter base class from the framework's
+pre-written, protocol specific interface base class from the framework's
 :mod:`lewis.adapters`-package and overriding a few members. In this case this
-adapter is called :class:`~lewis.adapters.stream.StreamAdapter`. The first step
+base class is called :class:`~lewis.adapters.stream.StreamInterface`. The first step
 is to specify the available commands in terms of a collection of
 :class:`~lewis.adapters.stream.Cmd`-objects. These objects effectively bind
-commands specified in terms of regular expressions to a the adapter's methods.
+commands specified in terms of regular expressions to the interface's methods.
 According to the specifications above, the commands are defined like this:
 
 .. code:: python
 
-    from lewis.adapters.stream import StreamAdapter, Cmd
+    from lewis.adapters.stream import StreamInterface, Cmd
 
-    class ExampleMotorStreamInterface(StreamAdapter):
+    class ExampleMotorStreamInterface(StreamInterface):
         commands = {
             Cmd('get_status', r'^S\?$'),
             Cmd('get_position', r'^P\?$'),
@@ -274,11 +274,11 @@ overridden by supplying a callable object to ``return_mapping``, as it
 is the case for the ``stop``-command.
 
 You may have noticed that ``stop`` is not a method of the interface.
-:class:`~lewis.adapters.stream.StreamAdapter` tries to resolve the supplied method
+:class:`~lewis.adapters.stream.StreamInterface` tries to resolve the supplied method
 names in multiple ways. First it checks its own members, then it checks the members of the
 device it owns (accessible in the interface via the ``device``-member)
-and adds forwarders to itself if possible. If the method name can not be
-found in either the device or the adapter, an error is produced, which
+and binds to the appropriate method. If the method name can not be
+found in either the device or the interface, an error is produced, which
 minimizes the likelihood of typos. The definitions in the interface
 always have precedence, this is intentionally done so that device
 behavior can be overridden later on with minimal changes to the code.
@@ -330,11 +330,10 @@ User facing documentation
 The :class:`~lewis.adapters.stream.StreamAdapter`-class has a property
 ``documentation``, which generates user facing documentation from the
 :class:`~lewis.adapters.stream.Cmd`-objects (it can be displayed via the ``-i``-flag of
-``lewis.py`` or as the ``device_documentation``-property of the ``simulation``-object via
-``lewis-control.py``. The regular expression of each command is listed, along
-with a documentation string. If the ``doc``-parameter is provided to Cmd, it is used,
-otherwise the docstring of the wrapped method is used (it does not matter whether the
-method is part of the device or the interface for feature to work). The latter is the
+``lewis.py`` from the ``interface`` object via ``lewis-control.py``). The regular expression of
+each command is listed, along with a documentation string. If the ``doc``-parameter is provided
+to Cmd, it is used,otherwise the docstring of the wrapped method is used (it does not matter
+whether the method is part of the device or the interface for feature to work). The latter is the
 recommended way, because it avoids duplication. But in some cases, the user- and the
 developer facing documentation may be so different that it's useful to override the docstring.
 
@@ -412,7 +411,7 @@ suggestions for changes. Once the code is acceptable, it will be merged
 into Lewis' master branch and become a part of the distribution.
 
 If a second interface is added to a device, either using a different
-adapter or the same adapter but with different commands, the interface
+interface type or the same but with different commands, the interface
 definitions should be moved out of the ``__init__.py`` file. Lewis
 will continue to work if the interfaces are moved to a sub-folder of the
 device called ``interfaces``. This needs to have its own

docs/release_notes/release_1_1_0.rst

@@ -30,6 +30,32 @@ Bug fixes and other improvements
    also the SVGs are in the `source repository`_, feel free to include them in presentations,
    posters.
 
+Upgrade guide
+-------------
+
+ - Due to a change to how Adapters and Devices work together, device interfaces are not
+   inheriting from Adapter-classes anymore. Instead, there are dedicated Interface classes.
+   They are located in the same modules as the Adapters, so only small changes are necessary:
+
+   Old:
+   .. sourcecode:: Python
+
+       from lewis.adapters.stream import StreamAdapter, Cmd
+
+       class DeviceInterface(StreamAdapter):
+           pass
+
+   New:
+   .. sourcecode:: Python
+
+       from lewis.adapters.stream import StreamInterface, Cmd
+
+       class DeviceInterface(StreamInterface):
+           pass
+
+   The same goes for ``EpicsAdapter`` and ``ModbusAdapter``, which must be modified to
+   ``EpicsInterface`` and ``ModbusInterface`` respectively.
+
 .. _source repository: https://github.com/DMSC-Instrument-Data/lewis/docs/resources/logo
 .. _Rubik: https://github.com/googlefonts/rubik
 .. _inkscape: https://inkscape.org/

lewis/adapters/epics.py

@@ -22,6 +22,7 @@
 import inspect
 
 from lewis.core.adapters import Adapter
+from lewis.core.devices import InterfaceBase
 from six import iteritems, string_types
 
 from lewis.core.logging import has_log
@@ -293,6 +294,9 @@ def _get_target(self, prop, *targets):
         getter = self._create_getter(raw_getter, *targets)
         setter = self._create_setter(raw_setter, *targets)
 
+        if getter is None and setter is None:
+            return None
+
         if prop == 'value' and setter is None:
             self.read_only = True
 
@@ -388,38 +392,41 @@ def _function_has_n_args(self, func, n):
 
 @has_log
 class PropertyExposingDriver(Driver):
-    def __init__(self, target, pv_dict):
+    def __init__(self, interface):
         super(PropertyExposingDriver, self).__init__()
 
-        self._target = target
-        self._set_logging_context(target)
+        self._interface = interface
+        self._set_logging_context(interface)
 
-        self._pv_dict = pv_dict
-        self._timers = {k: 0.0 for k in self._pv_dict.keys()}
+        self._timers = {k: 0.0 for k in self._interface.bound_pvs.keys()}
         self._last_update_call = None
 
     def write(self, pv, value):
         self.log.debug('PV put request: %s=%s', pv, value)
 
-        pv_object = self._pv_dict.get(pv)
+        pv_object = self._interface.bound_pvs.get(pv)
 
         if not pv_object:
             return False
 
         try:
             pv_object.value = value
             self.setParam(pv, pv_object.value)
+
             return True
         except (LimitViolationException, AccessViolationException):
-            return False
+            self.log.exception('An error occurred when writing %s to PV %s', value, pv)
 
     def process_pv_updates(self, force=False):
         dt = seconds_since(self._last_update_call or datetime.now())
         # Updates bound parameters as needed
 
         updates = []
 
-        for pv, pv_object in iteritems(self._pv_dict):
+        for pv, pv_object in iteritems(self._interface.bound_pvs):
+            if pv not in self._timers:
+                self._timers = 0.0
+
             self._timers[pv] += dt
             if self._timers[pv] >= pv_object.poll_interval or force:
                 try:
@@ -443,60 +450,7 @@ def process_pv_updates(self, force=False):
 
 class EpicsAdapter(Adapter):
     """
-    Inheriting from this class provides an EPICS-interface to a device, powered by
-    the pcaspy-module. In the simplest case all that is required is to inherit
-    from this class and override the ``pvs``-member. It should be a dictionary
-    that contains PV-names (without prefix) as keys and instances of PV as
-    values.
-
-    For a simple device with two properties, speed and position, the first of which
-    should be read-only, it's enough to define the following:
-
-    .. sourcecode:: Python
-
-        class SimpleDeviceEpicsInterface(EpicsAdapter):
-            pvs = {
-                'VELO': PV('speed', read_only=True),
-                'POS': PV('position', lolo=0, hihi=100)
-            }
-
-    For more complex behavior, the interface could contain properties that do not
-    exist in the device itself. If the device should also have a PV called STOP
-    that "stops the device", the interface could look like this:
-
-    .. sourcecode:: Python
-
-        class SimpleDeviceEpicsInterface(EpicsAdapter):
-            pvs = {
-                'VELO': PV('speed', read_only=True),
-                'POS': PV('position', lolo=0, hihi=100),
-                'STOP': PV('stop', type='int'),
-            }
-
-            @property
-            def stop(self):
-                return 0
-
-            @stop.setter
-            def stop(self, value):
-                if value == 1:
-                    self.device.halt()
-
-    Even though the device does *not* have a property called ``stop`` (but a method called
-    ``halt``), issuing the command
-
-    ::
-
-        $ caput STOP 1
-
-    will achieve the desired behavior, because ``EpicsAdapter`` merges the properties
-    of the device into ``SimpleDeviceEpicsInterface`` itself, so that it is does not
-    matter whether the specified property in PV exists in the device or the adapter.
-
-    The intention of this design is to keep device classes small and free of
-    protocol specific stuff, such as in the case above where stopping a device
-    via EPICS might involve writing a value to a PV, whereas other protocols may
-    offer an RPC-way of achieving the same thing.
+    This adapter provides ChannelAccess server functionality through the pcaspy module.
 
     It's possible to configure the prefix for the PVs provided by this adapter. The
     corresponding key in the ``options`` dictionary is called ``prefix``:
@@ -509,8 +463,7 @@ def stop(self, value):
 
     :param options: Dictionary with options.
     """
-    protocol = 'epics'
-    pvs = None
+
     default_options = {'prefix': ''}
 
     def __init__(self, options=None):
@@ -519,42 +472,11 @@ def __init__(self, options=None):
         self._server = None
         self._driver = None
 
-        self._bound_pvs = {}
-
-    def _bind_device(self):
-        """
-        This method is re-implemented from :class:`~lewis.core.adapters.Adapter`. It uses
-        :meth:`_bind_properties` to generate a dict of bound PVs.
-        """
-        self._bound_pvs = self._bind_properties(self.pvs)
-
-        if self._driver is not None:
-            self._driver._pv_dict = self._bound_pvs
-
-    def _bind_properties(self, pvs):
-        """
-        This method transforms a dict of :class:`PV` objects to a dict of :class:`BoundPV` objects,
-        the keys are always the PV-names that are exposed via ChannelAccess.
-
-        In the transformation process, the method tries to find whether the attribute specified by
-        PV's ``property`` (and ``meta_data_property``) is part of the internally stored device
-        or the interface and constructs a BoundPV, which acts as a forwarder to the appropriate
-        objects.
-
-        :param pvs: Dict of PV-name/:class:`PV`-objects.
-        :return: Dict of PV-name/:class:`BoundPV`-objects.
-        """
-        bound_pvs = {}
-        for pv_name, pv in pvs.items():
-            bound_pvs[pv_name] = pv.bind(self, self.device)
-
-        return bound_pvs
-
     @property
     def documentation(self):
         pvs = []
 
-        for name, pv in self._bound_pvs.items():
+        for name, pv in self.interface.bound_pvs.items():
             complete_name = self._options.prefix + name
 
             data_type = pv.config.get('type', 'float')
@@ -564,7 +486,7 @@ def documentation(self):
                 complete_name, data_type, read_only_tag, format_doc_text(pv.doc)))
 
         return '\n\n'.join(
-            [inspect.getdoc(self) or '', 'PVs\n==='] + pvs)
+            [inspect.getdoc(self.interface) or '', 'PVs\n==='] + pvs)
 
     def start_server(self):
         """
@@ -577,12 +499,13 @@ def start_server(self):
         if self._server is None:
             self._server = SimpleServer()
             self._server.createPV(prefix=self._options.prefix,
-                                  pvdb={k: v.config for k, v in self._bound_pvs.items()})
-            self._driver = PropertyExposingDriver(target=self, pv_dict=self._bound_pvs)
+                                  pvdb={k: v.config for k, v in self.interface.bound_pvs.items()})
+            self._driver = PropertyExposingDriver(interface=self.interface)
             self._driver.process_pv_updates(force=True)
 
             self.log.info('Started serving PVs: %s',
-                          ', '.join((self._options.prefix + pv for pv in self._bound_pvs.keys())))
+                          ', '.join((self._options.prefix + pv for pv in
+                                     self.interface.bound_pvs.keys())))
 
     def stop_server(self):
         self._driver = None
@@ -604,3 +527,86 @@ def handle(self, cycle_delay=0.1):
         if self._server is not None:
             self._server.process(cycle_delay)
             self._driver.process_pv_updates()
+
+
+class EpicsInterface(InterfaceBase):
+    """
+    Inheriting from this class provides an EPICS-interface to a device for use with
+    :class:`EpicsAdapter`. In the simplest case all that is required is to inherit
+    from this class and override the ``pvs``-member. It should be a dictionary
+    that contains PV-names (without prefix) as keys and instances of PV as
+    values. The prefix is handled by ``EpicsAdapter``.
+
+    For a simple device with two properties, speed and position, the first of which
+    should be read-only, it's enough to define the following:
+
+    .. sourcecode:: Python
+
+        class SimpleDeviceEpicsInterface(EpicsInterface):
+            pvs = {
+                'VELO': PV('speed', read_only=True),
+                'POS': PV('position', lolo=0, hihi=100)
+            }
+
+    For more complex behavior, the interface could contain properties that do not
+    exist in the device itself. If the device should also have a PV called STOP
+    that "stops the device", the interface could look like this:
+
+    .. sourcecode:: Python
+
+        class SimpleDeviceEpicsInterface(EpicsInterface):
+            pvs = {
+                'VELO': PV('speed', read_only=True),
+                'POS': PV('position', lolo=0, hihi=100),
+                'STOP': PV('stop', type='int'),
+            }
+
+            @property
+            def stop(self):
+                return 0
+
+            @stop.setter
+            def stop(self, value):
+                if value == 1:
+                    self.device.halt()
+
+    Even though the device does *not* have a property called ``stop`` (but a method called
+    ``halt``), issuing the command
+
+    ::
+
+        $ caput STOP 1
+
+    will achieve the desired behavior, because ``EpicsInterface`` merges the properties
+    of the device into ``SimpleDeviceEpicsInterface`` itself, so that it is does not
+    matter whether the specified property in PV exists in the device or the adapter.
+
+    The intention of this design is to keep device classes small and free of
+    protocol specific stuff, such as in the case above where stopping a device
+    via EPICS might involve writing a value to a PV, whereas other protocols may
+    offer an RPC-way of achieving the same thing.
+    """
+
+    protocol = 'epics'
+    pvs = None
+
+    def __init__(self):
+        super(EpicsInterface, self).__init__()
+        self.bound_pvs = None
+
+    @property
+    def adapter(self):
+        return EpicsAdapter
+
+    def _bind_device(self):
+        """
+        This method transforms the ``self.pvs`` dict of :class:`PV` objects ``self.bound_pvs``,
+        a dict of :class:`BoundPV` objects, the keys are always the PV-names that are exposed
+        via ChannelAccess.
+
+        In the transformation process, the method tries to find whether the attribute specified by
+        PV's ``property`` (and ``meta_data_property``) is part of the internally stored device
+        or the interface and constructs a BoundPV, which acts as a forwarder to the appropriate
+        objects.
+        """
+        self.bound_pvs = {pv_name: pv.bind(self, self.device) for pv_name, pv in self.pvs.items()}