From 6e7be6b4c701e8b9accb4f9ab67a6e45402f86b5 Mon Sep 17 00:00:00 2001 From: Markus Zolliker Date: Mon, 26 Feb 2024 16:42:39 +0100 Subject: [PATCH] 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 Tested-by: Jenkins Automated Tests --- frappy/extparams.py | 14 ++-- frappy/modulebase.py | 94 ++++++++++--------------- frappy/persistent.py | 10 +-- frappy_psi/softcal.py | 47 +++++++------ test/test_callbacks.py | 155 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 228 insertions(+), 92 deletions(-) create mode 100644 test/test_callbacks.py diff --git a/frappy/extparams.py b/frappy/extparams.py index f74e15d..d70eb8c 100644 --- a/frappy/extparams.py +++ b/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 - def cb(value, modobj=modobj, name=self.name): - setattr(modobj, name, getattr(modobj, name)) - - modobj.valueCallbacks[self.idx_name].append(cb) + modobj.addCallback(self.idx_name, self.trigger_setter, modobj) diff --git a/frappy/modulebase.py b/frappy/modulebase.py index 4b30fa3..0990a9b 100644 --- a/frappy/modulebase.py +++ b/frappy/modulebase.py @@ -334,8 +334,7 @@ class Module(HasAccessibles): self.secNode = srv.secnode self.log = logger self.name = name - self.valueCallbacks = {} - self.errorCallbacks = {} + self.paramCallbacks = {} 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.errorCallbacks[pname] = [] + self.paramCallbacks[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: - # no change within short time -> omit - return - pobj.timestamp = timestamp or time.time() - if err: - callbacks = self.errorCallbacks - pobj.readerror = arg = err + value_err = value, err else: - callbacks = self.valueCallbacks - arg = value - pobj.readerror = None + if not changed and timestamp < (pobj.timestamp or 0) + pobj.omit_unchanged_within: + # no change within short time -> omit + 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: - try: - cb(arg) - except Exception: - # print(formatExtendedTraceback()) - pass + + def addCallback(self, pname, callback_function, *args): + self.paramCallbacks[pname].append((callback_function, args)) def registerCallbacks(self, modobj, autoupdate=()): """register callbacks to another module - - whenever a self. changes: - .update_ is called with the new value as argument. - If this method raises an exception, . gets into an error state. - If the method does not exist and is in autoupdate, - . is updated to self. - - whenever . gets into an error state: - .error_update_ is called with the exception as argument. - If this method raises an error, . gets into an error state. - If this method does not exist, and is in autoupdate, - . gets into the same error state as self. - """ - 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) + whenever a self. changes or changes its error state: + .update_param( [, ]) is called, + where is the new value and is given only in case of error. + if the method does not exist, and is in autoupdate + .announceUpdate(, , ) is called + with being None in case of no error. - updfunc = getattr(modobj, 'update_' + pname, None) - if updfunc: - def cb(value, ufunc=updfunc, efunc=errcb): - try: - ufunc(value) - except Exception as e: - efunc(e) - self.valueCallbacks[pname].append(cb) + Remark: when .update_ does not accept the argument, + nothing happens (the callback is catched by try / except). + Any exceptions raised by the callback function are silently ignored. + """ + autoupdate = set(autoupdate) + for pname in self.parameters: + cbfunc = getattr(modobj, 'update_' + pname, None) + if cbfunc: + self.addCallback(pname, cbfunc) elif pname in autoupdate: - def cb(value, p=pname): - modobj.announceUpdate(p, value) - self.valueCallbacks[pname].append(cb) + self.addCallback(pname, modobj.announceUpdate, pname) 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: - mobj.valueCallbacks['pollinterval'].append(pinfo.update_interval) + if 'pollinterval' in mobj.paramCallbacks: + mobj.addCallback('pollinterval', pinfo.update_interval) for pname, pobj in mobj.parameters.items(): rfunc = getattr(mobj, 'read_' + pname) diff --git a/frappy/persistent.py b/frappy/persistent.py index e43897c..ed62a62 100644 --- a/frappy/persistent.py +++ b/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): - m.saveParameters() - self.valueCallbacks[pname].append(cb) + self.addCallback(pname, self.saveParameters) 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() diff --git a/frappy_psi/softcal.py b/frappy_psi/softcal.py index c60a3dd..6faabf5 100644 --- a/frappy_psi/softcal.py +++ b/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)) - self.value = self._calib(value) - self._value_error = None + rawvalue = abs(float(rawvalue)) + return self._calib(rawvalue) - def error_update_value(self, err): - if self.abs and str(err) == 'R_UNDER': # hack: ignore R_UNDER from ls370 - self._value_error = None - return None - self._value_error = repr(err) - raise err + def _get_status(self, rawstatus): + return rawstatus if self._value_error is None else (self.Status.ERROR, self._value_error) - def update_status(self, value): - if self._value_error is None: - self.status = value + def update_value(self, rawvalue, err=None): + if err: + 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.status + return self._get_status(self.rawsensor.read_status()) diff --git a/test/test_callbacks.py b/test/test_callbacks.py new file mode 100644 index 0000000..45ec419 --- /dev/null +++ b/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 +# +# ***************************************************************************** +"""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)