-
Notifications
You must be signed in to change notification settings - Fork 8
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
make device factory decorator #597
base: main
Are you sure you want to change the base?
Changes from 64 commits
6d1e89b
dd9036f
5cf60d0
538f96f
85d1d7e
dcf1586
dbabb07
af74ac6
ce52660
25ee88c
dc840be
b017502
e5b6715
7e53897
3fdbe46
b548d6d
992f8b0
ccae219
ed434a3
d10d6d7
f64975d
88ee2e2
29a22b0
4b3235d
069381a
c295f05
f9dad19
19e3bac
166f58b
d9356d7
f93319d
e8b0b74
0748039
22f307c
4e33ae5
786fc65
a0efba7
9306ba6
e41f7b7
84e640b
80cded5
924b97c
0455eb8
7a64267
f22c1ac
dbbb974
62d13a7
5b06829
4c545ad
0d27715
5c3441c
69e2139
2de70eb
d7e72b7
a7c8d11
3ba106c
23116ea
e240056
edcb5c4
5643473
9595f95
4573095
686fc25
bc4cf6f
2f20ee3
f224a17
f188505
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
# 3. Add device factory decorator with lazy connect support | ||
|
||
Date: 2024-04-26 | ||
|
||
## Status | ||
|
||
Accepted | ||
|
||
## Context | ||
|
||
We should add a decorator to support verified device connection. | ||
|
||
## Decision | ||
|
||
DAQ members led us to this proposal: | ||
|
||
- ophyd-async: make Device.connect(mock, timeout, force=False) be idempotent | ||
- ophyd-async: make ensure_connected(\*devices) plan stub | ||
- dodal: make device_factory(eager=False) decorator that makes, names, caches and optionally connects a device | ||
- dodal: make get_device_factories() that returns all device factories and whether they should be connected at startup | ||
- blueapi: call get_device_factories(), make all the Devices, connect the ones that should be connected at startup in parallel and log those that fail | ||
- blueapi: when plan is called, run ensure_connected on all plan args and defaults that are Devices | ||
|
||
We can then iterate on this if the parallel connect causes a broadcast storm. We could also in future add a monitor to a heartbeat PV per device in Device.connect so that it would reconnect next time it was called. | ||
|
||
## Consequences | ||
|
||
The changes above. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
from collections.abc import Callable | ||
from typing import TypeAlias | ||
|
||
from ophyd.device import Device as OphydV1Device | ||
from ophyd_async.core import Device as OphydV2Device | ||
|
||
AnyDevice: TypeAlias = OphydV1Device | OphydV2Device | ||
V1DeviceFactory: TypeAlias = Callable[..., OphydV1Device] | ||
V2DeviceFactory: TypeAlias = Callable[..., OphydV2Device] | ||
AnyDeviceFactory: TypeAlias = V1DeviceFactory | V2DeviceFactory |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
from ophyd_async.fastcs.panda import HDFPanda | ||
|
||
from dodal.common.beamlines.beamline_utils import get_path_provider | ||
from dodal.common.beamlines.device_factory import device_factory | ||
from dodal.devices.i22.fswitch import FSwitch | ||
from dodal.utils import get_beamline_name | ||
|
||
BL = get_beamline_name("i-min") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should: What is this beamline? Is it just used for testing things? Why do we need a specific test beamline? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was just for local testing, can be deleted There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great, please can you delete it then? |
||
|
||
|
||
@device_factory() | ||
def fswitch() -> FSwitch: | ||
return FSwitch( | ||
prefix="-MO-FSWT-01:", | ||
lens_geometry="paraboloid", | ||
cylindrical=True, | ||
lens_material="Beryllium", | ||
) | ||
|
||
|
||
@device_factory() | ||
def panda1() -> HDFPanda: | ||
return HDFPanda( | ||
prefix="-EA-PANDA-01:", | ||
path_provider=get_path_provider(), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should: A unit test covering this would be good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok that is one of the lines not covered in the test, I'll try to come up with something