bluesky / ophyd-async Goto Github PK

View Code? Open in Web Editor NEW

6.0 7.0 21.0 16.87 MB

Hardware abstraction for bluesky written using asyncio

Home Page: https://blueskyproject.io/ophyd-async

License: BSD 3-Clause "New" or "Revised" License

Python 99.87% Dockerfile 0.13%

ophyd-async's Introduction

ophyd-async

Asynchronous Bluesky hardware abstraction code, compatible with control systems like EPICS and Tango.

Source	https://github.com/bluesky/ophyd-async
PyPI	`pip install ophyd-async`
Documentation	https://bluesky.github.io/ophyd-async
Releases	https://github.com/bluesky/ophyd-async/releases

Ophyd-async is a Python library for asynchronously interfacing with hardware, intended to be used as an abstraction layer that enables experiment orchestration and data acquisition code to operate above the specifics of particular devices and control systems.

Both ophyd and ophyd-async are typically used with the Bluesky Run Engine for experiment orchestration and data acquisition.

While EPICS is the most common control system layer that ophyd-async can interface with, support for other control systems like Tango will be supported in the future. The focus of ophyd-async is:

Asynchronous signal access, opening the possibility for hardware-triggered scanning (also known as fly-scanning)
Simpler instantiation of devices (groupings of signals) with less reliance upon complex class hierarchies

See https://bluesky.github.io/ophyd-async for more detailed documentation.

ophyd-async's People

Contributors

Stargazers

Watchers

ophyd-async's Issues

Create an instantly moving mock motor

It may be useful for tests to have a mock motor that instantly moves it's readback to it's setpoint. This mock should live near the Motor object. Maybe it should be a function that mocks out any Motor children of a given device?

This will be easier done when #49 is done.

Discussion: Should children automaticaly traverse more of the object

Currently the children function on a device checks if the Device has any direct attributes that are Devices. This results in the following:

class MyDevice(Device):
    def __init__():
        self.channels = [epics_signal_r(float, f"A"), epics_signal_r(float, f"B")]

my_device.connect() # channels will not get connected here

The fix for this is something like:

class MyDevice(Device):
    def __init__():
        self.channels = [epics_signal_r(float, f"A"), epics_signal_r(float, f"B")]
   def children():
       for signal in self.channels:
           yield signal
       return super().children()

@coretl, @callumforrester - I feel like we may have discussed this already but needs writing down

Acceptance Criteria

We decide if children should be more clever
Decision and reasoning is documented
If the above behaviour remains it should be documented as a gotcha

Multiple backends and removal of _sim_backend global var

As part of restructuring, I wanted to make the signal.py file currently in ophyd_async.epics._signal smaller, as this actually contains a lot of classes/concepts within it which can make it tedious to read through. In doing so, I was reminded of the existence of _sim_backend, a global variable that gets appended to in the connect() method of any sim device.

The main utility of this, is then one can call set_sim_value() and use other helper methods that directly interact with a SimSignalBackend rather than a SignalBackend, like ._set_value, which bypasses caget/pv access. My understanding is this is mostly to help with tests.

However, I don't think it's very neat. I'd like to get rid of this, and either change the tests so they interact with the signals/backends as if they were 'real' (i.e. having to await them) or just handle the backend directly from the signal object and include some '#type: ignore' comments in case mypy isn't placated.

In general, I think signals should be able to have more than one backend, and a user should be able to switch between them in the long run; why wouldn't you, if it's available? Or, do we just want to restrict users here and now and say, either you're using CA or PVA and there's no in-between.

Soft signals vs attributes

In the TetrAMM we have attribute setting:
https://github.com/DiamondLightSource/dodal/blob/5ff8127eb378c1087c66cdb7c95eb5d2424c9456/src/dodal/devices/tetramm.py#L113-L120

This is then set in a plan, which means you can't listify it without having side effects. We could turn this into a soft signal and do bps.abs_set on it instead, but is it worth it? Or do we never intend to preprocess or listify plans...

If we do want a soft signal we should make it easier to construct one and write some docs around why you would use one.

@callumforrester @rosesyrett @tpoliaw thoughts?

Improve `repr` of AsyncStatus

Callbacks to a Bluesky plan often log the reason for the exit_status, such as for instance appending it to the ISPyB deposition. If the plan has failed because an exception occurred in any AsyncStatus, the only information that makes it through the Bluesky FailedStatus and to the emitted document is "<AsyncStatus errored>". With many possible AsyncStatuses this is not very enlightening. I propose adding information about the task to the __repr__, as well as information about the exception if it exists.

flaky tests

Sometimes CI tests fail. Flaky tests are bad and we should aim to not have any. Here is an example of such a test failing on one instance, but passing if it's rerun:
https://github.com/bluesky/ophyd-async/actions/runs/6405250037/job/17387404995?pr=22

It seems the offending error is this:

Traceback (most recent call last):
157
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/_pytest/runner.py", line 341, in from_call
158
    result: Optional[TResult] = func()
159
                                ^^^^^^
160
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/_pytest/runner.py", line 262, in <lambda>
161
    lambda: ihook(item=item, **kwds), when=when, reraise=reraise
162
            ^^^^^^^^^^^^^^^^^^^^^^^^
163
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/pluggy/_hooks.py", line 493, in __call__
164
    return self._hookexec(self.name, self._hookimpls, kwargs, firstresult)
165
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
166
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/pluggy/_manager.py", line 115, in _hookexec
167
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
168
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
169
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/pluggy/_callers.py", line 130, in _multicall
170
    teardown[0].send(outcome)
171
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/_pytest/unraisableexception.py", line 88, in pytest_runtest_call
172
    yield from unraisable_exception_runtest_hook()
173
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/_pytest/unraisableexception.py", line 78, in unraisable_exception_runtest_hook
174
    warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))
175
pytest.PytestUnraisableExceptionWarning: Exception ignored in: <coroutine object SignalR.unstage at 0x7ff6e4e03920>
176

177
Traceback (most recent call last):
178
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/warnings.py", line 537, in _warn_unawaited_coroutine
179
    warn(msg, category=RuntimeWarning, stacklevel=2, source=coro)
180
RuntimeWarning: coroutine 'SignalR.unstage' was never awaited
181
Coroutine created at (most recent call last)
182
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/threading.py", line 995, in _bootstrap
183
    self._bootstrap_inner()
184
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/threading.py", line 1038, in _bootstrap_inner
185
    self.run()
186
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/threading.py", line 975, in run
187
    self._target(*self._args, **self._kwargs)
188
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/asyncio/base_events.py", line 607, in run_forever
189
    self._run_once()
