Skip to content
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
wants to merge 67 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 64 commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
6d1e89b
example beamline with implementation
stan-dot Jul 24, 2024
dd9036f
small cleanup
stan-dot Jul 24, 2024
5cf60d0
added controller demonstration
stan-dot Jul 24, 2024
538f96f
fix wrong extension
stan-dot Jul 26, 2024
85d1d7e
Update src/dodal/beamlines/example_beamline.py
stan-dot Jul 26, 2024
dcf1586
Update src/dodal/beamlines/example_beamline.py
stan-dot Jul 26, 2024
dbabb07
Update src/dodal/beamlines/example_beamline.py
stan-dot Jul 26, 2024
af74ac6
Update src/dodal/beamlines/example_beamline.py
stan-dot Jul 26, 2024
ce52660
fix example beamline into a dataclass
stan-dot Jul 26, 2024
25ee88c
new-i22 as an example
stan-dot Jul 26, 2024
dc840be
respond to feedback
stan-dot Jul 30, 2024
b017502
respond to comments
stan-dot Aug 1, 2024
e5b6715
initial test
stan-dot Aug 1, 2024
7e53897
comment out some tests to check coverage
stan-dot Aug 1, 2024
3fdbe46
lint
stan-dot Aug 7, 2024
b548d6d
use factory name for active devices dict key
stan-dot Aug 8, 2024
992f8b0
this cannot be active devices, globals harder to mock
stan-dot Aug 8, 2024
ccae219
amend the test
stan-dot Aug 21, 2024
ed434a3
added a red test for caching by name
stan-dot Aug 21, 2024
d10d6d7
fix the tests and add the support for a name flag
stan-dot Aug 21, 2024
f64975d
add the API for blueapi
stan-dot Aug 21, 2024
88ee2e2
remove todos
stan-dot Aug 22, 2024
29a22b0
unused utils deleted for coverage
stan-dot Aug 27, 2024
4b3235d
Just one i22 now
stan-dot Aug 27, 2024
069381a
respond to comments
stan-dot Aug 28, 2024
c295f05
rename to device factory
stan-dot Aug 28, 2024
f9dad19
is this enough to replace skip_device()
stan-dot Aug 28, 2024
19e3bac
ruff
stan-dot Aug 28, 2024
166f58b
remove the connect default timeout =10
stan-dot Aug 28, 2024
d9356d7
add support for the terminal use case and some tests
stan-dot Aug 29, 2024
f93319d
fix mock status and add the new decorator@
stan-dot Aug 29, 2024
e8b0b74
fix the tests
stan-dot Aug 29, 2024
0748039
delete bad comments, add good comments
stan-dot Aug 29, 2024
22f307c
ruff
stan-dot Aug 29, 2024
4e33ae5
feedback response
stan-dot Aug 29, 2024
786fc65
add return type annotation for testing coverage purposes
stan-dot Aug 29, 2024
a0efba7
respond to feedback
stan-dot Sep 9, 2024
9306ba6
adapt tetramm
stan-dot Sep 10, 2024
e41f7b7
respond to feedback
stan-dot Sep 10, 2024
84e640b
add repr that includes the config
stan-dot Sep 10, 2024
80cded5
lint
stan-dot Sep 10, 2024
924b97c
remove old panda ref
stan-dot Sep 10, 2024
0455eb8
fix import
stan-dot Sep 10, 2024
7a64267
fix the tests
stan-dot Sep 12, 2024
f22c1ac
pin pyright version due to is_device check failing
stan-dot Sep 12, 2024
dbbb974
cancel pyright pinning
stan-dot Sep 13, 2024
62d13a7
renaming
stan-dot Sep 13, 2024
5b06829
partial update to fix the test coverage for the factory
stan-dot Sep 13, 2024
4c545ad
maybe fixed
stan-dot Sep 13, 2024
0d27715
imports fix
stan-dot Sep 13, 2024
5c3441c
fix imports
stan-dot Sep 13, 2024
69e2139
forgot one
stan-dot Sep 13, 2024
2de70eb
added test to increase coverage, still broken a bit
stan-dot Sep 20, 2024
d7e72b7
towards a stable problem definitio a stable problme definition
stan-dot Sep 20, 2024
a7c8d11
fix the test instance
stan-dot Sep 20, 2024
3ba106c
finally fixed tests
stan-dot Sep 20, 2024
23116ea
lint
stan-dot Sep 20, 2024
e240056
fix tests
stan-dot Sep 20, 2024
edcb5c4
exit some tests early
stan-dot Sep 20, 2024
5643473
remove pytest skip
stan-dot Sep 23, 2024
9595f95
hardcode the i22 slit refs
stan-dot Sep 23, 2024
4573095
i22 remove device_instantiation
stan-dot Sep 24, 2024
686fc25
respond to feedback
stan-dot Sep 24, 2024
bc4cf6f
more tests to raise coverage
stan-dot Sep 24, 2024
2f20ee3
respond to comment on the connect logic in controller call
stan-dot Sep 25, 2024
f224a17
deprecation warning added
stan-dot Sep 27, 2024
f188505
remove the v1 ophyd relevant piece of logic - fake_with_ophyd_sim
stan-dot Sep 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions docs/explanations/decisions/0003-make-devices-factory.md
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.
10 changes: 10 additions & 0 deletions src/dodal/aliases.py
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
61 changes: 40 additions & 21 deletions src/dodal/beamlines/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,49 @@
}


