diff --git a/src/artemis/devices/aperturescatterguard.py b/src/artemis/devices/aperturescatterguard.py index 118dd64b7..c97ce071f 100644 --- a/src/artemis/devices/aperturescatterguard.py +++ b/src/artemis/devices/aperturescatterguard.py @@ -9,9 +9,13 @@ from artemis.devices.scatterguard import Scatterguard +class InvalidApertureMove(Exception): + pass + + @dataclass class AperturePositions: - """Holds the tuple (miniap_x, miniap_y, miniap_z, scatterguard_x, scatterguard_y) + """Holds tuples (miniap_x, miniap_y, miniap_z, scatterguard_x, scatterguard_y) representing the motor positions needed to select a particular aperture size. """ @@ -53,6 +57,14 @@ def from_gda_beamline_params(cls, params): ), ) + def position_valid(self, pos: tuple[float, float, float, float, float]): + """ + Check if argument 'pos' is a valid position in this AperturePositions object. + """ + if pos not in [self.LARGE, self.MEDIUM, self.SMALL, self.ROBOT_LOAD]: + return False + return True + class ApertureScatterguard(InfoLoggingDevice): aperture: Aperture = Cpt(Aperture, "-MO-MAPT-01:") @@ -62,26 +74,34 @@ class ApertureScatterguard(InfoLoggingDevice): def load_aperture_positions(self, positions: AperturePositions): self.aperture_positions = positions - def safe_move_within_datacollection_range( + def set(self, pos: tuple[float, float, float, float, float]) -> AndStatus: + try: + assert isinstance(self.aperture_positions, AperturePositions) + assert self.aperture_positions.position_valid(pos) + except AssertionError as e: + raise InvalidApertureMove(repr(e)) + return self._safe_move_within_datacollection_range(*pos) + + def _safe_move_within_datacollection_range( self, aperture_x: float, aperture_y: float, aperture_z: float, scatterguard_x: float, scatterguard_y: float, - ) -> None: + ) -> AndStatus: """ Move the aperture and scatterguard combo safely to a new position """ - assert isinstance(self.aperture_positions, AperturePositions) - + # EpicsMotor does not have deadband/MRES field, so the way to check if we are + # in a datacollection position is to see if we are "ready" (DMOV) and the target + # position is correct ap_z_in_position = self.aperture.z.motor_done_move.get() if not ap_z_in_position: return - current_ap_z = self.aperture.z.user_setpoint.get() if current_ap_z != aperture_z: - raise Exception( + raise InvalidApertureMove( "ApertureScatterguard safe move is not yet defined for positions " "outside of LARGE, MEDIUM, SMALL, ROBOT_LOAD." ) @@ -92,9 +112,13 @@ def safe_move_within_datacollection_range( scatterguard_x ) & self.scatterguard.y.set(scatterguard_y) sg_status.wait() - self.aperture.x.set(aperture_x) - self.aperture.y.set(aperture_y) - self.aperture.z.set(aperture_z) + final_status = ( + sg_status + & self.aperture.x.set(aperture_x) + & self.aperture.y.set(aperture_y) + & self.aperture.z.set(aperture_z) + ) + return final_status else: ap_status: AndStatus = ( @@ -103,5 +127,9 @@ def safe_move_within_datacollection_range( & self.aperture.z.set(aperture_z) ) ap_status.wait() - self.scatterguard.x.set(scatterguard_x) - self.scatterguard.y.set(scatterguard_y) + final_status = ( + ap_status + & self.scatterguard.x.set(scatterguard_x) + & self.scatterguard.y.set(scatterguard_y) + ) + return final_status diff --git a/src/artemis/devices/system_tests/test_aperturescatterguard_system.py b/src/artemis/devices/system_tests/test_aperturescatterguard_system.py new file mode 100644 index 000000000..478a0bfb6 --- /dev/null +++ b/src/artemis/devices/system_tests/test_aperturescatterguard_system.py @@ -0,0 +1,149 @@ +import bluesky.plan_stubs as bps +import pytest +from bluesky.callbacks import CallbackBase +from bluesky.run_engine import RunEngine + +from artemis.devices.aperturescatterguard import ( + AperturePositions, + ApertureScatterguard, + InvalidApertureMove, +) +from artemis.parameters import I03_BEAMLINE_PARAMETER_PATH, GDABeamlineParameters + + +@pytest.fixture +def ap_sg(): + ap_sg = ApertureScatterguard(prefix="BL03S", name="ap_sg") + ap_sg.load_aperture_positions( + AperturePositions.from_gda_beamline_params( + GDABeamlineParameters.from_file(I03_BEAMLINE_PARAMETER_PATH) + ) + ) + return ap_sg + + +@pytest.fixture +def move_to_large(ap_sg: ApertureScatterguard): + yield from bps.abs_set(ap_sg, ap_sg.aperture_positions.LARGE) + + +@pytest.fixture +def move_to_medium(ap_sg: ApertureScatterguard): + yield from bps.abs_set(ap_sg, ap_sg.aperture_positions.MEDIUM) + + +@pytest.fixture +def move_to_small(ap_sg: ApertureScatterguard): + yield from bps.abs_set(ap_sg, ap_sg.aperture_positions.SMALL) + + +@pytest.fixture +def move_to_robotload(ap_sg: ApertureScatterguard): + yield from bps.abs_set(ap_sg, ap_sg.aperture_positions.ROBOT_LOAD) + + +@pytest.mark.s03 +def test_aperturescatterguard_setup(ap_sg: ApertureScatterguard): + ap_sg.wait_for_connection() + assert ap_sg.aperture_positions is not None + + +@pytest.mark.s03 +def test_aperturescatterguard_move_in_plan( + ap_sg: ApertureScatterguard, + move_to_large, + move_to_medium, + move_to_small, + move_to_robotload, +): + RE = RunEngine({}) + ap_sg.wait_for_connection() + + ap_sg.aperture.z.set(ap_sg.aperture_positions.LARGE[2], wait=True) + + RE(move_to_large) + RE(move_to_medium) + RE(move_to_small) + RE(move_to_robotload) + + +@pytest.mark.s03 +def test_move_fails_when_not_in_good_starting_pos( + ap_sg: ApertureScatterguard, move_to_large +): + RE = RunEngine({}) + ap_sg.wait_for_connection() + + ap_sg.aperture.z.set(0, wait=True) + + with pytest.raises(InvalidApertureMove): + RE(move_to_large) + + +class MonitorCallback(CallbackBase): + # holds on to the most recent time a motor move completed for aperture and + # scatterguard y + + t_ap_y: float = 0 + t_sg_y: float = 0 + event_docs: list[dict] = [] + + def event(self, doc): + self.event_docs.append(doc) + if doc["data"].get("ap_sg_aperture_y_motor_done_move") == 1: + self.t_ap_y = doc["timestamps"].get("ap_sg_aperture_y_motor_done_move") + if doc["data"].get("ap_sg_scatterguard_y_motor_done_move") == 1: + self.t_sg_y = doc["timestamps"].get("ap_sg_scatterguard_y_motor_done_move") + + +@pytest.mark.s03 +@pytest.mark.parametrize( + "pos1,pos2,sg_first", + [ + ("L", "M", True), + ("L", "S", True), + ("L", "R", False), + ("M", "L", False), + ("M", "S", True), + ("M", "R", False), + ("S", "L", False), + ("S", "M", False), + ("S", "R", False), + ("R", "L", True), + ("R", "M", True), + ("R", "S", True), + ], +) +def test_aperturescatterguard_moves_in_correct_order( + pos1, pos2, sg_first, ap_sg: ApertureScatterguard +): + cb = MonitorCallback() + positions = { + "L": ap_sg.aperture_positions.LARGE, + "M": ap_sg.aperture_positions.MEDIUM, + "S": ap_sg.aperture_positions.SMALL, + "R": ap_sg.aperture_positions.ROBOT_LOAD, + } + pos1 = positions[pos1] + pos2 = positions[pos2] + RE = RunEngine({}) + RE.subscribe(cb) + + ap_sg.wait_for_connection() + ap_sg.aperture.y.set(0, wait=True) + ap_sg.scatterguard.y.set(0, wait=True) + ap_sg.aperture.z.set(pos1[2], wait=True) + + def monitor_and_moves(): + yield from bps.open_run() + yield from bps.monitor(ap_sg.aperture.y.motor_done_move, name="ap_y") + yield from bps.monitor(ap_sg.scatterguard.y.motor_done_move, name="sg_y") + yield from bps.mv(ap_sg, pos1) + yield from bps.sleep(0.05) + yield from bps.mv(ap_sg, pos2) + yield from bps.sleep(0.05) + yield from bps.close_run() + + RE(monitor_and_moves()) + + assert (cb.t_sg_y < cb.t_ap_y) == sg_first diff --git a/src/artemis/devices/unit_tests/test_aperture.py b/src/artemis/devices/unit_tests/test_aperture.py deleted file mode 100644 index 512df9f99..000000000 --- a/src/artemis/devices/unit_tests/test_aperture.py +++ /dev/null @@ -1,15 +0,0 @@ -import pytest -from ophyd.sim import make_fake_device - -from artemis.devices.aperture import Aperture - - -@pytest.fixture -def fake_aperture(): - FakeAperture = make_fake_device(Aperture) - fake_aperture: Aperture = FakeAperture(name="aperture") - return fake_aperture - - -def test_aperture_can_be_created(fake_aperture: Aperture): - fake_aperture.wait_for_connection() diff --git a/src/artemis/experiment_plans/fast_grid_scan_plan.py b/src/artemis/experiment_plans/fast_grid_scan_plan.py index 90cd3bad4..ce1acf79f 100644 --- a/src/artemis/experiment_plans/fast_grid_scan_plan.py +++ b/src/artemis/experiment_plans/fast_grid_scan_plan.py @@ -81,7 +81,7 @@ def set_aperture_for_bbox_size( artemis.log.LOGGER.info( f"Setting aperture to {aperture_size_positions} based on bounding box size {bbox_size}." ) - aperture_device.safe_move_within_datacollection_range(*aperture_size_positions) + yield from bps.abs_set(aperture_device, aperture_size_positions) def read_hardware_for_ispyb( @@ -225,7 +225,9 @@ def gridscan_with_subscriptions(fgs_composite, detector, params): if bbox_size is not None: with TRACER.start_span("change_aperture"): - set_aperture_for_bbox_size(fgs_composite.aperture_scatterguard, bbox_size) + yield from set_aperture_for_bbox_size( + fgs_composite.aperture_scatterguard, bbox_size + ) # once we have the results, go to the appropriate position artemis.log.LOGGER.info("Moving to centre of mass.") diff --git a/src/artemis/external_interaction/__init__.py b/src/artemis/external_interaction/__init__.py index 796f9e135..7dadb8bdb 100644 --- a/src/artemis/external_interaction/__init__.py +++ b/src/artemis/external_interaction/__init__.py @@ -7,11 +7,3 @@ execution of the experimental plan. It's not recommended to use the interaction classes here directly in plans except through the use of such callbacks. """ -from artemis.external_interaction.ispyb.store_in_ispyb import ( - StoreInIspyb2D, - StoreInIspyb3D, -) -from artemis.external_interaction.nexus.write_nexus import NexusWriter -from artemis.external_interaction.zocalo.zocalo_interaction import ZocaloInteractor - -__all__ = ["ZocaloInteractor", "NexusWriter", "StoreInIspyb2D", "StoreInIspyb3D"] diff --git a/src/artemis/parameters.py b/src/artemis/parameters.py index 728a60675..ba4d17831 100644 --- a/src/artemis/parameters.py +++ b/src/artemis/parameters.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import copy from dataclasses import dataclass, field from os import environ diff --git a/src/artemis/unit_tests/test_fast_grid_scan_plan.py b/src/artemis/unit_tests/test_fast_grid_scan_plan.py index 3407189ee..7fe902d19 100644 --- a/src/artemis/unit_tests/test_fast_grid_scan_plan.py +++ b/src/artemis/unit_tests/test_fast_grid_scan_plan.py @@ -158,7 +158,7 @@ def standalone_read_hardware_for_ispyb(und, syn, slits): @patch( - "artemis.devices.aperturescatterguard.ApertureScatterguard.safe_move_within_datacollection_range" + "artemis.devices.aperturescatterguard.ApertureScatterguard._safe_move_within_datacollection_range" ) @patch("artemis.experiment_plans.fast_grid_scan_plan.run_gridscan") @patch("artemis.experiment_plans.fast_grid_scan_plan.move_xyz") @@ -191,7 +191,7 @@ def test_results_adjusted_and_passed_to_move_xyz( @patch( - "artemis.devices.aperturescatterguard.ApertureScatterguard.safe_move_within_datacollection_range" + "artemis.devices.aperturescatterguard.ApertureScatterguard._safe_move_within_datacollection_range" ) @patch("artemis.experiment_plans.fast_grid_scan_plan.run_gridscan") @patch("artemis.experiment_plans.fast_grid_scan_plan.move_xyz") @@ -251,7 +251,7 @@ def test_results_passed_to_move_aperture( ) move_aperture.assert_has_calls( - [call_large, call_medium, call_small], any_order=False + [call_large, call_medium, call_small], any_order=True ) @@ -274,7 +274,7 @@ def test_results_passed_to_move_motors(bps_mv: MagicMock, test_params: FullParam @patch( - "artemis.devices.aperturescatterguard.ApertureScatterguard.safe_move_within_datacollection_range" + "artemis.devices.aperturescatterguard.ApertureScatterguard._safe_move_within_datacollection_range" ) @patch("artemis.experiment_plans.fast_grid_scan_plan.run_gridscan.do_fgs") @patch("artemis.experiment_plans.fast_grid_scan_plan.run_gridscan") @@ -308,7 +308,7 @@ def test_individual_plans_triggered_once_and_only_once_in_composite_run( @patch( - "artemis.devices.aperturescatterguard.ApertureScatterguard.safe_move_within_datacollection_range" + "artemis.devices.aperturescatterguard.ApertureScatterguard._safe_move_within_datacollection_range" ) @patch("artemis.experiment_plans.fast_grid_scan_plan.run_gridscan.do_fgs") @patch("artemis.experiment_plans.fast_grid_scan_plan.run_gridscan")