simplify callbacks

on Module, use one single callback list 'paramsCallback' instead of
'valueCallbacks', 'errorCallbacks'. Redesign the mechanism to
avoid most of the closures.

Change-Id: Ie7f68f6bf97ab3f3cd961faa20b0e77730e5b37d
Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/33118
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>

frappy/extparams.py

@@ -159,7 +159,7 @@ class StructParam(Parameter):
                     for membername, param in structparam.paramdict.items():
                         setattr(modobj, param.name, value[membername])
 
-                modobj.valueCallbacks[self.name].append(cb)
+                modobj.addCallback(self.name, cb)
             else:
                 for membername, param in self.paramdict.items():
                     def cb(value, modobj=modobj, structparam=self, membername=membername):
@@ -168,7 +168,7 @@ class StructParam(Parameter):
                             prev[membername] = value
                             setattr(modobj, structparam.name, prev)
 
-                    modobj.valueCallbacks[param.name].append(cb)
+                    modobj.addCallback(param.name, cb)
 
 
 class FloatEnumParam(Parameter):
@@ -291,12 +291,12 @@ class FloatEnumParam(Parameter):
             return self
         return self.valuedict[instance.parameters[self.idx_name].value]
 
+    def trigger_setter(self, modobj, _):
+        # trigger update of float parameter on change of enum parameter
+        modobj.announceUpdate(self.name, getattr(modobj, self.name))
+
     def finish(self, modobj=None):
         """register callbacks for consistency"""
         super().finish(modobj)
         if modobj:
-            # trigger setter of float parameter on change of enum parameter
+            modobj.addCallback(self.idx_name, self.trigger_setter, modobj)
-            def cb(value, modobj=modobj, name=self.name):
-                setattr(modobj, name, getattr(modobj, name))
-
-            modobj.valueCallbacks[self.idx_name].append(cb)

frappy/modulebase.py

@@ -334,8 +334,7 @@ class Module(HasAccessibles):
         self.secNode = srv.secnode
         self.log = logger
         self.name = name
-        self.valueCallbacks = {}
+        self.paramCallbacks = {}
-        self.errorCallbacks = {}
         self.earlyInitDone = False
         self.initModuleDone = False
         self.startModuleDone = False
@@ -469,8 +468,7 @@ class Module(HasAccessibles):
         apply default when no value is given (in cfg or as Parameter argument)
         or complain, when cfg is needed
         """
-        self.valueCallbacks[pname] = []
+        self.paramCallbacks[pname] = []
-        self.errorCallbacks[pname] = []
         if isinstance(pobj, Limit):
             basepname = pname.rpartition('_')[0]
             baseparam = self.parameters.get(basepname)
@@ -535,68 +533,46 @@ class Module(HasAccessibles):
                     err.report_error = False
                     return  # no updates for repeated errors
                 err = secop_error(err)
-            elif not changed and timestamp < (pobj.timestamp or 0) + pobj.omit_unchanged_within:
+                value_err = value, err
-                # no change within short time -> omit
-                return
-            pobj.timestamp = timestamp or time.time()
-            if err:
-                callbacks = self.errorCallbacks
-                pobj.readerror = arg = err
             else:
-                callbacks = self.valueCallbacks
+                if not changed and timestamp < (pobj.timestamp or 0) + pobj.omit_unchanged_within:
-                arg = value
+                    # no change within short time -> omit
-                pobj.readerror = None
+                    return
+                value_err = (value,)
+            pobj.timestamp = timestamp or time.time()
+            pobj.readerror = err
+            for cbfunc, cbargs in self.paramCallbacks[pname]:
+                try:
+                    cbfunc(*cbargs, *value_err)
+                except Exception:
+                    pass
             if pobj.export:
                 self.updateCallback(self, pobj)
-            cblist = callbacks[pname]
+
-            for cb in cblist:
+    def addCallback(self, pname, callback_function, *args):
-                try:
+        self.paramCallbacks[pname].append((callback_function, args))
-                    cb(arg)
-                except Exception:
-                    # print(formatExtendedTraceback())
-                    pass
 
     def registerCallbacks(self, modobj, autoupdate=()):
         """register callbacks to another module <modobj>
 
