From 2fca548323712d64930ed2118e71c9fe40e5a7ab Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Wed, 3 Apr 2024 23:40:36 -0500 Subject: [PATCH 01/30] MNT #916 force done False at start of move --- apstools/devices/positioner_soft_done.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index cf3fb389b..bb5f4a2ce 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -227,12 +227,13 @@ def _setup_move(self, position): kwargs["wait"] = True # Signal.put() warns if kwargs are given self.target.put(position, **kwargs) self.setpoint.put(position, wait=True) + self.done.put(not self.done_value) # TODO: confirm 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) # This is needed because in a special case the setpoint.put does not # run the "sub_value" subscriptions. - self.cb_setpoint() + self.cb_setpoint() # FIXME: review this code self.cb_readback() # This is needed to force the first check. From ef5371a5d4ffe18c77451c9f2bfe6ce6abdef6b9 Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Wed, 3 Apr 2024 23:42:12 -0500 Subject: [PATCH 02/30] MNT #810 comment pytest.mark.local --- apstools/devices/tests/test_positioner_soft_done.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index b2c9908e0..17c5bf3bc 100644 --- a/apstools/devices/tests/test_positioner_soft_done.py +++ b/apstools/devices/tests/test_positioner_soft_done.py @@ -206,7 +206,7 @@ def test_structure(device, has_inposition): assert pos.tolerance.get() == -1 -@pytest.mark.local +# @pytest.mark.local def test_put_and_stop(rbv, prec, pos): assert pos.tolerance.get() == -1 assert pos.precision == prec.get() @@ -230,7 +230,7 @@ def motion(rb_initial, target, rb_mid=None): # force a stop now pos.stop() pos.cb_readback() - assert pos.setpoint.get(use_monitor=False) == rb_mid + assert pos.setpoint.get(use_monitor=False) == rb_mid # FIXME: fails sometimes 1.0 == 0.5 assert pos.readback.get(use_monitor=False) == rb_mid assert pos.position == rb_mid else: # interrupted move @@ -248,7 +248,7 @@ def motion(rb_initial, target, rb_mid=None): motion(1, 0, 0.5) # interrupted move -@pytest.mark.local +# @pytest.mark.local def test_move_and_stop_nonzero(rbv, pos): timed_pause() @@ -271,7 +271,7 @@ def test_move_and_stop_nonzero(rbv, pos): assert pos.inposition -@pytest.mark.local +# @pytest.mark.local def test_move_and_stopped_early(rbv, pos): def motion(target, delay, interrupt=False): timed_pause(0.1) # allow previous activities to settle down From 7093b19cf4c434e7f8e92fa2b278b282f7da60bf Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 4 Apr 2024 11:18:05 -0500 Subject: [PATCH 03/30] TST #810 remove local test markers and re-run CI --- apstools/devices/tests/test_positioner_soft_done.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index 17c5bf3bc..d469a4aca 100644 --- a/apstools/devices/tests/test_positioner_soft_done.py +++ b/apstools/devices/tests/test_positioner_soft_done.py @@ -206,7 +206,6 @@ def test_structure(device, has_inposition): assert pos.tolerance.get() == -1 -# @pytest.mark.local def test_put_and_stop(rbv, prec, pos): assert pos.tolerance.get() == -1 assert pos.precision == prec.get() @@ -248,7 +247,6 @@ def motion(rb_initial, target, rb_mid=None): motion(1, 0, 0.5) # interrupted move -# @pytest.mark.local def test_move_and_stop_nonzero(rbv, pos): timed_pause() @@ -271,7 +269,6 @@ def test_move_and_stop_nonzero(rbv, pos): assert pos.inposition -# @pytest.mark.local def test_move_and_stopped_early(rbv, pos): def motion(target, delay, interrupt=False): timed_pause(0.1) # allow previous activities to settle down From 5e6ed98cd8f63951ffb83b5774e0d2fd9b5bae36 Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 4 Apr 2024 13:03:42 -0500 Subject: [PATCH 04/30] MNT #810 remove SP & RB counters not needed for operations --- apstools/devices/positioner_soft_done.py | 24 ++++++++----------- .../tests/test_positioner_soft_done.py | 14 ++--------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index bb5f4a2ce..f4d1d93de 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -103,9 +103,6 @@ class PVPositionerSoftDone(PVPositioner): target = Component(Signal, value="None", kind="config") - _rb_count = 0 - _sp_count = 0 - def __init__( self, prefix="", @@ -171,8 +168,6 @@ def cb_readback(self, *args, **kwargs): if idle: return - self._rb_count += 1 - if self.inposition: self.done.put(self.done_value) if self.report_dmov_changes.get(): @@ -184,15 +179,11 @@ def cb_setpoint(self, *args, **kwargs): 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``. - - 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._sp_count += 1 + # Only update when the expected kwargs are received. self.done.put(not self.done_value) logger.debug("cb_setpoint: done=%s, setpoint=%s", self.done.get(), self.setpoint.get()) @@ -226,15 +217,20 @@ def _setup_move(self, position): if issubclass(self.target.__class__, EpicsSignalBase): kwargs["wait"] = True # Signal.put() warns if kwargs are given self.target.put(position, **kwargs) + + # Write the setpoint value. self.setpoint.put(position, wait=True) - self.done.put(not self.done_value) # TODO: confirm + # A new move requires done to become unset (here). + # Move is finished (in cb_readback()) when done is reset. + self.done.put(not self.done_value) + 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) - # This is needed because in a special case the setpoint.put does not - # run the "sub_value" subscriptions. + self.cb_setpoint() # FIXME: review this code - self.cb_readback() # This is needed to force the first check. + # Force the first check for done. + self.cb_readback() class PVPositionerSoftDoneWithStop(PVPositionerSoftDone): diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index d469a4aca..4a96954b1 100644 --- a/apstools/devices/tests/test_positioner_soft_done.py +++ b/apstools/devices/tests/test_positioner_soft_done.py @@ -93,9 +93,8 @@ def confirm_in_position(p, dt): p.readback.get(use_monitor=False) # force a read from the IOC p.cb_readback() # update self.done - # collect these values at one instant (as close as possible). - c_rb = p._rb_count # do not expect any new callbacks during this method - c_sp = p._sp_count + # Collect these values at one instant (as close in time as possible). + # Do not expect any new callbacks during this function. dmov = p.done.get() rb = p.readback.get() sp = p.setpoint.get() @@ -106,18 +105,13 @@ def confirm_in_position(p, dt): f"{p.name=}" f" {rb=:.5f} {sp=:.5f} {tol=}" f" {dt=:.4f}s" - f" {p._sp_count=}" - f" {p._rb_count=}" f" {p.done=}" f" {p.done_value=}" f" {time.time()=:.4f}" ) # fmt: on - assert p._rb_count == c_rb, diagnostics - assert p._sp_count == c_sp, diagnostics assert dmov == p.done_value, diagnostics - # assert math.isclose(rb, sp, abs_tol=tol), diagnostics @run_in_thread @@ -212,9 +206,7 @@ def test_put_and_stop(rbv, prec, pos): def motion(rb_initial, target, rb_mid=None): rbv.put(rb_initial) # make the readback to different - c_sp = pos._sp_count pos.setpoint.put(target) - assert pos._sp_count == c_sp + 1 assert math.isclose(pos.readback.get(use_monitor=False), rb_initial, abs_tol=0.02) assert math.isclose(pos.setpoint.get(use_monitor=False), target, abs_tol=0.02) assert pos.done.get() != pos.done_value @@ -347,11 +339,9 @@ def test_position_sequence_calcpos(target, calcpos): def motion(p, goal): timed_pause(0.1) # allow previous activities to settle down - c_sp = p._sp_count t0 = time.time() status = p.move(goal) dt = time.time() - t0 - assert p._sp_count == c_sp + 1 assert status.elapsed > 0, str(status) assert status.done, str(status) From 20c2fa5569c0931a560d978449747e0a480c2599 Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 4 Apr 2024 13:06:11 -0500 Subject: [PATCH 05/30] MNT #810 remove unnecessary call --- apstools/devices/positioner_soft_done.py | 1 - 1 file changed, 1 deletion(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index f4d1d93de..1fe219dce 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -228,7 +228,6 @@ def _setup_move(self, position): self.log.debug("%s.actuate = %s", self.name, self.actuate_value) self.actuate.put(self.actuate_value, wait=False) - self.cb_setpoint() # FIXME: review this code # Force the first check for done. self.cb_readback() From edeef539aceb30ac43b2435c170a64ba7981a027 Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 4 Apr 2024 13:10:12 -0500 Subject: [PATCH 06/30] DOC #916 update the release notes --- CHANGES.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index fda58dd04..2eeab5007 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -28,7 +28,7 @@ describe future plans. 1.6.19 ****** -release expected by 2024-04-02 +release expected by 2024-04-12 Fixes ----- @@ -36,6 +36,7 @@ Fixes * lineup2() should work with low intensity peaks. * lineup2() would raise ZeroDivideError in some cases. * Increase minimum aps-dm-api version to 8. +* PVPositionerSoftDone should set 'done' to False at start of a move. Maintenance ----------- From c18d86073e4b1eb810ea73ac1c33c6135dd6cead Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 4 Apr 2024 13:36:00 -0500 Subject: [PATCH 07/30] TST #810 adjust for observed CI failure --- apstools/devices/tests/test_positioner_soft_done.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index 4a96954b1..2157cf7ed 100644 --- a/apstools/devices/tests/test_positioner_soft_done.py +++ b/apstools/devices/tests/test_positioner_soft_done.py @@ -263,6 +263,12 @@ def test_move_and_stop_nonzero(rbv, pos): def test_move_and_stopped_early(rbv, pos): def motion(target, delay, interrupt=False): + """ + Test moving pos to target. Update rbv after delay. + + If interrupt is True, stop the move before it is done + (at a time that is less than the 'delay' value). + """ timed_pause(0.1) # allow previous activities to settle down t0 = time.time() @@ -275,7 +281,7 @@ def motion(target, delay, interrupt=False): rb_new = pos.readback.get(use_monitor=False) arrived = math.isclose(rb_new, target, abs_tol=pos.actual_tolerance) # fmt: on - if interrupt: + if interrupt and not status.done: assert not status.done assert not status.success assert not arrived, f"{dt=:.3f}" From 71a2531f21fda2165b22fe8f8319212cfd46c40a Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 4 Apr 2024 17:41:31 -0500 Subject: [PATCH 08/30] TST #810 discard unnecessary test --- apstools/devices/tests/test_positioner_soft_done.py | 1 - 1 file changed, 1 deletion(-) diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index 2157cf7ed..e11e6ecf6 100644 --- a/apstools/devices/tests/test_positioner_soft_done.py +++ b/apstools/devices/tests/test_positioner_soft_done.py @@ -282,7 +282,6 @@ def motion(target, delay, interrupt=False): arrived = math.isclose(rb_new, target, abs_tol=pos.actual_tolerance) # fmt: on if interrupt and not status.done: - assert not status.done assert not status.success assert not arrived, f"{dt=:.3f}" pos.stop() From 9e8a79398bb432523508c1113159a219fe15d957 Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Tue, 9 Apr 2024 12:43:42 -0500 Subject: [PATCH 09/30] TST #810 not failing now --- apstools/devices/tests/test_positioner_soft_done.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index e11e6ecf6..a48baba05 100644 --- a/apstools/devices/tests/test_positioner_soft_done.py +++ b/apstools/devices/tests/test_positioner_soft_done.py @@ -221,7 +221,7 @@ def motion(rb_initial, target, rb_mid=None): # force a stop now pos.stop() pos.cb_readback() - assert pos.setpoint.get(use_monitor=False) == rb_mid # FIXME: fails sometimes 1.0 == 0.5 + assert pos.setpoint.get(use_monitor=False) == rb_mid assert pos.readback.get(use_monitor=False) == rb_mid assert pos.position == rb_mid else: # interrupted move From 64a3c64205ea605d9740593695f91f415ae7a96b Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 11 Apr 2024 14:51:30 -0500 Subject: [PATCH 10/30] MNT #954 refactor setpoint into custom subclass --- apstools/devices/positioner_soft_done.py | 57 ++++++++++++++++-------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index 1fe219dce..bd4313703 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -25,6 +25,35 @@ logger = logging.getLogger(__name__) +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() + value = target or value + return value + + class PVPositionerSoftDone(PVPositioner): """ PVPositioner that computes ``done`` as a soft signal. @@ -49,7 +78,7 @@ class PVPositionerSoftDone(PVPositioner): Defaults to ``10^(-1*precision)``, where ``precision = setpoint.precision``. update_target : bool - ``True`` when this object update the ``target`` Component directly. + ``True`` when this object updates 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``. @@ -92,7 +121,7 @@ class PVPositionerSoftDone(PVPositioner): EpicsSignalRO, "{prefix}{_readback_pv}", kind="hinted", auto_monitor=True ) setpoint = FormattedComponent( - EpicsSignal, "{prefix}{_setpoint_pv}", kind="normal", put_complete=True + _EpicsPositionerSetpointSignal, "{prefix}{_setpoint_pv}", kind="normal", put_complete=True ) # fmt: on done = Component(Signal, value=True, kind="config") @@ -101,7 +130,7 @@ class PVPositionerSoftDone(PVPositioner): tolerance = Component(Signal, value=-1, kind="config") report_dmov_changes = Component(Signal, value=False, kind="omitted") - target = Component(Signal, value="None", kind="config") + target = Component(Signal, value=None, kind="config") def __init__( self, @@ -177,14 +206,12 @@ def cb_setpoint(self, *args, **kwargs): """ Called when setpoint changes (EPICS CA monitor event). - 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``. + This method is called when the setpoint is changed by this code or from + some other EPICS client. + + The 'done' signal is set to False in the custom + _EpicsPositionerSetpointSignal class. """ - if "value" in kwargs and "status" not in kwargs: - # Only update when the expected kwargs are received. - self.done.put(not self.done_value) logger.debug("cb_setpoint: done=%s, setpoint=%s", self.done.get(), self.setpoint.get()) @property @@ -212,17 +239,11 @@ 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) - if self.update_target: - kwargs = {} - if issubclass(self.target.__class__, EpicsSignalBase): - kwargs["wait"] = True # Signal.put() warns if kwargs are given - self.target.put(position, **kwargs) # Write the setpoint value. self.setpoint.put(position, wait=True) - # A new move requires done to become unset (here). - # Move is finished (in cb_readback()) when done is reset. - self.done.put(not self.done_value) + # The 'done' and 'target' signals are handled by + # the custom '_EpicsPositionerSetpointSignal' class. if self.actuate is not None: self.log.debug("%s.actuate = %s", self.name, self.actuate_value) From 70a6dc193d86c8d5747af27febdcc61fa1a8522d Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 11 Apr 2024 14:51:50 -0500 Subject: [PATCH 11/30] TST #954 respond to refactor --- apstools/devices/tests/test_positioner_soft_done.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index a48baba05..b7d0921a5 100644 --- a/apstools/devices/tests/test_positioner_soft_done.py +++ b/apstools/devices/tests/test_positioner_soft_done.py @@ -196,7 +196,7 @@ def test_structure(device, has_inposition): assert pos.setpoint.pvname == "v" assert pos.done.get() is True assert pos.done_value is True - assert pos.target.get() == "None" + assert pos.target.get() is None assert pos.tolerance.get() == -1 From 3fa21503b3bf1a5ba474846fcfab2c5c5ed506c8 Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 11 Apr 2024 15:04:49 -0500 Subject: [PATCH 12/30] TST #954 more tests to change since target=None is default now --- apstools/devices/tests/test_lakeshores.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apstools/devices/tests/test_lakeshores.py b/apstools/devices/tests/test_lakeshores.py index b7617a896..9338d9102 100644 --- a/apstools/devices/tests/test_lakeshores.py +++ b/apstools/devices/tests/test_lakeshores.py @@ -16,8 +16,8 @@ def test_lakeshore_336(): assert not t336.connected # Signal components - assert t336.loop1.target.get() == "None" - assert t336.loop2.target.get() == "None" + assert t336.loop1.target.get() == None + assert t336.loop2.target.get() == None assert t336.loop1.tolerance.get() == 0.1 assert t336.loop2.tolerance.get() == 0.1 @@ -68,8 +68,8 @@ def test_lakeshore_340(): assert not t340.connected # Signal components - assert t340.control.target.get() == "None" - assert t340.sample.target.get() == "None" + assert t340.control.target.get() == None + assert t340.sample.target.get() == None assert t340.control.tolerance.get() == 0.1 assert t340.sample.tolerance.get() == 0.1 From 10788f24c77cf9c6ff7f85b8423c0d4d182b1333 Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 11 Apr 2024 15:22:18 -0500 Subject: [PATCH 13/30] TST #954 stylistic change of test for None --- apstools/devices/tests/test_lakeshores.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apstools/devices/tests/test_lakeshores.py b/apstools/devices/tests/test_lakeshores.py index 9338d9102..ef93ffe0e 100644 --- a/apstools/devices/tests/test_lakeshores.py +++ b/apstools/devices/tests/test_lakeshores.py @@ -16,8 +16,8 @@ def test_lakeshore_336(): assert not t336.connected # Signal components - assert t336.loop1.target.get() == None - assert t336.loop2.target.get() == None + assert t336.loop1.target.get() is None + assert t336.loop2.target.get() is None assert t336.loop1.tolerance.get() == 0.1 assert t336.loop2.tolerance.get() == 0.1 @@ -68,8 +68,8 @@ def test_lakeshore_340(): assert not t340.connected # Signal components - assert t340.control.target.get() == None - assert t340.sample.target.get() == None + assert t340.control.target.get() is None + assert t340.sample.target.get() is None assert t340.control.tolerance.get() == 0.1 assert t340.sample.tolerance.get() == 0.1 From 162cb3dbe184afe58aa4d180f1082574bc161be4 Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 11 Apr 2024 16:23:34 -0500 Subject: [PATCH 14/30] MNT #954 TARGET_UNDEFINED, remove custom get method --- apstools/devices/positioner_soft_done.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index bd4313703..0af3b6a6e 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -24,6 +24,12 @@ logger = logging.getLogger(__name__) +# Must use a data type that can be serialized as json (Python's None cannot be serialized) +# This ValueError: Cannot determine the appropriate bluesky-friendly data type for value +# None of Python type . Supported types include: int, float, str, and +# iterables such as list, tuple, np.ndarray, and so on. +TARGET_UNDEFINED = "undefined" + class _EpicsPositionerSetpointSignal(EpicsSignal): """ @@ -46,12 +52,13 @@ def put(self, value, *args, **kwargs): 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() - value = target or value - return value + # 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): @@ -130,7 +137,7 @@ class PVPositionerSoftDone(PVPositioner): tolerance = Component(Signal, value=-1, kind="config") report_dmov_changes = Component(Signal, value=False, kind="omitted") - target = Component(Signal, value=None, kind="config") + target = Component(Signal, value=TARGET_UNDEFINED, kind="config") def __init__( self, From 68d0d3e2ba56eca5ab98cd7c5041714f8ec193e7 Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 11 Apr 2024 16:26:28 -0500 Subject: [PATCH 15/30] TST #954 TARGET_UNDEFINED symbol --- apstools/devices/tests/test_lakeshores.py | 9 +++++---- apstools/devices/tests/test_positioner_soft_done.py | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/apstools/devices/tests/test_lakeshores.py b/apstools/devices/tests/test_lakeshores.py index ef93ffe0e..89c369511 100644 --- a/apstools/devices/tests/test_lakeshores.py +++ b/apstools/devices/tests/test_lakeshores.py @@ -7,6 +7,7 @@ from ...tests import IOC_GP from ..lakeshore_controllers import LakeShore336Device from ..lakeshore_controllers import LakeShore340Device +from ..positioner_soft_done import TARGET_UNDEFINED PV_PREFIX = f"phony:{IOC_GP}lakeshore:" @@ -16,8 +17,8 @@ def test_lakeshore_336(): assert not t336.connected # Signal components - assert t336.loop1.target.get() is None - assert t336.loop2.target.get() is None + assert t336.loop1.target.get() == TARGET_UNDEFINED + assert t336.loop2.target.get() == TARGET_UNDEFINED assert t336.loop1.tolerance.get() == 0.1 assert t336.loop2.tolerance.get() == 0.1 @@ -68,8 +69,8 @@ def test_lakeshore_340(): assert not t340.connected # Signal components - assert t340.control.target.get() is None - assert t340.sample.target.get() is None + assert t340.control.target.get() == TARGET_UNDEFINED + assert t340.sample.target.get() == TARGET_UNDEFINED assert t340.control.tolerance.get() == 0.1 assert t340.sample.tolerance.get() == 0.1 diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index b7d0921a5..376ea7a35 100644 --- a/apstools/devices/tests/test_positioner_soft_done.py +++ b/apstools/devices/tests/test_positioner_soft_done.py @@ -13,6 +13,7 @@ from ...utils import run_in_thread from ..positioner_soft_done import PVPositionerSoftDone from ..positioner_soft_done import PVPositionerSoftDoneWithStop +from ..positioner_soft_done import TARGET_UNDEFINED PV_PREFIX = f"{IOC_GP}gp:" delay_active = False @@ -196,7 +197,7 @@ def test_structure(device, has_inposition): assert pos.setpoint.pvname == "v" assert pos.done.get() is True assert pos.done_value is True - assert pos.target.get() is None + assert pos.target.get() == TARGET_UNDEFINED assert pos.tolerance.get() == -1 From 9c26b8a016dc93c22086f655acdd4cfbe88ac265 Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Thu, 11 Apr 2024 16:27:08 -0500 Subject: [PATCH 16/30] TST #954 rename class so does not start with "Test" --- apstools/plans/tests/test_alignment.py | 92 +++++++++++++------------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/apstools/plans/tests/test_alignment.py b/apstools/plans/tests/test_alignment.py index f85e13d13..f32ab8d02 100644 --- a/apstools/plans/tests/test_alignment.py +++ b/apstools/plans/tests/test_alignment.py @@ -100,15 +100,15 @@ def test_SynPseudoVoigt_randomize(): assert signal.noise_multiplier == 1 -TestParameters = collections.namedtuple("TestParameters", "signal mover start finish npts feature") -parms_slower = TestParameters(noisy, m1, -1.2, 1.2, 11, "max") -parms_faster = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max") -parms_cen = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen") -parms_com = TestParameters(pvoigt, axis, -1.2, 1.2, 51, "com") +_TestParameters = collections.namedtuple("TestParameters", "signal mover start finish npts feature") +parms_slower = _TestParameters(noisy, m1, -1.2, 1.2, 11, "max") +parms_faster = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max") +parms_cen = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen") +parms_com = _TestParameters(pvoigt, axis, -1.2, 1.2, 51, "com") @pytest.mark.parametrize("parms", [parms_slower, parms_faster, parms_cen, parms_com]) -def test_direct_implementation_with_rel_scan(parms: TestParameters): +def test_direct_implementation_with_rel_scan(parms: _TestParameters): RE(bps.mv(parms.mover, 0)) assert get_position(parms.mover) == 0.0 @@ -132,17 +132,17 @@ def test_direct_implementation_with_rel_scan(parms: TestParameters): assert math.isclose(position, center, abs_tol=0.001) -TestParameters = collections.namedtuple("TestParameters", "signals mover start finish npts feature rescan") -parms_motor_slower = TestParameters(noisy, m1, -1.2, 1.2, 11, "max", True) -parms_ao_faster = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", True) -parms_ao_max = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", True) -parms_det_list_of_1 = TestParameters([pvoigt], axis, -1.2, 1.2, 11, "max", True) -parms_det_list_of_3 = TestParameters([pvoigt, noisy, scaler1], axis, -1.2, 1.2, 11, "max", True) -parms_det_tuple = TestParameters((pvoigt), axis, -1.2, 1.2, 11, "max", True) -parms_cen = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", False) -parms_com = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "com", False) -parms_max = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", False) -parms_min___pathological = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "min", False) +_TestParameters = collections.namedtuple("TestParameters", "signals mover start finish npts feature rescan") +parms_motor_slower = _TestParameters(noisy, m1, -1.2, 1.2, 11, "max", True) +parms_ao_faster = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", True) +parms_ao_max = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", True) +parms_det_list_of_1 = _TestParameters([pvoigt], axis, -1.2, 1.2, 11, "max", True) +parms_det_list_of_3 = _TestParameters([pvoigt, noisy, scaler1], axis, -1.2, 1.2, 11, "max", True) +parms_det_tuple = _TestParameters((pvoigt), axis, -1.2, 1.2, 11, "max", True) +parms_cen = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", False) +parms_com = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "com", False) +parms_max = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", False) +parms_min___pathological = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "min", False) @pytest.mark.parametrize( @@ -160,7 +160,7 @@ def test_direct_implementation_with_rel_scan(parms: TestParameters): parms_min___pathological, ], ) -def test_lineup(parms: TestParameters): +def test_lineup(parms: _TestParameters): if isinstance(parms.signals, SynPseudoVoigt): parms.signals.randomize_parameters(scale=250_000, bkg=0.000_000_000_1) else: @@ -197,17 +197,17 @@ def test_lineup(parms: TestParameters): # # assert lo <= position <= hi, f"{bec=} {bec.peaks=} {position=} {center=} {width=}" -TestParameters = collections.namedtuple("TestParameters", "signals, mover, start, finish, npts, feature, nscans") -parms_motor_slower = TestParameters(noisy, m1, -1.2, 1.2, 11, "max", 2) -parms_ao_faster = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", 2) -parms_ao_max = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", 2) -parms_det_list_of_1 = TestParameters([pvoigt], axis, -1.2, 1.2, 11, "max", 2) -parms_det_list_of_3 = TestParameters([pvoigt, noisy, scaler1], axis, -1.2, 1.2, 11, "max", 2) -parms_det_tuple = TestParameters((pvoigt), axis, -1.2, 1.2, 11, "max", 2) -parms_cen = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", 1) -parms_com = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "com", 1) -parms_max = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", 1) -parms_min___pathological = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "min", 1) +_TestParameters = collections.namedtuple("TestParameters", "signals, mover, start, finish, npts, feature, nscans") +parms_motor_slower = _TestParameters(noisy, m1, -1.2, 1.2, 11, "max", 2) +parms_ao_faster = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", 2) +parms_ao_max = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", 2) +parms_det_list_of_1 = _TestParameters([pvoigt], axis, -1.2, 1.2, 11, "max", 2) +parms_det_list_of_3 = _TestParameters([pvoigt, noisy, scaler1], axis, -1.2, 1.2, 11, "max", 2) +parms_det_tuple = _TestParameters((pvoigt), axis, -1.2, 1.2, 11, "max", 2) +parms_cen = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", 1) +parms_com = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "com", 1) +parms_max = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", 1) +parms_min___pathological = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "min", 1) @pytest.mark.parametrize( @@ -225,7 +225,7 @@ def test_lineup(parms: TestParameters): parms_min___pathological, ], ) -def test_lineup2(parms: TestParameters): +def test_lineup2(parms: _TestParameters): if isinstance(parms.signals, SynPseudoVoigt): parms.signals.randomize_parameters(scale=250_000, bkg=0.000_000_000_1) else: @@ -338,23 +338,23 @@ class ImitatorForTesting(Device): assert results.report() is None -TestParameters = collections.namedtuple( +_TestParameters = collections.namedtuple( "TestParameters", "peak base noise center sigma xlo xhi npts nscans tol outcome" ) -parms_signal_but_high_background = TestParameters(1e5, 1e6, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) -parms_model_peak = TestParameters(1e5, 0, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) -parms_high_background_poor_resolution = TestParameters(1e5, 1e4, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.1, True) -parms_not_much_better = TestParameters(1e5, 1e4, 10, 0.1, 0.2, -0.7, 0.5, 11, 2, 0.05, True) -parms_neg_peak_1x_base = TestParameters(-1e5, -1e4, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.1, True) -parms_neg_base = TestParameters(1e5, -10, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) -parms_small_signal_zero_base = TestParameters(1e-5, 0, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) -parms_neg_small_signal_zero_base = TestParameters(-1e5, 0, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) -parms_small_signal_finite_base = TestParameters(1e-5, 1e-7, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) -parms_no_signal_only_noise = TestParameters(0, 0, 1e-8, 0.1, 0.2, -1.0, 0.5, 11, 1, 0.05, False) -parms_bkg_plus_noise = TestParameters(0, 1, 0.1, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) -parms_bkg_plus_big_noise = TestParameters(0, 1, 100, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) -parms_no_signal__ZeroDivisionError = TestParameters(0, 0, 0, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.005, None) -parms_bkg_only__ZeroDivisionError = TestParameters(0, 1, 0, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.005, None) +parms_signal_but_high_background = _TestParameters(1e5, 1e6, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) +parms_model_peak = _TestParameters(1e5, 0, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) +parms_high_background_poor_resolution = _TestParameters(1e5, 1e4, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.1, True) +parms_not_much_better = _TestParameters(1e5, 1e4, 10, 0.1, 0.2, -0.7, 0.5, 11, 2, 0.05, True) +parms_neg_peak_1x_base = _TestParameters(-1e5, -1e4, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.1, True) +parms_neg_base = _TestParameters(1e5, -10, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) +parms_small_signal_zero_base = _TestParameters(1e-5, 0, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) +parms_neg_small_signal_zero_base = _TestParameters(-1e5, 0, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) +parms_small_signal_finite_base = _TestParameters(1e-5, 1e-7, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) +parms_no_signal_only_noise = _TestParameters(0, 0, 1e-8, 0.1, 0.2, -1.0, 0.5, 11, 1, 0.05, False) +parms_bkg_plus_noise = _TestParameters(0, 1, 0.1, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) +parms_bkg_plus_big_noise = _TestParameters(0, 1, 100, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) +parms_no_signal__ZeroDivisionError = _TestParameters(0, 0, 0, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.005, None) +parms_bkg_only__ZeroDivisionError = _TestParameters(0, 1, 0, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.005, None) @pytest.mark.parametrize( @@ -376,7 +376,7 @@ class ImitatorForTesting(Device): parms_bkg_only__ZeroDivisionError, ], ) -def test_lineup2_signal_permutations(parms: TestParameters): +def test_lineup2_signal_permutations(parms: _TestParameters): starting_position = 0.0 m1.move(starting_position) time.sleep(1) # without this, the test IOC crashes, sometimes From 80e86e631c45922e80f2cb03bb7ce7de8e0094eb Mon Sep 17 00:00:00 2001 From: Pete R Jemian Date: Tue, 9 Jul 2024 13:22:15 -0500 Subject: [PATCH 17/30] GIT #916 ignore dev_* markdown files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index c99e3e057..c1cd66a6c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ # local developer code dev_*.py dev_*.ipynb +dev_*.md # local note-taking _notes*.md From 40acf1e7491a2fd6e2a02b909d476e1d93d4265b Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Thu, 11 Jul 2024 14:41:58 -0500 Subject: [PATCH 18/30] update positioner_soft_done --- apstools/devices/positioner_soft_done.py | 94 +++++++++--------------- 1 file changed, 35 insertions(+), 59 deletions(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index e290b04fd..e6f5b8bbe 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -31,36 +31,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. @@ -85,7 +55,7 @@ class PVPositionerSoftDone(PVPositioner): Defaults to ``10^(-1*precision)``, where ``precision = setpoint.precision``. update_target : bool - ``True`` when this object updates the ``target`` Component directly. + ``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``. @@ -128,7 +98,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") @@ -139,6 +109,9 @@ class PVPositionerSoftDone(PVPositioner): target = Component(Signal, value=TARGET_UNDEFINED, kind="config") + _rb_count = 0 + _sp_count = 0 + def __init__( self, prefix="", @@ -169,6 +142,7 @@ def __init__( self.readback.subscribe(self.cb_readback) self.setpoint.subscribe(self.cb_setpoint) + self.setpoint.subscribe(self.cb_update_target) # 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 +166,9 @@ def actual_tolerance(self): ) # fmt: on + def cb_update_target(self, value, *args, **kwargs): + self.target.put(value, wait=True) + def cb_readback(self, *args, **kwargs): """ Called when readback changes (EPICS CA monitor event) or on-demand. @@ -204,22 +181,30 @@ def cb_readback(self, *args, **kwargs): if idle: return + self._rb_count += 1 + if self.inposition: self.done.put(self.done_value) - if self.report_dmov_changes.get(): - logger.debug(f"{self.name} reached: {True}") + # if self.report_dmov_changes.get(): + # logger.debug(f"{self.name} reached: {True}") 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. """ - logger.debug("cb_setpoint: done=%s, setpoint=%s", self.done.get(), self.setpoint.get()) + if "value" in kwargs and "status" not in kwargs: + self._sp_count += 1 + self.done.put(not self.done_value) + # logger.debug("cb_setpoint: done=%s, setpoint=%s", self.done.get(), self.setpoint.get()) @property def inposition(self): @@ -236,7 +221,7 @@ def inposition(self): sp = self.setpoint.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) + # logger.debug("inposition: inpos=%s rb=%s sp=%s tol=%s", inpos, rb, sp, tol) return inpos @property @@ -246,18 +231,20 @@ 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. + # TODO: The stuff in this if statement might not be necessary anymore. + # if self.update_target: + # kwargs = {} + # if issubclass(self.target.__class__, EpicsSignalBase): + # kwargs["wait"] = True # Signal.put() warns if kwargs are given + # self.target.put(position, **kwargs) self.setpoint.put(position, wait=True) - # The 'done' and 'target' signals are handled by - # the custom '_EpicsPositionerSetpointSignal' class. - 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() + # This is needed because in a special case the setpoint.put does not + # run the "sub_value" subscriptions. + self.cb_setpoint() + self.cb_readback() # This is needed to force the first check. class PVPositionerSoftDoneWithStop(PVPositionerSoftDone): @@ -279,14 +266,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. -# ----------------------------------------------------------------------------- From c77c5038164fde3ac573fcb73f1d4c323d517a9a Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Thu, 11 Jul 2024 14:47:37 -0500 Subject: [PATCH 19/30] remove unused import --- apstools/devices/positioner_soft_done.py | 1 - 1 file changed, 1 deletion(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index e6f5b8bbe..c07d17736 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 From 029d3efd3c86328bf5405cbc57669b19cea5294a Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Thu, 11 Jul 2024 15:34:06 -0500 Subject: [PATCH 20/30] re-add logger --- apstools/devices/positioner_soft_done.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index c07d17736..b5191c350 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -184,8 +184,8 @@ def cb_readback(self, *args, **kwargs): if self.inposition: self.done.put(self.done_value) - # if self.report_dmov_changes.get(): - # logger.debug(f"{self.name} reached: {True}") + if self.report_dmov_changes.get(): + logger.debug(f"{self.name} reached: {True}") def cb_setpoint(self, *args, **kwargs): """ @@ -203,7 +203,7 @@ def cb_setpoint(self, *args, **kwargs): if "value" in kwargs and "status" not in kwargs: self._sp_count += 1 self.done.put(not self.done_value) - # logger.debug("cb_setpoint: done=%s, setpoint=%s", self.done.get(), self.setpoint.get()) + logger.debug("cb_setpoint: done=%s, setpoint=%s", self.done.get(), self.setpoint.get()) @property def inposition(self): @@ -220,7 +220,7 @@ def inposition(self): sp = self.setpoint.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) + logger.debug("inposition: inpos=%s rb=%s sp=%s tol=%s", inpos, rb, sp, tol) return inpos @property From d0d46af62ea59e2f493d308c2171d2f21da5d282 Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Thu, 11 Jul 2024 17:24:33 -0500 Subject: [PATCH 21/30] tweaks and cleanup --- apstools/devices/positioner_soft_done.py | 27 +++++++----------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index b5191c350..5c7c3154f 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -53,11 +53,11 @@ class PVPositionerSoftDone(PVPositioner): Defaults to ``10^(-1*precision)``, where ``precision = setpoint.precision``. - update_target : bool + 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` @@ -108,9 +108,6 @@ class PVPositionerSoftDone(PVPositioner): target = Component(Signal, value=TARGET_UNDEFINED, kind="config") - _rb_count = 0 - _sp_count = 0 - def __init__( self, prefix="", @@ -118,7 +115,7 @@ def __init__( readback_pv="", setpoint_pv="", tolerance=None, - update_target=True, + use_target=False, **kwargs, ): # fmt: off @@ -137,11 +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) + 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) @@ -166,7 +164,7 @@ def actual_tolerance(self): # fmt: on def cb_update_target(self, value, *args, **kwargs): - self.target.put(value, wait=True) + self.target.put(value) def cb_readback(self, *args, **kwargs): """ @@ -217,7 +215,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) @@ -230,19 +228,10 @@ 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) - # TODO: The stuff in this if statement might not be necessary anymore. - # if self.update_target: - # kwargs = {} - # if issubclass(self.target.__class__, EpicsSignalBase): - # kwargs["wait"] = True # Signal.put() warns if kwargs are given - # self.target.put(position, **kwargs) self.setpoint.put(position, wait=True) 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) - # This is needed because in a special case the setpoint.put does not - # run the "sub_value" subscriptions. - self.cb_setpoint() self.cb_readback() # This is needed to force the first check. From 633802cd1a4046233ea58e4d51f0bd232e64c2ef Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Thu, 11 Jul 2024 17:34:03 -0500 Subject: [PATCH 22/30] update eurotherm to `use_target` --- apstools/devices/eurotherm_2216e.py | 2 +- apstools/devices/tests/test_eurotherm_2216e.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apstools/devices/eurotherm_2216e.py b/apstools/devices/eurotherm_2216e.py index 8afa6c772..c20c4c1f8 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/tests/test_eurotherm_2216e.py b/apstools/devices/tests/test_eurotherm_2216e.py index 59ceaa17d..0941fad5d 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 = """ From 2b3d5cec35024dcadc0a7b108c7f6cb5785ab1f7 Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Thu, 11 Jul 2024 17:49:30 -0500 Subject: [PATCH 23/30] tweak test to use_target=True --- apstools/devices/tests/test_positioner_soft_done.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index 376ea7a35..51e926ce9 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() From a05c3fce19c2fcf4abec8e9c3aa0aa7cb6d87b1c Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Thu, 11 Jul 2024 18:04:23 -0500 Subject: [PATCH 24/30] tweaks --- apstools/devices/positioner_soft_done.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index 5c7c3154f..652135cc8 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -200,7 +200,8 @@ def cb_setpoint(self, *args, **kwargs): """ if "value" in kwargs and "status" not in kwargs: self._sp_count += 1 - self.done.put(not self.done_value) + # self.done.put(not self.done_value) + self.done.put(self.inposition) logger.debug("cb_setpoint: done=%s, setpoint=%s", self.done.get(), self.setpoint.get()) @property From 4dc5dc36ff1091a718ec96a3d524256831750b40 Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Thu, 11 Jul 2024 18:16:50 -0500 Subject: [PATCH 25/30] debug --- apstools/devices/positioner_soft_done.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index 652135cc8..743833f84 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -178,8 +178,6 @@ def cb_readback(self, *args, **kwargs): if idle: return - self._rb_count += 1 - if self.inposition: self.done.put(self.done_value) if self.report_dmov_changes.get(): @@ -199,9 +197,7 @@ def cb_setpoint(self, *args, **kwargs): and do not react to the "wrong" signature. """ if "value" in kwargs and "status" not in kwargs: - self._sp_count += 1 - # self.done.put(not self.done_value) - self.done.put(self.inposition) + self.done.put(not self.done_value) logger.debug("cb_setpoint: done=%s, setpoint=%s", self.done.get(), self.setpoint.get()) @property From 8bce07ed12077749fa6bd5846bb2dc51f436c507 Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Mon, 22 Jul 2024 11:14:26 -0500 Subject: [PATCH 26/30] Force done to be False in the start of the motion --- apstools/devices/positioner_soft_done.py | 1 + 1 file changed, 1 insertion(+) diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index 743833f84..666a86397 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -226,6 +226,7 @@ def _setup_move(self, position): """Move and do not wait until motion is complete (asynchronous)""" self.log.debug("%s.setpoint = %s", self.name, position) self.setpoint.put(position, wait=True) + 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) From 0e32b4a7aea31feaee379ce854f0549f0bac1329 Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Mon, 22 Jul 2024 11:23:56 -0500 Subject: [PATCH 27/30] python 3.8 breaking test --- .github/workflows/code.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code.yml b/.github/workflows/code.yml index 05cd7c69f..a8e9ba46c 100644 --- a/.github/workflows/code.yml +++ b/.github/workflows/code.yml @@ -98,7 +98,7 @@ jobs: strategy: matrix: python-version: - - "3.8" + # - "3.8" # TODO: Breaking - "3.9" - "3.10" - "3.11" From 4ef2ab5d92d915cbd620dca836d246d89383796e Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Mon, 22 Jul 2024 13:56:29 -0500 Subject: [PATCH 28/30] add use_target=True to lakeshores --- apstools/devices/lakeshore_controllers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apstools/devices/lakeshore_controllers.py b/apstools/devices/lakeshore_controllers.py index 86e587acf..aa387ff73 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 From 791232a0f379d631b72765007bf87c1ba45d3709 Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Mon, 26 Aug 2024 16:43:34 -0500 Subject: [PATCH 29/30] retrigger checks From 78a76f511854cfbe6cee30ba91e6e46e4d660265 Mon Sep 17 00:00:00 2001 From: Gilberto Fabbris Date: Wed, 28 Aug 2024 09:32:50 -0500 Subject: [PATCH 30/30] re-insert the 3.8 unit testing, see https://github.com/BCDA-APS/apstools/pull/1014#issuecomment-2313336302 --- .github/workflows/code.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code.yml b/.github/workflows/code.yml index 238b893d8..d5caea945 100644 --- a/.github/workflows/code.yml +++ b/.github/workflows/code.yml @@ -99,7 +99,7 @@ jobs: strategy: matrix: python-version: - # - "3.8" # TODO: Breaking + - "3.8" - "3.9" - "3.10" - "3.11"