190
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/asyncio/base_events.py", line 1914, in _run_once
191
    handle._run()
192
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/asyncio/events.py", line 80, in _run
193
    self._context.run(self._callback, *self._args)
194
  File "/home/runner/work/ophyd-async/ophyd-async/src/ophyd_async/core/_device/standard_readable.py", line 51, in unstage
195
    await sig.unstage().task
196
  File "/home/runner/work/ophyd-async/ophyd-async/src/ophyd_async/core/async_status.py", line 80, in wrap_f
197
    return AsyncStatus(f(self))

with the following logs recorded:

11:19:29,664 ERROR (MainThread) Task was destroyed but it is pending!
source_traceback: Object created at (most recent call last):
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/threading.py", line 995, in _bootstrap
    self._bootstrap_inner()
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/threading.py", line 1038, in _bootstrap_inner
    self.run()
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/threading.py", line 975, in run
    self._target(*self._args, **self._kwargs)
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/asyncio/base_events.py", line 607, in run_forever
    self._run_once()
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/asyncio/base_events.py", line 1914, in _run_once
    handle._run()
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/bluesky/run_engine.py", line 1588, in _run
    new_response = await coro(msg)
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/bluesky/run_engine.py", line 2420, in _unstage
    ret = obj.unstage()
  File "/home/runner/work/ophyd-async/ophyd-async/src/ophyd_async/core/async_status.py", line 80, in wrap_f
    return AsyncStatus(f(self))
  File "/home/runner/work/ophyd-async/ophyd-async/src/ophyd_async/core/async_status.py", line 23, in __init__
    self.task = asyncio.create_task(awaitable)  # type: ignore
  File "/opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/asyncio/tasks.py", line 374, in create_task
    task = loop.create_task(coro)
task: <Task pending name='Task-604' coro=<HDFStreamerDet.unstage() running at /home/runner/work/ophyd-async/ophyd-async/src/ophyd_async/epics/areadetector/hdf_streamer_det.py:167> wait_for=<Task finished name='Task-608' coro=<StandardReadable.unstage() done, defined at /home/runner/work/ophyd-async/ophyd-async/src/ophyd_async/core/_device/standard_readable.py:48> result=None created at /opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/asyncio/tasks.py:374> cb=[AsyncStatus._run_callbacks()] created at /opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/asyncio/tasks.py:374>

Move writer collect_asset_docs logic into detector

Currently, the Flyer has a collect_asset_docs method which calls (in parallel) the collect_stream_docs(idx) methods of each writer underneath it. This is essentially a way to concurrently collect the asset docs of all writers underneath the flyer.

A new issue has been added, bluesky/bluesky#1649, which will allow this logic to be moved into the Detector. So the plan is to:

Remove collect_asset_docs entirely from the Flyer; the Flyer should be transparent to any document generation. Remove it from the DetectorGroupLogic protocol and implementation(s) also for the same reason.
Make the StandardDetector obey the new protocol in the above issue; that is, it should have a collect_asset_docs(idx) method as well as something equivalent to get_index, which should mean it calls it's underlying writer to figure out what index of data it has written.

In order to test this, we should probably do #91 alongside this.

Issue trying to run linkam_plan for i22: prepare

Just tried to run the linkam_plan on i22, and blueapi had the following error trace:

ERROR:blueapi.worker.reworker:<AsyncStatus errored>
Traceback (most recent call last):
  File "/dls_sw/i22/software/blueapi/scratch/ophyd-async/src/ophyd_async/core/flyer.py", line 224, in _prepare
    await asyncio.gather(
  File "/dls_sw/i22/software/blueapi/scratch/ophyd-async/src/ophyd_async/core/flyer.py", line 112, in ensure_armed
    self._arm_statuses = await gather_list(
                         ^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/ophyd-async/src/ophyd_async/core/utils.py", line 104, in gather_list
    return await asyncio.gather(*coros)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/dodal/src/dodal/devices/tetramm.py", line 131, in arm
    raise Exception("Tetramm has no concept of setting a number of exposures.")
Exception: Tetramm has no concept of setting a number of exposures.
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
  File "/venv/lib/python3.11/site-packages/blueapi/worker/reworker.py", line 235, in _cycle
    self._current.task.do_task(self._ctx)
  File "/venv/lib/python3.11/site-packages/blueapi/worker/task.py", line 33, in do_task
    ctx.run_engine(wrapped_plan_generator)
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 904, in __call__
    plan_return = self._resume_task(init_func=_build_task)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 1043, in _resume_task
    raise exc
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 1673, in _run
    raise err
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 1533, in _run
    msg = self._plan_stack[-1].send(resp)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/blueapi/core/context.py", line 76, in wrap
    yield from wrapped_plan
  File "/venv/lib/python3.11/site-packages/blueapi/preprocessors/attach_metadata.py", line 36, in attach_metadata
    yield from bpp.inject_md_wrapper(
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 752, in inject_md_wrapper
    return (yield from msg_mutator(plan, _inject_md))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 263, in msg_mutator
    msg = plan.send(_s)
          ^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/i22-bluesky/src/i22_bluesky/plans/linkam.py", line 159, in linkam_plan
    rs_uid = yield from inner_linkam_plan()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 675, in dec_inner
    ret = yield from plan
          ^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 675, in dec_inner
    ret = yield from plan
          ^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/utils/__init__.py", line 1203, in dec_inner
    return (yield from plan)
            ^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 985, in stage_wrapper
    return (yield from finalize_wrapper(inner(), unstage_devices()))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 535, in finalize_wrapper
    ret = yield from plan
          ^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 983, in inner
    return (yield from plan)
            ^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/utils/__init__.py", line 1203, in dec_inner
    return (yield from plan)
            ^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 351, in run_wrapper
    yield from contingency_wrapper(plan,
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 606, in contingency_wrapper
    ret = yield from plan
          ^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/i22-bluesky/src/i22_bluesky/plans/linkam.py", line 133, in inner_linkam_plan
    yield from scan_linkam(
  File "/dls_sw/i22/software/blueapi/scratch/i22-bluesky/src/i22_bluesky/stubs/linkam.py", line 54, in scan_linkam
    yield from fly_and_collect(flyer)
  File "/venv/lib/python3.11/site-packages/dls_bluesky_core/stubs/flyables.py", line 51, in fly_and_collect
    yield from bps.wait(group=complete_group, timeout=flush_period)
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/plan_stubs.py", line 504, in wait
    return (yield Msg('wait', None, group=group, timeout=timeout))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 251, in msg_mutator
    _s = yield msg
         ^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 2277, in _status_object_completed
    raise FailedStatus(ret) from exc
bluesky.utils.FailedStatus: <AsyncStatus errored>

Seems to be an issue with the prepare protocol in how it calls controller.arm.

Failing docs

Docs job needs to be fixed

No longer explicitly support n=0 frames to go forever. Plan should pass this to the detectors.

Currently, the HardwareTriggeredFlyable has many detectors, and calls .arm for all of them. By default, the signature of the DetectorControl.arm method has a keyword argument, num=0 which is set to 0 by default to indicate that this controller should take infinite pictures.

This should be changed, so that actually the number of images is fixed. The easiest way of doing this is,

Remove the default value; this has to be passed from the Flyer.
Make the Flyer get this value from it's constructor
Test it to check it still works, e.g. on p38

This can be done in isolation to the other changes to Flyers, and is a good first issue.

Panda should have "name" kwarg in constructor

In a recent change I did this:

3aaedaf

I'm no longer sure I should have done that; MX are using pandas and connecting them/ creating them with names.

The rationale behind the original idea, was to decouple Device hierarchy from naming. I think I originally thought about this in terms of the DeviceCollector. So this is an issue to:

Discuss if we should have hierarchies just for naming things (i.e. calls to super().__init__(name=name) for the sole purpose of setting a name)
Revert the above change.

Allow for configuration signals without a backend

ophyd devices provide configuration through Signal objects, which have a default implementation using a dict within the object.
ophyd_async devices are bounded by SignalR, which does not have an implementation that does not require a backend.

A simple SignalR implementation would make adding configuration to devices simpler, allowing this data to propagate through documents.

Unpin Bluesky dependency from master after next bluesky release

issue/pr migration from ophyd/v2

Issues/PRs relating to ophyd v2 should be migrated to this repository, perhaps by using a tool like Github Issue Mover.

Add kickoff, complete and describe_collect on StandardDetector

This requires moving the existing logic of kickoff/complete and describe_collect from the flyer to the StandardDetector.

This is part of a general work to disassemble the flyer object to purely hold logic triggers for underlying detectors, such that the stream resource/datum documents produced from a fly scan do not eject data about the flyer itself. See the project page readme for more details.

Creating too many statuses

In reference to DiamondLightSource/blueapi#331

It appears that a very large number of status objects are sometimes creating during long-running flyscans. Can reproduce by running the linkam plan: https://github.com/DiamondLightSource/i22-bluesky/blob/move_ophyd_async/src/i22_bluesky/plans/linkam.py and subscribing to the RE waiting hook.

At this point this is a very mysterious bug and this issue is for further investigation.

Implement "prepare" protocol for ophyd-async Devices

Dependant upon bluesky/bluesky#1637 being implemented and merged.

DeviceCollector throws some exceptions away in tests that use it

Recently, when writing a bit of code for blueapi (DiamondLightSource/blueapi#321) we found that if we tried to test the Device Collector (as per bluesky/ophyd#1151)

The tests in Callum's PR seem to fail for the original Device Collector if assert statements on the logs are present. This is odd behaviour and should be fixed.

Deal with stalled timeout

We need a check in place to ensure that hdf writers can write data from triggers faster than triggers can be produced. If the hdf writer takes too long before the next trigger, it should timeout.

GeneratedChoices enum can't handle signals with choices ["", ""]

when startingup an IOC, some enums have ZNAM and ONAM set to "", which means ophyd falls over when trying to set up the enums dynamically (e.g. as for a panda), i.e. when the datatype for epics_signal_xyz is set to None, because of this line in _p4p.py:

enum_class = Enum("GeneratedChoices", {x: x for x in pv_choices}, type=str)  # type: ignore

to fix, for now we could do the following check beforehand:

if "" in pv_choices:
    pv_choices = tuple(["_" if i=="" else i for i in pv_choices])

i.e. meaning there are no choices for this PV.

This is only a temporary fix; really we should think about whether these PV's by default should have ZNAM and ONAM set to something, which is a pandablocks-ioc issue.

Don't harcode HDFFile dataset location

At the moment there is just a hardcoded string in the HDFWriter indicating any data written by scans will be stored in 'entry/data/data'.
This is fine for our current use case (i22), except it'd be better to not hardcode this and consider the wider implications of that, i.e. do we make this a soft signal and have that set in the plan?

Flyer: no existing protection against grouping of stream resources/datums across points

Problem

At the moment, in the flyer collect_asset_docs method, the flyer collects together stream datums, and emits them after all stream resources have been emitted:

        current_frame = self._current_frame
        stream_datums: List[Asset] = []
        async for asset in self._detector_group_logic.collect_asset_docs():
            name, doc = asset
            if name == "stream_datum":
                current_frame = doc["indices"]["stop"] + self._offset
                # Defer stream_datums until all stream_resources have been produced
                # In a single collect, all the stream_resources are produced first
                # followed by their stream_datums
                stream_datums.append(asset)
            else:
                yield asset
        for asset in stream_datums:
            yield asset

The detector group logic has a method, collect_asset_docs, which just spits out all the stream datums/resources for each writer involved in the fly scan (i.e .each hdf writer):

        for writer in self._writers:
            async for doc in writer.collect_stream_docs(indices_written):
                yield doc

In a flyscan, stream resources and datums are grouped for each collection point in the flyscan. i.e. with each set of triggers sent to the detectors. It is technically possible that, if we have lots of writers involved in the fly scan, we could still be spitting out stream datums/resources for one collection point as the trigger for the next one has been set. This could lead to unexpected results, e.g. if we expect n stream datums/stream resources per collection point we could see the following set of documents being produced:

start
descriptor
stream resource (2n)
stream datum (2n)

Instead of, what we would really want:

start
descriptor
stream resource (n)
stream datum (n)
stream resource (n)
stream datum (n)

Possible fix

We could fix this by making the writers emit stream datums/resources in parallel, instead of in series as is currently happening. Ultimately though, we could be asking our flyer (i.e. the panda sending triggers, in i22's case) to be sending out triggers faster than the python logic to produce stream resources/datums can run. At which point we run into a bigger architectural problem. However, as it stands, if we have a huge list of detectors and set up the panda to send lots of triggers in quick succession, we can reach this problem. I don't know if it is worth including a warning just by looking at how quickly those triggers have been set up.

Create a test to reproduce connection failures

In Hyperion we are seeing random ophyd-async reconnect issues DiamondLightSource/hyperion#1159. It would be good to get to the bottom of these.

Acceptance Criteria

We write a dummy application that periodically reconnects to ophyd-async devices on i03 and try and reproduce the error. (We do not want to do this in a tight loop and steal all the resources, also probably best ran supervised on a beam off day)
We try and find a pattern that can help us with diagnosing the issue

Produce TriggerInfo on the plan level instead of in the Flyer.

Currently the flyer gives you the TriggerInfo from the trigger logic it receives. This Logic -> info process should happen up in the plan level (as a util function) so it can be passed to the devices that need it (detectors).

Add support for watchers into AsyncStatus.wrap

Pass is as an extra argument into the wrapped function.

Helper to check against motor limits

For Hyperion, and in general across MX DAQ, we want to be able to easily check if a position is within motion limits. Specifically, we use optical edge detection to find a pin tip and then want to check if that's within the smargon limits or else stop the collection.

In ophyd we do something like:

class Smargon(Device):
    x: EpicsMotor = Component(EpicsMotor, "X")

    def get_x_in_limits(self, position: float):
        low = self.x.low_limit_travel.get()
        high = self.x.high_limit_travel.get()
        return low <= position <= high

def plan():
    if smargon.get_x_in_limits(100):
        yield from ...
    else:
        yield from ...

There are issues in doing this in ophyd-async:

The limit PVs are not in the Motor object (#42)
The above is a bad solution anyway as we're doing gets without going through the RE

Signal Kind

In ophyd there is the concept of attribute kind https://nsls-ii.github.io/ophyd/signals.html?highlight=kind#kind

This is quite useful when looking at data, separating signals into stuff that's really important and other stuff that is just useful for context. Differentiating normal and hinted signals is also useful for plotting

Is this also planned for ophyd-async, or is there another method for differentiating signals?

Devices and exposing read/describe methods

This came out of a discussion with @coretl and @callumforrester:

Do we want devices to look like:

class Sensor(EpicsDevice): #i.e. import from some base class that doesn't currently exist yet but looks something like Device
    """A demo sensor that produces a scalar value based on X and Y Movers"""

    value: A[SignalR[float], READ, pv_suffix("Value")]
    mode: A[SignalRW[EnergyMode], CONFIG, pv_suffix("Mode")]

    # Here is how you define the size of a vector
    value: A[SignalR[float], READ] = epics_signal_r("Value")
    mode: A[SignalRW[EnergyMode], CONFIG] = epics_signal_r("Mode")
    some_vector: A[DeviceVector[SignalRW[float]], CONFIG] = DeviceVector(
        {i: epics_signal_r(...) for i in range(10)}
    )

    describe, read = from_signals_tagged(READ)
    describe_configuration, read_configuration = from_signals_tagged(CONFIG)

    # We think this should probably go in the base class pending discussion with actual users
    # might chat with Dom
    async def read(self) -> Reading:
        reading = {}
        for key, value in self.__annotations__:
            if value is A and READ in value:
                reading[key] = getattr(self, key).read()

    def __init__(self, prefix: str, name="") -> None:
        # Define some signals
        self.value = epics_signal_r(float, prefix + "Value")
        self.mode = epics_signal_rw(EnergyMode, prefix + "Mode")
        # Set name and signals for read() and read_configuration()
        self.set_readable_signals(
            read=[self.value],
            config=[self.mode],
        )
        super().__init__(name=name)

i.e. do we make ophyd-async have a similar approach to ophyd in terms of how things like read/describe work? Or do we not define read/describe in the parent class (in this case, EpicsDevice) and expect users to overwrite them, either explicitly or with helper methods, because actually most use cases will be unique?

We need to actually ask users and the ophyd people what their thoughts are. Personally, I think it's best for things like read to exist in the parent class, and pull out signals that are annotated with 'READ', and just expose those. Then any new user can just tag a signal with 'READ' and it'll be read. I don't think the default behaviour for the read method should be too controversial because at the end of the day it should just spit out a dictionary with signals that it's read. The controversial bit is, which signals need to be read? In ophyd currently, all signals are read. And that can probably get a bit spammy.

Open to more discussions on this.

Change `SignalX.execute` to `trigger`

The existing code is:

ophyd-async/src/ophyd_async/core/signal.py

Lines 226 to 231 in f1b4aba

 class SignalX(Signal): 

 """Signal that puts the default value""" 

 async def execute(self, wait=True, timeout=None): 

 """Execute the action and return a status saying when it's done""" 

 await self._backend.put(None, wait=wait, timeout=timeout or self._timeout)

This does not have a natural verb to use in plans, but trigger does, and the signature matches. I propose we change it to:

class SignalX(Signal):
    """Signal that puts the default value"""

    async def trigger(self, wait=True, timeout=USE_DEFAULT_TIMEOUT) -> AsyncStatus:
        """Trigger the action and return a status saying when it's done"""
        if timeout is USE_DEFAULT_TIMEOUT:
            timeout = self._timeout
        coro = self._backend.put(None, wait=wait, timeout=timeout)
        return AsyncStatus(coro)

We can then use bps.trigger for this (along with its exception handling) rather than bps.wait_for

Add save/load

As a follow up to bluesky/ophyd-epics-devices#28, a method should be added to save all PV's of a device to a yaml file, and a load method should be added to set the device to a state specified in the yaml file.

Add context for callback to be removed from sim signal

When writing tests I want to simulate some basic signal behaviour e.g. writing to one signal will put to another. However, I would like to make sure this behaviour is properly cleaned up between tests. I can simulate with set_sim_callback and set_sim_value but there is no way to remove this callback.

Acceptance Criteria

There is a context that removes callbacks from simulated signals on cleanup

Discussion: Mapping Flyscanning Hardware

Background

The block-and-part structure of devices in malcolm is designed to be a map of the physical hardware, including which triggering systems are plugged into which detectors et al. This has numerous advantages and costs.

A map of the hardware can be inspected when designing a scan to see if it is even possible (for example it may not be because motor x is not plugged into PandA y). It can inform GUIs that are used to construct scans. It can also be used to validate the parameters of a scan/state of the hardware where automatic validation is possible (e.g. checking the wiring of a PandA or AreaDetector). Even where it not possible to automatically check (e.g. physical wiring), a useful error message can be displayed to the user indicating how the software thinks the hardware is wired up, which may lead them to diagnosing problems.

On the other hand, maintaining a map of the hardware is a cost, especially since there is no automatic way to check it is up-to-date if the physical wiring changes. This fundamentally makes it a useful addon rather than a necessary part of the system, since a plan can be written with implicit knowledge of the hardware (i.e. the hardware map inside the developer's brain). Requiring it for all ophyd-async scans therefore reduces the flexibility of the system.

Additionally, the rigid coupling of Python object structure to mapping of hardware has not always been desirable. It may well be that the grouping/layering of devices that makes sense to a scientist is different to the actual physical mapping of hardware.

Thoughts on Solutions

The simplest solution is to leave the mapping implicit by default and allow users to develop their own plans/devices that take explicit mappings if they want to. The main issue with which is of course https://xkcd.com/927/. This could be somewhat mitigated by still providing a standard in ophyd-async but not committing all devices to using it. Leaving open the possibility of users adopting their own standards or even just using implicit knowledge.

Latest release (0.2.0) fails on startup

Upon starting Hyperion using the latest ophyd-async release, we encountered the following error:

*** Error in `python': corrupted size vs. prev_size: 0x000055c258f67dd0 ***
======= Backtrace: =========
/lib64/libc.so.6(+0x7f474)[0x7facdcb3e474]
/lib64/libc.so.6(+0x8156b)[0x7facdcb4056b]
/dls_sw/i03/software/bluesky/hyperion_v8.2.0/hyperion/.venv/lib/python3.10/site-packages/epicscorelibs/lib/./libCom.so.7.0.7.99.0(_ZN9fdManagerD1Ev+0xd3)[0x7facc8ceb6b3]
/lib64/libc.so.6(+0x39ce9)[0x7facdcaf8ce9]
/lib64/libc.so.6(+0x39d37)[0x7facdcaf8d37]
/lib64/libc.so.6(__libc_start_main+0xfc)[0x7facdcae155c]
python(+0x1cb421)[0x55c256a7d421]

followed by a long memory map.

Additional info:
We are only importing opyhd-async and not actually using it in the code - but this still breaks upon using regular ophyd deviecs

Rename Sim -> Mock and add SoftSignalBackend

Currently, you can set up ophyd-async devices in "sim" mode, which actually just mock out the control layer underneath (i.e. EPICs) to let someone test the stuff above it, i.e. plans and filewriting, works as expected. As such, we need to rename the "sim" interface to "mock" to prevent confusion.

p4p cython issue

Running pytest with python 3.8 fails, both locally and in CI. Pytest fails to collect all tests, all of them having identical error messages. Here is an example of such a message:

__________________ ERROR collecting tests/panda/test_panda.py __________________
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/runner.py", line 341, in from_call
    result: Optional[TResult] = func()
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/runner.py", line 372, in <lambda>
    call = CallInfo.from_call(lambda: list(collector.collect()), "collect")
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/python.py", line 531, in collect
    self._inject_setup_module_fixture()
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/python.py", line 545, in _inject_setup_module_fixture
    self.obj, ("setUpModule", "setup_module")
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/python.py", line 310, in obj
    self._obj = obj = self._getobj()
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/python.py", line 528, in _getobj
    return self._importtestmodule()
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/python.py", line 617, in _importtestmodule
    mod = import_path(self.path, mode=importmode, root=self.config.rootpath)
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/pathlib.py", line 567, in import_path
    importlib.import_module(module_name)
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/assertion/rewrite.py", line 178, in exec_module
    exec(co, module.__dict__)
  File "/home/runner/work/ophyd-async/ophyd-async/tests/panda/test_panda.py", line 9, in <module>
    from ophyd_async.panda import PandA, PVIEntry, SeqTable, SeqTrigger, pvi
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/ophyd_async/panda/__init__.py", line 1, in <module>
    from .panda import (
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/ophyd_async/panda/panda.py", line 23, in <module>
    from p4p.client.thread import Context
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/p4p/__init__.py", line 14, in <module>
    from .wrapper import Value, Type
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/p4p/wrapper.py", line 5, in <module>
    from . import _p4p
  File "src/p4p/_p4p.pyx", line 605, in init p4p._p4p
AttributeError: type object 'p4p._p4p.ClientProvider' has no attribute '__reduce_cython__'

Don't import epicscorelibs in ophyd_async.core

The core module should be agnostic of the underlying control system used (epics, tango, etc). A recent PR has introduced epics into the core package, which should not be happening. We need to discuss where else the logic from this PR should live: #66

Put some code here

Start with the Bluesky version of skeleton
Move across the ophyd.v2 and ophyd-epics-devices code, tests and docs, and restructure as you see fit. I don't think we care about git history.
- I think we want a ophyd_async.core namespace, but I don't know if we want ophyd_async.panda or ophyd_async.devices.panda
If you make any decisions about structure then write an ADR in the ophyd-async repo with the details
Remove refs to v1 in ophyd, and replace refs to v2 with pointers to this repo
Archive ophyd-epics-devices with a suitable message

Change motor internals to better match ophyd

To ease in migration it would be good if the underlying signals in Motor had the same name as EpicsMotor. For example, user_readback in EpicsMotor is readback in Motor. We should go through and try and keep consistency unless there is a good reason to change the name.

Additionally, there are a number of parameters that are not exposed:

Limits (see #41)
Acceleration

Issue on machine day: 30/01/24

blueapi logs for i22

INFO:     127.0.0.1:34098 - "POST /tasks HTTP/1.1" 201 Created
INFO:blueapi.worker.reworker:Submitting: task_id='33018899-f7f9-420c-8f16-92db2e08d22e' task=RunPlan(name='linkam_plan', params={'heat_temp': 30, 'cool_temp': 0, 'start_temp': 30, 'cool_step': 5, 'heat_rate': 10, 'exposure': 0.1, 'heat_step': 5, 'num_frames': 10, 'cool_rate': 10}) is_complete=False is_pending=True errors=[]
INFO:blueapi.worker.reworker:Got new task: task_id='33018899-f7f9-420c-8f16-92db2e08d22e' task=RunPlan(name='linkam_plan', params={'heat_temp': 30, 'cool_temp': 0, 'start_temp': 30, 'cool_step': 5, 'heat_rate': 10, 'exposure': 0.1, 'heat_step': 5, 'num_frames': 10, 'cool_rate': 10}) is_complete=False is_pending=True errors=[]
INFO:blueapi.worker.task:Asked to run plan linkam_plan with {'heat_temp': 30, 'cool_temp': 0, 'start_temp': 30, 'cool_step': 5, 'heat_rate': 10, 'exposure': 0.1, 'heat_step': 5, 'num_frames': 10, 'cool_rate': 10}
INFO:bluesky:Executing plan <generator object BlueskyContext.wrap at 0x7fe17765f5b0>
INFO:bluesky.RE.state:Change state on <bluesky.run_engine.RunEngine object at 0x7fe1a1c99c10> from 'idle' -> 'running'
INFO:blueapi.messaging.stomptemplate:SENDING {"state": "RUNNING", "task_status": {"task_id": "33018899-f7f9-420c-8f16-92db2e08d22e", "task_complete": false, "task_failed": false}, "errors": [], "warnings": []} to /topic/public.worker.event
INFO:     127.0.0.1:34098 - "PUT /worker/task HTTP/1.1" 200 OK
ERROR:bluesky:Run aborted
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/asyncio/tasks.py", line 500, in wait_for
    return fut.result()
           ^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/p4p/client/asyncio.py", line 207, in put
    return (await self._put_one(name, values, request=request, get=get))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/p4p/client/asyncio.py", line 236, in _put_one
    value = await F
            ^^^^^^^
asyncio.exceptions.CancelledError

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/dls_sw/i22/software/blueapi/scratch/ophyd-async/src/ophyd_async/epics/_backend/_p4p.py", line 260, in put
    await asyncio.wait_for(coro, timeout)
  File "/usr/local/lib/python3.11/asyncio/tasks.py", line 502, in wait_for
    raise exceptions.TimeoutError() from exc
TimeoutError

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 1508, in _run
    msg = self._plan_stack[-1].throw(stashed_exception or resp)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/blueapi/core/context.py", line 76, in wrap
    yield from wrapped_plan
  File "/venv/lib/python3.11/site-packages/blueapi/preprocessors/attach_metadata.py", line 36, in attach_metadata
    yield from bpp.inject_md_wrapper(
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 752, in inject_md_wrapper
    return (yield from msg_mutator(plan, _inject_md))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 257, in msg_mutator
    msg = plan.throw(_e)
          ^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/i22-bluesky/src/i22_bluesky/plans/linkam.py", line 116, in linkam_plan
    yield from load_device(panda)
  File "/dls_sw/i22/software/blueapi/scratch/i22-bluesky/src/i22_bluesky/stubs/load.py", line 29, in load_device
    yield from set_signal_values(signals, phases)
  File "/dls_sw/i22/software/blueapi/scratch/ophyd-async/src/ophyd_async/core/device_save_loader.py", line 220, in set_signal_values
    yield from wait(f"load-phase{phase_number}")
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/plan_stubs.py", line 504, in wait
    return (yield Msg('wait', None, group=group, timeout=timeout))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 251, in msg_mutator
    _s = yield msg
         ^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 2277, in _status_object_completed
    raise FailedStatus(ret) from exc
bluesky.utils.FailedStatus: <AsyncStatus, task: <coroutine object PvaSignalBackend.put at 0x7fe1764559c0>, errored: TimeoutError()>
INFO:bluesky.RE.state:Change state on <bluesky.run_engine.RunEngine object at 0x7fe1a1c99c10> from 'running' -> 'idle'
INFO:blueapi.messaging.stomptemplate:SENDING {"state": "IDLE", "task_status": {"task_id": "33018899-f7f9-420c-8f16-92db2e08d22e", "task_complete": false, "task_failed": false}, "errors": [], "warnings": []} to /topic/public.worker.event
ERROR:blueapi.worker.reworker:<AsyncStatus, task: <coroutine object PvaSignalBackend.put at 0x7fe1764559c0>, errored: TimeoutError()>
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/asyncio/tasks.py", line 500, in wait_for
    return fut.result()
           ^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/p4p/client/asyncio.py", line 207, in put
    return (await self._put_one(name, values, request=request, get=get))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/p4p/client/asyncio.py", line 236, in _put_one
    value = await F
            ^^^^^^^
asyncio.exceptions.CancelledError

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/dls_sw/i22/software/blueapi/scratch/ophyd-async/src/ophyd_async/epics/_backend/_p4p.py", line 260, in put
    await asyncio.wait_for(coro, timeout)
  File "/usr/local/lib/python3.11/asyncio/tasks.py", line 502, in wait_for
    raise exceptions.TimeoutError() from exc
TimeoutError

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/venv/lib/python3.11/site-packages/blueapi/worker/reworker.py", line 235, in _cycle
    self._current.task.do_task(self._ctx)
  File "/venv/lib/python3.11/site-packages/blueapi/worker/task.py", line 33, in do_task
    ctx.run_engine(wrapped_plan_generator)
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 904, in __call__
    plan_return = self._resume_task(init_func=_build_task)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 1043, in _resume_task
    raise exc
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 1673, in _run
    raise err
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/run_engine.py", line 1508, in _run
    msg = self._plan_stack[-1].throw(stashed_exception or resp)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/blueapi/core/context.py", line 76, in wrap
    yield from wrapped_plan
  File "/venv/lib/python3.11/site-packages/blueapi/preprocessors/attach_metadata.py", line 36, in attach_metadata
    yield from bpp.inject_md_wrapper(
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 752, in inject_md_wrapper
    return (yield from msg_mutator(plan, _inject_md))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 257, in msg_mutator
    msg = plan.throw(_e)
          ^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/i22-bluesky/src/i22_bluesky/plans/linkam.py", line 116, in linkam_plan
    yield from load_device(panda)
  File "/dls_sw/i22/software/blueapi/scratch/i22-bluesky/src/i22_bluesky/stubs/load.py", line 29, in load_device
    yield from set_signal_values(signals, phases)
  File "/dls_sw/i22/software/blueapi/scratch/ophyd-async/src/ophyd_async/core/device_save_loader.py", line 220, in set_signal_values
    yield from wait(f"load-phase{phase_number}")
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/plan_stubs.py", line 504, in wait
    return (yield Msg('wait', None, group=group, timeout=timeout))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/dls_sw/i22/software/blueapi/scratch/bluesky/bluesky/preprocessors.py", line 251, in msg_mutator
    _s = yield msg

should we be able to connect devices without the run engine?

Part of the appeal of ophyd v1 is that you can use it stand-alone, without the run engine or bluesky. This is not true of ophyd-async, only because we need the bluesky event loop to be running to connect devices. Could we not change the connection logic so that;

You can connect your devices in a separate (i.e. local) event loop,
When you make the run engine, in the background something works out that your running event loop should be used for the run engine instead.

Ephemeral flyers made within plans should not change the description of contained devices

Example descriptor document using a Flyer:

{
  "name": "descriptor",
  "doc": {
    "configuration": {
      "flyer": {
        "data": {},
        "timestamps": {},
        "data_keys": {}
      }
    },
    "data_keys": {
      "saxs": {
        "source": "ca://BL22I-EA-PILAT-01:HDF5:FullFileName_RBV",
        "shape": [
          1679,
          1475
        ],
        "dtype": "array",
        "external": "STREAM:",
        "object_name": "flyer"
      },
      "saxs-sum": {
        "source": "ca://BL22I-EA-PILAT-01:HDF5:FullFileName_RBV",
        "shape": [],
        "dtype": "number",
        "external": "STREAM:",
        "object_name": "flyer"
      },
      "waxs": {
        "source": "ca://BL22I-EA-PILAT-03:HDF5:FullFileName_RBV",
        "shape": [
          1679,
          1475
        ],
        "dtype": "array",
        "external": "STREAM:",
        "object_name": "flyer"
      },
      "waxs-sum": {
        "source": "ca://BL22I-EA-PILAT-03:HDF5:FullFileName_RBV",
        "shape": [],
        "dtype": "number",
        "external": "STREAM:",
        "object_name": "flyer"
      },
      "i0": {
        "source": "ca://BL22I-EA-XBPM-02:HDF5:FullFileName_RBV",
        "shape": [
          11,
          1000
        ],
        "dtype": "array",
        "external": "STREAM:",
        "object_name": "flyer"
      },
      "i0-temperature": {
        "source": "ca://BL22I-EA-XBPM-02:HDF5:FullFileName_RBV",
        "shape": [],
        "dtype": "number",
        "external": "STREAM:",
        "object_name": "flyer"
      },
      "it": {
        "source": "ca://BL22I-EA-TTRM-02:HDF5:FullFileName_RBV",
        "shape": [
          11,
          1000
        ],
        "dtype": "array",
        "external": "STREAM:",
        "object_name": "flyer"
      }
    },
    "name": "primary",
    "object_keys": {
      "flyer": [
        "saxs",
        "saxs-sum",
        "waxs",
        "waxs-sum",
        "i0",
        "i0-temperature",
        "it"
      ]
    },
    "run_start": "685941e0-9563-4a28-a303-541d099d15e3",
    "time": 1700565839.0465171,
    "uid": "28e70ece-ad7f-492d-9c37-0b14a19f6567",
    "hints": {
      "flyer": {
        "fields": [
          "saxs",
          "waxs",
          "i0",
          "it"
        ]
      }
    }
  }
}

Expected DescriptorDocument (and descriptor document when not using Flyer):

{
  "name": "descriptor",
  "doc": {
    "configuration": {
      "saxs": {
        "data": {},
        "timestamps": {},
        "data_keys": {}
      },
      "waxs": {
        "data": {},
        "timestamps": {},
        "data_keys": {}
      },
      "i0": {
        "data": {},
        "timestamps": {},
        "data_keys": {}
      },
      "it": {
        "data": {},
        "timestamps": {},
        "data_keys": {}
      }
    },
    "data_keys": {
      "saxs": {
        "source": "ca://BL22I-EA-PILAT-01:HDF5:FullFileName_RBV",
        "shape": [
          1679,
          1475
        ],
        "dtype": "array",
        "external": "STREAM:",
        "object_name": "saxs"
      },
      "saxs-sum": {
        "source": "ca://BL22I-EA-PILAT-01:HDF5:FullFileName_RBV",
        "shape": [],
        "dtype": "number",
        "external": "STREAM:",
        "object_name": "saxs"
      },
      "waxs": {
        "source": "ca://BL22I-EA-PILAT-03:HDF5:FullFileName_RBV",
        "shape": [
          1679,
          1475
        ],
        "dtype": "array",
        "external": "STREAM:",
        "object_name": "waxs"
      },
      "waxs-sum": {
        "source": "ca://BL22I-EA-PILAT-03:HDF5:FullFileName_RBV",
        "shape": [],
        "dtype": "number",
        "external": "STREAM:",
        "object_name": "waxs"
      },
      "i0": {
        "source": "ca://BL22I-EA-XBPM-02:HDF5:FullFileName_RBV",
        "shape": [
          11,
          1000
        ],
        "dtype": "array",
        "external": "STREAM:",
        "object_name": "i0"
      },
      "i0-temperature": {
        "source": "ca://BL22I-EA-XBPM-02:HDF5:FullFileName_RBV",
        "shape": [],
        "dtype": "number",
        "external": "STREAM:",
        "object_name": "i0"
      },
      "it": {
        "source": "ca://BL22I-EA-TTRM-02:HDF5:FullFileName_RBV",
        "shape": [
          11,
          1000
        ],
        "dtype": "array",
        "external": "STREAM:",
        "object_name": "it"
      }
    },
    "name": "primary",
    "object_keys": {
      "saxs": [
        "saxs",
        "saxs-sum"
      ],
      "waxs": [
        "waxs",
        "waxs-sum"
      ],
      "i0": [
        "i0",
        "i0-temperature"
      ],
      "it": [
        "it"
      ]
    },
    "run_start": "685941e0-9563-4a28-a303-541d099d15e3",
    "time": 1700565839.0465171,
    "uid": "28e70ece-ad7f-492d-9c37-0b14a19f6567",
    "hints": {
      "saxs": [
        "saxs"
      ],
      "waxs": [
        "waxs"
      ],
      "i0": [
        "i0"
      ],
      "it": [
        "it"
      ]
    }
  }
}

Improve `connect` protocol to take timeout

At the moment we have:

ophyd-async/src/ophyd_async/core/device.py

Lines 53 to 65 in f1b4aba

 async def connect(self, sim: bool = False): 

 """Connect self and all child Devices. 

  Parameters 

  ---------- 

  sim: 

  If True then connect in simulation mode. 

  """ 

 coros = { 

 name: child_device.connect(sim) for name, child_device in self.children() 

 } 

 if coros: 

 await wait_for_connection(**coros)

This doesn't take timeout, so to add timeout on top we wrap with asyncio.wait or similar (like other primitives in the asyncio library). Unfortunately this makes logging the error difficult as connect calls recursively connect child devices. The way we report reasonable errors at the moment is to catch the CancelledError that is injected when the task times out, and raise NotConnectedError with the name of the signal in question, accumulating them in the parent until we produce a single top level NotConnectedError with all the failing signals:

ophyd-async/src/ophyd_async/core/utils.py

Lines 26 to 53 in f1b4aba

 async def wait_for_connection(**coros: Awaitable[None]): 

 """Call many underlying signals, accumulating `NotConnected` exceptions 

  Raises 

  ------ 

  `NotConnected` if cancelled 

  """ 

 ts = {k: asyncio.create_task(c) for (k, c) in coros.items()} # type: ignore 

 try: 

 done, pending = await asyncio.wait(ts.values()) 

 except asyncio.CancelledError: 

 for t in ts.values(): 

 t.cancel() 

 lines: List[str] = [] 

 for k, t in ts.items(): 

 try: 

 await t 

 except NotConnected as e: 

 if len(e.lines) == 1: 

 lines.append(f"{k}: {e.lines[0]}") 

 else: 

 lines.append(f"{k}:") 

 lines += [f" {line}" for line in e.lines] 

 raise NotConnected(*lines) 

 else: 

 # Wait for everything to foreground the exceptions 

 for f in list(done) + list(pending): 

 await f

This is horrible. It also bites people who await device.connect() rather than using asyncio.wait like in DiamondLightSource/dodal#223.

@callumforrester suggested a better way, pass the timeout down, then let each child produce a class ConnectTimeoutError(TimeoutError) when it times out, then assemble it at the top level with an asyncio.gather(*coros, return_exceptions=True), squashing ConnectTimeoutErrors into a single one.

This would change the signature to:

async def connect(self, sim: bool = False, timeout: float = DEFAULT_TIMEOUT):

with each concrete class raising ConnectTimeoutError if it times out, then wait_for_connection doing the squashing of ConnectTimeoutErrors.

Is num=0 standard for all detectors in a flyscan?

At the moment, flyscanning assumes that setting num=0 for the detector controller will, essentially, tell the detector to take images forever.

There are two PR's that have been merged because this assumption turned out not to be true for pilatus detectors (#59 and #67). The question is; is this true regardless of facility? Is this just because of how our EPICS IOC's are configured for those detectors? We need to ensure the pilatus logic we have in ophyd_async.core is applicable across several institutions.

limited functionality of AsyncStatus wrap

At the moment, AsyncStatus.wrap does not allow wrapping:

methods of classes which have args or kwargs,
functions outside of classes

    @classmethod
    def wrap(cls, f: Callable[[T], Coroutine]) -> Callable[[T], "AsyncStatus"]:
        @functools.wraps(f)
        def wrap_f(self) -> AsyncStatus:
            return AsyncStatus(f(self))

        return wrap_f

The above means if a function is wrapped with this class method, my_func = AsyncStatus.wrap(my_func), if the function signature originally looked something like my_func(self) it behaves as expected, but if it has a signature like my_func(self, a, b, c, extra=None) the wrapping means you can no longer pass extra arguments to it.

pvget .put doesn't work with enums

Working on a panda on P38, I've come across as scenario where a .set on a SignalRW works with channel access, but doesn't work with pvaccess.

That is,

In[3]: source = "BL38P-PANDA:SEQ1:PRESCALE:UNITS"

In[4]: pv_signal = epics_signal_rw(None, "pva://" + source)
In[5]: ca_signal = epics_signal_rw(None, source)

In[6]: await ca_signal.set("s")

In[7]: await ca_signal.get_value()
Out[7]: <GeneratedChoices.s: 's'>

In[8]: await ca_signal.set("ms")

In[9]: await ca_signal.get_value()
Out[9]: <GeneratedChoices.ms: 'ms'>

In[10]: await pv_signal.set("s")
Out[9]: <GeneratedChoices.ms: 'ms'>

Discussion: Device Collector

Background

DeviceCollector is an context manager that can be used to connect one or more devices to their backends in an easy-to-write way:

# Outside a running event loop
with DeviceCollector():
    x = MyMotor(...)
    y = MyMotor(...)
    det = MyDetector(...)

# Inside a running event loop
async with DeviceCollector():
    x = MyMotor(...)
    y = MyMotor(...)
    det = MyDetector(...)

Because it is ophyd-async, a running event loop is actually needed in both cases. The outside case is for when there is already an event loop running in the background, which will have been created by the RunEngine constructor when, for example, you have a RunEngine in an IPython terminal.

This introduces two potential failure cases:

If you call with DeviceCollector() before RE = RunEngine() the call will fail with a cryptic error message about being unable to find the event loop, which will confuse anyone who does not know about this hidden functionality.
If you call async with DeviceCollector() inside a function invoked by asyncio.run (as we do in the ophyd-async tests) it will work without a RunEngine. However if you then create a RunEngine and try to use the devices you created inside it it will fail because the RunEngine uses its own background event loop by default and the devices are using the main event loop. The RunEngine can be set to use the same event loop as the devices by passing an additional argument to the constructor but again, the user has to know that.

Currently we are providing a totally RunEngine-independent route to making and controlling ophyd-async devices (currently async with) and an async-free route which annoyingly requires a RunEngine (with).

Finally, in the interest of modularity, it is desirable to minimize or even remove ophyd-async's dependency on bluesky.

First Steps

The first step is to improve the error message in the first failure case so the user is at least informed that they need to make a RunEngine first. Ideally the message should point to a docs page that explains these nuances and what the two possible context managers give you.

	class SignalX(Signal):
	"""Signal that puts the default value"""

	async def execute(self, wait=True, timeout=None):
	"""Execute the action and return a status saying when it's done"""
	await self._backend.put(None, wait=wait, timeout=timeout or self._timeout)

	async def connect(self, sim: bool = False):
	"""Connect self and all child Devices.

	Parameters
	----------
	sim:
	If True then connect in simulation mode.
	"""
	coros = {
	name: child_device.connect(sim) for name, child_device in self.children()
	}
	if coros:
	await wait_for_connection(**coros)

	async def wait_for_connection(**coros: Awaitable[None]):
	"""Call many underlying signals, accumulating `NotConnected` exceptions

	Raises
	------
	`NotConnected` if cancelled
	"""
	ts = {k: asyncio.create_task(c) for (k, c) in coros.items()} # type: ignore
	try:
	done, pending = await asyncio.wait(ts.values())
	except asyncio.CancelledError:
	for t in ts.values():
	t.cancel()
	lines: List[str] = []
	for k, t in ts.items():
	try:
	await t
	except NotConnected as e:
	if len(e.lines) == 1:
	lines.append(f"{k}: {e.lines[0]}")
	else:
	lines.append(f"{k}:")
	lines += [f" {line}" for line in e.lines]
	raise NotConnected(*lines)
	else:
	# Wait for everything to foreground the exceptions
	for f in list(done) + list(pending):
	await f

bluesky / ophyd-async Goto Github PK

ophyd-async's Introduction

ophyd-async

ophyd-async's People

Contributors

Stargazers

Watchers

Forkers

ophyd-async's Issues

Acceptance Criteria

Problem

Possible fix

Acceptance Criteria

Acceptance Criteria

Background

Thoughts on Solutions

Background

First Steps

Other Solutions Discussed

Recommend Projects

Recommend Topics

Recommend Org