def all_beamline_modules() -> Iterable[str]:
"""
Get the names of all importable modules in beamlines
class ModuleDiscoveryError(Exception):
"""Custom exception for module discovery errors."""

pass


def get_all_beamline_modules() -> Iterable[str]:
"""
Get the names of all importable modules in the beamlines package by inspecting file paths.
Returns:
Iterable[str]: Generator of beamline module names
Iterable[str]: Generator of beamline module names (excluding __init__.py and __pycache__).
Raises:
ModuleDiscoveryError: If the beamlines module could not be found.
"""

# This is done by inspecting file names rather than modules to avoid
# premature importing
spec = importlib.util.find_spec(__name__)
if spec is not None:
assert spec.submodule_search_locations
search_paths = [Path(path) for path in spec.submodule_search_locations]
for path in search_paths:
for subpath in path.glob("**/*"):
if (
subpath.name.endswith(".py")
and subpath.name != "__init__.py"
and ("__pycache__" not in str(subpath))
):
yield subpath.with_suffix("").name
else:
raise KeyError(f"Unable to find {__name__} module")
try:
# Find the package spec for the current module
spec = importlib.util.find_spec(__name__)
except Exception as e:
raise ModuleDiscoveryError(
f"Error while finding module spec for {__name__}: {e}"
) from e

if not spec or not spec.submodule_search_locations:
raise ModuleDiscoveryError(
Copy link
Contributor

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

Copy link
Contributor Author

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

f"Unable to find module search locations for {__name__}"
)

search_paths = [Path(path) for path in spec.submodule_search_locations]

# Yield valid module names by filtering Python files
for path in search_paths:
for subpath in path.glob("**/*.py"):
if _is_valid_module(subpath):
yield subpath.stem # `.stem` gives the filename without the suffix


def _is_valid_module(subpath):
return (
subpath.name.endswith(".py")
and subpath.name != "__init__.py"
and ("__pycache__" not in str(subpath))
)


def all_beamline_names() -> Iterable[str]:
Expand All @@ -54,7 +73,7 @@ def all_beamline_names() -> Iterable[str]:
Iterable[str]: Generator of beamline names that dodal supports
"""
inverse_mapping = _module_name_overrides()
for module_name in all_beamline_modules():
for module_name in get_all_beamline_modules():
yield from inverse_mapping.get(module_name, set()).union({module_name})


Expand Down
26 changes: 26 additions & 0 deletions src/dodal/beamlines/i-min.py
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")
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was just for local testing, can be deleted

Copy link
Contributor

Choose a reason for hiding this comment

The 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(),
)
4 changes: 3 additions & 1 deletion src/dodal/beamlines/i03.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

from dodal.common.beamlines.beamline_parameters import get_beamline_parameters
from dodal.common.beamlines.beamline_utils import (
BeamlinePrefix,
device_instantiation,
get_path_provider,
set_path_provider,
skip_device,
)
from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline
from dodal.common.udc_directory_provider import PandASubpathProvider
Expand Down Expand Up @@ -40,7 +42,7 @@
from dodal.devices.zebra_controlled_shutter import ZebraShutter
from dodal.devices.zocalo import ZocaloResults
from dodal.log import set_beamline as set_log_beamline
from dodal.utils import BeamlinePrefix, get_beamline_name, skip_device
from dodal.utils import get_beamline_name

ZOOM_PARAMS_FILE = (
"/dls_sw/i03/software/gda/configurations/i03-config/xml/jCameraManZoomLevels.xml"
Expand Down
8 changes: 6 additions & 2 deletions src/dodal/beamlines/i04.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
from dodal.common.beamlines.beamline_parameters import get_beamline_parameters
from dodal.common.beamlines.beamline_utils import device_instantiation
from dodal.common.beamlines.beamline_utils import (
BeamlinePrefix,
device_instantiation,
skip_device,
)
from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline
from dodal.devices.aperturescatterguard import (
AperturePosition,
Expand Down Expand Up @@ -29,7 +33,7 @@
from dodal.devices.zebra import Zebra
from dodal.devices.zebra_controlled_shutter import ZebraShutter
from dodal.log import set_beamline as set_log_beamline
from dodal.utils import BeamlinePrefix, get_beamline_name, skip_device
from dodal.utils import get_beamline_name

ZOOM_PARAMS_FILE = (
"/dls_sw/i04/software/gda/configurations/i04-config/xml/jCameraManZoomLevels.xml"
Expand Down
8 changes: 6 additions & 2 deletions src/dodal/beamlines/i04_1.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
from dodal.common.beamlines.beamline_utils import device_instantiation
from dodal.common.beamlines.beamline_utils import (
BeamlinePrefix,
device_instantiation,
skip_device,
)
from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline
from dodal.devices.backlight import Backlight
from dodal.devices.detector import DetectorParams
Expand All @@ -9,7 +13,7 @@
from dodal.devices.undulator import Undulator
from dodal.devices.zebra import Zebra
from dodal.log import set_beamline as set_log_beamline
from dodal.utils import BeamlinePrefix, get_beamline_name, skip_device
from dodal.utils import get_beamline_name

ZOOM_PARAMS_FILE = "/dls_sw/i04-1/software/gda/config/xml/jCameraManZoomLevels.xml"
DISPLAY_CONFIG = "/dls_sw/i04-1/software/gda_versions/var/display.configuration"
Expand Down
Loading
Loading