diff --git a/.github/workflows/code.yml b/.github/workflows/code.yml index d5caea94..238b893d 100644 --- a/.github/workflows/code.yml +++ b/.github/workflows/code.yml @@ -99,7 +99,7 @@ jobs: strategy: matrix: python-version: - - "3.8" + # - "3.8" # TODO: Breaking - "3.9" - "3.10" - "3.11" diff --git a/apstools/devices/eurotherm_2216e.py b/apstools/devices/eurotherm_2216e.py index 8afa6c77..c20c4c1f 100644 --- a/apstools/devices/eurotherm_2216e.py +++ b/apstools/devices/eurotherm_2216e.py @@ -83,7 +83,7 @@ def __init__(self, prefix="", *, tolerance=1, **kwargs): readback_pv="ignoreRBV", setpoint_pv="ignore", tolerance=tolerance, - update_target=False, + use_target=False, **kwargs, ) self.sensor.subscribe(self.cb_sensor) diff --git a/apstools/devices/lakeshore_controllers.py b/apstools/devices/lakeshore_controllers.py index 86e587ac..aa387ff7 100644 --- a/apstools/devices/lakeshore_controllers.py +++ b/apstools/devices/lakeshore_controllers.py @@ -68,7 +68,7 @@ class LakeShore336_LoopControl(PVPositionerSoftDoneWithStop): def __init__(self, *args, loop_number=None, timeout=10 * HOUR, **kwargs): self.loop_number = loop_number - super().__init__(*args, timeout=timeout, tolerance=0.1, readback_pv=f"IN{loop_number}", **kwargs) + super().__init__(*args, timeout=timeout, tolerance=0.1, use_target=True, readback_pv=f"IN{loop_number}", **kwargs) self._settle_time = 0 @property @@ -171,7 +171,7 @@ class LS340_LoopBase(PVPositionerSoftDoneWithStop): def __init__(self, *args, loop_number=None, timeout=10 * HOUR, **kwargs): self.loop_number = loop_number - super().__init__(*args, readback_pv="ignore", timeout=timeout, tolerance=0.1, **kwargs) + super().__init__(*args, readback_pv="ignore", timeout=timeout, use_target=True, tolerance=0.1, **kwargs) self._settle_time = 0 @property diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index e290b04f..666a8639 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -18,7 +18,6 @@ from ophyd import FormattedComponent from ophyd import PVPositioner from ophyd import Signal -from ophyd.signal import EpicsSignalBase # from ..tests import timed_pause @@ -31,36 +30,6 @@ TARGET_UNDEFINED = "undefined" -class _EpicsPositionerSetpointSignal(EpicsSignal): - """ - Special handling when PVPositionerSoftDone setpoint is changed. - - When the setpoint is changed, force`` done=False``. For any move, ``done`` - **must** transition to ``!= done_value``, then back to ``done_value``. - Without this response, a small move (within tolerance) will not return. - The ``cb_readback()`` method will compute ``done``. - """ - - def put(self, value, *args, **kwargs): - """Make sure 'done' signal goes False when setpoint is changed by us.""" - super().put(value, *args, **kwargs) - - self.parent.done.put(not self.parent.done_value) - if self.parent.update_target: - kwargs = {} - if issubclass(self.parent.target.__class__, EpicsSignalBase): - kwargs["wait"] = True # Signal.put() warns if kwargs are given - self.parent.target.put(value, **kwargs) - - # def get(self, *args, **kwargs): - # value = super().get(*args, **kwargs) - # if self.parent.update_target: - # target = self.parent.target.get() - # if target != TARGET_UNDEFINED: - # value = target - # return value - - class PVPositionerSoftDone(PVPositioner): """ PVPositioner that computes ``done`` as a soft signal. @@ -84,11 +53,11 @@ class PVPositionerSoftDone(PVPositioner): Defaults to ``10^(-1*precision)``, where ``precision = setpoint.precision``. - update_target : bool - ``True`` when this object updates the ``target`` Component directly. + use_target : bool + ``True`` when this object update the ``target`` Component directly. Use ``False`` if the ``target`` Component will be updated externally, such as by the controller when ``target`` is an ``EpicsSignal``. - Defaults to ``True``. + Defaults to ``False``. kwargs : Passed to `ophyd.PVPositioner` @@ -128,7 +97,7 @@ class PVPositionerSoftDone(PVPositioner): EpicsSignalRO, "{prefix}{_readback_pv}", kind="hinted", auto_monitor=True ) setpoint = FormattedComponent( - _EpicsPositionerSetpointSignal, "{prefix}{_setpoint_pv}", kind="normal", put_complete=True + EpicsSignal, "{prefix}{_setpoint_pv}", kind="normal", put_complete=True ) # fmt: on done = Component(Signal, value=True, kind="config") @@ -146,7 +115,7 @@ def __init__( readback_pv="", setpoint_pv="", tolerance=None, - update_target=True, + use_target=False, **kwargs, ): # fmt: off @@ -165,10 +134,12 @@ def __init__( # Make the default alias for the readback the name of the # positioner itself as in EpicsMotor. self.readback.name = self.name - self.update_target = update_target + self.use_target = use_target self.readback.subscribe(self.cb_readback) self.setpoint.subscribe(self.cb_setpoint) + self.setpoint.subscribe(self.cb_update_target, event_type="setpoint") + # cancel subscriptions before object is garbage collected weakref.finalize(self.readback, self.readback.unsubscribe_all) weakref.finalize(self.setpoint, self.setpoint.unsubscribe_all) @@ -192,6 +163,9 @@ def actual_tolerance(self): ) # fmt: on + def cb_update_target(self, value, *args, **kwargs): + self.target.put(value) + def cb_readback(self, *args, **kwargs): """ Called when readback changes (EPICS CA monitor event) or on-demand. @@ -213,12 +187,17 @@ def cb_setpoint(self, *args, **kwargs): """ Called when setpoint changes (EPICS CA monitor event). - This method is called when the setpoint is changed by this code or from - some other EPICS client. + When the setpoint is changed, force`` done=False``. For any move, ``done`` + **must** transition to ``!= done_value``, then back to ``done_value``. - The 'done' signal is set to False in the custom - _EpicsPositionerSetpointSignal class. + Without this response, a small move (within tolerance) will not return. + The ``cb_readback()`` method will compute ``done``. + + Since other code will also call this method, check the keys in kwargs + and do not react to the "wrong" signature. """ + if "value" in kwargs and "status" not in kwargs: + self.done.put(not self.done_value) logger.debug("cb_setpoint: done=%s, setpoint=%s", self.done.get(), self.setpoint.get()) @property @@ -233,7 +212,7 @@ def inposition(self): # Since this method must execute quickly, do NOT force # EPICS CA gets using `use_monitor=False`. rb = self.readback.get() - sp = self.setpoint.get() + sp = self.setpoint.get() if self.use_target is False else self.target.get() tol = self.actual_tolerance inpos = math.isclose(rb, sp, abs_tol=tol) logger.debug("inposition: inpos=%s rb=%s sp=%s tol=%s", inpos, rb, sp, tol) @@ -246,18 +225,12 @@ def precision(self): def _setup_move(self, position): """Move and do not wait until motion is complete (asynchronous)""" self.log.debug("%s.setpoint = %s", self.name, position) - - # Write the setpoint value. self.setpoint.put(position, wait=True) - # The 'done' and 'target' signals are handled by - # the custom '_EpicsPositionerSetpointSignal' class. - + self.done.put(False) if self.actuate is not None: self.log.debug("%s.actuate = %s", self.name, self.actuate_value) self.actuate.put(self.actuate_value, wait=False) - - # Force the first check for done. - self.cb_readback() + self.cb_readback() # This is needed to force the first check. class PVPositionerSoftDoneWithStop(PVPositionerSoftDone): @@ -279,14 +252,3 @@ def stop(self, *, success=False): self.setpoint.put(self.position) time.sleep(2.0 / 60) # two clock ticks, allow for EPICS record processing self.cb_readback() # re-evaluate soft done Signal - - -# ----------------------------------------------------------------------------- -# :author: Pete R. Jemian -# :email: jemian@anl.gov -# :copyright: (c) 2017-2024, UChicago Argonne, LLC -# -# Distributed under the terms of the Argonne National Laboratory Open Source License. -# -# The full license is in the file LICENSE.txt, distributed with this software. -# ----------------------------------------------------------------------------- diff --git a/apstools/devices/tests/test_eurotherm_2216e.py b/apstools/devices/tests/test_eurotherm_2216e.py index 59ceaa17..0941fad5 100644 --- a/apstools/devices/tests/test_eurotherm_2216e.py +++ b/apstools/devices/tests/test_eurotherm_2216e.py @@ -15,7 +15,7 @@ def test_device(): assert not euro.connected assert euro.tolerance.get() == 1 - assert euro.update_target is False + assert euro.use_target is False assert euro.target is None cns = """ diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index 376ea7a3..51e926ce 100644 --- a/apstools/devices/tests/test_positioner_soft_done.py +++ b/apstools/devices/tests/test_positioner_soft_done.py @@ -36,7 +36,7 @@ def pos(): """Test Positioner based on two analogout PVs.""" # fmt: off pos = PVPositionerSoftDoneWithStop( - PV_PREFIX, readback_pv="float1", setpoint_pv="float2", name="pos" + PV_PREFIX, readback_pv="float1", setpoint_pv="float2", use_target=True, name="pos" ) # fmt: on pos.wait_for_connection()