merge 'parameters' and 'commands' to 'accessibles'

- for now, the definition also accepts the old syntax
  (to be changed later)
- Commands have datatype CommandType
- do not need keyword for the decription parameter of Override
- issue a Warning when a Parameter is overwritten without Overrride
  (this should be turned into an error message)
-

Change-Id: Ib2c0f520abb5b4d7e6aed4d77a0d2b8bc470a85a
Reviewed-on: https://forge.frm2.tum.de/review/18251
Tested-by: JenkinsCodeReview <bjoern_pedersen@frm2.tum.de>
Reviewed-by: Enrico Faulhaber <enrico.faulhaber@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>

etc/epics.cfg

@@ -17,11 +17,11 @@ framing=eol
 encoding=secop
 
 [module tc1]
-class=secop_demo.demo.CoilTemp
+class=secop_demo.modules.CoilTemp
 sensor="X34598T7"
 
 [module tc2]
-class=secop_demo.demo.CoilTemp
+class=secop_demo.modules.CoilTemp
 sensor="X39284Q8'
 
 

secop/client/baseclient.py

@@ -41,7 +41,7 @@ except ImportError:
 
 import mlzlog
 
-from secop.datatypes import get_datatype, EnumType
+from secop.datatypes import get_datatype, EnumType, CommandType
 from secop.lib import mkthread, formatException, formatExtendedStack
 from secop.lib.parsing import parse_time, format_time
 #from secop.protocol.encoding import ENCODERS
@@ -342,7 +342,7 @@ class Client(object):
         return self.describingModulesData[module]
 
     def _getDescribingParameterData(self, module, parameter):
-        return self._getDescribingModuleData(module)['parameters'][parameter]
+        return self._getDescribingModuleData(module)['accessibles'][parameter]
 
     def _decode_list_to_ordereddict(self, data):
         # takes a list of 2*N <key>, <value> entries and
@@ -371,7 +371,7 @@ class Client(object):
                 ['modules'], describing_data)
             for modname, module in list(describing_data['modules'].items()):
                 describing_data['modules'][modname] = self._decode_substruct(
-                    ['parameters', 'commands'], module)
+                    ['accessibles'], module)
 
             self.describing_data = describing_data
 #            import pprint
@@ -382,18 +382,15 @@ class Client(object):
 #            pprint.pprint(r(describing_data))
 
             for module, moduleData in self.describing_data['modules'].items():
-                for parameter, parameterData in moduleData['parameters'].items():
+                for aname, adata in moduleData['accessibles'].items():
-                    datatype = get_datatype(parameterData['datatype'])
+                    datatype = get_datatype(adata['datatype'])
                     # *sigh* special handling for 'some' parameters....
                     if isinstance(datatype, EnumType):
-                        datatype._enum.name = parameter
+                        datatype._enum.name = aname
-                    if parameter == 'status':
+                    if aname == 'status':
-                        datatype.subtypes[0]._enum.name = 'status'
+                        datatype.subtypes[0]._enum.name = 'Status'
-                    self.describing_data['modules'][module]['parameters'] \
+                    self.describing_data['modules'][module]['accessibles'] \
-                        [parameter]['datatype'] = datatype
+                        [aname]['datatype'] = datatype
-                for _cmdname, cmdData in moduleData['commands'].items():
-                    cmdData['arguments'] = list(map(get_datatype, cmdData['arguments']))
-                    cmdData['resulttype'] = get_datatype(cmdData['resulttype'])
         except Exception as _exc:
             print(formatException(verbose=True))
             raise
@@ -551,7 +548,9 @@ class Client(object):
         return list(self.describing_data['modules'].keys())
 
     def getParameters(self, module):
-        return list(self.describing_data['modules'][module]['parameters'].keys())
+        params = filter(lambda item: not isinstance(item[1]['datatype'], CommandType),
+                        self.describing_data['modules'][module]['accessibles'].items())
+        return list(param[0] for param in params)
 
     def getModuleProperties(self, module):
         return self.describing_data['modules'][module]['properties']
@@ -560,14 +559,16 @@ class Client(object):
         return self.getModuleProperties(module)['interface_class']
 
     def getCommands(self, module):
