fix error in write wrapper and more

- write wrapper must return the result, not the argument
- modify test_modules.py for this
- mixins are not required to inherit from HasAttributes -> modify method check
- config for Attach may be mandatory (default: True)

Change-Id: I34f2965b12d69717e81d9296715467df6f3ac447
Reviewed-on: https://forge.frm2.tum.de/review/c/sine2020/secop/playground/+/27934
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Enrico Faulhaber <enrico.faulhaber@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>

secop/modules.py

@@ -181,14 +181,14 @@ class HasAccessibles(HasProperties):
                             self.log.debug('validate %r for %r', value, pname)
                             # we do not need to handle errors here, we do not
                             # want to make a parameter invalid, when a write failed
-                            value = pobj.datatype(value)
-                            returned_value = wfunc(self, value)
-                            self.log.debug('write_%s(%r) returned %r', pname, value, returned_value)
-                            if returned_value is Done:
+                            new_value = pobj.datatype(value)
+                            new_value = wfunc(self, new_value)
+                            self.log.debug('write_%s(%r) returned %r', pname, value, new_value)
+                            if new_value is Done:
                                 # setattr(self, pname, getattr(self, pname))
                                 return getattr(self, pname)
-                            setattr(self, pname, value)  # important! trigger the setter
-                            return value
+                            setattr(self, pname, new_value)  # important! trigger the setter
+                            return new_value
                     else:
 
                         def new_wfunc(self, value, pname=pname):
@@ -201,14 +201,14 @@ class HasAccessibles(HasProperties):
                     setattr(cls, 'write_' + pname, new_wfunc)
 
         # check for programming errors
-        for attrname, attrvalue in cls.__dict__.items():
+        for attrname in dir(cls):
             prefix, _, pname = attrname.partition('_')
             if not pname:
                 continue
             if prefix == 'do':
                 raise ProgrammingError('%r: old style command %r not supported anymore'
                                        % (cls.__name__, attrname))
-            if prefix in ('read', 'write') and not getattr(attrvalue, 'wrapped', False):
+            if prefix in ('read', 'write') and not getattr(getattr(cls, attrname), 'wrapped', False):
                 raise ProgrammingError('%s.%s defined, but %r is no parameter'
                                        % (cls.__name__, attrname, pname))
 
@@ -804,9 +804,9 @@ class Attached(Property):
     assign a module name to this property in the cfg file,
     and the server will create an attribute with this module
     """
-    def __init__(self, basecls=Module, description='attached module'):
+    def __init__(self, basecls=Module, description='attached module', mandatory=True):
         self.basecls = basecls
-        super().__init__(description, StringType(), mandatory=False)
+        super().__init__(description, StringType(), mandatory=mandatory)
 
     def __get__(self, obj, owner):
         if obj is None:
@@ -815,4 +815,4 @@ class Attached(Property):
             # return the name of the module (called from Server on startup)
             return super().__get__(obj, owner)
         # return the module (called after startup)
-        return obj.attachedModules[self.name]
+        return obj.attachedModules.get(self.name)  # return None if not given

secop/server.py

@@ -283,12 +283,13 @@ class Server:
                 if isinstance(propobj, Attached):
                     try:
                         attname = getattr(modobj, propname)
-                        attobj = self.dispatcher.get_module(attname)
-                        if isinstance(attobj, propobj.basecls):
-                            attached_modules[propname] = attobj
-                        else:
-                            errors.append('attached module %s=%r must inherit from %r'
-                                          % (propname, attname, propobj.basecls.__qualname__))
+                        if attname:  # attached module specified in cfg file
+                            attobj = self.dispatcher.get_module(attname)
+                            if isinstance(attobj, propobj.basecls):
+                                attached_modules[propname] = attobj
+                            else:
+                                errors.append('attached module %s=%r must inherit from %r'
+                                              % (propname, attname, propobj.basecls.__qualname__))
                     except SECoPError as e:
                         errors.append('module %s, attached %s: %s' % (modname, propname, str(e)))
             modobj.attachedModules = attached_modules

test/test_modules.py

@@ -149,7 +149,7 @@ def test_ModuleMagic():
         a1 = Parameter(datatype=FloatRange(unit='$/s'), readonly=False)
         # remark: it might be a programming error to override the datatype
         # and not overriding the read_* method. This is not checked!
-        b2 = Parameter('<b2>', datatype=BoolType(), default=True,
+        b2 = Parameter('<b2>', datatype=StringType(), default='empty',
                        readonly=False, initwrite=True)
 
         def write_a1(self, value):
@@ -157,6 +157,7 @@ def test_ModuleMagic():
             return value
 
         def write_b2(self, value):
+            value = value.upper()
             self._b2_written = value
             return value
 
@@ -211,11 +212,11 @@ def test_ModuleMagic():
     o2.startModule(event)
     event.wait()
     # value has changed type, b2 and a1 are written
-    expectedAfterStart.update(value=0, b2=True, a1=True)
+    expectedAfterStart.update(value=0, b2='EMPTY', a1=True)
     # ramerk: a1=True: this behaviour is a Porgamming error
     assert updates.pop('o2') == expectedAfterStart
     assert o2._a1_written == 2.7
-    assert o2._b2_written is True
+    assert o2._b2_written == 'EMPTY'
 
     assert not updates