BUG: position does not get initialized to RBV until move happens

[stuwilkins]

For EpicsMotor when the object is created the property position is None while the RBV is contained in the value. After a move the position property correctly returns a position.

[dchabot]

Right. Thanks, Stuart. I’ve noticed that one too. Will submit a fix shortly.

— dc

On Dec 4, 2014, at 3:15 PM, Stuart Wilkins notifications@github.com wrote:

For EpicsMotor when the object is created the property position is None while the RBV is contained in the value. After a move the position property correctly returns a position.

— Reply to this email directly or view it on GitHub.

[dchabot]

Hmmm. Thought I understood what the problem was, but I'm mistaken. Still digging...

On Thu, Dec 4, 2014 at 3:31 PM, Daron Chabot daronchabot@gmail.com wrote:

Right. Thanks, Stuart. I’ve noticed that one too. Will submit a fix shortly.

— dc

On Dec 4, 2014, at 3:15 PM, Stuart Wilkins notifications@github.com wrote:

For EpicsMotor when the object is created the property position is None while the RBV is contained in the value. After a move the position property correctly returns a position.

— Reply to this email directly or view it on GitHub.

[klauer]

I think what happens here is that:

EpicsSignal() creates an epics.PV(), it connects and fires its monitor callback. Then EpicsMotor._readback.subscribe() happens, too late to get the initial position callback.

I believe the default EpicsSignal.subscribe implementation should fire a callback as soon as it’s added.

(sorry for the double-reply)

[dchabot]

On Dec 4, 2014, at 5:32 PM, K Lauer notifications@github.com wrote:

I think what happens here is that:

EpicsSignal() creates an epics.PV(), it connects and fires its monitor callback. Then EpicsMotor._readback.subscribe() happens, too late to get the initial position callback.

Yes, confirmed.

I believe the default EpicsSignal.subscribe implementation should fire a callback as soon as it’s added.

Yep. That’s the CA protocol - a subscription request returns value, timestamp, etc on successful establishment (just as though a monitor had fired).

Simplest, narrow-scope fix (does NOT solve the general problem): --- a/ophyd/controls/positioner.py +++ b/ophyd/controls/positioner.py @@ -206,6 +206,7 @@ class EpicsMotor(Positioner): self._moving = bool(self._is_moving.value) self._done_move.subscribe(self._move_changed) self._user_readback.subscribe(self._pos_changed)

•  self._position = self._user_readback.value

  def stop(self): self._stop._set_request(1, wait=False)

Need to think through the consequences of NOT registering a default subscription for every channel we connect…

[dchabot]

Oops. Hit send too soon.

On Dec 4, 2014, at 6:54 PM, Daron Chabot daronchabot@gmail.com wrote:

On Dec 4, 2014, at 5:32 PM, K Lauer notifications@github.com wrote:

I think what happens here is that:

EpicsSignal() creates an epics.PV(), it connects and fires its monitor callback. Then EpicsMotor._readback.subscribe() happens, too late to get the initial position callback.

Yes, confirmed.

I believe the default EpicsSignal.subscribe implementation should fire a callback as soon as it’s added.

Yep. That’s the CA protocol - a subscription request returns value, timestamp, etc on successful establishment (just as though a monitor had fired).

Simplest, narrow-scope fix (does NOT solve the general problem): --- a/ophyd/controls/positioner.py +++ b/ophyd/controls/positioner.py @@ -206,6 +206,7 @@ class EpicsMotor(Positioner): self._moving = bool(self._is_moving.value) self._done_move.subscribe(self._move_changed) self._user_readback.subscribe(self._pos_changed)

•  self._position = self._user_readback.value

  def stop(self): self._stop._set_request(1, wait=False)

Need to think through the consequences of NOT registering a default subscription for every channel we connect…

Or, look at registering callbacks at the time of EpicsSignal creation? This seems like the right thing to, as it at least makes a better effort to ensure that the object is “ready for use” after construction...

[klauer]

| I believe the default EpicsSignal.subscribe implementation should fire a callback as soon as it’s added. Yep. That’s the CA protocol - a subscription request returns value, timestamp, etc on successful establishment (just as though a monitor had fired).

What I meant by this was a possible solution: when you call EpicsSignal.subscribe() you are guaranteed to get a callback with the current value, just after subscribing.

Or, look at registering callbacks at the time of EpicsSignal creation?

This is a possibility, but it would not solve the issue of adding subscriptions post-instantiation as the above solution would. Additionally, I think adding more kwargs to __init__ (like: callback for readback update, callback for request update) doesn't seem quite as clean.

[dchabot]

On Thu, Dec 4, 2014 at 7:12 PM, K Lauer notifications@github.com wrote:

| I believe the default EpicsSignal.subscribe implementation should fire a callback as soon as it’s added. Yep. That’s the CA protocol - a subscription request returns value, timestamp, etc on successful establishment (just as though a monitor had fired).

What I meant by this was a possible solution: when you call EpicsSignal.subscribe() you are guaranteed to get a callback with the current value, just after subscribing.

Ahhh. Ok, something like this?

dchabot@xf23id1-ioc2--> git diff diff --git a/ophyd/controls/signal.py b/ophyd/controls/signal.py index b57ab12..94c325e 100644 --- a/ophyd/controls/signal.py +++ b/ophyd/controls/signal.py @@ -389,6 +389,15 @@ class EpicsSignal(Signal): else: return ret

• def subscribe(self, callback, event_type=None):
• Signal.subscribe(self, callback, event_type) +

• ensure that callbacks are run to get current values, etc

• self._read_pv.run_callbacks() +
• if self._write_pv is not None:
• self._write_pv.run_callbacks() + @property def readback(self): '''

This seems to work.

Or, look at registering callbacks at the time of EpicsSignal creation?

This is a possibility, but it would not solve the issue of adding subscriptions post-instantiation as the above solution would. Additionally, I think adding more kwargs to init (like: callback for readback update, callback for request update) doesn't seem quite as clean.

Agreed - twas a bad idea on all accounts.

[dchabot]

@klauer - I don't remember: where do you stand on my temporary fix for this (above)?

[klauer]

I'm going to try to fix the underlying bug this afternoon (I got sidetracked with PVPositioner, it was bugging me). If I don't get to it today, I'm fine with using the temporary fix.

[dchabot]

Marked as closed by Ken's callback fix.