-        return self.describing_data['modules'][module]['commands']
+        cmds = filter(lambda item: isinstance(item[1]['datatype'], CommandType),
+                        self.describing_data['modules'][module]['accessibles'].items())
+        return OrderedDict(cmds)
 
     def execCommand(self, module, command, args):
         #  ignore reply message + reply specifier, only return data
         return self._communicate('do', '%s:%s' % (module, command), list(args) if args else None)[2]
 
     def getProperties(self, module, parameter):
-        return self.describing_data['modules'][module]['parameters'][parameter]
+        return self.describing_data['modules'][module]['accessibles'][parameter]
 
     def syncCommunicate(self, *msg):
         res = self._communicate(*msg)  # pylint: disable=E1120

secop/datatypes.py

@@ -46,7 +46,7 @@ __all__ = [
     u'BoolType', u'EnumType',
     u'BLOBType', u'StringType',
     u'TupleOf', u'ArrayOf', u'StructOf',
-    u'Command',
+    u'CommandType',
 ]
 
 # base class for all DataTypes
@@ -516,17 +516,16 @@ class StructOf(DataType):
         return self.validate(dict(value))
 
 
-# idea to mix commands and params, not yet used....
+class CommandType(DataType):
-class Command(DataType):
     IS_COMMAND = True
 
     def __init__(self, argtypes=tuple(), resulttype=None):
         for arg in argtypes:
             if not isinstance(arg, DataType):
-                raise ValueError(u'Command: Argument types must be DataTypes!')
+                raise ValueError(u'CommandType: Argument types must be DataTypes!')
         if resulttype is not None:
             if not isinstance(resulttype, DataType):
-                raise ValueError(u'Command: result type must be DataTypes!')
+                raise ValueError(u'CommandType: result type must be DataTypes!')
         self.argtypes = argtypes
         self.resulttype = resulttype
 
@@ -542,8 +541,8 @@ class Command(DataType):
     def __repr__(self):
         argstr = u', '.join(repr(arg) for arg in self.argtypes)
         if self.resulttype is None:
-            return u'Command(%s)' % argstr
+            return u'CommandType(%s)' % argstr
-        return u'Command(%s)->%s' % (argstr, repr(self.resulttype))
+        return u'CommandType(%s)->%s' % (argstr, repr(self.resulttype))
 
     def validate(self, value):
         """return the validated arguments value or raise"""
@@ -606,7 +605,7 @@ DATATYPES = dict(
     enum=lambda kwds: EnumType('', **kwds),
     struct=lambda named_subtypes: StructOf(
         **dict((n, get_datatype(t)) for n, t in list(named_subtypes.items()))),
-    command=Command,
+    command=CommandType,
 )
 
 

secop/gui/modulectrl.py

@@ -127,8 +127,8 @@ class CommandButton(QPushButton):
         super(CommandButton, self).__init__(parent)
 
         self._cmdname = cmdname
-        self._argintypes = cmdinfo['arguments']   # list of datatypes
+        self._argintypes = cmdinfo['datatype'].argtypes   # list of datatypes
-        self.resulttype = cmdinfo['resulttype']
+        self.resulttype = cmdinfo['datatype'].resulttype
         self._cb = cb  # callback function for exection
 
         self.setText(cmdname)

secop/gui/nodectrl.py

@@ -194,7 +194,7 @@ class ReadableWidget(QWidget):
 
         # XXX: avoid a nasty race condition, mainly biting on M$
         for i in range(15):
-            if 'status' in self._node.describing_data['modules'][module]['parameters']:
+            if 'status' in self._node.describing_data['modules'][module]['accessibles']:
                 break
             sleep(0.01*i)
 
@@ -246,7 +246,7 @@ class ReadableWidget(QWidget):
         # XXX: also connect update_status signal to LineEdit ??
 
     def update_status(self, status, qualifiers=None):
-        display_string = self._status_type.subtypes[0].entries.get(status[0])
+        display_string = self._status_type.subtypes[0]._enum[status[0]].name
         if status[1]:
             display_string += ':' + status[1]
         self.statusLineEdit.setText(display_string)

secop/metaclass.py

@@ -22,6 +22,7 @@
 """Define Metaclass for Modules/Features"""
 
 from __future__ import print_function
+from collections import OrderedDict
 
 try:
     # pylint: disable=unused-import
@@ -47,7 +48,7 @@ import time
 
 from secop.errors import ProgrammingError
 from secop.datatypes import EnumType
-from secop.params import Parameter
+from secop.params import Parameter, Override, Command
 
 EVENT_ONLY_ON_CHANGED_VALUES = True
 
@@ -68,36 +69,61 @@ class ModuleMeta(type):
         if '__constructed__' in attrs:
             return newtype
 
-        # merge properties, Parameter and commands from all sub-classes
+        # merge properties from all sub-classes
-        for entry in ['properties', 'parameters', 'commands']:
+        newentry = {}
-            newentry = {}
-            for base in reversed(bases):
-                if hasattr(base, entry):
-                    newentry.update(getattr(base, entry))
-            newentry.update(attrs.get(entry, {}))
-            setattr(newtype, entry, newentry)
-
-        # apply Overrides from all sub-classes
-        newparams = getattr(newtype, 'parameters')
         for base in reversed(bases):
-            overrides = getattr(base, 'overrides', {})
+            newentry.update(getattr(base, "properties", {}))
-            for n, o in overrides.items():
+        newentry.update(attrs.get("properties", {}))
-                newparams[n] = o.apply(newparams[n].copy())
+        newtype.properties = newentry
-        for n, o in attrs.get('overrides', {}).items():
+
-            newparams[n] = o.apply(newparams[n].copy())
+        # merge accessibles from all sub-classes, treat overrides
+        # for now, allow to use also the old syntax (parameters/commands dict)
+        accessibles_list = []
+        for base in reversed(bases):
+            if hasattr(base, "accessibles"):
+                accessibles_list.append(base.accessibles)
+        for entry in ['accessibles', 'parameters', 'commands', 'overrides']:
+            accessibles_list.append(attrs.get(entry, {}))
+        accessibles = {} # unordered dict of accessibles
+        newtype.parameters = {}
+        for accessibles_dict in accessibles_list:
+            for key, obj in accessibles_dict.items():
+                if isinstance(obj, Override):
+                    try:
+                        obj = obj.apply(accessibles[key])
+                        accessibles[key] = obj
+                        newtype.parameters[key] = obj
+                    except KeyError:
+                        raise ProgrammingError("module %s: %s does not exist"
+                                       % (name, key))
+                else:
+                    if key in accessibles:
+                        # for now, accept redefinitions:
+                        print("WARNING: module %s: %s should not be redefined"
+                              % (name, key))
+                        # raise ProgrammingError("module %s: %s must not be redefined"
+                        #               % (name, key))
+                    if isinstance(obj, Parameter):
+                        newtype.parameters[key] = obj
+                        accessibles[key] = obj
+                    elif isinstance(obj, Command):
+                        accessibles[key] = obj
+                    else:
+                        raise ProgrammingError('%r: accessibles entry %r should be a '
+                                               'Parameter or Command object!' % (name, key))
 
         # Correct naming of EnumTypes
-        for k, v in newparams.items():
+        for k, v in newtype.parameters.items():
             if isinstance(v.datatype, EnumType) and not v.datatype._enum.name:
                 v.datatype._enum.name = k
 
+        # newtype.accessibles will be used in 2 places only:
+        # 1) for inheritance (see above)
+        # 2) for the describing message
+        newtype.accessibles = OrderedDict(sorted(accessibles.items(), key=lambda item: item[1].ctr))
+
         # check validity of Parameter entries
         for pname, pobj in newtype.parameters.items():
-            # XXX: allow dicts for overriding certain aspects only.
-            if not isinstance(pobj, Parameter):
-                raise ProgrammingError('%r: Parameters entry %r should be a '
-                                       'Parameter object!' % (name, pname))
-
             # XXX: create getters for the units of params ??
 
             # wrap of reading/writing funcs
@@ -165,8 +191,7 @@ class ModuleMeta(type):
 
             setattr(newtype, pname, property(getter, setter))
 
-        # also collect/update information about Command's
+        # check information about Command's
-        setattr(newtype, 'commands', getattr(newtype, 'commands', {}))
         for attrname in attrs:
             if attrname.startswith('do_'):
                 if attrname[3:] not in newtype.commands:

secop/params.py

@@ -23,11 +23,10 @@
 
 from secop.lib import unset_value
 from secop.errors import ProgrammingError
-from secop.datatypes import DataType
+from secop.datatypes import DataType, CommandType
 
 EVENT_ONLY_ON_CHANGED_VALUES = False
 
-
 class CountedObj(object):
     ctr = [0]
     def __init__(self):
