From ffaa9c83bdb8eb7ba92d249c89ff0c0d460d1ee3 Mon Sep 17 00:00:00 2001 From: Markus Zolliker Date: Thu, 17 Aug 2023 17:44:47 +0200 Subject: [PATCH] introduce FrozenParam For the case when the readback of a parameter does not reflect the change immediately. May also be used on Writable.target or Drivable.target with a short BUSY period. + bug fix in an error message in frappy.datatypes.IntRange Change-Id: I5e1c871629f9e3940ae80f35cb6307f404202b4a Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/31981 Tested-by: Jenkins Automated Tests Reviewed-by: Markus Zolliker --- frappy_psi/frozenparam.py | 123 ++++++++++++++++++++++++++++++++++++++ frappy_psi/triton.py | 27 ++++++--- 2 files changed, 142 insertions(+), 8 deletions(-) create mode 100644 frappy_psi/frozenparam.py diff --git a/frappy_psi/frozenparam.py b/frappy_psi/frozenparam.py new file mode 100644 index 0000000..a531154 --- /dev/null +++ b/frappy_psi/frozenparam.py @@ -0,0 +1,123 @@ +# ***************************************************************************** +# 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 +# ***************************************************************************** + +import time +import math +from frappy.core import Parameter, FloatRange, IntRange, Property +from frappy.errors import ProgrammingError + + +class FrozenParam(Parameter): + """workaround for lazy hardware + + Some hardware does not react nicely: when a parameter is changed, + and read back immediately, still the old value is returned. + This special parameter helps fixing this problem. + + Mechanism: + + - after a call to write_ for a short time ( * ) + the hardware is polled until the readback changes before the 'changed' + message is replied to the client + - if there is no change yet within short time, the 'changed' message is + set with the given value and further calls to read_ return also + this given value until the readback value has changed or until + sec have passed. + + For float parameters, the behaviour for small changes is improved + when the write_ method tries to return the (may be rounded) value, + as if it would be returned by the hardware. If this behaviour is not + known, or the programmer is too lazy to implement it, write_ + should return None or the given value. + Also it will help to adjust the datatype properties + 'absolute_resolution' and 'relative_resolution' to reasonable values. + """ + timeout = Property('timeout for freezing readback value', + FloatRange(0, unit='s'), default=30) + n_polls = Property("""number polls within write method""", + IntRange(0), default=1) + interval = Property("""interval for polls within write method + + the product n_polls * interval should not be more than a fraction of a second + in order not to block the connection for too long + """, + FloatRange(0, unit='s'), default=0.05) + new_value = None + previous_value = None + expire = 0 + is_float = True # assume float. will be fixed later + + def isclose(self, v1, v2): + if v1 == v2: + return True + if self.is_float: + dt = self.datatype + try: + return math.isclose(v1, v2, abs_tol=dt.absolute_tolerance, + rel_tol=dt.relative_tolerance) + except AttributeError: + # fix once for ever when datatype is not a float + self.is_float = False + return False + + def __set_name__(self, owner, name): + try: + rfunc = getattr(owner, f'read_{name}') + wfunc = getattr(owner, f'write_{name}') + except AttributeError: + raise ProgrammingError(f'FrozenParam: methods read_{name} and write_{name} must exist') from None + + super().__set_name__(owner, name) + + def read_wrapper(self, pname=name, rfunc=rfunc): + pobj = self.parameters[pname] + value = rfunc(self) + if pobj.new_value is None: + return value + if not pobj.isclose(value, pobj.new_value): + if value == pobj.previous_value: + if time.time() < pobj.expire: + return pobj.new_value + self.log.warning('%s readback did not change within %g sec', + pname, pobj.timeout) + else: + # value has changed, but is not matching new value + self.log.warning('%s readback changed from %r to %r but %r was given', + pname, pobj.previous_value, value, pobj.new_value) + # readback value has changed or returned value is roughly equal to the new value + pobj.new_value = None + return value + + def write_wrapper(self, value, wfunc=wfunc, rfunc=rfunc, read_wrapper=read_wrapper, pname=name): + pobj = self.parameters[pname] + pobj.previous_value = rfunc(self) + pobj.new_value = wfunc(self, value) + if pobj.new_value is None: # as wfunc is the unwrapped write_* method, the return value may be None + pobj.new_value = value + pobj.expire = time.time() + pobj.timeout + for cnt in range(pobj.n_polls): + if cnt: # we may be lucky, and the readback value has already changed + time.sleep(pobj.interval) + value = read_wrapper(self) + if pobj.new_value is None: + return value + return pobj.new_value + + setattr(owner, f'read_{name}', read_wrapper) + setattr(owner, f'write_{name}', write_wrapper) diff --git a/frappy_psi/triton.py b/frappy_psi/triton.py index 1852d9d..584c0a0 100644 --- a/frappy_psi/triton.py +++ b/frappy_psi/triton.py @@ -26,6 +26,7 @@ from frappy.datatypes import EnumType, FloatRange, StringType from frappy.lib.enum import Enum from frappy_psi.mercury import MercuryChannel, Mapped, off_on, HasInput from frappy_psi import mercury +from frappy_psi.frozenparam import FrozenParam actions = Enum(none=0, condense=1, circulate=2, collect=3) open_close = Mapped(CLOSE=0, OPEN=1) @@ -39,22 +40,23 @@ class Action(MercuryChannel, Writable): mix_channel = Property('mix channel', StringType(), 'T5') still_channel = Property('cool down channel', StringType(), 'T4') value = Parameter('running action', EnumType(actions)) - target = Parameter('action to do', EnumType(none=0, condense=1, collect=3), readonly=False) + target = FrozenParam('action to do', EnumType(none=0, condense=1, collect=3), readonly=False) _target = 0 def read_value(self): return self.query('SYS:DR:ACTN', actions_map) - def read_target(self): - return self._target + # as target is a FrozenParam, value might be still lag behind target + # but will be updated when changed from an other source + read_target = read_value def write_target(self, value): - self._target = value self.change('SYS:DR:CHAN:COOL', self.cooldown_channel, str) self.change('SYS:DR:CHAN:STIL', self.still_channel, str) self.change('SYS:DR:CHAN:MC', self.mix_channel, str) self.change('DEV:T5:TEMP:MEAS:ENAB', 'ON', str) - return self.change('SYS:DR:ACTN', value, actions_map) + self.change('SYS:DR:ACTN', value, actions_map) + return value # actions: # NONE (no action) @@ -74,7 +76,7 @@ class Action(MercuryChannel, Writable): class Valve(MercuryChannel, Drivable): kind = 'VALV' value = Parameter('valve state', EnumType(closed=0, opened=1)) - target = Parameter('valve target', EnumType(close=0, open=1)) + target = FrozenParam('valve target', EnumType(close=0, open=1)) _try_count = None @@ -108,6 +110,10 @@ class Valve(MercuryChannel, Drivable): self.change('DEV::VALV:SIG:STATE', self.target, open_close) return BUSY, 'waiting' + # as target is a FrozenParam, value might be still lag behind target + # but will be updated when changed from an other source + read_target = read_value + def write_target(self, value): if value != self.read_value(): self._try_count = 0 @@ -120,13 +126,18 @@ class Valve(MercuryChannel, Drivable): class Pump(MercuryChannel, Writable): kind = 'PUMP' value = Parameter('pump state', EnumType(off=0, on=1)) - target = Parameter('pump target', EnumType(off=0, on=1)) + target = FrozenParam('pump target', EnumType(off=0, on=1)) def read_value(self): return self.query('DEV::PUMP:SIG:STATE', off_on) + # as target is a FrozenParam, value might be still lag behind target + # but will be updated when changed from an other source + read_target = read_value + def write_target(self, value): - return self.change('DEV::PUMP:SIG:STATE', value, off_on) + self.change('DEV::PUMP:SIG:STATE', value, off_on) + return value def read_status(self): return IDLE, ''