Open burkeds opened 2 months ago
Need to add support for the run
argument to the subscribe
method, as per https://github.com/bluesky/bluesky/issues/1789
This means adding the run
argument to both of these:
https://github.com/bluesky/ophyd-async/blob/406ac0885c119e21439918d08ff10325c415dd19/src/ophyd_async/core/_signal.py#L215-L221
and if it is False then after https://github.com/bluesky/ophyd-async/blob/406ac0885c119e21439918d08ff10325c415dd19/src/ophyd_async/core/_signal.py#L158 add it to a set
of listeners that want to drop the next notification
In https://github.com/bluesky/ophyd-async/blob/406ac0885c119e21439918d08ff10325c415dd19/src/ophyd_async/core/_signal.py#L151 then if function
is in the set
of listeners that want to drop the next notification, then remove from the set and return without notifying, otherwise do the current behaviour
Working on a branch with those changes + added in a test to confirm when the callback gets called. There's a couple other API changes required in bluesky or ophyd-async:
_get_justification
for many of the Suspenders calls self._sig._get()
, when there is no synch (or async) method on ophyd-async Signal
called get
. Probably simplest to cache the last value somewhere in the suspender instead of manually calling get
on the signal, or remove the value from the justification message.install
calls self._sig.subscribe()
, which in ophyd-async returns a dict with {name: reading with metadata}, when just the value is expected. If we're adopting the ophyd interface perhaps could be fixed by renaming subscribe
and subscribe_value
to something like subscribe_reading
and subscribe
respectively. Though obviously that's quite a big breaking change.
Any thoughts @coretl?Working on a branch with those changes + added in a test to confirm when the callback gets called. There's a couple other API changes required in bluesky or ophyd-async:
* `_get_justification` for many of the Suspenders calls `self._sig._get()`, when there is no synch (or async) method on ophyd-async `Signal` called `get`. Probably simplest to cache the last value somewhere in the suspender instead of manually calling `get` on the signal, or remove the value from the justification message.
Caching makes sense.
* `install` calls `self._sig.subscribe()`, which in ophyd-async returns a dict with {name: reading with metadata}, when just the value is expected. If we're adopting the ophyd interface perhaps could be fixed by renaming `subscribe` and `subscribe_value` to something like `subscribe_reading` and `subscribe` respectively. Though obviously that's quite a big breaking change. Any thoughts @coretl?
Hmm, I only looked at https://github.com/bluesky/bluesky/blob/f46502482e3afcda57e4d6b3ae6bcc0eda129d42/src/bluesky/bundlers.py#L421-L422 which didn't do anything with the value, just went and called read()
. I didn't know about suspenders at the time. I guess we have 3 options:
subscribe
subscribe_value
in preference to subscribe
if it exists to get the valuesubscribe_value
from ophyd-async, and add sub_type: Literal["value", "reading"]
to the signature for bluesky.protocols.subscribe
to align it with ophyd, and implement that in ophyd-asyncI think I prefer 1 or 3. @tacaswell thoughts?
@danielballan prefers 1 so we go with that
Struggling to get a test to work here, it seems like copying this test https://github.com/bluesky/bluesky/blob/b6828c53a68252caeefc3712ea50c49e507d8db2/src/bluesky/tests/test_suspenders.py#L37
and replacing the threading.Timer
s with asyncio.create_task
s with the coroutine
async def _set_after_time(val, sleep):
await asyncio.sleep(sleep)
await signal.set(val)
and starting those tasks before calling
scan = [Msg("checkpoint"), Msg("sleep", None, 0.2)]
RE(scan)
that the RE finishes its scan before control returns to the asyncio tasks. I've tried in a few different ways but maybe there's a much more obvious way to test this. The suspender's resume/suspend methods do get called at least but I haven't been able to prove it interrupts as intended.
I have also seen that when trying to set a signal to a value that should suspend while inside a plan using
yield from bps.abs_set(signal, fail_val)
that bluesky.suspender.SuspenderBase.__make_event
fails to create an asyncio.Event
-- h = self.RE._loop.call_soon_threadsafe(really_make_the_event)
seems to never actually call the function so h gets cancelled. This doesn't seem to happen in identical circumstances if I replaced the ophyd_async signal with an ophyd signal.
Are you using the RE event loop to start the tasks? E.g. call_in_bluesky_event_loop(my_task_creation_func())
where async def my_task_creation_func
does the sleep
then signal.set
or creates a task that does it?
I've tried both, call_in_bluesky_event_loop is probably what we want, but that has the issue I described above where the really_make_the_event
never gets called.
There is currently no support for bluesky suspenders. SignalR.subscribe() may need a refactor to comply with the expected structure.