From 1dc26a58dfe242cdb5895ca9029f6a11c60edf51 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 18 Sep 2024 10:44:21 +0100 Subject: [PATCH] Hotfixes found from testing on the beamline (#477) * Test that robot load and centre composite can be converted to centring and fix * Fix parameter conversion and add test for it * Fix rotation plan to properly trigger zocalo and add tests * Fix assumption in dls_dev_env * Fix zocalo system test --------- Co-authored-by: Robert Tuck --- .../robot_load_then_centre_plan.py | 2 + .../experiment_plans/rotation_scan_plan.py | 1 + .../callbacks/ispyb_callback_base.py | 6 +- tests/conftest.py | 8 +- .../test_zocalo_system.py | 1 + .../test_rotation_scan_plan.py | 73 +++++++++++++++++++ .../test_wait_for_robot_load_then_centre.py | 72 +++++++++++++++--- utility_scripts/dls_dev_env.sh | 17 ++++- 8 files changed, 161 insertions(+), 19 deletions(-) diff --git a/src/mx_bluesky/hyperion/experiment_plans/robot_load_then_centre_plan.py b/src/mx_bluesky/hyperion/experiment_plans/robot_load_then_centre_plan.py index 310c0348d..ab412cff5 100644 --- a/src/mx_bluesky/hyperion/experiment_plans/robot_load_then_centre_plan.py +++ b/src/mx_bluesky/hyperion/experiment_plans/robot_load_then_centre_plan.py @@ -32,6 +32,7 @@ from dodal.devices.webcam import Webcam from dodal.devices.xbpm_feedback import XBPMFeedback from dodal.devices.zebra import Zebra +from dodal.devices.zebra_controlled_shutter import ZebraShutter from dodal.devices.zocalo import ZocaloResults from dodal.plans.motor_util_plans import MoveTooLarge, home_and_reset_wrapper from ophyd_async.fastcs.panda import HDFPanda @@ -79,6 +80,7 @@ class RobotLoadThenCentreComposite: panda: HDFPanda panda_fast_grid_scan: PandAFastGridScan thawer: Thawer + sample_shutter: ZebraShutter # SetEnergyComposite fields vfm: FocusingMirrorWithStripes 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 98a6a27d3..4e8f7a20c 100644 --- a/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py +++ b/src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py @@ -353,6 +353,7 @@ def rotation_scan( md={ "subplan_name": CONST.PLAN.ROTATION_OUTER, CONST.TRIGGER.ZOCALO: CONST.PLAN.ROTATION_MAIN, + "zocalo_environment": parameters.zocalo_environment, "hyperion_parameters": parameters.model_dump_json(), "activate_callbacks": [ "RotationISPyBCallback", diff --git a/src/mx_bluesky/hyperion/external_interaction/callbacks/ispyb_callback_base.py b/src/mx_bluesky/hyperion/external_interaction/callbacks/ispyb_callback_base.py index 3bd77cb91..6eaa3712b 100644 --- a/src/mx_bluesky/hyperion/external_interaction/callbacks/ispyb_callback_base.py +++ b/src/mx_bluesky/hyperion/external_interaction/callbacks/ispyb_callback_base.py @@ -166,8 +166,8 @@ def populate_info_for_update( def activity_gated_stop(self, doc: RunStop) -> RunStop: """Subclasses must check that they are recieving a stop document for the correct uid to use this method!""" - assert isinstance( - self.ispyb, StoreInIspyb + assert ( + self.ispyb is not None ), "ISPyB handler received stop document, but deposition object doesn't exist!" ISPYB_LOGGER.debug("ISPyB handler received stop document.") exit_status = ( @@ -184,7 +184,7 @@ def activity_gated_stop(self, doc: RunStop) -> RunStop: return self._tag_doc(doc) def _append_to_comment(self, id: int, comment: str) -> None: - assert isinstance(self.ispyb, StoreInIspyb) + assert self.ispyb is not None try: self.ispyb.append_to_comment(id, comment) except TypeError: diff --git a/tests/conftest.py b/tests/conftest.py index 302647ff0..3bb0ddaf4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -294,8 +294,7 @@ def smargon(RE: RunEngine) -> Generator[Smargon, None, None]: @pytest.fixture -def zebra(): - RunEngine() +def zebra(RE): zebra = i03.zebra(fake_with_ophyd_sim=True) def mock_side(*args, **kwargs): @@ -695,6 +694,11 @@ def mock_gridscan_kickoff_complete(gridscan: FastGridScanCommon): gridscan.complete = MagicMock(return_value=async_status_done) +@pytest.fixture +def panda_fast_grid_scan(RE): + return i03.panda_fast_grid_scan(fake_with_ophyd_sim=True) + + @pytest.fixture async def fake_fgs_composite( smargon: Smargon, diff --git a/tests/system_tests/hyperion/external_interaction/test_zocalo_system.py b/tests/system_tests/hyperion/external_interaction/test_zocalo_system.py index f8a54d2c9..69d86255f 100644 --- a/tests/system_tests/hyperion/external_interaction/test_zocalo_system.py +++ b/tests/system_tests/hyperion/external_interaction/test_zocalo_system.py @@ -75,6 +75,7 @@ def trigger_zocalo_after_fast_grid_scan(): md={ "subplan_name": CONST.PLAN.GRIDSCAN_OUTER, CONST.TRIGGER.ZOCALO: CONST.PLAN.DO_FGS, + "zocalo_environment": dummy_params.zocalo_environment, "hyperion_parameters": dummy_params.model_dump_json(), } ) 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 88221509e..26959d36a 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 @@ -28,6 +28,13 @@ rotation_scan, rotation_scan_plan, ) +from mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback import ( + RotationISPyBCallback, +) +from mx_bluesky.hyperion.external_interaction.callbacks.zocalo_callback import ( + ZocaloCallback, +) +from mx_bluesky.hyperion.external_interaction.ispyb.ispyb_store import IspybIds from mx_bluesky.hyperion.parameters.constants import CONST, DocDescriptorNames from mx_bluesky.hyperion.parameters.rotation import RotationScan @@ -600,3 +607,69 @@ def test_rotation_scan_arms_detector_and_takes_snapshots_whilst_arming( lambda msg: msg.command == "wait" and msg.kwargs["group"] == CONST.WAIT.ROTATION_READY_FOR_DC, ) + + +@patch( + "mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb" +) +def test_rotation_scan_correctly_triggers_ispyb_callback( + mock_store_in_ispyb, + RE: RunEngine, + test_rotation_params: RotationScan, + fake_create_rotation_devices: RotationScanComposite, + oav_parameters_for_rotation: OAVParameters, +): + mock_ispyb_callback = RotationISPyBCallback() + RE.subscribe(mock_ispyb_callback) + with ( + patch("bluesky.plan_stubs.wait", autospec=True), + patch( + "bluesky.preprocessors.__read_and_stash_a_motor", + fake_read, + ), + ): + RE( + rotation_scan( + fake_create_rotation_devices, + test_rotation_params, + oav_parameters_for_rotation, + ), + ) + mock_store_in_ispyb.assert_called() + + +@patch( + "mx_bluesky.hyperion.external_interaction.callbacks.zocalo_callback.ZocaloTrigger" +) +@patch( + "mx_bluesky.hyperion.external_interaction.callbacks.rotation.ispyb_callback.StoreInIspyb" +) +def test_rotation_scan_correctly_triggers_zocalo_callback( + mock_store_in_ispyb, + mock_zocalo_interactor, + RE: RunEngine, + test_rotation_params: RotationScan, + fake_create_rotation_devices: RotationScanComposite, + oav_parameters_for_rotation: OAVParameters, +): + mock_zocalo_callback = ZocaloCallback() + mock_ispyb_callback = RotationISPyBCallback(emit=mock_zocalo_callback) + mock_store_in_ispyb.return_value.update_deposition.return_value = IspybIds( + data_collection_ids=(0, 1) + ) + RE.subscribe(mock_ispyb_callback) + with ( + patch("bluesky.plan_stubs.wait", autospec=True), + patch( + "bluesky.preprocessors.__read_and_stash_a_motor", + fake_read, + ), + ): + RE( + rotation_scan( + fake_create_rotation_devices, + test_rotation_params, + oav_parameters_for_rotation, + ), + ) + mock_zocalo_interactor.return_value.run_start.assert_called_once() diff --git a/tests/unit_tests/hyperion/experiment_plans/test_wait_for_robot_load_then_centre.py b/tests/unit_tests/hyperion/experiment_plans/test_wait_for_robot_load_then_centre.py index 371425919..d19575a26 100644 --- a/tests/unit_tests/hyperion/experiment_plans/test_wait_for_robot_load_then_centre.py +++ b/tests/unit_tests/hyperion/experiment_plans/test_wait_for_robot_load_then_centre.py @@ -13,6 +13,9 @@ from ophyd.sim import NullStatus from ophyd_async.core import set_mock_value +from mx_bluesky.hyperion.experiment_plans.grid_detect_then_xray_centre_plan import ( + GridDetectThenXRayCentreComposite, +) from mx_bluesky.hyperion.experiment_plans.robot_load_then_centre_plan import ( RobotLoadThenCentreComposite, prepare_for_robot_load, @@ -33,21 +36,67 @@ @pytest.fixture def robot_load_composite( - smargon, dcm, robot, aperture_scatterguard, oav, webcam, thawer, lower_gonio, eiger + smargon, + dcm, + robot, + aperture_scatterguard, + oav, + webcam, + thawer, + lower_gonio, + eiger, + xbpm_feedback, + flux, + zocalo, + panda, + backlight, + attenuator, + pin_tip, + fast_grid_scan, + detector_motion, + synchrotron, + s4_slit_gaps, + undulator, + zebra, + panda_fast_grid_scan, + vfm, + vfm_mirror_voltages, + undulator_dcm, + sample_shutter, ) -> RobotLoadThenCentreComposite: - composite: RobotLoadThenCentreComposite = MagicMock() - composite.smargon = smargon - composite.dcm = dcm + composite: RobotLoadThenCentreComposite = RobotLoadThenCentreComposite( + smargon=smargon, + dcm=dcm, + robot=robot, + aperture_scatterguard=aperture_scatterguard, + oav=oav, + webcam=webcam, + lower_gonio=lower_gonio, + thawer=thawer, + eiger=eiger, + xbpm_feedback=xbpm_feedback, + flux=flux, + zocalo=zocalo, + panda=panda, + backlight=backlight, + attenuator=attenuator, + pin_tip_detection=pin_tip, + zebra_fast_grid_scan=fast_grid_scan, + detector_motion=detector_motion, + synchrotron=synchrotron, + s4_slit_gaps=s4_slit_gaps, + undulator=undulator, + zebra=zebra, + panda_fast_grid_scan=panda_fast_grid_scan, + vfm=vfm, + vfm_mirror_voltages=vfm_mirror_voltages, + undulator_dcm=undulator_dcm, + sample_shutter=sample_shutter, + ) set_mock_value(composite.dcm.energy_in_kev.user_readback, 11.105) - composite.robot = robot composite.aperture_scatterguard = aperture_scatterguard composite.smargon.stub_offsets.set = MagicMock(return_value=NullStatus()) composite.aperture_scatterguard.set = MagicMock(return_value=NullStatus()) - composite.oav = oav - composite.webcam = webcam - composite.lower_gonio = lower_gonio - composite.thawer = thawer - composite.eiger = eiger return composite @@ -90,6 +139,9 @@ def test_when_plan_run_then_centring_plan_run_with_expected_parameters( for name, value in vars(composite_passed).items(): assert value == getattr(robot_load_composite, name) + for name in GridDetectThenXRayCentreComposite.__dataclass_fields__.keys(): + assert getattr(composite_passed, name), f"{name} not in composite" + assert isinstance(params_passed, PinTipCentreThenXrayCentre) assert params_passed.detector_params.expected_energy_ev == 11100 diff --git a/utility_scripts/dls_dev_env.sh b/utility_scripts/dls_dev_env.sh index 3a20909ca..26b6bd254 100755 --- a/utility_scripts/dls_dev_env.sh +++ b/utility_scripts/dls_dev_env.sh @@ -1,9 +1,18 @@ #!/bin/bash -# Check we're in the right place -dir_name=${PWD##*/} -if [ "$dir_name" != "mx-bluesky" ]; then - echo "This script should be run from the 'mx-bluesky' directory" +# Check we're in the right place (assuming the location of this script in the repo) + +# Get the directory where the script is located +script_dir=$(dirname "$(readlink -f "$0")") + +# Get the current working directory +current_dir=$(pwd) + +# Get the directory up from the script's location +two_levels_up=$(dirname "$script_dir") + +if [ "$current_dir" != "$two_levels_up" ]; then + echo "This script should be run from the top directory of the repo" exit 1 fi