From d7b7155db3919cb02ef35810d399edf81ddf7779 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 19 Feb 2026 11:29:21 +0000 Subject: [PATCH 1/6] Move rotation ispyb callbacks to common --- .../callbacks/rotation/__init__.py | 0 .../callbacks/rotation/ispyb_callback.py | 6 ++--- .../callbacks/rotation/ispyb_mapping.py | 0 .../callbacks/rotation/nexus_callback.py | 0 .../callbacks/__main__.py | 14 +++++------ .../test_ispyb_dev_connection.py | 6 ++--- .../test_load_centre_collect_full_plan.py | 6 ++--- .../external_interaction/test_nexgen.py | 6 ++--- .../callbacks/test_zocalo_handler.py | 6 ++--- .../test_rotation_scan_plan.py | 24 +++++++++---------- .../callbacks/rotation/test_ispyb_callback.py | 2 +- .../callbacks/test_rotation_callbacks.py | 18 +++++++------- .../test_compare_nexus_to_gda_exhaustively.py | 6 ++--- .../test_write_rotation_nexus.py | 6 ++--- 14 files changed, 50 insertions(+), 50 deletions(-) rename src/mx_bluesky/{hyperion => common}/external_interaction/callbacks/rotation/__init__.py (100%) rename src/mx_bluesky/{hyperion => common}/external_interaction/callbacks/rotation/ispyb_callback.py (99%) rename src/mx_bluesky/{hyperion => common}/external_interaction/callbacks/rotation/ispyb_mapping.py (100%) rename src/mx_bluesky/{hyperion => common}/external_interaction/callbacks/rotation/nexus_callback.py (100%) diff --git a/src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/__init__.py b/src/mx_bluesky/common/external_interaction/callbacks/rotation/__init__.py similarity index 100% rename from src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/__init__.py rename to src/mx_bluesky/common/external_interaction/callbacks/rotation/__init__.py diff --git a/src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py b/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py similarity index 99% rename from src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py rename to src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py index 6b251c86d1..5311d025b5 100644 --- a/src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py +++ b/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py @@ -15,6 +15,9 @@ from mx_bluesky.common.external_interaction.callbacks.common.zocalo_callback import ( ZocaloInfoGenerator, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_mapping import ( + populate_data_collection_info_for_rotation, +) from mx_bluesky.common.external_interaction.ispyb.data_model import ( DataCollectionInfo, DataCollectionPositionInfo, @@ -30,9 +33,6 @@ ) from mx_bluesky.common.utils.log import ISPYB_ZOCALO_CALLBACK_LOGGER, set_dcgid_tag from mx_bluesky.common.utils.utils import number_of_frames_from_scan_spec -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_mapping import ( - populate_data_collection_info_for_rotation, -) from mx_bluesky.hyperion.parameters.constants import CONST if TYPE_CHECKING: diff --git a/src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/ispyb_mapping.py b/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_mapping.py similarity index 100% rename from src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/ispyb_mapping.py rename to src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_mapping.py diff --git a/src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/nexus_callback.py b/src/mx_bluesky/common/external_interaction/callbacks/rotation/nexus_callback.py similarity index 100% rename from src/mx_bluesky/hyperion/external_interaction/callbacks/rotation/nexus_callback.py rename to src/mx_bluesky/common/external_interaction/callbacks/rotation/nexus_callback.py diff --git a/src/mx_bluesky/hyperion/external_interaction/callbacks/__main__.py b/src/mx_bluesky/hyperion/external_interaction/callbacks/__main__.py index c93c746221..7f53b5ed16 100644 --- a/src/mx_bluesky/hyperion/external_interaction/callbacks/__main__.py +++ b/src/mx_bluesky/hyperion/external_interaction/callbacks/__main__.py @@ -25,6 +25,13 @@ from mx_bluesky.common.external_interaction.callbacks.common.zocalo_callback import ( ZocaloCallback, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( + RotationISPyBCallback, + generate_start_info_from_ordered_runs, +) +from mx_bluesky.common.external_interaction.callbacks.rotation.nexus_callback import ( + RotationNexusFileCallback, +) from mx_bluesky.common.external_interaction.callbacks.sample_handling.sample_handling_callback import ( SampleHandlingCallback, ) @@ -47,13 +54,6 @@ from mx_bluesky.hyperion.external_interaction.callbacks.robot_actions.ispyb_callback import ( RobotLoadISPyBCallback, ) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( - RotationISPyBCallback, - generate_start_info_from_ordered_runs, -) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.nexus_callback import ( - RotationNexusFileCallback, -) from mx_bluesky.hyperion.external_interaction.callbacks.snapshot_callback import ( BeamDrawingCallback, ) diff --git a/tests/system_tests/hyperion/external_interaction/test_ispyb_dev_connection.py b/tests/system_tests/hyperion/external_interaction/test_ispyb_dev_connection.py index 026f3d0b50..44ad334218 100644 --- a/tests/system_tests/hyperion/external_interaction/test_ispyb_dev_connection.py +++ b/tests/system_tests/hyperion/external_interaction/test_ispyb_dev_connection.py @@ -17,6 +17,9 @@ populate_data_collection_group, populate_remaining_data_collection_info, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( + RotationISPyBCallback, +) from mx_bluesky.common.external_interaction.callbacks.xray_centre.ispyb_callback import ( GridscanISPyBCallback, ) @@ -47,9 +50,6 @@ RotationScanComposite, rotation_scan, ) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( - RotationISPyBCallback, -) from mx_bluesky.hyperion.parameters.device_composites import ( HyperionGridDetectThenXRayCentreComposite, ) 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 70736010b7..5fe2c2e3e2 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 @@ -27,6 +27,9 @@ from mx_bluesky.common.external_interaction.callbacks.common.ispyb_mapping import ( get_proposal_and_session_from_visit_string, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( + RotationISPyBCallback, +) from mx_bluesky.common.external_interaction.callbacks.sample_handling.sample_handling_callback import ( SampleHandlingCallback, ) @@ -46,9 +49,6 @@ from mx_bluesky.hyperion.external_interaction.callbacks.robot_actions.ispyb_callback import ( RobotLoadISPyBCallback, ) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( - RotationISPyBCallback, -) from mx_bluesky.hyperion.external_interaction.callbacks.snapshot_callback import ( BeamDrawingCallback, ) diff --git a/tests/system_tests/hyperion/external_interaction/test_nexgen.py b/tests/system_tests/hyperion/external_interaction/test_nexgen.py index 133f9f950c..c9472f4beb 100644 --- a/tests/system_tests/hyperion/external_interaction/test_nexgen.py +++ b/tests/system_tests/hyperion/external_interaction/test_nexgen.py @@ -11,15 +11,15 @@ from mx_bluesky.common.experiment_plans.inner_plans.read_hardware import ( standard_read_hardware_during_collection, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.nexus_callback import ( + RotationNexusFileCallback, +) from mx_bluesky.common.parameters.rotation import ( SingleRotationScan, ) from mx_bluesky.hyperion.experiment_plans.rotation_scan_plan import ( RotationScanComposite, ) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.nexus_callback import ( - RotationNexusFileCallback, -) from mx_bluesky.hyperion.parameters.constants import CONST from ....conftest import extract_metafile, raw_params_from_file diff --git a/tests/unit_tests/common/external_interaction/callbacks/test_zocalo_handler.py b/tests/unit_tests/common/external_interaction/callbacks/test_zocalo_handler.py index aca1a0eb62..2bdde1b412 100644 --- a/tests/unit_tests/common/external_interaction/callbacks/test_zocalo_handler.py +++ b/tests/unit_tests/common/external_interaction/callbacks/test_zocalo_handler.py @@ -7,6 +7,9 @@ ZocaloCallback, ZocaloTrigger, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( + generate_start_info_from_ordered_runs, +) from mx_bluesky.common.external_interaction.ispyb.ispyb_store import ( IspybIds, StoreInIspyb, @@ -15,9 +18,6 @@ from mx_bluesky.hyperion.external_interaction.callbacks.__main__ import ( create_gridscan_callbacks, ) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( - generate_start_info_from_ordered_runs, -) EXPECTED_RUN_START_MESSAGE = {"subplan_name": "test_plan_name", "uid": "my_uuid"} EXPECTED_RUN_END_MESSAGE = {"event": "end", "run_start": "my_uuid"} 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 55cc8612d7..3b92cb9d7f 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 @@ -33,6 +33,13 @@ from mx_bluesky.common.external_interaction.callbacks.common.zocalo_callback import ( ZocaloCallback, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( + RotationISPyBCallback, + generate_start_info_from_ordered_runs, +) +from mx_bluesky.common.external_interaction.callbacks.rotation.nexus_callback import ( + RotationNexusFileCallback, +) from mx_bluesky.common.external_interaction.ispyb.ispyb_store import ( IspybIds, StoreInIspyb, @@ -56,13 +63,6 @@ from mx_bluesky.hyperion.external_interaction.callbacks.__main__ import ( create_rotation_callbacks, ) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( - RotationISPyBCallback, - generate_start_info_from_ordered_runs, -) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.nexus_callback import ( - RotationNexusFileCallback, -) from mx_bluesky.hyperion.parameters.constants import CONST from ....conftest import ( @@ -796,7 +796,7 @@ def test_rotation_scan_arms_detector_and_takes_snapshots_whilst_arming( @patch( - "mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb" + "mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb" ) def test_rotation_scan_correctly_triggers_ispyb_callback( mock_store_in_ispyb, @@ -828,7 +828,7 @@ def test_rotation_scan_correctly_triggers_ispyb_callback( "mx_bluesky.common.external_interaction.callbacks.common.zocalo_callback.ZocaloTrigger" ) @patch( - "mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb" + "mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb" ) def test_rotation_scan_correctly_triggers_zocalo_callback( mock_store_in_ispyb, @@ -1171,7 +1171,7 @@ def test_full_multi_rotation_plan_docs_emitted( @patch( - "mx_bluesky.hyperion.external_interaction.callbacks.rotation.nexus_callback.NexusWriter" + "mx_bluesky.common.external_interaction.callbacks.rotation.nexus_callback.NexusWriter" ) @patch( "mx_bluesky.hyperion.experiment_plans.rotation_scan_plan.check_topup_and_wait_if_necessary", @@ -1468,7 +1468,7 @@ def test_full_multi_rotation_plan_arms_eiger_asynchronously_and_disarms( @patch( - "mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb" + "mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb" ) @patch( "mx_bluesky.hyperion.experiment_plans.rotation_scan_plan.check_topup_and_wait_if_necessary", @@ -1512,7 +1512,7 @@ def test_zocalo_callback_end_only_gets_called_after_eiger_unstage( @patch( - "mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb" + "mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb" ) @patch( "mx_bluesky.hyperion.experiment_plans.rotation_scan_plan.check_topup_and_wait_if_necessary", diff --git a/tests/unit_tests/hyperion/external_interaction/callbacks/rotation/test_ispyb_callback.py b/tests/unit_tests/hyperion/external_interaction/callbacks/rotation/test_ispyb_callback.py index 4f76b4e73c..31e2e2e4e1 100644 --- a/tests/unit_tests/hyperion/external_interaction/callbacks/rotation/test_ispyb_callback.py +++ b/tests/unit_tests/hyperion/external_interaction/callbacks/rotation/test_ispyb_callback.py @@ -1,7 +1,7 @@ import json from unittest.mock import MagicMock, patch -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( +from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( RotationISPyBCallback, ) diff --git a/tests/unit_tests/hyperion/external_interaction/callbacks/test_rotation_callbacks.py b/tests/unit_tests/hyperion/external_interaction/callbacks/test_rotation_callbacks.py index 4005532a89..f9b2725e37 100644 --- a/tests/unit_tests/hyperion/external_interaction/callbacks/test_rotation_callbacks.py +++ b/tests/unit_tests/hyperion/external_interaction/callbacks/test_rotation_callbacks.py @@ -4,6 +4,12 @@ from bluesky.run_engine import RunEngine from event_model import RunStart +from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( + RotationISPyBCallback, +) +from mx_bluesky.common.external_interaction.callbacks.rotation.nexus_callback import ( + RotationNexusFileCallback, +) from mx_bluesky.common.external_interaction.ispyb.data_model import ( ScanDataInfo, ) @@ -19,12 +25,6 @@ from mx_bluesky.hyperion.external_interaction.callbacks.__main__ import ( create_rotation_callbacks, ) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( - RotationISPyBCallback, -) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.nexus_callback import ( - RotationNexusFileCallback, -) from mx_bluesky.hyperion.parameters.constants import CONST from .....conftest import raw_params_from_file @@ -56,7 +56,7 @@ def do_rotation_scan( @pytest.mark.timeout(2) @patch( - "mx_bluesky.hyperion.external_interaction.callbacks.rotation.nexus_callback.NexusWriter", + "mx_bluesky.common.external_interaction.callbacks.rotation.nexus_callback.NexusWriter", autospec=True, ) def test_nexus_handler_gets_documents_in_plan( @@ -82,7 +82,7 @@ def test_nexus_handler_gets_documents_in_plan( @pytest.mark.timeout(2) @patch( - "mx_bluesky.hyperion.external_interaction.callbacks.rotation.nexus_callback.NexusWriter", + "mx_bluesky.common.external_interaction.callbacks.rotation.nexus_callback.NexusWriter", autospec=True, ) def test_nexus_handler_only_writes_once( @@ -198,7 +198,7 @@ def test_ispyb_reuses_dcgid_on_same_sample_id( @pytest.mark.parametrize("n_images,store_id", n_images_store_id) @patch( - "mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb", + "mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb", new=MagicMock(), ) def test_ispyb_handler_stores_sampleid_for_full_collection_not_screening( diff --git a/tests/unit_tests/hyperion/external_interaction/nexus/test_compare_nexus_to_gda_exhaustively.py b/tests/unit_tests/hyperion/external_interaction/nexus/test_compare_nexus_to_gda_exhaustively.py index 2ecfe3c6ed..5bdd2bacc0 100644 --- a/tests/unit_tests/hyperion/external_interaction/nexus/test_compare_nexus_to_gda_exhaustively.py +++ b/tests/unit_tests/hyperion/external_interaction/nexus/test_compare_nexus_to_gda_exhaustively.py @@ -13,15 +13,15 @@ from mx_bluesky.common.experiment_plans.inner_plans.read_hardware import ( standard_read_hardware_during_collection, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.nexus_callback import ( + RotationNexusFileCallback, +) from mx_bluesky.common.parameters.rotation import ( RotationScan, ) from mx_bluesky.hyperion.experiment_plans.rotation_scan_plan import ( RotationScanComposite, ) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.nexus_callback import ( - RotationNexusFileCallback, -) from mx_bluesky.hyperion.parameters.constants import CONST from .....conftest import extract_metafile, raw_params_from_file diff --git a/tests/unit_tests/hyperion/external_interaction/test_write_rotation_nexus.py b/tests/unit_tests/hyperion/external_interaction/test_write_rotation_nexus.py index a40495b4f2..5b9aadca86 100644 --- a/tests/unit_tests/hyperion/external_interaction/test_write_rotation_nexus.py +++ b/tests/unit_tests/hyperion/external_interaction/test_write_rotation_nexus.py @@ -14,6 +14,9 @@ from mx_bluesky.common.experiment_plans.inner_plans.read_hardware import ( standard_read_hardware_during_collection, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.nexus_callback import ( + RotationNexusFileCallback, +) from mx_bluesky.common.external_interaction.nexus.write_nexus import NexusWriter from mx_bluesky.common.parameters.rotation import ( SingleRotationScan, @@ -22,9 +25,6 @@ from mx_bluesky.hyperion.experiment_plans.rotation_scan_plan import ( RotationScanComposite, ) -from mx_bluesky.hyperion.external_interaction.callbacks.rotation.nexus_callback import ( - RotationNexusFileCallback, -) from mx_bluesky.hyperion.parameters.constants import CONST from ....conftest import extract_metafile, raw_params_from_file From 9de1986a63a9920788a521ae39c40cc5ea84bf15 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Thu, 19 Feb 2026 16:36:05 +0000 Subject: [PATCH 2/6] Support ispyb deposition for jungfrau rotation scans --- .../callbacks/metadata_writer.py | 7 ++++- .../experiment_plans/rotation_scan_plan.py | 27 +++++++++++++++---- .../beamlines/i24/parameters/constants.py | 9 ------- .../callbacks/common/ispyb_mapping.py | 25 +++++++++++++---- .../callbacks/rotation/ispyb_callback.py | 27 ++++++++++++++----- .../common/parameters/components.py | 6 ++--- src/mx_bluesky/common/parameters/constants.py | 1 + 7 files changed, 72 insertions(+), 30 deletions(-) delete mode 100644 src/mx_bluesky/beamlines/i24/parameters/constants.py diff --git a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py index fa80c19cde..78207613bd 100644 --- a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py +++ b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py @@ -3,7 +3,8 @@ from bluesky.callbacks import CallbackBase -from mx_bluesky.beamlines.i24.parameters.constants import PlanNameConstants +from mx_bluesky.common.external_interaction.ispyb.ispyb_store import IspybIds +from mx_bluesky.common.parameters.constants import PlanNameConstants from mx_bluesky.common.parameters.rotation import SingleRotationScan from mx_bluesky.common.utils.log import LOGGER @@ -43,6 +44,9 @@ def start(self, doc: dict): # type: ignore ) self.parameters = SingleRotationScan(**json.loads(json_params)) self.run_start_uid = doc.get("uid") + dcid = doc.get("dcid") + assert isinstance(dcid, IspybIds) + self.dcid = dcid.data_collection_ids[0] def descriptor(self, doc: dict): # type: ignore self.descriptors[doc["uid"]] = doc @@ -81,6 +85,7 @@ def stop(self, doc: dict): # type: ignore "energy_kev": self.energy_in_kev, "angular_increment_deg": self.parameters.rotation_increment_deg, "detector_distance_mm": self.detector_distance_mm, + "dcid": self.dcid, } ) ) diff --git a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py index 477521be3c..99cf109197 100644 --- a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py +++ b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py @@ -27,9 +27,6 @@ JF_COMPLETE_GROUP, fly_jungfrau, ) -from mx_bluesky.beamlines.i24.parameters.constants import ( - PlanNameConstants, -) from mx_bluesky.common.device_setup_plans.setup_zebra_and_shutter import ( setup_zebra_for_rotation, tidy_up_zebra_after_rotation_scan, @@ -41,10 +38,15 @@ RotationMotionProfile, calculate_motion_profile, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( + RotationISPyBCallback, +) +from mx_bluesky.common.external_interaction.ispyb.ispyb_store import IspybIds from mx_bluesky.common.parameters.components import get_param_version from mx_bluesky.common.parameters.constants import ( USE_NUMTRACKER, PlanGroupCheckpointConstants, + PlanNameConstants, ) from mx_bluesky.common.parameters.rotation import ( SingleRotationScan, @@ -152,8 +154,17 @@ def single_rotation_plan( about a fixed axis - for now this axis is limited to omega. Needs additional setup of the sample environment and a wrapper to clean up.""" - @bpp.set_run_key_decorator(PlanNameConstants.SINGLE_ROTATION_SCAN) - @run_decorator() + ispyb_callback = RotationISPyBCallback() + + @bpp.subs_decorator(ispyb_callback) + @bpp.set_run_key_decorator(PlanNameConstants.ROTATION_OUTER) + @run_decorator( + md={ + "subplan_name": PlanNameConstants.ROTATION_OUTER, + "mx_bluesky_parameters": params.model_dump_json(), + "activate_callbacks": ["RotationISPyBCallback"], + } + ) def _plan_in_run_decorator(): if not params.detector_distance_mm: LOGGER.info( @@ -185,6 +196,7 @@ def _plan_in_run_decorator(): "scan_points": [params.scan_points], "rotation_scan_params": params.model_dump_json(), "detector_file_template": params.file_name, + "dcid": IspybIds(), } ) def _rotation_scan_plan( @@ -232,6 +244,9 @@ def _rotation_scan_plan( # Read hardware after preparing jungfrau so that device metadata output from callback is correct # Whilst metadata is being written in bluesky we need to access the private writer here + # We also need to access the private writer so that we get path information to send to ispyb. + # This will be needed until numtracker / StartDocumentPathProvider can fully support the MX + # usecase, see #issue read_hardware_partial = partial( read_hardware_plan, [ @@ -239,6 +254,8 @@ def _rotation_scan_plan( composite.dcm.wavelength_in_a, composite.det_stage.z, composite.jungfrau._writer.file_path, # noqa: SLF001 N + composite.jungfrau._writer.file_name, # noqa: SLF001 N + composite.jungfrau.ispyb_detector_id, ], PlanNameConstants.ROTATION_DEVICE_READ, ) diff --git a/src/mx_bluesky/beamlines/i24/parameters/constants.py b/src/mx_bluesky/beamlines/i24/parameters/constants.py deleted file mode 100644 index e4f21934b0..0000000000 --- a/src/mx_bluesky/beamlines/i24/parameters/constants.py +++ /dev/null @@ -1,9 +0,0 @@ -from pydantic.dataclasses import dataclass - - -@dataclass(frozen=True) -class PlanNameConstants: - ROTATION_DEVICE_READ = "ROTATION DEVICE READ" - SINGLE_ROTATION_SCAN = "OUTER SINGLE ROTATION SCAN" - MULTI_ROTATION_SCAN = "OUTER MULTI ROTATION SCAN" - ROTATION_MAIN = "ROTATION MAIN" diff --git a/src/mx_bluesky/common/external_interaction/callbacks/common/ispyb_mapping.py b/src/mx_bluesky/common/external_interaction/callbacks/common/ispyb_mapping.py index 7df7c2a176..24ef543394 100644 --- a/src/mx_bluesky/common/external_interaction/callbacks/common/ispyb_mapping.py +++ b/src/mx_bluesky/common/external_interaction/callbacks/common/ispyb_mapping.py @@ -1,5 +1,7 @@ from __future__ import annotations +from event_model import RunStart + from mx_bluesky.common.external_interaction.ispyb.data_model import ( DataCollectionGroupInfo, DataCollectionInfo, @@ -8,6 +10,7 @@ get_current_time_string, ) from mx_bluesky.common.parameters.components import DiffractionExperimentWithSample +from mx_bluesky.common.parameters.constants import USE_NUMTRACKER EIGER_FILE_SUFFIX = "h5" @@ -26,16 +29,25 @@ def populate_remaining_data_collection_info( data_collection_group_id, data_collection_info: DataCollectionInfo, params: DiffractionExperimentWithSample, + doc: RunStart | None = None, ): data_collection_info.sample_id = params.sample_id - data_collection_info.visit_string = params.visit data_collection_info.parent_id = data_collection_group_id data_collection_info.comments = comment data_collection_info.detector_distance = params.detector_params.detector_distance data_collection_info.exp_time = params.detector_params.exposure_time_s - data_collection_info.imgdir = params.detector_params.directory - data_collection_info.imgprefix = params.detector_params.prefix - data_collection_info.imgsuffix = EIGER_FILE_SUFFIX + if params.visit == USE_NUMTRACKER: + # Plans using StandardDetector, numtracker and BlueAPI get its information via metadata. + # This metadata gets set after a run is open, and is provided by numtracker + assert doc, ( + "Expected RunStart doc to be provided to populate_remaining_data_collection_info when using numtracker for visit" + ) + data_collection_info.visit_string = doc.get("instrument_session") + else: + data_collection_info.visit_string = params.visit + data_collection_info.imgdir = params.detector_params.directory + data_collection_info.imgprefix = params.detector_params.prefix + data_collection_info.imgsuffix = EIGER_FILE_SUFFIX # Both overlap and n_passes included for backwards compatibility, # planned to be removed later data_collection_info.n_passes = 1 @@ -47,7 +59,10 @@ def populate_remaining_data_collection_info( data_collection_info.xbeam = beam_position[0] data_collection_info.ybeam = beam_position[1] data_collection_info.start_time = get_current_time_string() - if data_collection_info.data_collection_number is not None: + if ( + data_collection_info.data_collection_number is not None + and params.visit != USE_NUMTRACKER + ): # Do not write the file template if we don't have sufficient information - for gridscans we may not # know the data collection number until later data_collection_info.file_template = f"{params.detector_params.prefix}_{data_collection_info.data_collection_number}_master.h5" diff --git a/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py b/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py index 5311d025b5..a71a348929 100644 --- a/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py +++ b/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py @@ -28,6 +28,7 @@ StoreInIspyb, ) from mx_bluesky.common.parameters.components import IspybExperimentType +from mx_bluesky.common.parameters.constants import PlanNameConstants from mx_bluesky.common.parameters.rotation import ( SingleRotationScan, ) @@ -69,9 +70,9 @@ def activity_gated_start(self, doc: RunStart): ISPYB_ZOCALO_CALLBACK_LOGGER.info( "ISPyB callback received start document with experiment parameters." ) - hyperion_params = doc.get("mx_bluesky_parameters") - assert isinstance(hyperion_params, str) - self.params = SingleRotationScan.model_validate_json(hyperion_params) + params = doc.get("mx_bluesky_parameters") + assert isinstance(params, str) + self.params = SingleRotationScan.model_validate_json(params) dcgid = ( self.ispyb_ids.data_collection_group_id if (self.params.sample_id == self.last_sample_id) @@ -97,10 +98,7 @@ def activity_gated_start(self, doc: RunStart): self.params ) data_collection_info = populate_remaining_data_collection_info( - self.params.comment, - dcgid, - data_collection_info, - self.params, + self.params.comment, dcgid, data_collection_info, self.params, doc ) data_collection_info.parent_id = dcgid scan_data_info = ScanDataInfo( @@ -150,6 +148,7 @@ def _handle_ispyb_hardware_read(self, doc: Event): def activity_gated_event(self, doc: Event): doc = super().activity_gated_event(doc) + assert self.params, "IspyB callback event triggered before parameters were set" set_dcgid_tag(self.ispyb_ids.data_collection_group_id) descriptor_name = self.descriptors[doc["descriptor"]].get("name") @@ -158,6 +157,20 @@ def activity_gated_event(self, doc: Event): self.ispyb_ids = self.ispyb.update_deposition( self.ispyb_ids, scan_data_infos ) + elif descriptor_name == PlanNameConstants.ROTATION_DEVICE_READ: + data = doc["data"] + filepath = data.get("commissioning_jungfrau-_writer-file_path") + filename = data.get("commissioning_jungfrau-_writer-file_name") + updated_dc = DataCollectionInfo( + file_template=f"{filename}.nxs", + imgdir=f"{filepath}/", + imgprefix=f"{filename}", + imgsuffix="nxs", + ) + scan_data_info = self.populate_info_for_update( + updated_dc, None, self.params + ) + self.ispyb.update_deposition(self.ispyb_ids, scan_data_info) return doc diff --git a/src/mx_bluesky/common/parameters/components.py b/src/mx_bluesky/common/parameters/components.py index e80f6ed8ee..7b78e90a0b 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( @@ -232,7 +230,9 @@ def scan_indices(self) -> Sequence[SupportsInt]: class WithSample(BaseModel): - sample_id: int + sample_id: int | None = ( + None # None is invalid for dc groups, but valid for regular data collections + ) sample_puck: int | None = None sample_pin: int | None = None diff --git a/src/mx_bluesky/common/parameters/constants.py b/src/mx_bluesky/common/parameters/constants.py index e89ec7c564..fee396485e 100644 --- a/src/mx_bluesky/common/parameters/constants.py +++ b/src/mx_bluesky/common/parameters/constants.py @@ -76,6 +76,7 @@ class PlanNameConstants: ROTATION_MULTI_OUTER = "multi_rotation_outer" ROTATION_OUTER = "rotation_scan_with_cleanup" ROTATION_MAIN = "rotation_scan_main" + ROTATION_DEVICE_READ = "ROTATION DEVICE READ" SET_ENERGY = "set_energy" From 1c314fa852830d44d9178004d2d61b2d95aad412 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 20 Feb 2026 15:18:47 +0000 Subject: [PATCH 3/6] Fixes --- .../callbacks/metadata_writer.py | 33 +++++++--- .../experiment_plans/rotation_scan_plan.py | 3 +- .../callbacks/rotation/ispyb_callback.py | 14 ++++- .../common/parameters/components.py | 10 +++- .../hyperion/parameters/constants.py | 5 +- .../callbacks/test_metadata_writer.py | 60 ++++++++++++++----- .../test_rotation_scan.py | 15 +++-- tests/unit_tests/conftest.py | 6 ++ 8 files changed, 111 insertions(+), 35 deletions(-) diff --git a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py index 78207613bd..508e225256 100644 --- a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py +++ b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py @@ -14,11 +14,25 @@ class JsonMetadataWriter(CallbackBase): """Callback class to handle the creation of metadata json files for commissioning. - To use, subscribe the Bluesky RunEngine to an instance of this class. - E.g.: - metadata_writer_callback = JsonMetadataWriter(parameters) - RE.subscribe(metadata_writer_callback) - Or decorate a plan using bluesky.preprocessors.subs_decorator. + Currently, nexus files aren't being written by nexgen, so writer needs to include + the dcid produced by an ispyb callback. To get this working, the ispyb callback need + to be triggered in the right way before the start function here is run. + + To use: + 1. subscribe the Bluesky RunEngine to an instance of this class. + E.g.: + metadata_writer_callback = JsonMetadataWriter(parameters) + RE.subscribe(metadata_writer_callback) + Or decorate a plan using bluesky.preprocessors.subs_decorator. + 2. Subscribe the RE to the rotation callbacks: + ispyb_callback = RotationISPyBCallback() + @bpp.subs_decorator(ispyb_callback) + ... + 3. Open a run with key decorator PlanNameConstants.ROTATION_OUTER, with metadata: + "activate_callbacks": ["RotationISPyBCallback"] + 4. Open a run with key decorator PlanNameConstants.ROTATION_MAIN, with metadata: + "dcid": ispyb_callback.ispyb_ids + See: https://blueskyproject.io/bluesky/callbacks.html#ways-to-invoke-callbacks @@ -45,8 +59,11 @@ def start(self, doc: dict): # type: ignore self.parameters = SingleRotationScan(**json.loads(json_params)) self.run_start_uid = doc.get("uid") dcid = doc.get("dcid") - assert isinstance(dcid, IspybIds) - self.dcid = dcid.data_collection_ids[0] + assert isinstance(dcid, IspybIds), ( + "Rotation start document should include dcid of type IspybIds to use JF metadata writer. This should come from RotationISPyBCallback activated by a " + "PlanNameConstants.ROTATION_OUTER run" + ) + self.dcid = dcid def descriptor(self, doc: dict): # type: ignore self.descriptors[doc["uid"]] = doc @@ -85,7 +102,7 @@ def stop(self, doc: dict): # type: ignore "energy_kev": self.energy_in_kev, "angular_increment_deg": self.parameters.rotation_increment_deg, "detector_distance_mm": self.detector_distance_mm, - "dcid": self.dcid, + "dcid": self.dcid.data_collection_ids[0], } ) ) diff --git a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py index 99cf109197..72002495dc 100644 --- a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py +++ b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py @@ -41,7 +41,6 @@ from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( RotationISPyBCallback, ) -from mx_bluesky.common.external_interaction.ispyb.ispyb_store import IspybIds from mx_bluesky.common.parameters.components import get_param_version from mx_bluesky.common.parameters.constants import ( USE_NUMTRACKER, @@ -196,7 +195,7 @@ def _plan_in_run_decorator(): "scan_points": [params.scan_points], "rotation_scan_params": params.model_dump_json(), "detector_file_template": params.file_name, - "dcid": IspybIds(), + "dcid": ispyb_callback.ispyb_ids, } ) def _rotation_scan_plan( diff --git a/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py b/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py index a71a348929..0408848fd1 100644 --- a/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py +++ b/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py @@ -28,7 +28,7 @@ StoreInIspyb, ) from mx_bluesky.common.parameters.components import IspybExperimentType -from mx_bluesky.common.parameters.constants import PlanNameConstants +from mx_bluesky.common.parameters.constants import USE_NUMTRACKER, PlanNameConstants from mx_bluesky.common.parameters.rotation import ( SingleRotationScan, ) @@ -73,6 +73,18 @@ def activity_gated_start(self, doc: RunStart): params = doc.get("mx_bluesky_parameters") assert isinstance(params, str) self.params = SingleRotationScan.model_validate_json(params) + + # Todo this chunk needs to go at the start of any numtracker+ispyb-using plan + if self.params.visit == USE_NUMTRACKER: + try: + visit = doc.get("instrument_session") + assert isinstance(visit, str) + self.params.visit = visit + except Exception as e: + raise ValueError( + f"Error trying to retrieve instrument session from document {doc}" + ) from e + dcgid = ( self.ispyb_ids.data_collection_group_id if (self.params.sample_id == self.last_sample_id) diff --git a/src/mx_bluesky/common/parameters/components.py b/src/mx_bluesky/common/parameters/components.py index 7b78e90a0b..26acbf2d5c 100644 --- a/src/mx_bluesky/common/parameters/components.py +++ b/src/mx_bluesky/common/parameters/components.py @@ -12,6 +12,7 @@ DetectorParams, TriggerMode, ) +from dodal.utils import BeamlinePrefix, get_beamline_name from pydantic import ( BaseModel, ConfigDict, @@ -29,8 +30,12 @@ GridscanParamConstants, ) +TEST_MODE = os.environ.get("HYPERION_TEST_MODE") + PARAMETER_VERSION = Version.parse("5.3.0") +BL = get_beamline_name("i03") + def get_param_version() -> SemanticVersion: return SemanticVersion.validate_from_str(str(PARAMETER_VERSION)) @@ -157,6 +162,9 @@ class WithVisit(BaseModel): default=DetectorParamConstants.BEAM_XY_LUT_PATH ) detector_distance_mm: float | None = Field(default=None, gt=0) + insertion_prefix: str = ( + f"{BeamlinePrefix(BL).insertion_prefix}" if TEST_MODE else "SR03I" + ) class DiffractionExperiment( @@ -189,7 +197,7 @@ def validate_directories(cls, values): Path(values["storage_directory"], "snapshots").as_posix(), ) else: - values["snapshot_directory"] = Path("/tmp") + values["snapshot_directory"] = Path("/tmp").as_posix() return values @property diff --git a/src/mx_bluesky/hyperion/parameters/constants.py b/src/mx_bluesky/hyperion/parameters/constants.py index 07b6fb19ad..b1a1139ca1 100644 --- a/src/mx_bluesky/hyperion/parameters/constants.py +++ b/src/mx_bluesky/hyperion/parameters/constants.py @@ -1,8 +1,7 @@ -import os - from dodal.devices.detector import EIGER2_X_16M_SIZE from pydantic.dataclasses import dataclass +from mx_bluesky.common.parameters.components import TEST_MODE from mx_bluesky.common.parameters.constants import ( DeviceSettingsConstants, DocDescriptorNames, @@ -15,8 +14,6 @@ PlanNameConstants, ) -TEST_MODE = os.environ.get("HYPERION_TEST_MODE") - @dataclass(frozen=True) class I03Constants: diff --git a/tests/unit_tests/beamlines/i24/jungfrau_commissioning/callbacks/test_metadata_writer.py b/tests/unit_tests/beamlines/i24/jungfrau_commissioning/callbacks/test_metadata_writer.py index 797ca8fe2e..966f2580ad 100644 --- a/tests/unit_tests/beamlines/i24/jungfrau_commissioning/callbacks/test_metadata_writer.py +++ b/tests/unit_tests/beamlines/i24/jungfrau_commissioning/callbacks/test_metadata_writer.py @@ -3,6 +3,7 @@ import json from pathlib import Path +import bluesky.plan_stubs as bps import bluesky.preprocessors as bpp import pytest from bluesky.run_engine import RunEngine @@ -17,18 +18,26 @@ from mx_bluesky.beamlines.i24.jungfrau_commissioning.experiment_plans.rotation_scan_plan import ( RotationScanComposite, ) -from mx_bluesky.beamlines.i24.parameters.constants import PlanNameConstants from mx_bluesky.common.experiment_plans.inner_plans.read_hardware import ( read_hardware_plan, ) +from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( + RotationISPyBCallback, +) +from mx_bluesky.common.parameters.constants import PlanNameConstants from mx_bluesky.common.parameters.rotation import SingleRotationScan +from tests.expeye_helpers import TEST_DATA_COLLECTION_IDS from tests.unit_tests.beamlines.i24.jungfrau_commissioning.utils import ( get_good_single_rotation_params, ) async def test_metadata_writer_produces_correct_output( - run_engine: RunEngine, tmp_path, rotation_composite: RotationScanComposite + run_engine: RunEngine, + tmp_path, + rotation_composite: RotationScanComposite, + with_numtracker, + mock_ispyb_conn, ): params = get_good_single_rotation_params(tmp_path) metadata_writer = JsonMetadataWriter() @@ -47,9 +56,10 @@ async def test_metadata_writer_produces_correct_output( "energy_kev": energy, "detector_distance_mm": det_z, "angular_increment_deg": 0.1, + "dcid": TEST_DATA_COLLECTION_IDS[0], } run_engine( - _do_metadata_writing_read( + _do_metadata_writing_read_with_ispyb_callbacks( [ rotation_composite.dcm.energy_in_keV, rotation_composite.dcm.wavelength_in_a, @@ -71,7 +81,11 @@ async def test_metadata_writer_produces_correct_output( async def test_assertion_error_if_no_jf_path_found( - run_engine: RunEngine, tmp_path, rotation_composite: RotationScanComposite + run_engine: RunEngine, + tmp_path, + rotation_composite: RotationScanComposite, + with_numtracker, + mock_ispyb_conn, ): params = get_good_single_rotation_params(tmp_path) metadata_writer = JsonMetadataWriter() @@ -85,7 +99,7 @@ async def test_assertion_error_if_no_jf_path_found( with pytest.raises(AssertionError, match="No detector writer path was found"): run_engine( - _do_metadata_writing_read( + _do_metadata_writing_read_with_ispyb_callbacks( [ rotation_composite.dcm.energy_in_keV, rotation_composite.dcm.wavelength_in_a, @@ -98,23 +112,39 @@ async def test_assertion_error_if_no_jf_path_found( ) -def _do_metadata_writing_read( +def _do_metadata_writing_read_with_ispyb_callbacks( signals: list[SignalR | Motor], params: SingleRotationScan, writer: JsonMetadataWriter, ): - @bpp.subs_decorator([writer]) - @bpp.set_run_key_decorator(PlanNameConstants.ROTATION_MAIN) + ispyb_callback = RotationISPyBCallback() + + @bpp.subs_decorator([writer, ispyb_callback]) + @bpp.set_run_key_decorator(PlanNameConstants.ROTATION_OUTER) @bpp.run_decorator( md={ - "subplan_name": PlanNameConstants.ROTATION_MAIN, - "rotation_scan_params": params.model_dump_json(), + "subplan_name": PlanNameConstants.ROTATION_OUTER, + "mx_bluesky_parameters": params.model_dump_json(), + "activate_callbacks": ["RotationISPyBCallback"], } ) - def _inner_read(): - yield from read_hardware_plan( - signals, - PlanNameConstants.ROTATION_DEVICE_READ, + def _rotation_outer(): + yield from bps.null() + + @bpp.set_run_key_decorator(PlanNameConstants.ROTATION_MAIN) + @bpp.run_decorator( + md={ + "subplan_name": PlanNameConstants.ROTATION_MAIN, + "rotation_scan_params": params.model_dump_json(), + "dcid": ispyb_callback.ispyb_ids, + } ) + def _rotation_main(): + yield from read_hardware_plan( + signals, + PlanNameConstants.ROTATION_DEVICE_READ, + ) + + yield from _rotation_main() - yield from _inner_read() + yield from _rotation_outer() diff --git a/tests/unit_tests/beamlines/i24/jungfrau_commissioning/test_rotation_scan.py b/tests/unit_tests/beamlines/i24/jungfrau_commissioning/test_rotation_scan.py index 34380bdd6d..e159f9adff 100644 --- a/tests/unit_tests/beamlines/i24/jungfrau_commissioning/test_rotation_scan.py +++ b/tests/unit_tests/beamlines/i24/jungfrau_commissioning/test_rotation_scan.py @@ -24,11 +24,13 @@ from mx_bluesky.beamlines.i24.jungfrau_commissioning.plan_stubs.plan_utils import ( JF_COMPLETE_GROUP, ) -from mx_bluesky.beamlines.i24.parameters.constants import PlanNameConstants from mx_bluesky.common.experiment_plans.rotation.rotation_utils import ( calculate_motion_profile, ) -from mx_bluesky.common.parameters.constants import PlanGroupCheckpointConstants +from mx_bluesky.common.parameters.constants import ( + PlanGroupCheckpointConstants, + PlanNameConstants, +) from tests.conftest import raw_params_from_file from tests.unit_tests.beamlines.i24.jungfrau_commissioning.utils import ( get_good_single_rotation_params, @@ -70,12 +72,16 @@ async def test_rotation_scan_plan_in_re( run_engine: RunEngine, tmp_path, rotation_composite: RotationScanComposite, + with_numtracker, + mock_ispyb_conn, ): required_hardware_read_signals = [ rotation_composite.dcm.energy_in_keV, rotation_composite.dcm.wavelength_in_a, rotation_composite.det_stage.z, rotation_composite.jungfrau._writer.file_path, + rotation_composite.jungfrau._writer.file_name, + rotation_composite.jungfrau.ispyb_detector_id, ] rotation_composite.jungfrau._writer.final_path = ( @@ -115,6 +121,7 @@ def test_single_rotation_plan_in_simulator( sim_run_engine: RunEngineSimulator, rotation_composite: RotationScanComposite, tmp_path, + with_numtracker, ): params = get_good_single_rotation_params(tmp_path) set_mock_value(rotation_composite.hutch_shutter.status, ShutterState.OPEN) @@ -125,7 +132,7 @@ def test_single_rotation_plan_in_simulator( assert_message_and_return_remaining( msgs, lambda msg: msg.command == "open_run" - and msg.run == "OUTER SINGLE ROTATION SCAN", + and msg.run == PlanNameConstants.ROTATION_OUTER, ) # Wait for rotation devices to be ready before reading metadata @@ -153,7 +160,7 @@ def test_single_rotation_plan_in_simulator( assert_message_and_return_remaining( msgs, lambda msg: msg.command == "close_run" - and msg.run == PlanNameConstants.SINGLE_ROTATION_SCAN, + and msg.run == PlanNameConstants.ROTATION_OUTER, ) diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index adbe3e43ca..923fea5581 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -559,3 +559,9 @@ async def noop(_): @pytest.fixture async def ipin(): yield i03.ipin.build(connect_immediately=True, mock=True) + + +@pytest.fixture +async def with_numtracker(run_engine: RunEngine): + """Plans using numtracker will get their visit from instrument session, set in BlueAPI""" + run_engine.md["instrument_session"] = "cm31105-4" From 51aa6e784aad3db382cf27921a9c8d6139cafc40 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Fri, 20 Feb 2026 15:30:47 +0000 Subject: [PATCH 4/6] pin dodal for ci --- pyproject.toml | 4 ++-- uv.lock | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 0d1d88242c..baa4632f17 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,7 +55,7 @@ dependencies = [ "ophyd >= 1.10.5", "ophyd-async >= 0.14.0", "bluesky >= 1.14.6", - "dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@main", + "dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@2ce4bb42ef4c388fa23e944acb3cc28f2c59144b", ] @@ -94,7 +94,7 @@ dev = [ "tox-uv", "types-mock", "types-requests", - "sphinxcontrib.mermaid", # To build dodal docs + "sphinxcontrib.mermaid", # To build dodal docs ] [project.scripts] diff --git a/uv.lock b/uv.lock index 0ef617fd84..a9204e2520 100644 --- a/uv.lock +++ b/uv.lock @@ -802,8 +802,8 @@ wheels = [ [[package]] name = "dls-dodal" -version = "2.0.1.dev24+gfd21a9ece" -source = { git = "https://github.com/DiamondLightSource/dodal.git?rev=main#fd21a9ece9734c58fa94306a6861d250bdaadef2" } +version = "2.0.1.dev26+g2ce4bb42e" +source = { git = "https://github.com/DiamondLightSource/dodal.git?rev=2ce4bb42ef4c388fa23e944acb3cc28f2c59144b#2ce4bb42ef4c388fa23e944acb3cc28f2c59144b" } dependencies = [ { name = "aiofiles", marker = "platform_machine == 'x86_64' and sys_platform == 'linux'" }, { name = "aiohttp", marker = "platform_machine == 'x86_64' and sys_platform == 'linux'" }, @@ -2108,7 +2108,7 @@ requires-dist = [ { name = "caproto" }, { name = "daq-config-server", specifier = ">=1.0.0rc2" }, { name = "deepdiff" }, - { name = "dls-dodal", git = "https://github.com/DiamondLightSource/dodal.git?rev=main" }, + { name = "dls-dodal", git = "https://github.com/DiamondLightSource/dodal.git?rev=2ce4bb42ef4c388fa23e944acb3cc28f2c59144b" }, { name = "fastapi", extras = ["all"] }, { name = "flask-restful" }, { name = "jupyterlab" }, From fc43d9a56565f771f26f4d50e9ba20bbfb0a818c Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Mon, 23 Feb 2026 09:40:52 +0000 Subject: [PATCH 5/6] tidy up --- .../callbacks/metadata_writer.py | 2 +- .../experiment_plans/rotation_scan_plan.py | 4 +-- .../callbacks/common/ispyb_mapping.py | 3 +++ .../callbacks/rotation/ispyb_callback.py | 26 ++++++++++++------- .../common/parameters/components.py | 2 +- 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py index 508e225256..40700ce27b 100644 --- a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py +++ b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/callbacks/metadata_writer.py @@ -14,7 +14,7 @@ class JsonMetadataWriter(CallbackBase): """Callback class to handle the creation of metadata json files for commissioning. - Currently, nexus files aren't being written by nexgen, so writer needs to include + Currently, nexus files aren't being written by nexgen, so this writer needs to include the dcid produced by an ispyb callback. To get this working, the ispyb callback need to be triggered in the right way before the start function here is run. diff --git a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py index 72002495dc..b9e07b02ed 100644 --- a/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py +++ b/src/mx_bluesky/beamlines/i24/jungfrau_commissioning/experiment_plans/rotation_scan_plan.py @@ -243,9 +243,7 @@ def _rotation_scan_plan( # Read hardware after preparing jungfrau so that device metadata output from callback is correct # Whilst metadata is being written in bluesky we need to access the private writer here - # We also need to access the private writer so that we get path information to send to ispyb. - # This will be needed until numtracker / StartDocumentPathProvider can fully support the MX - # usecase, see #issue + # Won't be accessing private writer when using ophyd-async v0.16 read_hardware_partial = partial( read_hardware_plan, [ diff --git a/src/mx_bluesky/common/external_interaction/callbacks/common/ispyb_mapping.py b/src/mx_bluesky/common/external_interaction/callbacks/common/ispyb_mapping.py index 24ef543394..d660d9b4c9 100644 --- a/src/mx_bluesky/common/external_interaction/callbacks/common/ispyb_mapping.py +++ b/src/mx_bluesky/common/external_interaction/callbacks/common/ispyb_mapping.py @@ -43,6 +43,9 @@ def populate_remaining_data_collection_info( "Expected RunStart doc to be provided to populate_remaining_data_collection_info when using numtracker for visit" ) data_collection_info.visit_string = doc.get("instrument_session") + assert isinstance(data_collection_info.visit_string, str), ( + "Failed to get instrument session during ispyb callbacks" + ) else: data_collection_info.visit_string = params.visit data_collection_info.imgdir = params.detector_params.directory diff --git a/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py b/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py index 0408848fd1..c75153c20a 100644 --- a/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py +++ b/src/mx_bluesky/common/external_interaction/callbacks/rotation/ispyb_callback.py @@ -27,7 +27,10 @@ IspybIds, StoreInIspyb, ) -from mx_bluesky.common.parameters.components import IspybExperimentType +from mx_bluesky.common.parameters.components import ( + DiffractionExperiment, + IspybExperimentType, +) from mx_bluesky.common.parameters.constants import USE_NUMTRACKER, PlanNameConstants from mx_bluesky.common.parameters.rotation import ( SingleRotationScan, @@ -40,6 +43,17 @@ from event_model.documents import Event, RunStart, RunStop +def get_instrument_session_from_md(doc: RunStart, params: DiffractionExperiment): + try: + visit = doc.get("instrument_session") + assert isinstance(visit, str) + params.visit = visit + except Exception as e: + raise ValueError( + f"Error trying to retrieve instrument session from document {doc}" + ) from e + + class RotationISPyBCallback(BaseISPyBCallback): """Callback class to handle the deposition of experiment parameters into the ISPyB database. Listens for 'event' and 'descriptor' documents. Creates the ISpyB entry on @@ -74,16 +88,8 @@ def activity_gated_start(self, doc: RunStart): assert isinstance(params, str) self.params = SingleRotationScan.model_validate_json(params) - # Todo this chunk needs to go at the start of any numtracker+ispyb-using plan if self.params.visit == USE_NUMTRACKER: - try: - visit = doc.get("instrument_session") - assert isinstance(visit, str) - self.params.visit = visit - except Exception as e: - raise ValueError( - f"Error trying to retrieve instrument session from document {doc}" - ) from e + get_instrument_session_from_md(doc, self.params) dcgid = ( self.ispyb_ids.data_collection_group_id diff --git a/src/mx_bluesky/common/parameters/components.py b/src/mx_bluesky/common/parameters/components.py index 26acbf2d5c..99549ce1bc 100644 --- a/src/mx_bluesky/common/parameters/components.py +++ b/src/mx_bluesky/common/parameters/components.py @@ -239,7 +239,7 @@ def scan_indices(self) -> Sequence[SupportsInt]: class WithSample(BaseModel): sample_id: int | None = ( - None # None is invalid for dc groups, but valid for regular data collections + None # Expeye won't accept None for dc groups, but it will for regular data collections ) sample_puck: int | None = None sample_pin: int | None = None From 7822e1ac5dcc165a044e20f821f262508c3b1b66 Mon Sep 17 00:00:00 2001 From: Oliver Silvester Date: Mon, 23 Feb 2026 10:52:56 +0000 Subject: [PATCH 6/6] fix codecov --- .../experiment_plans/robot_load_plan.py | 1 + .../load_centre_collect_full_plan.py | 2 + .../robot_load_and_change_energy.py | 2 + .../aithre_lasershaping/test_robot_load.py | 3 ++ .../callbacks/common/test_ispyb_mapping.py | 38 +++++++++++++++++++ .../callbacks/rotation/test_ispyb_callback.py | 13 +++++++ 6 files changed, 59 insertions(+) create mode 100644 tests/unit_tests/common/external_interaction/callbacks/common/test_ispyb_mapping.py diff --git a/src/mx_bluesky/beamlines/aithre_lasershaping/experiment_plans/robot_load_plan.py b/src/mx_bluesky/beamlines/aithre_lasershaping/experiment_plans/robot_load_plan.py index 3fbcd1229f..ecd90e73df 100644 --- a/src/mx_bluesky/beamlines/aithre_lasershaping/experiment_plans/robot_load_plan.py +++ b/src/mx_bluesky/beamlines/aithre_lasershaping/experiment_plans/robot_load_plan.py @@ -142,6 +142,7 @@ def robot_load_and_snapshots_plan( ): assert params.sample_puck is not None assert params.sample_pin is not None + assert params.sample_id sample_location = SampleLocation(params.sample_puck, params.sample_pin) 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 00b52a9f5e..0aa904667e 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 @@ -165,6 +165,8 @@ def _samples_and_locations_to_collect( initial_y_mm = yield from bps.rd(composite.gonio.y.user_readback) initial_z_mm = yield from bps.rd(composite.gonio.z.user_readback) + assert parameters.sample_id + return [ ( parameters.sample_id, diff --git a/src/mx_bluesky/hyperion/experiment_plans/robot_load_and_change_energy.py b/src/mx_bluesky/hyperion/experiment_plans/robot_load_and_change_energy.py index a8148efc11..68b35f54c8 100644 --- a/src/mx_bluesky/hyperion/experiment_plans/robot_load_and_change_energy.py +++ b/src/mx_bluesky/hyperion/experiment_plans/robot_load_and_change_energy.py @@ -155,6 +155,8 @@ def robot_load_and_change_energy_plan( yield from prepare_for_robot_load(composite.aperture_scatterguard, composite.gonio) + assert params.sample_id + yield from bpp.set_run_key_wrapper( bpp.run_wrapper( robot_load_and_snapshots( diff --git a/tests/unit_tests/beamlines/aithre_lasershaping/test_robot_load.py b/tests/unit_tests/beamlines/aithre_lasershaping/test_robot_load.py index 20f68e3b3e..6667325c8a 100644 --- a/tests/unit_tests/beamlines/aithre_lasershaping/test_robot_load.py +++ b/tests/unit_tests/beamlines/aithre_lasershaping/test_robot_load.py @@ -239,6 +239,7 @@ def test_when_robot_load_and_snapshot_plan_called_correct_plan_called( tip_offset = 0 oav_config = CONST.OAV_CENTRING_FILE mock_pin_tip_detection = MagicMock(spec=PinTipDetection) + assert robot_load_params.sample_id run_engine( robot_load_and_snapshot( aithre_robot_load_composite.robot, @@ -267,6 +268,8 @@ def test_when_robot_unload_plan_called_correct_plan_called( robot_load_params: AithreRobotLoad, run_engine: RunEngine, ): + assert robot_load_params.sample_id + run_engine( robot_unload( aithre_robot_load_composite.robot, diff --git a/tests/unit_tests/common/external_interaction/callbacks/common/test_ispyb_mapping.py b/tests/unit_tests/common/external_interaction/callbacks/common/test_ispyb_mapping.py new file mode 100644 index 0000000000..9ab4a8878a --- /dev/null +++ b/tests/unit_tests/common/external_interaction/callbacks/common/test_ispyb_mapping.py @@ -0,0 +1,38 @@ +import pytest +from event_model.documents import RunStart + +from mx_bluesky.common.external_interaction.callbacks.common.ispyb_mapping import ( + populate_remaining_data_collection_info, +) +from mx_bluesky.common.external_interaction.ispyb.data_model import DataCollectionInfo +from mx_bluesky.common.parameters.constants import USE_NUMTRACKER +from mx_bluesky.common.parameters.gridscan import SpecifiedThreeDGridScan + + +def test_populate_remaining_data_collection_info_no_doc( + test_fgs_params: SpecifiedThreeDGridScan, +): + test_fgs_params.visit = USE_NUMTRACKER + dc = DataCollectionInfo() + with pytest.raises(AssertionError, match="Expected RunStart doc to be provided"): + populate_remaining_data_collection_info("str", 10, dc, test_fgs_params) + + +def test_populate_remaining_data_collection_info_no_visit( + test_fgs_params: SpecifiedThreeDGridScan, +): + test_fgs_params.visit = USE_NUMTRACKER + dc = DataCollectionInfo() + doc = RunStart(uid="id", time=0) + with pytest.raises(AssertionError, match="Failed to get instrument session "): + populate_remaining_data_collection_info("str", 10, dc, test_fgs_params, doc) + + +def test_good_populate_remaining_data_collection_info( + test_fgs_params: SpecifiedThreeDGridScan, +): + test_fgs_params.visit = USE_NUMTRACKER + doc = RunStart(uid="id", time=0) + doc["instrument_session"] = "0" # type: ignore + dc = DataCollectionInfo() + populate_remaining_data_collection_info("str", 10, dc, test_fgs_params, doc) diff --git a/tests/unit_tests/hyperion/external_interaction/callbacks/rotation/test_ispyb_callback.py b/tests/unit_tests/hyperion/external_interaction/callbacks/rotation/test_ispyb_callback.py index 31e2e2e4e1..9812726eaf 100644 --- a/tests/unit_tests/hyperion/external_interaction/callbacks/rotation/test_ispyb_callback.py +++ b/tests/unit_tests/hyperion/external_interaction/callbacks/rotation/test_ispyb_callback.py @@ -1,9 +1,14 @@ import json from unittest.mock import MagicMock, patch +import pytest +from event_model.documents import RunStart + from mx_bluesky.common.external_interaction.callbacks.rotation.ispyb_callback import ( RotationISPyBCallback, + get_instrument_session_from_md, ) +from mx_bluesky.common.parameters.gridscan import SpecifiedThreeDGridScan from ......conftest import ( EXPECTED_END_TIME, @@ -283,3 +288,11 @@ def test_comment_correct_after_hardware_read( assert update_dc_comment_req.body == { "comments": " Sample position (µm): (158, 24, 3)" } + + +def test_exception_if_no_instrument_session_in_md( + test_fgs_params: SpecifiedThreeDGridScan, +): + doc = RunStart(time=0, uid="0") + with pytest.raises(ValueError, match="Error trying to retrieve instrument session"): + get_instrument_session_from_md(doc, test_fgs_params)