Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PVPositionerIsClose gets stuck when setpoint matches desired position #1229

Open
Igort4 opened this issue Jan 14, 2025 · 2 comments
Open

PVPositionerIsClose gets stuck when setpoint matches desired position #1229

Igort4 opened this issue Jan 14, 2025 · 2 comments

Comments

@Igort4
Copy link

Igort4 commented Jan 14, 2025

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
Copy link
Member

ZLLentz commented Jan 14, 2025

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
Copy link
Contributor

def __init_subclass__(cls, **kwargs):
"""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.readback.sub_value(cls._update_readback)

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....)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants