From 45e7fb52936c896b9d38f3b7c109a4c5be091137 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 24 Apr 2026 10:53:43 +0100 Subject: [PATCH 1/3] Correct names for injected devices --- .../beamlines/i04/oav_centering_plans/oav_imaging.py | 12 ++++++------ uv.lock | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py b/src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py index 0fca782755..255c1637ff 100644 --- a/src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py +++ b/src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py @@ -35,10 +35,10 @@ class FindBeamCentresComposite: scintillator: Scintillator xbpm_feedback: XBPMFeedback max_pixel: MaxPixel - centre_ellipse: CentreEllipseMethod + beam_centre: CentreEllipseMethod attenuator: BinaryFilterAttenuator zoom_controller: ZoomControllerWithBeamCentres - shutter: ZebraShutter + sample_shutter: ZebraShutter def take_oav_image_with_scintillator_in( @@ -299,7 +299,7 @@ def find_beam_centres( composite.backlight, composite.scintillator, composite.xbpm_feedback, - composite.shutter, + composite.sample_shutter, OAV_PREPARE_BEAMLINE_FOR_SCINT_WAIT, ) LOGGER.info("Waiting for prepare beamline plan to complete...") @@ -320,9 +320,9 @@ def find_beam_centres( xbpm_feedback=composite.xbpm_feedback, ) - yield from bps.trigger(composite.centre_ellipse, wait=True) - centre_x = yield from bps.rd(composite.centre_ellipse.center_x_val) - centre_y = yield from bps.rd(composite.centre_ellipse.center_y_val) + yield from bps.trigger(composite.beam_centre, wait=True) + centre_x = yield from bps.rd(composite.beam_centre.center_x_val) + centre_y = yield from bps.rd(composite.beam_centre.center_y_val) centre_x = round(centre_x) centre_y = round(centre_y) LOGGER.info( diff --git a/uv.lock b/uv.lock index 0336186b8e..8f31905622 100644 --- a/uv.lock +++ b/uv.lock @@ -802,8 +802,8 @@ wheels = [ [[package]] name = "dls-dodal" -version = "2.2.2.dev2+g8cf8dfb9c" -source = { git = "https://github.com/DiamondLightSource/dodal.git?rev=main#8cf8dfb9ca0716f588d1c67695b07da442cec4ca" } +version = "2.2.3.dev5+g257b180e8" +source = { git = "https://github.com/DiamondLightSource/dodal.git?rev=main#257b180e8b2cf5121944f3cf518e9ff96da9cb4a" } dependencies = [ { name = "aiofiles", marker = "platform_machine == 'x86_64' and sys_platform == 'linux'" }, { name = "aiohttp", marker = "platform_machine == 'x86_64' and sys_platform == 'linux'" }, From cf7de44ef356bb575572ae1894e70a8c2a786a97 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 28 Apr 2026 17:07:22 +0100 Subject: [PATCH 2/3] Fix i04 xrc callbacks --- .../i04_grid_detect_then_xray_centre_plan.py | 31 ++++++++++--------- uv.lock | 4 +-- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/mx_bluesky/beamlines/i04/experiment_plans/i04_grid_detect_then_xray_centre_plan.py b/src/mx_bluesky/beamlines/i04/experiment_plans/i04_grid_detect_then_xray_centre_plan.py index a4c5e08f5a..6524240daf 100644 --- a/src/mx_bluesky/beamlines/i04/experiment_plans/i04_grid_detect_then_xray_centre_plan.py +++ b/src/mx_bluesky/beamlines/i04/experiment_plans/i04_grid_detect_then_xray_centre_plan.py @@ -226,21 +226,22 @@ def grid_detect_then_xray_centre_with_callbacks(): oav_config=oav_config, ) - try: - yield from grid_detect_then_xray_centre_with_callbacks() - assert isinstance( - grid_common_params.specified_grid_params, SpecifiedThreeDGridScan - ), "Specified grid params couldn't be found after grid detection" - yield from get_results_and_move_to_xtal( - composite, - grid_common_params.specified_grid_params, - flyscan_event_handler, - ) - except CrystalNotFoundError: - yield from bps.mv( - smargon.x, initial_x, smargon.y, initial_y, smargon.z, initial_z - ) - raise + try: + assert isinstance( + grid_common_params.specified_grid_params, SpecifiedThreeDGridScan + ), "Specified grid params couldn't be found after grid detection" + yield from get_results_and_move_to_xtal( + composite, + grid_common_params.specified_grid_params, + flyscan_event_handler, + ) + except CrystalNotFoundError: + yield from bps.mv( + smargon.x, initial_x, smargon.y, initial_y, smargon.z, initial_z + ) + raise + + yield from grid_detect_then_xray_centre_with_callbacks() yield from _change_beamsize( transfocator, DEFAULT_XRC_BEAMSIZE_MICRONS, grid_common_params diff --git a/uv.lock b/uv.lock index 8f31905622..d69419d2fa 100644 --- a/uv.lock +++ b/uv.lock @@ -802,8 +802,8 @@ wheels = [ [[package]] name = "dls-dodal" -version = "2.2.3.dev5+g257b180e8" -source = { git = "https://github.com/DiamondLightSource/dodal.git?rev=main#257b180e8b2cf5121944f3cf518e9ff96da9cb4a" } +version = "2.2.4.dev2+g334d34d91" +source = { git = "https://github.com/DiamondLightSource/dodal.git?rev=main#334d34d911dad4d2e5c6b37ab8b3170dae96e335" } dependencies = [ { name = "aiofiles", marker = "platform_machine == 'x86_64' and sys_platform == 'linux'" }, { name = "aiohttp", marker = "platform_machine == 'x86_64' and sys_platform == 'linux'" }, From 2706acd11bc576ceae11b4107ff534f425275d11 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 28 Apr 2026 18:04:19 +0100 Subject: [PATCH 3/3] Fix tests --- tests/conftest.py | 6 +- ...t_i04_grid_detect_then_xray_centre_plan.py | 54 ++++++-- .../beamlines/i04/test_oav_imaging.py | 129 +++++++++++------- uv.lock | 4 +- 4 files changed, 122 insertions(+), 71 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 567fe4a1a3..b6e4bfa946 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1255,7 +1255,7 @@ def default_raw_gridscan_params( return raw_params_from_file(json_file, tmp_path) -def _dummy_params(tmp_path): +def dummy_params(tmp_path): dummy_params = SpecifiedThreeDGridScan( **raw_params_from_file( "tests/test_data/parameter_json_files/test_gridscan_param_defaults.json", @@ -1366,7 +1366,7 @@ def test_grid_detect_and_gridscan_start_document(self) -> RunStart: "scan_id": 1, "plan_type": "generator", "subplan_name": PlanNameConstants.GRID_DETECT_AND_DO_GRIDSCAN, - "mx_bluesky_parameters": _dummy_params(self._tmp_path).model_dump_json(), + "mx_bluesky_parameters": dummy_params(self._tmp_path).model_dump_json(), } @property @@ -1400,7 +1400,7 @@ def test_gridscan_outer_start_document(self): "plan_name": PlanNameConstants.GRIDSCAN_OUTER, "subplan_name": PlanNameConstants.GRIDSCAN_OUTER, "zocalo_environment": EnvironmentConstants.ZOCALO_ENV, - "mx_bluesky_parameters": _dummy_params(self._tmp_path).model_dump_json(), + "mx_bluesky_parameters": dummy_params(self._tmp_path).model_dump_json(), } @property diff --git a/tests/unit_tests/beamlines/i04/test_i04_grid_detect_then_xray_centre_plan.py b/tests/unit_tests/beamlines/i04/test_i04_grid_detect_then_xray_centre_plan.py index 686504acc4..8a2b2262c2 100644 --- a/tests/unit_tests/beamlines/i04/test_i04_grid_detect_then_xray_centre_plan.py +++ b/tests/unit_tests/beamlines/i04/test_i04_grid_detect_then_xray_centre_plan.py @@ -50,7 +50,12 @@ ) from mx_bluesky.common.utils.exceptions import CrystalNotFoundError from mx_bluesky.common.utils.xrc_result import XRayCentreResult -from tests.conftest import TEST_RESULT_LARGE, fake_generator, simulate_xrc_result +from tests.conftest import ( + TEST_RESULT_LARGE, + dummy_params, + fake_generator, + simulate_xrc_result, +) from tests.unit_tests.common.experiment_plans.test_common_flyscan_xray_centre_plan import ( CompleteError, ) @@ -153,9 +158,11 @@ def test_get_ready_for_oav_and_close_shutter_closes_shutter_and_calls_setup_for_ ) msgs = assert_message_and_return_remaining( msgs, - predicate=lambda msg: msg.command == "set" - and msg.obj.name == "detector_motion-shutter" - and msg.args[0] == 0, + predicate=lambda msg: ( + msg.command == "set" + and msg.obj.name == "detector_motion-shutter" + and msg.args[0] == 0 + ), ) msgs = assert_message_and_return_remaining( msgs, predicate=lambda msg: msg.command == "wait" @@ -259,9 +266,11 @@ def test_i04_default_grid_detect_and_xray_centre_sets_transmission_and_triggers_ msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "attenuator" - and msg.args == (desired_transmission,), + lambda msg: ( + msg.command == "set" + and msg.obj.name == "attenuator" + and msg.args == (desired_transmission,) + ), ) msgs = assert_message_and_return_remaining( @@ -270,14 +279,16 @@ def test_i04_default_grid_detect_and_xray_centre_sets_transmission_and_triggers_ ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "open_run" - and msg.run == PlanNameConstants.GRIDSCAN_OUTER, + lambda msg: ( + msg.command == "open_run" and msg.run == PlanNameConstants.GRIDSCAN_OUTER + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "close_run" - and msg.run == PlanNameConstants.GRIDSCAN_OUTER, + lambda msg: ( + msg.command == "close_run" and msg.run == PlanNameConstants.GRIDSCAN_OUTER + ), ) mock_pause_feedback.assert_not_called() @@ -398,6 +409,10 @@ async def test_i04_grid_detect_then_xrc_sets_beamsize_before_grid_detect_then_re ] +@patch( + "mx_bluesky.beamlines.i04.experiment_plans.i04_grid_detect_then_xray_centre_plan.get_results_and_move_to_xtal", + autospec=True, +) @patch( "mx_bluesky.beamlines.i04.experiment_plans.i04_grid_detect_then_xray_centre_plan.get_ready_for_oav_and_close_shutter", autospec=True, @@ -409,16 +424,23 @@ async def test_i04_grid_detect_then_xrc_sets_beamsize_before_grid_detect_then_re async def test_given_no_diffraction_found_i04_grid_detect_then_xrc_returns_sample_to_initial_position( mock_grid_detect_then_xray_centre: MagicMock, mock_get_ready_for_oav_and_close_shutter: MagicMock, + mock_get_results_and_move_to_xtal: MagicMock, run_engine: RunEngine, i04_grid_detect_then_xrc_default_params: partial[MsgGenerator], smargon: Smargon, + tmp_path, ): initial_x, initial_y, initial_z = 1, 2, 3 set_mock_value(smargon.x.user_readback, initial_x) set_mock_value(smargon.y.user_readback, initial_y) set_mock_value(smargon.z.user_readback, initial_z) - mock_grid_detect_then_xray_centre.side_effect = CrystalNotFoundError + def fake_xray_centre(parameters: GridCommon, **__): + parameters.set_specified_grid_params(dummy_params(tmp_path)) + yield Msg(command="open_run") + + mock_grid_detect_then_xray_centre.side_effect = fake_xray_centre + mock_get_results_and_move_to_xtal.side_effect = CrystalNotFoundError with pytest.raises(CrystalNotFoundError): run_engine(i04_grid_detect_then_xrc_default_params()) @@ -539,9 +561,11 @@ def test_grid_detect_then_xrc_stages_and_unstages_zocalo_and_gets_results( msgs = assert_message_and_return_remaining( msgs, - predicate=lambda msg: msg.command == "stage" - and msg.obj.name == "zocalo" - and msg.kwargs["group"] == ZOCALO_STAGE_GROUP, + predicate=lambda msg: ( + msg.command == "stage" + and msg.obj.name == "zocalo" + and msg.kwargs["group"] == ZOCALO_STAGE_GROUP + ), ) msgs = assert_message_and_return_remaining( diff --git a/tests/unit_tests/beamlines/i04/test_oav_imaging.py b/tests/unit_tests/beamlines/i04/test_oav_imaging.py index 6a0d775c86..990cef1c85 100644 --- a/tests/unit_tests/beamlines/i04/test_oav_imaging.py +++ b/tests/unit_tests/beamlines/i04/test_oav_imaging.py @@ -98,42 +98,52 @@ def test_prepare_beamline_for_scint_images( messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "beamstop-selected_pos" - and msg.args[0] == BeamstopPositions.DATA_COLLECTION - and msg.kwargs["group"] == test_group, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "beamstop-selected_pos" + and msg.args[0] == BeamstopPositions.DATA_COLLECTION + and msg.kwargs["group"] == test_group + ), ) messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "backlight" - and msg.args[0] == InOut.OUT, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "backlight" + and msg.args[0] == InOut.OUT + ), ) messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "scintillator-selected_pos" - and msg.args[0] == InOut.IN - and msg.kwargs["group"] == test_group, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "scintillator-selected_pos" + and msg.args[0] == InOut.IN + and msg.kwargs["group"] == test_group + ), ) messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "sample_shutter-control_mode" - and msg.args[0] == ZebraShutterControl.MANUAL, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "sample_shutter-control_mode" + and msg.args[0] == ZebraShutterControl.MANUAL + ), ) messages = assert_message_and_return_remaining( messages, lambda msg: msg.command == "wait" ) assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "sample_shutter" - and msg.args[0] == ZebraShutterState.OPEN - and msg.kwargs["group"] == test_group, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "sample_shutter" + and msg.args[0] == ZebraShutterState.OPEN + and msg.kwargs["group"] == test_group + ), ) @@ -163,9 +173,9 @@ def test_plan_stubs_called_in_correct_order( messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "attenuator" - and msg.args[0] == 1, + lambda msg: ( + msg.command == "set" and msg.obj.name == "attenuator" and msg.args[0] == 1 + ), ) messages = assert_message_and_return_remaining( @@ -175,28 +185,34 @@ def test_plan_stubs_called_in_correct_order( messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "sample_shutter-control_mode" - and msg.args[0] == ZebraShutterControl.MANUAL, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "sample_shutter-control_mode" + and msg.args[0] == ZebraShutterControl.MANUAL + ), ) messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == messages[0].kwargs["group"], + lambda msg: ( + msg.command == "wait" and msg.kwargs["group"] == messages[0].kwargs["group"] + ), ) messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "sample_shutter" - and msg.args[0] == ZebraShutterState.OPEN, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "sample_shutter" + and msg.args[0] == ZebraShutterState.OPEN + ), ) messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == messages[0].kwargs["group"], + lambda msg: ( + msg.command == "wait" and msg.kwargs["group"] == messages[0].kwargs["group"] + ), ) @@ -232,9 +248,11 @@ def test_plan_called_with_specified_transmission_then_transmission_set( messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "attenuator" - and msg.args[0] == transmission, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "attenuator" + and msg.args[0] == transmission + ), ) @@ -247,16 +265,20 @@ def test_oav_image(sim_run_engine: RunEngineSimulator, oav: OAV): messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "oav-snapshot-filename" - and msg.args[0] == mock_filename, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "oav-snapshot-filename" + and msg.args[0] == mock_filename + ), ) messages = assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "oav-snapshot-directory" - and msg.args[0] == mock_filepath, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "oav-snapshot-directory" + and msg.args[0] == mock_filepath + ), ) messages = assert_message_and_return_remaining( @@ -682,10 +704,10 @@ def find_beam_centre_devices( scintillator=scintillator, xbpm_feedback=xbpm_feedback, max_pixel=max_pixel, - centre_ellipse=centre_ellipse, + beam_centre=centre_ellipse, attenuator=attenuator, zoom_controller=zoom_controller_with_centres, - shutter=sample_shutter, + sample_shutter=sample_shutter, ) @@ -728,7 +750,8 @@ def test_find_beam_centres_starts_by_prepping_scintillator( ): run_engine(find_beam_centres(composite=find_beam_centre_devices)) mock_prepare_scintillator.assert_called_once() - assert find_beam_centre_devices.centre_ellipse.trigger.call_count == 4 # type: ignore + assert isinstance(find_beam_centre_devices.beam_centre.trigger, MagicMock) + assert find_beam_centre_devices.beam_centre.trigger.call_count == 4 def mock_centre_ellipse_with_given_centres( @@ -759,7 +782,7 @@ async def test_find_beam_centres_iterates_and_sets_centres( new_centres = [(100, 100), (200, 200), (300, 300), (400, 400)] expected_centres = new_centres.copy() - centre_ellipse = find_beam_centre_devices.centre_ellipse + centre_ellipse = find_beam_centre_devices.beam_centre zoom_controller_with_centres: ZoomControllerWithBeamCentres = ( find_beam_centre_devices.zoom_controller ) @@ -795,7 +818,7 @@ async def test_if_only_some_levels_given_then_find_beam_centres_iterates_and_set ): new_centres = [(100, 100), (200, 200), (300, 300), (400, 400)] - centre_ellipse = find_beam_centre_devices.centre_ellipse + centre_ellipse = find_beam_centre_devices.beam_centre zoom_controller_with_centres: ZoomControllerWithBeamCentres = ( find_beam_centre_devices.zoom_controller ) @@ -908,15 +931,19 @@ def test_find_beam_centres_prepares_beamline_and_waits( ) assert_message_and_return_remaining( messages, - lambda msg: msg.command == "set" - and msg.obj.name == "sample_shutter" - and msg.args[0] == ZebraShutterState.OPEN - and msg.kwargs["group"] == OAV_PREPARE_BEAMLINE_FOR_SCINT_WAIT, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "sample_shutter" + and msg.args[0] == ZebraShutterState.OPEN + and msg.kwargs["group"] == OAV_PREPARE_BEAMLINE_FOR_SCINT_WAIT + ), ) assert_message_and_return_remaining( messages, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == OAV_PREPARE_BEAMLINE_FOR_SCINT_WAIT, + lambda msg: ( + msg.command == "wait" + and msg.kwargs["group"] == OAV_PREPARE_BEAMLINE_FOR_SCINT_WAIT + ), ) diff --git a/uv.lock b/uv.lock index d69419d2fa..fe339b647f 100644 --- a/uv.lock +++ b/uv.lock @@ -802,8 +802,8 @@ wheels = [ [[package]] name = "dls-dodal" -version = "2.2.4.dev2+g334d34d91" -source = { git = "https://github.com/DiamondLightSource/dodal.git?rev=main#334d34d911dad4d2e5c6b37ab8b3170dae96e335" } +version = "2.2.4.dev3+gd4c4ae983" +source = { git = "https://github.com/DiamondLightSource/dodal.git?rev=main#d4c4ae98370aa28bbe4458f087b742445209e5ca" } dependencies = [ { name = "aiofiles", marker = "platform_machine == 'x86_64' and sys_platform == 'linux'" }, { name = "aiohttp", marker = "platform_machine == 'x86_64' and sys_platform == 'linux'" },