From 09e8e2a3b3ec7c26cb9bcda19b92cb09e3a0cdc3 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Wed, 8 Jan 2025 13:16:35 +0000 Subject: [PATCH] Remove initial_xyz and just read in the setup plan (#673) * Remove read of smargon initial_xyz and just read it in the setup plan * Remove sample_motors from various composites --------- Co-authored-by: Robert Tuck --- .../flyscan_xray_centre_plan.py | 27 ++++------------- .../grid_detect_then_xray_centre_plan.py | 4 --- .../load_centre_collect_full_plan.py | 5 ---- .../robot_load_then_centre_plan.py | 4 --- .../experiment_plans/test_fgs_plan.py | 30 ++++++------------- .../test_manipulate_sample.py | 16 +++++----- .../test_flyscan_xray_centre_plan.py | 14 ++++----- 7 files changed, 28 insertions(+), 72 deletions(-) diff --git a/src/mx_bluesky/hyperion/experiment_plans/flyscan_xray_centre_plan.py b/src/mx_bluesky/hyperion/experiment_plans/flyscan_xray_centre_plan.py index cb18a6964..43a95a034 100755 --- a/src/mx_bluesky/hyperion/experiment_plans/flyscan_xray_centre_plan.py +++ b/src/mx_bluesky/hyperion/experiment_plans/flyscan_xray_centre_plan.py @@ -112,11 +112,6 @@ class FlyScanXRayCentreComposite: robot: BartRobot sample_shutter: ZebraShutter - @property - def sample_motors(self) -> Smargon: - """Convenience alias with a more user-friendly name""" - return self.smargon - class XRayCentreEventHandler(CallbackBase): def __init__(self): @@ -225,16 +220,7 @@ def run_gridscan_and_fetch_results( """A multi-run plan which runs a gridscan, gets the results from zocalo and fires an event with the centres of mass determined by zocalo""" - # We get the initial motor positions so we can return to them on zocalo failure - initial_xyz = np.array( - [ - (yield from bps.rd(fgs_composite.sample_motors.x)), - (yield from bps.rd(fgs_composite.sample_motors.y)), - (yield from bps.rd(fgs_composite.sample_motors.z)), - ] - ) - - yield from feature_controlled.setup_trigger(fgs_composite, parameters, initial_xyz) + yield from feature_controlled.setup_trigger(fgs_composite, parameters) LOGGER.info("Starting grid scan") yield from bps.stage( @@ -331,11 +317,9 @@ def run_gridscan( "plan_name": CONST.PLAN.GRIDSCAN_MAIN, }, ): - sample_motors = fgs_composite.sample_motors - # Currently gridscan only works for omega 0, see # with TRACER.start_span("moving_omega_to_0"): - yield from bps.abs_set(sample_motors.omega, 0) + yield from bps.abs_set(fgs_composite.smargon.omega, 0) # We only subscribe to the communicator callback for run_gridscan, so this is where # we should generate an event reading the values which need to be included in the @@ -408,7 +392,6 @@ def __call__( self, fgs_composite: FlyScanXRayCentreComposite, parameters: HyperionThreeDGridScan, - initial_xyz: np.ndarray, ) -> MsgGenerator: ... setup_trigger: _ExtraSetup @@ -469,7 +452,6 @@ def _panda_tidy(fgs_composite: FlyScanXRayCentreComposite): def _zebra_triggering_setup( fgs_composite: FlyScanXRayCentreComposite, parameters: HyperionThreeDGridScan, - initial_xyz: np.ndarray, ): yield from setup_zebra_for_gridscan( fgs_composite.zebra, fgs_composite.sample_shutter, wait=True @@ -479,7 +461,6 @@ def _zebra_triggering_setup( def _panda_triggering_setup( fgs_composite: FlyScanXRayCentreComposite, parameters: HyperionThreeDGridScan, - initial_xyz: np.ndarray, ): LOGGER.info("Setting up Panda for flyscan") @@ -492,6 +473,8 @@ def _panda_triggering_setup( time_between_x_steps_ms = (DEADTIME_S + parameters.exposure_time_s) * 1e3 + current_x = yield from bps.rd(fgs_composite.smargon.x.user_readback) + smargon_speed_limit_mm_per_s = yield from bps.rd( fgs_composite.smargon.x.max_velocity ) @@ -524,7 +507,7 @@ def _panda_triggering_setup( yield from setup_panda_for_flyscan( fgs_composite.panda, parameters.panda_FGS_params, - initial_xyz[0], + current_x, parameters.exposure_time_s, time_between_x_steps_ms, sample_velocity_mm_per_s, diff --git a/src/mx_bluesky/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py b/src/mx_bluesky/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py index 0945e064f..8afdd1f80 100644 --- a/src/mx_bluesky/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py +++ b/src/mx_bluesky/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py @@ -95,10 +95,6 @@ class GridDetectThenXRayCentreComposite: robot: BartRobot sample_shutter: ZebraShutter - @property - def sample_motors(self): - return self.smargon - def create_devices(context: BlueskyContext) -> GridDetectThenXRayCentreComposite: return device_composite_from_context(context, GridDetectThenXRayCentreComposite) 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 8f1a3a0bd..161a17135 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 @@ -7,7 +7,6 @@ from bluesky.preprocessors import run_decorator, set_run_key_decorator, subs_wrapper from bluesky.utils import MsgGenerator from dodal.devices.oav.oav_parameters import OAVParameters -from dodal.devices.smargon import Smargon import mx_bluesky.hyperion.experiment_plans.common.xrc_result as flyscan_result from mx_bluesky.common.utils.log import LOGGER @@ -35,10 +34,6 @@ class LoadCentreCollectComposite(RobotLoadThenCentreComposite, RotationScanComposite): """Composite that provides access to the required devices.""" - @property - def sample_motors(self) -> Smargon: - return self.smargon - def create_devices(context: BlueskyContext) -> LoadCentreCollectComposite: """Create the necessary devices for the plan.""" 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 6359e661b..8e37c5193 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 @@ -104,10 +104,6 @@ class RobotLoadThenCentreComposite: lower_gonio: XYZPositioner beamstop: Beamstop - @property - def sample_motors(self): - return self.smargon - def create_devices(context: BlueskyContext) -> RobotLoadThenCentreComposite: from mx_bluesky.hyperion.utils.context import device_composite_from_context diff --git a/tests/system_tests/hyperion/experiment_plans/test_fgs_plan.py b/tests/system_tests/hyperion/experiment_plans/test_fgs_plan.py index 7a936e7c9..79ed76711 100644 --- a/tests/system_tests/hyperion/experiment_plans/test_fgs_plan.py +++ b/tests/system_tests/hyperion/experiment_plans/test_fgs_plan.py @@ -298,18 +298,9 @@ def zocalo_trigger(): RE(flyscan_xray_centre(fxc_composite, params)) # The following numbers are derived from the centre returned in fake_zocalo - assert ( - await fxc_composite.sample_motors.x.user_readback.get_value() - == pytest.approx(-1) - ) - assert ( - await fxc_composite.sample_motors.y.user_readback.get_value() - == pytest.approx(-1) - ) - assert ( - await fxc_composite.sample_motors.z.user_readback.get_value() - == pytest.approx(-1) - ) + assert await fxc_composite.smargon.x.user_readback.get_value() == pytest.approx(-1) + assert await fxc_composite.smargon.y.user_readback.get_value() == pytest.approx(-1) + assert await fxc_composite.smargon.z.user_readback.get_value() == pytest.approx(-1) @pytest.mark.s03 @@ -336,15 +327,12 @@ async def test_complete_xray_centre_plan_with_callbacks_moves_to_centre( RE(flyscan_xray_centre(fxc_composite, params)) # The following numbers are derived from the centre returned in fake_zocalo - assert ( - await fxc_composite.sample_motors.x.user_readback.get_value() - == pytest.approx(0.05) + assert await fxc_composite.smargon.x.user_readback.get_value() == pytest.approx( + 0.05 ) - assert ( - await fxc_composite.sample_motors.y.user_readback.get_value() - == pytest.approx(0.15) + assert await fxc_composite.smargon.y.user_readback.get_value() == pytest.approx( + 0.15 ) - assert ( - await fxc_composite.sample_motors.z.user_readback.get_value() - == pytest.approx(0.25) + assert await fxc_composite.smargon.z.user_readback.get_value() == pytest.approx( + 0.25 ) diff --git a/tests/unit_tests/hyperion/device_setup_plans/test_manipulate_sample.py b/tests/unit_tests/hyperion/device_setup_plans/test_manipulate_sample.py index 67b611469..04cea4328 100644 --- a/tests/unit_tests/hyperion/device_setup_plans/test_manipulate_sample.py +++ b/tests/unit_tests/hyperion/device_setup_plans/test_manipulate_sample.py @@ -67,14 +67,14 @@ def test_move_x_y_z( motor_position: list[float], expected_moves: list[float | None], ): - RE(move_x_y_z(fake_fgs_composite.sample_motors, *motor_position)) # type: ignore + RE(move_x_y_z(fake_fgs_composite.smargon, *motor_position)) # type: ignore expected_calls = [ call(axis, pos, group="move_x_y_z") for axis, pos in zip( [ - fake_fgs_composite.sample_motors.x, - fake_fgs_composite.sample_motors.y, - fake_fgs_composite.sample_motors.z, + fake_fgs_composite.smargon.x, + fake_fgs_composite.smargon.y, + fake_fgs_composite.smargon.z, ], expected_moves, strict=False, @@ -108,14 +108,14 @@ def test_move_phi_chi_omega( motor_position: list[float], expected_moves: list[float | None], ): - RE(move_phi_chi_omega(fake_fgs_composite.sample_motors, *motor_position)) # type: ignore + RE(move_phi_chi_omega(fake_fgs_composite.smargon, *motor_position)) # type: ignore expected_calls = [ call(axis, pos, group="move_phi_chi_omega") for axis, pos in zip( [ - fake_fgs_composite.sample_motors.phi, - fake_fgs_composite.sample_motors.chi, - fake_fgs_composite.sample_motors.omega, + fake_fgs_composite.smargon.phi, + fake_fgs_composite.smargon.chi, + fake_fgs_composite.smargon.omega, ], expected_moves, strict=False, diff --git a/tests/unit_tests/hyperion/experiment_plans/test_flyscan_xray_centre_plan.py b/tests/unit_tests/hyperion/experiment_plans/test_flyscan_xray_centre_plan.py index fa75af7bf..d9bb63011 100644 --- a/tests/unit_tests/hyperion/experiment_plans/test_flyscan_xray_centre_plan.py +++ b/tests/unit_tests/hyperion/experiment_plans/test_flyscan_xray_centre_plan.py @@ -201,9 +201,7 @@ def test_when_run_gridscan_called_ispyb_deposition_made_and_records_errors( error = None with pytest.raises(FailedStatus) as exc: - with patch.object( - fake_fgs_composite.sample_motors.omega, "set" - ) as mock_set: + with patch.object(fake_fgs_composite.smargon.omega, "set") as mock_set: error = AssertionError("Test Exception") mock_set.return_value = FailedStatus(error) @@ -415,7 +413,7 @@ def test_results_adjusted_and_passed_to_move_xyz( move_aperture.assert_has_calls([ap_call_large, ap_call_large, ap_call_medium]) mv_to_centre = call( - fgs_composite_with_panda_pcap.sample_motors, + fgs_composite_with_panda_pcap.smargon, 0.05, pytest.approx(0.15), 0.25, @@ -438,21 +436,21 @@ def test_results_passed_to_move_motors( motor_position = test_fgs_params.FGS_params.grid_position_to_motor_position( np.array([1, 2, 3]) ) - RE(move_x_y_z(fake_fgs_composite.sample_motors, *motor_position)) + RE(move_x_y_z(fake_fgs_composite.smargon, *motor_position)) bps_abs_set.assert_has_calls( [ call( - fake_fgs_composite.sample_motors.x, + fake_fgs_composite.smargon.x, motor_position[0], group="move_x_y_z", ), call( - fake_fgs_composite.sample_motors.y, + fake_fgs_composite.smargon.y, motor_position[1], group="move_x_y_z", ), call( - fake_fgs_composite.sample_motors.z, + fake_fgs_composite.smargon.z, motor_position[2], group="move_x_y_z", ),