PVPositionerSoftDone should set done False at start of move

CHANGES.rst

@@ -28,14 +28,15 @@ describe future plans.
 1.6.19
 ******
 
-release expected by 2024-04-02
+release expected by 2024-04-12
 
 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.
 * Race condition with SR570 pre-amp.
 
 Maintenance

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,14 +217,19 @@ 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)
+        # 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()
-        self.cb_readback()  # This is needed to force the first check.
+
+        # Force the first check for done.
+        self.cb_readback()
 
 
 class PVPositionerSoftDoneWithStop(PVPositionerSoftDone):

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
@@ -206,16 +200,13 @@ 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()
 
     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
@@ -248,7 +239,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,9 +261,14 @@ 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):
+        """
+        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()
@@ -286,8 +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:
-            assert not status.done
+        if interrupt and not status.done:
             assert not status.success
             assert not arrived, f"{dt=:.3f}"
             pos.stop()
@@ -350,11 +344,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)