-
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
Open
stan-dot
wants to merge
71
commits into
main
Choose a base branch
from
483-make-device_factory-decorator-to-ease-making-and-connecting-ophyd-async-devices
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
71 commits
Select commit
Hold shift + click to select a range
2a43ada
example beamline with implementation
stan-dot a0f7516
small cleanup
stan-dot 73717bb
added controller demonstration
stan-dot 9ca8158
fix wrong extension
stan-dot 8785072
Update src/dodal/beamlines/example_beamline.py
stan-dot 6fd997a
Update src/dodal/beamlines/example_beamline.py
stan-dot d124d39
Update src/dodal/beamlines/example_beamline.py
stan-dot 6d15da1
Update src/dodal/beamlines/example_beamline.py
stan-dot 2a7b434
fix example beamline into a dataclass
stan-dot d4c1966
new-i22 as an example
stan-dot 4b6b461
respond to feedback
stan-dot 525ff16
respond to comments
stan-dot 0abbd03
initial test
stan-dot a3158d3
comment out some tests to check coverage
stan-dot 933950e
lint
stan-dot 16901f6
use factory name for active devices dict key
stan-dot 6ba5373
this cannot be active devices, globals harder to mock
stan-dot 2339cd0
amend the test
stan-dot 0742041
added a red test for caching by name
stan-dot 81610a1
fix the tests and add the support for a name flag
stan-dot b39af24
add the API for blueapi
stan-dot fa5c9ef
remove todos
stan-dot 5dd9536
unused utils deleted for coverage
stan-dot b00629a
Just one i22 now
stan-dot 0b81dfa
respond to comments
stan-dot 1429067
rename to device factory
stan-dot fc3e45a
is this enough to replace skip_device()
stan-dot 7fa038e
ruff
stan-dot d00d131
remove the connect default timeout =10
stan-dot 7126703
add support for the terminal use case and some tests
stan-dot 32608cf
fix mock status and add the new decorator@
stan-dot e10c941
fix the tests
stan-dot 9162925
delete bad comments, add good comments
stan-dot 77082e4
ruff
stan-dot 940cf3b
feedback response
stan-dot 1a9e5fe
add return type annotation for testing coverage purposes
stan-dot 1ffb221
respond to feedback
stan-dot 84953f0
adapt tetramm
stan-dot fe4cfb8
respond to feedback
stan-dot 00747a5
add repr that includes the config
stan-dot 4ea0de7
lint
stan-dot 733d623
remove old panda ref
stan-dot dab0263
fix import
stan-dot 73ccdc3
fix the tests
stan-dot 4b7feeb
pin pyright version due to is_device check failing
stan-dot 9a982bc
cancel pyright pinning
stan-dot e610a46
renaming
stan-dot 9ed7099
partial update to fix the test coverage for the factory
stan-dot 63a11eb
maybe fixed
stan-dot 1103792
imports fix
stan-dot ada96ba
fix imports
stan-dot 67ec50b
forgot one
stan-dot 7791803
added test to increase coverage, still broken a bit
stan-dot 68a3261
towards a stable problem definitio a stable problme definition
stan-dot 8bf345e
fix the test instance
stan-dot f9afc85
finally fixed tests
stan-dot c28359c
lint
stan-dot d8046b7
fix tests
stan-dot 3083cec
exit some tests early
stan-dot b0711b2
remove pytest skip
stan-dot 4d216d5
hardcode the i22 slit refs
stan-dot 6b3f62a
i22 remove device_instantiation
stan-dot b6a9a34
respond to feedback
stan-dot 77a8f12
more tests to raise coverage
stan-dot 108d78e
respond to comment on the connect logic in controller call
stan-dot 98414db
deprecation warning added
stan-dot e06e4ee
remove the v1 ophyd relevant piece of logic - fake_with_ophyd_sim
stan-dot 5d43fa8
halfway through
stan-dot 757aa98
identified the issue
stan-dot 9138761
name param issues
stan-dot 97f5450
add new appropriate mocks
stan-dot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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