-        - whenever a self.<param> changes:
+        whenever a self.<param> changes or changes its error state:
-          <modobj>.update_<param> is called with the new value as argument.
+        <modobj>.update_param(<value> [, <exc>]) is called,
-          If this method raises an exception, <modobj>.<param> gets into an error state.
+        where <value> is the new value and <exc> is given only in case of error.
-          If the method does not exist and <param> is in autoupdate,
+        if the method does not exist, and <param> is in autoupdate
-          <modobj>.<param> is updated to self.<param>
+        <modobj>.announceUpdate(<pname>, <value>, <exc>) is called
-        - whenever <self>.<param> gets into an error state:
+        with <exc> being None in case of no error.
-          <modobj>.error_update_<param> is called with the exception as argument.
-          If this method raises an error, <modobj>.<param> gets into an error state.
-          If this method does not exist, and <param> is in autoupdate,
-          <modobj>.<param> gets into the same error state as self.<param>
-        """
-        for pname in self.parameters:
-            errfunc = getattr(modobj, 'error_update_' + pname, None)
-            if errfunc:
-                def errcb(err, p=pname, efunc=errfunc):
-                    try:
-                        efunc(err)
-                    except Exception as e:
-                        modobj.announceUpdate(p, err=e)
-                self.errorCallbacks[pname].append(errcb)
-            else:
-                def errcb(err, p=pname):
-                    modobj.announceUpdate(p, err=err)
-                if pname in autoupdate:
-                    self.errorCallbacks[pname].append(errcb)
 
-            updfunc = getattr(modobj, 'update_' + pname, None)
+        Remark: when <modobj>.update_<param> does not accept the <exc> argument,
-            if updfunc:
+        nothing happens (the callback is catched by try / except).
-                def cb(value, ufunc=updfunc, efunc=errcb):
+        Any exceptions raised by the callback function are silently ignored.
-                    try:
+        """
-                        ufunc(value)
+        autoupdate = set(autoupdate)
-                    except Exception as e:
+        for pname in self.parameters:
-                        efunc(e)
+            cbfunc = getattr(modobj, 'update_' + pname, None)
-                self.valueCallbacks[pname].append(cb)
+            if cbfunc:
+                self.addCallback(pname, cbfunc)
             elif pname in autoupdate:
-                def cb(value, p=pname):
+                self.addCallback(pname, modobj.announceUpdate, pname)
-                    modobj.announceUpdate(p, value)
-                self.valueCallbacks[pname].append(cb)
 
     def isBusy(self, status=None):
         """helper function for treating substates of BUSY correctly"""
@@ -717,8 +693,8 @@ class Module(HasAccessibles):
         for mobj in polled_modules:
             pinfo = mobj.pollInfo = PollInfo(mobj.pollinterval, self.triggerPoll)
             # trigger a poll interval change when self.pollinterval changes.
-            if 'pollinterval' in mobj.valueCallbacks:
+            if 'pollinterval' in mobj.paramCallbacks:
-                mobj.valueCallbacks['pollinterval'].append(pinfo.update_interval)
+                mobj.addCallback('pollinterval', pinfo.update_interval)
 
             for pname, pobj in mobj.parameters.items():
                 rfunc = getattr(mobj, 'read_' + pname)

frappy/persistent.py

@@ -84,9 +84,7 @@ class PersistentMixin(Module):
             flag = getattr(pobj, 'persistent', False)
             if flag:
                 if flag == 'auto':
-                    def cb(value, m=self):
+                    self.addCallback(pname, self.saveParameters)
-                        m.saveParameters()
-                    self.valueCallbacks[pname].append(cb)
                 self.initData[pname] = pobj.value
                 if not pobj.given:
                     if pname in loaded:
@@ -129,16 +127,18 @@ class PersistentMixin(Module):
         self.writeInitParams()
         return loaded
 
-    def saveParameters(self):
+    def saveParameters(self, _=None):
         """save persistent parameters
 
         - to be called regularly explicitly by the module
         - the caller has to make sure that this is not called after
           a power down of the connected hardware before loadParameters
+
+        dummy argument to avoid closure for callback
         """
         if self.writeDict:
             # do not save before all values are written to the hw, as potentially
-            # factory default values were read in the mean time
+            # factory default values were read in the meantime
             return
         self.__save_params()
 

frappy_psi/softcal.py

@@ -27,7 +27,7 @@ import numpy as np
 from scipy.interpolate import splev, splrep  # pylint: disable=import-error
 
 from frappy.core import Attached, BoolType, Parameter, Readable, StringType, \
-    FloatRange
+    FloatRange, nopoll
 
 
 def linear(x):
@@ -195,35 +195,40 @@ class Sensor(Readable):
         if self.description == '_':
             self.description = f'{self.rawsensor!r} calibrated with curve {self.calib!r}'
 
-    def doPoll(self):
-        self.read_status()
-
     def write_calib(self, value):
         self._calib = CalCurve(value)
         return value
 
-    def update_value(self, value):
+    def _get_value(self, rawvalue):
         if self.abs:
-            value = abs(float(value))
+            rawvalue = abs(float(rawvalue))
-        self.value = self._calib(value)
+        return self._calib(rawvalue)
-        self._value_error = None
 
-    def error_update_value(self, err):
+    def _get_status(self, rawstatus):
-        if self.abs and str(err) == 'R_UNDER':  # hack: ignore R_UNDER from ls370
+        return rawstatus if self._value_error is None else (self.Status.ERROR, self._value_error)
-            self._value_error = None
-            return None
-        self._value_error = repr(err)
-        raise err
 
