From fd4e268d7fde7ee01435e8720e53e2a223f53a9e Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 15 Sep 2025 13:52:15 +0000 Subject: [PATCH 01/13] Update beamlines to use shared --- src/dodal/beamlines/b07_1.py | 12 +++------- src/dodal/beamlines/b07_shared.py | 18 ++++++++++++++ src/dodal/beamlines/i05.py | 7 ------ src/dodal/beamlines/i05_1.py | 7 ------ .../i05_shared.py | 5 ++-- src/dodal/beamlines/i09.py | 24 ++++++------------- src/dodal/beamlines/i09_1.py | 7 +----- src/dodal/beamlines/i09_1_shared.py | 13 ++++++++++ src/dodal/beamlines/i09_2.py | 7 ------ src/dodal/beamlines/i09_2_shared.py | 14 +++++++++++ 10 files changed, 59 insertions(+), 55 deletions(-) create mode 100644 src/dodal/beamlines/b07_shared.py rename src/dodal/{beamline_specific_utils => beamlines}/i05_shared.py (68%) create mode 100644 src/dodal/beamlines/i09_1_shared.py create mode 100644 src/dodal/beamlines/i09_2_shared.py diff --git a/src/dodal/beamlines/b07_1.py b/src/dodal/beamlines/b07_1.py index 264476b235c..0c3846f58f2 100644 --- a/src/dodal/beamlines/b07_1.py +++ b/src/dodal/beamlines/b07_1.py @@ -1,14 +1,13 @@ +from dodal.beamlines.b07_shared import pgm from dodal.common.beamlines.beamline_utils import device_factory from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.b07 import PsuMode from dodal.devices.b07_1 import ( ChannelCutMonochromator, - Grating, LensMode, ) from dodal.devices.electron_analyser import SelectedSource from dodal.devices.electron_analyser.specs import SpecsAnalyserDriverIO -from dodal.devices.pgm import PGM from dodal.devices.synchrotron import Synchrotron from dodal.log import set_beamline as set_log_beamline from dodal.utils import BeamlinePrefix, get_beamline_name @@ -24,18 +23,13 @@ def synchrotron() -> Synchrotron: return Synchrotron() -@device_factory() -def pgm() -> PGM: - return PGM(prefix=f"{PREFIX.beamline_prefix}-OP-PGM-01:", grating=Grating) - - -# Connect will work again after this work completed -# https://jira.diamond.ac.uk/browse/B07-1104 @device_factory() def ccmc() -> ChannelCutMonochromator: return ChannelCutMonochromator(prefix=f"{PREFIX.beamline_prefix}-OP-CCM-01:") +# Connect will work again after this work completed +# https://jira.diamond.ac.uk/browse/B07-1104 @device_factory() def analyser_driver() -> SpecsAnalyserDriverIO[LensMode, PsuMode]: return SpecsAnalyserDriverIO[LensMode, PsuMode]( diff --git a/src/dodal/beamlines/b07_shared.py b/src/dodal/beamlines/b07_shared.py new file mode 100644 index 00000000000..39190328b48 --- /dev/null +++ b/src/dodal/beamlines/b07_shared.py @@ -0,0 +1,18 @@ +from dodal.common.beamlines.beamline_utils import ( + device_factory, +) +from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline +from dodal.devices.b07 import Grating +from dodal.devices.pgm import PGM +from dodal.log import set_beamline as set_log_beamline +from dodal.utils import BeamlinePrefix, get_beamline_name + +BL = get_beamline_name("b07") +PREFIX = BeamlinePrefix(BL, suffix="B") +set_log_beamline(BL) +set_utils_beamline(BL) + + +@device_factory() +def pgm() -> PGM: + return PGM(prefix=f"{PREFIX.beamline_prefix}-OP-PGM-01:", grating=Grating) diff --git a/src/dodal/beamlines/i05.py b/src/dodal/beamlines/i05.py index 85f6879d212..af611ad186b 100644 --- a/src/dodal/beamlines/i05.py +++ b/src/dodal/beamlines/i05.py @@ -1,7 +1,5 @@ -from dodal.beamline_specific_utils.i05_shared import pgm as i05_pgm from dodal.common.beamlines.beamline_utils import device_factory from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline -from dodal.devices.pgm import PGM from dodal.devices.synchrotron import Synchrotron from dodal.log import set_beamline as set_log_beamline from dodal.utils import BeamlinePrefix, get_beamline_name @@ -15,8 +13,3 @@ @device_factory() def synchrotron() -> Synchrotron: return Synchrotron() - - -@device_factory() -def pgm() -> PGM: - return i05_pgm() diff --git a/src/dodal/beamlines/i05_1.py b/src/dodal/beamlines/i05_1.py index fd9826d96c9..7ad194e59c1 100644 --- a/src/dodal/beamlines/i05_1.py +++ b/src/dodal/beamlines/i05_1.py @@ -1,7 +1,5 @@ -from dodal.beamline_specific_utils.i05_shared import pgm as i05_pgm from dodal.common.beamlines.beamline_utils import device_factory from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline -from dodal.devices.pgm import PGM from dodal.devices.synchrotron import Synchrotron from dodal.log import set_beamline as set_log_beamline from dodal.utils import BeamlinePrefix, get_beamline_name @@ -12,11 +10,6 @@ set_utils_beamline(BL) -@device_factory() -def pgm() -> PGM: - return i05_pgm() - - @device_factory() def synchrotron() -> Synchrotron: return Synchrotron() diff --git a/src/dodal/beamline_specific_utils/i05_shared.py b/src/dodal/beamlines/i05_shared.py similarity index 68% rename from src/dodal/beamline_specific_utils/i05_shared.py rename to src/dodal/beamlines/i05_shared.py index c0b43915f16..96cd9c413b5 100644 --- a/src/dodal/beamline_specific_utils/i05_shared.py +++ b/src/dodal/beamlines/i05_shared.py @@ -1,9 +1,10 @@ from dodal.common.beamlines.beamline_utils import device_factory from dodal.devices.i05.enums import Grating from dodal.devices.pgm import PGM -from dodal.utils import BeamlinePrefix +from dodal.utils import BeamlinePrefix, get_beamline_name -PREFIX = BeamlinePrefix("i05", "I") +BL = get_beamline_name("i05") +PREFIX = BeamlinePrefix(BL, "I") @device_factory() diff --git a/src/dodal/beamlines/i09.py b/src/dodal/beamlines/i09.py index 096b83348fe..add9970dc3f 100644 --- a/src/dodal/beamlines/i09.py +++ b/src/dodal/beamlines/i09.py @@ -1,11 +1,14 @@ +from dodal.beamlines.i09_1_shared import dcm +from dodal.beamlines.i09_2_shared import pgm from dodal.common.beamlines.beamline_utils import ( device_factory, ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.electron_analyser import SelectedSource from dodal.devices.electron_analyser.vgscienta import VGScientaAnalyserDriverIO -from dodal.devices.i09 import DCM, Grating, LensMode, PassEnergy, PsuMode -from dodal.devices.pgm import PGM +from dodal.devices.i09 import LensMode, PassEnergy, PsuMode + +# from dodal.devices.pgm import PGM from dodal.devices.synchrotron import Synchrotron from dodal.log import set_beamline as set_log_beamline from dodal.utils import BeamlinePrefix, get_beamline_name @@ -21,21 +24,8 @@ def synchrotron() -> Synchrotron: return Synchrotron() -@device_factory() -def pgm() -> PGM: - return PGM( - prefix=f"{BeamlinePrefix(BL, suffix='J').beamline_prefix}-MO-PGM-01:", - grating=Grating, - ) - - -@device_factory() -def dcm() -> DCM: - return DCM(prefix=f"{PREFIX.beamline_prefix}-MO-DCM-01:") - - -# Connect will work again after this work completed -# https://jira.diamond.ac.uk/browse/I09-651 +# # Connect will work again after this work completed +# # https://jira.diamond.ac.uk/browse/I09-651 @device_factory() def analyser_driver() -> VGScientaAnalyserDriverIO[LensMode, PsuMode, PassEnergy]: energy_sources = { diff --git a/src/dodal/beamlines/i09_1.py b/src/dodal/beamlines/i09_1.py index 9384968106d..123a408dfb3 100644 --- a/src/dodal/beamlines/i09_1.py +++ b/src/dodal/beamlines/i09_1.py @@ -1,10 +1,10 @@ +from dodal.beamlines.i09_1_shared import dcm from dodal.common.beamlines.beamline_utils import ( device_factory, ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.electron_analyser import SelectedSource from dodal.devices.electron_analyser.specs import SpecsAnalyserDriverIO -from dodal.devices.i09.dcm import DCM from dodal.devices.i09_1 import LensMode, PsuMode from dodal.devices.synchrotron import Synchrotron from dodal.log import set_beamline as set_log_beamline @@ -21,11 +21,6 @@ def synchrotron() -> Synchrotron: return Synchrotron() -@device_factory() -def dcm() -> DCM: - return DCM(prefix=f"{PREFIX.beamline_prefix}-MO-DCM-01:") - - # Connect will work again after this work completed # https://jira.diamond.ac.uk/browse/I09-651 @device_factory() diff --git a/src/dodal/beamlines/i09_1_shared.py b/src/dodal/beamlines/i09_1_shared.py new file mode 100644 index 00000000000..d60f00a3743 --- /dev/null +++ b/src/dodal/beamlines/i09_1_shared.py @@ -0,0 +1,13 @@ +from dodal.common.beamlines.beamline_utils import ( + device_factory, +) +from dodal.devices.i09.dcm import DCM +from dodal.utils import BeamlinePrefix, get_beamline_name + +BL = get_beamline_name("i09-1") +PREFIX = BeamlinePrefix(BL, suffix="I") + + +@device_factory() +def dcm() -> DCM: + return DCM(prefix=f"{PREFIX.beamline_prefix}-MO-DCM-01:") diff --git a/src/dodal/beamlines/i09_2.py b/src/dodal/beamlines/i09_2.py index 8819764e121..d7e04a2cd57 100644 --- a/src/dodal/beamlines/i09_2.py +++ b/src/dodal/beamlines/i09_2.py @@ -2,8 +2,6 @@ device_factory, ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline -from dodal.devices.i09.enums import Grating -from dodal.devices.pgm import PGM from dodal.devices.synchrotron import Synchrotron from dodal.log import set_beamline as set_log_beamline from dodal.utils import BeamlinePrefix, get_beamline_name @@ -17,8 +15,3 @@ @device_factory() def synchrotron() -> Synchrotron: return Synchrotron() - - -@device_factory() -def pgm() -> PGM: - return PGM(prefix=f"{PREFIX.beamline_prefix}-MO-PGM-01:", grating=Grating) diff --git a/src/dodal/beamlines/i09_2_shared.py b/src/dodal/beamlines/i09_2_shared.py new file mode 100644 index 00000000000..b8daabc1fbb --- /dev/null +++ b/src/dodal/beamlines/i09_2_shared.py @@ -0,0 +1,14 @@ +from dodal.common.beamlines.beamline_utils import ( + device_factory, +) +from dodal.devices.i09.enums import Grating +from dodal.devices.pgm import PGM +from dodal.utils import BeamlinePrefix, get_beamline_name + +BL = get_beamline_name("i09") +PREFIX = BeamlinePrefix(BL, suffix="J") + + +@device_factory() +def pgm() -> PGM: + return PGM(prefix=f"{PREFIX.beamline_prefix}-MO-PGM-01:", grating=Grating) From 8106dd49bc6b53b9f7d388087001edbedd72104d Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 15 Sep 2025 13:52:37 +0000 Subject: [PATCH 02/13] Update dodal connect command to connect shared components --- src/dodal/beamlines/__init__.py | 19 +++++++++ src/dodal/cli.py | 70 ++++++++++++++++++++++----------- 2 files changed, 67 insertions(+), 22 deletions(-) diff --git a/src/dodal/beamlines/__init__.py b/src/dodal/beamlines/__init__.py index e5d29c0706f..3ce00a8c199 100644 --- a/src/dodal/beamlines/__init__.py +++ b/src/dodal/beamlines/__init__.py @@ -12,6 +12,7 @@ "i05-1": "i05_1", "b07-1": "b07_1", "i09-1": "i09_1", + "i09-2": "i09_2", "i13-1": "i13_1", "i20-1": "i20_1", "i19-1": "i19_1", @@ -24,6 +25,19 @@ "t01": "adsim", } +# Some beamlines have shared components between branch lines. This configuration is +# used by dodal connect to know which beamlines are shared so that when running dodal +# connect, it will connect your beamline + shared components. +_BEAMLINE_SHARED = { + "i05": ["i05", "i05_shared"], + "i05_1": ["i05_1", "i05_shared"], + "b07": ["b07", "b07_shared"], + "b07_1": ["b07_1", "b07_shared"], + "i09": ["i09", "i09_1_shared", "i09_2_shared"], + "i09_1": ["i09_1", "i09_1_shared"], + "i09_2": ["i09_2", "i09_2_shared"], +} + def all_beamline_modules() -> Iterable[str]: """ @@ -94,3 +108,8 @@ def module_name_for_beamline(beamline: str) -> str: """ return _BEAMLINE_NAME_OVERRIDES.get(beamline, beamline) + + +def shared_beamline_modules(beamline: str) -> list[str]: + bl = module_name_for_beamline(beamline) + return [module_name_for_beamline(b) for b in _BEAMLINE_SHARED.get(bl, [bl])] diff --git a/src/dodal/cli.py b/src/dodal/cli.py index ae55074b5dd..914036d09bb 100644 --- a/src/dodal/cli.py +++ b/src/dodal/cli.py @@ -7,7 +7,11 @@ from ophyd_async.core import NotConnected, StaticPathProvider, UUIDFilenameProvider from ophyd_async.plan_stubs import ensure_connected -from dodal.beamlines import all_beamline_names, module_name_for_beamline +from dodal.beamlines import ( + all_beamline_names, + module_name_for_beamline, + shared_beamline_modules, +) from dodal.common.beamlines.beamline_utils import set_path_provider from dodal.utils import AnyDevice, filter_ophyd_devices, make_all_devices @@ -43,7 +47,15 @@ def main(ctx: click.Context) -> None: "attempt any I/O. Useful as a a dry-run.", default=False, ) -def connect(beamline: str, all: bool, sim_backend: bool) -> None: +@click.option( + "-m", + "--module-only", + is_flag=True, + help="If a beamline depends on a shared beamline module, test devices only within" + "the selected module.", + default=False, +) +def connect(beamline: str, all: bool, sim_backend: bool, module_only: bool) -> None: """Initialises a beamline module, connects to all devices, reports any connection issues.""" @@ -53,32 +65,46 @@ def connect(beamline: str, all: bool, sim_backend: bool) -> None: # it is not used in dodal connect _spoof_path_provider() - module_name = module_name_for_beamline(beamline) - full_module_path = f"dodal.beamlines.{module_name}" - # We need to make a RunEngine to allow ophyd-async devices to connect. # See https://blueskyproject.io/ophyd-async/main/explanations/event-loop-choice.html RE = RunEngine(call_returns_result=True) - print(f"Attempting connection to {beamline} (using {full_module_path})") - - # Force all devices to be lazy (don't connect to PVs on instantiation) and do - # connection as an extra step, because the alternatives is handling the fact - # that only some devices may be lazy. - devices, instance_exceptions = make_all_devices( - full_module_path, - include_skipped=all, - fake_with_ophyd_sim=sim_backend, - wait_for_connection=False, - ) - devices, connect_exceptions = _connect_devices(RE, devices, sim_backend) - - # Inform user of successful connections - _report_successful_devices(devices, sim_backend) + exceptions = {} - # If exceptions have occurred, this will print details of the relevant PVs - exceptions = {**instance_exceptions, **connect_exceptions} + if module_only: + beamline_modules = [module_name_for_beamline(beamline)] + else: + beamline_modules = shared_beamline_modules(beamline) + + for bl_module in beamline_modules: + module_name = module_name_for_beamline(bl_module) + full_module_path = f"dodal.beamlines.{module_name}" + + print("=" * 100) + print(f"Attempting connection to {bl_module} (using {full_module_path})") + + # Force all devices to be lazy (don't connect to PVs on instantiation) and do + # connection as an extra step, because the alternatives is handling the fact + # that only some devices may be lazy. + devices, instance_exceptions = make_all_devices( + full_module_path, + include_skipped=all, + fake_with_ophyd_sim=sim_backend, + wait_for_connection=False, + ) + devices, connect_exceptions = _connect_devices(RE, devices, sim_backend) + + # Inform user of successful connections + _report_successful_devices(devices, sim_backend) + + # If exceptions have occurred, this will print details of the relevant PVs + e = {**instance_exceptions, **connect_exceptions} + exceptions = exceptions | e + + print("Finished all device connections.") if len(exceptions) > 0: + print("=" * 100) + print("Had the following errors:") raise NotConnected(exceptions) From 8c214973bff2268b925e6d074bdc7ab79999720a Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 15 Sep 2025 13:56:15 +0000 Subject: [PATCH 03/13] Remove PGM from b07 as it is in shared --- src/dodal/beamlines/b07.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/dodal/beamlines/b07.py b/src/dodal/beamlines/b07.py index 1aac35d815b..5a230e5af3f 100644 --- a/src/dodal/beamlines/b07.py +++ b/src/dodal/beamlines/b07.py @@ -1,11 +1,11 @@ +from dodal.beamlines.b07_shared import pgm from dodal.common.beamlines.beamline_utils import ( device_factory, ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline -from dodal.devices.b07 import Grating, LensMode, PsuMode +from dodal.devices.b07 import LensMode, PsuMode from dodal.devices.electron_analyser import SelectedSource from dodal.devices.electron_analyser.specs import SpecsAnalyserDriverIO -from dodal.devices.pgm import PGM from dodal.devices.synchrotron import Synchrotron from dodal.log import set_beamline as set_log_beamline from dodal.utils import BeamlinePrefix, get_beamline_name @@ -21,11 +21,6 @@ def synchrotron() -> Synchrotron: return Synchrotron() -@device_factory() -def pgm() -> PGM: - return PGM(prefix=f"{PREFIX.beamline_prefix}-OP-PGM-01:", grating=Grating) - - # Connect will work again after this work completed # https://jira.diamond.ac.uk/browse/B07-1104 @device_factory() From 81760e207c42849e3a3e977c5530e51e5ed5e181 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 15 Sep 2025 15:25:36 +0000 Subject: [PATCH 04/13] Added additional tests --- src/dodal/cli.py | 5 ++--- tests/test_cli.py | 42 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/dodal/cli.py b/src/dodal/cli.py index 914036d09bb..d411cd7fbe5 100644 --- a/src/dodal/cli.py +++ b/src/dodal/cli.py @@ -51,7 +51,7 @@ def main(ctx: click.Context) -> None: "-m", "--module-only", is_flag=True, - help="If a beamline depends on a shared beamline module, test devices only within" + help="If a beamline depends on a shared beamline module, test devices only within " "the selected module.", default=False, ) @@ -77,8 +77,7 @@ def connect(beamline: str, all: bool, sim_backend: bool, module_only: bool) -> N beamline_modules = shared_beamline_modules(beamline) for bl_module in beamline_modules: - module_name = module_name_for_beamline(bl_module) - full_module_path = f"dodal.beamlines.{module_name}" + full_module_path = f"dodal.beamlines.{bl_module}" print("=" * 100) print(f"Attempting connection to {bl_module} (using {full_module_path})") diff --git a/tests/test_cli.py b/tests/test_cli.py index 7d8e10976a8..b3d06a90bbb 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -17,6 +17,7 @@ # Test with an example beamline, device instantiation is already tested # in beamline unit tests EXAMPLE_BEAMLINE = "i22" +EXAMPLE_BEAMLINE_SHARED = EXAMPLE_BEAMLINE + "_shared" @pytest.fixture @@ -125,6 +126,31 @@ def test_cli_connect_in_sim_mode(runner: CliRunner): assert "6 devices connected (sim mode)" in result.stdout +@patch.dict(os.environ, clear=True) +def test_cli_connect_with_shared_beamline(runner: CliRunner): + result = _mock_connect( + EXAMPLE_BEAMLINE, + beamline_modules=[EXAMPLE_BEAMLINE, EXAMPLE_BEAMLINE_SHARED], + runner=runner, + devices=device_results(ophyd_async_happy_devices=6), + ) + assert "using dodal.beamlines." + EXAMPLE_BEAMLINE in result.stdout + assert "using dodal.beamlines." + EXAMPLE_BEAMLINE_SHARED in result.stdout + + +@patch.dict(os.environ, clear=True) +def test_cli_connect_with_shared_beamline_module_only(runner: CliRunner): + result = _mock_connect( + "-m", + EXAMPLE_BEAMLINE, + beamline_modules=[EXAMPLE_BEAMLINE, EXAMPLE_BEAMLINE_SHARED], + runner=runner, + devices=device_results(ophyd_async_happy_devices=6), + ) + assert "using dodal.beamlines." + EXAMPLE_BEAMLINE in result.stdout + assert "using dodal.beamlines." + EXAMPLE_BEAMLINE_SHARED not in result.stdout + + @patch.dict(os.environ, clear=True) @pytest.mark.parametrize( "devices,expected_connections", @@ -294,11 +320,21 @@ def _mock_connect( *args, runner: CliRunner, devices: tuple[dict[str, AnyDevice], dict[str, Exception]] = ({}, {}), + beamline_modules: list[str] | None = None, catch_exceptions: bool = False, ) -> Result: - with patch( - "dodal.cli.make_all_devices", - return_value=devices, + if beamline_modules is None: + beamline_modules = [EXAMPLE_BEAMLINE] + + with ( + patch( + "dodal.cli.shared_beamline_modules", + side_effect=lambda beamline: beamline_modules, + ), + patch( + "dodal.cli.make_all_devices", + return_value=devices, + ), ): result = runner.invoke( main, From 81fa727fbffb0d0e5e3dabe9415df9ebb047101a Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 15 Sep 2025 15:45:42 +0000 Subject: [PATCH 05/13] Update docs --- .../decisions/0006-devices-shared-between-endstations.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/explanations/decisions/0006-devices-shared-between-endstations.md b/docs/explanations/decisions/0006-devices-shared-between-endstations.md index ac74e51b581..d445ca5240f 100644 --- a/docs/explanations/decisions/0006-devices-shared-between-endstations.md +++ b/docs/explanations/decisions/0006-devices-shared-between-endstations.md @@ -8,12 +8,13 @@ Proposed ## Context -Some beamlines have multiple endstations with shared hardware in the optics or experiment hutch, and could potentially be trying to control it at the same time. Any device in the common hutch should only be fully controlled by one endstation at a time - the one that is taking data - but still be readable from the other endstations. +Some beamlines have multiple endstations with shared hardware in the optics or experiment hutch, and could potentially be trying to control it at the same time. Any device in the common hutch should only be fully controlled by one endstation at a time - the one that is taking data - but still be readable from the other endstations. Additionally, it is desirable to reduce device configuration as much as possible. ## Decision -The current solution is to have a separate blueapi instance for the shared hutch in order to be able to control the access to all the devices defined there. -For all hardware in the shared optics hutch, the architecture should follow this structure: +Beamlines specific devices should be defined in `dodal/beamlines/iXX.py` and shared components between end stations should be defined in `dodal/beamlines/iXX_shared.py`. They should either have separate blueapi instances, or if not possible, a single blueapi instance which the base beamline module is imported with any additional beamline modules e.g `iXX_shared.py`. + +For all hardware in the shared optics hutch where access control is required, the architecture should follow this structure: - There is a base device in dodal that sends a REST call to the shared blueapi with plan and devices names, as well as the name of the endstation performing the call. - There are read-only versions of the shared devices in the endstation blueapi which inherit from the base device above and set up the request parameters. From c9208a6d6247bb487a253d44bc71b491b20355cc Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 15 Sep 2025 15:53:17 +0000 Subject: [PATCH 06/13] Added info on dodal connect with shared endstations --- .../0006-devices-shared-between-endstations.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/explanations/decisions/0006-devices-shared-between-endstations.md b/docs/explanations/decisions/0006-devices-shared-between-endstations.md index d445ca5240f..3c10b4683b5 100644 --- a/docs/explanations/decisions/0006-devices-shared-between-endstations.md +++ b/docs/explanations/decisions/0006-devices-shared-between-endstations.md @@ -22,7 +22,20 @@ For all hardware in the shared optics hutch where access control is required, th - The shared blueapi instance also has an ``AccessControl`` device that reads the endstation in use for beamtime from a PV. - Every plan should then be wrapped in a decorator that reads the ``AccessControl`` device, check which endstation is making the request and only allows the plan to run if the two values match. - :::{seealso} [Optics hutch implementation on I19](https://diamondlightsource.github.io/i19-bluesky/main/explanations/decisions/0004-optics-blueapi-architecture.html) for an example. ::: + +## Dodal connect with shared endstations + +If you have beamline that is composed of multiple modules, dodal connect can be extended to include all shared components. This is done by extending the `_BEAMLINE_SHARED` configuration (found in `dodal.beamlines.__init__.py`) to join together beamline modules when running the dodal connect command. + +```Python +_BEAMLINE_SHARED = { + "i05": ["i05", "i05_shared"], + "i05_1": ["i05_1", "i05_shared"], + ... +} +``` + +For example, when running `dodal connect i05` will do the device connections for `dodal.beamlines.i05` and `dodal.beamlines.i05_shared`. From e808493bb2b54bd8a129ad08d884d82b0d642a49 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 16 Sep 2025 08:26:41 +0000 Subject: [PATCH 07/13] Added test for shared_beamline_modules --- tests/beamlines/test_mapping.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/beamlines/test_mapping.py b/tests/beamlines/test_mapping.py index 756e765752d..271a77470cc 100644 --- a/tests/beamlines/test_mapping.py +++ b/tests/beamlines/test_mapping.py @@ -3,6 +3,7 @@ from dodal.beamlines import ( all_beamline_names, module_name_for_beamline, + shared_beamline_modules, ) @@ -18,6 +19,18 @@ def test_beamline_name_mapping(beamline: str, expected_module: str): assert module_name_for_beamline(beamline) == expected_module +@pytest.mark.parametrize( + "beamline, expected_modules", + { + "i09": ["i09", "i09_1_shared", "i09_2_shared"], + "i09_1": ["i09_1", "i09_1_shared"], + "i22": ["i22"], + }.items(), +) +def test_beamline_shared_beamline_moudles(beamline: str, expected_modules: list[str]): + assert shared_beamline_modules(beamline) == expected_modules + + def test_all_beamline_names_includes_non_overridden_modules(): beamlines = set(all_beamline_names()) assert "i22" in beamlines From eb2889834b0081ad2f1f38ee80b5559c55c5d440 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 18 Sep 2025 10:02:54 +0000 Subject: [PATCH 08/13] Updated make_all_devices to accept a list instead --- src/dodal/cli.py | 44 ++++++++++++++++++++++---------------------- src/dodal/utils.py | 20 ++++++++++++++++---- tests/test_cli.py | 42 +++--------------------------------------- 3 files changed, 41 insertions(+), 65 deletions(-) diff --git a/src/dodal/cli.py b/src/dodal/cli.py index d411cd7fbe5..8f910912104 100644 --- a/src/dodal/cli.py +++ b/src/dodal/cli.py @@ -76,29 +76,29 @@ def connect(beamline: str, all: bool, sim_backend: bool, module_only: bool) -> N else: beamline_modules = shared_beamline_modules(beamline) - for bl_module in beamline_modules: - full_module_path = f"dodal.beamlines.{bl_module}" + full_module_paths = [ + f"dodal.beamlines.{bl_module}" for bl_module in beamline_modules + ] + print("=" * 100) + print(f"Attempting connection to {beamline_modules})") + + # Force all devices to be lazy (don't connect to PVs on instantiation) and do + # connection as an extra step, because the alternatives is handling the fact + # that only some devices may be lazy. + devices, instance_exceptions = make_all_devices( + full_module_paths, + include_skipped=all, + fake_with_ophyd_sim=sim_backend, + wait_for_connection=False, + ) + devices, connect_exceptions = _connect_devices(RE, devices, sim_backend) - print("=" * 100) - print(f"Attempting connection to {bl_module} (using {full_module_path})") - - # Force all devices to be lazy (don't connect to PVs on instantiation) and do - # connection as an extra step, because the alternatives is handling the fact - # that only some devices may be lazy. - devices, instance_exceptions = make_all_devices( - full_module_path, - include_skipped=all, - fake_with_ophyd_sim=sim_backend, - wait_for_connection=False, - ) - devices, connect_exceptions = _connect_devices(RE, devices, sim_backend) - - # Inform user of successful connections - _report_successful_devices(devices, sim_backend) - - # If exceptions have occurred, this will print details of the relevant PVs - e = {**instance_exceptions, **connect_exceptions} - exceptions = exceptions | e + # Inform user of successful connections + _report_successful_devices(devices, sim_backend) + + # If exceptions have occurred, this will print details of the relevant PVs + e = {**instance_exceptions, **connect_exceptions} + exceptions = exceptions | e print("Finished all device connections.") if len(exceptions) > 0: diff --git a/src/dodal/utils.py b/src/dodal/utils.py index 9be4675e23e..73295a2321c 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -238,7 +238,9 @@ def make_device( def make_all_devices( - module: str | ModuleType | None = None, include_skipped: bool = False, **kwargs + modules: list[str] | list[ModuleType] | str | ModuleType | None = None, + include_skipped: bool = False, + **kwargs, ) -> tuple[dict[str, AnyDevice], dict[str, Exception]]: """Makes all devices in the given beamline module. @@ -255,9 +257,19 @@ def make_all_devices( A dictionary where the keys are device names and the values are devices. A dictionary where the keys are device names and the values are exceptions. """ - if isinstance(module, str) or module is None: - module = import_module(module or __name__) - factories = collect_factories(module, include_skipped) + if modules is None: + mods = [__name__] + elif not isinstance(modules, list): + mods = [modules] + else: + mods = modules + + factories = {} + for m in mods: + if isinstance(m, str) or m is None: + m = import_module(m) + factories = factories | collect_factories(m, include_skipped) + devices: tuple[dict[str, AnyDevice], dict[str, Exception]] = invoke_factories( factories, **kwargs ) diff --git a/tests/test_cli.py b/tests/test_cli.py index b3d06a90bbb..7d8e10976a8 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -17,7 +17,6 @@ # Test with an example beamline, device instantiation is already tested # in beamline unit tests EXAMPLE_BEAMLINE = "i22" -EXAMPLE_BEAMLINE_SHARED = EXAMPLE_BEAMLINE + "_shared" @pytest.fixture @@ -126,31 +125,6 @@ def test_cli_connect_in_sim_mode(runner: CliRunner): assert "6 devices connected (sim mode)" in result.stdout -@patch.dict(os.environ, clear=True) -def test_cli_connect_with_shared_beamline(runner: CliRunner): - result = _mock_connect( - EXAMPLE_BEAMLINE, - beamline_modules=[EXAMPLE_BEAMLINE, EXAMPLE_BEAMLINE_SHARED], - runner=runner, - devices=device_results(ophyd_async_happy_devices=6), - ) - assert "using dodal.beamlines." + EXAMPLE_BEAMLINE in result.stdout - assert "using dodal.beamlines." + EXAMPLE_BEAMLINE_SHARED in result.stdout - - -@patch.dict(os.environ, clear=True) -def test_cli_connect_with_shared_beamline_module_only(runner: CliRunner): - result = _mock_connect( - "-m", - EXAMPLE_BEAMLINE, - beamline_modules=[EXAMPLE_BEAMLINE, EXAMPLE_BEAMLINE_SHARED], - runner=runner, - devices=device_results(ophyd_async_happy_devices=6), - ) - assert "using dodal.beamlines." + EXAMPLE_BEAMLINE in result.stdout - assert "using dodal.beamlines." + EXAMPLE_BEAMLINE_SHARED not in result.stdout - - @patch.dict(os.environ, clear=True) @pytest.mark.parametrize( "devices,expected_connections", @@ -320,21 +294,11 @@ def _mock_connect( *args, runner: CliRunner, devices: tuple[dict[str, AnyDevice], dict[str, Exception]] = ({}, {}), - beamline_modules: list[str] | None = None, catch_exceptions: bool = False, ) -> Result: - if beamline_modules is None: - beamline_modules = [EXAMPLE_BEAMLINE] - - with ( - patch( - "dodal.cli.shared_beamline_modules", - side_effect=lambda beamline: beamline_modules, - ), - patch( - "dodal.cli.make_all_devices", - return_value=devices, - ), + with patch( + "dodal.cli.make_all_devices", + return_value=devices, ): result = runner.invoke( main, From 8849c8afd673779467725efe0e990e4674bb0b50 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 18 Sep 2025 11:30:47 +0000 Subject: [PATCH 09/13] Moved fake_beamlines to folder and add new tests --- src/dodal/utils.py | 2 +- .../all_devices_raise_exception.py} | 0 .../beamline_1.py} | 9 +-- tests/fake_beamline/beamline_2.py | 17 +++++ .../broken_dependency.py} | 9 +-- .../dependencies.py} | 9 +-- .../disordered_dependencies.py} | 9 +-- .../factory_beamline.py} | 0 .../misbehaving_builtins.py} | 0 .../some_devices_working.py} | 9 +-- tests/fake_beamline/util.py | 7 ++ tests/test_cli.py | 25 +++++++ tests/test_utils.py | 72 ++++++++++++++----- 13 files changed, 109 insertions(+), 59 deletions(-) rename tests/{fake_beamline_all_devices_raise_exception.py => fake_beamline/all_devices_raise_exception.py} (100%) rename tests/{fake_beamline.py => fake_beamline/beamline_1.py} (82%) create mode 100644 tests/fake_beamline/beamline_2.py rename tests/{fake_beamline_broken_dependency.py => fake_beamline/broken_dependency.py} (71%) rename tests/{fake_beamline_dependencies.py => fake_beamline/dependencies.py} (71%) rename tests/{fake_beamline_disordered_dependencies.py => fake_beamline/disordered_dependencies.py} (71%) rename tests/{fake_device_factory_beamline.py => fake_beamline/factory_beamline.py} (100%) rename tests/{fake_beamline_misbehaving_builtins.py => fake_beamline/misbehaving_builtins.py} (100%) rename tests/{fake_beamline_some_devices_working.py => fake_beamline/some_devices_working.py} (68%) create mode 100644 tests/fake_beamline/util.py diff --git a/src/dodal/utils.py b/src/dodal/utils.py index 73295a2321c..a2af09673a4 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -266,7 +266,7 @@ def make_all_devices( factories = {} for m in mods: - if isinstance(m, str) or m is None: + if isinstance(m, str): m = import_module(m) factories = factories | collect_factories(m, include_skipped) diff --git a/tests/fake_beamline_all_devices_raise_exception.py b/tests/fake_beamline/all_devices_raise_exception.py similarity index 100% rename from tests/fake_beamline_all_devices_raise_exception.py rename to tests/fake_beamline/all_devices_raise_exception.py diff --git a/tests/fake_beamline.py b/tests/fake_beamline/beamline_1.py similarity index 82% rename from tests/fake_beamline.py rename to tests/fake_beamline/beamline_1.py index 1f49108ae08..4636dd49733 100644 --- a/tests/fake_beamline.py +++ b/tests/fake_beamline/beamline_1.py @@ -1,7 +1,6 @@ -from unittest.mock import MagicMock - from bluesky.protocols import Readable from ophyd_async.epics.motor import Motor +from tests.fake_beamline.util import _mock_with_name from dodal.devices.cryostream import CryoStream from dodal.devices.diamond_filter import DiamondFilter, I03Filters @@ -30,9 +29,3 @@ def plain_ophyd_v2_device() -> OphydV2Device: def not_device() -> int: return 5 - - -def _mock_with_name(name: str) -> MagicMock: - mock = MagicMock() - mock.name = name - return mock diff --git a/tests/fake_beamline/beamline_2.py b/tests/fake_beamline/beamline_2.py new file mode 100644 index 00000000000..7b4867df10c --- /dev/null +++ b/tests/fake_beamline/beamline_2.py @@ -0,0 +1,17 @@ +from bluesky.protocols import Readable +from ophyd_async.epics.motor import Motor +from tests.fake_beamline.util import _mock_with_name + +from dodal.devices.cryostream import CryoStream + + +def device_e() -> Readable: + return _mock_with_name("device_e") + + +def device_f() -> Motor: + return _mock_with_name("device_f") + + +def device_g() -> CryoStream: + return _mock_with_name("device_g") diff --git a/tests/fake_beamline_broken_dependency.py b/tests/fake_beamline/broken_dependency.py similarity index 71% rename from tests/fake_beamline_broken_dependency.py rename to tests/fake_beamline/broken_dependency.py index 9e04106a865..c48d78e429e 100644 --- a/tests/fake_beamline_broken_dependency.py +++ b/tests/fake_beamline/broken_dependency.py @@ -1,7 +1,6 @@ -from unittest.mock import MagicMock - from bluesky.protocols import Readable from ophyd_async.epics.motor import Motor +from tests.fake_beamline.util import _mock_with_name from dodal.devices.cryostream import CryoStream @@ -16,9 +15,3 @@ def device_y() -> Motor: def device_z(device_x: Readable, device_y: Motor) -> CryoStream: return _mock_with_name("cryo") - - -def _mock_with_name(name: str) -> MagicMock: - mock = MagicMock() - mock.name = name - return mock diff --git a/tests/fake_beamline_dependencies.py b/tests/fake_beamline/dependencies.py similarity index 71% rename from tests/fake_beamline_dependencies.py rename to tests/fake_beamline/dependencies.py index 6874878801e..c23180e43a8 100644 --- a/tests/fake_beamline_dependencies.py +++ b/tests/fake_beamline/dependencies.py @@ -1,7 +1,6 @@ -from unittest.mock import MagicMock - from bluesky.protocols import Readable from ophyd_async.epics.motor import Motor +from tests.fake_beamline.util import _mock_with_name from dodal.devices.cryostream import CryoStream @@ -16,9 +15,3 @@ def device_y() -> Motor: def device_z(device_x: Readable, device_y: Motor) -> CryoStream: return _mock_with_name("cryo") - - -def _mock_with_name(name: str) -> MagicMock: - mock = MagicMock() - mock.name = name - return mock diff --git a/tests/fake_beamline_disordered_dependencies.py b/tests/fake_beamline/disordered_dependencies.py similarity index 71% rename from tests/fake_beamline_disordered_dependencies.py rename to tests/fake_beamline/disordered_dependencies.py index b38cf89db7e..c0bcb8c18b2 100644 --- a/tests/fake_beamline_disordered_dependencies.py +++ b/tests/fake_beamline/disordered_dependencies.py @@ -1,7 +1,6 @@ -from unittest.mock import MagicMock - from bluesky.protocols import Readable from ophyd_async.epics.motor import Motor +from tests.fake_beamline.util import _mock_with_name from dodal.devices.cryostream import CryoStream @@ -16,9 +15,3 @@ def device_x() -> Readable: def device_y() -> Motor: return _mock_with_name("motor") - - -def _mock_with_name(name: str) -> MagicMock: - mock = MagicMock() - mock.name = name - return mock diff --git a/tests/fake_device_factory_beamline.py b/tests/fake_beamline/factory_beamline.py similarity index 100% rename from tests/fake_device_factory_beamline.py rename to tests/fake_beamline/factory_beamline.py diff --git a/tests/fake_beamline_misbehaving_builtins.py b/tests/fake_beamline/misbehaving_builtins.py similarity index 100% rename from tests/fake_beamline_misbehaving_builtins.py rename to tests/fake_beamline/misbehaving_builtins.py diff --git a/tests/fake_beamline_some_devices_working.py b/tests/fake_beamline/some_devices_working.py similarity index 68% rename from tests/fake_beamline_some_devices_working.py rename to tests/fake_beamline/some_devices_working.py index bd0d1f14087..216476a2caf 100644 --- a/tests/fake_beamline_some_devices_working.py +++ b/tests/fake_beamline/some_devices_working.py @@ -1,7 +1,6 @@ -from unittest.mock import MagicMock - from bluesky.protocols import Readable from ophyd_async.epics.motor import Motor +from tests.fake_beamline.util import _mock_with_name from dodal.devices.undulator import Undulator @@ -16,9 +15,3 @@ def device_b() -> Motor: def device_c() -> Undulator: return _mock_with_name("undulator") - - -def _mock_with_name(name: str) -> MagicMock: - mock = MagicMock() - mock.name = name - return mock diff --git a/tests/fake_beamline/util.py b/tests/fake_beamline/util.py new file mode 100644 index 00000000000..ab66de8583b --- /dev/null +++ b/tests/fake_beamline/util.py @@ -0,0 +1,7 @@ +from unittest.mock import MagicMock + + +def _mock_with_name(name: str) -> MagicMock: + mock = MagicMock() + mock.name = name + return mock diff --git a/tests/test_cli.py b/tests/test_cli.py index 7d8e10976a8..c624b1e5378 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -17,6 +17,7 @@ # Test with an example beamline, device instantiation is already tested # in beamline unit tests EXAMPLE_BEAMLINE = "i22" +EXAMPLE_BEAMLINE_SHARED = EXAMPLE_BEAMLINE + "_shared" @pytest.fixture @@ -125,6 +126,30 @@ def test_cli_connect_in_sim_mode(runner: CliRunner): assert "6 devices connected (sim mode)" in result.stdout +@patch.dict(os.environ, clear=True) +def test_cli_connect_with_shared_beamline_module_only(runner: CliRunner): + result = _mock_connect( + "-m", + EXAMPLE_BEAMLINE, + runner=runner, + devices=device_results(ophyd_async_happy_devices=6), + ) + + assert "using dodal.beamlines." + EXAMPLE_BEAMLINE in result.stdout + + +@patch.dict(os.environ, clear=True) +def test_cli_connect_with_shared_beamline_module_only(runner: CliRunner): + result = _mock_connect( + "-m", + EXAMPLE_BEAMLINE, + runner=runner, + devices=device_results(ophyd_async_happy_devices=6), + ) + + assert "using dodal.beamlines." + EXAMPLE_BEAMLINE in result.stdout + + @patch.dict(os.environ, clear=True) @pytest.mark.parametrize( "devices,expected_connections", diff --git a/tests/test_utils.py b/tests/test_utils.py index 1f0a40681eb..e9465429681 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -40,7 +40,7 @@ def alternate_config(tmp_path) -> str: @pytest.fixture() def fake_device_factory_beamline(): - import tests.fake_device_factory_beamline as beamline + import tests.fake_beamline.factory_beamline as beamline factories = [ f @@ -53,11 +53,11 @@ def fake_device_factory_beamline(): def test_finds_device_factories() -> None: - import tests.fake_beamline as fake_beamline + import tests.fake_beamline.beamline_1 as beamline_1 - factories = collect_factories(fake_beamline) + factories = collect_factories(beamline_1) - from tests.fake_beamline import ( + from tests.fake_beamline.beamline_1 import ( device_a, device_b, device_c, @@ -75,34 +75,70 @@ def test_finds_device_factories() -> None: def test_makes_devices() -> None: - import tests.fake_beamline as fake_beamline + import tests.fake_beamline.beamline_1 as beamline_1 - devices, exceptions = make_all_devices(fake_beamline) + devices, exceptions = make_all_devices(beamline_1) + assert { + "readable", + "motor", + "cryo", + "diamond_filter", + "ophyd_v2_device", + } == devices.keys() and len(exceptions) == 0 + + +def test_makes_devices_from_multiple_beamlines_as_str() -> None: + devices, exceptions = make_all_devices( + [ + "tests.fake_beamline.beamline_1", + "tests.fake_beamline.beamline_2", + ] + ) + assert { + "readable", + "motor", + "cryo", + "diamond_filter", + "ophyd_v2_device", + "device_e", + "device_f", + "device_g", + } == devices.keys() and len(exceptions) == 0 + + +def test_makes_devices_from_multiple_beamlines_as_modules() -> None: + import tests.fake_beamline.beamline_1 as beamline_1 + import tests.fake_beamline.beamline_2 as beamline_2 + + devices, exceptions = make_all_devices([beamline_1, beamline_2]) assert { "readable", "motor", "cryo", "diamond_filter", "ophyd_v2_device", + "device_e", + "device_f", + "device_g", } == devices.keys() and len(exceptions) == 0 def test_makes_devices_with_dependencies() -> None: - import tests.fake_beamline_dependencies as fake_beamline + import tests.fake_beamline.dependencies as fake_beamline devices, exceptions = make_all_devices(fake_beamline) assert {"readable", "motor", "cryo"} == devices.keys() and len(exceptions) == 0 def test_makes_devices_with_disordered_dependencies() -> None: - import tests.fake_beamline_disordered_dependencies as fake_beamline + import tests.fake_beamline.disordered_dependencies as fake_beamline devices, exceptions = make_all_devices(fake_beamline) assert {"readable", "motor", "cryo"} == devices.keys() and len(exceptions) == 0 def test_makes_devices_with_module_name() -> None: - devices, exceptions = make_all_devices("tests.fake_beamline") + devices, exceptions = make_all_devices("tests.fake_beamline.beamline_1") assert { "readable", "motor", @@ -119,7 +155,7 @@ def test_get_hostname() -> None: def test_no_signature_builtins_not_devices() -> None: - import tests.fake_beamline_misbehaving_builtins as fake_beamline + import tests.fake_beamline.misbehaving_builtins as fake_beamline devices, exceptions = make_all_devices(fake_beamline) assert len(devices) == 0 @@ -127,7 +163,7 @@ def test_no_signature_builtins_not_devices() -> None: def test_no_devices_when_all_factories_raise_exceptions() -> None: - import tests.fake_beamline_all_devices_raise_exception as fake_beamline + import tests.fake_beamline.all_devices_raise_exception as fake_beamline devices, exceptions = make_all_devices(fake_beamline) assert len(devices) == 0 @@ -137,7 +173,7 @@ def test_no_devices_when_all_factories_raise_exceptions() -> None: def test_some_devices_when_some_factories_raise_exceptions() -> None: - import tests.fake_beamline_some_devices_working as fake_beamline + import tests.fake_beamline.some_devices_working as fake_beamline devices, exceptions = make_all_devices(fake_beamline) assert len(devices) == 2 @@ -147,40 +183,40 @@ def test_some_devices_when_some_factories_raise_exceptions() -> None: def test_make_device_with_dependency(): - import tests.fake_beamline_dependencies as fake_beamline + import tests.fake_beamline.dependencies as fake_beamline devices = make_device(fake_beamline, "device_z") assert devices.keys() == {"device_x", "device_y", "device_z"} def test_make_device_no_dependency(): - import tests.fake_beamline_dependencies as fake_beamline + import tests.fake_beamline.dependencies as fake_beamline devices = make_device(fake_beamline, "device_x") assert devices.keys() == {"device_x"} def test_make_device_with_exception(): - import tests.fake_beamline_all_devices_raise_exception as fake_beamline + import tests.fake_beamline.all_devices_raise_exception as fake_beamline with pytest.raises(ValueError): make_device(fake_beamline, "device_c") def test_make_device_with_module_name(): - devices = make_device("tests.fake_beamline", "device_a") + devices = make_device("tests.fake_beamline.beamline_1", "device_a") assert {"device_a"} == devices.keys() def test_make_device_no_factory(): - import tests.fake_beamline_dependencies as fake_beamline + import tests.fake_beamline.dependencies as fake_beamline with pytest.raises(ValueError): make_device(fake_beamline, "this_device_does_not_exist") def test_make_device_dependency_throws(): - import tests.fake_beamline_broken_dependency as fake_beamline + import tests.fake_beamline.broken_dependency as fake_beamline with pytest.raises(RuntimeError): make_device(fake_beamline, "device_z") From 5e493ad6b7a22cf1d135638906445d4fbcf85743 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 18 Sep 2025 11:36:45 +0000 Subject: [PATCH 10/13] Removed failing test --- tests/test_cli.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index c624b1e5378..9c64f4419ec 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -126,16 +126,16 @@ def test_cli_connect_in_sim_mode(runner: CliRunner): assert "6 devices connected (sim mode)" in result.stdout -@patch.dict(os.environ, clear=True) -def test_cli_connect_with_shared_beamline_module_only(runner: CliRunner): - result = _mock_connect( - "-m", - EXAMPLE_BEAMLINE, - runner=runner, - devices=device_results(ophyd_async_happy_devices=6), - ) - - assert "using dodal.beamlines." + EXAMPLE_BEAMLINE in result.stdout +# @patch.dict(os.environ, clear=True) +# def test_cli_connect_with_shared_beamline_module_only(runner: CliRunner): +# result = _mock_connect( +# "-m", +# EXAMPLE_BEAMLINE, +# runner=runner, +# devices=device_results(ophyd_async_happy_devices=6), +# ) + +# assert "using dodal.beamlines." + EXAMPLE_BEAMLINE in result.stdout @patch.dict(os.environ, clear=True) From 588c28bced1b55986256f837a981201e7e0b4cd4 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 18 Sep 2025 12:00:26 +0000 Subject: [PATCH 11/13] Fixed tests --- src/dodal/cli.py | 4 ++-- tests/test_cli.py | 48 +++++++++++++++++++++++++++-------------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/dodal/cli.py b/src/dodal/cli.py index 8f910912104..15a935780e8 100644 --- a/src/dodal/cli.py +++ b/src/dodal/cli.py @@ -79,8 +79,8 @@ def connect(beamline: str, all: bool, sim_backend: bool, module_only: bool) -> N full_module_paths = [ f"dodal.beamlines.{bl_module}" for bl_module in beamline_modules ] - print("=" * 100) - print(f"Attempting connection to {beamline_modules})") + print(f"Attempting connection to {beamline} (using {full_module_paths})") + print(shared_beamline_modules) # Force all devices to be lazy (don't connect to PVs on instantiation) and do # connection as an extra step, because the alternatives is handling the fact diff --git a/tests/test_cli.py b/tests/test_cli.py index 9c64f4419ec..a52a19f4317 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -17,7 +17,7 @@ # Test with an example beamline, device instantiation is already tested # in beamline unit tests EXAMPLE_BEAMLINE = "i22" -EXAMPLE_BEAMLINE_SHARED = EXAMPLE_BEAMLINE + "_shared" +EXAMPLE_SHARED_BEAMLINE = [EXAMPLE_BEAMLINE, EXAMPLE_BEAMLINE + "_shared"] @pytest.fixture @@ -126,28 +126,36 @@ def test_cli_connect_in_sim_mode(runner: CliRunner): assert "6 devices connected (sim mode)" in result.stdout -# @patch.dict(os.environ, clear=True) -# def test_cli_connect_with_shared_beamline_module_only(runner: CliRunner): -# result = _mock_connect( -# "-m", -# EXAMPLE_BEAMLINE, -# runner=runner, -# devices=device_results(ophyd_async_happy_devices=6), -# ) - -# assert "using dodal.beamlines." + EXAMPLE_BEAMLINE in result.stdout +@patch.dict(os.environ, clear=True) +def test_cli_connect_with_shared_beamline(runner: CliRunner): + with patch( + "dodal.cli.shared_beamline_modules", + side_effect=lambda _: EXAMPLE_SHARED_BEAMLINE, + ): + result = _mock_connect( + EXAMPLE_BEAMLINE, + runner=runner, + devices=device_results(ophyd_async_happy_devices=6), + ) + assert ( + f"using ['dodal.beamlines.{EXAMPLE_SHARED_BEAMLINE[0]}', 'dodal.beamlines.{EXAMPLE_SHARED_BEAMLINE[1]}']" + in result.stdout + ) @patch.dict(os.environ, clear=True) -def test_cli_connect_with_shared_beamline_module_only(runner: CliRunner): - result = _mock_connect( - "-m", - EXAMPLE_BEAMLINE, - runner=runner, - devices=device_results(ophyd_async_happy_devices=6), - ) - - assert "using dodal.beamlines." + EXAMPLE_BEAMLINE in result.stdout +def test_cli_connect_with_shared_beamline_module_only_argument(runner: CliRunner): + with patch( + "dodal.cli.shared_beamline_modules", + side_effect=lambda _: EXAMPLE_SHARED_BEAMLINE, + ): + result = _mock_connect( + "-m", + EXAMPLE_BEAMLINE, + runner=runner, + devices=device_results(ophyd_async_happy_devices=6), + ) + assert f"using ['dodal.beamlines.{EXAMPLE_BEAMLINE}']" in result.stdout @patch.dict(os.environ, clear=True) From 02b853f5828bdb0e018e9b1103c782af7d7784ae Mon Sep 17 00:00:00 2001 From: Fajin Yuan Date: Thu, 18 Sep 2025 14:45:51 +0100 Subject: [PATCH 12/13] add PGM support for i06 #116 (#1526) * add PGM support for i06 #116 * add PGM support using i06_shared for i06 and i06-1 * add PGM support for i06 #116 * add PGM support using i06_shared for i06 and i06-1 * configure _BEAMLINE_SHARED to include i06_shared for i06 and i06-1 * fix spell mistake * Update src/dodal/beamlines/i06_shared.py remove name argument as it is the same as default value Co-authored-by: oliwenmandiamond <136330507+oliwenmandiamond@users.noreply.github.com> --------- Co-authored-by: oliwenmandiamond <136330507+oliwenmandiamond@users.noreply.github.com> --- src/dodal/beamlines/__init__.py | 3 +++ src/dodal/beamlines/i06.py | 17 +++++++++++++++++ src/dodal/beamlines/i06_1.py | 15 +++++++++++++++ src/dodal/beamlines/i06_shared.py | 18 ++++++++++++++++++ src/dodal/devices/i06_shared/__init__.py | 5 +++++ src/dodal/devices/i06_shared/enums.py | 7 +++++++ 6 files changed, 65 insertions(+) create mode 100644 src/dodal/beamlines/i06.py create mode 100644 src/dodal/beamlines/i06_1.py create mode 100644 src/dodal/beamlines/i06_shared.py create mode 100644 src/dodal/devices/i06_shared/__init__.py create mode 100644 src/dodal/devices/i06_shared/enums.py diff --git a/src/dodal/beamlines/__init__.py b/src/dodal/beamlines/__init__.py index 3ce00a8c199..b5e5abb38bc 100644 --- a/src/dodal/beamlines/__init__.py +++ b/src/dodal/beamlines/__init__.py @@ -10,6 +10,7 @@ # dictionary, which maps ${BEAMLINE} to dodal.beamlines. _BEAMLINE_NAME_OVERRIDES = { "i05-1": "i05_1", + "i06-1": "i06_1", "b07-1": "b07_1", "i09-1": "i09_1", "i09-2": "i09_2", @@ -31,6 +32,8 @@ _BEAMLINE_SHARED = { "i05": ["i05", "i05_shared"], "i05_1": ["i05_1", "i05_shared"], + "i06": ["i06", "i06_shared"], + "i06_1": ["i06_1", "i06_shared"], "b07": ["b07", "b07_shared"], "b07_1": ["b07_1", "b07_shared"], "i09": ["i09", "i09_1_shared", "i09_2_shared"], diff --git a/src/dodal/beamlines/i06.py b/src/dodal/beamlines/i06.py new file mode 100644 index 00000000000..f08a74fca10 --- /dev/null +++ b/src/dodal/beamlines/i06.py @@ -0,0 +1,17 @@ +from dodal.common.beamlines.beamline_utils import ( + device_factory, +) +from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline +from dodal.devices.synchrotron import Synchrotron +from dodal.log import set_beamline as set_log_beamline +from dodal.utils import BeamlinePrefix, get_beamline_name + +BL = get_beamline_name("i06") +PREFIX = BeamlinePrefix(BL, suffix="I") +set_log_beamline(BL) +set_utils_beamline(BL) + + +@device_factory() +def synchrotron() -> Synchrotron: + return Synchrotron() diff --git a/src/dodal/beamlines/i06_1.py b/src/dodal/beamlines/i06_1.py new file mode 100644 index 00000000000..2eaa6810f55 --- /dev/null +++ b/src/dodal/beamlines/i06_1.py @@ -0,0 +1,15 @@ +from dodal.common.beamlines.beamline_utils import device_factory +from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline +from dodal.devices.synchrotron import Synchrotron +from dodal.log import set_beamline as set_log_beamline +from dodal.utils import BeamlinePrefix, get_beamline_name + +BL = get_beamline_name("i06-1") +PREFIX = BeamlinePrefix(BL, suffix="J") +set_log_beamline(BL) +set_utils_beamline(BL) + + +@device_factory() +def synchrotron() -> Synchrotron: + return Synchrotron() diff --git a/src/dodal/beamlines/i06_shared.py b/src/dodal/beamlines/i06_shared.py new file mode 100644 index 00000000000..6efadbd6849 --- /dev/null +++ b/src/dodal/beamlines/i06_shared.py @@ -0,0 +1,18 @@ +from dodal.common.beamlines.beamline_utils import ( + device_factory, +) +from dodal.devices.i06_shared import Grating +from dodal.devices.pgm import PGM +from dodal.utils import BeamlinePrefix, get_beamline_name + +BL = get_beamline_name("i06") +PREFIX = BeamlinePrefix(BL, suffix="I") + + +@device_factory() +def pgm() -> PGM: + return PGM( + prefix=f"{PREFIX.beamline_prefix}-OP-PGM-01:", + grating=Grating, + gratingPv="NLINES2", + ) diff --git a/src/dodal/devices/i06_shared/__init__.py b/src/dodal/devices/i06_shared/__init__.py new file mode 100644 index 00000000000..5e70b3928bc --- /dev/null +++ b/src/dodal/devices/i06_shared/__init__.py @@ -0,0 +1,5 @@ +from dodal.devices.i06_shared.enums import Grating + +__all__ = [ + "Grating", +] diff --git a/src/dodal/devices/i06_shared/enums.py b/src/dodal/devices/i06_shared/enums.py new file mode 100644 index 00000000000..4ea1486e655 --- /dev/null +++ b/src/dodal/devices/i06_shared/enums.py @@ -0,0 +1,7 @@ +from ophyd_async.core import StrictEnum + + +class Grating(StrictEnum): + G_150 = "150 lines/mm" + G_400 = "400 lines/mm" + G_1200 = "1200 lines/mm" From 2cd50bd43ebeba1d2a7f8042e2b8662eadb1c9b2 Mon Sep 17 00:00:00 2001 From: Fajin Yuan Date: Mon, 6 Oct 2025 09:31:51 +0100 Subject: [PATCH 13/13] Revert "add PGM support for i06 #116 (#1526)" (#1553) This reverts commit 02b853f5828bdb0e018e9b1103c782af7d7784ae. --- src/dodal/beamlines/__init__.py | 3 --- src/dodal/beamlines/i06.py | 17 ----------------- src/dodal/beamlines/i06_1.py | 15 --------------- src/dodal/beamlines/i06_shared.py | 18 ------------------ src/dodal/devices/i06_shared/__init__.py | 5 ----- src/dodal/devices/i06_shared/enums.py | 7 ------- 6 files changed, 65 deletions(-) delete mode 100644 src/dodal/beamlines/i06.py delete mode 100644 src/dodal/beamlines/i06_1.py delete mode 100644 src/dodal/beamlines/i06_shared.py delete mode 100644 src/dodal/devices/i06_shared/__init__.py delete mode 100644 src/dodal/devices/i06_shared/enums.py diff --git a/src/dodal/beamlines/__init__.py b/src/dodal/beamlines/__init__.py index b5e5abb38bc..3ce00a8c199 100644 --- a/src/dodal/beamlines/__init__.py +++ b/src/dodal/beamlines/__init__.py @@ -10,7 +10,6 @@ # dictionary, which maps ${BEAMLINE} to dodal.beamlines. _BEAMLINE_NAME_OVERRIDES = { "i05-1": "i05_1", - "i06-1": "i06_1", "b07-1": "b07_1", "i09-1": "i09_1", "i09-2": "i09_2", @@ -32,8 +31,6 @@ _BEAMLINE_SHARED = { "i05": ["i05", "i05_shared"], "i05_1": ["i05_1", "i05_shared"], - "i06": ["i06", "i06_shared"], - "i06_1": ["i06_1", "i06_shared"], "b07": ["b07", "b07_shared"], "b07_1": ["b07_1", "b07_shared"], "i09": ["i09", "i09_1_shared", "i09_2_shared"], diff --git a/src/dodal/beamlines/i06.py b/src/dodal/beamlines/i06.py deleted file mode 100644 index f08a74fca10..00000000000 --- a/src/dodal/beamlines/i06.py +++ /dev/null @@ -1,17 +0,0 @@ -from dodal.common.beamlines.beamline_utils import ( - device_factory, -) -from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline -from dodal.devices.synchrotron import Synchrotron -from dodal.log import set_beamline as set_log_beamline -from dodal.utils import BeamlinePrefix, get_beamline_name - -BL = get_beamline_name("i06") -PREFIX = BeamlinePrefix(BL, suffix="I") -set_log_beamline(BL) -set_utils_beamline(BL) - - -@device_factory() -def synchrotron() -> Synchrotron: - return Synchrotron() diff --git a/src/dodal/beamlines/i06_1.py b/src/dodal/beamlines/i06_1.py deleted file mode 100644 index 2eaa6810f55..00000000000 --- a/src/dodal/beamlines/i06_1.py +++ /dev/null @@ -1,15 +0,0 @@ -from dodal.common.beamlines.beamline_utils import device_factory -from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline -from dodal.devices.synchrotron import Synchrotron -from dodal.log import set_beamline as set_log_beamline -from dodal.utils import BeamlinePrefix, get_beamline_name - -BL = get_beamline_name("i06-1") -PREFIX = BeamlinePrefix(BL, suffix="J") -set_log_beamline(BL) -set_utils_beamline(BL) - - -@device_factory() -def synchrotron() -> Synchrotron: - return Synchrotron() diff --git a/src/dodal/beamlines/i06_shared.py b/src/dodal/beamlines/i06_shared.py deleted file mode 100644 index 6efadbd6849..00000000000 --- a/src/dodal/beamlines/i06_shared.py +++ /dev/null @@ -1,18 +0,0 @@ -from dodal.common.beamlines.beamline_utils import ( - device_factory, -) -from dodal.devices.i06_shared import Grating -from dodal.devices.pgm import PGM -from dodal.utils import BeamlinePrefix, get_beamline_name - -BL = get_beamline_name("i06") -PREFIX = BeamlinePrefix(BL, suffix="I") - - -@device_factory() -def pgm() -> PGM: - return PGM( - prefix=f"{PREFIX.beamline_prefix}-OP-PGM-01:", - grating=Grating, - gratingPv="NLINES2", - ) diff --git a/src/dodal/devices/i06_shared/__init__.py b/src/dodal/devices/i06_shared/__init__.py deleted file mode 100644 index 5e70b3928bc..00000000000 --- a/src/dodal/devices/i06_shared/__init__.py +++ /dev/null @@ -1,5 +0,0 @@ -from dodal.devices.i06_shared.enums import Grating - -__all__ = [ - "Grating", -] diff --git a/src/dodal/devices/i06_shared/enums.py b/src/dodal/devices/i06_shared/enums.py deleted file mode 100644 index 4ea1486e655..00000000000 --- a/src/dodal/devices/i06_shared/enums.py +++ /dev/null @@ -1,7 +0,0 @@ -from ophyd_async.core import StrictEnum - - -class Grating(StrictEnum): - G_150 = "150 lines/mm" - G_400 = "400 lines/mm" - G_1200 = "1200 lines/mm"