From f7b5b8ce288c2753bbf857e73a695f57883ce726 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 20 Aug 2024 17:40:58 +0100 Subject: [PATCH] Fix to work with updated aperture scatterguard (#1541) * Fix to work with updated aperture scatterguard * Don't use private variable on aperture_scatterguard * Unpin mx-bluesky --- setup.cfg | 2 +- .../device_setup_plans/manipulate_sample.py | 3 +- .../flyscan_xray_centre_plan.py | 22 ++--- .../grid_detect_then_xray_centre_plan.py | 11 --- .../experiment_plans/oav_snapshot_plan.py | 5 +- .../robot_load_then_centre_plan.py | 5 +- .../experiment_plans/rotation_scan_plan.py | 11 --- src/hyperion/utils/aperturescatterguard.py | 16 ---- tests/conftest.py | 83 +++++++++---------- .../experiment_plans/test_fgs_plan.py | 23 ++--- .../test_aperturescatterguard_system.py | 20 +++-- .../test_manipulate_sample.py | 5 +- .../test_flyscan_xray_centre_plan.py | 23 +++-- .../test_grid_detect_then_xray_centre_plan.py | 9 +- .../test_rotation_scan_plan.py | 12 +-- .../test_wait_for_robot_load_then_centre.py | 4 +- .../callbacks/test_rotation_callbacks.py | 4 - .../utils/test_aperturescatterguard_utils.py | 32 ------- 18 files changed, 94 insertions(+), 196 deletions(-) delete mode 100644 src/hyperion/utils/aperturescatterguard.py delete mode 100644 tests/unit_tests/utils/test_aperturescatterguard_utils.py diff --git a/setup.cfg b/setup.cfg index 58dfd5351..6c0add12b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -40,7 +40,7 @@ install_requires = ophyd-async >= 0.3a5 bluesky >= 1.13.0a4 blueapi >= 0.4.3-rc1 - mx-bluesky @ git+https://github.com/DiamondLightSource/mx-bluesky.git@0492e92f75eefa8885be59dde3eff323c1ea81c2 + mx-bluesky @ git+https://github.com/DiamondLightSource/mx-bluesky.git dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git [options.entry_points] diff --git a/src/hyperion/device_setup_plans/manipulate_sample.py b/src/hyperion/device_setup_plans/manipulate_sample.py index ae2af0503..ee942b17a 100644 --- a/src/hyperion/device_setup_plans/manipulate_sample.py +++ b/src/hyperion/device_setup_plans/manipulate_sample.py @@ -55,8 +55,7 @@ def move_aperture_if_required( ) else: - assert aperture_scatterguard.aperture_positions - aperture_position = aperture_scatterguard.aperture_positions.get_position_from_gda_aperture_name( + aperture_position = aperture_scatterguard.get_position_from_gda_aperture_name( aperture_position_gda_name ) LOGGER.info(f"Setting aperture position to {aperture_position}") diff --git a/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py b/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py index 547847143..92c071471 100755 --- a/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py +++ b/src/hyperion/experiment_plans/flyscan_xray_centre_plan.py @@ -12,8 +12,8 @@ from attr import dataclass from blueapi.core import BlueskyContext, MsgGenerator from dodal.devices.aperturescatterguard import ( + AperturePosition, ApertureScatterguard, - SingleAperturePosition, ) from dodal.devices.attenuator import Attenuator from dodal.devices.backlight import Backlight @@ -72,9 +72,6 @@ from hyperion.parameters.constants import CONST from hyperion.parameters.gridscan import ThreeDGridScan from hyperion.tracing import TRACER -from hyperion.utils.aperturescatterguard import ( - load_default_aperture_scatterguard_positions_if_unset, -) from hyperion.utils.context import device_composite_from_context @@ -109,12 +106,6 @@ def sample_motors(self) -> Smargon: """Convenience alias with a more user-friendly name""" return self.smargon - def __post_init__(self): - """Ensure that aperture positions are loaded whenever this class is created.""" - load_default_aperture_scatterguard_positions_if_unset( - self.aperture_scatterguard - ) - def create_devices(context: BlueskyContext) -> FlyScanXRayCentreComposite: """Creates the devices required for the plan and connect to them""" @@ -376,13 +367,10 @@ def set_aperture_for_bbox_size( bbox_size: list[int] | np.ndarray, ): # bbox_size is [x,y,z], for i03 we only care about x - assert aperture_device.aperture_positions is not None - - new_selected_aperture: SingleAperturePosition = ( - aperture_device.aperture_positions.MEDIUM - if bbox_size[0] < 2 - else aperture_device.aperture_positions.LARGE + new_selected_aperture = ( + AperturePosition.MEDIUM if bbox_size[0] < 2 else AperturePosition.LARGE ) + gda_name = aperture_device.get_gda_name_for_position(new_selected_aperture) LOGGER.info( f"Setting aperture to {new_selected_aperture} based on bounding box size {bbox_size}." ) @@ -391,7 +379,7 @@ def set_aperture_for_bbox_size( @bpp.run_decorator( md={ "subplan_name": "change_aperture", - "aperture_size": new_selected_aperture.GDA_name, + "aperture_size": gda_name, } ) def set_aperture(): diff --git a/src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py b/src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py index 0ed219f3e..89940b41b 100644 --- a/src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py +++ b/src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py @@ -51,9 +51,6 @@ from hyperion.log import LOGGER from hyperion.parameters.constants import CONST from hyperion.parameters.gridscan import GridScanWithEdgeDetect, ThreeDGridScan -from hyperion.utils.aperturescatterguard import ( - load_default_aperture_scatterguard_positions_if_unset, -) from hyperion.utils.context import device_composite_from_context @@ -82,12 +79,6 @@ class GridDetectThenXRayCentreComposite: panda_fast_grid_scan: PandAFastGridScan robot: BartRobot - def __post_init__(self): - """Ensure that aperture positions are loaded whenever this class is created.""" - load_default_aperture_scatterguard_positions_if_unset( - self.aperture_scatterguard - ) - def create_devices(context: BlueskyContext) -> GridDetectThenXRayCentreComposite: return device_composite_from_context(context, GridDetectThenXRayCentreComposite) @@ -109,8 +100,6 @@ def detect_grid_and_do_gridscan( parameters: GridScanWithEdgeDetect, oav_params: OAVParameters, ): - assert composite.aperture_scatterguard.aperture_positions is not None - snapshot_template = f"{parameters.detector_params.prefix}_{parameters.detector_params.run_number}_{{angle}}" grid_params_callback = GridDetectionCallback(composite.oav.parameters) diff --git a/src/hyperion/experiment_plans/oav_snapshot_plan.py b/src/hyperion/experiment_plans/oav_snapshot_plan.py index 9228627be..1e3fdaea6 100644 --- a/src/hyperion/experiment_plans/oav_snapshot_plan.py +++ b/src/hyperion/experiment_plans/oav_snapshot_plan.py @@ -3,7 +3,7 @@ from blueapi.core import MsgGenerator from bluesky import plan_stubs as bps -from dodal.devices.aperturescatterguard import ApertureScatterguard +from dodal.devices.aperturescatterguard import AperturePosition, ApertureScatterguard from dodal.devices.backlight import Backlight, BacklightPosition from dodal.devices.oav.oav_detector import OAV from dodal.devices.oav.oav_parameters import OAVParameters @@ -39,10 +39,9 @@ def setup_oav_snapshot_plan( yield from bps.abs_set( composite.backlight, BacklightPosition.IN, group=OAV_SNAPSHOT_SETUP_GROUP ) - assert composite.aperture_scatterguard.aperture_positions is not None yield from bps.abs_set( composite.aperture_scatterguard, - composite.aperture_scatterguard.aperture_positions.ROBOT_LOAD, + AperturePosition.ROBOT_LOAD, group=OAV_SNAPSHOT_SETUP_GROUP, ) diff --git a/src/hyperion/experiment_plans/robot_load_then_centre_plan.py b/src/hyperion/experiment_plans/robot_load_then_centre_plan.py index c56b6dd77..0bb840145 100644 --- a/src/hyperion/experiment_plans/robot_load_then_centre_plan.py +++ b/src/hyperion/experiment_plans/robot_load_then_centre_plan.py @@ -8,7 +8,7 @@ import bluesky.plan_stubs as bps import bluesky.preprocessors as bpp from blueapi.core import BlueskyContext, MsgGenerator -from dodal.devices.aperturescatterguard import ApertureScatterguard +from dodal.devices.aperturescatterguard import AperturePosition, ApertureScatterguard from dodal.devices.attenuator import Attenuator from dodal.devices.backlight import Backlight from dodal.devices.dcm import DCM @@ -128,10 +128,9 @@ def take_robot_snapshots(oav: OAV, webcam: Webcam, directory: Path): def prepare_for_robot_load(composite: RobotLoadThenCentreComposite): - assert composite.aperture_scatterguard.aperture_positions yield from bps.abs_set( composite.aperture_scatterguard, - composite.aperture_scatterguard.aperture_positions.ROBOT_LOAD, + AperturePosition.ROBOT_LOAD, group="prepare_robot_load", ) diff --git a/src/hyperion/experiment_plans/rotation_scan_plan.py b/src/hyperion/experiment_plans/rotation_scan_plan.py index a8937bb92..292bc2a8c 100644 --- a/src/hyperion/experiment_plans/rotation_scan_plan.py +++ b/src/hyperion/experiment_plans/rotation_scan_plan.py @@ -51,9 +51,6 @@ MultiRotationScan, RotationScan, ) -from hyperion.utils.aperturescatterguard import ( - load_default_aperture_scatterguard_positions_if_unset, -) from hyperion.utils.context import device_composite_from_context @@ -76,12 +73,6 @@ class RotationScanComposite(OavSnapshotComposite): zebra: Zebra oav: OAV - def __post_init__(self): - """Ensure that aperture positions are loaded whenever this class is created.""" - load_default_aperture_scatterguard_positions_if_unset( - self.aperture_scatterguard - ) - def create_devices(context: BlueskyContext) -> RotationScanComposite: """Ensures necessary devices have been instantiated""" @@ -364,7 +355,6 @@ def rotation_scan_plan_with_stage_and_cleanup( @bpp.stage_decorator([eiger]) @bpp.finalize_decorator(lambda: _cleanup_plan(composite)) def rotation_with_cleanup_and_stage(params: RotationScan): - assert composite.aperture_scatterguard.aperture_positions is not None LOGGER.info("setting up sample environment...") yield from begin_sample_environment_setup( composite.detector_motion, @@ -390,7 +380,6 @@ def multi_rotation_scan( oav_params = OAVParameters(context="xrayCentring") eiger: EigerDetector = composite.eiger eiger.set_detector_parameters(parameters.detector_params) - assert composite.aperture_scatterguard.aperture_positions is not None LOGGER.info("setting up sample environment...") yield from begin_sample_environment_setup( composite.detector_motion, diff --git a/src/hyperion/utils/aperturescatterguard.py b/src/hyperion/utils/aperturescatterguard.py deleted file mode 100644 index c4a657bbe..000000000 --- a/src/hyperion/utils/aperturescatterguard.py +++ /dev/null @@ -1,16 +0,0 @@ -from dodal.common.beamlines.beamline_parameters import get_beamline_parameters -from dodal.devices.aperturescatterguard import AperturePositions, ApertureScatterguard - - -def load_default_aperture_scatterguard_positions_if_unset( - aperture_scatterguard: ApertureScatterguard, -) -> None: - """ - If aperture scatterguard positions are `None`, load the default set of positions. - - If the positions are already set, do nothing. - """ - if aperture_scatterguard.aperture_positions is None: - params = get_beamline_parameters() - aperture_positions = AperturePositions.from_gda_beamline_params(params) - aperture_scatterguard.load_aperture_positions(aperture_positions) diff --git a/tests/conftest.py b/tests/conftest.py index afb7bc106..443197f41 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,7 +22,7 @@ from dodal.common.beamlines.beamline_utils import clear_devices from dodal.devices.aperturescatterguard import ( ApertureFiveDimensionalLocation, - AperturePositions, + AperturePosition, ApertureScatterguard, ApertureScatterguardTolerances, SingleAperturePosition, @@ -452,51 +452,50 @@ def thawer(RE) -> Generator[Thawer, Any, Any]: @pytest.fixture def aperture_scatterguard(RE): - AperturePositions.LARGE = SingleAperturePosition( - location=ApertureFiveDimensionalLocation(0, 1, 2, 3, 4), - name="Large", - GDA_name="LARGE_APERTURE", - radius_microns=100, - ) - AperturePositions.MEDIUM = SingleAperturePosition( - location=ApertureFiveDimensionalLocation(5, 6, 2, 8, 9), - name="Medium", - GDA_name="MEDIUM_APERTURE", - radius_microns=50, - ) - AperturePositions.SMALL = SingleAperturePosition( - location=ApertureFiveDimensionalLocation(10, 11, 2, 13, 14), - name="Small", - GDA_name="SMALL_APERTURE", - radius_microns=20, - ) - AperturePositions.ROBOT_LOAD = SingleAperturePosition( - location=ApertureFiveDimensionalLocation(15, 16, 2, 18, 19), - name="Robot_load", - GDA_name="ROBOT_LOAD", - radius_microns=None, - ) - ap_sg = i03.aperture_scatterguard( - fake_with_ophyd_sim=True, - aperture_positions=AperturePositions( - AperturePositions.LARGE, - AperturePositions.MEDIUM, - AperturePositions.SMALL, - AperturePositions.ROBOT_LOAD, - tolerances=ApertureScatterguardTolerances(0.1, 0.1, 0.1, 0.1, 0.1), + positions = { + AperturePosition.LARGE: SingleAperturePosition( + location=ApertureFiveDimensionalLocation(0, 1, 2, 3, 4), + name="Large", + GDA_name="LARGE_APERTURE", + radius_microns=100, ), - ) + AperturePosition.MEDIUM: SingleAperturePosition( + location=ApertureFiveDimensionalLocation(5, 6, 2, 8, 9), + name="Medium", + GDA_name="MEDIUM_APERTURE", + radius_microns=50, + ), + AperturePosition.SMALL: SingleAperturePosition( + location=ApertureFiveDimensionalLocation(10, 11, 2, 13, 14), + name="Small", + GDA_name="SMALL_APERTURE", + radius_microns=20, + ), + AperturePosition.ROBOT_LOAD: SingleAperturePosition( + location=ApertureFiveDimensionalLocation(15, 16, 2, 18, 19), + name="Robot_load", + GDA_name="ROBOT_LOAD", + radius_microns=None, + ), + } + with patch( + "dodal.beamlines.i03.load_positions_from_beamline_parameters", + return_value=positions, + ), patch( + "dodal.beamlines.i03.load_tolerances_from_beamline_params", + return_value=ApertureScatterguardTolerances(0.1, 0.1, 0.1, 0.1, 0.1), + ): + ap_sg = i03.aperture_scatterguard(fake_with_ophyd_sim=True) with ( - patch_async_motor(ap_sg.aperture.x), - patch_async_motor(ap_sg.aperture.y), - patch_async_motor(ap_sg.aperture.z, 2), - patch_async_motor(ap_sg.scatterguard.x), - patch_async_motor(ap_sg.scatterguard.y), + patch_async_motor(ap_sg._aperture.x), + patch_async_motor(ap_sg._aperture.y), + patch_async_motor(ap_sg._aperture.z, 2), + patch_async_motor(ap_sg._scatterguard.x), + patch_async_motor(ap_sg._scatterguard.y), ): - assert ap_sg.aperture_positions - RE(bps.abs_set(ap_sg, ap_sg.aperture_positions.SMALL)) + RE(bps.abs_set(ap_sg, AperturePosition.SMALL)) - set_mock_value(ap_sg.aperture.small, 1) + set_mock_value(ap_sg._aperture.small, 1) yield ap_sg diff --git a/tests/system_tests/experiment_plans/test_fgs_plan.py b/tests/system_tests/experiment_plans/test_fgs_plan.py index cafdc7f40..cc317c7be 100644 --- a/tests/system_tests/experiment_plans/test_fgs_plan.py +++ b/tests/system_tests/experiment_plans/test_fgs_plan.py @@ -8,11 +8,7 @@ import pytest_asyncio from bluesky.run_engine import RunEngine from dodal.beamlines import i03 -from dodal.common.beamlines.beamline_parameters import ( - BEAMLINE_PARAMETER_PATHS, - GDABeamlineParameters, -) -from dodal.devices.aperturescatterguard import AperturePositions +from dodal.devices.aperturescatterguard import AperturePosition from dodal.devices.smargon import Smargon from ophyd.sim import NullStatus from ophyd_async.core import set_mock_value @@ -103,17 +99,8 @@ async def fxc_composite(): await composite.robot.barcode._backend.put("ABCDEFGHIJ") # type: ignore composite.dcm.energy_in_kev.user_readback.sim_put(12.345) # type: ignore - gda_beamline_parameters = GDABeamlineParameters.from_file( - BEAMLINE_PARAMETER_PATHS["i03"] - ) - - aperture_positions = AperturePositions.from_gda_beamline_params( - gda_beamline_parameters - ) - composite.aperture_scatterguard.load_aperture_positions(aperture_positions) - await composite.aperture_scatterguard._set_raw_unsafe( - aperture_positions.LARGE.location - ) + large = composite.aperture_scatterguard._loaded_positions[AperturePosition.LARGE] + await composite.aperture_scatterguard._set_raw_unsafe(large.location) composite.eiger.cam.manual_trigger.put("Yes") composite.eiger.odin.check_odin_initialised = lambda: (True, "") composite.eiger.stage = MagicMock(return_value=NullStatus()) @@ -141,7 +128,9 @@ def read_run(u, s, g, r, a, f, dcm, ap_sg, sm): yield from read_hardware_pre_collection( undulator=u, synchrotron=s, s4_slit_gaps=g, robot=r, smargon=sm ) - yield from read_hardware_during_collection(ap_sg, a, f, dcm) + yield from read_hardware_during_collection( + ap_sg, a, f, dcm, fxc_composite.eiger + ) RE( read_run( diff --git a/tests/system_tests/hyperion/test_aperturescatterguard_system.py b/tests/system_tests/hyperion/test_aperturescatterguard_system.py index 1f813fb7b..acd425fa5 100644 --- a/tests/system_tests/hyperion/test_aperturescatterguard_system.py +++ b/tests/system_tests/hyperion/test_aperturescatterguard_system.py @@ -3,17 +3,24 @@ BEAMLINE_PARAMETER_PATHS, GDABeamlineParameters, ) -from dodal.devices.aperturescatterguard import AperturePositions, ApertureScatterguard +from dodal.devices.aperturescatterguard import ( + ApertureScatterguard, + load_positions_from_beamline_parameters, + load_tolerances_from_beamline_params, +) +from ophyd_async.core import DeviceCollector @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(BEAMLINE_PARAMETER_PATHS["i03"]) + params = GDABeamlineParameters.from_file(BEAMLINE_PARAMETER_PATHS["i03"]) + with DeviceCollector(): + ap_sg = ApertureScatterguard( + prefix="BL03S", + name="ap_sg", + loaded_positions=load_positions_from_beamline_parameters(params), + tolerances=load_tolerances_from_beamline_params(params), ) - ) return ap_sg @@ -28,7 +35,6 @@ def test_aperture_change_callback(ap_sg: ApertureScatterguard): ApertureChangeCallback, ) - ap_sg.wait_for_connection() cb = ApertureChangeCallback() RE = RunEngine({}) RE.subscribe(cb) diff --git a/tests/unit_tests/device_setup_plans/test_manipulate_sample.py b/tests/unit_tests/device_setup_plans/test_manipulate_sample.py index 80b22d2ce..91ebb958e 100644 --- a/tests/unit_tests/device_setup_plans/test_manipulate_sample.py +++ b/tests/unit_tests/device_setup_plans/test_manipulate_sample.py @@ -2,6 +2,7 @@ from bluesky.run_engine import RunEngine from dodal.devices.aperturescatterguard import ( + AperturePosition, AperturePositionGDANames, ApertureScatterguard, ) @@ -12,7 +13,6 @@ async def test_move_aperture_goes_to_correct_position( aperture_scatterguard: ApertureScatterguard, RE: RunEngine ): - assert aperture_scatterguard.aperture_positions with patch.object(aperture_scatterguard, "set") as mock_set: RE( move_aperture_if_required( @@ -20,14 +20,13 @@ async def test_move_aperture_goes_to_correct_position( ) ) mock_set.assert_called_once_with( - aperture_scatterguard.aperture_positions.LARGE, + AperturePosition.LARGE, ) async def test_move_aperture_does_nothing_when_none_selected( aperture_scatterguard: ApertureScatterguard, RE: RunEngine ): - assert aperture_scatterguard.aperture_positions with patch.object(aperture_scatterguard, "set") as mock_set: RE(move_aperture_if_required(aperture_scatterguard, None)) mock_set.assert_not_called() diff --git a/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py b/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py index d62f2b8c0..d87a5c058 100644 --- a/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py +++ b/tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py @@ -13,6 +13,7 @@ from bluesky.utils import FailedStatus, Msg from dodal.beamlines import i03 from dodal.common.beamlines.beamline_utils import clear_device +from dodal.devices.aperturescatterguard import AperturePosition from dodal.devices.detector.det_dim_constants import ( EIGER_TYPE_EIGER2_X_16M, ) @@ -64,7 +65,10 @@ from hyperion.log import ISPYB_LOGGER from hyperion.parameters.constants import CONST from hyperion.parameters.gridscan import ThreeDGridScan -from tests.conftest import RunEngineSimulator, create_dummy_scan_spec +from tests.conftest import ( + RunEngineSimulator, + create_dummy_scan_spec, +) from ...system_tests.external_interaction.conftest import ( TEST_RESULT_LARGE, @@ -244,7 +248,7 @@ def test_read_hardware_for_ispyb_updates_from_ophyd_devices( RE( bps.abs_set( fake_fgs_composite.aperture_scatterguard, - fake_fgs_composite.aperture_scatterguard.aperture_positions.LARGE, # type: ignore + AperturePosition.LARGE, ) ) @@ -353,16 +357,11 @@ def test_results_adjusted_and_passed_to_move_xyz( ) ) - assert ( - fgs_composite_with_panda_pcap.aperture_scatterguard.aperture_positions - is not None - ) - ap_call_large = call( - fgs_composite_with_panda_pcap.aperture_scatterguard.aperture_positions.LARGE.location - ) - ap_call_medium = call( - fgs_composite_with_panda_pcap.aperture_scatterguard.aperture_positions.MEDIUM.location - ) + aperture_scatterguard = fgs_composite_with_panda_pcap.aperture_scatterguard + large = aperture_scatterguard._loaded_positions[AperturePosition.LARGE] + medium = aperture_scatterguard._loaded_positions[AperturePosition.MEDIUM] + ap_call_large = call(large.location) + ap_call_medium = call(medium.location) move_aperture.assert_has_calls( [ap_call_large, ap_call_large, ap_call_medium], any_order=True diff --git a/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py b/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py index 765ce299e..8f7810c64 100644 --- a/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py +++ b/tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py @@ -7,6 +7,7 @@ from bluesky.run_engine import RunEngine from bluesky.simulators import RunEngineSimulator, assert_message_and_return_remaining from dodal.beamlines import i03 +from dodal.devices.aperturescatterguard import AperturePosition from dodal.devices.backlight import BacklightPosition from dodal.devices.eiger import EigerDetector from dodal.devices.oav.oav_detector import OAVConfigParams @@ -121,10 +122,6 @@ async def test_detect_grid_and_do_gridscan( test_config_files: Dict, ): mock_grid_detection_plan.side_effect = _fake_grid_detection - assert ( - grid_detect_devices_with_oav_config_params.aperture_scatterguard.aperture_positions - is not None - ) with patch.object( grid_detect_devices_with_oav_config_params.aperture_scatterguard, @@ -153,9 +150,7 @@ async def test_detect_grid_and_do_gridscan( ) # Check aperture was changed to SMALL - mock_aperture_scatterguard.assert_called_once_with( - grid_detect_devices_with_oav_config_params.aperture_scatterguard.aperture_positions.SMALL - ) + mock_aperture_scatterguard.assert_called_once_with(AperturePosition.SMALL) # Check we called out to underlying fast grid scan plan mock_flyscan_xray_centre_plan.assert_called_once_with(ANY, ANY) diff --git a/tests/unit_tests/experiment_plans/test_rotation_scan_plan.py b/tests/unit_tests/experiment_plans/test_rotation_scan_plan.py index ab0b2a901..f832733d0 100644 --- a/tests/unit_tests/experiment_plans/test_rotation_scan_plan.py +++ b/tests/unit_tests/experiment_plans/test_rotation_scan_plan.py @@ -7,7 +7,7 @@ import pytest from bluesky.run_engine import RunEngine from bluesky.simulators import RunEngineSimulator, assert_message_and_return_remaining -from dodal.devices.aperturescatterguard import AperturePositions, ApertureScatterguard +from dodal.devices.aperturescatterguard import AperturePosition, ApertureScatterguard from dodal.devices.backlight import BacklightPosition from dodal.devices.oav.oav_parameters import OAVParameters from dodal.devices.smargon import Smargon @@ -208,7 +208,8 @@ async def test_full_rotation_plan_smargon_settings( assert await smargon.z.user_readback.get_value() == params.z_start_um / 1000 # type: ignore assert ( # 4 * snapshots, restore omega, 1 * rotation sweep - omega_set.call_count == 4 + 1 + 1 + omega_set.call_count + == 4 + 1 + 1 ) # 1 to max vel in outer plan, 1 to max vel in setup_oav_snapshot_plan, 1 set before rotation, 1 restore in cleanup plan assert omega_velocity_set.call_count == 4 @@ -227,10 +228,9 @@ async def test_rotation_plan_moves_aperture_correctly( aperture_scatterguard: ApertureScatterguard = ( run_full_rotation_plan.aperture_scatterguard ) - assert aperture_scatterguard.aperture_positions assert ( await aperture_scatterguard.get_current_aperture_position() - == aperture_scatterguard.aperture_positions.SMALL + == aperture_scatterguard._loaded_positions[AperturePosition.SMALL] ) @@ -422,7 +422,7 @@ def test_rotation_scan_moves_aperture_in_backlight_out_after_snapshots_before_ro msgs, lambda msg: msg.command == "set" and msg.obj.name == "aperture_scatterguard" - and msg.args[0] == AperturePositions.SMALL + and msg.args[0] == AperturePosition.SMALL and msg.kwargs["group"] == "setup_senv", ) msgs = assert_message_and_return_remaining( @@ -509,7 +509,7 @@ def test_rotation_snapshot_setup_called_to_move_backlight_in_aperture_out_before msgs, lambda msg: msg.command == "set" and msg.obj.name == "aperture_scatterguard" - and msg.args[0] == AperturePositions.ROBOT_LOAD + and msg.args[0] == AperturePosition.ROBOT_LOAD and msg.kwargs["group"] == OAV_SNAPSHOT_SETUP_GROUP, ) msgs = assert_message_and_return_remaining( diff --git a/tests/unit_tests/experiment_plans/test_wait_for_robot_load_then_centre.py b/tests/unit_tests/experiment_plans/test_wait_for_robot_load_then_centre.py index 66915a111..0ab146979 100644 --- a/tests/unit_tests/experiment_plans/test_wait_for_robot_load_then_centre.py +++ b/tests/unit_tests/experiment_plans/test_wait_for_robot_load_then_centre.py @@ -6,7 +6,7 @@ from bluesky.run_engine import RunEngine from bluesky.simulators import RunEngineSimulator, assert_message_and_return_remaining from bluesky.utils import Msg -from dodal.devices.aperturescatterguard import AperturePositions +from dodal.devices.aperturescatterguard import AperturePosition from dodal.devices.oav.oav_detector import OAV from dodal.devices.smargon import StubPosition from dodal.devices.webcam import Webcam @@ -286,7 +286,7 @@ async def test_when_prepare_for_robot_load_called_then_moves_as_expected( assert await smargon.omega.user_readback.get_value() == 0 smargon.stub_offsets.set.assert_called_once_with(StubPosition.RESET_TO_ROBOT_LOAD) # type: ignore - aperture_scatterguard.set.assert_called_once_with(AperturePositions.ROBOT_LOAD) # type: ignore + aperture_scatterguard.set.assert_called_once_with(AperturePosition.ROBOT_LOAD) # type: ignore @patch( diff --git a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py index d3f3b057b..196cb0791 100644 --- a/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py +++ b/tests/unit_tests/external_interaction/callbacks/test_rotation_callbacks.py @@ -41,9 +41,6 @@ ) from hyperion.parameters.constants import CONST from hyperion.parameters.rotation import RotationScan -from hyperion.utils.aperturescatterguard import ( - load_default_aperture_scatterguard_positions_if_unset, -) from ....conftest import raw_params_from_file @@ -93,7 +90,6 @@ def fake_rotation_scan( eiger = make_fake_device(EigerDetector)(name="eiger") dcm = i03.dcm(fake_with_ophyd_sim=True) ap_sg = i03.aperture_scatterguard(fake_with_ophyd_sim=True) - load_default_aperture_scatterguard_positions_if_unset(ap_sg) set_mock_value(dcm.energy_in_kev.user_readback, 12.1) @bpp.subs_decorator(list(subscriptions)) diff --git a/tests/unit_tests/utils/test_aperturescatterguard_utils.py b/tests/unit_tests/utils/test_aperturescatterguard_utils.py deleted file mode 100644 index 0a050162c..000000000 --- a/tests/unit_tests/utils/test_aperturescatterguard_utils.py +++ /dev/null @@ -1,32 +0,0 @@ -from unittest.mock import MagicMock, patch - -from hyperion.utils.aperturescatterguard import ( - ApertureScatterguard, - load_default_aperture_scatterguard_positions_if_unset, -) - - -@patch("hyperion.utils.aperturescatterguard.AperturePositions.from_gda_beamline_params") -def test_if_aperture_scatterguard_positions_unset_then_defaults_loaded( - from_gda_beamline_params, -): - asg = MagicMock(spec=ApertureScatterguard) - asg.aperture_positions = None - - load_default_aperture_scatterguard_positions_if_unset(asg) - - asg.load_aperture_positions.assert_called_once_with( - from_gda_beamline_params.return_value - ) - - -@patch("hyperion.utils.aperturescatterguard.AperturePositions.from_gda_beamline_params") -def test_if_aperture_scatterguard_positions_unset_then_nothing_loaded( - from_gda_beamline_params, -): - asg = MagicMock(spec=ApertureScatterguard) - asg.aperture_positions = object() - - load_default_aperture_scatterguard_positions_if_unset(asg) - - asg.load_aperture_positions.assert_not_called()