@@ -88,6 +87,8 @@ class Parameter(CountedObj):
         # internal caching: value and timestamp of last change...
         self.value = default
         self.timestamp = 0
+        if ctr is not None:
+            self.ctr = ctr
 
     def __repr__(self):
         return '%s_%d(%s)' % (self.__class__.__name__, self.ctr, ', '.join(
@@ -119,21 +120,30 @@ class Override(CountedObj):
 
     note: overrides are applied by the metaclass during class creating
     """
-    def __init__(self, **kwds):
+    def __init__(self, description="", **kwds):
         super(Override, self).__init__()
         self.kwds = kwds
-        self.kwds['ctr'] = self.ctr
+        # allow to override description without keyword
+        if description:
+            self.kwds['description'] = description
+        # for now, do not use the Override ctr
+        # self.kwds['ctr'] = self.ctr
+
+    def __repr__(self):
+        return '%s_%d(%s)' % (self.__class__.__name__, self.ctr, ', '.join(
+            ['%s=%r' % (k, v) for k, v in sorted(self.kwds.items())]))
 
     def apply(self, paramobj):
         if isinstance(paramobj, Parameter):
+            props = paramobj.__dict__.copy()
             for k, v in self.kwds.items():
-                if hasattr(paramobj, k):
+                if k in props:
-                    setattr(paramobj, k, v)
+                    props[k] = v
-                    return paramobj
                 else:
                     raise ProgrammingError(
                         "Can not apply Override(%s=%r) to %r: non-existing property!" %
-                        (k, v, paramobj))
+                        (k, v, props))
+            return Parameter(**props)
         else:
             raise ProgrammingError(
                 "Overrides can only be applied to Parameter's, %r is none!" %
@@ -143,16 +153,16 @@ class Override(CountedObj):
 class Command(CountedObj):
     """storage for Commands settings (description + call signature...)
     """
-    def __init__(self, description, arguments=None, result=None, optional=False):
+    def __init__(self, description, arguments=None, result=None, export=True, optional=False):
         super(Command, self).__init__()
         # descriptive text for humans
         self.description = description
         # list of datatypes for arguments
         self.arguments = arguments or []
-        # datatype for result
+        self.datatype = CommandType(arguments, result)
-        self.resulttype = result
         # whether implementation is optional
         self.optional = optional
+        self.export = export
 
     def __repr__(self):
         return '%s_%d(%s)' % (self.__class__.__name__, self.ctr, ', '.join(
@@ -160,8 +170,8 @@ class Command(CountedObj):
 
     def for_export(self):
         # used for serialisation only
+
         return dict(
             description=self.description,
-            arguments=[arg.export_datatype() for arg in self.arguments],
+            datatype = self.datatype.export_datatype(),
-            resulttype=self.resulttype.export_datatype() if self.resulttype else None,
         )

secop/protocol/dispatcher.py

@@ -150,32 +150,20 @@ class Dispatcher(object):
         # return a copy of our list
         return self._export[:]
 
-    def list_module_params(self, modulename):
+    def export_accessibles(self, modulename):
-        self.log.debug(u'list_module_params(%r)' % modulename)
+        self.log.debug(u'export_accessibles(%r)' % modulename)
         if modulename in self._export:
             # omit export=False params!
-            res = {}
+            res = []
-            for paramname, param in list(self.get_module(modulename).parameters.items()):
+            for aname, aobj in self.get_module(modulename).accessibles.items():
-                if param.export:
+                if aobj.export:
-                    res[paramname] = param.for_export()
+                    res.extend([aname, aobj.for_export()])
-            self.log.debug(u'list params for module %s -> %r' %
+            self.log.debug(u'list accessibles for module %s -> %r' %
                            (modulename, res))
             return res
         self.log.debug(u'-> module is not to be exported!')
         return {}
 
-    def list_module_cmds(self, modulename):
-        self.log.debug(u'list_module_cmds(%r)' % modulename)
-        if modulename in self._export:
-            # omit export=False params!
-            res = {}
-            for cmdname, cmdobj in list(self.get_module(modulename).commands.items()):
-                res[cmdname] = cmdobj.for_export()
-            self.log.debug(u'list cmds for module %s -> %r' % (modulename, res))
-            return res
-        self.log.debug(u'-> module is not to be exported!')
-        return {}
-
     def get_descriptive_data(self):
         """returns a python object which upon serialisation results in the descriptive data"""
         # XXX: be lazy and cache this?
@@ -184,12 +172,7 @@ class Dispatcher(object):
         for modulename in self._export:
             module = self.get_module(modulename)
             # some of these need rework !
-            mod_desc = {u'parameters': [], u'commands': []}
+            mod_desc = {u'accessibles': self.export_accessibles(modulename)}
-            for pname, param in list(self.list_module_params(
-                    modulename).items()):
-                mod_desc[u'parameters'].extend([pname, param])
-            for cname, cmd in list(self.list_module_cmds(modulename).items()):
-                mod_desc[u'commands'].extend([cname, cmd])
             for propname, prop in list(module.properties.items()):
                 mod_desc[propname] = prop
             result[u'modules'].extend([modulename, mod_desc])
@@ -211,7 +194,7 @@ class Dispatcher(object):
         cmdspec = moduleobj.commands.get(command, None)
         if cmdspec is None:
             raise NoSuchCommandError(module=modulename, command=command)
-        if len(cmdspec.arguments) != len(arguments):
+        if len(cmdspec.datatype.argtypes) != len(arguments):
             raise BadValueError(
                 module=modulename,
                 command=command,

secop_demo/modules.py

@@ -25,7 +25,7 @@ import random
 import threading
 
 from secop.lib.enum import Enum
-from secop.modules import Readable, Drivable, Parameter
+from secop.modules import Readable, Drivable, Parameter, Override
 from secop.datatypes import EnumType, FloatRange, IntRange, ArrayOf, StringType, TupleOf, StructOf, BoolType
 
 
@@ -33,10 +33,10 @@ class Switch(Drivable):
     """switch it on or off....
     """
     parameters = {
-        'value': Parameter('current state (on or off)',
+        'value': Override('current state (on or off)',
                        datatype=EnumType(on=1, off=0), default=0,
                        ),
-        'target': Parameter('wanted state (on or off)',
+        'target': Override('wanted state (on or off)',
                         datatype=EnumType(on=1, off=0), default=0,
                         readonly=False,
                         ),
@@ -95,10 +95,10 @@ class MagneticField(Drivable):
     """a liquid magnet
     """
     parameters = {
-        'value': Parameter('current field in T',
+        'value': Override('current field in T',
                        unit='T', datatype=FloatRange(-15, 15), default=0,
                        ),
-        'target': Parameter('target field in T',
+        'target': Override('target field in T',
                         unit='T', datatype=FloatRange(-15, 15), default=0,
                         readonly=False,
                         ),
@@ -183,7 +183,7 @@ class CoilTemp(Readable):
     """a coil temperature
     """
     parameters = {
-        'value': Parameter('Coil temperatur',
+        'value': Override('Coil temperatur',
                        unit='K', datatype=FloatRange(), default=0,
                        ),
         'sensor': Parameter("Sensor number or calibration id",
@@ -199,7 +199,7 @@ class SampleTemp(Drivable):
     """a sample temperature
     """
     parameters = {
-        'value': Parameter('Sample temperature',
+        'value': Override('Sample temperature',
                        unit='K', datatype=FloatRange(), default=10,
                        ),
         'sensor': Parameter("Sensor number or calibration id",
@@ -255,7 +255,7 @@ class Label(Readable):
         'subdev_ts': Parameter("name of subdevice for sample temp",
                            datatype=StringType, export=False,
                            ),
-        'value': Parameter("final value of label string",
+        'value': Override("final value of label string", default='',
                        datatype=StringType,
                        ),
     }

secop_ess/epics.py

@@ -181,7 +181,7 @@ class EpicsDrivable(Drivable):
             return Drivable.Status.UNKNOWN, self._read_pv(self.status_pv)
         # status_pv is unset, derive status from equality of value + target
         if self.read_value() == self.read_target():
-            return (Drivable.Status.OK, '')
+            return (Drivable.Status.IDLE, '')
         return (Drivable.Status.BUSY, 'Moving')
 
 
@@ -221,7 +221,7 @@ class EpicsTempCtrl(EpicsDrivable):
         at_target = abs(self.read_value(maxage) - self.read_target(maxage)) \
             <= self.tolerance
         if at_target:
-            return (Drivable.Status.OK, 'at Target')
+            return (Drivable.Status.IDLE, 'at Target')
         return (Drivable.Status.BUSY, 'Moving')
 
     # TODO: add support for strings over epics pv