PVPositionerIsClose gets stuck when setpoint matches desired position

[Igort4]

Description I was working with an Ophyd device of type PVPositionerIsClose, and I noticed a strange behavior when the setpoint is already at the desired position. In this situation, the device gets stuck because the setpoint or readback callbacks are never triggered (assuming the device setpoint will never oscillate).

This issue is particularly problematic in a common use case: setting a positioner to a specific position and starting a relscan from that position. If I start the relscan with a value of 0, it gets stuck indefinitely.

Proposed Discussion I was wondering if we could bypass/trigger the PVPositioner callback in this case, but maintaining the core logic of the positioner for the self.done.

Related Discussion I came across this related issue: #402. While the problem described there is somewhat different, it was an interesting discussion that inspired me to consider this approach.

Steps to Reproduce Use a PVPositionerIsClose device with a setpoint already at the desired position. Attempt to start a relscan beginning at a position of 0. Observe that the device gets stuck indefinitely without triggering the setpoint or readback callbacks.

Let me know your thoughts or if there’s another recommended approach to address this.

[ZLLentz]

Possibly the simplest would be to extend _setup_move in PVPositionerComparator to call _update_setpoint. I'm somewhat second-guessing my previous decision to subscribe to the setpoint signal at all, it would probably be cleaner and more consistent to only call _update_setpoint during move or set.

[tacaswell]

https://github.com/bluesky/ophyd/blob/1e5f03752f6a6b36ae5738ebbdf4f7645129b20d/ophyd/pv_positioner.py#L396-L401

changing this something like

diff --git a/ophyd/pv_positioner.py b/ophyd/pv_positioner.py
index 25e86770..7c1b1f5d 100644
--- a/ophyd/pv_positioner.py
+++ b/ophyd/pv_positioner.py
@@ -397,7 +397,7 @@ class PVPositionerComparator(PVPositioner):
         """Set up callbacks in subclass."""
         super().__init_subclass__(**kwargs)
         if None not in (cls.setpoint, cls.readback):
-            cls.setpoint.sub_value(cls._update_setpoint)
+            cls.setpoint.subscriptions(EpicsSignal.SUB_SETPOINT)(cls._update_setpoint)
             cls.readback.sub_value(cls._update_readback)

     def done_comparator(self, readback: Any, setpoint: Any) -> bool:

may be the easiest path (assuming the setpoint sub propogates up correctly....)

[Igort4]

Could we override the move at PVPositionerIsComparator @ZLLentz ? Would be something like:

def move(self, position, wait=True, timeout=None, moved_cb=None):
        if self.stop_signal is not None:
            self.stop_signal.wait_for_connection()

        current_readback = self.readback.get(use_monitor=False)
        if self.done_comparator(current_readback, position):
            self.log.debug(
                "%s: already at position %s (readback=%s), skipping move.",
                self.name,
                position,
                current_readback
            )
            self._update_setpoint(value=position)
            self._set_position(current_readback)
            return MoveStatus(self, position, done=True, success=True)

        return super().move(position, wait=wait, timeout=timeout, moved_cb=moved_cb)

[ZLLentz]

Go ahead and try it, if you like. I gave my pointer on an easy way already, Tom gave another easy approach. If you want to take a third approach I don't mind. I unfortunately don't have time allocated right now to implement a fix or test this.