-    def update_status(self, value):
+    def update_value(self, rawvalue, err=None):
-        if self._value_error is None:
+        if err:
-            self.status = value
+            if self.abs and str(err) == 'R_UNDER':  # hack: ignore R_UNDER from ls370
+                self._value_error = None
+                return
+            err = repr(err)
         else:
-            self.status = self.Status.ERROR, self._value_error
+            try:
+                self.value = self._get_value(rawvalue)
+            except Exception as e:
+                err = repr(e)
+        if err != self._value_error:
+            self._value_error = err
+            self.status = self._get_status(self.rawsensor.status)
 
+    def update_status(self, rawstatus):
+        self.status = self._get_status(rawstatus)
+
+    @nopoll
     def read_value(self):
-        return self._calib(self.rawsensor.read_value())
+        return self._get_value(self.rawsensor.read_value())
 
+    @nopoll
     def read_status(self):
-        self.update_status(self.rawsensor.status)
+        return self._get_status(self.rawsensor.read_status())
-        return self.status

test/test_callbacks.py

@@ -0,0 +1,155 @@
+# *****************************************************************************
+#
+# This program is free software; you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation; either version 2 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc.,
+# 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+#
+# Module authors:
+#   Markus Zolliker <markus.zolliker@psi.ch>
+#
+# *****************************************************************************
+"""test parameter callbacks"""
+
+from test.test_modules import LoggerStub, ServerStub
+import pytest
+from frappy.core import Module, Parameter, FloatRange
+from frappy.errors import WrongTypeError
+
+
+WRONG_TYPE = WrongTypeError()
+
+
+class Mod(Module):
+    a = Parameter('', FloatRange())
+    b = Parameter('', FloatRange())
+    c = Parameter('', FloatRange())
+
+    def read_a(self):
+        raise WRONG_TYPE
+
+    def read_b(self):
+        raise WRONG_TYPE
+
+    def read_c(self):
+        raise WRONG_TYPE
+
+
+class Dbl(Module):
+    a = Parameter('', FloatRange())
+    b = Parameter('', FloatRange())
+    c = Parameter('', FloatRange())
+    _error_a = None
+    _value_b = None
+    _error_c = None
+
+    def update_a(self, value, err=None):
+        # treat error updates
+        try:
+            self.a = value * 2
+        except TypeError:  # value is None -> err
+            self.announceUpdate('a', None, err)
+
+    def update_b(self, value):
+        self._value_b = value
+        # error updates are ignored
+        self.b = value * 2
+
+
+def make(cls):
+    logger = LoggerStub()
+    srv = ServerStub({})
+    return cls('mod1', logger, {'description': ''}, srv)
+
+
+def test_simple_callback():
+    mod1 = make(Mod)
+    result = []
+
+    def cbfunc(arg1, arg2, value):
+        result[:] = arg1, arg2, value
+
+    mod1.addCallback('a', cbfunc, 'ARG1', 'arg2')
+
+    mod1.a = 1.5
+    assert result == ['ARG1', 'arg2', 1.5]
+
+    result.clear()
+
+    with pytest.raises(WrongTypeError):
+        mod1.read_a()
+
+    assert not result  # callback function is NOT called
+
+
+def test_combi_callback():
+    mod1 = make(Mod)
+    result = []
+
+    def cbfunc(arg1, arg2, value, err=None):
+        result[:] = arg1, arg2, value, err
+
+    mod1.addCallback('a', cbfunc, 'ARG1', 'arg2')
+
+    mod1.a = 1.5
+    assert result == ['ARG1', 'arg2', 1.5, None]
+
+    result.clear()
+
+    with pytest.raises(WrongTypeError):
+        mod1.read_a()
+
+    assert result[:3] == ['ARG1', 'arg2', None]  # callback function called with value None
+    assert isinstance(result[3], WrongTypeError)
+
+
+def test_autoupdate():
+    mod1 = make(Mod)
+    mod2 = make(Dbl)
+    mod1.registerCallbacks(mod2, autoupdate=['c'])
+
+    result = {}
+
+    def cbfunc(pname, *args):
+        result[pname] = args
+
+    for param in 'a', 'b', 'c':
+        mod2.addCallback(param, cbfunc, param)
+
+    # test update_a without error
+    mod1.a = 5
+    assert mod2.a == 10
+    assert result.pop('a') == (10,)
+
+    # test update_a with error
+    with pytest.raises(WrongTypeError):
+        mod1.read_a()
+
+    assert result.pop('a') == (None, WRONG_TYPE)
+
+    # test that update_b is ignored in case of error
+    mod1.b = 3
+    assert mod2.b == 6  # no error
+    assert result.pop('b') == (6,)
+
+    with pytest.raises(WrongTypeError):
+        mod1.read_b()
+    assert 'b' not in result
+
+    # test autoupdate
+    mod1.c = 3
+    assert mod2.c == 3
+    assert result['c'] == (3,)
+
+    with pytest.raises(WrongTypeError):
+        mod1.read_c()
+    assert result['c'] == (None, WRONG_TYPE)