diff --git a/docs/explanations/decisions/0003-make-devices-factory.md b/docs/explanations/decisions/0003-make-devices-factory.md new file mode 100644 index 0000000000..7a878eddf0 --- /dev/null +++ b/docs/explanations/decisions/0003-make-devices-factory.md @@ -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. diff --git a/src/dodal/aliases.py b/src/dodal/aliases.py new file mode 100644 index 0000000000..275b1db165 --- /dev/null +++ b/src/dodal/aliases.py @@ -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 diff --git a/src/dodal/beamlines/__init__.py b/src/dodal/beamlines/__init__.py index 7e71435698..25f649b6d9 100644 --- a/src/dodal/beamlines/__init__.py +++ b/src/dodal/beamlines/__init__.py @@ -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( + 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]: @@ -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}) diff --git a/src/dodal/beamlines/i-min.py b/src/dodal/beamlines/i-min.py new file mode 100644 index 0000000000..39222d7107 --- /dev/null +++ b/src/dodal/beamlines/i-min.py @@ -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") + + +@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(), + ) diff --git a/src/dodal/beamlines/i03.py b/src/dodal/beamlines/i03.py index 0401c288f6..23f395fc22 100644 --- a/src/dodal/beamlines/i03.py +++ b/src/dodal/beamlines/i03.py @@ -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 @@ -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" diff --git a/src/dodal/beamlines/i04.py b/src/dodal/beamlines/i04.py index 96b6a48de0..876acc1091 100644 --- a/src/dodal/beamlines/i04.py +++ b/src/dodal/beamlines/i04.py @@ -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, @@ -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" diff --git a/src/dodal/beamlines/i04_1.py b/src/dodal/beamlines/i04_1.py index 2341d9a033..c2147c1a7c 100644 --- a/src/dodal/beamlines/i04_1.py +++ b/src/dodal/beamlines/i04_1.py @@ -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 @@ -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" diff --git a/src/dodal/beamlines/i22.py b/src/dodal/beamlines/i22.py index 4b42694a98..e50cfe6df6 100644 --- a/src/dodal/beamlines/i22.py +++ b/src/dodal/beamlines/i22.py @@ -5,13 +5,16 @@ from ophyd_async.fastcs.panda import HDFPanda from dodal.common.beamlines.beamline_utils import ( - device_instantiation, + BeamlinePrefix, get_path_provider, set_path_provider, ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline -from dodal.common.beamlines.device_helpers import numbered_slits -from dodal.common.visit import RemoteDirectoryServiceClient, StaticVisitPathProvider +from dodal.common.beamlines.device_factory import device_factory +from dodal.common.visit import ( + RemoteDirectoryServiceClient, + StaticVisitPathProvider, +) from dodal.devices.focusing_mirror import FocusingMirror from dodal.devices.i22.dcm import CrystalMetadata, DoubleCrystalMonochromator from dodal.devices.i22.fswitch import FSwitch @@ -22,7 +25,7 @@ from dodal.devices.tetramm import TetrammDetector from dodal.devices.undulator import Undulator 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 BL = get_beamline_name("i22") set_log_beamline(BL) @@ -42,339 +45,214 @@ ) -def saxs( - wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False -) -> PilatusDetector: - return device_instantiation( - NXSasPilatus, - "saxs", - "-EA-PILAT-01:", - wait_for_connection, - fake_with_ophyd_sim, +@device_factory() +def saxs() -> PilatusDetector: + metadata_holder = NXSasMetadataHolder( + x_pixel_size=(1.72e-1, "mm"), + y_pixel_size=(1.72e-1, "mm"), + description="Dectris Pilatus3 2M", + type="Photon Counting Hybrid Pixel", + sensor_material="silicon", + sensor_thickness=(0.45, "mm"), + distance=(4711.833684146172, "mm"), + ) + + return NXSasPilatus( + prefix="-EA-PILAT-01:", drv_suffix="CAM:", hdf_suffix="HDF5:", - metadata_holder=NXSasMetadataHolder( - x_pixel_size=(1.72e-1, "mm"), - y_pixel_size=(1.72e-1, "mm"), - description="Dectris Pilatus3 2M", - type="Photon Counting Hybrid Pixel", - sensor_material="silicon", - sensor_thickness=(0.45, "mm"), - distance=(4711.833684146172, "mm"), - ), + metadata_holder=metadata_holder, path_provider=get_path_provider(), ) -def synchrotron( - wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False -) -> Synchrotron: - return device_instantiation( - Synchrotron, - "synchrotron", - "", - wait_for_connection, - fake_with_ophyd_sim, - ) +@device_factory() +def synchrotron() -> Synchrotron: + return Synchrotron() + +@device_factory() +def waxs() -> PilatusDetector: + metadata_holder = NXSasMetadataHolder( + x_pixel_size=(1.72e-1, "mm"), + y_pixel_size=(1.72e-1, "mm"), + description="Dectris Pilatus3 2M", + type="Photon Counting Hybrid Pixel", + sensor_material="silicon", + sensor_thickness=(0.45, "mm"), + distance=(175.4199417092314, "mm"), + ) -def waxs( - wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False -) -> PilatusDetector: - return device_instantiation( - NXSasPilatus, - "waxs", - "-EA-PILAT-03:", - wait_for_connection, - fake_with_ophyd_sim, + return NXSasPilatus( + prefix="-EA-PILAT-03:", drv_suffix="CAM:", hdf_suffix="HDF5:", - metadata_holder=NXSasMetadataHolder( - x_pixel_size=(1.72e-1, "mm"), - y_pixel_size=(1.72e-1, "mm"), - description="Dectris Pilatus3 2M", - type="Photon Counting Hybrid Pixel", - sensor_material="silicon", - sensor_thickness=(0.45, "mm"), - distance=(175.4199417092314, "mm"), - ), + metadata_holder=metadata_holder, path_provider=get_path_provider(), ) -def i0( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> TetrammDetector: - return device_instantiation( - TetrammDetector, - "i0", - "-EA-XBPM-02:", - wait_for_connection, - fake_with_ophyd_sim, +@device_factory() +def i0() -> TetrammDetector: + return TetrammDetector( + prefix="-EA-XBPM-02:", type="Cividec Diamond XBPM", path_provider=get_path_provider(), ) -def it( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> TetrammDetector: - return device_instantiation( - TetrammDetector, - "it", - "-EA-TTRM-02:", - wait_for_connection, - fake_with_ophyd_sim, +@device_factory() +def it() -> TetrammDetector: + return TetrammDetector( + prefix="-EA-TTRM-02:", type="PIN Diode", path_provider=get_path_provider(), ) -def vfm( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> FocusingMirror: - return device_instantiation( - FocusingMirror, - "vfm", - "-OP-KBM-01:VFM:", - wait_for_connection, - fake_with_ophyd_sim, - ) +@device_factory() +def vfm() -> FocusingMirror: + return FocusingMirror(name="vfm", prefix="-OP-KBM-01:VFM:") + +@device_factory() +def hfm() -> FocusingMirror: + return FocusingMirror(name="hfm", prefix="-OP-KBM-01:HFM:") -def hfm( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> FocusingMirror: - return device_instantiation( - FocusingMirror, - "hfm", - "-OP-KBM-01:HFM:", - wait_for_connection, - fake_with_ophyd_sim, + +@device_factory() +def dcm() -> DoubleCrystalMonochromator: + prefix = BeamlinePrefix(BL).beamline_prefix + silicon_111 = CrystalMetadata( + usage="Bragg", + type="silicon", + reflection=(1, 1, 1), + d_spacing=(3.13475, "nm"), ) + silicon_311 = CrystalMetadata( + usage="Bragg", + type="silicon", + reflection=(3, 1, 1), + # todo update d_spacing values to match the reflection maths + d_spacing=(3.13475, "nm"), + ) -def dcm( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> DoubleCrystalMonochromator: - return device_instantiation( - DoubleCrystalMonochromator, - "dcm", - "", - wait_for_connection, - fake_with_ophyd_sim, - bl_prefix=False, - motion_prefix=f"{BeamlinePrefix(BL).beamline_prefix}-MO-DCM-01:", - temperature_prefix=f"{BeamlinePrefix(BL).beamline_prefix}-DI-DCM-01:", - crystal_1_metadata=CrystalMetadata( - usage="Bragg", - type="silicon", - reflection=(1, 1, 1), - d_spacing=(3.13475, "nm"), - ), - crystal_2_metadata=CrystalMetadata( - usage="Bragg", - type="silicon", - reflection=(1, 1, 1), - d_spacing=(3.13475, "nm"), - ), + return DoubleCrystalMonochromator( + motion_prefix=f"{prefix}-MO-DCM-01:", + temperature_prefix=f"{prefix}-DI-DCM-01:", + crystal_1_metadata=silicon_111, + crystal_2_metadata=silicon_311, ) -def undulator( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> Undulator: - return device_instantiation( - Undulator, - "undulator", - f"{BeamlinePrefix(BL).insertion_prefix}-MO-SERVC-01:", - wait_for_connection, - fake_with_ophyd_sim, - bl_prefix=False, +@device_factory() +def undulator() -> Undulator: + return Undulator( + prefix=f"{BeamlinePrefix(BL).insertion_prefix}-MO-SERVC-01:", poles=80, length=2.0, id_gap_lookup_table_path="/dls_sw/i22/software/daq_configuration/lookup/BeamLine_Undulator_toGap.txt", ) -def slits_1( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> Slits: - return numbered_slits( - 1, - wait_for_connection, - fake_with_ophyd_sim, - ) +# slits section, s1 is on the frontend, s6 is closest to the sample +@device_factory() +def slits_1() -> Slits: + slit_number = 1 + return Slits(f"-AL-SLITS-{slit_number:02}:", f"slits_{slit_number}") -def slits_2( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> Slits: - return numbered_slits( - 2, - wait_for_connection, - fake_with_ophyd_sim, - ) +@device_factory() +def slits_2() -> Slits: + slit_number = 2 + return Slits(f"-AL-SLITS-{slit_number:02}:", f"slits_{slit_number}") -def slits_3( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> Slits: - return numbered_slits( - 3, - wait_for_connection, - fake_with_ophyd_sim, - ) +@device_factory() +def slits_3() -> Slits: + slit_number = 3 + return Slits(f"-AL-SLITS-{slit_number:02}:", f"slits_{slit_number}") -def slits_4( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> Slits: - return numbered_slits( - 4, - wait_for_connection, - fake_with_ophyd_sim, - ) +@device_factory() +def slits_4() -> Slits: + slit_number = 4 + return Slits(f"-AL-SLITS-{slit_number:02}:", f"slits_{slit_number}") -def slits_5( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> Slits: - return numbered_slits( - 5, - wait_for_connection, - fake_with_ophyd_sim, - ) +@device_factory() +def slits_5() -> Slits: + slit_number = 5 + return Slits(f"-AL-SLITS-{slit_number:02}:", f"slits_{slit_number}") -def slits_6( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> Slits: - return numbered_slits( - 6, - wait_for_connection, - fake_with_ophyd_sim, - ) +@device_factory() +def slits_6() -> Slits: + slit_number = 6 + return Slits(f"-AL-SLITS-{slit_number:02}:", f"slits_{slit_number}") -def fswitch( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> FSwitch: - return device_instantiation( - FSwitch, - "fswitch", - "-MO-FSWT-01:", - wait_for_connection, - fake_with_ophyd_sim, +@device_factory() +def fswitch() -> FSwitch: + return FSwitch( + prefix="-MO-FSWT-01:", lens_geometry="paraboloid", cylindrical=True, lens_material="Beryllium", ) -# Must document what PandAs are physically connected to -# See: https://github.com/bluesky/ophyd-async/issues/284 -def panda1( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> HDFPanda: - return device_instantiation( - HDFPanda, - "panda1", - "-EA-PANDA-01:", - wait_for_connection, - fake_with_ophyd_sim, +@device_factory() +def panda1() -> HDFPanda: + return HDFPanda( + prefix="-EA-PANDA-01:", path_provider=get_path_provider(), ) -@skip_device() -def panda2( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> HDFPanda: - return device_instantiation( - HDFPanda, - "panda2", - "-EA-PANDA-02:", - wait_for_connection, - fake_with_ophyd_sim, +@device_factory(skip=True) +def panda2() -> HDFPanda: + return HDFPanda( + prefix="-EA-PANDA-02:", path_provider=get_path_provider(), ) -@skip_device() -def panda3( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> HDFPanda: - return device_instantiation( - HDFPanda, - "panda3", - "-EA-PANDA-03:", - wait_for_connection, - fake_with_ophyd_sim, +@device_factory(skip=True) +def panda3() -> HDFPanda: + return HDFPanda( + prefix="-EA-PANDA-03:", path_provider=get_path_provider(), ) -@skip_device() -def panda4( - wait_for_connection: bool = True, - fake_with_ophyd_sim: bool = False, -) -> HDFPanda: - return device_instantiation( - HDFPanda, - "panda4", - "-EA-PANDA-04:", - wait_for_connection, - fake_with_ophyd_sim, +@device_factory(skip=True) +def panda4() -> HDFPanda: + return HDFPanda( + prefix="-EA-PANDA-04:", path_provider=get_path_provider(), ) -def oav( - wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False -) -> AravisDetector: - return device_instantiation( - NXSasOAV, - "oav", - "-DI-OAV-01:", - wait_for_connection, - fake_with_ophyd_sim, +@device_factory() +def oav() -> AravisDetector: + metadata_holder = NXSasMetadataHolder( + x_pixel_size=(3.45e-3, "mm"), # Double check this figure + y_pixel_size=(3.45e-3, "mm"), + description="AVT Mako G-507B", + distance=(-1.0, "m"), + ) + + return NXSasOAV( + prefix="-DI-OAV-01:", drv_suffix="DET:", hdf_suffix="HDF5:", - metadata_holder=NXSasMetadataHolder( - x_pixel_size=(3.45e-3, "mm"), # Double check this figure - y_pixel_size=(3.45e-3, "mm"), - description="AVT Mako G-507B", - distance=(-1.0, "m"), - ), + metadata_holder=metadata_holder, path_provider=get_path_provider(), ) -@skip_device() -def linkam( - wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False -) -> Linkam3: - return device_instantiation( - Linkam3, - "linkam", - "-EA-TEMPC-05", - wait_for_connection, - fake_with_ophyd_sim, - ) +@device_factory() +def linkam() -> Linkam3: + return Linkam3(prefix="-EA-TEMPC-05") diff --git a/src/dodal/beamlines/i23.py b/src/dodal/beamlines/i23.py index 4cbbf549f6..a667e1f3dd 100644 --- a/src/dodal/beamlines/i23.py +++ b/src/dodal/beamlines/i23.py @@ -1,8 +1,8 @@ -from dodal.common.beamlines.beamline_utils import device_instantiation +from dodal.common.beamlines.beamline_utils import device_instantiation, skip_device from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.log import set_beamline as set_log_beamline -from dodal.utils import get_beamline_name, get_hostname, skip_device +from dodal.utils import get_beamline_name, get_hostname BL = get_beamline_name("i23") set_log_beamline(BL) diff --git a/src/dodal/beamlines/i24.py b/src/dodal/beamlines/i24.py index a827eea738..4679f3c544 100644 --- a/src/dodal/beamlines/i24.py +++ b/src/dodal/beamlines/i24.py @@ -1,4 +1,4 @@ -from dodal.common.beamlines.beamline_utils import BL, device_instantiation +from dodal.common.beamlines.beamline_utils import BL, device_instantiation, skip_device from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.detector import DetectorParams from dodal.devices.eiger import EigerDetector @@ -14,7 +14,7 @@ from dodal.devices.oav.oav_parameters import OAVConfigParams from dodal.devices.zebra import Zebra from dodal.log import set_beamline as set_log_beamline -from dodal.utils import get_beamline_name, skip_device +from dodal.utils import get_beamline_name ZOOM_PARAMS_FILE = ( "/dls_sw/i24/software/gda_versions/gda_9_34/config/xml/jCameraManZoomLevels.xml" diff --git a/src/dodal/beamlines/p38.py b/src/dodal/beamlines/p38.py index 11da31b0da..4773574547 100644 --- a/src/dodal/beamlines/p38.py +++ b/src/dodal/beamlines/p38.py @@ -4,9 +4,11 @@ from ophyd_async.fastcs.panda import HDFPanda 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.beamlines.device_helpers import numbered_slits @@ -19,7 +21,7 @@ from dodal.devices.tetramm import TetrammDetector from dodal.devices.undulator import Undulator 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 BL = get_beamline_name("p38") set_log_beamline(BL) diff --git a/src/dodal/beamlines/p45.py b/src/dodal/beamlines/p45.py index 4a55060f38..e2d803ba0a 100644 --- a/src/dodal/beamlines/p45.py +++ b/src/dodal/beamlines/p45.py @@ -7,12 +7,13 @@ 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.visit import StaticVisitPathProvider from dodal.devices.p45 import Choppers, TomoStageWithStretchAndSkew from dodal.log import set_beamline as set_log_beamline -from dodal.utils import get_beamline_name, skip_device +from dodal.utils import get_beamline_name BL = get_beamline_name("p45") set_log_beamline(BL) diff --git a/src/dodal/common/beamlines/beamline_utils.py b/src/dodal/common/beamlines/beamline_utils.py index ac1418e44d..52d3df8c1f 100644 --- a/src/dodal/common/beamlines/beamline_utils.py +++ b/src/dodal/common/beamlines/beamline_utils.py @@ -1,5 +1,7 @@ import inspect from collections.abc import Callable +from dataclasses import dataclass +from functools import wraps from typing import Final, TypeVar, cast from bluesky.run_engine import call_in_bluesky_event_loop @@ -8,8 +10,39 @@ from ophyd_async.core import Device as OphydV2Device from ophyd_async.core import wait_for_connection as v2_device_wait_for_connection +from dodal.aliases import AnyDevice from dodal.common.types import UpdatingPathProvider -from dodal.utils import AnyDevice, BeamlinePrefix, skip_device + +T = TypeVar("T", bound=AnyDevice) + + +def skip_device(precondition=lambda: True): + """ + DEPRECATED: Use 'device_factory' decorator instead. + """ + + def decorator(func: Callable[..., T]) -> Callable[..., T]: + @wraps(func) + def wrapper(*args, **kwds) -> T: + return func(*args, **kwds) + + if precondition(): + wrapper.__skip__ = True # type: ignore + return wrapper + + return decorator + + +@dataclass +class BeamlinePrefix: + ixx: str + suffix: str | None = None + + def __post_init__(self): + self.suffix = self.ixx[0].upper() if not self.suffix else self.suffix + self.beamline_prefix = f"BL{self.ixx[1:3]}{self.suffix}" + self.insertion_prefix = f"SR{self.ixx[1:3]}{self.suffix}" + DEFAULT_CONNECTION_TIMEOUT: Final[float] = 5.0 diff --git a/src/dodal/common/beamlines/device_factory.py b/src/dodal/common/beamlines/device_factory.py new file mode 100644 index 0000000000..861d071672 --- /dev/null +++ b/src/dodal/common/beamlines/device_factory.py @@ -0,0 +1,127 @@ +from collections.abc import Callable +from dataclasses import dataclass +from typing import Generic, ParamSpec, TypeVar + +from bluesky.run_engine import call_in_bluesky_event_loop +from ophyd_async.core import DEFAULT_TIMEOUT, Device + +from dodal.common.beamlines.beamline_utils import ACTIVE_DEVICES + +P = ParamSpec("P") +T = TypeVar("T", bound=Device) +_skip = bool | Callable[[], bool] + + +@dataclass +class DeviceInitializationConfig: + """ + eager_connect: connect or raise Exception at startup + use_factory_name: use factory name as name of device + timeout: timeout for connecting to the device + mock: use Signals with mock backends for device + skip: mark the factory to be (conditionally) skipped + when beamline is imported by external program + """ + + eager_connect: bool + use_factory_name: bool + timeout: float + mock: bool + skip: _skip + + +class DeviceInitializationController(Generic[P, T]): + def __init__(self, config: DeviceInitializationConfig, factory: Callable[P, T]): + self._factory: Callable[P, T] = factory + self._config: DeviceInitializationConfig = config + self._cached_device: T | None = None + self.__name__ = factory.__name__ + + @property + def skip(self) -> bool: + return self._config.skip() if callable(self._config.skip) else self._config.skip + + def __repr__(self) -> str: + config_details = f""" + Device: + - factory: {self._factory} + - name: {self.__name__} + - device object: {self.device} + Config settings: + - eager_connect: {self._config.eager_connect} + - use_factory_name: {self._config.use_factory_name} + - timeout: {self._config.timeout} + - mock: {self._config.mock} + - skip: {self._config.skip} + """ + return f"Device initalization controller with:\n{config_details}" + + @property + def device(self) -> T | None: + return self._cached_device + + def __call__( + self, + connect: bool | None = None, + name: str | None = None, + timeout: float | None = None, + mock: bool | None = None, + *args: P.args, + **kwargs: P.kwargs, + ) -> T: + # that is for the second and later times the device is called + if self.device is not None: + return self.device + + # unpack the arguments, fill in the defaults + name = name or ( + self._factory.__name__ if self._config.use_factory_name else None + ) + mock = mock or self._config.mock + timeout = timeout or self._config.timeout + + device = self._factory(*args, **kwargs) + if name: + device.set_name(name) + + # connect the device if needed + if connect: + call_in_bluesky_event_loop( + device.connect( + timeout=timeout, + mock=mock, + ) + ) + + self._cache_device(device) + return device + + def _cache_device(self, device: T): + if device.name: + ACTIVE_DEVICES[device.name] = device + self._cached_device = device + + +def device_factory( + *, + eager: bool = True, + use_factory_name: bool = True, + timeout: float = DEFAULT_TIMEOUT, + mock: bool = False, + skip: _skip = False, +) -> Callable[[Callable[P, T]], DeviceInitializationController[P, T]]: + config = DeviceInitializationConfig( + eager, + use_factory_name, + timeout, + mock, + skip, + ) + + def decorator(factory: Callable[P, T]) -> DeviceInitializationController[P, T]: + return DeviceInitializationController( + config, + factory, + ) + + return decorator diff --git a/src/dodal/common/beamlines/device_helpers.py b/src/dodal/common/beamlines/device_helpers.py index 8e699361aa..e0bf0c0433 100644 --- a/src/dodal/common/beamlines/device_helpers.py +++ b/src/dodal/common/beamlines/device_helpers.py @@ -1,6 +1,5 @@ -from dodal.common.beamlines.beamline_utils import device_instantiation +from dodal.common.beamlines.beamline_utils import device_instantiation, skip_device from dodal.devices.slits import Slits -from dodal.utils import skip_device @skip_device() diff --git a/src/dodal/devices/linkam3.py b/src/dodal/devices/linkam3.py index 7f1c0b77c9..7ed213c875 100644 --- a/src/dodal/devices/linkam3.py +++ b/src/dodal/devices/linkam3.py @@ -34,7 +34,7 @@ class Linkam3(StandardReadable): tolerance: float = 0.5 settle_time: int = 0 - def __init__(self, prefix: str, name: str): + def __init__(self, prefix: str, name: str = ""): self.temp = epics_signal_r(float, prefix + "TEMP:") self.dsc = epics_signal_r(float, prefix + "DSC:") self.start_heat = epics_signal_rw(bool, prefix + "STARTHEAT:") diff --git a/src/dodal/devices/tetramm.py b/src/dodal/devices/tetramm.py index 20e35e4afd..fb23d14eca 100644 --- a/src/dodal/devices/tetramm.py +++ b/src/dodal/devices/tetramm.py @@ -219,7 +219,7 @@ def __init__( self, prefix: str, path_provider: PathProvider, - name: str, + name: str = "", type: str | None = None, **scalar_sigs: str, ) -> None: diff --git a/src/dodal/utils.py b/src/dodal/utils.py index e11b8f2ca5..6df0422d27 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -5,8 +5,6 @@ import socket import string from collections.abc import Callable, Iterable, Mapping -from dataclasses import dataclass -from functools import wraps from importlib import import_module from inspect import signature from os import environ @@ -14,7 +12,6 @@ from typing import ( Any, TypeGuard, - TypeVar, ) from bluesky.protocols import ( @@ -33,16 +30,11 @@ Triggerable, WritesExternalAssets, ) -from ophyd.device import Device as OphydV1Device from ophyd_async.core import Device as OphydV2Device import dodal.log - -try: - from typing import TypeAlias -except ImportError: - from typing import TypeAlias - +from dodal.aliases import AnyDevice, AnyDeviceFactory, V1DeviceFactory, V2DeviceFactory +from dodal.common.beamlines.device_factory import DeviceInitializationController #: Protocols defining interface to hardware BLUESKY_PROTOCOLS = [ @@ -62,11 +54,6 @@ Triggerable, ] -AnyDevice: TypeAlias = OphydV1Device | OphydV2Device -V1DeviceFactory: TypeAlias = Callable[..., OphydV1Device] -V2DeviceFactory: TypeAlias = Callable[..., OphydV2Device] -AnyDeviceFactory: TypeAlias = V1DeviceFactory | V2DeviceFactory - def get_beamline_name(default: str) -> str: return environ.get("BEAMLINE") or default @@ -76,33 +63,6 @@ def get_hostname() -> str: return socket.gethostname().split(".")[0] -@dataclass -class BeamlinePrefix: - ixx: str - suffix: str | None = None - - def __post_init__(self): - self.suffix = self.ixx[0].upper() if not self.suffix else self.suffix - self.beamline_prefix = f"BL{self.ixx[1:3]}{self.suffix}" - self.insertion_prefix = f"SR{self.ixx[1:3]}{self.suffix}" - - -T = TypeVar("T", bound=AnyDevice) - - -def skip_device(precondition=lambda: True): - def decorator(func: Callable[..., T]) -> Callable[..., T]: - @wraps(func) - def wrapper(*args, **kwds) -> T: - return func(*args, **kwds) - - if precondition(): - wrapper.__skip__ = True # type: ignore - return wrapper - - return decorator - - def make_device( module: str | ModuleType, device_name: str, @@ -181,7 +141,7 @@ def invoke_factories( exceptions: dict[str, Exception] = {} # Compute tree of dependencies, - dependencies = { + dependencies: dict[str, set[str]] = { factory_name: set(extract_dependencies(factories, factory_name)) for factory_name in factories.keys() } @@ -243,19 +203,28 @@ def collect_factories( Returns: dict[str, AnyDeviceFactory]: Mapping of factory name -> factory. """ - factories: dict[str, AnyDeviceFactory] = {} - for var in module.__dict__.values(): - if ( - callable(var) - and is_any_device_factory(var) - and (include_skipped or not _is_device_skipped(var)) + all_variables_in_beamline_file = module.__dict__.values() + for variable in all_variables_in_beamline_file: + if _is_valid_factory(include_skipped, variable) and hasattr( + variable, "__name__" ): - factories[var.__name__] = var + factories[variable.__name__] = variable + # elif isinstance(variable, DeviceInitializationController): + # factories[variable._factory.__name__] = variable._factory # noqa: SLF001 + return factories +def _is_valid_factory(include_skipped: bool, var: Any): + return ( + callable(var) + and is_any_device_factory(var) + and (include_skipped or not _is_device_skipped(var)) + ) + + def _is_device_skipped(func: AnyDeviceFactory) -> bool: return getattr(func, "__skip__", False) @@ -276,8 +245,19 @@ def is_v2_device_factory(func: Callable) -> TypeGuard[V2DeviceFactory]: return False +def is_new_device_factory(func: Callable) -> TypeGuard[AnyDeviceFactory]: + try: + return isinstance(func, DeviceInitializationController) + except ValueError: + return False + + def is_any_device_factory(func: Callable) -> TypeGuard[AnyDeviceFactory]: - return is_v1_device_factory(func) or is_v2_device_factory(func) + return ( + is_v1_device_factory(func) + or is_v2_device_factory(func) + or is_new_device_factory(func) + ) def is_v2_device_type(obj: type[Any]) -> bool: diff --git a/tests/beamlines/unit_tests/test_i22.py b/tests/beamlines/unit_tests/test_i22.py new file mode 100644 index 0000000000..26e55e9114 --- /dev/null +++ b/tests/beamlines/unit_tests/test_i22.py @@ -0,0 +1,30 @@ +import pytest +from ophyd_async.epics.adpilatus import PilatusDetector + + +@pytest.mark.parametrize("module_and_devices_for_beamline", ["i22"], indirect=True) +def test_device_creation(RE, module_and_devices_for_beamline): + _, devices = module_and_devices_for_beamline + print(devices) + expected_keys = [ + "saxs", + "synchrotron", + "waxs", + # "i0", # todo it is missing the driver for now + # "it", # todo it is missing the driver for now + "dcm", + "undulator", + "vfm", + "hfm", + "fswitch", + "panda1", + "panda2", + "panda3", + "panda4", + "oav", + "linkam", + ] + for key in expected_keys: + assert key in devices + assert devices[key] is not None + assert isinstance(devices["saxs"], PilatusDetector) diff --git a/tests/beamlines/unit_tests/test_i_min.py b/tests/beamlines/unit_tests/test_i_min.py new file mode 100644 index 0000000000..74de10d8c9 --- /dev/null +++ b/tests/beamlines/unit_tests/test_i_min.py @@ -0,0 +1,11 @@ +import pytest + + +@pytest.mark.parametrize("module_and_devices_for_beamline", ["i-min"], indirect=True) +def test_device_creation(RE, module_and_devices_for_beamline): + _, devices = module_and_devices_for_beamline + print(devices) + f = devices["fswitch"] + p = devices["panda1"] + assert f is not None + assert p is not None diff --git a/tests/common/beamlines/test_device_factory.py b/tests/common/beamlines/test_device_factory.py new file mode 100644 index 0000000000..e6fff5986c --- /dev/null +++ b/tests/common/beamlines/test_device_factory.py @@ -0,0 +1,103 @@ +from bluesky.run_engine import RunEngine +from ophyd_async.epics.motor import Motor +from ophyd_async.plan_stubs import ensure_connected + +from dodal.common.beamlines.beamline_utils import ACTIVE_DEVICES +from dodal.common.beamlines.device_factory import ( + DeviceInitializationConfig, + device_factory, +) + + +def test_terminal_use_case_decorated_motor_not_mock(RE: RunEngine): + @device_factory(mock=True) + def motor(name: str = "motor", prefix: str = "motor:"): + return Motor(name=name, prefix=prefix) + + m = motor(name="foo", mock=True) + + assert m is not None + assert m.name == "foo" + + RE(ensure_connected(m, mock=True)) + + +def test_terminal_use_case_decorated_motor_mock(RE: RunEngine): + @device_factory(mock=True) + def motor(name: str = "motor", prefix: str = "motor:"): + return Motor(name=name, prefix=prefix) + + m = motor() + RE(ensure_connected(m, mock=True)) + + assert m is not None + assert m.name == "motor" + + +def test_decorator_directly_with_name_override(RE: RunEngine): + ACTIVE_DEVICES.clear() + + @device_factory(mock=True, use_factory_name=False) + def m2(): + return Motor(name="foo", prefix="xyz:") + + device = m2() + assert device is not None + assert "foo" in ACTIVE_DEVICES.keys() + + +def test_decorator_directly_without_name_override(RE: RunEngine): + ACTIVE_DEVICES.clear() + + @device_factory(mock=True) + def m2(): + return Motor(name="foo", prefix="xyz:") + + device = m2() + assert device is not None + assert "m2" in ACTIVE_DEVICES.keys() + + +def test_custom_values(): + config = DeviceInitializationConfig( + eager_connect=False, + use_factory_name=False, + timeout=5.0, + mock=True, + skip=True, + ) + assert not config.eager_connect + assert not config.use_factory_name + assert config.timeout == 5.0 + assert config.mock + assert config.skip + + +def test_config_with_lambda_skip(): + config = DeviceInitializationConfig( + eager_connect=False, + use_factory_name=False, + timeout=5.0, + mock=True, + skip=lambda: True, + ) + assert not config.eager_connect + assert not config.use_factory_name + assert config.timeout == 5.0 + assert config.mock + assert config.skip + + +def test_device_caching(RE: RunEngine): + beamline_prefix = "example:" + + @device_factory(mock=True) + def my_motor(): + return Motor(prefix=f"{beamline_prefix}xyz:") + + ACTIVE_DEVICES.clear() + + d = my_motor() + + assert "my_motor" in ACTIVE_DEVICES + assert ACTIVE_DEVICES["my_motor"] is d diff --git a/tests/common/beamlines/test_device_instantiation.py b/tests/common/beamlines/test_device_instantiation.py index 704bad1343..19619e935f 100644 --- a/tests/common/beamlines/test_device_instantiation.py +++ b/tests/common/beamlines/test_device_instantiation.py @@ -1,8 +1,9 @@ from typing import Any +from unittest.mock import patch import pytest -from dodal.beamlines import all_beamline_modules +from dodal.beamlines import ModuleDiscoveryError, get_all_beamline_modules from dodal.common.beamlines import beamline_utils from dodal.utils import BLUESKY_PROTOCOLS, make_all_devices @@ -13,7 +14,7 @@ def follows_bluesky_protocols(obj: Any) -> bool: @pytest.mark.parametrize( "module_and_devices_for_beamline", - set(all_beamline_modules()), + set(get_all_beamline_modules()), indirect=True, ) def test_device_creation(RE, module_and_devices_for_beamline): @@ -28,17 +29,18 @@ def test_device_creation(RE, module_and_devices_for_beamline): f"devices are {beamline_utils.ACTIVE_DEVICES.keys()}" ) assert follows_bluesky_protocols(device) - assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) + if module.BL != "i22": + assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) @pytest.mark.parametrize( "module_and_devices_for_beamline", - set(all_beamline_modules()), + set(get_all_beamline_modules()), indirect=True, ) def test_devices_are_identical(RE, module_and_devices_for_beamline): """ - Ensures that for every beamline all device functions prevent duplicate instantiation. + Ensures that for every beamline all device functions are singletons to prevent duplicate instantiation. """ bl_mod, devices_a = module_and_devices_for_beamline devices_b, _ = make_all_devices( @@ -48,3 +50,33 @@ def test_devices_are_identical(RE, module_and_devices_for_beamline): ) for device_name in devices_a.keys(): assert devices_a[device_name] is devices_b[device_name] + + +def test_get_all_beamline_modules_raises_module_discovery_error_on_spec_failure(): + """ + Tests that get_all_beamline_modules raises ModuleDiscoveryError if module spec cannot be found. + """ + + # Simulate the failure of `importlib.util.find_spec` by returning None + with patch("importlib.util.find_spec", return_value=None): + with pytest.raises( + ModuleDiscoveryError, match="Unable to find module search locations" + ): + list( + get_all_beamline_modules() + ) # Convert the generator to a list to trigger execution + + +def test_get_all_beamline_modules_raises_module_discovery_error_on_exception(): + """ + Tests that get_all_beamline_modules raises ModuleDiscoveryError if an exception is raised in find_spec. + """ + + # Simulate an exception being raised from `importlib.util.find_spec` + with patch("importlib.util.find_spec", side_effect=Exception("Mocked exception")): + with pytest.raises( + ModuleDiscoveryError, match="Error while finding module spec" + ): + list( + get_all_beamline_modules() + ) # Convert the generator to a list to trigger execution