From e5dcdf384efc6a05aaa4d07e6d8d2ad9163f91a6 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Thu, 9 Apr 2026 16:43:29 +0100 Subject: [PATCH 01/36] Additional documentation regarding environment variables --- .../advanced/advanced_configuration.rst | 18 ++++++++++++++++++ docs/user/hyperion/configuration.rst | 5 +++++ 2 files changed, 23 insertions(+) create mode 100644 docs/user/hyperion/advanced/advanced_configuration.rst diff --git a/docs/user/hyperion/advanced/advanced_configuration.rst b/docs/user/hyperion/advanced/advanced_configuration.rst new file mode 100644 index 0000000000..eeddbbf77d --- /dev/null +++ b/docs/user/hyperion/advanced/advanced_configuration.rst @@ -0,0 +1,18 @@ +Advanced Configuration +====================== + +Environment Variables +--------------------- + +Hyperion deployment is configurable by number of different environment variables: + +.. csv-table:: Environment variables + :widths: auto + :header: "Environment Variable", "Description" + + "ISPYB_CONFIG_PATH", "Path to endpoint and credential configuration for ISPyB and ExpEye" + "LOG_DIR", "Path to the logging directory for file-based logging" + "DEBUG_LOG_DIR", "Path to the logging directory for the debug log" + "ZOCALO_CONFIG", "Path to the configuration YAML file for zocalo" + "BEAMLINE", "Defines the name of the beamline" + "CONFIG_SERVER_URL", "URL for the config server" diff --git a/docs/user/hyperion/configuration.rst b/docs/user/hyperion/configuration.rst index ff62c4a309..6f92003190 100644 --- a/docs/user/hyperion/configuration.rst +++ b/docs/user/hyperion/configuration.rst @@ -57,3 +57,8 @@ ultimately it will be the source of all configuration and the remainder of the f moved over to it. .. _Config Server: https://github.com/DiamondLightSource/daq-config-server/ + + +See also `Advanced Configuration`_ for details of configuration performed at deployment time. + +.. _`Code Map`: advanced/advanced_configuration.rst From 8c167a02a212f6f9644f52d78ca85a37ed085f82 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Thu, 9 Apr 2026 16:52:01 +0100 Subject: [PATCH 02/36] External callbacks now have configurable config server URL --- .../external_interaction/callbacks/__main__.py | 13 +++++++++++++ .../callbacks/test_external_callbacks.py | 12 ++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/mx_bluesky/hyperion/external_interaction/callbacks/__main__.py b/src/mx_bluesky/hyperion/external_interaction/callbacks/__main__.py index c93c746221..a5f0ab770a 100644 --- a/src/mx_bluesky/hyperion/external_interaction/callbacks/__main__.py +++ b/src/mx_bluesky/hyperion/external_interaction/callbacks/__main__.py @@ -1,4 +1,5 @@ import logging +import os from abc import abstractmethod from collections.abc import Callable from contextlib import AbstractContextManager @@ -12,6 +13,8 @@ from bluesky.callbacks.zmq import Proxy, RemoteDispatcher from bluesky_stomp.messaging import StompClient from bluesky_stomp.models import Broker +from daq_config_server import ConfigClient +from dodal.common.beamlines.beamline_utils import set_config_client from dodal.log import LOGGER as DODAL_LOGGER from dodal.log import set_up_all_logging_handlers @@ -141,6 +144,15 @@ def setup_logging(dev_mode: bool): log_debug("nexgen logger added to nexus logger") +def create_config_client() -> ConfigClient: + config_server_url = os.getenv("CONFIG_SERVER_URL") + if not config_server_url: + raise ValueError( + "CONFIG_SERVER_URL must be specified to run external callbacks." + ) + return ConfigClient(config_server_url) + + def log_info(msg, *args, **kwargs): ISPYB_ZOCALO_CALLBACK_LOGGER.info(msg, *args, **kwargs) NEXUS_LOGGER.info(msg, *args, **kwargs) @@ -157,6 +169,7 @@ class HyperionCallbackRunner: def __init__(self, callback_args: CallbackArgs) -> None: setup_logging(callback_args.dev_mode) log_info("Hyperion callback process started.") + set_config_client(create_config_client()) set_alerting_service(LoggingAlertService(CONST.GRAYLOG_STREAM_ID)) self.callbacks = setup_callbacks() diff --git a/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py b/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py index 6371f553ed..227c4a6438 100644 --- a/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py +++ b/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py @@ -33,6 +33,7 @@ ) @patch("mx_bluesky.hyperion.external_interaction.callbacks.__main__.setup_callbacks") @patch("mx_bluesky.hyperion.external_interaction.callbacks.__main__.setup_logging") +@patch("mx_bluesky.hyperion.external_interaction.callbcaks.__main__.set_config_client") @patch( "mx_bluesky.hyperion.external_interaction.callbacks.__main__.set_alerting_service" ) @@ -42,6 +43,7 @@ def test_main_function( mock_proxy: MagicMock, mock_dispatcher: MagicMock, setup_alerting: MagicMock, + set_config_client: MagicMock, setup_logging: MagicMock, setup_callbacks: MagicMock, parse_callback_args: MagicMock, @@ -60,12 +62,22 @@ def test_main_function( dispatcher_started.wait(0.5) mock_run_watchdog.wait(0.5) setup_logging.assert_called() + set_config_client.assert_called() setup_callbacks.assert_called() setup_alerting.assert_called_once() mock_run_watchdog.assert_called_once() assert isinstance(setup_alerting.mock_calls[0].args[0], LoggingAlertService) +@patch( + "mx_bluesky.hyperion.external_interaction.callbacks.__main__.parse_callback_args", + MagicMock(return_value=CallbackArgs(True, HyperionConstants.SUPERVISOR_PORT)), +) +def test_no_config_server_url_raises_exception(): + with pytest.raises(ValueError, match="CONFIG_SERVER_URL must be specified"): + main() + + def test_setup_callbacks(): current_number_of_callbacks = 8 cbs = setup_callbacks() From c204aec3d32dc0a9ecb82089441f19a3b85450ce Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Mon, 13 Apr 2026 12:15:02 +0100 Subject: [PATCH 03/36] Set CONFIG_SERVER_URL in external callback unit tests --- .../external_interaction/callbacks/test_external_callbacks.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py b/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py index 227c4a6438..0c20e2d6d2 100644 --- a/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py +++ b/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py @@ -48,7 +48,9 @@ def test_main_function( setup_callbacks: MagicMock, parse_callback_args: MagicMock, mock_run_watchdog: MagicMock, + monkeypatch, ): + monkeypatch.setenv("CONFIG_SERVER_URL", "http://127.0.0.1:8555") proxy_started = Event() dispatcher_started = Event() watchdog_started = Event() @@ -178,7 +180,9 @@ def test_launch_with_stomp_launches_stomp_backend( mock_setup_callbacks: MagicMock, mock_client_cls: MagicMock, mock_dispatcher_cls: MagicMock, + monkeypatch, ): + monkeypatch.setenv("CONFIG_SERVER_URL", "http://127.0.0.1:8555") stomp_client = mock_client_cls.for_broker.return_value dispatcher = mock_dispatcher_cls.return_value stomp_client.is_connected.side_effect = [True, False] From 912861f6b4bac6a2f421615f476290b3552c4010 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Thu, 16 Apr 2026 10:52:19 +0100 Subject: [PATCH 04/36] Add CONFIG_SERVER_URL to the launcher script --- run_hyperion.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/run_hyperion.sh b/run_hyperion.sh index 37b95a40c4..f0299e9bac 100755 --- a/run_hyperion.sh +++ b/run_hyperion.sh @@ -110,6 +110,8 @@ if [ -z "${BEAMLINE}" ]; then exit 1 fi +export CONFIG_SERVER_URL="https://${BEAMLINE}-daq-config.diamond.ac.uk" + if [[ $STOP == 1 ]]; then if [ $IN_DEV == false ]; then check_user From 5ed0afefa7b54ac0e376d60bc13c4bfe0867547c Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Thu, 16 Apr 2026 11:01:28 +0100 Subject: [PATCH 05/36] Fix broken doc links --- docs/user/hyperion/configuration.rst | 5 +++-- docs/user/hyperion/troubleshooting.rst | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/user/hyperion/configuration.rst b/docs/user/hyperion/configuration.rst index 6f92003190..d0ed16fd33 100644 --- a/docs/user/hyperion/configuration.rst +++ b/docs/user/hyperion/configuration.rst @@ -56,9 +56,10 @@ Note that currently the rest of the configuration files are not read from the co ultimately it will be the source of all configuration and the remainder of the files in ``daq_configuration`` will be moved over to it. -.. _Config Server: https://github.com/DiamondLightSource/daq-config-server/ +See `Config Server`_ for details of the config server and how it is configured and deployed. +.. _Config Server: https://github.com/DiamondLightSource/daq-config-server/ See also `Advanced Configuration`_ for details of configuration performed at deployment time. -.. _`Code Map`: advanced/advanced_configuration.rst +.. _`Advanced Configuration`: advanced/advanced_configuration.html diff --git a/docs/user/hyperion/troubleshooting.rst b/docs/user/hyperion/troubleshooting.rst index a1b0e3ac42..75ecc99251 100644 --- a/docs/user/hyperion/troubleshooting.rst +++ b/docs/user/hyperion/troubleshooting.rst @@ -40,7 +40,7 @@ following However on inspection the start log will not show any errors. Hyperion running can be verified as above `Verifying that Hyperion is running`_ -.. _`Verifying that Hyperion is running`: advanced/install.rst +.. _`Verifying that Hyperion is running`: advanced/install.html Smargon Motion ~~~~~~~~~~~~~~ From af67b261622defc9731182d2c1f30a670a67fce6 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Mon, 13 Apr 2026 12:12:18 +0100 Subject: [PATCH 06/36] fix typo in test_external_callbacks.py --- .../external_interaction/callbacks/test_external_callbacks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py b/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py index 0c20e2d6d2..04da7ee0ac 100644 --- a/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py +++ b/tests/unit_tests/hyperion/external_interaction/callbacks/test_external_callbacks.py @@ -33,7 +33,7 @@ ) @patch("mx_bluesky.hyperion.external_interaction.callbacks.__main__.setup_callbacks") @patch("mx_bluesky.hyperion.external_interaction.callbacks.__main__.setup_logging") -@patch("mx_bluesky.hyperion.external_interaction.callbcaks.__main__.set_config_client") +@patch("mx_bluesky.hyperion.external_interaction.callbacks.__main__.set_config_client") @patch( "mx_bluesky.hyperion.external_interaction.callbacks.__main__.set_alerting_service" ) From 4e3020f749cbb3efdc4d67c6e7e01543ef454ac2 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Wed, 15 Apr 2026 17:18:16 +0100 Subject: [PATCH 07/36] Update uv.lock --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 6ade793d98..f35ae65e60 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,7 +55,7 @@ dependencies = [ "ophyd >= 1.10.5", "ophyd-async >= 0.16.0", "bluesky >= 1.14.6", - "dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@main", + "dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@fix_config_server_system_test_breakage", ] From 152627f888d22b135dcac05a1364a8665281629e Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Tue, 28 Apr 2026 15:12:09 +0100 Subject: [PATCH 08/36] Unpin dodal --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f35ae65e60..6ade793d98 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,7 +55,7 @@ dependencies = [ "ophyd >= 1.10.5", "ophyd-async >= 0.16.0", "bluesky >= 1.14.6", - "dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@fix_config_server_system_test_breakage", + "dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@main", ] From c9e1260c82ec307454b819202038dfd712b34d68 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Fri, 10 Apr 2026 14:53:08 +0100 Subject: [PATCH 09/36] Remove insertion_prefix from WithVisit parameter model --- src/mx_bluesky/common/parameters/components.py | 2 -- tests/test_data/hyperion_blueapi/test_load_centre_collect.json | 1 - .../good_test_grid_with_edge_detect_parameters.json | 1 - .../good_test_load_centre_collect_params.json | 1 - .../good_test_load_centre_collect_params_multi_rotation.json | 1 - .../good_test_multi_rotation_scan_parameters.json | 1 - .../good_test_one_multi_rotation_scan_parameters.json | 1 - .../good_test_one_multi_rotation_scan_parameters_nomove.json | 1 - tests/test_data/parameter_json_files/good_test_parameters.json | 1 - .../good_test_pin_centre_then_xray_centre_parameters.json | 1 - .../good_test_robot_load_and_centre_params.json | 1 - .../parameter_json_files/good_test_robot_load_params.json | 1 - .../good_test_rotation_scan_parameters.json | 1 - .../ispyb_gridscan_system_test_parameters.json | 1 - .../parameter_json_files/test_gridscan_param_defaults.json | 1 - .../experiment_plans/test_load_centre_collect_full_plan.py | 2 -- 16 files changed, 18 deletions(-) diff --git a/src/mx_bluesky/common/parameters/components.py b/src/mx_bluesky/common/parameters/components.py index 97615e82a5..52d2317426 100644 --- a/src/mx_bluesky/common/parameters/components.py +++ b/src/mx_bluesky/common/parameters/components.py @@ -24,7 +24,6 @@ from semver import Version from mx_bluesky.common.parameters.constants import ( - TEST_MODE, USE_NUMTRACKER, DetectorParamConstants, GridscanParamConstants, @@ -158,7 +157,6 @@ class WithVisit(BaseModel): default=DetectorParamConstants.BEAM_XY_LUT_PATH ) detector_distance_mm: float | None = Field(default=None, gt=0) - insertion_prefix: str = "SR03S" if TEST_MODE else "SR03I" class DiffractionExperiment( diff --git a/tests/test_data/hyperion_blueapi/test_load_centre_collect.json b/tests/test_data/hyperion_blueapi/test_load_centre_collect.json index 376eeace9f..2f55a641a1 100644 --- a/tests/test_data/hyperion_blueapi/test_load_centre_collect.json +++ b/tests/test_data/hyperion_blueapi/test_load_centre_collect.json @@ -5,7 +5,6 @@ "parameter_model_version": "5.0.0", "beamline": "BL03S", "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", - "insertion_prefix": "SR03S", "visit": "cm31105-4", "detector_distance_mm": 255, "sample_id": 12345, diff --git a/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json b/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json index e0b2d30681..2f196197d0 100644 --- a/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "beamline": "BL03S", - "insertion_prefix": "SR03S", "storage_directory": "/tmp", "file_name": "file_name", "run_number": 0, diff --git a/tests/test_data/parameter_json_files/good_test_load_centre_collect_params.json b/tests/test_data/parameter_json_files/good_test_load_centre_collect_params.json index 5f98f92e58..bc77e8f286 100644 --- a/tests/test_data/parameter_json_files/good_test_load_centre_collect_params.json +++ b/tests/test_data/parameter_json_files/good_test_load_centre_collect_params.json @@ -2,7 +2,6 @@ "parameter_model_version": "5.0.0", "beamline": "BL03S", "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", - "insertion_prefix": "SR03S", "visit": "cm31105-4", "detector_distance_mm": 255, "sample_id": 12345, diff --git a/tests/test_data/parameter_json_files/good_test_load_centre_collect_params_multi_rotation.json b/tests/test_data/parameter_json_files/good_test_load_centre_collect_params_multi_rotation.json index eb84f9bb94..a9b183631a 100644 --- a/tests/test_data/parameter_json_files/good_test_load_centre_collect_params_multi_rotation.json +++ b/tests/test_data/parameter_json_files/good_test_load_centre_collect_params_multi_rotation.json @@ -2,7 +2,6 @@ "parameter_model_version": "5.0.0", "beamline": "BL03S", "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", - "insertion_prefix": "SR03S", "visit": "cm31105-4", "detector_distance_mm": 255, "sample_id": 12345, diff --git a/tests/test_data/parameter_json_files/good_test_multi_rotation_scan_parameters.json b/tests/test_data/parameter_json_files/good_test_multi_rotation_scan_parameters.json index bb2958c3ef..0061c4cfbe 100644 --- a/tests/test_data/parameter_json_files/good_test_multi_rotation_scan_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_multi_rotation_scan_parameters.json @@ -6,7 +6,6 @@ "detector_distance_mm": 100.0, "demand_energy_ev": 100, "exposure_time_s": 0.1, - "insertion_prefix": "SR03S", "file_name": "file_name", "run_number": 0, "shutter_opening_time_s": 0.6, diff --git a/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters.json b/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters.json index e1874e5164..738a2af0c8 100644 --- a/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters.json @@ -6,7 +6,6 @@ "detector_distance_mm": 100.0, "demand_energy_ev": 100, "exposure_time_s": 0.1, - "insertion_prefix": "SR03S", "file_name": "file_name", "visit": "cm31105-4", "transmission_frac": 0.1, diff --git a/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters_nomove.json b/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters_nomove.json index e8a2c24089..61057c8b9d 100644 --- a/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters_nomove.json +++ b/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters_nomove.json @@ -6,7 +6,6 @@ "detector_distance_mm": 100.0, "demand_energy_ev": 100, "exposure_time_s": 0.1, - "insertion_prefix": "SR03S", "file_name": "file_name", "rotation_increment_deg": 0.1, "run_number": 0, diff --git a/tests/test_data/parameter_json_files/good_test_parameters.json b/tests/test_data/parameter_json_files/good_test_parameters.json index 14b9b80900..745d28bd59 100644 --- a/tests/test_data/parameter_json_files/good_test_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_parameters.json @@ -6,7 +6,6 @@ "detector_distance_mm": 100.0, "visit": "cm31105-4", "exposure_time_s": 0.1, - "insertion_prefix": "SR03S", "omega_start_deg": 0.0, "file_name": "file_name", "sample_id": 123456, diff --git a/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json b/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json index 4bae2ad6d8..d379d3a276 100644 --- a/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "beamline": "BL03S", - "insertion_prefix": "SR03S", "storage_directory": "/tmp", "file_name": "file_name", "run_number": 0, diff --git a/tests/test_data/parameter_json_files/good_test_robot_load_and_centre_params.json b/tests/test_data/parameter_json_files/good_test_robot_load_and_centre_params.json index da32330450..e74fd38c31 100644 --- a/tests/test_data/parameter_json_files/good_test_robot_load_and_centre_params.json +++ b/tests/test_data/parameter_json_files/good_test_robot_load_and_centre_params.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "beamline": "BL03S", - "insertion_prefix": "SR03S", "storage_directory": "/tmp/", "visit": "cm31105-4", "file_name": "file_name", diff --git a/tests/test_data/parameter_json_files/good_test_robot_load_params.json b/tests/test_data/parameter_json_files/good_test_robot_load_params.json index 72e111faa6..959fa6a999 100644 --- a/tests/test_data/parameter_json_files/good_test_robot_load_params.json +++ b/tests/test_data/parameter_json_files/good_test_robot_load_params.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "beamline": "BL03S", - "insertion_prefix": "SR03S", "snapshot_directory": "/tmp/", "storage_directory":"/tmp/", "visit": "cm31105-4", diff --git a/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json b/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json index f149b31936..cdc7238f05 100644 --- a/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json @@ -6,7 +6,6 @@ "detector_distance_mm": 100.0, "demand_energy_ev": 100, "exposure_time_s": 0.1, - "insertion_prefix": "SR03S", "omega_start_deg": 0.0, "file_name": "file_name", "scan_width_deg": 180.0, diff --git a/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json b/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json index 490188798b..30efe5af4a 100644 --- a/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json +++ b/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "beamline": "BL03S", - "insertion_prefix": "SR03S", "storage_directory": "{tmp_data}", "file_name": "file_name", "run_number": 0, diff --git a/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json b/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json index 5383c7bea8..62f8d54f87 100644 --- a/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json +++ b/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "beamline": "BL03S", - "insertion_prefix": "SR03S", "demand_energy_ev": 100, "storage_directory": "{tmp_data}", "visit": "cm31105-4", diff --git a/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py b/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py index bcff9171f4..f9f3fad209 100644 --- a/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py +++ b/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py @@ -278,7 +278,6 @@ def test_params_with_different_energy_for_rotation_gridscan_rejected(tmp_path): # WithVisit ["beamline", "i03"], ["visit", "cm12345"], - ["insertion_prefix", "SR03"], ["detector_distance_mm", 123], ["det_dist_to_beam_converter_path", "/foo/bar"], ], @@ -308,7 +307,6 @@ def test_params_with_unexpected_info_in_robot_load_rejected( # WithVisit ["beamline", "i03"], ["visit", "cm12345"], - ["insertion_prefix", "SR03"], ["detector_distance_mm", 123], ["det_dist_to_beam_converter_path", "/foo/bar"], ], From aa3edff574b82fa2dff775bdc888b2e967f31391 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Fri, 10 Apr 2026 14:54:21 +0100 Subject: [PATCH 10/36] remove GDA_DOMAIN_PROPERTIES_PATH from constants.py --- src/mx_bluesky/common/parameters/constants.py | 6 ----- .../parameters/test_parameter_model.py | 22 ++++++++----------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/mx_bluesky/common/parameters/constants.py b/src/mx_bluesky/common/parameters/constants.py index 6dea822334..7fd726ece9 100644 --- a/src/mx_bluesky/common/parameters/constants.py +++ b/src/mx_bluesky/common/parameters/constants.py @@ -17,12 +17,6 @@ TEST_MODE = BEAMLINE == "test" ZEBRA_STATUS_TIMEOUT = 30 -GDA_DOMAIN_PROPERTIES_PATH = ( - "tests/test_data/test_domain_properties" - if TEST_MODE - else (f"/dls_sw/{BEAMLINE}/software/daq_configuration/domain/domain.properties") -) - @dataclass(frozen=True) class DocDescriptorNames: diff --git a/tests/unit_tests/hyperion/parameters/test_parameter_model.py b/tests/unit_tests/hyperion/parameters/test_parameter_model.py index b6ba9e24b1..2d30aa70e5 100644 --- a/tests/unit_tests/hyperion/parameters/test_parameter_model.py +++ b/tests/unit_tests/hyperion/parameters/test_parameter_model.py @@ -29,19 +29,15 @@ def always_use_beamline_i03(use_beamline_i03): ... @pytest.fixture def load_centre_collect_params_with_panda(tmp_path, request): - with patch( - "mx_bluesky.common.parameters.constants.GDA_DOMAIN_PROPERTIES_PATH", - new="tests/test_data/test_domain_properties_with_panda", - ): - params = raw_params_from_file( - "tests/test_data/parameter_json_files/good_test_load_centre_collect_params.json", - tmp_path, - ) - params["features"]["use_panda_for_gridscan"] = True - if params_dict := getattr(request, "param", {}): - for k, v in params_dict.items(): - params.setdefault("features", {})[k] = v - return LoadCentreCollect(**params) + params = raw_params_from_file( + "tests/test_data/parameter_json_files/good_test_load_centre_collect_params.json", + tmp_path, + ) + params["features"]["use_panda_for_gridscan"] = True + if params_dict := getattr(request, "param", {}): + for k, v in params_dict.items(): + params.setdefault("features", {})[k] = v + return LoadCentreCollect(**params) @pytest.fixture() From 8d897a4eeb28475ad5eae7eb768621b51a627069 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Fri, 10 Apr 2026 14:56:04 +0100 Subject: [PATCH 11/36] remove load_panda_yaml in favour of the copy in dodal --- src/mx_bluesky/common/device_setup_plans/setup_panda.py | 9 --------- .../hyperion/device_setup_plans/setup_panda.py | 2 +- .../common/device_setup_plans/test_setup_panda.py | 9 ++++----- 3 files changed, 5 insertions(+), 15 deletions(-) delete mode 100644 src/mx_bluesky/common/device_setup_plans/setup_panda.py diff --git a/src/mx_bluesky/common/device_setup_plans/setup_panda.py b/src/mx_bluesky/common/device_setup_plans/setup_panda.py deleted file mode 100644 index 685f5fd077..0000000000 --- a/src/mx_bluesky/common/device_setup_plans/setup_panda.py +++ /dev/null @@ -1,9 +0,0 @@ -from ophyd_async.core import YamlSettingsProvider -from ophyd_async.fastcs.panda import HDFPanda, apply_panda_settings -from ophyd_async.plan_stubs import retrieve_settings - - -def load_panda_from_yaml(yaml_directory: str, yaml_file_name: str, panda: HDFPanda): - provider = YamlSettingsProvider(yaml_directory) - settings = yield from retrieve_settings(provider, yaml_file_name, panda) - yield from apply_panda_settings(settings) diff --git a/src/mx_bluesky/hyperion/device_setup_plans/setup_panda.py b/src/mx_bluesky/hyperion/device_setup_plans/setup_panda.py index baae38ce95..4a05956d71 100644 --- a/src/mx_bluesky/hyperion/device_setup_plans/setup_panda.py +++ b/src/mx_bluesky/hyperion/device_setup_plans/setup_panda.py @@ -9,13 +9,13 @@ from dodal.common.types import UpdatingPathProvider from dodal.devices.fast_grid_scan import PandAGridScanParams from dodal.devices.smargon import Smargon +from dodal.plans.load_panda_yaml import load_panda_from_yaml from ophyd_async.fastcs.panda import ( HDFPanda, SeqTable, SeqTrigger, ) -from mx_bluesky.common.device_setup_plans.setup_panda import load_panda_from_yaml from mx_bluesky.common.parameters.constants import DeviceSettingsConstants from mx_bluesky.common.utils.log import LOGGER diff --git a/tests/unit_tests/common/device_setup_plans/test_setup_panda.py b/tests/unit_tests/common/device_setup_plans/test_setup_panda.py index da62f238cb..c6e11472f7 100644 --- a/tests/unit_tests/common/device_setup_plans/test_setup_panda.py +++ b/tests/unit_tests/common/device_setup_plans/test_setup_panda.py @@ -2,19 +2,18 @@ from bluesky.plan_stubs import null from bluesky.run_engine import RunEngine +from dodal.plans.load_panda_yaml import load_panda_from_yaml from ophyd_async.fastcs.panda import HDFPanda -from mx_bluesky.common.device_setup_plans.setup_panda import load_panda_from_yaml - def get_test_plan(*args): yield from null() return "retrieved_settings" -@patch("mx_bluesky.common.device_setup_plans.setup_panda.YamlSettingsProvider") -@patch("mx_bluesky.common.device_setup_plans.setup_panda.retrieve_settings") -@patch("mx_bluesky.common.device_setup_plans.setup_panda.apply_panda_settings") +@patch("dodal.plans.load_panda_yaml.YamlSettingsProvider") +@patch("dodal.plans.load_panda_yaml.retrieve_settings") +@patch("dodal.plans.load_panda_yaml.apply_panda_settings") def test_load_panda_from_yaml( mock_apply_panda_settings: MagicMock, mock_retrieve_settings: MagicMock, From 5da7099d28a1430d375303fe1b29e7d3ce2bf0ac Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Mon, 13 Apr 2026 11:45:10 +0100 Subject: [PATCH 12/36] Remove unused constants TEST_MODE, BEAMLINE, INSERTION_PREFIX from constants.py --- conftest.py | 2 -- src/mx_bluesky/hyperion/parameters/constants.py | 4 ---- 2 files changed, 6 deletions(-) diff --git a/conftest.py b/conftest.py index b48b008f9b..8cf90c9ddd 100644 --- a/conftest.py +++ b/conftest.py @@ -7,8 +7,6 @@ # Ensure that the blueapi entry point is not invoked by doctest as this will fail collect_ignore = ["src/mx_bluesky/hyperion/blueapi/plans.py"] -environ["HYPERION_TEST_MODE"] = "true" - pytest_plugins = ["dodal.testing.fixtures.run_engine"] diff --git a/src/mx_bluesky/hyperion/parameters/constants.py b/src/mx_bluesky/hyperion/parameters/constants.py index db10e25409..31e78dabe8 100644 --- a/src/mx_bluesky/hyperion/parameters/constants.py +++ b/src/mx_bluesky/hyperion/parameters/constants.py @@ -13,14 +13,10 @@ PlanNameConstants, ) -TEST_MODE = os.environ.get("HYPERION_TEST_MODE") - @dataclass(frozen=True) class I03Constants: - BEAMLINE = "BL03S" if TEST_MODE else "BL03I" DETECTOR = EIGER2_X_16M_SIZE - INSERTION_PREFIX = "SR03S" if TEST_MODE else "SR03I" OAV_CENTRING_FILE = OavConstants.OAV_CONFIG_JSON SHUTTER_TIME_S = 0.06 USE_GPU_RESULTS = True From 1828dd3ab3b3d44dac50233071d505e5c901c1ec Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Mon, 13 Apr 2026 12:09:37 +0100 Subject: [PATCH 13/36] remove det_dist_to_beam_converter_path from parameter model, fix unit tests, system tests still broken --- src/mx_bluesky/common/parameters/components.py | 3 --- src/mx_bluesky/common/parameters/constants.py | 6 +----- src/mx_bluesky/common/parameters/gridscan.py | 6 +----- src/mx_bluesky/common/parameters/rotation.py | 6 +----- .../hyperion_blueapi/test_load_centre_collect.json | 1 - .../good_test_grid_with_edge_detect_parameters.json | 1 - .../good_test_load_centre_collect_params.json | 1 - ...test_load_centre_collect_params_multi_rotation.json | 1 - .../good_test_multi_rotation_scan_parameters.json | 1 - .../good_test_one_multi_rotation_scan_parameters.json | 1 - ...test_one_multi_rotation_scan_parameters_nomove.json | 1 - .../parameter_json_files/good_test_parameters.json | 1 - ...od_test_pin_centre_then_xray_centre_parameters.json | 1 - .../good_test_robot_load_and_centre_params.json | 1 - .../good_test_rotation_scan_parameters.json | 1 - .../ispyb_gridscan_system_test_parameters.json | 1 - .../test_gridscan_param_defaults.json | 1 - tests/unit_tests/conftest.py | 6 ++++++ .../test_load_centre_collect_full_plan.py | 10 +++++----- 19 files changed, 14 insertions(+), 36 deletions(-) diff --git a/src/mx_bluesky/common/parameters/components.py b/src/mx_bluesky/common/parameters/components.py index 52d2317426..35ed5ebc98 100644 --- a/src/mx_bluesky/common/parameters/components.py +++ b/src/mx_bluesky/common/parameters/components.py @@ -153,9 +153,6 @@ class WithOptionalEnergyChange(BaseModel): class WithVisit(BaseModel): beamline: str = Field(default="BL03I", pattern=r"BL\d{2}[BIJS]") visit: str = Field(min_length=1) - det_dist_to_beam_converter_path: str = Field( - default=DetectorParamConstants.BEAM_XY_LUT_PATH - ) detector_distance_mm: float | None = Field(default=None, gt=0) diff --git a/src/mx_bluesky/common/parameters/constants.py b/src/mx_bluesky/common/parameters/constants.py index 7fd726ece9..f1f168d329 100644 --- a/src/mx_bluesky/common/parameters/constants.py +++ b/src/mx_bluesky/common/parameters/constants.py @@ -119,11 +119,7 @@ class RotationParamConstants: @dataclass(frozen=True) class DetectorParamConstants: - BEAM_XY_LUT_PATH = ( - "tests/test_data/test_det_dist_converter.txt" - if TEST_MODE - else f"/dls_sw/{BEAMLINE}/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" - ) + BEAM_XY_LUT_PATH = f"/dls_sw/{BEAMLINE}/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" DETECTOR = EIGER2_X_16M_SIZE diff --git a/src/mx_bluesky/common/parameters/gridscan.py b/src/mx_bluesky/common/parameters/gridscan.py index 15af489686..f33951a50e 100644 --- a/src/mx_bluesky/common/parameters/gridscan.py +++ b/src/mx_bluesky/common/parameters/gridscan.py @@ -78,10 +78,6 @@ def specified_grid_params(self) -> SpecifiedGrid | None: @property def detector_params(self): - self.det_dist_to_beam_converter_path = ( - self.det_dist_to_beam_converter_path - or DetectorParamConstants.BEAM_XY_LUT_PATH - ) optional_args = {} if self.run_number: optional_args["run_number"] = self.run_number @@ -100,7 +96,7 @@ def detector_params(self): num_images_per_trigger=1, num_triggers=self.num_images, use_roi_mode=self.use_roi_mode, - det_dist_to_beam_converter_path=self.det_dist_to_beam_converter_path, + det_dist_to_beam_converter_path=DetectorParamConstants.BEAM_XY_LUT_PATH, trigger_mode=self.trigger_mode, **optional_args, ) diff --git a/src/mx_bluesky/common/parameters/rotation.py b/src/mx_bluesky/common/parameters/rotation.py index 80a3965568..402a547864 100644 --- a/src/mx_bluesky/common/parameters/rotation.py +++ b/src/mx_bluesky/common/parameters/rotation.py @@ -67,10 +67,6 @@ class RotationExperiment(DiffractionExperiment): def _detector_params_impl( self, omega_start_deg: float, num_images_per_trigger: int, num_triggers: int ) -> DetectorParams: - self.det_dist_to_beam_converter_path = ( - self.det_dist_to_beam_converter_path - or DetectorParamConstants.BEAM_XY_LUT_PATH - ) optional_args = {} if self.run_number: optional_args["run_number"] = self.run_number @@ -88,7 +84,7 @@ def _detector_params_impl( num_images_per_trigger=num_images_per_trigger, num_triggers=num_triggers, use_roi_mode=False, - det_dist_to_beam_converter_path=self.det_dist_to_beam_converter_path, + det_dist_to_beam_converter_path=DetectorParamConstants.BEAM_XY_LUT_PATH, **optional_args, ) diff --git a/tests/test_data/hyperion_blueapi/test_load_centre_collect.json b/tests/test_data/hyperion_blueapi/test_load_centre_collect.json index 2f55a641a1..1b8e4a497c 100644 --- a/tests/test_data/hyperion_blueapi/test_load_centre_collect.json +++ b/tests/test_data/hyperion_blueapi/test_load_centre_collect.json @@ -4,7 +4,6 @@ "parameters": { "parameter_model_version": "5.0.0", "beamline": "BL03S", - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "visit": "cm31105-4", "detector_distance_mm": 255, "sample_id": 12345, diff --git a/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json b/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json index 2f196197d0..d4ff01b5af 100644 --- a/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_grid_with_edge_detect_parameters.json @@ -6,7 +6,6 @@ "run_number": 0, "sample_id": 123456, "use_roi_mode": false, - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "exposure_time_s": 0.1, "detector_distance_mm": 100.0, "omega_start_deg": 0.0, diff --git a/tests/test_data/parameter_json_files/good_test_load_centre_collect_params.json b/tests/test_data/parameter_json_files/good_test_load_centre_collect_params.json index bc77e8f286..f134738d7a 100644 --- a/tests/test_data/parameter_json_files/good_test_load_centre_collect_params.json +++ b/tests/test_data/parameter_json_files/good_test_load_centre_collect_params.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "beamline": "BL03S", - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "visit": "cm31105-4", "detector_distance_mm": 255, "sample_id": 12345, diff --git a/tests/test_data/parameter_json_files/good_test_load_centre_collect_params_multi_rotation.json b/tests/test_data/parameter_json_files/good_test_load_centre_collect_params_multi_rotation.json index a9b183631a..bb2092fca9 100644 --- a/tests/test_data/parameter_json_files/good_test_load_centre_collect_params_multi_rotation.json +++ b/tests/test_data/parameter_json_files/good_test_load_centre_collect_params_multi_rotation.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "beamline": "BL03S", - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "visit": "cm31105-4", "detector_distance_mm": 255, "sample_id": 12345, diff --git a/tests/test_data/parameter_json_files/good_test_multi_rotation_scan_parameters.json b/tests/test_data/parameter_json_files/good_test_multi_rotation_scan_parameters.json index 0061c4cfbe..3d0e3d4a2d 100644 --- a/tests/test_data/parameter_json_files/good_test_multi_rotation_scan_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_multi_rotation_scan_parameters.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "comment": "test", - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "storage_directory": "{tmp_data}/123456/", "detector_distance_mm": 100.0, "demand_energy_ev": 100, diff --git a/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters.json b/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters.json index 738a2af0c8..dbefcfee04 100644 --- a/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "comment": "test", - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "storage_directory": "{tmp_data}/123456/", "detector_distance_mm": 100.0, "demand_energy_ev": 100, diff --git a/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters_nomove.json b/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters_nomove.json index 61057c8b9d..498791f3b7 100644 --- a/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters_nomove.json +++ b/tests/test_data/parameter_json_files/good_test_one_multi_rotation_scan_parameters_nomove.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "comment": "test", - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "storage_directory": "{tmp_data}/123456/", "detector_distance_mm": 100.0, "demand_energy_ev": 100, diff --git a/tests/test_data/parameter_json_files/good_test_parameters.json b/tests/test_data/parameter_json_files/good_test_parameters.json index 745d28bd59..8886acdcf3 100644 --- a/tests/test_data/parameter_json_files/good_test_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_parameters.json @@ -2,7 +2,6 @@ "parameter_model_version": "5.0.0", "demand_energy_ev": 100, "comment": "test", - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "detector_distance_mm": 100.0, "visit": "cm31105-4", "exposure_time_s": 0.1, diff --git a/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json b/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json index d379d3a276..b7c860c34e 100644 --- a/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_pin_centre_then_xray_centre_parameters.json @@ -6,7 +6,6 @@ "run_number": 0, "sample_id": 123456, "use_roi_mode": false, - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "exposure_time_s": 0.1, "detector_distance_mm": 100.0, "omega_start_deg": 0.0, diff --git a/tests/test_data/parameter_json_files/good_test_robot_load_and_centre_params.json b/tests/test_data/parameter_json_files/good_test_robot_load_and_centre_params.json index e74fd38c31..3acb2b739c 100644 --- a/tests/test_data/parameter_json_files/good_test_robot_load_and_centre_params.json +++ b/tests/test_data/parameter_json_files/good_test_robot_load_and_centre_params.json @@ -6,7 +6,6 @@ "file_name": "file_name", "run_number": 0, "use_roi_mode": false, - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "omega_start_deg": 0, "transmission_frac": 1.0, "exposure_time_s": 0.004, diff --git a/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json b/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json index cdc7238f05..a85d796a39 100644 --- a/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json +++ b/tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json @@ -1,7 +1,6 @@ { "parameter_model_version": "5.0.0", "comment": "test", - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "storage_directory": "{tmp_data}/123456/", "detector_distance_mm": 100.0, "demand_energy_ev": 100, diff --git a/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json b/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json index 30efe5af4a..15b19b5bad 100644 --- a/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json +++ b/tests/test_data/parameter_json_files/ispyb_gridscan_system_test_parameters.json @@ -6,7 +6,6 @@ "run_number": 0, "sample_id": 123456, "use_roi_mode": false, - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "exposure_time_s": 0.12, "detector_distance_mm": 100.0, "omega_start_deg": 0.0, diff --git a/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json b/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json index 62f8d54f87..d273fe6bf7 100644 --- a/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json +++ b/tests/test_data/parameter_json_files/test_gridscan_param_defaults.json @@ -9,7 +9,6 @@ "run_number": 0, "comment": "Descriptive comment.", "use_roi_mode": false, - "det_dist_to_beam_converter_path": "tests/test_data/test_lookup_table.txt", "transmission_frac": 1.0, "x_steps": 40, "y_steps": 20, diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 82291f9771..7a0db24205 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -200,6 +200,12 @@ def create_gridscan_callbacks() -> tuple[ ) +@pytest.fixture(autouse=True) +def mock_default_beam_xy_lut(): + with patch("mx_bluesky.common.parameters.constants.DetectorParamConstants.BEAM_XY_LUT_PATH", "tests/test_data/test_lookup_table.txt"): + yield + + @pytest.fixture def use_beamline_t01(): """Beamline t01 is a beamline for unit tests that just contains a baton, so that diff --git a/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py b/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py index f9f3fad209..17f1dce497 100644 --- a/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py +++ b/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py @@ -279,7 +279,6 @@ def test_params_with_different_energy_for_rotation_gridscan_rejected(tmp_path): ["beamline", "i03"], ["visit", "cm12345"], ["detector_distance_mm", 123], - ["det_dist_to_beam_converter_path", "/foo/bar"], ], ) def test_params_with_unexpected_info_in_robot_load_rejected( @@ -308,7 +307,6 @@ def test_params_with_unexpected_info_in_robot_load_rejected( ["beamline", "i03"], ["visit", "cm12345"], ["detector_distance_mm", 123], - ["det_dist_to_beam_converter_path", "/foo/bar"], ], ) def test_params_with_unexpected_info_in_multi_rotation_scan_rejected( @@ -560,9 +558,11 @@ def test_load_centre_collect_moves_beamstop_into_place( ) msgs = assert_message_and_return_remaining( msgs, - predicate=lambda msg: msg.command == "set" - and msg.obj.name == "beamstop-selected_pos" - and msg.args[0] == BeamstopPositions.DATA_COLLECTION, + predicate=lambda msg: ( + msg.command == "set" + and msg.obj.name == "beamstop-selected_pos" + and msg.args[0] == BeamstopPositions.DATA_COLLECTION + ), ) msgs = assert_message_and_return_remaining( msgs, predicate=lambda msg: msg.command == "pin_tip_then_flyscan_plan" From 2b017abf5106cf45d4f43015dd728a2d16be4cc1 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Thu, 16 Apr 2026 12:06:26 +0100 Subject: [PATCH 14/36] Make ruff happy --- conftest.py | 1 - src/mx_bluesky/common/parameters/components.py | 1 - src/mx_bluesky/hyperion/parameters/constants.py | 2 -- tests/unit_tests/conftest.py | 5 ++++- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/conftest.py b/conftest.py index 8cf90c9ddd..5fe22e4ebd 100644 --- a/conftest.py +++ b/conftest.py @@ -1,5 +1,4 @@ from collections.abc import Iterator -from os import environ from unittest.mock import patch import pytest diff --git a/src/mx_bluesky/common/parameters/components.py b/src/mx_bluesky/common/parameters/components.py index 35ed5ebc98..052674efc1 100644 --- a/src/mx_bluesky/common/parameters/components.py +++ b/src/mx_bluesky/common/parameters/components.py @@ -25,7 +25,6 @@ from mx_bluesky.common.parameters.constants import ( USE_NUMTRACKER, - DetectorParamConstants, GridscanParamConstants, ) diff --git a/src/mx_bluesky/hyperion/parameters/constants.py b/src/mx_bluesky/hyperion/parameters/constants.py index 31e78dabe8..ba2fb065c5 100644 --- a/src/mx_bluesky/hyperion/parameters/constants.py +++ b/src/mx_bluesky/hyperion/parameters/constants.py @@ -1,5 +1,3 @@ -import os - from dodal.devices.detector import EIGER2_X_16M_SIZE from pydantic.dataclasses import dataclass diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 7a0db24205..8273e755e2 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -202,7 +202,10 @@ def create_gridscan_callbacks() -> tuple[ @pytest.fixture(autouse=True) def mock_default_beam_xy_lut(): - with patch("mx_bluesky.common.parameters.constants.DetectorParamConstants.BEAM_XY_LUT_PATH", "tests/test_data/test_lookup_table.txt"): + with patch( + "mx_bluesky.common.parameters.constants.DetectorParamConstants.BEAM_XY_LUT_PATH", + "tests/test_data/test_lookup_table.txt", + ): yield From b26c497617439999348694d68531cba52812e011 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 27 Mar 2026 16:01:19 +0000 Subject: [PATCH 15/36] Fix most system tests --- tests/system_tests/conftest.py | 14 ++++---------- .../hyperion/external_interaction/conftest.py | 5 +++++ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/system_tests/conftest.py b/tests/system_tests/conftest.py index 7857ea9b9e..0947c5adfb 100644 --- a/tests/system_tests/conftest.py +++ b/tests/system_tests/conftest.py @@ -10,6 +10,7 @@ from aiohttp import ClientResponse from daq_config_server import ConfigClient from dodal.beamlines import i03 +from dodal.common.beamlines.beamline_utils import set_config_client from dodal.devices.beamlines.i03.undulator_dcm import UndulatorDCM from dodal.devices.oav.oav_parameters import OAVConfigBeamCentre from ophyd_async.core import AsyncStatus, set_mock_value @@ -255,16 +256,9 @@ def config_client(): @pytest.fixture(autouse=True) -def patch_i03_config_client(): - """Fix default i03 beamline parameters to refer to a test file not the /dls_sw folder""" - with patch.dict( - "dodal.common.beamlines.config_client.BEAMLINE_CONFIG_SERVER_ENDPOINTS", - { - "i03": LOCAL_CONFIG_SERVER_URL, - "test": LOCAL_CONFIG_SERVER_URL, - }, - ): - yield +def patch_config_client(): + config_client = ConfigClient(LOCAL_CONFIG_SERVER_URL) + set_config_client(config_client) @pytest.fixture(autouse=True) diff --git a/tests/system_tests/hyperion/external_interaction/conftest.py b/tests/system_tests/hyperion/external_interaction/conftest.py index 1fc532e8e5..4b750ec9e3 100644 --- a/tests/system_tests/hyperion/external_interaction/conftest.py +++ b/tests/system_tests/hyperion/external_interaction/conftest.py @@ -199,6 +199,11 @@ def dummy_params(tmp_path): "tests/test_data/parameter_json_files/test_gridscan_param_defaults.json", tmp_path, ) + + # As system tests use real locally deployed config server + params_dict["det_dist_to_beam_converter_path"] = ( + "/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" + ) dummy_params = HyperionSpecifiedThreeDGridScan(**params_dict) dummy_params.visit = SimConstants.ST_VISIT dummy_params.sample_id = SimConstants.ST_SAMPLE_ID From f081a721abd04d3cf8e33a19e431635929025c07 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 27 Mar 2026 16:16:16 +0000 Subject: [PATCH 16/36] Fix some more --- src/mx_bluesky/common/parameters/constants.py | 7 -- .../hyperion/external_interaction/conftest.py | 73 +++++++++++++++++++ .../test_load_centre_collect_full_plan.py | 10 ++- .../external_interaction/test_nexgen.py | 3 + 4 files changed, 84 insertions(+), 9 deletions(-) diff --git a/src/mx_bluesky/common/parameters/constants.py b/src/mx_bluesky/common/parameters/constants.py index f1f168d329..3be3669d48 100644 --- a/src/mx_bluesky/common/parameters/constants.py +++ b/src/mx_bluesky/common/parameters/constants.py @@ -123,13 +123,6 @@ class DetectorParamConstants: DETECTOR = EIGER2_X_16M_SIZE -@dataclass(frozen=True) -class ExperimentParamConstants: - DETECTOR = DetectorParamConstants() - GRIDSCAN = GridscanParamConstants() - ROTATION = RotationParamConstants() - - @dataclass(frozen=True) class PlanGroupCheckpointConstants: # For places to synchronise / stop and wait in plans, use as bluesky group names diff --git a/tests/system_tests/hyperion/external_interaction/conftest.py b/tests/system_tests/hyperion/external_interaction/conftest.py index 4b750ec9e3..256abb0c26 100644 --- a/tests/system_tests/hyperion/external_interaction/conftest.py +++ b/tests/system_tests/hyperion/external_interaction/conftest.py @@ -358,6 +358,79 @@ async def mock_pin_tip_detect(): yield composite +@pytest.fixture +async def hyperion_flyscan_xrc_composite( + smargon: Smargon, + hyperion_fgs_params: HyperionSpecifiedThreeDGridScan, + attenuator, + xbpm_feedback, + synchrotron, + aperture_scatterguard, + zocalo, + dcm, + panda, + backlight, + s4_slit_gaps, + fast_grid_scan, + panda_fast_grid_scan, + beamsize, +) -> HyperionFlyScanXRayCentreComposite: + fake_composite = HyperionFlyScanXRayCentreComposite( + aperture_scatterguard=aperture_scatterguard, + attenuator=attenuator, + backlight=backlight, + dcm=dcm, + # We don't use the eiger fixture here because .unstage() is used in some tests + eiger=i03.eiger.build(mock=True), + zebra_fast_grid_scan=fast_grid_scan, + flux=i03.flux.build(connect_immediately=True, mock=True), + s4_slit_gaps=s4_slit_gaps, + gonio=smargon, + undulator=i03.undulator.build(connect_immediately=True, mock=True), + synchrotron=synchrotron, + xbpm_feedback=xbpm_feedback, + zebra=i03.zebra.build(connect_immediately=True, mock=True), + zocalo=zocalo, + panda=panda, + panda_fast_grid_scan=panda_fast_grid_scan, + robot=i03.robot.build(connect_immediately=True, mock=True), + sample_shutter=i03.sample_shutter.build(connect_immediately=True, mock=True), + beamsize=beamsize, + ) + + fake_composite.eiger.stage = MagicMock(side_effect=lambda: completed_status()) + # unstage should be mocked on a per-test basis because several rely on unstage + hyperion_fgs_params.det_dist_to_beam_converter_path = ( + "/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" + ) + fake_composite.eiger.set_detector_parameters(hyperion_fgs_params.detector_params) + fake_composite.eiger.stop_odin_when_all_frames_collected = MagicMock() + fake_composite.eiger.odin.check_and_wait_for_odin_state = lambda timeout: True + + test_result = { + "centre_of_mass": [6, 6, 6], + "max_voxel": [5, 5, 5], + "max_count": 123456, + "n_voxels": 321, + "total_count": 999999, + "bounding_box": [[3, 3, 3], [9, 9, 9]], + } + + @AsyncStatus.wrap + async def mock_complete(result): + await fake_composite.zocalo._put_results([result], {"dcid": 0, "dcgid": 0}) + + fake_composite.zocalo.trigger = MagicMock( + side_effect=partial(mock_complete, test_result) + ) # type: ignore + fake_composite.zocalo.timeout_s = 3 + set_mock_value(fake_composite.gonio.x.max_velocity, 10) + + set_mock_value(fake_composite.robot.barcode, "BARCODE") + + return fake_composite + + @pytest.fixture def fgs_composite_for_fake_zocalo( hyperion_flyscan_xrc_composite: HyperionFlyScanXRayCentreComposite, diff --git a/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py b/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py index a06451e9f4..ff1f88e01f 100644 --- a/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py +++ b/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py @@ -103,14 +103,20 @@ def load_centre_collect_params(tmp_path): ) json_dict["visit"] = SimConstants.ST_VISIT json_dict["sample_id"] = SimConstants.ST_SAMPLE_ID - return LoadCentreCollect(**json_dict) + return LoadCentreCollect( + **json_dict, + det_dist_to_beam_converter_path="/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt", + ) @pytest.fixture -def load_centre_collect_msp_params(load_centre_collect_params): +def load_centre_collect_msp_params(load_centre_collect_params: LoadCentreCollect): load_centre_collect_params.select_centres = TopNByMaxCountForEachSampleSelection( n=5 ) + load_centre_collect_params.det_dist_to_beam_converter_path = ( + "/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" + ) load_centre_collect_params.sample_id = SimConstants.ST_MSP_SAMPLE_IDS[0] load_centre_collect_params.robot_load_then_centre.sample_id = ( load_centre_collect_params.sample_id diff --git a/tests/system_tests/hyperion/external_interaction/test_nexgen.py b/tests/system_tests/hyperion/external_interaction/test_nexgen.py index 133f9f950c..e65874d535 100644 --- a/tests/system_tests/hyperion/external_interaction/test_nexgen.py +++ b/tests/system_tests/hyperion/external_interaction/test_nexgen.py @@ -41,6 +41,9 @@ def test_params(tmp_path): params.y_start_um = 0 params.z_start_um = 0 params.exposure_time_s = 0.004 + params.det_dist_to_beam_converter_path = ( + "/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" + ) return params From b733aea8e1b1112f4a8f326d6168a73547713fdf Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Tue, 31 Mar 2026 15:49:48 +0100 Subject: [PATCH 17/36] Disable soak tests until ophyd-async has the caching bug fixed in a release --- .../hyperion/external_interaction/test_baton_handler_soak.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/system_tests/hyperion/external_interaction/test_baton_handler_soak.py b/tests/system_tests/hyperion/external_interaction/test_baton_handler_soak.py index 5c3d3197b2..6206d666a2 100644 --- a/tests/system_tests/hyperion/external_interaction/test_baton_handler_soak.py +++ b/tests/system_tests/hyperion/external_interaction/test_baton_handler_soak.py @@ -67,6 +67,9 @@ def patched_func(*devices: Device, mock=False, timeout=1, force_reconnect=False) yield p +@pytest.mark.skip( + reason="Waiting for https://github.com/bluesky/ophyd-async/pull/1222 to make it into a stable release" +) @pytest.mark.parametrize( "i, patch_setup_devices", [[i, True] for i in range(1, 101)], From 3b8ad2473c5018cf43956b89b10c71edecbf636f Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Thu, 9 Apr 2026 15:23:04 +0100 Subject: [PATCH 18/36] Fix oav system tests --- tests/conftest.py | 52 ++++++++----------- tests/system_tests/conftest.py | 19 +++++-- .../test_load_centre_collect_full_plan.py | 1 + tests/unit_tests/conftest.py | 9 ++++ 4 files changed, 45 insertions(+), 36 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 517e2495ce..4e2f9cc6e9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -53,7 +53,6 @@ from dodal.devices.fast_grid_scan import FastGridScanCommon from dodal.devices.flux import Flux from dodal.devices.oav.oav_detector import OAV, OAVConfigBeamCentre -from dodal.devices.oav.oav_parameters import OAVParameters from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.devices.robot import BartRobot, SampleLocation from dodal.devices.s4_slit_gaps import S4SlitGaps @@ -578,29 +577,27 @@ def beamstop_phase1( beamline_parameters: dict[str, Any], sim_run_engine: RunEngineSimulator, ) -> Generator[Beamstop, Any, Any]: - with patch( - "dodal.beamlines.i03.BEAMLINE_PARAMETERS_PATH", - TEST_BEAMLINE_PARAMETERS, - ): - beamstop = i03.beamstop.build(connect_immediately=True, mock=True) - - set_mock_value(beamstop.x_mm.user_readback, 1.52) - set_mock_value(beamstop.y_mm.user_readback, 44.78) - set_mock_value(beamstop.z_mm.user_readback, 30.0) - - # sim_run_engine.add_read_handler_for( - # beamstop.selected_pos, BeamstopPositions.DATA_COLLECTION - # ) - # Can uncomment and remove below when https://github.com/bluesky/bluesky/issues/1906 is fixed - def locate_beamstop(_): - return {"readback": BeamstopPositions.DATA_COLLECTION} - - sim_run_engine.add_handler( - "locate", locate_beamstop, beamstop.selected_pos.name - ) - - yield beamstop - beamline_utils.clear_devices() + # with patch( + # "dodal.beamlines.i03.BEAMLINE_PARAMETERS_PATH", + # TEST_BEAMLINE_PARAMETERS, + # ): + beamstop = i03.beamstop.build(connect_immediately=True, mock=True) + + set_mock_value(beamstop.x_mm.user_readback, 1.52) + set_mock_value(beamstop.y_mm.user_readback, 44.78) + set_mock_value(beamstop.z_mm.user_readback, 30.0) + + # sim_run_engine.add_read_handler_for( + # beamstop.selected_pos, BeamstopPositions.DATA_COLLECTION + # ) + # Can uncomment and remove below when https://github.com/bluesky/bluesky/issues/1906 is fixed + def locate_beamstop(_): + return {"readback": BeamstopPositions.DATA_COLLECTION} + + sim_run_engine.add_handler("locate", locate_beamstop, beamstop.selected_pos.name) + + yield beamstop + beamline_utils.clear_devices() @pytest.fixture @@ -947,13 +944,6 @@ async def create_mock_signals(devices_and_signals: dict[Device, dict[str, Any]]) return panda -@pytest.fixture -def oav_parameters_for_rotation(test_config_files) -> OAVParameters: - return OAVParameters( - ConfigClient(""), oav_config_json=test_config_files["oav_config_json"] - ) - - async def async_status_done(): await asyncio.sleep(0) diff --git a/tests/system_tests/conftest.py b/tests/system_tests/conftest.py index 0947c5adfb..87407f0d77 100644 --- a/tests/system_tests/conftest.py +++ b/tests/system_tests/conftest.py @@ -10,9 +10,10 @@ from aiohttp import ClientResponse from daq_config_server import ConfigClient from dodal.beamlines import i03 +from dodal.beamlines.i03 import DISPLAY_CONFIG, ZOOM_PARAMS_FILE from dodal.common.beamlines.beamline_utils import set_config_client from dodal.devices.beamlines.i03.undulator_dcm import UndulatorDCM -from dodal.devices.oav.oav_parameters import OAVConfigBeamCentre +from dodal.devices.oav.oav_parameters import OAVConfigBeamCentre, OAVParameters from ophyd_async.core import AsyncStatus, set_mock_value from PIL import Image @@ -169,11 +170,11 @@ def next_oav_system_test_image(): @pytest.fixture -def oav_for_system_test(test_config_files, next_oav_system_test_image): +def oav_for_system_test(config_client: ConfigClient, next_oav_system_test_image): parameters = OAVConfigBeamCentre( - test_config_files["zoom_params_file"], - test_config_files["display_config"], - ConfigClient(""), + ZOOM_PARAMS_FILE, + DISPLAY_CONFIG, + config_client, ) oav = i03.oav.build(connect_immediately=True, mock=True, params=parameters) set_mock_value(oav.cam.array_size_x, 1024) @@ -282,3 +283,11 @@ def undulator_dcm(sim_run_engine, dcm, undulator) -> Generator[UndulatorDCM]: ) set_up_dcm(undulator_dcm.dcm_ref(), sim_run_engine) yield undulator_dcm + + +@pytest.fixture +def oav_parameters_for_rotation(config_client) -> OAVParameters: + return OAVParameters( + config_client, + oav_config_json="/dls_sw/i03/software/daq_configuration/json/OAVCentring.json", + ) diff --git a/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py b/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py index ff1f88e01f..8e8391ffe2 100644 --- a/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py +++ b/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py @@ -12,6 +12,7 @@ import pytest from bluesky.run_engine import RunEngine from daq_config_server import ConfigClient +from dodal.common.beamlines.beamline_utils import get_config_client from dodal.devices.beamsize.beamsize import BeamsizeBase from dodal.devices.oav.oav_parameters import OAVParameters from dodal.devices.oav.pin_image_recognition import PinTipDetection diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 8273e755e2..41076161d3 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -11,6 +11,7 @@ import pytest from _pytest.fixtures import FixtureRequest from bluesky.run_engine import RunEngine +from daq_config_server import ConfigClient from dodal.beamlines import i03 from dodal.devices.aperturescatterguard import ApertureScatterguard, ApertureValue from dodal.devices.backlight import Backlight @@ -23,6 +24,7 @@ from dodal.devices.hutch_shutter import ShutterState from dodal.devices.mx_phase1.beamstop import Beamstop from dodal.devices.oav.oav_detector import OAV +from dodal.devices.oav.oav_parameters import OAVParameters from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.devices.robot import BartRobot from dodal.devices.s4_slit_gaps import S4SlitGaps @@ -621,3 +623,10 @@ def patch_config_paths(monkeypatch): "dodal.beamlines.i04.BEAMLINE_PARAMETERS_PATH", TEST_BEAMLINE_PARAMETERS, ) + + +@pytest.fixture +def oav_parameters_for_rotation(test_config_files) -> OAVParameters: + return OAVParameters( + ConfigClient(""), oav_config_json=test_config_files["oav_config_json"] + ) From 1d3351b24533a1c32098f9b24adc4b4cb5e8de3a Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Thu, 9 Apr 2026 15:50:24 +0100 Subject: [PATCH 19/36] Use different OAV fixture in system tests --- tests/conftest.py | 84 +-------------- tests/system_tests/conftest.py | 83 +++++++++++++- .../external_interaction/test_nexgen.py | 6 +- tests/unit_tests/conftest.py | 101 +++++++++++++++++- 4 files changed, 182 insertions(+), 92 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4e2f9cc6e9..5b8b383695 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -47,18 +47,14 @@ from dodal.devices.beamlines.i03.dcm import DCM from dodal.devices.beamlines.i03.undulator_dcm import UndulatorDCM from dodal.devices.beamlines.i04.transfocator import Transfocator -from dodal.devices.beamsize.beamsize import BeamsizeBase from dodal.devices.detector.detector_motion import DetectorMotion from dodal.devices.eiger import EigerDetector from dodal.devices.fast_grid_scan import FastGridScanCommon -from dodal.devices.flux import Flux -from dodal.devices.oav.oav_detector import OAV, OAVConfigBeamCentre from dodal.devices.oav.pin_image_recognition import PinTipDetection -from dodal.devices.robot import BartRobot, SampleLocation -from dodal.devices.s4_slit_gaps import S4SlitGaps +from dodal.devices.robot import SampleLocation from dodal.devices.scintillator import Scintillator from dodal.devices.smargon import Smargon -from dodal.devices.synchrotron import Synchrotron, SynchrotronMode +from dodal.devices.synchrotron import SynchrotronMode from dodal.devices.thawer import Thawer from dodal.devices.undulator import UndulatorInKeV from dodal.devices.webcam import Webcam @@ -109,9 +105,6 @@ do_default_logging_setup, ) from mx_bluesky.hyperion.baton_handler import HYPERION_USER -from mx_bluesky.hyperion.experiment_plans.rotation_scan_plan import ( - RotationScanComposite, -) from mx_bluesky.hyperion.parameters.device_composites import ( HyperionFlyScanXRayCentreComposite, ) @@ -489,24 +482,6 @@ def synchrotron(): return synchrotron -@pytest.fixture -def oav(test_config_files): - parameters = OAVConfigBeamCentre( - test_config_files["zoom_params_file"], - test_config_files["display_config"], - ConfigClient(""), - ) - oav = i03.oav.build(mock=True, connect_immediately=True, params=parameters) - - set_mock_value(oav.zoom_controller.level, "5.0x") - set_mock_value(oav.grid_snapshot.x_size, 1024) - set_mock_value(oav.grid_snapshot.y_size, 768) - - oav.snapshot.trigger = MagicMock(side_effect=lambda: completed_status()) - oav.grid_snapshot.trigger = MagicMock(side_effect=lambda: completed_status()) - yield oav - - @pytest.fixture def flux(): return i03.flux.build(connect_immediately=True, mock=True) @@ -808,61 +783,6 @@ def fake_create_devices( return devices -@pytest.fixture() -def fake_create_rotation_devices( - beamstop_phase1: Beamstop, - eiger: EigerDetector, - smargon: Smargon, - zebra: Zebra, - detector_motion: DetectorMotion, - backlight: Backlight, - attenuator: BinaryFilterAttenuator, - flux: Flux, - undulator: UndulatorInKeV, - aperture_scatterguard: ApertureScatterguard, - synchrotron: Synchrotron, - s4_slit_gaps: S4SlitGaps, - dcm: DCM, - robot: BartRobot, - oav: OAV, - sample_shutter: ZebraShutter, - xbpm_feedback: XBPMFeedback, - thawer: Thawer, - beamsize: BeamsizeBase, - sim_run_engine: RunEngineSimulator, -): - set_mock_value(smargon.omega.max_velocity, 131) # type: ignore - undulator.set = MagicMock(side_effect=lambda _: completed_status()) - sim_run_engine.add_handler( - "read", - lambda msg: { - "gonio-wrapped_omega-offset_and_phase": {"value": np.array([0, 0])} - }, - "gonio-wrapped_omega", - ) - return RotationScanComposite( - attenuator=attenuator, - backlight=backlight, - beamsize=beamsize, - beamstop=beamstop_phase1, - dcm=dcm, - detector_motion=detector_motion, - eiger=eiger, - flux=flux, - gonio=smargon, - undulator=undulator, - aperture_scatterguard=aperture_scatterguard, - synchrotron=synchrotron, - s4_slit_gaps=s4_slit_gaps, - zebra=zebra, - robot=robot, - oav=oav, - sample_shutter=sample_shutter, - xbpm_feedback=xbpm_feedback, - thawer=thawer, - ) - - @pytest.fixture def zocalo(): zoc = i03.zocalo.build(connect_immediately=True, mock=True) diff --git a/tests/system_tests/conftest.py b/tests/system_tests/conftest.py index 87407f0d77..4d96065fda 100644 --- a/tests/system_tests/conftest.py +++ b/tests/system_tests/conftest.py @@ -11,12 +11,44 @@ from daq_config_server import ConfigClient from dodal.beamlines import i03 from dodal.beamlines.i03 import DISPLAY_CONFIG, ZOOM_PARAMS_FILE -from dodal.common.beamlines.beamline_utils import set_config_client +from dodal.common.beamlines.beamline_utils import ( + set_config_client, +) +from dodal.devices.aperturescatterguard import ( + ApertureScatterguard, +) +from dodal.devices.attenuator.attenuator import ( + BinaryFilterAttenuator, +) +from dodal.devices.backlight import Backlight +from dodal.devices.beamlines.i03 import Beamstop +from dodal.devices.beamlines.i03.dcm import DCM from dodal.devices.beamlines.i03.undulator_dcm import UndulatorDCM +from dodal.devices.beamsize.beamsize import BeamsizeBase +from dodal.devices.detector.detector_motion import DetectorMotion +from dodal.devices.eiger import EigerDetector +from dodal.devices.flux import Flux +from dodal.devices.oav.oav_detector import OAV from dodal.devices.oav.oav_parameters import OAVConfigBeamCentre, OAVParameters -from ophyd_async.core import AsyncStatus, set_mock_value +from dodal.devices.robot import BartRobot +from dodal.devices.s4_slit_gaps import S4SlitGaps +from dodal.devices.smargon import Smargon +from dodal.devices.synchrotron import Synchrotron +from dodal.devices.thawer import Thawer +from dodal.devices.undulator import UndulatorInKeV +from dodal.devices.xbpm_feedback import XBPMFeedback +from dodal.devices.zebra.zebra import Zebra +from dodal.devices.zebra.zebra_controlled_shutter import ZebraShutter +from ophyd_async.core import ( + AsyncStatus, + completed_status, + set_mock_value, +) from PIL import Image +from mx_bluesky.hyperion.experiment_plans.rotation_scan_plan import ( + RotationScanComposite, +) from tests.conftest import set_up_dcm # Map all the case-sensitive column names from their normalised versions @@ -291,3 +323,50 @@ def oav_parameters_for_rotation(config_client) -> OAVParameters: config_client, oav_config_json="/dls_sw/i03/software/daq_configuration/json/OAVCentring.json", ) + + +@pytest.fixture() +def system_tests_rotation_devices( + beamstop_phase1: Beamstop, + eiger: EigerDetector, + smargon: Smargon, + zebra: Zebra, + detector_motion: DetectorMotion, + backlight: Backlight, + attenuator: BinaryFilterAttenuator, + flux: Flux, + undulator: UndulatorInKeV, + aperture_scatterguard: ApertureScatterguard, + synchrotron: Synchrotron, + s4_slit_gaps: S4SlitGaps, + dcm: DCM, + robot: BartRobot, + oav_for_system_test: OAV, + sample_shutter: ZebraShutter, + xbpm_feedback: XBPMFeedback, + thawer: Thawer, + beamsize: BeamsizeBase, +): + set_mock_value(smargon.omega.max_velocity, 131) + undulator.set = MagicMock(side_effect=lambda _: completed_status()) + return RotationScanComposite( + attenuator=attenuator, + backlight=backlight, + beamsize=beamsize, + beamstop=beamstop_phase1, + dcm=dcm, + detector_motion=detector_motion, + eiger=eiger, + flux=flux, + gonio=smargon, + undulator=undulator, + aperture_scatterguard=aperture_scatterguard, + synchrotron=synchrotron, + s4_slit_gaps=s4_slit_gaps, + zebra=zebra, + robot=robot, + oav=oav_for_system_test, + sample_shutter=sample_shutter, + xbpm_feedback=xbpm_feedback, + thawer=thawer, + ) diff --git a/tests/system_tests/hyperion/external_interaction/test_nexgen.py b/tests/system_tests/hyperion/external_interaction/test_nexgen.py index e65874d535..e30bdbc12a 100644 --- a/tests/system_tests/hyperion/external_interaction/test_nexgen.py +++ b/tests/system_tests/hyperion/external_interaction/test_nexgen.py @@ -72,7 +72,7 @@ def test_params(tmp_path): def test_rotation_nexgen( test_params: SingleRotationScan, tmpdir, - fake_create_rotation_devices: RotationScanComposite, + system_tests_rotation_devices: RotationScanComposite, test_data_directory, prefix, reference_file, @@ -87,11 +87,11 @@ def test_rotation_nexgen( f"{test_data_directory}/{meta_file}", f"{tmpdir}/{prefix}_{run_number}_meta.h5" ) - fake_create_rotation_devices.eiger.bit_depth.sim_put(32) # type: ignore + system_tests_rotation_devices.eiger.bit_depth.sim_put(32) # type: ignore run_engine( _fake_rotation_scan( - test_params, RotationNexusFileCallback(), fake_create_rotation_devices + test_params, RotationNexusFileCallback(), system_tests_rotation_devices ) ) diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 41076161d3..c6e1c5b30d 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -11,26 +11,41 @@ import pytest from _pytest.fixtures import FixtureRequest from bluesky.run_engine import RunEngine +from bluesky.simulators import RunEngineSimulator from daq_config_server import ConfigClient from dodal.beamlines import i03 -from dodal.devices.aperturescatterguard import ApertureScatterguard, ApertureValue +from dodal.devices.aperturescatterguard import ( + ApertureScatterguard, + ApertureValue, +) +from dodal.devices.attenuator.attenuator import ( + BinaryFilterAttenuator, +) from dodal.devices.backlight import Backlight +from dodal.devices.beamlines.i03 import Beamstop +from dodal.devices.beamlines.i03.dcm import DCM from dodal.devices.beamlines.i24.commissioning_jungfrau import CommissioningJungfrau from dodal.devices.beamsize.beamsize import BeamsizeBase from dodal.devices.detector.detector_motion import DetectorMotion from dodal.devices.eiger import EigerDetector -from dodal.devices.fast_grid_scan import PandAFastGridScan, ZebraFastGridScanThreeD +from dodal.devices.fast_grid_scan import ( + PandAFastGridScan, + ZebraFastGridScanThreeD, +) from dodal.devices.flux import Flux from dodal.devices.hutch_shutter import ShutterState -from dodal.devices.mx_phase1.beamstop import Beamstop from dodal.devices.oav.oav_detector import OAV -from dodal.devices.oav.oav_parameters import OAVParameters +from dodal.devices.oav.oav_parameters import OAVConfigBeamCentre, OAVParameters from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.devices.robot import BartRobot from dodal.devices.s4_slit_gaps import S4SlitGaps from dodal.devices.smargon import Smargon from dodal.devices.synchrotron import Synchrotron, SynchrotronMode -from dodal.devices.zebra.zebra_controlled_shutter import ZebraShutterState +from dodal.devices.thawer import Thawer +from dodal.devices.undulator import UndulatorInKeV +from dodal.devices.xbpm_feedback import XBPMFeedback +from dodal.devices.zebra.zebra import Zebra +from dodal.devices.zebra.zebra_controlled_shutter import ZebraShutter, ZebraShutterState from dodal.devices.zocalo import ZocaloResults from event_model import RunStart from event_model.documents import Event @@ -76,6 +91,9 @@ GridDetectThenXRayCentreComposite, ) from mx_bluesky.common.parameters.gridscan import GridCommon, SpecifiedThreeDGridScan +from mx_bluesky.hyperion.experiment_plans.rotation_scan_plan import ( + RotationScanComposite, +) from mx_bluesky.hyperion.parameters.device_composites import ( HyperionGridDetectThenXRayCentreComposite, ) @@ -630,3 +648,76 @@ def oav_parameters_for_rotation(test_config_files) -> OAVParameters: return OAVParameters( ConfigClient(""), oav_config_json=test_config_files["oav_config_json"] ) + + +@pytest.fixture +def oav(test_config_files): + parameters = OAVConfigBeamCentre( + test_config_files["zoom_params_file"], + test_config_files["display_config"], + ConfigClient(""), + ) + oav = i03.oav.build(mock=True, connect_immediately=True, params=parameters) + + set_mock_value(oav.zoom_controller.level, "5.0x") + set_mock_value(oav.grid_snapshot.x_size, 1024) + set_mock_value(oav.grid_snapshot.y_size, 768) + + oav.snapshot.trigger = MagicMock(side_effect=lambda: completed_status()) + oav.grid_snapshot.trigger = MagicMock(side_effect=lambda: completed_status()) + yield oav + + +@pytest.fixture() +def fake_create_rotation_devices( + beamstop_phase1: Beamstop, + eiger: EigerDetector, + smargon: Smargon, + zebra: Zebra, + detector_motion: DetectorMotion, + backlight: Backlight, + attenuator: BinaryFilterAttenuator, + flux: Flux, + undulator: UndulatorInKeV, + aperture_scatterguard: ApertureScatterguard, + synchrotron: Synchrotron, + s4_slit_gaps: S4SlitGaps, + dcm: DCM, + robot: BartRobot, + oav: OAV, + sample_shutter: ZebraShutter, + xbpm_feedback: XBPMFeedback, + thawer: Thawer, + beamsize: BeamsizeBase, + sim_run_engine: RunEngineSimulator, +): + set_mock_value(smargon.omega.max_velocity, 131) + undulator.set = MagicMock(side_effect=lambda _: completed_status()) + sim_run_engine.add_handler( + "read", + lambda msg: { + "gonio-wrapped_omega-offset_and_phase": {"value": np.array([0, 0])} + }, + "gonio-wrapped_omega", + ) + return RotationScanComposite( + attenuator=attenuator, + backlight=backlight, + beamsize=beamsize, + beamstop=beamstop_phase1, + dcm=dcm, + detector_motion=detector_motion, + eiger=eiger, + flux=flux, + gonio=smargon, + undulator=undulator, + aperture_scatterguard=aperture_scatterguard, + synchrotron=synchrotron, + s4_slit_gaps=s4_slit_gaps, + zebra=zebra, + robot=robot, + oav=oav, + sample_shutter=sample_shutter, + xbpm_feedback=xbpm_feedback, + thawer=thawer, + ) From ea2de3147e9d863781c5527f16c10f4abeb8cc4a Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Thu, 9 Apr 2026 16:54:09 +0100 Subject: [PATCH 20/36] Simplify external callbacks fixtures for system tests, system test config server fixture now sets the URL env var --- tests/system_tests/conftest.py | 23 +++--- .../hyperion/external_interaction/conftest.py | 77 +++---------------- 2 files changed, 20 insertions(+), 80 deletions(-) diff --git a/tests/system_tests/conftest.py b/tests/system_tests/conftest.py index 4d96065fda..37acaf543f 100644 --- a/tests/system_tests/conftest.py +++ b/tests/system_tests/conftest.py @@ -11,9 +11,6 @@ from daq_config_server import ConfigClient from dodal.beamlines import i03 from dodal.beamlines.i03 import DISPLAY_CONFIG, ZOOM_PARAMS_FILE -from dodal.common.beamlines.beamline_utils import ( - set_config_client, -) from dodal.devices.aperturescatterguard import ( ApertureScatterguard, ) @@ -161,7 +158,7 @@ ] } -LOCAL_CONFIG_SERVER_URL = "http://0.0.0.0:8555" +LOCAL_CONFIG_SERVER_URL = "http://127.0.0.1:8555" def _system_test_env_error_message(env_var: str): @@ -278,20 +275,20 @@ def compare_comment( assert truncated_comment == expected_comment -@pytest.fixture -def config_client(): +@pytest.fixture(autouse=True) +def config_client(monkeypatch): # Connects to real config server hosted locally # Test files are stored in the hyperion-system-tests repo under ./daq_config_server/config/ # They have been mounted to match the paths in /dls_sw/i03/ so that the whitelist # and file converter map behave as expected with no mocking needed. # https://gitlab.diamond.ac.uk/MX-GDA/hyperion-system-testing/-/tree/add_daq_config_server/daq-config-server/config/?ref_type=heads - return ConfigClient(url=LOCAL_CONFIG_SERVER_URL) - - -@pytest.fixture(autouse=True) -def patch_config_client(): - config_client = ConfigClient(LOCAL_CONFIG_SERVER_URL) - set_config_client(config_client) + config_server_url = os.getenv("CONFIG_SERVER_URL", default=LOCAL_CONFIG_SERVER_URL) + config_client = ConfigClient(url=config_server_url) + monkeypatch.setattr( + "dodal.common.beamlines.beamline_utils.CONFIG_CLIENT", config_client + ) + monkeypatch.setenv("CONFIG_SERVER_URL", config_server_url) + return config_client @pytest.fixture(autouse=True) diff --git a/tests/system_tests/hyperion/external_interaction/conftest.py b/tests/system_tests/hyperion/external_interaction/conftest.py index 256abb0c26..118733c0c5 100644 --- a/tests/system_tests/hyperion/external_interaction/conftest.py +++ b/tests/system_tests/hyperion/external_interaction/conftest.py @@ -359,76 +359,19 @@ async def mock_pin_tip_detect(): @pytest.fixture -async def hyperion_flyscan_xrc_composite( - smargon: Smargon, - hyperion_fgs_params: HyperionSpecifiedThreeDGridScan, - attenuator, - xbpm_feedback, - synchrotron, - aperture_scatterguard, - zocalo, - dcm, - panda, - backlight, - s4_slit_gaps, - fast_grid_scan, - panda_fast_grid_scan, - beamsize, -) -> HyperionFlyScanXRayCentreComposite: - fake_composite = HyperionFlyScanXRayCentreComposite( - aperture_scatterguard=aperture_scatterguard, - attenuator=attenuator, - backlight=backlight, - dcm=dcm, - # We don't use the eiger fixture here because .unstage() is used in some tests - eiger=i03.eiger.build(mock=True), - zebra_fast_grid_scan=fast_grid_scan, - flux=i03.flux.build(connect_immediately=True, mock=True), - s4_slit_gaps=s4_slit_gaps, - gonio=smargon, - undulator=i03.undulator.build(connect_immediately=True, mock=True), - synchrotron=synchrotron, - xbpm_feedback=xbpm_feedback, - zebra=i03.zebra.build(connect_immediately=True, mock=True), - zocalo=zocalo, - panda=panda, - panda_fast_grid_scan=panda_fast_grid_scan, - robot=i03.robot.build(connect_immediately=True, mock=True), - sample_shutter=i03.sample_shutter.build(connect_immediately=True, mock=True), - beamsize=beamsize, +def hyperion_fgs_params(tmp_path): + params = HyperionSpecifiedThreeDGridScan( + **( + raw_params_from_file( + "tests/test_data/parameter_json_files/good_test_parameters.json", + tmp_path, + ) + ) ) - - fake_composite.eiger.stage = MagicMock(side_effect=lambda: completed_status()) - # unstage should be mocked on a per-test basis because several rely on unstage - hyperion_fgs_params.det_dist_to_beam_converter_path = ( + params.det_dist_to_beam_converter_path = ( "/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" ) - fake_composite.eiger.set_detector_parameters(hyperion_fgs_params.detector_params) - fake_composite.eiger.stop_odin_when_all_frames_collected = MagicMock() - fake_composite.eiger.odin.check_and_wait_for_odin_state = lambda timeout: True - - test_result = { - "centre_of_mass": [6, 6, 6], - "max_voxel": [5, 5, 5], - "max_count": 123456, - "n_voxels": 321, - "total_count": 999999, - "bounding_box": [[3, 3, 3], [9, 9, 9]], - } - - @AsyncStatus.wrap - async def mock_complete(result): - await fake_composite.zocalo._put_results([result], {"dcid": 0, "dcgid": 0}) - - fake_composite.zocalo.trigger = MagicMock( - side_effect=partial(mock_complete, test_result) - ) # type: ignore - fake_composite.zocalo.timeout_s = 3 - set_mock_value(fake_composite.gonio.x.max_velocity, 10) - - set_mock_value(fake_composite.robot.barcode, "BARCODE") - - return fake_composite + return params @pytest.fixture From 739ff535b490bd039d3c9ce219fb66fd7aa6793b Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Mon, 13 Apr 2026 12:02:34 +0100 Subject: [PATCH 21/36] Additional patch of load_panda_yaml.retrieve_settings for unit tests --- tests/conftest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5b8b383695..46d3cab816 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -861,7 +861,9 @@ async def create_mock_signals(devices_and_signals: dict[Device, dict[str, Any]]) DatasetTable(name=["name"], dtype=[PandaHdf5DatasetType.FLOAT_64]), ) - return panda + with (patch("dodal.plans.load_panda_yaml.retrieve_settings"), + patch("dodal.plans.load_panda_yaml.apply_panda_settings")): + yield panda async def async_status_done(): From e4945f8c122850d41975282afbdc1094cf08d19b Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Mon, 13 Apr 2026 12:16:54 +0100 Subject: [PATCH 22/36] Extend the timeout in murko callback tests due to random failures --- tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py b/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py index cb81c984fe..b4ef671b9b 100644 --- a/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py +++ b/tests/unit_tests/beamlines/i04/callbacks/test_murko_callback.py @@ -291,6 +291,7 @@ def test_if_redis_connection_fails_then_there_is_no_error( callback.stop(doc) # type: ignore +@pytest.mark.timeout(10) def test_warning_is_logged_if_redis_connection_fails(caplog): callback = MurkoCallback("", "") doc = {} From 7fcf0ca7e20e797f34a38493de7c4a3804d481dd Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Wed, 15 Apr 2026 15:54:59 +0100 Subject: [PATCH 23/36] mock panda_fast_grid_scan trigger --- tests/conftest.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 46d3cab816..25f88844f4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -880,7 +880,18 @@ def panda_fast_grid_scan(): scan = i03.panda_fast_grid_scan.build(connect_immediately=True, mock=True) for signal in [scan.x_scan_valid, scan.y_scan_valid, scan.z_scan_valid]: set_mock_value(signal, 1) - return scan + + def mock_trigger(*args, **kwargs): + set_mock_value(scan.status, 1) + + get_mock_put(scan.run_cmd).side_effect = mock_trigger + + def mock_complete(): + set_mock_value(scan.status, 0) + return completed_status() + + with patch.object(scan, "complete", side_effect=mock_complete): + yield scan @pytest.fixture From 33e3da1d694c9cb277c9af9a34cfd165775dbc75 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Wed, 15 Apr 2026 15:59:27 +0100 Subject: [PATCH 24/36] Config file-related system test failures now fixed --- pyproject.toml | 5 +- tests/conftest.py | 16 ++----- .../test_daq_config_server.py | 7 --- tests/system_tests/conftest.py | 48 +++++++++++++------ .../hyperion/external_interaction/conftest.py | 8 +--- .../test_load_centre_collect_full_plan.py | 23 ++++++--- .../external_interaction/test_nexgen.py | 5 +- 7 files changed, 59 insertions(+), 53 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 6ade793d98..d8d187775d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -238,7 +238,7 @@ commands = [ ], extend = true }, ], ] -set_env = { DODAL_TEST_MODE = "true", BEAMLINE = "test" } +set_env = { DODAL_TEST_MODE = "true", BEAMLINE = "i03" } # Runs the system tests locally @@ -260,7 +260,8 @@ DOCKER = "podman" DODAL_TEST_MODE = "true" ISPYB_CONFIG_PATH = "{toxinidir}/tests/test_data/ispyb-test-credentials.cfg" ZOCALO_CONFIG = "{toxinidir}/tests/test_data/zocalo-test-configuration.yaml" -BEAMLINE = "test" +BEAMLINE = "i03" +CONFIG_SERVER_URL = "http://localhost:8555" [tool.tox.env.docs] description = "Run docs build with clean environment" diff --git a/tests/conftest.py b/tests/conftest.py index 25f88844f4..a9ee8f5006 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,13 +18,10 @@ import pytest from bluesky.simulators import RunEngineSimulator from bluesky.utils import Msg, MsgGenerator -from daq_config_server import ConfigClient from dodal.beamlines import aithre, i03 from dodal.common.beamlines import beamline_utils from dodal.common.beamlines.beamline_utils import ( - clear_config_client, clear_devices, - set_config_client, ) from dodal.common.beamlines.commissioning_mode import set_commissioning_signal from dodal.devices.aperturescatterguard import ( @@ -861,8 +858,10 @@ async def create_mock_signals(devices_and_signals: dict[Device, dict[str, Any]]) DatasetTable(name=["name"], dtype=[PandaHdf5DatasetType.FLOAT_64]), ) - with (patch("dodal.plans.load_panda_yaml.retrieve_settings"), - patch("dodal.plans.load_panda_yaml.apply_panda_settings")): + with ( + patch("dodal.plans.load_panda_yaml.retrieve_settings"), + patch("dodal.plans.load_panda_yaml.apply_panda_settings"), + ): yield panda @@ -1695,10 +1694,3 @@ def mock_alert_service(): @pytest.fixture() def patch_beamline_env_variable(monkeypatch): monkeypatch.setenv("BEAMLINE", "test") - - -@pytest.fixture(autouse=True) -def reset_config_client(): - set_config_client(ConfigClient("")) - yield - clear_config_client() diff --git a/tests/system_tests/common/daq_config_server/test_daq_config_server.py b/tests/system_tests/common/daq_config_server/test_daq_config_server.py index bc9eff6980..2da974aa1c 100644 --- a/tests/system_tests/common/daq_config_server/test_daq_config_server.py +++ b/tests/system_tests/common/daq_config_server/test_daq_config_server.py @@ -4,12 +4,10 @@ HyperionFeatureSettings, ) from dodal.common.beamlines.beamline_parameters import BEAMLINE_PARAMETER_PATHS -from dodal.common.beamlines.beamline_utils import get_config_client from mx_bluesky.hyperion.external_interaction.config_server import ( GDA_DOMAIN_PROPERTIES_PATH, ) -from tests.system_tests.conftest import LOCAL_CONFIG_SERVER_URL @pytest.mark.system_test @@ -35,8 +33,3 @@ def test_get_domain_proeprties_from_real_config_server( config_client.get_file_contents( filepath, desired_return_type=HyperionFeatureSettings ) - - -@pytest.mark.system_test -def test_local_config_server_being_used_with_get_config_client(): - assert get_config_client()._url == LOCAL_CONFIG_SERVER_URL diff --git a/tests/system_tests/conftest.py b/tests/system_tests/conftest.py index 37acaf543f..0feec5f037 100644 --- a/tests/system_tests/conftest.py +++ b/tests/system_tests/conftest.py @@ -10,7 +10,7 @@ from aiohttp import ClientResponse from daq_config_server import ConfigClient from dodal.beamlines import i03 -from dodal.beamlines.i03 import DISPLAY_CONFIG, ZOOM_PARAMS_FILE +from dodal.common.beamlines.beamline_utils import clear_config_client from dodal.devices.aperturescatterguard import ( ApertureScatterguard, ) @@ -185,6 +185,16 @@ def zocalo_env(): yield zocalo_config +@pytest.fixture(autouse=True) +def test_beamline_parameters(): + """Fix default test beamline parameters to refer to a test file not the /dls_sw folder""" + with patch.dict( + "dodal.common.beamlines.beamline_parameters.BEAMLINE_PARAMETER_PATHS", + {"test": "/dls_sw/i03/software/daq_configuration/domain/beamlineParameters"}, + ) as params: + yield params + + @pytest.fixture def undulator_for_system_test(undulator): set_mock_value(undulator.current_gap, 1.11) @@ -198,11 +208,23 @@ def next_oav_system_test_image(): ) +@pytest.fixture() +def test_config_files(): + """Override the default system test config""" + return { + "zoom_params_file": "/dls_sw/i03/software/gda/configurations/i03-config/xml/jCameraManZoomLevels.xml", + "oav_config_json": "/dls_sw/i03/software/daq_configuration/json/OAVCentring_hyperion.json", + "display_config": "/dls_sw/i03/software/gda_versions/var/display.configuration", + } + + @pytest.fixture -def oav_for_system_test(config_client: ConfigClient, next_oav_system_test_image): +def oav_for_system_test( + config_client: ConfigClient, next_oav_system_test_image, test_config_files +): parameters = OAVConfigBeamCentre( - ZOOM_PARAMS_FILE, - DISPLAY_CONFIG, + test_config_files["zoom_params_file"], + test_config_files["display_config"], config_client, ) oav = i03.oav.build(connect_immediately=True, mock=True, params=parameters) @@ -276,19 +298,17 @@ def compare_comment( @pytest.fixture(autouse=True) -def config_client(monkeypatch): - # Connects to real config server hosted locally +def config_client(): + # The system tests connect to a real config server hosted in a container + # The end point is determined by the CONFIG_SERVER_URL environment variable # Test files are stored in the hyperion-system-tests repo under ./daq_config_server/config/ # They have been mounted to match the paths in /dls_sw/i03/ so that the whitelist # and file converter map behave as expected with no mocking needed. # https://gitlab.diamond.ac.uk/MX-GDA/hyperion-system-testing/-/tree/add_daq_config_server/daq-config-server/config/?ref_type=heads - config_server_url = os.getenv("CONFIG_SERVER_URL", default=LOCAL_CONFIG_SERVER_URL) - config_client = ConfigClient(url=config_server_url) - monkeypatch.setattr( - "dodal.common.beamlines.beamline_utils.CONFIG_CLIENT", config_client - ) - monkeypatch.setenv("CONFIG_SERVER_URL", config_server_url) - return config_client + clear_config_client() + i03.config_client.cache_clear() + print("CONFIG_CLIENT CLEARED") + return i03.config_client() @pytest.fixture(autouse=True) @@ -318,7 +338,7 @@ def undulator_dcm(sim_run_engine, dcm, undulator) -> Generator[UndulatorDCM]: def oav_parameters_for_rotation(config_client) -> OAVParameters: return OAVParameters( config_client, - oav_config_json="/dls_sw/i03/software/daq_configuration/json/OAVCentring.json", + oav_config_json="/dls_sw/i03/software/daq_configuration/json/OAVCentring_hyperion.json", ) diff --git a/tests/system_tests/hyperion/external_interaction/conftest.py b/tests/system_tests/hyperion/external_interaction/conftest.py index 118733c0c5..3bfdfa100b 100644 --- a/tests/system_tests/hyperion/external_interaction/conftest.py +++ b/tests/system_tests/hyperion/external_interaction/conftest.py @@ -200,10 +200,6 @@ def dummy_params(tmp_path): tmp_path, ) - # As system tests use real locally deployed config server - params_dict["det_dist_to_beam_converter_path"] = ( - "/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" - ) dummy_params = HyperionSpecifiedThreeDGridScan(**params_dict) dummy_params.visit = SimConstants.ST_VISIT dummy_params.sample_id = SimConstants.ST_SAMPLE_ID @@ -368,14 +364,12 @@ def hyperion_fgs_params(tmp_path): ) ) ) - params.det_dist_to_beam_converter_path = ( - "/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" - ) return params @pytest.fixture def fgs_composite_for_fake_zocalo( + config_client, hyperion_flyscan_xrc_composite: HyperionFlyScanXRayCentreComposite, zocalo_for_fake_zocalo: ZocaloResults, ) -> HyperionFlyScanXRayCentreComposite: diff --git a/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py b/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py index 8e8391ffe2..bedaf5a775 100644 --- a/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py +++ b/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py @@ -104,10 +104,13 @@ def load_centre_collect_params(tmp_path): ) json_dict["visit"] = SimConstants.ST_VISIT json_dict["sample_id"] = SimConstants.ST_SAMPLE_ID - return LoadCentreCollect( - **json_dict, - det_dist_to_beam_converter_path="/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt", - ) + with patch( + "mx_bluesky.common.parameters.gridscan.DetectorParamConstants.BEAM_XY_LUT_PATH", + "/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter_load_centre_collect.txt", + ): + yield LoadCentreCollect( + **json_dict, + ) @pytest.fixture @@ -115,9 +118,6 @@ def load_centre_collect_msp_params(load_centre_collect_params: LoadCentreCollect load_centre_collect_params.select_centres = TopNByMaxCountForEachSampleSelection( n=5 ) - load_centre_collect_params.det_dist_to_beam_converter_path = ( - "/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" - ) load_centre_collect_params.sample_id = SimConstants.ST_MSP_SAMPLE_IDS[0] load_centre_collect_params.robot_load_then_centre.sample_id = ( load_centre_collect_params.sample_id @@ -1142,6 +1142,15 @@ def grid_detect_for_snapshot_generation(): class TestGenerateSnapshot: + @pytest.fixture() + def test_config_files(self): + """Override the default system test config""" + return { + "zoom_params_file": "/dls_sw/i03/software/gda/configurations/i03-config/xml/jCameraManZoomLevels.xml", + "oav_config_json": "/dls_sw/i03/software/daq_configuration/json/OAVCentring_snapshot.json", + "display_config": "/dls_sw/i03/software/gda_versions/var/snapshot_display.configuration", + } + @pytest.mark.system_test def test_load_centre_collect_generate_rotation_snapshots( self, diff --git a/tests/system_tests/hyperion/external_interaction/test_nexgen.py b/tests/system_tests/hyperion/external_interaction/test_nexgen.py index e30bdbc12a..8bff181cb3 100644 --- a/tests/system_tests/hyperion/external_interaction/test_nexgen.py +++ b/tests/system_tests/hyperion/external_interaction/test_nexgen.py @@ -28,7 +28,7 @@ @pytest.fixture -def test_params(tmp_path): +def test_params(tmp_path, config_client): param_dict = raw_params_from_file( "tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json", tmp_path, @@ -41,9 +41,6 @@ def test_params(tmp_path): params.y_start_um = 0 params.z_start_um = 0 params.exposure_time_s = 0.004 - params.det_dist_to_beam_converter_path = ( - "/dls_sw/i03/software/daq_configuration/lookup/DetDistToBeamXYConverter.txt" - ) return params From 0acdd820f62d5da93ef6dabf4021a9d0aef62322 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Thu, 16 Apr 2026 17:04:07 +0100 Subject: [PATCH 25/36] Remove commented out code --- tests/conftest.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index a9ee8f5006..d1fa7f40f4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -549,10 +549,6 @@ def beamstop_phase1( beamline_parameters: dict[str, Any], sim_run_engine: RunEngineSimulator, ) -> Generator[Beamstop, Any, Any]: - # with patch( - # "dodal.beamlines.i03.BEAMLINE_PARAMETERS_PATH", - # TEST_BEAMLINE_PARAMETERS, - # ): beamstop = i03.beamstop.build(connect_immediately=True, mock=True) set_mock_value(beamstop.x_mm.user_readback, 1.52) From cbfd727cb45c2043f312262724876c921eaec289 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Tue, 28 Apr 2026 16:02:45 +0100 Subject: [PATCH 26/36] Fix unit test breakage following rebase --- tests/unit_tests/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index c6e1c5b30d..936df44aea 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -8,6 +8,7 @@ from typing import Any, cast from unittest.mock import MagicMock, patch +import numpy as np import pytest from _pytest.fixtures import FixtureRequest from bluesky.run_engine import RunEngine From 751294c6fb1b4bf887f4199fba15cfcf19c90047 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Tue, 28 Apr 2026 17:21:43 +0100 Subject: [PATCH 27/36] Make type-checking happy --- tests/system_tests/conftest.py | 2 +- .../external_interaction/test_load_centre_collect_full_plan.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/system_tests/conftest.py b/tests/system_tests/conftest.py index 0feec5f037..d1593e377b 100644 --- a/tests/system_tests/conftest.py +++ b/tests/system_tests/conftest.py @@ -306,7 +306,7 @@ def config_client(): # and file converter map behave as expected with no mocking needed. # https://gitlab.diamond.ac.uk/MX-GDA/hyperion-system-testing/-/tree/add_daq_config_server/daq-config-server/config/?ref_type=heads clear_config_client() - i03.config_client.cache_clear() + i03.config_client.cache_clear() # type: ignore print("CONFIG_CLIENT CLEARED") return i03.config_client() diff --git a/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py b/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py index bedaf5a775..2ea4f9fdb5 100644 --- a/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py +++ b/tests/system_tests/hyperion/external_interaction/test_load_centre_collect_full_plan.py @@ -12,7 +12,6 @@ import pytest from bluesky.run_engine import RunEngine from daq_config_server import ConfigClient -from dodal.common.beamlines.beamline_utils import get_config_client from dodal.devices.beamsize.beamsize import BeamsizeBase from dodal.devices.oav.oav_parameters import OAVParameters from dodal.devices.oav.pin_image_recognition import PinTipDetection From 718d48bafb1024074bfce493e199fd7eb487e205 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Tue, 21 Apr 2026 14:32:12 +0100 Subject: [PATCH 28/36] Initial refactor introducing AperturePolicy Add json schema documentation generation --- docs/conf.py | 2 + docs/developer/hyperion/index.rst | 1 + .../hyperion/reference/blueapi_schema.json | 365 ++++++++++++++++ .../hyperion/reference/blueapi_schema.rst | 4 + pyproject.toml | 1 + .../i04_grid_detect_then_xray_centre_plan.py | 6 +- .../device_setup_plans/manipulate_sample.py | 39 +- ...ommon_grid_detect_then_xray_centre_plan.py | 18 +- .../common/parameters/components.py | 27 +- src/mx_bluesky/common/parameters/gridscan.py | 2 - src/mx_bluesky/common/parameters/rotation.py | 16 +- src/mx_bluesky/hyperion/blueapi/in_process.py | 4 +- .../load_centre_collect_full_plan.py | 7 +- .../experiment_plans/rotation_scan_plan.py | 7 +- ...ommon_grid_detect_then_xray_centre_plan.py | 137 ++++-- tests/unit_tests/conftest.py | 11 + .../test_rotation_scan_plan.py | 406 ++++++++++++------ .../parameters/test_parameter_model.py | 9 +- 18 files changed, 836 insertions(+), 226 deletions(-) create mode 100644 docs/developer/hyperion/reference/blueapi_schema.json create mode 100644 docs/developer/hyperion/reference/blueapi_schema.rst diff --git a/docs/conf.py b/docs/conf.py index 76b71cb01c..1b3ca13b14 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -51,6 +51,8 @@ "myst_parser", # For plantUML diagrams "plantweb.directive", + # For rendering the Hyperion BlueAPI json schema + "sphinx-jsonschema", ] myst_enable_extensions = [ diff --git a/docs/developer/hyperion/index.rst b/docs/developer/hyperion/index.rst index a6f542affe..ffeb742466 100644 --- a/docs/developer/hyperion/index.rst +++ b/docs/developer/hyperion/index.rst @@ -14,6 +14,7 @@ Documentation specific for the Hyperion module within MX-Bluesky reference/readme reference/baton + reference/blueapi_schema reference/param-hierarchy reference/coordinate-systems system-tests diff --git a/docs/developer/hyperion/reference/blueapi_schema.json b/docs/developer/hyperion/reference/blueapi_schema.json new file mode 100644 index 0000000000..4d56775682 --- /dev/null +++ b/docs/developer/hyperion/reference/blueapi_schema.json @@ -0,0 +1,365 @@ +{ + "plans": [ + { + "name": "clean_up_udc", + "description": "\n Perform actions that are always performed on ending UDC, regardless of whether\n it terminated abnormally or not.\n Currently the only thing this does is close the detector shutter.\n Args:\n detector_motion:\n\n Returns:\n ", + "schema": { + "additionalProperties": false, + "properties": { + "cleanup_group": { + "title": "Cleanup Group", + "type": "string" + }, + "detector_motion": { + "enum": [ + "detector_motion" + ], + "title": "Detector Motion", + "type": "dodal.devices.detector.detector_motion.DetectorMotion" + } + }, + "title": "clean_up_udc", + "type": "object" + } + }, + { + "name": "load_centre_collect", + "description": "\n Attempt a complete data collection experiment, consisting of the following:\n * Load the sample if necessary\n * Move to the specified goniometer start angles\n * Perform optical centring, then X-ray centring\n * If X-ray centring finds one or more diffracting centres then for each centre\n that satisfies the chosen selection function,\n move to that centre and do a collection with the specified parameters.\n ", + "schema": { + "$defs": { + "LoadCentreCollectParams": { + "additionalProperties": false, + "description": "This model is exposed as the BlueAPI REST parameter model for Hyperion Collections.\nIt can represent the full range of operations that are supported by LoadCentreCollect;\nthis is a superset of the operations that are actually required to follow Agamemnon instructions.\n\nThis is intended to provide additional flexibility to allow a potential future point of configuration\nwhere the supervisor can supply default values for values not explicitly specified by Agamemnon,\nalso to allow future exposure of this BlueAPI plan for e.g. commissioning purposes.\nThis may also permit the supervisor to implement future functionality by adjusting these parameters.", + "properties": { + "select_centres": { + "default": { + "name": "TopNByMaxCount", + "ignore_xtal_not_found": false, + "n": 1 + }, + "discriminator": { + "mapping": { + "TopNByMaxCount": "#/$defs/TopNByMaxCountSelection", + "TopNByMaxCountForEachSample": "#/$defs/TopNByMaxCountForEachSampleSelection" + }, + "propertyName": "name" + }, + "oneOf": [ + { + "$ref": "#/$defs/TopNByMaxCountSelection" + }, + { + "$ref": "#/$defs/TopNByMaxCountForEachSampleSelection" + } + ], + "title": "Select Centres" + }, + "visit": { + "title": "Visit", + "type": "string" + }, + "detector_distance_mm": { + "title": "Detector Distance Mm", + "type": "number" + }, + "sample_id": { + "title": "Sample Id", + "type": "integer" + }, + "sample_puck": { + "title": "Sample Puck", + "type": "integer" + }, + "sample_pin": { + "title": "Sample Pin", + "type": "integer" + }, + "demand_energy_ev": { + "title": "Demand Energy Ev", + "type": "number" + }, + "robot_load_then_centre": { + "$ref": "#/$defs/RobotLoadThenCentreParams" + }, + "multi_rotation_scan": { + "$ref": "#/$defs/MultiRotationScanParams" + } + }, + "required": [ + "visit", + "detector_distance_mm", + "sample_id", + "sample_puck", + "sample_pin", + "demand_energy_ev", + "robot_load_then_centre", + "multi_rotation_scan" + ], + "title": "LoadCentreCollectParams", + "type": "object" + }, + "MultiRotationScanParams": { + "additionalProperties": false, + "properties": { + "comment": { + "title": "Comment", + "type": "string" + }, + "file_name": { + "title": "File Name", + "type": "string" + }, + "storage_directory": { + "title": "Storage Directory", + "type": "string" + }, + "exposure_time_s": { + "title": "Exposure Time S", + "type": "number" + }, + "rotation_increment_deg": { + "title": "Rotation Increment Deg", + "type": "number" + }, + "snapshot_omegas_deg": { + "items": { + "type": "number" + }, + "title": "Snapshot Omegas Deg", + "type": "array" + }, + "rotation_scans": { + "items": { + "$ref": "#/$defs/SingleRotationScanParams" + }, + "title": "Rotation Scans", + "type": "array" + }, + "transmission_frac": { + "title": "Transmission Frac", + "type": "number" + }, + "ispyb_experiment_type": { + "title": "Ispyb Experiment Type", + "type": "string" + } + }, + "required": [ + "comment", + "file_name", + "storage_directory", + "exposure_time_s", + "rotation_increment_deg", + "snapshot_omegas_deg", + "rotation_scans", + "transmission_frac", + "ispyb_experiment_type" + ], + "title": "MultiRotationScanParams", + "type": "object" + }, + "RobotLoadThenCentreParams": { + "additionalProperties": false, + "properties": { + "storage_directory": { + "title": "Storage Directory", + "type": "string" + }, + "file_name": { + "title": "File Name", + "type": "string" + }, + "transmission_frac": { + "title": "Transmission Frac", + "type": "number" + }, + "exposure_time_s": { + "title": "Exposure Time S", + "type": "number" + }, + "omega_start_deg": { + "title": "Omega Start Deg", + "type": "number" + }, + "chi_start_deg": { + "title": "Chi Start Deg", + "type": "number" + }, + "tip_offset_um": { + "title": "Tip Offset Um", + "type": "number" + }, + "grid_width_um": { + "title": "Grid Width Um", + "type": "number" + } + }, + "required": [ + "storage_directory", + "file_name", + "transmission_frac", + "exposure_time_s", + "omega_start_deg", + "chi_start_deg", + "tip_offset_um", + "grid_width_um" + ], + "title": "RobotLoadThenCentreParams", + "type": "object" + }, + "SingleRotationScanParams": { + "additionalProperties": false, + "properties": { + "omega_start_deg": { + "title": "Omega Start Deg", + "type": "number" + }, + "phi_start_deg": { + "title": "Phi Start Deg", + "type": "number" + }, + "chi_start_deg": { + "title": "Chi Start Deg", + "type": "number" + }, + "rotation_direction": { + "title": "Rotation Direction", + "type": "string" + }, + "scan_width_deg": { + "title": "Scan Width Deg", + "type": "number" + } + }, + "required": [ + "omega_start_deg", + "phi_start_deg", + "chi_start_deg", + "rotation_direction", + "scan_width_deg" + ], + "title": "SingleRotationScanParams", + "type": "object" + }, + "TopNByMaxCountForEachSampleSelection": { + "properties": { + "name": { + "const": "TopNByMaxCountForEachSample", + "default": "TopNByMaxCountForEachSample", + "title": "Name", + "type": "string" + }, + "ignore_xtal_not_found": { + "default": false, + "title": "Ignore Xtal Not Found", + "type": "boolean" + }, + "n": { + "title": "N", + "type": "integer" + } + }, + "required": [ + "n" + ], + "title": "TopNByMaxCountForEachSampleSelection", + "type": "object" + }, + "TopNByMaxCountSelection": { + "properties": { + "name": { + "const": "TopNByMaxCount", + "default": "TopNByMaxCount", + "title": "Name", + "type": "string" + }, + "ignore_xtal_not_found": { + "default": false, + "title": "Ignore Xtal Not Found", + "type": "boolean" + }, + "n": { + "title": "N", + "type": "integer" + } + }, + "required": [ + "n" + ], + "title": "TopNByMaxCountSelection", + "type": "object" + } + }, + "additionalProperties": false, + "properties": { + "parameters": { + "$ref": "#/$defs/LoadCentreCollectParams" + } + }, + "required": [ + "parameters" + ], + "title": "load_centre_collect", + "type": "object" + } + }, + { + "name": "move_to_udc_default_state", + "description": "\n Move beamline hardware to known positions prior to UDC start.\n ", + "schema": { + "additionalProperties": false, + "properties": {}, + "title": "move_to_udc_default_state", + "type": "object" + } + }, + { + "name": "robot_unload", + "description": "\n Unload the currently mounted pin into the location that it was loaded from.\n This is to be invoked as the final step upon successful completion of the UDC queue.\n ", + "schema": { + "additionalProperties": false, + "properties": { + "visit": { + "title": "Visit", + "type": "string" + }, + "robot": { + "enum": [ + "robot" + ], + "title": "Robot", + "type": "dodal.devices.robot.BartRobot" + }, + "smargon": { + "enum": [ + "gonio" + ], + "title": "Smargon", + "type": "dodal.devices.smargon.Smargon" + }, + "aperture_scatterguard": { + "enum": [ + "aperture_scatterguard" + ], + "title": "Aperture Scatterguard", + "type": "dodal.devices.aperturescatterguard.ApertureScatterguard" + }, + "lower_gonio": { + "enum": [ + "detector_motion", + "gonio", + "lower_gonio" + ], + "title": "Lower Gonio", + "type": "dodal.devices.motors.XYZStage" + } + }, + "required": [ + "visit" + ], + "title": "robot_unload", + "type": "object" + } + } + ] +} diff --git a/docs/developer/hyperion/reference/blueapi_schema.rst b/docs/developer/hyperion/reference/blueapi_schema.rst new file mode 100644 index 0000000000..1da24497ae --- /dev/null +++ b/docs/developer/hyperion/reference/blueapi_schema.rst @@ -0,0 +1,4 @@ +BlueAPI Schema +============== + +.. jsonchema:: blueapi_schema.json diff --git a/pyproject.toml b/pyproject.toml index d8d187775d..8fa820e879 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -91,6 +91,7 @@ dev = [ "sphinx-autobuild", "sphinx-copybutton", "sphinx-design", + "sphinx-jsonschema", "tox-uv", "types-mock", "types-requests", 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..80d0320e76 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 @@ -6,7 +6,7 @@ import bluesky.preprocessors as bpp from bluesky.utils import MsgGenerator from dodal.common import inject -from dodal.devices.aperturescatterguard import ApertureScatterguard, ApertureValue +from dodal.devices.aperturescatterguard import ApertureScatterguard from dodal.devices.attenuator.attenuator import BinaryFilterAttenuator from dodal.devices.backlight import Backlight from dodal.devices.beamlines.i04.beamsize import Beamsize @@ -69,7 +69,7 @@ from mx_bluesky.common.external_interaction.callbacks.xray_centre.nexus_callback import ( GridscanNexusFileCallback, ) -from mx_bluesky.common.parameters.components import get_param_version +from mx_bluesky.common.parameters.components import AperturePolicy, get_param_version from mx_bluesky.common.parameters.constants import ( EnvironmentConstants, OavConstants, @@ -112,7 +112,7 @@ def _change_beamsize( An aperture is needed to reduce scatter but the transfocator is best used for beamsize changes as it gives more flux compared to a bigger beam with a small aperture. """ - parameters.selected_aperture = ApertureValue.LARGE + parameters.selected_aperture = AperturePolicy.LARGE yield from bps.abs_set( transfocator, beamsize, group=PlanGroupCheckpointConstants.GRID_READY_FOR_DC ) diff --git a/src/mx_bluesky/common/device_setup_plans/manipulate_sample.py b/src/mx_bluesky/common/device_setup_plans/manipulate_sample.py index 0147a67ea7..b218a2a584 100644 --- a/src/mx_bluesky/common/device_setup_plans/manipulate_sample.py +++ b/src/mx_bluesky/common/device_setup_plans/manipulate_sample.py @@ -10,6 +10,7 @@ from dodal.devices.smargon import CombinedMove, Smargon from dodal.devices.thawer import OnOff, Thawer +from mx_bluesky.common.parameters.components import AperturePolicy from mx_bluesky.common.parameters.constants import PlanGroupCheckpointConstants from mx_bluesky.common.utils.log import LOGGER @@ -18,7 +19,7 @@ def setup_sample_environment( aperture_scatterguard: ApertureScatterguard, - aperture_position_gda_name: str | None, + aperture_policy: AperturePolicy, backlight: Backlight, thawer: Thawer, group="setup_senv", @@ -29,11 +30,8 @@ def setup_sample_environment( yield from bps.abs_set(backlight, InOut.OUT, group=group) - aperture_value = ( - None - if not aperture_position_gda_name - else ApertureValue(aperture_position_gda_name) - ) + aperture_value = _rotation_aperture_value_from_policy(aperture_policy) + yield from move_aperture_if_required( aperture_scatterguard, aperture_value, group=group ) @@ -41,6 +39,19 @@ def setup_sample_environment( yield from bps.abs_set(thawer, OnOff.OFF, group=group) +def prepare_aperture_if_required( + aperture_scatterguard: ApertureScatterguard, + aperture_policy: AperturePolicy, +): + aperture_value = _rotation_aperture_value_from_policy(aperture_policy) + if aperture_value: + yield from bps.prepare( + aperture_scatterguard, + aperture_value, + group=PlanGroupCheckpointConstants.PREPARE_APERTURE, + ) + + def move_aperture_if_required( aperture_scatterguard: ApertureScatterguard, aperture_value: ApertureValue | None, @@ -110,3 +121,19 @@ def move_phi_chi_omega( ) if wait: yield from bps.wait(group) + + +def _rotation_aperture_value_from_policy( + policy: AperturePolicy, +) -> ApertureValue | None: + match policy: + case AperturePolicy.SMALL: + return ApertureValue.SMALL + case AperturePolicy.MEDIUM: + return ApertureValue.MEDIUM + case AperturePolicy.LARGE | AperturePolicy.AUTO: + return ApertureValue.LARGE + case AperturePolicy.CURRENT_POSITION: + return None + case _: + raise ValueError(f"Unsupported aperture policy {policy}") diff --git a/src/mx_bluesky/common/experiment_plans/common_grid_detect_then_xray_centre_plan.py b/src/mx_bluesky/common/experiment_plans/common_grid_detect_then_xray_centre_plan.py index 9da2347b27..092c86b73f 100644 --- a/src/mx_bluesky/common/experiment_plans/common_grid_detect_then_xray_centre_plan.py +++ b/src/mx_bluesky/common/experiment_plans/common_grid_detect_then_xray_centre_plan.py @@ -7,6 +7,7 @@ from bluesky import preprocessors as bpp from bluesky.utils import MsgGenerator from dodal.common.beamlines.beamline_utils import get_config_client +from dodal.devices.aperturescatterguard import ApertureValue from dodal.devices.backlight import InOut from dodal.devices.eiger import EigerDetector from dodal.devices.oav.oav_parameters import OAVParameters @@ -35,6 +36,7 @@ from mx_bluesky.common.external_interaction.callbacks.xray_centre.ispyb_callback import ( ispyb_activation_wrapper, ) +from mx_bluesky.common.parameters.components import AperturePolicy from mx_bluesky.common.parameters.constants import ( OavConstants, PlanGroupCheckpointConstants, @@ -139,7 +141,7 @@ def run_grid_detection_plan( # Start moving the aperture/scatterguard into position without moving it in yield from bps.prepare( composite.aperture_scatterguard, - parameters.selected_aperture, + _xrc_aperture_value_from_policy(parameters.selected_aperture), group=PlanGroupCheckpointConstants.PREPARE_APERTURE, ) @@ -158,7 +160,7 @@ def run_grid_detection_plan( yield from bps.wait(PlanGroupCheckpointConstants.PREPARE_APERTURE) yield from move_aperture_if_required( composite.aperture_scatterguard, - parameters.selected_aperture, + _xrc_aperture_value_from_policy(parameters.selected_aperture), group=PlanGroupCheckpointConstants.GRID_READY_FOR_DC, ) xrc_params = create_parameters_for_flyscan_xray_centre( @@ -190,3 +192,15 @@ def create_parameters_for_flyscan_xray_centre( flyscan_xray_centre_parameters = xrc_params_type(**params_json) LOGGER.info(f"Parameters for FGS: {flyscan_xray_centre_parameters}") return flyscan_xray_centre_parameters + + +def _xrc_aperture_value_from_policy(policy: AperturePolicy) -> ApertureValue: + match policy: + case AperturePolicy.SMALL | AperturePolicy.AUTO: + return ApertureValue.SMALL + case AperturePolicy.MEDIUM: + return ApertureValue.MEDIUM + case AperturePolicy.LARGE: + return ApertureValue.LARGE + case _: + raise ValueError(f"Unsupported aperture policy {policy}") diff --git a/src/mx_bluesky/common/parameters/components.py b/src/mx_bluesky/common/parameters/components.py index 052674efc1..1585500b02 100644 --- a/src/mx_bluesky/common/parameters/components.py +++ b/src/mx_bluesky/common/parameters/components.py @@ -7,7 +7,6 @@ from pathlib import Path from typing import Self, SupportsInt -from dodal.devices.aperturescatterguard import ApertureValue from dodal.devices.detector import ( DetectorParams, TriggerMode, @@ -155,6 +154,30 @@ class WithVisit(BaseModel): detector_distance_mm: float | None = Field(default=None, gt=0) +class AperturePolicy(StrEnum): + """Defines the aperture that will be selected for the experiment. + The precise meanings of small, medium and large are relative and may be + specific to particular beamlines and/or experiments. + At some point in the future we may define additional AUTO_ values to allow requests to choose + from of a range of different ways to automatically determine an aperture. + In the longer term, it may be that we want to specify beam size more generally in parameters rather than + specifying a particular mechanism such as aperture as this would make plan parameters more portable across + beamlines. + Attributes: + SMALL: Select the small aperture + MEDIUM: Select the medium aperture + LARGE: Select the large aperture + AUTO: Automatically select an aperture based on an implementation-dependent methodology. + CURRENT_POSITION: Do not move the aperture and instead collect in whatever position it is in currently + """ + + SMALL = "SMALL_APERTURE" + MEDIUM = "MEDIUM_APERTURE" + LARGE = "LARGE_APERTURE" + AUTO = "AUTO_UNSPECIFIED" + CURRENT_POSITION = "CURRENT_POSITION" + + class DiffractionExperiment( MxBlueskyParameters, WithSnapshot, WithOptionalEnergyChange, WithVisit ): @@ -165,7 +188,7 @@ class DiffractionExperiment( comment: str = Field(default="") trigger_mode: TriggerMode = Field(default=TriggerMode.FREE_RUN) run_number: int | None = Field(default=None, ge=0) - selected_aperture: ApertureValue | None = Field(default=None) + selected_aperture: AperturePolicy = Field(default=AperturePolicy.AUTO) transmission_frac: float = Field(default=0.1) ispyb_experiment_type: IspybExperimentType storage_directory: str diff --git a/src/mx_bluesky/common/parameters/gridscan.py b/src/mx_bluesky/common/parameters/gridscan.py index f33951a50e..dfa62e3533 100644 --- a/src/mx_bluesky/common/parameters/gridscan.py +++ b/src/mx_bluesky/common/parameters/gridscan.py @@ -3,7 +3,6 @@ from abc import abstractmethod from typing import Generic, TypeVar -from dodal.devices.aperturescatterguard import ApertureValue from dodal.devices.detector.det_dim_constants import EIGER2_X_9M_SIZE, EIGER2_X_16M_SIZE from dodal.devices.detector.detector import DetectorParams from dodal.devices.fast_grid_scan import ( @@ -60,7 +59,6 @@ class GridCommon( ispyb_experiment_type: IspybExperimentType = Field( default=IspybExperimentType.GRIDSCAN_3D ) - selected_aperture: ApertureValue | None = Field(default=ApertureValue.SMALL) tip_offset_um: float = Field(default=HardwareConstants.TIP_OFFSET_UM) diff --git a/src/mx_bluesky/common/parameters/rotation.py b/src/mx_bluesky/common/parameters/rotation.py index 402a547864..e5bdf32169 100644 --- a/src/mx_bluesky/common/parameters/rotation.py +++ b/src/mx_bluesky/common/parameters/rotation.py @@ -6,13 +6,11 @@ from typing import Annotated, Any, Self from annotated_types import Len -from dodal.devices.aperturescatterguard import ApertureValue from dodal.devices.detector import DetectorParams from dodal.devices.zebra.zebra import ( RotationDirection, ) -from dodal.log import LOGGER -from pydantic import Field, field_validator, model_validator +from pydantic import Field, model_validator from scanspec.core import AxesPoints from scanspec.core import Path as ScanPath from scanspec.specs import Line @@ -91,18 +89,6 @@ def _detector_params_impl( def _detector_params(self, omega_start_deg: float) -> DetectorParams: return self._detector_params_impl(omega_start_deg, self.num_images, 1) - @field_validator("selected_aperture") - @classmethod - def _set_default_aperture_position(cls, aperture_position: ApertureValue | None): - if not aperture_position: - default_aperture = RotationParamConstants.DEFAULT_APERTURE_POSITION - LOGGER.warning( - f"No aperture position selected. Defaulting to {default_aperture}" - ) - return default_aperture - else: - return aperture_position - class SingleRotationScan( WithScan, RotationExperiment, RotationScanPerSweep, DiffractionExperimentWithSample diff --git a/src/mx_bluesky/hyperion/blueapi/in_process.py b/src/mx_bluesky/hyperion/blueapi/in_process.py index fc0cd495d9..e958f6b0e2 100644 --- a/src/mx_bluesky/hyperion/blueapi/in_process.py +++ b/src/mx_bluesky/hyperion/blueapi/in_process.py @@ -59,8 +59,8 @@ def load_centre_collect( * Move to the specified goniometer start angles * Perform optical centring, then X-ray centring * If X-ray centring finds one or more diffracting centres then for each centre - that satisfies the chosen selection function, - move to that centre and do a collection with the specified parameters. + that satisfies the chosen selection function, + move to that centre and do a collection with the specified parameters. """ yield from _load_centre_collect_full( composite, load_centre_collect_to_internal(parameters) diff --git a/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py b/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py index 994df67d36..1aeca06f6c 100644 --- a/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py +++ b/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py @@ -58,7 +58,12 @@ def load_centre_collect_full( that satisfies the chosen selection function, move to that centre and do a collection with the specified parameters. """ - + LOGGER.info( + f"aperture from parameters ROBOT LOAD is {parameters.robot_load_then_centre.selected_aperture}" + ) + LOGGER.info( + f"aperture from parameters ROTATION is {parameters.multi_rotation_scan.selected_aperture}" + ) if not oav_params: oav_params = OAVParameters(get_config_client(), context="xrayCentring") oav_config_file = oav_params.oav_config_json diff --git a/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py b/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py index a3110e4e32..0b77c07470 100644 --- a/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py +++ b/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py @@ -31,6 +31,7 @@ from mx_bluesky.common.device_setup_plans.manipulate_sample import ( cleanup_sample_environment, + prepare_aperture_if_required, setup_sample_environment, ) from mx_bluesky.common.device_setup_plans.setup_zebra_and_shutter import ( @@ -238,10 +239,8 @@ def _div_by_1000_if_not_none(num: float | None): ) if params.selected_aperture: - yield from bps.prepare( - composite.aperture_scatterguard, - params.selected_aperture, - group=CONST.WAIT.PREPARE_APERTURE, + yield from prepare_aperture_if_required( + composite.aperture_scatterguard, params.selected_aperture ) yield from oav_snapshot_plan(composite, params, oav_params) diff --git a/tests/unit_tests/common/experiment_plans/test_common_grid_detect_then_xray_centre_plan.py b/tests/unit_tests/common/experiment_plans/test_common_grid_detect_then_xray_centre_plan.py index 8c39c374b0..28b47e655a 100644 --- a/tests/unit_tests/common/experiment_plans/test_common_grid_detect_then_xray_centre_plan.py +++ b/tests/unit_tests/common/experiment_plans/test_common_grid_detect_then_xray_centre_plan.py @@ -1,4 +1,5 @@ import dataclasses +from collections.abc import Generator from unittest.mock import ANY, MagicMock, call, patch import bluesky.plan_stubs as bps @@ -28,6 +29,7 @@ from mx_bluesky.common.external_interaction.callbacks.xray_centre.ispyb_callback import ( ispyb_activation_wrapper, ) +from mx_bluesky.common.parameters.components import AperturePolicy from mx_bluesky.common.parameters.constants import ( DocDescriptorNames, PlanGroupCheckpointConstants, @@ -280,57 +282,63 @@ def test_detect_grid_and_do_gridscan_does_not_activate_ispyb_callback( @pytest.fixture() -@patch( - "mx_bluesky.common.experiment_plans.common_grid_detect_then_xray_centre_plan.grid_detection_plan", - autospec=True, -) -@patch( - "mx_bluesky.common.experiment_plans.common_grid_detect_then_xray_centre_plan.common_flyscan_xray_centre", - autospec=True, - side_effect=_fake_flyscan, -) -def msgs_from_simulated_grid_detect_then_xray_centre( - mock_flyscan, - mock_grid_detection_plan, +def grid_detect_then_xrc_simulator( sim_run_engine: RunEngineSimulator, +) -> Generator[RunEngineSimulator, None, None]: + with ( + patch( + "mx_bluesky.common.experiment_plans.common_grid_detect_then_xray_centre_plan.grid_detection_plan", + autospec=True, + ) as mock_grid_detection_plan, + patch( + "mx_bluesky.common.experiment_plans.common_grid_detect_then_xray_centre_plan.common_flyscan_xray_centre", + autospec=True, + side_effect=_fake_flyscan, + ), + ): + mock_grid_detection_plan.return_value = iter( + [ + Msg("save_oav_grids"), + Msg( + "open_run", + run=DocDescriptorNames.FLYSCAN_RESULTS, + xray_centre_results=[dataclasses.asdict(FLYSCAN_RESULT_MED)], + ), + ] + ) + sim_run_engine.add_handler_for_callback_subscribes() + sim_fire_event_on_open_run(sim_run_engine, DocDescriptorNames.FLYSCAN_RESULTS) + sim_run_engine.add_callback_handler_for_multiple( + "save_oav_grids", + [ + [ + ( + "descriptor", + OavGridSnapshotTestEvents.test_descriptor_document_oav_snapshot, # type: ignore + ), + ( + "event", + OavGridSnapshotTestEvents.test_event_document_oav_snapshot_xy, # type: ignore + ), + ( + "event", + OavGridSnapshotTestEvents.test_event_document_oav_snapshot_xz, # type: ignore + ), + ] + ], + ) + yield sim_run_engine + + +@pytest.fixture +def msgs_from_simulated_grid_detect_then_xray_centre( + grid_detect_then_xrc_simulator: RunEngineSimulator, grid_detect_xrc_devices: GridDetectThenXRayCentreComposite, test_full_grid_scan_params: GridScanWithEdgeDetect, test_config_files: dict[str, str], construct_beamline_specific: ConstructBeamlineSpecificFeatures, ): - mock_grid_detection_plan.return_value = iter( - [ - Msg("save_oav_grids"), - Msg( - "open_run", - run=DocDescriptorNames.FLYSCAN_RESULTS, - xray_centre_results=[dataclasses.asdict(FLYSCAN_RESULT_MED)], - ), - ] - ) - - sim_run_engine.add_handler_for_callback_subscribes() - sim_fire_event_on_open_run(sim_run_engine, DocDescriptorNames.FLYSCAN_RESULTS) - sim_run_engine.add_callback_handler_for_multiple( - "save_oav_grids", - [ - [ - ( - "descriptor", - OavGridSnapshotTestEvents.test_descriptor_document_oav_snapshot, # type: ignore - ), - ( - "event", - OavGridSnapshotTestEvents.test_event_document_oav_snapshot_xy, # type: ignore - ), - ( - "event", - OavGridSnapshotTestEvents.test_event_document_oav_snapshot_xz, # type: ignore - ), - ] - ], - ) - return sim_run_engine.simulate_plan( + return grid_detect_then_xrc_simulator.simulate_plan( grid_detect_then_xray_centre( grid_detect_xrc_devices, test_full_grid_scan_params, @@ -384,6 +392,45 @@ def test_detect_grid_and_do_gridscan_waits_for_aperture_to_be_prepared_before_mo ) +@pytest.mark.parametrize( + "aperture_policy, expected_aperture", + [ + [AperturePolicy.LARGE, ApertureValue.LARGE], + [AperturePolicy.MEDIUM, ApertureValue.MEDIUM], + [AperturePolicy.SMALL, ApertureValue.SMALL], + [AperturePolicy.AUTO, ApertureValue.SMALL], + ], +) +def test_detect_grid_and_do_gridscan_maps_aperture_policy( + aperture_policy: AperturePolicy, + expected_aperture: ApertureValue, + grid_detect_then_xrc_simulator: RunEngineSimulator, + grid_detect_xrc_devices: GridDetectThenXRayCentreComposite, + test_full_grid_scan_params: GridScanWithEdgeDetect, + test_config_files: dict[str, str], + construct_beamline_specific: ConstructBeamlineSpecificFeatures, +): + test_full_grid_scan_params.selected_aperture = aperture_policy + msgs = grid_detect_then_xrc_simulator.simulate_plan( + grid_detect_then_xray_centre( + grid_detect_xrc_devices, + test_full_grid_scan_params, + xrc_params_type=SpecifiedThreeDGridScan, + construct_beamline_specific=construct_beamline_specific, + oav_config=test_config_files["oav_config_json"], + ) + ) + assert_message_and_return_remaining( + msgs, + lambda msg: ( + msg.command == "set" + and msg.obj + is grid_detect_xrc_devices.aperture_scatterguard.selected_aperture + and msg.args[0] == expected_aperture + ), + ) + + @patch( "mx_bluesky.common.experiment_plans.common_grid_detect_then_xray_centre_plan.detect_grid_and_do_gridscan" ) diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 936df44aea..183bdc7ab8 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -117,6 +117,17 @@ ] +@pytest.fixture(autouse=True) +def patch_config_server(): + # This client isn't used but enables get_config_client to return a client with patched calls + with patch( + "dodal.common.beamlines.beamline_utils.CONFIG_CLIENT", + ConfigClient(""), + create=True, + ): + yield + + def _error_and_kill_pending_tasks( loop: asyncio.AbstractEventLoop, test_name: str, test_passed: bool ) -> set[asyncio.Task]: diff --git a/tests/unit_tests/hyperion/experiment_plans/test_rotation_scan_plan.py b/tests/unit_tests/hyperion/experiment_plans/test_rotation_scan_plan.py index c79cc5f55a..aa4e5c7724 100644 --- a/tests/unit_tests/hyperion/experiment_plans/test_rotation_scan_plan.py +++ b/tests/unit_tests/hyperion/experiment_plans/test_rotation_scan_plan.py @@ -15,7 +15,7 @@ from bluesky.run_engine import RunEngine from bluesky.simulators import RunEngineSimulator, assert_message_and_return_remaining from dodal.common.maths import AngleWithPhase -from dodal.devices.aperturescatterguard import ApertureScatterguard, ApertureValue +from dodal.devices.aperturescatterguard import ApertureValue from dodal.devices.backlight import InOut from dodal.devices.beamlines.i03 import BeamstopPositions from dodal.devices.detector.detector_motion import ShutterState @@ -39,6 +39,7 @@ StoreInIspyb, ) from mx_bluesky.common.external_interaction.nexus.nexus_utils import AxisDirection +from mx_bluesky.common.parameters.components import AperturePolicy from mx_bluesky.common.parameters.constants import DocDescriptorNames from mx_bluesky.common.parameters.rotation import ( RotationScan, @@ -341,14 +342,59 @@ async def test_full_rotation_plan_smargon_settings( @pytest.mark.timeout(2) +@patch( + "bluesky.preprocessors.__read_and_stash_a_motor", + MagicMock(fake_read), +) +@pytest.mark.parametrize( + "aperture_policy, expected_aperture", + [ + [AperturePolicy.SMALL, ApertureValue.SMALL], + [AperturePolicy.MEDIUM, ApertureValue.MEDIUM], + [AperturePolicy.LARGE, ApertureValue.LARGE], + [AperturePolicy.AUTO, ApertureValue.LARGE], + ], +) async def test_rotation_plan_moves_aperture_correctly( - run_full_rotation_plan: RotationScanComposite, + aperture_policy: AperturePolicy, + expected_aperture: ApertureValue, + sim_run_engine_for_rotation: RunEngineSimulator, + test_rotation_params: RotationScan, + fake_create_rotation_devices: RotationScanComposite, + oav_parameters_for_rotation: OAVParameters, ) -> None: - aperture_scatterguard: ApertureScatterguard = ( - run_full_rotation_plan.aperture_scatterguard + test_rotation_params.selected_aperture = aperture_policy + msgs = sim_run_engine_for_rotation.simulate_plan( + rotation_scan_internal( + fake_create_rotation_devices, + test_rotation_params, + oav_parameters_for_rotation, + ), ) - assert ( - await aperture_scatterguard.selected_aperture.get_value() == ApertureValue.SMALL + aperture_scatterguard = fake_create_rotation_devices.aperture_scatterguard + msgs = assert_message_and_return_remaining( + msgs, + lambda msg: ( + msg.command == "prepare" + and msg.obj is aperture_scatterguard + and msg.args[0] == expected_aperture + ), + ) + prepare_group = msgs[0].kwargs["group"] + msgs = assert_message_and_return_remaining( + msgs, lambda msg: msg.command == "wait" and msg.kwargs["group"] == prepare_group + ) + msgs = assert_message_and_return_remaining( + msgs, + lambda msg: ( + msg.command == "set" + and msg.obj is aperture_scatterguard.selected_aperture + and msg.args[0] == expected_aperture + ), + ) + set_group = msgs[0].kwargs["group"] + assert_message_and_return_remaining( + msgs, lambda msg: msg.command == "wait" and msg.kwargs["group"] == set_group ) @@ -426,8 +472,10 @@ def test_rotation_plan_reads_hardware( msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "create" - and msg.kwargs["name"] == CONST.DESCRIPTORS.HARDWARE_READ_PRE, + lambda msg: ( + msg.command == "create" + and msg.kwargs["name"] == CONST.DESCRIPTORS.HARDWARE_READ_PRE + ), ) msgs_in_event = list(takewhile(lambda msg: msg.command != "save", msgs)) assert_message_and_return_remaining( @@ -459,22 +507,28 @@ def test_rotation_scan_initialises_detector_distance_shutter_and_tx_fraction( ): msgs = assert_message_and_return_remaining( rotation_scan_simulated_messages, - lambda msg: msg.command == "set" - and msg.args[0] == test_rotation_params.detector_distance_mm - and msg.obj.name == "detector_motion-z" - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "set" + and msg.args[0] == test_rotation_params.detector_distance_mm + and msg.obj.name == "detector_motion-z" + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.args[0] == ShutterState.OPEN - and msg.obj.name == "detector_motion-shutter" - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "set" + and msg.args[0] == ShutterState.OPEN + and msg.obj.name == "detector_motion-shutter" + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "wait" + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) @@ -488,15 +542,19 @@ def test_rotation_scan_triggers_xbpm_then_pauses_xbpm_and_sets_transmission( ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "xbpm_feedback-pause_feedback" - and msg.args[0] == Pause.PAUSE.value, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "xbpm_feedback-pause_feedback" + and msg.args[0] == Pause.PAUSE.value + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "attenuator" - and msg.args[0] == test_rotation_params.transmission_frac, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "attenuator" + and msg.args[0] == test_rotation_params.transmission_frac + ), ) @@ -510,15 +568,17 @@ def test_rotation_scan_does_not_change_transmission_back_until_after_data_collec ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "xbpm_feedback-pause_feedback" - and msg.args[0] == Pause.RUN.value, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "xbpm_feedback-pause_feedback" + and msg.args[0] == Pause.RUN.value + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "attenuator" - and msg.args[0] == 1.0, + lambda msg: ( + msg.command == "set" and msg.obj.name == "attenuator" and msg.args[0] == 1.0 + ), ) @@ -527,13 +587,16 @@ def test_rotation_scan_moves_gonio_to_start_before_snapshots( ): msgs = assert_message_and_return_remaining( rotation_scan_simulated_messages, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == CONST.WAIT.MOVE_GONIO_TO_START, + lambda msg: ( + msg.command == "wait" + and msg.kwargs["group"] == CONST.WAIT.MOVE_GONIO_TO_START + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV, + lambda msg: ( + msg.command == "wait" and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV + ), ) @@ -542,28 +605,36 @@ def test_rotation_scan_moves_aperture_in_backlight_out_after_snapshots_before_ro ): msgs = assert_message_and_return_remaining( rotation_scan_simulated_messages, - lambda msg: msg.command == "create" - and msg.kwargs["name"] == DocDescriptorNames.OAV_ROTATION_SNAPSHOT_TRIGGERED, + lambda msg: ( + msg.command == "create" + and msg.kwargs["name"] == DocDescriptorNames.OAV_ROTATION_SNAPSHOT_TRIGGERED + ), ) msgs = assert_message_and_return_remaining(msgs, lambda msg: msg.command == "save") msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "backlight" - and msg.args[0] == InOut.OUT - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "backlight" + and msg.args[0] == InOut.OUT + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "aperture_scatterguard-selected_aperture" - and msg.args[0] == ApertureValue.SMALL - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "aperture_scatterguard-selected_aperture" + and msg.args[0] == ApertureValue.SMALL + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "wait" + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) @@ -572,22 +643,27 @@ def test_rotation_scan_waits_on_aperture_being_prepared_before_moving_in( ): msgs = assert_message_and_return_remaining( rotation_scan_simulated_messages, - lambda msg: msg.command == "prepare" - and msg.obj.name == "aperture_scatterguard" - and msg.args[0] == ApertureValue.SMALL - and msg.kwargs["group"] == CONST.WAIT.PREPARE_APERTURE, + lambda msg: ( + msg.command == "prepare" + and msg.obj.name == "aperture_scatterguard" + and msg.args[0] == ApertureValue.SMALL + and msg.kwargs["group"] == CONST.WAIT.PREPARE_APERTURE + ), ) assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == CONST.WAIT.PREPARE_APERTURE, + lambda msg: ( + msg.command == "wait" and msg.kwargs["group"] == CONST.WAIT.PREPARE_APERTURE + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "aperture_scatterguard-selected_aperture" - and msg.args[0] == ApertureValue.SMALL - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "aperture_scatterguard-selected_aperture" + and msg.args[0] == ApertureValue.SMALL + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) @@ -596,15 +672,19 @@ def test_rotation_scan_waits_on_thawing_being_off_before_collection( ): msgs = assert_message_and_return_remaining( rotation_scan_simulated_messages, - lambda msg: msg.command == "set" - and msg.args[0] == OnOff.OFF - and msg.obj.name == "thawer" - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "set" + and msg.args[0] == OnOff.OFF + and msg.obj.name == "thawer" + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "wait" + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) @@ -614,26 +694,34 @@ def test_rotation_scan_resets_omega_waits_for_sample_env_complete_after_snapshot params = next(test_rotation_params.single_rotation_scans) msgs = assert_message_and_return_remaining( rotation_scan_simulated_messages, - lambda msg: msg.command == "create" - and msg.kwargs["name"] == DocDescriptorNames.OAV_ROTATION_SNAPSHOT_TRIGGERED, + lambda msg: ( + msg.command == "create" + and msg.kwargs["name"] == DocDescriptorNames.OAV_ROTATION_SNAPSHOT_TRIGGERED + ), ) msgs = assert_message_and_return_remaining(msgs, lambda msg: msg.command == "save") msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "gonio-omega" - and msg.args[0] == params.omega_start_deg - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "gonio-omega" + and msg.args[0] == params.omega_start_deg + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "wait" + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "create" - and msg.kwargs["name"] == CONST.DESCRIPTORS.ZOCALO_HW_READ, + lambda msg: ( + msg.command == "create" + and msg.kwargs["name"] == CONST.DESCRIPTORS.ZOCALO_HW_READ + ), ) @@ -642,22 +730,27 @@ def test_rotation_snapshot_setup_called_to_move_backlight_in_aperture_out_before ): msgs = assert_message_and_return_remaining( rotation_scan_simulated_messages, - lambda msg: msg.command == "set" - and msg.obj.name == "backlight" - and msg.args[0] == InOut.IN - and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "backlight" + and msg.args[0] == InOut.IN + and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "aperture_scatterguard-selected_aperture" - and msg.args[0] == ApertureValue.OUT_OF_BEAM - and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "aperture_scatterguard-selected_aperture" + and msg.args[0] == ApertureValue.OUT_OF_BEAM + and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV, + lambda msg: ( + msg.command == "wait" and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV + ), ) msgs = assert_message_and_return_remaining( msgs, lambda msg: msg.command == "trigger" and msg.obj.name == "oav-snapshot" @@ -764,29 +857,39 @@ def test_rotation_scan_turns_shutter_to_auto_with_pc_gate_then_back_to_manual( ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "sample_shutter-control_mode" - and msg.args[0] == ZebraShutterControl.AUTO, # type:ignore + lambda msg: ( + msg.command == "set" + and msg.obj.name == "sample_shutter-control_mode" + and msg.args[0] == ZebraShutterControl.AUTO + ), # type:ignore ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "zebra-logic_gates-and_gates-2-sources-1" - and msg.args[0] == fake_create_rotation_devices.zebra.mapping.sources.SOFT_IN1, # type:ignore + lambda msg: ( + msg.command == "set" + and msg.obj.name == "zebra-logic_gates-and_gates-2-sources-1" + and msg.args[0] + == fake_create_rotation_devices.zebra.mapping.sources.SOFT_IN1 + ), # type:ignore ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "zebra-logic_gates-and_gates-2-sources-2" - and msg.args[0] == fake_create_rotation_devices.zebra.mapping.sources.PC_GATE, # type:ignore + lambda msg: ( + msg.command == "set" + and msg.obj.name == "zebra-logic_gates-and_gates-2-sources-2" + and msg.args[0] + == fake_create_rotation_devices.zebra.mapping.sources.PC_GATE + ), # type:ignore ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "sample_shutter-control_mode" - and msg.args[0] == ZebraShutterControl.MANUAL, # type:ignore + lambda msg: ( + msg.command == "set" + and msg.obj.name == "sample_shutter-control_mode" + and msg.args[0] == ZebraShutterControl.MANUAL + ), # type:ignore ) @@ -799,29 +902,37 @@ def test_rotation_scan_arms_detector_and_takes_snapshots_whilst_arming( composite = fake_create_rotation_devices msgs = assert_message_and_return_remaining( rotation_scan_simulated_messages, - lambda msg: msg.command == "set" - and msg.obj.name == "eiger_do_arm" - and msg.args[0] == 1 - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "eiger_do_arm" + and msg.args[0] == 1 + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj is composite.oav.snapshot.directory - and msg.args[0] == str(test_rotation_params.snapshot_directory), + lambda msg: ( + msg.command == "set" + and msg.obj is composite.oav.snapshot.directory + and msg.args[0] == str(test_rotation_params.snapshot_directory) + ), ) for omega in test_rotation_params.snapshot_omegas_deg: msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj is composite.gonio.wrapped_omega.phase - and msg.args[0] == omega, + lambda msg: ( + msg.command == "set" + and msg.obj is composite.gonio.wrapped_omega.phase + and msg.args[0] == omega + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj is composite.oav.snapshot.filename - and f"_oav_snapshot_{omega:.0f}" in msg.args[0], + lambda msg: ( + msg.command == "set" + and msg.obj is composite.oav.snapshot.filename + and f"_oav_snapshot_{omega:.0f}" in msg.args[0] + ), ) msgs = assert_message_and_return_remaining( msgs, @@ -829,9 +940,11 @@ def test_rotation_scan_arms_detector_and_takes_snapshots_whilst_arming( ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "create" - and msg.kwargs["name"] - == DocDescriptorNames.OAV_ROTATION_SNAPSHOT_TRIGGERED, + lambda msg: ( + msg.command == "create" + and msg.kwargs["name"] + == DocDescriptorNames.OAV_ROTATION_SNAPSHOT_TRIGGERED + ), ) msgs = assert_message_and_return_remaining( msgs, lambda msg: msg.command == "read" and msg.obj is composite.oav @@ -841,8 +954,10 @@ def test_rotation_scan_arms_detector_and_takes_snapshots_whilst_arming( ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "wait" - and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, + lambda msg: ( + msg.command == "wait" + and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC + ), ) @@ -933,9 +1048,11 @@ def test_rotation_scan_moves_beamstop_into_place( ) msgs = assert_message_and_return_remaining( msgs, - predicate=lambda msg: msg.command == "set" - and msg.obj.name == "beamstop-selected_pos" - and msg.args[0] == BeamstopPositions.DATA_COLLECTION, + predicate=lambda msg: ( + msg.command == "set" + and msg.obj.name == "beamstop-selected_pos" + and msg.args[0] == BeamstopPositions.DATA_COLLECTION + ), ) msgs = assert_message_and_return_remaining( msgs, predicate=lambda msg: msg.command == "rotation_scan_plan" @@ -1028,8 +1145,10 @@ def test_rotation_scan_plan_with_omega_flip_inverts_motor_movements_but_not_even ) rotation_outer_start_event = next( dropwhile( - lambda _: _.args[0] != "start" - or _.args[1].get("subplan_name") != CONST.PLAN.ROTATION_OUTER, + lambda _: ( + _.args[0] != "start" + or _.args[1].get("subplan_name") != CONST.PLAN.ROTATION_OUTER + ), mock_callback.mock_calls, ) ) @@ -1100,8 +1219,9 @@ async def test_multi_rotation_plan_runs_multiple_plans_in_one_arm( msgs_within_arming = list( takewhile( - lambda msg: msg.command != "unstage" - and (not msg.obj or msg.obj.name != "eiger"), + lambda msg: ( + msg.command != "unstage" and (not msg.obj or msg.obj.name != "eiger") + ), msgs, ) ) @@ -1115,15 +1235,17 @@ async def test_multi_rotation_plan_runs_multiple_plans_in_one_arm( # moving to the start position msgs_within_arming = assert_message_and_return_remaining( msgs_within_arming, - lambda msg: msg.command == "set" - and msg.obj == smargon - and msg.args[0] - == CombinedMove( - x=scan.x_start_um / 1000, # type: ignore - y=scan.y_start_um / 1000, # type: ignore - z=scan.z_start_um / 1000, # type: ignore - phi=scan.phi_start_deg, - chi=scan.chi_start_deg, + lambda msg: ( + msg.command == "set" + and msg.obj == smargon + and msg.args[0] + == CombinedMove( + x=scan.x_start_um / 1000, # type: ignore + y=scan.y_start_um / 1000, # type: ignore + z=scan.z_start_um / 1000, # type: ignore + phi=scan.phi_start_deg, + chi=scan.chi_start_deg, + ) ), ) # arming the zebra @@ -1134,12 +1256,14 @@ async def test_multi_rotation_plan_runs_multiple_plans_in_one_arm( # the final rel_set of omega to trigger the scan assert_message_and_return_remaining( msgs_within_arming, - lambda msg: msg.command == "set" - and msg.obj.name == "gonio-omega" - and msg.args - == ( - (scan.scan_width_deg + motion_values.shutter_opening_deg) - * motion_values.direction.multiplier, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "gonio-omega" + and msg.args + == ( + (scan.scan_width_deg + motion_values.shutter_opening_deg) + * motion_values.direction.multiplier, + ) ), ) @@ -1735,15 +1859,17 @@ def test_multi_rotation_scan_does_not_change_transmission_back_until_after_data_ ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "xbpm_feedback-pause_feedback" - and msg.args[0] == Pause.RUN.value, + lambda msg: ( + msg.command == "set" + and msg.obj.name == "xbpm_feedback-pause_feedback" + and msg.args[0] == Pause.RUN.value + ), ) msgs = assert_message_and_return_remaining( msgs, - lambda msg: msg.command == "set" - and msg.obj.name == "attenuator" - and msg.args[0] == 1.0, + lambda msg: ( + msg.command == "set" and msg.obj.name == "attenuator" and msg.args[0] == 1.0 + ), ) diff --git a/tests/unit_tests/hyperion/parameters/test_parameter_model.py b/tests/unit_tests/hyperion/parameters/test_parameter_model.py index 2d30aa70e5..707ceffe40 100644 --- a/tests/unit_tests/hyperion/parameters/test_parameter_model.py +++ b/tests/unit_tests/hyperion/parameters/test_parameter_model.py @@ -3,12 +3,12 @@ from unittest.mock import patch import pytest -from dodal.devices.aperturescatterguard import ApertureValue from pydantic import ValidationError from mx_bluesky.common.external_interaction.callbacks.common.grid_detection_callback import ( GridParamUpdate, ) +from mx_bluesky.common.parameters.components import AperturePolicy from mx_bluesky.common.parameters.constants import GridscanParamConstants from mx_bluesky.common.parameters.rotation import ( SingleRotationScan, @@ -81,6 +81,7 @@ def test_minimal_3d_gridscan_params(minimal_3d_gridscan_params): assert test_params.scan_indices == [0, 35] assert test_params.num_images == (5 * 7 + 5 * 9) assert test_params.exposure_time_s == GridscanParamConstants.EXPOSURE_TIME_S + assert test_params.selected_aperture == AperturePolicy.AUTO def test_cant_do_panda_fgs_with_odd_y_steps(minimal_3d_gridscan_params): @@ -160,14 +161,14 @@ def test_osc_is_used(tmp_path): assert params.num_images == int(params.scan_width_deg / osc) -def test_selected_aperture_uses_default(tmp_path): +def test_rotation_selected_aperture_default_is_auto(tmp_path): raw_params = raw_params_from_file( "tests/test_data/parameter_json_files/good_test_rotation_scan_parameters.json", tmp_path, ) - raw_params["selected_aperture"] = None + del raw_params["selected_aperture"] params = SingleRotationScan(**raw_params) - assert params.selected_aperture == ApertureValue.LARGE + assert params.selected_aperture == AperturePolicy.AUTO @pytest.mark.parametrize( From c5d6621499403cc302145b7e28ca361afd9eed9c Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Tue, 21 Apr 2026 14:37:46 +0100 Subject: [PATCH 29/36] Fix typo in docs --- docs/developer/hyperion/reference/blueapi_schema.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/hyperion/reference/blueapi_schema.rst b/docs/developer/hyperion/reference/blueapi_schema.rst index 1da24497ae..d5e3e33fc8 100644 --- a/docs/developer/hyperion/reference/blueapi_schema.rst +++ b/docs/developer/hyperion/reference/blueapi_schema.rst @@ -1,4 +1,4 @@ BlueAPI Schema ============== -.. jsonchema:: blueapi_schema.json +.. jsonschema:: blueapi_schema.json From 0e682957d0faf645a51795ee20dbc1418a85e2e0 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Tue, 21 Apr 2026 15:11:41 +0100 Subject: [PATCH 30/36] Attempt at parameter docs from schema --- .../reference/blueapi/clean_up_udc.json | 1 + .../blueapi/load_centre_collect.json | 1 + .../blueapi/move_to_udc_default_state.json | 1 + .../reference/blueapi/robot_unload.json | 1 + .../hyperion/reference/blueapi_schema.rst | 2 +- utility_scripts/generate_parameter_schemas.py | 29 +++++++++++++++++++ 6 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 docs/developer/hyperion/reference/blueapi/clean_up_udc.json create mode 100644 docs/developer/hyperion/reference/blueapi/load_centre_collect.json create mode 100644 docs/developer/hyperion/reference/blueapi/move_to_udc_default_state.json create mode 100644 docs/developer/hyperion/reference/blueapi/robot_unload.json create mode 100644 utility_scripts/generate_parameter_schemas.py diff --git a/docs/developer/hyperion/reference/blueapi/clean_up_udc.json b/docs/developer/hyperion/reference/blueapi/clean_up_udc.json new file mode 100644 index 0000000000..087f7576f4 --- /dev/null +++ b/docs/developer/hyperion/reference/blueapi/clean_up_udc.json @@ -0,0 +1 @@ +{"$schema": "https://json-schema.org/draft/2020-12/schema", "description": "Perform actions that are always performed on ending UDC, regardless of whether\n it terminated abnormally or not.\n Currently the only thing this does is close the detector shutter.\n Args:\n detector_motion:\n\n Returns:", "additionalProperties": false, "properties": {"cleanup_group": {"title": "Cleanup Group", "type": "string"}, "detector_motion": {"enum": ["detector_motion"], "title": "Detector Motion", "type": "dodal.devices.detector.detector_motion.DetectorMotion"}}, "title": "clean_up_udc", "type": "object"} diff --git a/docs/developer/hyperion/reference/blueapi/load_centre_collect.json b/docs/developer/hyperion/reference/blueapi/load_centre_collect.json new file mode 100644 index 0000000000..984f9ef5e1 --- /dev/null +++ b/docs/developer/hyperion/reference/blueapi/load_centre_collect.json @@ -0,0 +1 @@ +{"$schema": "https://json-schema.org/draft/2020-12/schema", "description": "Attempt a complete data collection experiment, consisting of the following:\n * Load the sample if necessary\n * Move to the specified goniometer start angles\n * Perform optical centring, then X-ray centring\n * If X-ray centring finds one or more diffracting centres then for each centre\n that satisfies the chosen selection function,\n move to that centre and do a collection with the specified parameters.", "$defs": {"LoadCentreCollectParams": {"additionalProperties": false, "description": "This model is exposed as the BlueAPI REST parameter model for Hyperion Collections.\nIt can represent the full range of operations that are supported by LoadCentreCollect;\nthis is a superset of the operations that are actually required to follow Agamemnon instructions.\n\nThis is intended to provide additional flexibility to allow a potential future point of configuration\nwhere the supervisor can supply default values for values not explicitly specified by Agamemnon,\nalso to allow future exposure of this BlueAPI plan for e.g. commissioning purposes.\nThis may also permit the supervisor to implement future functionality by adjusting these parameters.", "properties": {"select_centres": {"default": {"name": "TopNByMaxCount", "ignore_xtal_not_found": false, "n": 1}, "discriminator": {"mapping": {"TopNByMaxCount": "#/$defs/TopNByMaxCountSelection", "TopNByMaxCountForEachSample": "#/$defs/TopNByMaxCountForEachSampleSelection"}, "propertyName": "name"}, "oneOf": [{"$ref": "#/$defs/TopNByMaxCountSelection"}, {"$ref": "#/$defs/TopNByMaxCountForEachSampleSelection"}], "title": "Select Centres"}, "visit": {"title": "Visit", "type": "string"}, "detector_distance_mm": {"title": "Detector Distance Mm", "type": "number"}, "sample_id": {"title": "Sample Id", "type": "integer"}, "sample_puck": {"title": "Sample Puck", "type": "integer"}, "sample_pin": {"title": "Sample Pin", "type": "integer"}, "demand_energy_ev": {"title": "Demand Energy Ev", "type": "number"}, "robot_load_then_centre": {"$ref": "#/$defs/RobotLoadThenCentreParams"}, "multi_rotation_scan": {"$ref": "#/$defs/MultiRotationScanParams"}}, "required": ["visit", "detector_distance_mm", "sample_id", "sample_puck", "sample_pin", "demand_energy_ev", "robot_load_then_centre", "multi_rotation_scan"], "title": "LoadCentreCollectParams", "type": "object"}, "MultiRotationScanParams": {"additionalProperties": false, "properties": {"comment": {"title": "Comment", "type": "string"}, "file_name": {"title": "File Name", "type": "string"}, "storage_directory": {"title": "Storage Directory", "type": "string"}, "exposure_time_s": {"title": "Exposure Time S", "type": "number"}, "rotation_increment_deg": {"title": "Rotation Increment Deg", "type": "number"}, "snapshot_omegas_deg": {"items": {"type": "number"}, "title": "Snapshot Omegas Deg", "type": "array"}, "rotation_scans": {"items": {"$ref": "#/$defs/SingleRotationScanParams"}, "title": "Rotation Scans", "type": "array"}, "transmission_frac": {"title": "Transmission Frac", "type": "number"}, "ispyb_experiment_type": {"title": "Ispyb Experiment Type", "type": "string"}}, "required": ["comment", "file_name", "storage_directory", "exposure_time_s", "rotation_increment_deg", "snapshot_omegas_deg", "rotation_scans", "transmission_frac", "ispyb_experiment_type"], "title": "MultiRotationScanParams", "type": "object"}, "RobotLoadThenCentreParams": {"additionalProperties": false, "properties": {"storage_directory": {"title": "Storage Directory", "type": "string"}, "file_name": {"title": "File Name", "type": "string"}, "transmission_frac": {"title": "Transmission Frac", "type": "number"}, "exposure_time_s": {"title": "Exposure Time S", "type": "number"}, "omega_start_deg": {"title": "Omega Start Deg", "type": "number"}, "chi_start_deg": {"title": "Chi Start Deg", "type": "number"}, "tip_offset_um": {"title": "Tip Offset Um", "type": "number"}, "grid_width_um": {"title": "Grid Width Um", "type": "number"}}, "required": ["storage_directory", "file_name", "transmission_frac", "exposure_time_s", "omega_start_deg", "chi_start_deg", "tip_offset_um", "grid_width_um"], "title": "RobotLoadThenCentreParams", "type": "object"}, "SingleRotationScanParams": {"additionalProperties": false, "properties": {"omega_start_deg": {"title": "Omega Start Deg", "type": "number"}, "phi_start_deg": {"title": "Phi Start Deg", "type": "number"}, "chi_start_deg": {"title": "Chi Start Deg", "type": "number"}, "rotation_direction": {"title": "Rotation Direction", "type": "string"}, "scan_width_deg": {"title": "Scan Width Deg", "type": "number"}}, "required": ["omega_start_deg", "phi_start_deg", "chi_start_deg", "rotation_direction", "scan_width_deg"], "title": "SingleRotationScanParams", "type": "object"}, "TopNByMaxCountForEachSampleSelection": {"properties": {"name": {"const": "TopNByMaxCountForEachSample", "default": "TopNByMaxCountForEachSample", "title": "Name", "type": "string"}, "ignore_xtal_not_found": {"default": false, "title": "Ignore Xtal Not Found", "type": "boolean"}, "n": {"title": "N", "type": "integer"}}, "required": ["n"], "title": "TopNByMaxCountForEachSampleSelection", "type": "object"}, "TopNByMaxCountSelection": {"properties": {"name": {"const": "TopNByMaxCount", "default": "TopNByMaxCount", "title": "Name", "type": "string"}, "ignore_xtal_not_found": {"default": false, "title": "Ignore Xtal Not Found", "type": "boolean"}, "n": {"title": "N", "type": "integer"}}, "required": ["n"], "title": "TopNByMaxCountSelection", "type": "object"}}, "additionalProperties": false, "properties": {"parameters": {"$ref": "#/$defs/LoadCentreCollectParams"}}, "required": ["parameters"], "title": "load_centre_collect", "type": "object"} diff --git a/docs/developer/hyperion/reference/blueapi/move_to_udc_default_state.json b/docs/developer/hyperion/reference/blueapi/move_to_udc_default_state.json new file mode 100644 index 0000000000..f6e46ad89d --- /dev/null +++ b/docs/developer/hyperion/reference/blueapi/move_to_udc_default_state.json @@ -0,0 +1 @@ +{"$schema": "https://json-schema.org/draft/2020-12/schema", "description": "Move beamline hardware to known positions prior to UDC start.", "additionalProperties": false, "properties": {}, "title": "move_to_udc_default_state", "type": "object"} diff --git a/docs/developer/hyperion/reference/blueapi/robot_unload.json b/docs/developer/hyperion/reference/blueapi/robot_unload.json new file mode 100644 index 0000000000..48c6f57a96 --- /dev/null +++ b/docs/developer/hyperion/reference/blueapi/robot_unload.json @@ -0,0 +1 @@ +{"$schema": "https://json-schema.org/draft/2020-12/schema", "description": "Unload the currently mounted pin into the location that it was loaded from.\n This is to be invoked as the final step upon successful completion of the UDC queue.", "additionalProperties": false, "properties": {"visit": {"title": "Visit", "type": "string"}, "robot": {"enum": ["robot"], "title": "Robot", "type": "dodal.devices.robot.BartRobot"}, "smargon": {"enum": ["gonio"], "title": "Smargon", "type": "dodal.devices.smargon.Smargon"}, "aperture_scatterguard": {"enum": ["aperture_scatterguard"], "title": "Aperture Scatterguard", "type": "dodal.devices.aperturescatterguard.ApertureScatterguard"}, "lower_gonio": {"enum": ["detector_motion", "gonio", "lower_gonio"], "title": "Lower Gonio", "type": "dodal.devices.motors.XYZStage"}}, "required": ["visit"], "title": "robot_unload", "type": "object"} diff --git a/docs/developer/hyperion/reference/blueapi_schema.rst b/docs/developer/hyperion/reference/blueapi_schema.rst index d5e3e33fc8..54b75a1f12 100644 --- a/docs/developer/hyperion/reference/blueapi_schema.rst +++ b/docs/developer/hyperion/reference/blueapi_schema.rst @@ -1,4 +1,4 @@ BlueAPI Schema ============== -.. jsonschema:: blueapi_schema.json +.. jsonschema:: blueapi/*.json diff --git a/utility_scripts/generate_parameter_schemas.py b/utility_scripts/generate_parameter_schemas.py new file mode 100644 index 0000000000..f0b1f82965 --- /dev/null +++ b/utility_scripts/generate_parameter_schemas.py @@ -0,0 +1,29 @@ +import json +from argparse import ArgumentParser +from pathlib import Path + + +def main(): + parser = ArgumentParser() + parser.add_argument("input_file") + parser.add_argument("output_dir") + args = parser.parse_args() + + with open(args.input_file) as stream: + json_str = stream.read() + json_blob = json.loads(json_str) + + for plan_dict in json_blob["plans"]: + name = plan_dict["name"] + schema_blob = { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "description": plan_dict["description"].strip(), + **plan_dict["schema"], + } + output_path = Path(args.output_dir + "/" + name + ".json") + with open(output_path, "w") as output_stream: + output_stream.write(json.dumps(schema_blob)) + + +if __name__ == "__main__": + main() From 4e82f54afbe1066ab12f2cfc3fe3fa56466665f8 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Wed, 22 Apr 2026 13:10:32 +0100 Subject: [PATCH 31/36] Implement AperturePolicy, unit tests fixed --- .../hyperion/reference/blueapi_schema.rst | 4 +- .../change_aperture_then_move_plan.py | 69 ----------- .../common/parameters/components.py | 2 +- .../common/utils/aperture_selection.py | 36 ++++++ src/mx_bluesky/hyperion/blueapi/parameters.py | 2 + .../load_centre_collect_full_plan.py | 48 ++++++-- .../external_interaction/agamemnon.py | 7 ++ .../hyperion/parameters/constants.py | 1 + .../hyperion/utils/centre_selection.py | 27 +++-- .../external_load_centre_collect_params.json | 1 + .../test_change_aperture_then_move_plan.py | 105 ----------------- .../test_load_centre_collect_full_plan.py | 110 ++++++++++++++---- .../external_interaction/test_agamemnon.py | 33 ++++++ 13 files changed, 227 insertions(+), 218 deletions(-) create mode 100644 src/mx_bluesky/common/utils/aperture_selection.py delete mode 100644 tests/unit_tests/common/experiment_plans/test_change_aperture_then_move_plan.py diff --git a/docs/developer/hyperion/reference/blueapi_schema.rst b/docs/developer/hyperion/reference/blueapi_schema.rst index 54b75a1f12..862b706662 100644 --- a/docs/developer/hyperion/reference/blueapi_schema.rst +++ b/docs/developer/hyperion/reference/blueapi_schema.rst @@ -1,4 +1,6 @@ BlueAPI Schema ============== -.. jsonschema:: blueapi/*.json +.. jsonschema:: + {"$schema": "https://json-schema.org/draft/2020-12/schema", "description": "Perform actions that are always performed on ending UDC, regardless of whether\n it terminated abnormally or not.\n Currently the only thing this does is close the detector shutter.\n Args:\n detector_motion:\n\n Returns:", "additionalProperties": false, "properties": {"cleanup_group": {"title": "Cleanup Group", "type": "string"}, "detector_motion": {"enum": ["detector_motion"], "title": "Detector Motion", "type": "dodal.devices.detector.detector_motion.DetectorMotion"}}, "title": "clean_up_udc", "type": "object"} + diff --git a/src/mx_bluesky/common/experiment_plans/change_aperture_then_move_plan.py b/src/mx_bluesky/common/experiment_plans/change_aperture_then_move_plan.py index 860a5ed7a4..0282ba2e49 100644 --- a/src/mx_bluesky/common/experiment_plans/change_aperture_then_move_plan.py +++ b/src/mx_bluesky/common/experiment_plans/change_aperture_then_move_plan.py @@ -2,8 +2,6 @@ from typing import Any import bluesky.plan_stubs as bps -import numpy -from dodal.devices.aperturescatterguard import ApertureScatterguard, ApertureValue from dodal.devices.smargon import Smargon, StubPosition from dodal.devices.zocalo import ZocaloResults @@ -11,7 +9,6 @@ from mx_bluesky.common.experiment_plans.inner_plans.xrc_results_utils import ( fetch_xrc_results_from_zocalo, ) -from mx_bluesky.common.parameters.constants import PlanGroupCheckpointConstants from mx_bluesky.common.parameters.device_composites import ( GridDetectThenXRayCentreComposite, ) @@ -45,35 +42,6 @@ def get_results_and_move_to_xtal( yield from move_to_xtal(flyscan_results[0], composite.gonio) -# Currently not being used, but see https://github.com/DiamondLightSource/mx-bluesky/issues/561 -def get_results_then_change_aperture_and_move_to_xtal( - composite: GridDetectThenXRayCentreComposite, - parameters: SpecifiedThreeDGridScan, - flyscan_event_handler: XRayCentreEventHandler, -): - flyscan_results = yield from _get_xrc_results( - composite.zocalo, parameters, flyscan_event_handler - ) - yield from change_aperture(flyscan_results[0], composite.aperture_scatterguard) - yield from move_to_xtal(flyscan_results[0], composite.gonio) - - -def change_aperture( - best_hit: XRayCentreResult, - aperture_scatterguard: ApertureScatterguard, -): - """For the given x-ray centring result, - * Change the aperture so that the beam size is comparable to the crystal size - """ - bounding_box_size = numpy.abs( - best_hit.bounding_box_mm[1] - best_hit.bounding_box_mm[0] - ) - yield from _set_aperture_for_bbox_mm( - aperture_scatterguard, - bounding_box_size, - ) - - def move_to_xtal( best_hit: XRayCentreResult, smargon: Smargon, @@ -93,40 +61,3 @@ def move_to_xtal( if set_stub_offsets: LOGGER.info("Recentring smargon co-ordinate system to this point.") yield from bps.mv(smargon.stub_offsets, StubPosition.CURRENT_AS_CENTER) - - -def _set_aperture_for_bbox_mm( - aperture_device: ApertureScatterguard, - bbox_size_mm: list[float] | numpy.ndarray, - group=PlanGroupCheckpointConstants.GRID_READY_FOR_DC, -): - """Sets aperture size based on bbox_size. - - This function determines the aperture size needed to accommodate the bounding box - of a crystal. The x-axis length of the bounding box is used, setting the aperture - to Medium if this is less than 50um, and Large otherwise. - - Args: - aperture_device: The aperture scatter guard device we are controlling. - bbox_size_mm: The [x,y,z] lengths, in mm, of a bounding box - containing a crystal. This describes (in no particular order): - * The maximum width a crystal occupies - * The maximum height a crystal occupies - * The maximum depth a crystal occupies - constructing a three dimensional cuboid, completely encapsulating the crystal. - - Yields: - Iterator[MsgGenerator] - """ - - # bbox_size is [x,y,z], for i03 we only care about x - new_selected_aperture = ( - ApertureValue.MEDIUM if bbox_size_mm[0] < 0.05 else ApertureValue.LARGE - ) - LOGGER.info( - f"Setting aperture to {new_selected_aperture} based on bounding box size {bbox_size_mm}." - ) - - yield from bps.abs_set( - aperture_device.selected_aperture, new_selected_aperture, group=group - ) diff --git a/src/mx_bluesky/common/parameters/components.py b/src/mx_bluesky/common/parameters/components.py index 1585500b02..5c09d6fab2 100644 --- a/src/mx_bluesky/common/parameters/components.py +++ b/src/mx_bluesky/common/parameters/components.py @@ -174,7 +174,7 @@ class AperturePolicy(StrEnum): SMALL = "SMALL_APERTURE" MEDIUM = "MEDIUM_APERTURE" LARGE = "LARGE_APERTURE" - AUTO = "AUTO_UNSPECIFIED" + AUTO = "AUTO" CURRENT_POSITION = "CURRENT_POSITION" diff --git a/src/mx_bluesky/common/utils/aperture_selection.py b/src/mx_bluesky/common/utils/aperture_selection.py new file mode 100644 index 0000000000..4905a63ac6 --- /dev/null +++ b/src/mx_bluesky/common/utils/aperture_selection.py @@ -0,0 +1,36 @@ +import numpy + +from mx_bluesky.common.parameters.components import AperturePolicy +from mx_bluesky.common.utils.log import LOGGER + + +def select_aperture_for_bbox_mm( + bbox_size_mm: list[float] | numpy.ndarray, xtal_width_threshold_mm: float +) -> AperturePolicy: + """Sets aperture size based on bbox_size. + + This function determines the aperture size needed to accommodate the bounding box + of a crystal. The x-axis length of the bounding box is used, setting the aperture + to Medium if this is less than the threshold size, and Large otherwise. + + Args: + bbox_size_mm: The [x,y,z] lengths, in mm, of a bounding box + containing a crystal. This describes (in no particular order): + * The maximum width a crystal occupies + * The maximum height a crystal occupies + * The maximum depth a crystal occupies + constructing a three-dimensional cuboid, completely encapsulating the crystal. + xtal_width_threshold_mm (float): Threshold width below which medium aperture is selected + Returns: The selected aperture policy + """ + + # bbox_size is [x,y,z], for i03 we only care about x + new_selected_aperture = ( + AperturePolicy.MEDIUM + if bbox_size_mm[0] < xtal_width_threshold_mm + else AperturePolicy.LARGE + ) + LOGGER.info( + f"Setting aperture to {new_selected_aperture} based on bounding box size {bbox_size_mm}." + ) + return new_selected_aperture diff --git a/src/mx_bluesky/hyperion/blueapi/parameters.py b/src/mx_bluesky/hyperion/blueapi/parameters.py index 117367aee9..7e41fcf9a6 100644 --- a/src/mx_bluesky/hyperion/blueapi/parameters.py +++ b/src/mx_bluesky/hyperion/blueapi/parameters.py @@ -8,6 +8,7 @@ from pydantic.config import ConfigDict from mx_bluesky.common.parameters.components import ( + AperturePolicy, get_param_version, ) from mx_bluesky.common.parameters.constants import GridscanParamConstants @@ -65,6 +66,7 @@ class MultiRotationScanParams(HyperionParam): snapshot_omegas_deg: list[float] rotation_scans: list[SingleRotationScanParams] transmission_frac: float + selected_aperture: AperturePolicy ispyb_experiment_type: str diff --git a/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py b/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py index 1aeca06f6c..cb6c7750ac 100644 --- a/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py +++ b/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py @@ -11,14 +11,15 @@ from dodal.devices.baton import Baton from dodal.devices.oav.oav_parameters import OAVParameters -from mx_bluesky.common.parameters.components import WithSnapshot +from mx_bluesky.common.parameters.components import AperturePolicy, WithSnapshot from mx_bluesky.common.parameters.rotation import ( RotationScanPerSweep, ) +from mx_bluesky.common.utils.aperture_selection import select_aperture_for_bbox_mm from mx_bluesky.common.utils.context import device_composite_from_context from mx_bluesky.common.utils.exceptions import CrystalNotFoundError from mx_bluesky.common.utils.log import LOGGER -from mx_bluesky.common.utils.xrc_result import XRayCentreEventHandler +from mx_bluesky.common.utils.xrc_result import XRayCentreEventHandler, XRayCentreResult from mx_bluesky.hyperion.experiment_plans.robot_load_then_centre_plan import ( RobotLoadThenCentreComposite, robot_load_then_xray_centre, @@ -30,7 +31,7 @@ ) from mx_bluesky.hyperion.parameters.constants import CONST, I03Constants from mx_bluesky.hyperion.parameters.load_centre_collect import LoadCentreCollect -from mx_bluesky.hyperion.utils.centre_selection import samples_and_locations_to_collect +from mx_bluesky.hyperion.utils.centre_selection import samples_and_hits_to_collect @pydantic.dataclasses.dataclass(config={"arbitrary_types_allowed": True}) @@ -101,17 +102,17 @@ def plan_with_callback_subs(): else: raise - sample_ids_and_locations = yield from ( - samples_and_locations_to_collect( - parameters.selection_params, + sample_ids_and_hits = yield from ( + samples_and_hits_to_collect( + parameters, composite.gonio, - parameters.sample_id, flyscan_event_handler.xray_centre_results, ) ) - sample_ids_and_locations.sort(key=_x_coordinate) + sample_ids_and_hits.sort(key=_x_coordinate) multi_rotation = parameters.multi_rotation_scan + _update_aperture_selection(multi_rotation, sample_ids_and_hits) rotation_template = multi_rotation.rotation_scans.copy() multi_rotation.rotation_scans.clear() @@ -120,9 +121,11 @@ def plan_with_callback_subs(): generator = rotation_scan_generator(is_alternating) next(generator) - for sample_id, location in sample_ids_and_locations: + for sample_id, xray_centre_result in sample_ids_and_hits: for rot in rotation_template: - combination = generator.send((rot, location, sample_id)) + combination = generator.send( + (rot, xray_centre_result.centre_of_mass_mm * 1000, sample_id) + ) multi_rotation.rotation_scans.append(combination) multi_rotation = RotationScan.model_validate(multi_rotation) @@ -135,8 +138,29 @@ def plan_with_callback_subs(): yield from plan_with_callback_subs() -def _x_coordinate(sample_and_location: tuple[int, np.ndarray]) -> float: - return sample_and_location[1][0] # type: ignore +def _update_aperture_selection( + multi_rotation_scan: RotationScan, + sample_ids_and_hits: list[tuple[int, XRayCentreResult]], +): + """Select aperture if auto selection is specified. If more than one crystal is found (i.e. multipin), + select the large aperture, otherwise select based on crystal dimensions.""" + assert len(sample_ids_and_hits) > 0 + if multi_rotation_scan.selected_aperture == AperturePolicy.AUTO: + if len(sample_ids_and_hits) > 1: + multi_rotation_scan.selected_aperture = AperturePolicy.LARGE + else: + first_hit = sample_ids_and_hits[0][1] + bbox_size = first_hit.bounding_box_mm[1] - first_hit.bounding_box_mm[0] + multi_rotation_scan.selected_aperture = select_aperture_for_bbox_mm( + bbox_size, I03Constants.APERTURE_SELECTION_XTAL_WIDTH_THRESHOLD_MM + ) + + +def _x_coordinate( + sample_id_and_xrc_result: tuple[int, XRayCentreResult], +) -> float: + location = sample_id_and_xrc_result[1].centre_of_mass_mm + return location[0] # type: ignore def rotation_scan_generator( diff --git a/src/mx_bluesky/hyperion/external_interaction/agamemnon.py b/src/mx_bluesky/hyperion/external_interaction/agamemnon.py index e355b65d54..80b26c9422 100644 --- a/src/mx_bluesky/hyperion/external_interaction/agamemnon.py +++ b/src/mx_bluesky/hyperion/external_interaction/agamemnon.py @@ -11,6 +11,7 @@ from pydantic import BaseModel from mx_bluesky.common.parameters.components import ( + AperturePolicy, WithVisit, ) from mx_bluesky.common.parameters.constants import ( @@ -153,6 +154,11 @@ def _populate_parameters_from_agamemnon( collections = agamemnon_params["collection"] visit_directory, file_name = path.split(agamemnon_params["prefix"]) + aperture_policy = ( + AperturePolicy.AUTO + if pin_type.expected_number_of_crystals == 1 + else AperturePolicy.LARGE + ) return [ LoadCentreCollectParams.model_validate( { @@ -181,6 +187,7 @@ def _populate_parameters_from_agamemnon( "exposure_time_s": collection["exposure_time"], "file_name": file_name, "transmission_frac": collection["transmission"], + "selected_aperture": aperture_policy, "rotation_increment_deg": collection["omega_increment"], "ispyb_experiment_type": collection["experiment_type"], "snapshot_omegas_deg": [0.0, 90.0, 180.0, 270.0], diff --git a/src/mx_bluesky/hyperion/parameters/constants.py b/src/mx_bluesky/hyperion/parameters/constants.py index ba2fb065c5..a9c3c34220 100644 --- a/src/mx_bluesky/hyperion/parameters/constants.py +++ b/src/mx_bluesky/hyperion/parameters/constants.py @@ -19,6 +19,7 @@ class I03Constants: SHUTTER_TIME_S = 0.06 USE_GPU_RESULTS = True ALTERNATE_ROTATION_DIRECTION = True + APERTURE_SELECTION_XTAL_WIDTH_THRESHOLD_MM = 0.021 @dataclass(frozen=True) diff --git a/src/mx_bluesky/hyperion/utils/centre_selection.py b/src/mx_bluesky/hyperion/utils/centre_selection.py index 1eb946d026..ac316f0e5b 100644 --- a/src/mx_bluesky/hyperion/utils/centre_selection.py +++ b/src/mx_bluesky/hyperion/utils/centre_selection.py @@ -8,6 +8,7 @@ from bluesky import plan_stubs as bps from bluesky.utils import MsgGenerator from dodal.devices.smargon import Smargon +from mx_bluesky.common.parameters.constants import GridscanParamConstants from mx_bluesky.common.utils import xrc_result as flyscan_result from mx_bluesky.common.utils.log import LOGGER @@ -17,6 +18,7 @@ TopNByMaxCountForEachSampleSelection, TopNByMaxCountSelection, ) +from mx_bluesky.hyperion.parameters.load_centre_collect import LoadCentreCollect def top_n_by_max_count( @@ -52,19 +54,18 @@ def resolve_selection_fn( raise ValueError(f"Invalid selection function {params.name}") -def samples_and_locations_to_collect( - selection_params: MultiXtalSelection, +def samples_and_hits_to_collect( + parameters: LoadCentreCollect, gonio: Smargon, - default_sample_id: int, xrc_results: Sequence[flyscan_result.XRayCentreResult] | None, -) -> MsgGenerator[list[tuple[int, np.ndarray]]]: +) -> MsgGenerator[list[tuple[int, flyscan_result.XRayCentreResult]]]: """ Determine the sample IDs and positions to collect given the specified selection parameters. If no centres are present, return the default sample ID and current position, so that a collection can be performed without XRC should this be required. """ if xrc_results: - selection_func = resolve_selection_fn(selection_params) + selection_func = resolve_selection_fn(parameters.selection_params) hits = selection_func(xrc_results) hits_to_collect = [] for hit in hits: @@ -89,9 +90,21 @@ def samples_and_locations_to_collect( initial_y_mm = yield from bps.rd(gonio.y.user_readback) initial_z_mm = yield from bps.rd(gonio.z.user_readback) + com_mm = np.array([initial_x_mm, initial_y_mm, initial_z_mm]) + box_width_mm = GridscanParamConstants.BOX_WIDTH_UM / 1000 + return [ ( - default_sample_id, - np.array([initial_x_mm, initial_y_mm, initial_z_mm]) * 1000, + parameters.sample_id, + flyscan_result.XRayCentreResult( + centre_of_mass_mm=com_mm, + bounding_box_mm=( + com_mm - box_width_mm / 2, + com_mm + box_width_mm / 2, + ), + max_count=1000, + total_count=1000, + sample_id=parameters.sample_id, + ), ) ] diff --git a/tests/test_data/parameter_json_files/external_load_centre_collect_params.json b/tests/test_data/parameter_json_files/external_load_centre_collect_params.json index c394d19c83..75bcb72d6c 100644 --- a/tests/test_data/parameter_json_files/external_load_centre_collect_params.json +++ b/tests/test_data/parameter_json_files/external_load_centre_collect_params.json @@ -18,6 +18,7 @@ "file_name": "protk", "storage_directory": "{tmp_data}/123457/", "transmission_frac": 0.05, + "selected_aperture": "AUTO", "exposure_time_s": 0.004, "rotation_increment_deg": 0.1, "ispyb_experiment_type": "SAD", diff --git a/tests/unit_tests/common/experiment_plans/test_change_aperture_then_move_plan.py b/tests/unit_tests/common/experiment_plans/test_change_aperture_then_move_plan.py deleted file mode 100644 index 6ee2bb315f..0000000000 --- a/tests/unit_tests/common/experiment_plans/test_change_aperture_then_move_plan.py +++ /dev/null @@ -1,105 +0,0 @@ -from unittest.mock import MagicMock, patch - -import numpy -import pytest -from bluesky.run_engine import RunEngine -from bluesky.simulators import RunEngineSimulator, assert_message_and_return_remaining -from dodal.devices.aperturescatterguard import ApertureScatterguard, ApertureValue -from dodal.devices.smargon import CombinedMove, Smargon, StubPosition - -from mx_bluesky.common.experiment_plans.change_aperture_then_move_plan import ( - change_aperture, - get_results_then_change_aperture_and_move_to_xtal, - move_to_xtal, -) -from mx_bluesky.common.parameters.device_composites import ( - GridDetectThenXRayCentreComposite, -) -from mx_bluesky.common.parameters.gridscan import SpecifiedThreeDGridScan -from mx_bluesky.common.utils.xrc_result import XRayCentreEventHandler, XRayCentreResult - - -@pytest.fixture -def simple_flyscan_hit(): - return XRayCentreResult( - centre_of_mass_mm=numpy.array([0.1, 0.2, 0.3]), - bounding_box_mm=( - numpy.array([0.09, 0.19, 0.29]), - numpy.array([0.11, 0.21, 0.31]), - ), - max_count=20, - total_count=57, - sample_id=12345, - ) - - -@pytest.mark.parametrize("set_stub_offsets", [True, False]) -def test_change_aperture_then_move_to_xtal_plans_happy_path( - sim_run_engine: RunEngineSimulator, - simple_flyscan_hit: XRayCentreResult, - smargon: Smargon, - aperture_scatterguard: ApertureScatterguard, - set_stub_offsets: bool, -): - msgs = sim_run_engine.simulate_plan( - change_aperture( - simple_flyscan_hit, - aperture_scatterguard, - ) - ) - msgs += sim_run_engine.simulate_plan( - move_to_xtal(simple_flyscan_hit, smargon, set_stub_offsets) - ) - - msgs = assert_message_and_return_remaining( - msgs, - lambda msg: msg.command == "set" - and msg.obj is aperture_scatterguard.selected_aperture - and msg.args[0] == ApertureValue.MEDIUM, - ) - msgs = assert_message_and_return_remaining( - msgs, - lambda msg: msg.command == "set" - and msg.obj is smargon - and msg.args[0] == CombinedMove(x=0.1, y=0.2, z=0.3), - ) - if set_stub_offsets: - assert_message_and_return_remaining( - msgs, - lambda msg: msg.command == "set" - and msg.obj is smargon.stub_offsets - and msg.args[0] == StubPosition.CURRENT_AS_CENTER, - ) - else: - assert all( - not (msg.command == "set" and msg.obj is smargon.stub_offsets) - for msg in msgs - ) - - -@patch( - "mx_bluesky.common.experiment_plans.change_aperture_then_move_plan.change_aperture" -) -@patch("mx_bluesky.common.experiment_plans.change_aperture_then_move_plan.move_to_xtal") -@patch( - "mx_bluesky.common.experiment_plans.change_aperture_then_move_plan.fetch_xrc_results_from_zocalo" -) -def test_get_results_then_change_aperture_and_move_to_xtal_calls_expected_plans( - mock_fetch_results_from_zocalo: MagicMock, - mock_change_aperture: MagicMock, - mock__get_xrc_results: MagicMock, - run_engine: RunEngine, - grid_detect_xrc_devices: GridDetectThenXRayCentreComposite, - test_fgs_params: SpecifiedThreeDGridScan, -): - mock_flyscan_event_handler = MagicMock(spec=XRayCentreEventHandler) - mock_flyscan_event_handler.xray_centre_results = [0] - - run_engine( - get_results_then_change_aperture_and_move_to_xtal( - grid_detect_xrc_devices, test_fgs_params, mock_flyscan_event_handler - ) - ) - mock__get_xrc_results.assert_called_once() - mock_change_aperture.assert_called_once() - mock_fetch_results_from_zocalo.assert_called_once() diff --git a/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py b/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py index 17f1dce497..92cd48bb1a 100644 --- a/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py +++ b/tests/unit_tests/hyperion/experiment_plans/test_load_centre_collect_full_plan.py @@ -1,14 +1,17 @@ import dataclasses from collections.abc import Sequence +from functools import partial from typing import Any from unittest.mock import AsyncMock, MagicMock, call, patch import numpy as np import pytest +from _pytest.fixtures import FixtureRequest +from bluesky import plan_stubs as bps from bluesky.protocols import Location from bluesky.run_engine import RunEngine from bluesky.simulators import RunEngineSimulator, assert_message_and_return_remaining -from bluesky.utils import Msg +from bluesky.utils import Msg, MsgGenerator from dodal.devices.baton import Baton from dodal.devices.mx_phase1.beamstop import BeamstopPositions from dodal.devices.oav.oav_parameters import OAVParameters @@ -18,6 +21,7 @@ from ophyd_async.core import completed_status, set_mock_value from pydantic import ValidationError +from mx_bluesky.common.parameters.components import AperturePolicy from mx_bluesky.common.parameters.gridscan import SpecifiedThreeDGridScan from mx_bluesky.common.parameters.rotation import ( RotationScan, @@ -27,6 +31,7 @@ CrystalNotFoundError, WarningError, ) +from mx_bluesky.common.utils.xrc_result import XRayCentreResult from mx_bluesky.hyperion.blueapi.mixins import ( TopNByMaxCountForEachSampleSelection, ) @@ -85,6 +90,18 @@ ] ] +FLYSCAN_RESULT_MED_APERTURE = dataclasses.replace( + FLYSCAN_RESULT_MED, + bounding_box_mm=(np.array([0.020, 0.020, 0.020]), np.array([0.040, 0.040, 0.040])), + centre_of_mass_mm=np.array([0.030, 0.030, 0.030]), +) + +FLYSCAN_RESULT_LARGE_APERTURE = dataclasses.replace( + FLYSCAN_RESULT_MED, + bounding_box_mm=(np.array([1.020, 1.000, 1.000]), np.array([1.060, 1.020, 1.020])), + centre_of_mass_mm=np.array([1.040, 1.010, 1.010]), +) + @pytest.fixture def load_centre_collect_params_with_patched_create_params( @@ -207,6 +224,16 @@ def mock_multi_rotation_scan(): yield mock_rotation +def plan_returning_xrc_results(xrc_results: Sequence[XRayCentreResult]) -> MsgGenerator: + yield from bps.open_run( + md={ + "run": CONST.PLAN.FLYSCAN_RESULTS, + "xray_centre_results": [dataclasses.asdict(r) for r in xrc_results], + } + ) + yield from bps.close_run() + + def test_can_serialize_load_centre_collect_params(load_centre_collect_params): load_centre_collect_params.model_dump_json() @@ -382,28 +409,6 @@ def test_collect_full_plan_happy_path_invokes_all_steps_and_centres_on_best_flys msgs, lambda msg: msg.command == "open_run" and "xray_centre_results" in msg.kwargs, ) - # TODO re-enable tests see mx-bluesky 561 - # msgs = assert_message_and_return_remaining( - # msgs, lambda msg: msg.command == "set" and msg.args[0] == ApertureValue.MEDIUM - # ) - # msgs = assert_message_and_return_remaining( - # msgs, - # lambda msg: msg.command == "set" - # and msg.obj.name == "gonio-x" - # and msg.args[0] == 0.1, - # ) - # msgs = assert_message_and_return_remaining( - # msgs, - # lambda msg: msg.command == "set" - # and msg.obj.name == "gonio-y" - # and msg.args[0] == 0.2, - # ) - # msgs = assert_message_and_return_remaining( - # msgs, - # lambda msg: msg.command == "set" - # and msg.obj.name == "gonio-z" - # and msg.args[0] == 0.3, - # ) msgs = assert_message_and_return_remaining( msgs, lambda msg: msg.command == "multi_rotation_scan" ) @@ -1137,3 +1142,62 @@ def test_load_centre_collect_full_activates_beam_drawing_callback( msgs = assert_message_and_return_remaining( msgs, lambda msg: msg.command == "robot_load_then_xray_centre" ) + + +@pytest.fixture +def mock_gridscan_results(request: FixtureRequest): + results = request.param + with patch( + "mx_bluesky.hyperion.experiment_plans.robot_load_then_centre_plan.pin_centre_then_gridscan_plan", + MagicMock( + return_value=partial(plan_returning_xrc_results, xrc_results=results)() + ), + ): + yield results + + +@pytest.mark.parametrize( + "mock_gridscan_results, requested_aperture, expected_aperture", + [ + [[FLYSCAN_RESULT_MED_APERTURE], AperturePolicy.AUTO, AperturePolicy.MEDIUM], + [[FLYSCAN_RESULT_MED_APERTURE], AperturePolicy.SMALL, AperturePolicy.SMALL], + [[FLYSCAN_RESULT_MED_APERTURE], AperturePolicy.LARGE, AperturePolicy.LARGE], + [[FLYSCAN_RESULT_LARGE_APERTURE], AperturePolicy.AUTO, AperturePolicy.LARGE], + [ + [FLYSCAN_RESULT_MED_APERTURE, FLYSCAN_RESULT_LARGE_APERTURE], + AperturePolicy.AUTO, + AperturePolicy.LARGE, + ], + ], + indirect=["mock_gridscan_results"], +) +@patch( + "mx_bluesky.hyperion.experiment_plans.load_centre_collect_full_plan.rotation_scan_internal" +) +def test_load_centre_collect_applies_aperture_for_single_result_based_on_xtal_size( + mock_rotation_scan_internal: MagicMock, + mock_gridscan_results: list[XRayCentreResult], + requested_aperture: AperturePolicy, + expected_aperture: AperturePolicy, + sim_run_engine: RunEngineSimulator, + load_centre_collect_with_top_n_params: LoadCentreCollect, + oav_parameters_for_rotation: OAVParameters, + composite: LoadCentreCollectComposite, +): + load_centre_collect_with_top_n_params.multi_rotation_scan.selected_aperture = ( + requested_aperture + ) + mock_rotation_scan_internal.return_value = iter([]) + sim_run_engine.add_handler_for_callback_subscribes() + sim_fire_event_on_open_run(sim_run_engine, CONST.PLAN.FLYSCAN_RESULTS) + sim_run_engine.simulate_plan( + load_centre_collect_full( + composite, + load_centre_collect_with_top_n_params, + oav_parameters_for_rotation, + ) + ) + rotation_scan_calls = mock_rotation_scan_internal.mock_calls + assert len(rotation_scan_calls) == 1 + params = rotation_scan_calls[0].args[1] + assert params.selected_aperture == expected_aperture diff --git a/tests/unit_tests/hyperion/external_interaction/test_agamemnon.py b/tests/unit_tests/hyperion/external_interaction/test_agamemnon.py index bd3ea24824..04a72ae947 100644 --- a/tests/unit_tests/hyperion/external_interaction/test_agamemnon.py +++ b/tests/unit_tests/hyperion/external_interaction/test_agamemnon.py @@ -8,6 +8,7 @@ import pytest from dodal.devices.zebra.zebra import RotationDirection +from mx_bluesky.common.parameters.components import AperturePolicy from mx_bluesky.hyperion._plan_runner_params import Wait from mx_bluesky.hyperion.blueapi.parameters import ( LoadCentreCollectParams, @@ -417,3 +418,35 @@ def test_create_parameters_from_agamemnon_creates_wait(agamemnon_response): assert len(params) == 1 assert isinstance(params[0], Wait) assert params[0].duration_s == 12.34 + + +@pytest.mark.parametrize( + "agamemnon_response", + ["tests/test_data/agamemnon/example_collect_multipin.json"], + indirect=True, +) +def test_create_parameters_from_agamemnon_selects_large_aperture_for_multipin( + agamemnon_response, +): + params = create_parameters_from_agamemnon() + load_centre_collect_params: LoadCentreCollectParams = params[0] # type: ignore + assert ( + load_centre_collect_params.multi_rotation_scan.selected_aperture + == AperturePolicy.LARGE + ) + + +@pytest.mark.parametrize( + "agamemnon_response", + ["tests/test_data/agamemnon/example_native.json"], + indirect=True, +) +def test_create_parameters_from_agamemnon_selects_auto_aperture_for_single_sample_pin( + agamemnon_response, +): + params = create_parameters_from_agamemnon() + load_centre_collect_params: LoadCentreCollectParams = params[0] # type: ignore + assert ( + load_centre_collect_params.multi_rotation_scan.selected_aperture + == AperturePolicy.AUTO + ) From 3f3c4d8764a1f5f2cea205509d58f3a9fc28a077 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Wed, 22 Apr 2026 13:12:05 +0100 Subject: [PATCH 32/36] Revert changes relating to blueapi parameter documentation --- .../reference/blueapi/clean_up_udc.json | 1 - .../blueapi/load_centre_collect.json | 1 - .../blueapi/move_to_udc_default_state.json | 1 - .../reference/blueapi/robot_unload.json | 1 - .../hyperion/reference/blueapi_schema.rst | 6 ---- utility_scripts/generate_parameter_schemas.py | 29 ------------------- 6 files changed, 39 deletions(-) delete mode 100644 docs/developer/hyperion/reference/blueapi/clean_up_udc.json delete mode 100644 docs/developer/hyperion/reference/blueapi/load_centre_collect.json delete mode 100644 docs/developer/hyperion/reference/blueapi/move_to_udc_default_state.json delete mode 100644 docs/developer/hyperion/reference/blueapi/robot_unload.json delete mode 100644 docs/developer/hyperion/reference/blueapi_schema.rst delete mode 100644 utility_scripts/generate_parameter_schemas.py diff --git a/docs/developer/hyperion/reference/blueapi/clean_up_udc.json b/docs/developer/hyperion/reference/blueapi/clean_up_udc.json deleted file mode 100644 index 087f7576f4..0000000000 --- a/docs/developer/hyperion/reference/blueapi/clean_up_udc.json +++ /dev/null @@ -1 +0,0 @@ -{"$schema": "https://json-schema.org/draft/2020-12/schema", "description": "Perform actions that are always performed on ending UDC, regardless of whether\n it terminated abnormally or not.\n Currently the only thing this does is close the detector shutter.\n Args:\n detector_motion:\n\n Returns:", "additionalProperties": false, "properties": {"cleanup_group": {"title": "Cleanup Group", "type": "string"}, "detector_motion": {"enum": ["detector_motion"], "title": "Detector Motion", "type": "dodal.devices.detector.detector_motion.DetectorMotion"}}, "title": "clean_up_udc", "type": "object"} diff --git a/docs/developer/hyperion/reference/blueapi/load_centre_collect.json b/docs/developer/hyperion/reference/blueapi/load_centre_collect.json deleted file mode 100644 index 984f9ef5e1..0000000000 --- a/docs/developer/hyperion/reference/blueapi/load_centre_collect.json +++ /dev/null @@ -1 +0,0 @@ -{"$schema": "https://json-schema.org/draft/2020-12/schema", "description": "Attempt a complete data collection experiment, consisting of the following:\n * Load the sample if necessary\n * Move to the specified goniometer start angles\n * Perform optical centring, then X-ray centring\n * If X-ray centring finds one or more diffracting centres then for each centre\n that satisfies the chosen selection function,\n move to that centre and do a collection with the specified parameters.", "$defs": {"LoadCentreCollectParams": {"additionalProperties": false, "description": "This model is exposed as the BlueAPI REST parameter model for Hyperion Collections.\nIt can represent the full range of operations that are supported by LoadCentreCollect;\nthis is a superset of the operations that are actually required to follow Agamemnon instructions.\n\nThis is intended to provide additional flexibility to allow a potential future point of configuration\nwhere the supervisor can supply default values for values not explicitly specified by Agamemnon,\nalso to allow future exposure of this BlueAPI plan for e.g. commissioning purposes.\nThis may also permit the supervisor to implement future functionality by adjusting these parameters.", "properties": {"select_centres": {"default": {"name": "TopNByMaxCount", "ignore_xtal_not_found": false, "n": 1}, "discriminator": {"mapping": {"TopNByMaxCount": "#/$defs/TopNByMaxCountSelection", "TopNByMaxCountForEachSample": "#/$defs/TopNByMaxCountForEachSampleSelection"}, "propertyName": "name"}, "oneOf": [{"$ref": "#/$defs/TopNByMaxCountSelection"}, {"$ref": "#/$defs/TopNByMaxCountForEachSampleSelection"}], "title": "Select Centres"}, "visit": {"title": "Visit", "type": "string"}, "detector_distance_mm": {"title": "Detector Distance Mm", "type": "number"}, "sample_id": {"title": "Sample Id", "type": "integer"}, "sample_puck": {"title": "Sample Puck", "type": "integer"}, "sample_pin": {"title": "Sample Pin", "type": "integer"}, "demand_energy_ev": {"title": "Demand Energy Ev", "type": "number"}, "robot_load_then_centre": {"$ref": "#/$defs/RobotLoadThenCentreParams"}, "multi_rotation_scan": {"$ref": "#/$defs/MultiRotationScanParams"}}, "required": ["visit", "detector_distance_mm", "sample_id", "sample_puck", "sample_pin", "demand_energy_ev", "robot_load_then_centre", "multi_rotation_scan"], "title": "LoadCentreCollectParams", "type": "object"}, "MultiRotationScanParams": {"additionalProperties": false, "properties": {"comment": {"title": "Comment", "type": "string"}, "file_name": {"title": "File Name", "type": "string"}, "storage_directory": {"title": "Storage Directory", "type": "string"}, "exposure_time_s": {"title": "Exposure Time S", "type": "number"}, "rotation_increment_deg": {"title": "Rotation Increment Deg", "type": "number"}, "snapshot_omegas_deg": {"items": {"type": "number"}, "title": "Snapshot Omegas Deg", "type": "array"}, "rotation_scans": {"items": {"$ref": "#/$defs/SingleRotationScanParams"}, "title": "Rotation Scans", "type": "array"}, "transmission_frac": {"title": "Transmission Frac", "type": "number"}, "ispyb_experiment_type": {"title": "Ispyb Experiment Type", "type": "string"}}, "required": ["comment", "file_name", "storage_directory", "exposure_time_s", "rotation_increment_deg", "snapshot_omegas_deg", "rotation_scans", "transmission_frac", "ispyb_experiment_type"], "title": "MultiRotationScanParams", "type": "object"}, "RobotLoadThenCentreParams": {"additionalProperties": false, "properties": {"storage_directory": {"title": "Storage Directory", "type": "string"}, "file_name": {"title": "File Name", "type": "string"}, "transmission_frac": {"title": "Transmission Frac", "type": "number"}, "exposure_time_s": {"title": "Exposure Time S", "type": "number"}, "omega_start_deg": {"title": "Omega Start Deg", "type": "number"}, "chi_start_deg": {"title": "Chi Start Deg", "type": "number"}, "tip_offset_um": {"title": "Tip Offset Um", "type": "number"}, "grid_width_um": {"title": "Grid Width Um", "type": "number"}}, "required": ["storage_directory", "file_name", "transmission_frac", "exposure_time_s", "omega_start_deg", "chi_start_deg", "tip_offset_um", "grid_width_um"], "title": "RobotLoadThenCentreParams", "type": "object"}, "SingleRotationScanParams": {"additionalProperties": false, "properties": {"omega_start_deg": {"title": "Omega Start Deg", "type": "number"}, "phi_start_deg": {"title": "Phi Start Deg", "type": "number"}, "chi_start_deg": {"title": "Chi Start Deg", "type": "number"}, "rotation_direction": {"title": "Rotation Direction", "type": "string"}, "scan_width_deg": {"title": "Scan Width Deg", "type": "number"}}, "required": ["omega_start_deg", "phi_start_deg", "chi_start_deg", "rotation_direction", "scan_width_deg"], "title": "SingleRotationScanParams", "type": "object"}, "TopNByMaxCountForEachSampleSelection": {"properties": {"name": {"const": "TopNByMaxCountForEachSample", "default": "TopNByMaxCountForEachSample", "title": "Name", "type": "string"}, "ignore_xtal_not_found": {"default": false, "title": "Ignore Xtal Not Found", "type": "boolean"}, "n": {"title": "N", "type": "integer"}}, "required": ["n"], "title": "TopNByMaxCountForEachSampleSelection", "type": "object"}, "TopNByMaxCountSelection": {"properties": {"name": {"const": "TopNByMaxCount", "default": "TopNByMaxCount", "title": "Name", "type": "string"}, "ignore_xtal_not_found": {"default": false, "title": "Ignore Xtal Not Found", "type": "boolean"}, "n": {"title": "N", "type": "integer"}}, "required": ["n"], "title": "TopNByMaxCountSelection", "type": "object"}}, "additionalProperties": false, "properties": {"parameters": {"$ref": "#/$defs/LoadCentreCollectParams"}}, "required": ["parameters"], "title": "load_centre_collect", "type": "object"} diff --git a/docs/developer/hyperion/reference/blueapi/move_to_udc_default_state.json b/docs/developer/hyperion/reference/blueapi/move_to_udc_default_state.json deleted file mode 100644 index f6e46ad89d..0000000000 --- a/docs/developer/hyperion/reference/blueapi/move_to_udc_default_state.json +++ /dev/null @@ -1 +0,0 @@ -{"$schema": "https://json-schema.org/draft/2020-12/schema", "description": "Move beamline hardware to known positions prior to UDC start.", "additionalProperties": false, "properties": {}, "title": "move_to_udc_default_state", "type": "object"} diff --git a/docs/developer/hyperion/reference/blueapi/robot_unload.json b/docs/developer/hyperion/reference/blueapi/robot_unload.json deleted file mode 100644 index 48c6f57a96..0000000000 --- a/docs/developer/hyperion/reference/blueapi/robot_unload.json +++ /dev/null @@ -1 +0,0 @@ -{"$schema": "https://json-schema.org/draft/2020-12/schema", "description": "Unload the currently mounted pin into the location that it was loaded from.\n This is to be invoked as the final step upon successful completion of the UDC queue.", "additionalProperties": false, "properties": {"visit": {"title": "Visit", "type": "string"}, "robot": {"enum": ["robot"], "title": "Robot", "type": "dodal.devices.robot.BartRobot"}, "smargon": {"enum": ["gonio"], "title": "Smargon", "type": "dodal.devices.smargon.Smargon"}, "aperture_scatterguard": {"enum": ["aperture_scatterguard"], "title": "Aperture Scatterguard", "type": "dodal.devices.aperturescatterguard.ApertureScatterguard"}, "lower_gonio": {"enum": ["detector_motion", "gonio", "lower_gonio"], "title": "Lower Gonio", "type": "dodal.devices.motors.XYZStage"}}, "required": ["visit"], "title": "robot_unload", "type": "object"} diff --git a/docs/developer/hyperion/reference/blueapi_schema.rst b/docs/developer/hyperion/reference/blueapi_schema.rst deleted file mode 100644 index 862b706662..0000000000 --- a/docs/developer/hyperion/reference/blueapi_schema.rst +++ /dev/null @@ -1,6 +0,0 @@ -BlueAPI Schema -============== - -.. jsonschema:: - {"$schema": "https://json-schema.org/draft/2020-12/schema", "description": "Perform actions that are always performed on ending UDC, regardless of whether\n it terminated abnormally or not.\n Currently the only thing this does is close the detector shutter.\n Args:\n detector_motion:\n\n Returns:", "additionalProperties": false, "properties": {"cleanup_group": {"title": "Cleanup Group", "type": "string"}, "detector_motion": {"enum": ["detector_motion"], "title": "Detector Motion", "type": "dodal.devices.detector.detector_motion.DetectorMotion"}}, "title": "clean_up_udc", "type": "object"} - diff --git a/utility_scripts/generate_parameter_schemas.py b/utility_scripts/generate_parameter_schemas.py deleted file mode 100644 index f0b1f82965..0000000000 --- a/utility_scripts/generate_parameter_schemas.py +++ /dev/null @@ -1,29 +0,0 @@ -import json -from argparse import ArgumentParser -from pathlib import Path - - -def main(): - parser = ArgumentParser() - parser.add_argument("input_file") - parser.add_argument("output_dir") - args = parser.parse_args() - - with open(args.input_file) as stream: - json_str = stream.read() - json_blob = json.loads(json_str) - - for plan_dict in json_blob["plans"]: - name = plan_dict["name"] - schema_blob = { - "$schema": "https://json-schema.org/draft/2020-12/schema", - "description": plan_dict["description"].strip(), - **plan_dict["schema"], - } - output_path = Path(args.output_dir + "/" + name + ".json") - with open(output_path, "w") as output_stream: - output_stream.write(json.dumps(schema_blob)) - - -if __name__ == "__main__": - main() From 0fc691aefdd03dab549828028f53ccd38a37cba6 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Wed, 22 Apr 2026 15:14:00 +0100 Subject: [PATCH 33/36] Revert abortive parameter docs changes --- docs/developer/hyperion/index.rst | 1 - .../hyperion/reference/blueapi_schema.json | 365 ------------------ pyproject.toml | 1 - 3 files changed, 367 deletions(-) delete mode 100644 docs/developer/hyperion/reference/blueapi_schema.json diff --git a/docs/developer/hyperion/index.rst b/docs/developer/hyperion/index.rst index ffeb742466..a6f542affe 100644 --- a/docs/developer/hyperion/index.rst +++ b/docs/developer/hyperion/index.rst @@ -14,7 +14,6 @@ Documentation specific for the Hyperion module within MX-Bluesky reference/readme reference/baton - reference/blueapi_schema reference/param-hierarchy reference/coordinate-systems system-tests diff --git a/docs/developer/hyperion/reference/blueapi_schema.json b/docs/developer/hyperion/reference/blueapi_schema.json deleted file mode 100644 index 4d56775682..0000000000 --- a/docs/developer/hyperion/reference/blueapi_schema.json +++ /dev/null @@ -1,365 +0,0 @@ -{ - "plans": [ - { - "name": "clean_up_udc", - "description": "\n Perform actions that are always performed on ending UDC, regardless of whether\n it terminated abnormally or not.\n Currently the only thing this does is close the detector shutter.\n Args:\n detector_motion:\n\n Returns:\n ", - "schema": { - "additionalProperties": false, - "properties": { - "cleanup_group": { - "title": "Cleanup Group", - "type": "string" - }, - "detector_motion": { - "enum": [ - "detector_motion" - ], - "title": "Detector Motion", - "type": "dodal.devices.detector.detector_motion.DetectorMotion" - } - }, - "title": "clean_up_udc", - "type": "object" - } - }, - { - "name": "load_centre_collect", - "description": "\n Attempt a complete data collection experiment, consisting of the following:\n * Load the sample if necessary\n * Move to the specified goniometer start angles\n * Perform optical centring, then X-ray centring\n * If X-ray centring finds one or more diffracting centres then for each centre\n that satisfies the chosen selection function,\n move to that centre and do a collection with the specified parameters.\n ", - "schema": { - "$defs": { - "LoadCentreCollectParams": { - "additionalProperties": false, - "description": "This model is exposed as the BlueAPI REST parameter model for Hyperion Collections.\nIt can represent the full range of operations that are supported by LoadCentreCollect;\nthis is a superset of the operations that are actually required to follow Agamemnon instructions.\n\nThis is intended to provide additional flexibility to allow a potential future point of configuration\nwhere the supervisor can supply default values for values not explicitly specified by Agamemnon,\nalso to allow future exposure of this BlueAPI plan for e.g. commissioning purposes.\nThis may also permit the supervisor to implement future functionality by adjusting these parameters.", - "properties": { - "select_centres": { - "default": { - "name": "TopNByMaxCount", - "ignore_xtal_not_found": false, - "n": 1 - }, - "discriminator": { - "mapping": { - "TopNByMaxCount": "#/$defs/TopNByMaxCountSelection", - "TopNByMaxCountForEachSample": "#/$defs/TopNByMaxCountForEachSampleSelection" - }, - "propertyName": "name" - }, - "oneOf": [ - { - "$ref": "#/$defs/TopNByMaxCountSelection" - }, - { - "$ref": "#/$defs/TopNByMaxCountForEachSampleSelection" - } - ], - "title": "Select Centres" - }, - "visit": { - "title": "Visit", - "type": "string" - }, - "detector_distance_mm": { - "title": "Detector Distance Mm", - "type": "number" - }, - "sample_id": { - "title": "Sample Id", - "type": "integer" - }, - "sample_puck": { - "title": "Sample Puck", - "type": "integer" - }, - "sample_pin": { - "title": "Sample Pin", - "type": "integer" - }, - "demand_energy_ev": { - "title": "Demand Energy Ev", - "type": "number" - }, - "robot_load_then_centre": { - "$ref": "#/$defs/RobotLoadThenCentreParams" - }, - "multi_rotation_scan": { - "$ref": "#/$defs/MultiRotationScanParams" - } - }, - "required": [ - "visit", - "detector_distance_mm", - "sample_id", - "sample_puck", - "sample_pin", - "demand_energy_ev", - "robot_load_then_centre", - "multi_rotation_scan" - ], - "title": "LoadCentreCollectParams", - "type": "object" - }, - "MultiRotationScanParams": { - "additionalProperties": false, - "properties": { - "comment": { - "title": "Comment", - "type": "string" - }, - "file_name": { - "title": "File Name", - "type": "string" - }, - "storage_directory": { - "title": "Storage Directory", - "type": "string" - }, - "exposure_time_s": { - "title": "Exposure Time S", - "type": "number" - }, - "rotation_increment_deg": { - "title": "Rotation Increment Deg", - "type": "number" - }, - "snapshot_omegas_deg": { - "items": { - "type": "number" - }, - "title": "Snapshot Omegas Deg", - "type": "array" - }, - "rotation_scans": { - "items": { - "$ref": "#/$defs/SingleRotationScanParams" - }, - "title": "Rotation Scans", - "type": "array" - }, - "transmission_frac": { - "title": "Transmission Frac", - "type": "number" - }, - "ispyb_experiment_type": { - "title": "Ispyb Experiment Type", - "type": "string" - } - }, - "required": [ - "comment", - "file_name", - "storage_directory", - "exposure_time_s", - "rotation_increment_deg", - "snapshot_omegas_deg", - "rotation_scans", - "transmission_frac", - "ispyb_experiment_type" - ], - "title": "MultiRotationScanParams", - "type": "object" - }, - "RobotLoadThenCentreParams": { - "additionalProperties": false, - "properties": { - "storage_directory": { - "title": "Storage Directory", - "type": "string" - }, - "file_name": { - "title": "File Name", - "type": "string" - }, - "transmission_frac": { - "title": "Transmission Frac", - "type": "number" - }, - "exposure_time_s": { - "title": "Exposure Time S", - "type": "number" - }, - "omega_start_deg": { - "title": "Omega Start Deg", - "type": "number" - }, - "chi_start_deg": { - "title": "Chi Start Deg", - "type": "number" - }, - "tip_offset_um": { - "title": "Tip Offset Um", - "type": "number" - }, - "grid_width_um": { - "title": "Grid Width Um", - "type": "number" - } - }, - "required": [ - "storage_directory", - "file_name", - "transmission_frac", - "exposure_time_s", - "omega_start_deg", - "chi_start_deg", - "tip_offset_um", - "grid_width_um" - ], - "title": "RobotLoadThenCentreParams", - "type": "object" - }, - "SingleRotationScanParams": { - "additionalProperties": false, - "properties": { - "omega_start_deg": { - "title": "Omega Start Deg", - "type": "number" - }, - "phi_start_deg": { - "title": "Phi Start Deg", - "type": "number" - }, - "chi_start_deg": { - "title": "Chi Start Deg", - "type": "number" - }, - "rotation_direction": { - "title": "Rotation Direction", - "type": "string" - }, - "scan_width_deg": { - "title": "Scan Width Deg", - "type": "number" - } - }, - "required": [ - "omega_start_deg", - "phi_start_deg", - "chi_start_deg", - "rotation_direction", - "scan_width_deg" - ], - "title": "SingleRotationScanParams", - "type": "object" - }, - "TopNByMaxCountForEachSampleSelection": { - "properties": { - "name": { - "const": "TopNByMaxCountForEachSample", - "default": "TopNByMaxCountForEachSample", - "title": "Name", - "type": "string" - }, - "ignore_xtal_not_found": { - "default": false, - "title": "Ignore Xtal Not Found", - "type": "boolean" - }, - "n": { - "title": "N", - "type": "integer" - } - }, - "required": [ - "n" - ], - "title": "TopNByMaxCountForEachSampleSelection", - "type": "object" - }, - "TopNByMaxCountSelection": { - "properties": { - "name": { - "const": "TopNByMaxCount", - "default": "TopNByMaxCount", - "title": "Name", - "type": "string" - }, - "ignore_xtal_not_found": { - "default": false, - "title": "Ignore Xtal Not Found", - "type": "boolean" - }, - "n": { - "title": "N", - "type": "integer" - } - }, - "required": [ - "n" - ], - "title": "TopNByMaxCountSelection", - "type": "object" - } - }, - "additionalProperties": false, - "properties": { - "parameters": { - "$ref": "#/$defs/LoadCentreCollectParams" - } - }, - "required": [ - "parameters" - ], - "title": "load_centre_collect", - "type": "object" - } - }, - { - "name": "move_to_udc_default_state", - "description": "\n Move beamline hardware to known positions prior to UDC start.\n ", - "schema": { - "additionalProperties": false, - "properties": {}, - "title": "move_to_udc_default_state", - "type": "object" - } - }, - { - "name": "robot_unload", - "description": "\n Unload the currently mounted pin into the location that it was loaded from.\n This is to be invoked as the final step upon successful completion of the UDC queue.\n ", - "schema": { - "additionalProperties": false, - "properties": { - "visit": { - "title": "Visit", - "type": "string" - }, - "robot": { - "enum": [ - "robot" - ], - "title": "Robot", - "type": "dodal.devices.robot.BartRobot" - }, - "smargon": { - "enum": [ - "gonio" - ], - "title": "Smargon", - "type": "dodal.devices.smargon.Smargon" - }, - "aperture_scatterguard": { - "enum": [ - "aperture_scatterguard" - ], - "title": "Aperture Scatterguard", - "type": "dodal.devices.aperturescatterguard.ApertureScatterguard" - }, - "lower_gonio": { - "enum": [ - "detector_motion", - "gonio", - "lower_gonio" - ], - "title": "Lower Gonio", - "type": "dodal.devices.motors.XYZStage" - } - }, - "required": [ - "visit" - ], - "title": "robot_unload", - "type": "object" - } - } - ] -} diff --git a/pyproject.toml b/pyproject.toml index 8fa820e879..d8d187775d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -91,7 +91,6 @@ dev = [ "sphinx-autobuild", "sphinx-copybutton", "sphinx-design", - "sphinx-jsonschema", "tox-uv", "types-mock", "types-requests", From f15a9cbe80a9316ceb7d58954b3302df23a81148 Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Wed, 22 Apr 2026 16:38:33 +0100 Subject: [PATCH 34/36] Implement CURRENT_POSITION for gridscans, fix latent bug Validation for selected_aperture in LoadCentreCollectParams --- .../device_setup_plans/manipulate_sample.py | 2 +- ...ommon_grid_detect_then_xray_centre_plan.py | 33 +++++++++---- src/mx_bluesky/hyperion/blueapi/parameters.py | 15 +++++- .../experiment_plans/rotation_scan_plan.py | 4 +- ...ommon_grid_detect_then_xray_centre_plan.py | 49 +++++++++++++++++++ .../hyperion/blueapi_plans/test_parameters.py | 22 ++++++++- 6 files changed, 109 insertions(+), 16 deletions(-) diff --git a/src/mx_bluesky/common/device_setup_plans/manipulate_sample.py b/src/mx_bluesky/common/device_setup_plans/manipulate_sample.py index b218a2a584..c2e70d898d 100644 --- a/src/mx_bluesky/common/device_setup_plans/manipulate_sample.py +++ b/src/mx_bluesky/common/device_setup_plans/manipulate_sample.py @@ -39,7 +39,7 @@ def setup_sample_environment( yield from bps.abs_set(thawer, OnOff.OFF, group=group) -def prepare_aperture_if_required( +def prepare_aperture_for_rotation_if_required( aperture_scatterguard: ApertureScatterguard, aperture_policy: AperturePolicy, ): diff --git a/src/mx_bluesky/common/experiment_plans/common_grid_detect_then_xray_centre_plan.py b/src/mx_bluesky/common/experiment_plans/common_grid_detect_then_xray_centre_plan.py index 092c86b73f..15d9b4d558 100644 --- a/src/mx_bluesky/common/experiment_plans/common_grid_detect_then_xray_centre_plan.py +++ b/src/mx_bluesky/common/experiment_plans/common_grid_detect_then_xray_centre_plan.py @@ -7,7 +7,7 @@ from bluesky import preprocessors as bpp from bluesky.utils import MsgGenerator from dodal.common.beamlines.beamline_utils import get_config_client -from dodal.devices.aperturescatterguard import ApertureValue +from dodal.devices.aperturescatterguard import ApertureScatterguard, ApertureValue from dodal.devices.backlight import InOut from dodal.devices.eiger import EigerDetector from dodal.devices.oav.oav_parameters import OAVParameters @@ -108,6 +108,11 @@ def detect_grid_and_do_gridscan( grid_params_callback = GridDetectionCallback() + # Determine the aperture value before moving it for the OAV in case aperture_policy is CURRENT_POSITION + aperture_value = yield from _xrc_aperture_value_from_policy( + parameters.selected_aperture, composite.aperture_scatterguard + ) + yield from setup_beamline_for_oav( composite.gonio, composite.backlight, @@ -137,13 +142,12 @@ def run_grid_detection_plan( parameters.box_size_um, ) - if parameters.selected_aperture: - # Start moving the aperture/scatterguard into position without moving it in - yield from bps.prepare( - composite.aperture_scatterguard, - _xrc_aperture_value_from_policy(parameters.selected_aperture), - group=PlanGroupCheckpointConstants.PREPARE_APERTURE, - ) + # Start moving the aperture/scatterguard into position without moving it in + yield from bps.prepare( + composite.aperture_scatterguard, + aperture_value, + group=PlanGroupCheckpointConstants.PREPARE_APERTURE, + ) yield from run_grid_detection_plan( oav_params, @@ -160,7 +164,7 @@ def run_grid_detection_plan( yield from bps.wait(PlanGroupCheckpointConstants.PREPARE_APERTURE) yield from move_aperture_if_required( composite.aperture_scatterguard, - _xrc_aperture_value_from_policy(parameters.selected_aperture), + aperture_value, group=PlanGroupCheckpointConstants.GRID_READY_FOR_DC, ) xrc_params = create_parameters_for_flyscan_xray_centre( @@ -194,7 +198,9 @@ def create_parameters_for_flyscan_xray_centre( return flyscan_xray_centre_parameters -def _xrc_aperture_value_from_policy(policy: AperturePolicy) -> ApertureValue: +def _xrc_aperture_value_from_policy( + policy: AperturePolicy, aperture_scatterguard: ApertureScatterguard +) -> MsgGenerator[ApertureValue | None]: match policy: case AperturePolicy.SMALL | AperturePolicy.AUTO: return ApertureValue.SMALL @@ -202,5 +208,12 @@ def _xrc_aperture_value_from_policy(policy: AperturePolicy) -> ApertureValue: return ApertureValue.MEDIUM case AperturePolicy.LARGE: return ApertureValue.LARGE + case AperturePolicy.CURRENT_POSITION: + previous_aperture_position = yield from bps.rd(aperture_scatterguard) + assert isinstance(previous_aperture_position, ApertureValue) + LOGGER.info( + f"Using previously set aperture position {previous_aperture_position}" + ) + return previous_aperture_position case _: raise ValueError(f"Unsupported aperture policy {policy}") diff --git a/src/mx_bluesky/hyperion/blueapi/parameters.py b/src/mx_bluesky/hyperion/blueapi/parameters.py index 7e41fcf9a6..4bdd5dbdc0 100644 --- a/src/mx_bluesky/hyperion/blueapi/parameters.py +++ b/src/mx_bluesky/hyperion/blueapi/parameters.py @@ -2,9 +2,11 @@ This module contains the parameter models exported via the hyperion-blueapi REST interface. """ +from typing import Self + +from pydantic import BaseModel, model_validator, Field from typing import Any, Literal, TypeAlias -from pydantic import BaseModel, Field from pydantic.config import ConfigDict from mx_bluesky.common.parameters.components import ( @@ -90,6 +92,17 @@ class LoadCentreCollectParams(WithCentreSelection, HyperionParam): robot_load_then_centre: RobotLoadThenCentreParams multi_rotation_scan: MultiRotationScanParams + @model_validator(mode="after") + def _validate_model(self) -> Self: + if ( + self.multi_rotation_scan.selected_aperture + == AperturePolicy.CURRENT_POSITION + ): + raise ValueError( + "selected_aperture of CURRENT_POSITION is not supported for LoadCentreCollectParams" + ) + return self + def load_centre_collect_to_internal( external_params: LoadCentreCollectParams, diff --git a/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py b/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py index 0b77c07470..da60855357 100644 --- a/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py +++ b/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py @@ -31,7 +31,7 @@ from mx_bluesky.common.device_setup_plans.manipulate_sample import ( cleanup_sample_environment, - prepare_aperture_if_required, + prepare_aperture_for_rotation_if_required, setup_sample_environment, ) from mx_bluesky.common.device_setup_plans.setup_zebra_and_shutter import ( @@ -239,7 +239,7 @@ def _div_by_1000_if_not_none(num: float | None): ) if params.selected_aperture: - yield from prepare_aperture_if_required( + yield from prepare_aperture_for_rotation_if_required( composite.aperture_scatterguard, params.selected_aperture ) yield from oav_snapshot_plan(composite, params, oav_params) diff --git a/tests/unit_tests/common/experiment_plans/test_common_grid_detect_then_xray_centre_plan.py b/tests/unit_tests/common/experiment_plans/test_common_grid_detect_then_xray_centre_plan.py index 28b47e655a..e5fcffc825 100644 --- a/tests/unit_tests/common/experiment_plans/test_common_grid_detect_then_xray_centre_plan.py +++ b/tests/unit_tests/common/experiment_plans/test_common_grid_detect_then_xray_centre_plan.py @@ -431,6 +431,55 @@ def test_detect_grid_and_do_gridscan_maps_aperture_policy( ) +@pytest.mark.parametrize( + "current_aperture", + [ + ApertureValue.LARGE, + ApertureValue.MEDIUM, + ApertureValue.SMALL, + ], +) +def test_detect_grid_and_do_gridscan_maps_current_position_aperture_policy( + current_aperture: ApertureValue, + grid_detect_then_xrc_simulator: RunEngineSimulator, + grid_detect_xrc_devices: GridDetectThenXRayCentreComposite, + test_full_grid_scan_params: GridScanWithEdgeDetect, + test_config_files: dict[str, str], + construct_beamline_specific: ConstructBeamlineSpecificFeatures, +): + test_full_grid_scan_params.selected_aperture = AperturePolicy.CURRENT_POSITION + grid_detect_then_xrc_simulator.add_read_handler_for_multiple( + grid_detect_xrc_devices.aperture_scatterguard, + **{"aperture_scatterguard-selected_aperture": current_aperture}, + ) + msgs = grid_detect_then_xrc_simulator.simulate_plan( + grid_detect_then_xray_centre( + grid_detect_xrc_devices, + test_full_grid_scan_params, + xrc_params_type=SpecifiedThreeDGridScan, + construct_beamline_specific=construct_beamline_specific, + oav_config=test_config_files["oav_config_json"], + ) + ) + msgs = assert_message_and_return_remaining( + msgs, + lambda msg: ( + msg.command == "prepare" + and msg.obj is grid_detect_xrc_devices.aperture_scatterguard + ), + ) + + assert_message_and_return_remaining( + msgs, + lambda msg: ( + msg.command == "set" + and msg.obj + is grid_detect_xrc_devices.aperture_scatterguard.selected_aperture + and msg.args[0] + ), + ) + + @patch( "mx_bluesky.common.experiment_plans.common_grid_detect_then_xray_centre_plan.detect_grid_and_do_gridscan" ) diff --git a/tests/unit_tests/hyperion/blueapi_plans/test_parameters.py b/tests/unit_tests/hyperion/blueapi_plans/test_parameters.py index b5c70d0fd3..168c12e165 100644 --- a/tests/unit_tests/hyperion/blueapi_plans/test_parameters.py +++ b/tests/unit_tests/hyperion/blueapi_plans/test_parameters.py @@ -1,6 +1,7 @@ from pathlib import Path import pytest +from typing import Any from mx_bluesky.common.parameters.components import PARAMETER_VERSION, get_param_version from mx_bluesky.common.parameters.constants import GridscanParamConstants @@ -22,11 +23,16 @@ TEST_PIN = 15 -def test_map_external_to_internal_parameters(tmp_path): - raw_params = raw_params_from_file( +@pytest.fixture +def load_centre_collect_params_raw(tmp_path) -> dict[str, Any]: + return raw_params_from_file( "tests/test_data/parameter_json_files/external_load_centre_collect_params.json", tmp_path, ) + + +def test_map_external_to_internal_parameters(load_centre_collect_params_raw): + raw_params = load_centre_collect_params_raw external_params = LoadCentreCollectParams(**raw_params) raw_params["robot_load_then_centre"]["tip_offset_um"] = 300.0 expected_internal = LoadCentreCollect( @@ -103,3 +109,15 @@ def test_pin_tip_centre_then_xray_centre_to_internal(tmp_path: Path): visit=TEST_VISIT, storage_directory=str(tmp_path), ) + + +def test_load_centre_collect_current_position_aperture_not_supported( + load_centre_collect_params_raw, +): + load_centre_collect_params_raw["multi_rotation_scan"]["selected_aperture"] = ( + "CURRENT_POSITION" + ) + with pytest.raises( + ValueError, match="selected_aperture of CURRENT_POSITION is not supported" + ): + LoadCentreCollectParams(**load_centre_collect_params_raw) From 67166896ec7298c639a4ea6713176ae81b8f9a7a Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Mon, 27 Apr 2026 17:50:45 +0100 Subject: [PATCH 35/36] Remove errant sphinx-jsonschema in sphinx config --- docs/conf.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 1b3ca13b14..76b71cb01c 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -51,8 +51,6 @@ "myst_parser", # For plantUML diagrams "plantweb.directive", - # For rendering the Hyperion BlueAPI json schema - "sphinx-jsonschema", ] myst_enable_extensions = [ From 43043b9e99909953de29cdc035d8dbbcf6e7927e Mon Sep 17 00:00:00 2001 From: Robert Tuck Date: Tue, 28 Apr 2026 17:13:58 +0100 Subject: [PATCH 36/36] Fix unit tests after rebase borkage --- .../load_centre_collect_full_plan.py | 3 ++- .../pin_centre_then_xray_centre.py | 8 ++++---- .../hyperion/external_interaction/agamemnon.py | 2 +- .../hyperion/utils/centre_selection.py | 16 +++++++--------- ...rnal_load_centre_collect_params_multipin.json | 1 + 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py b/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py index cb6c7750ac..7a0170e426 100644 --- a/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py +++ b/src/mx_bluesky/hyperion/experiment_plans/load_centre_collect_full_plan.py @@ -104,8 +104,9 @@ def plan_with_callback_subs(): sample_ids_and_hits = yield from ( samples_and_hits_to_collect( - parameters, + parameters.select_centres, composite.gonio, + parameters.sample_id, flyscan_event_handler.xray_centre_results, ) ) diff --git a/src/mx_bluesky/hyperion/experiment_plans/pin_centre_then_xray_centre.py b/src/mx_bluesky/hyperion/experiment_plans/pin_centre_then_xray_centre.py index 901d20722f..1e0de658e4 100644 --- a/src/mx_bluesky/hyperion/experiment_plans/pin_centre_then_xray_centre.py +++ b/src/mx_bluesky/hyperion/experiment_plans/pin_centre_then_xray_centre.py @@ -15,7 +15,7 @@ HyperionGridDetectThenXRayCentreComposite, ) from mx_bluesky.hyperion.parameters.gridscan import PinTipCentreThenXrayCentre -from mx_bluesky.hyperion.utils.centre_selection import samples_and_locations_to_collect +from mx_bluesky.hyperion.utils.centre_selection import samples_and_hits_to_collect def pin_tip_centre_then_xray_centre( @@ -59,13 +59,13 @@ def pin_centre_flyscan_then_fetch_results() -> MsgGenerator: yield from pin_centre_then_gridscan_plan(composite, parameters, oav_config_file) results = xrc_event_handler.xray_centre_results - sample_ids_and_locations = yield from samples_and_locations_to_collect( + sample_ids_and_hits = yield from samples_and_hits_to_collect( centre_selection, composite.gonio, parameters.sample_id, results ) - if sample_ids_and_locations: + if sample_ids_and_hits: # Convert from um to mm since location is in motor coordinates. - location = [pos_um / 1000 for pos_um in sample_ids_and_locations[0][1]] + location = sample_ids_and_hits[0][1].centre_of_mass_mm yield from bps.abs_set( composite.gonio, CombinedMove(x=location[0], y=location[1], z=location[2]), diff --git a/src/mx_bluesky/hyperion/external_interaction/agamemnon.py b/src/mx_bluesky/hyperion/external_interaction/agamemnon.py index 80b26c9422..962e65e764 100644 --- a/src/mx_bluesky/hyperion/external_interaction/agamemnon.py +++ b/src/mx_bluesky/hyperion/external_interaction/agamemnon.py @@ -156,7 +156,7 @@ def _populate_parameters_from_agamemnon( aperture_policy = ( AperturePolicy.AUTO - if pin_type.expected_number_of_crystals == 1 + if isinstance(pin_type, SingleSamplePinTypeParam) else AperturePolicy.LARGE ) return [ diff --git a/src/mx_bluesky/hyperion/utils/centre_selection.py b/src/mx_bluesky/hyperion/utils/centre_selection.py index ac316f0e5b..e99642da72 100644 --- a/src/mx_bluesky/hyperion/utils/centre_selection.py +++ b/src/mx_bluesky/hyperion/utils/centre_selection.py @@ -8,8 +8,8 @@ from bluesky import plan_stubs as bps from bluesky.utils import MsgGenerator from dodal.devices.smargon import Smargon -from mx_bluesky.common.parameters.constants import GridscanParamConstants +from mx_bluesky.common.parameters.constants import GridscanParamConstants from mx_bluesky.common.utils import xrc_result as flyscan_result from mx_bluesky.common.utils.log import LOGGER from mx_bluesky.common.utils.xrc_result import XRayCentreResult @@ -18,7 +18,6 @@ TopNByMaxCountForEachSampleSelection, TopNByMaxCountSelection, ) -from mx_bluesky.hyperion.parameters.load_centre_collect import LoadCentreCollect def top_n_by_max_count( @@ -55,8 +54,9 @@ def resolve_selection_fn( def samples_and_hits_to_collect( - parameters: LoadCentreCollect, + selection_params: MultiXtalSelection, gonio: Smargon, + default_sample_id: int, xrc_results: Sequence[flyscan_result.XRayCentreResult] | None, ) -> MsgGenerator[list[tuple[int, flyscan_result.XRayCentreResult]]]: """ @@ -65,7 +65,7 @@ def samples_and_hits_to_collect( so that a collection can be performed without XRC should this be required. """ if xrc_results: - selection_func = resolve_selection_fn(parameters.selection_params) + selection_func = resolve_selection_fn(selection_params) hits = selection_func(xrc_results) hits_to_collect = [] for hit in hits: @@ -76,9 +76,7 @@ def samples_and_hits_to_collect( else: hits_to_collect.append(hit) - samples_and_locations = [ - (hit.sample_id, hit.centre_of_mass_mm * 1000) for hit in hits_to_collect - ] + samples_and_locations = [(hit.sample_id, hit) for hit in hits_to_collect] LOGGER.info( f"Selected hits {hits_to_collect} using {selection_func}, args={selection_params}" ) @@ -95,7 +93,7 @@ def samples_and_hits_to_collect( return [ ( - parameters.sample_id, + default_sample_id, flyscan_result.XRayCentreResult( centre_of_mass_mm=com_mm, bounding_box_mm=( @@ -104,7 +102,7 @@ def samples_and_hits_to_collect( ), max_count=1000, total_count=1000, - sample_id=parameters.sample_id, + sample_id=default_sample_id, ), ) ] diff --git a/tests/test_data/parameter_json_files/external_load_centre_collect_params_multipin.json b/tests/test_data/parameter_json_files/external_load_centre_collect_params_multipin.json index bb09a8143e..1d7de519c2 100644 --- a/tests/test_data/parameter_json_files/external_load_centre_collect_params_multipin.json +++ b/tests/test_data/parameter_json_files/external_load_centre_collect_params_multipin.json @@ -24,6 +24,7 @@ "file_name": "protk", "storage_directory": "{tmp_data}/123457/", "transmission_frac": 0.05, + "selected_aperture": "LARGE_APERTURE", "exposure_time_s": 0.004, "rotation_increment_deg": 0.1, "ispyb_experiment_type": "SAD",