frappy.ctrlby: improvements

Change-Id: I7ea2d0398fa3b32002dbaa066e3923fef72535fa

frappy/ctrlby.py

@@ -24,29 +24,20 @@ from frappy.core import Parameter, Attached
 
 
 class WrapControlledBy:
-    """mixin for modules with controlled_by
+    """mixin to add controlled_by to writable modules
 
-    Two use cases:
+    Create a wrapper class inheriting from this mixin
+    to add controlled_by to a Writable module
 
-    1) on the implementation of a hardware module it is already known that
-       HasControlledBy is wanted. In this case, functionality to apply the
-       target to the hardware has to be implemented in method 'set_target'
+    Usage:
 
-        class MyWritable(HasControlledBy, ...):
+        class Enhanced(WrapControlledBy, BaseWritable):
+            pass
 
-            def set_target(self, target):
-                "apply target to HW"
-                # no supercall needed!
+        # typically nothing else has to be implemented
 
-            # do not override write_target !
-
-    2) a hardware module is already available, and we extend it with the
-       controlled_by stuff
-
-        class Enhanced(HasControlledBy, BaseWritable):
-            set_target = BaseWritable.write_target
-
-        # nothing else is needed.
+    from a module with output (inheriting from HasOutput), the
+    method update_target must be called for internal updates
     """
 
     controlled_by = Parameter('source of target value', EnumType(members={'self': 0}), default=0)
@@ -87,12 +78,12 @@ class WrapControlledBy:
             self.write_controlled_by('self')
 
     def set_off(self):
-        """to be overriden if the off state should be different than the default
+        """to be overridden if the off state should be different from the default
 
         on a FloatRange() the default value is 0
         """
         self.self_controlled()
-        self.set_target_cby(self.parameters['target'].datatype.default)
+        self.internal_set_target(self.parameters['target'].datatype.default)
 
     def update_target(self, module, value):
         """update internal target value
@@ -108,28 +99,50 @@ class WrapControlledBy:
             if deactivate_control:
                 deactivate_control(module)
             self.controlled_by = module
-        target = self.set_target_cby(value)
+        target = self.internal_set_target(value)
         self.target = value if target is None else target
 
     def write_target(self, target):
         self.self_controlled()
-        return self.set_target_cby(target)
+        return self.internal_set_target(target)
 
-    def set_target_cby(self, target):
+    def internal_set_target(self, target):
+        # we need this additional indirection:
+        # super().write_target must refer to an inherited base class
+        # which is after WrapControlledBy in the method resolution order
         return super().write_target(target)
 
 
 class HasControlledBy(WrapControlledBy):
+    """mixin for controlled_by functionality
+
+    Create a wrapper class inheriting from this mixin
+    to add controlled_by to a Writable module
+
+    Usage:
+
+        class Enhanced(HasControlledBy, BaseWritable):
+            def set_target(self, value):
+                # implement here hardware access for setting target
+
+            # do not override write_target!
+
+    from a module with output (inheriting from HasOutput), the
+    method update_target must be called for internal updates
+    """
+
     def set_target(self, value):
         """to be overridden for setting target of HW"""
         raise NotImplementedError
 
-    def set_target_cby(self, value):
-        """to be overridden in case this mixin is not added on top"""
+    def internal_set_target(self, value):
+        # we need this additional indirection:
+        # self.write_target must refer to a base class which
+        # is before HasControlledBy in the method resolution order
         return self.set_target(value)
 
 
-class WrapOutputModule:
+class HasOutputModule:
     """mixin for modules having an output module
 
     this module will call the update_target method of an output module
@@ -145,7 +158,14 @@ class WrapOutputModule:
             self.output_module.register_input(self.name, self.deactivate_control)
 
     def write_control_active(self, value):
-        """override with supercall if needed"""
+        """override with supercall if needed
+
+        control_active is readonly by default, as specified in the SECoP standard.
+        This method is meant to be called internally.
+
+        However, it is possible to override control_active with readonly=False
+        and this is quite useful IMHO in some situations
+        """
         out = self.output_module
         if out:
             if value:
@@ -157,13 +177,16 @@ class WrapOutputModule:
                     out.set_off()  # this sets out.controlled_by to 0 (=self)
 
     def set_control_active(self, active):
-        """to be overridden for switching hw control"""
+        """to be overridden for switching hw control
+
+        TODO: remove this legacy method (replaced by write_control_active)
+        """
         self.control_active = active
 
     def activate_control(self):
         """method to switch control_active on
 
-        to be called from the write_target method, with the target as argument
+        TODO: remove this legacy method (replaced by write_control_active)
         """
         self.write_control_active(True)
 
@@ -178,16 +201,8 @@ class WrapOutputModule:
 
     def write_target(self, target):
         self.write_control_active(True)
-        return self.set_target_out(target)
-
-    def set_target_out(self, target):
-        return super().write_target(target)
-
-
-class HasOutputModule(WrapOutputModule):
-    def set_target(self, target):
-        """to be overridden except for WrapOutputModule"""
-        raise NotImplementedError
-
-    def set_target_out(self, target):
         return self.set_target(target)
+
+    def set_target(self, target):
+        """to be overridden"""
+        raise NotImplementedError