Set up centring correctly

src/mx_bluesky/hyperion/device_setup_plans/setup_oav.py

@@ -7,9 +7,10 @@
 from dodal.devices.oav.pin_image_recognition import PinTipDetection
 from dodal.devices.oav.utils import ColorMode
 
-oav_group = "oav_setup"
+from mx_bluesky.hyperion.parameters.constants import CONST
+
 # Helper function to make sure we set the waiting groups correctly
-set_using_group = partial(bps.abs_set, group=oav_group)
+set_using_group = partial(bps.abs_set, group=CONST.WAIT.READY_FOR_OAV)
 
 
 def setup_pin_tip_detection_params(
@@ -83,8 +84,4 @@ def pre_centring_setup_oav(
     """
     yield from setup_general_oav_params(oav, parameters)
     yield from setup_pin_tip_detection_params(pin_tip_detection_device, parameters)
-    yield from bps.wait(oav_group)
-
-    """
-    TODO: We require setting the backlight brightness to that in the json, we can't do this currently without a PV.
-    """
+    yield from bps.wait(CONST.WAIT.READY_FOR_OAV)

src/mx_bluesky/hyperion/experiment_plans/oav_snapshot_plan.py

@@ -11,9 +11,8 @@
 
 from mx_bluesky.hyperion.device_setup_plans.setup_oav import setup_general_oav_params
 from mx_bluesky.hyperion.parameters.components import WithSnapshot
-from mx_bluesky.hyperion.parameters.constants import DocDescriptorNames
+from mx_bluesky.hyperion.parameters.constants import CONST, DocDescriptorNames
 
-OAV_SNAPSHOT_SETUP_GROUP = "oav_snapshot_setup"
 OAV_SNAPSHOT_SETUP_SHOT = "oav_snapshot_setup_shot"
 OAV_SNAPSHOT_GROUP = "oav_snapshot_group"
 
@@ -25,24 +24,19 @@ class OavSnapshotComposite(Protocol):
     backlight: Backlight
 
 
-def setup_oav_snapshot_plan(
-    composite: OavSnapshotComposite,
-    parameters: WithSnapshot,
-    max_omega_velocity_deg_s: float,
+def setup_beamline_for_OAV(
+    smargon: Smargon,
+    backlight: Backlight,
+    aperture_scatterguard: ApertureScatterguard,
+    group=CONST.WAIT.READY_FOR_OAV,
 ):
-    if not parameters.take_snapshots:
-        return
-
-    yield from bps.abs_set(
-        composite.smargon.omega.velocity, max_omega_velocity_deg_s, wait=True
-    )
-    yield from bps.abs_set(
-        composite.backlight, BacklightPosition.IN, group=OAV_SNAPSHOT_SETUP_GROUP
-    )
+    max_vel = yield from bps.rd(smargon.omega.max_velocity)
+    yield from bps.abs_set(smargon.omega.velocity, max_vel, group=group)
+    yield from bps.abs_set(backlight, BacklightPosition.IN, group=group)
     yield from bps.abs_set(
-        composite.aperture_scatterguard,
+        aperture_scatterguard,
         ApertureValue.ROBOT_LOAD,
-        group=OAV_SNAPSHOT_SETUP_GROUP,
+        group=group,
     )
 
 
@@ -54,7 +48,7 @@ def oav_snapshot_plan(
 ) -> MsgGenerator:
     if not parameters.take_snapshots:
         return
-    yield from bps.wait(group=OAV_SNAPSHOT_SETUP_GROUP)
+    yield from bps.wait(group=CONST.WAIT.READY_FOR_OAV)
     yield from _setup_oav(composite, parameters, oav_parameters)
     for omega in parameters.snapshot_omegas_deg or []:
         yield from _take_oav_snapshot(composite, omega)

src/mx_bluesky/hyperion/experiment_plans/pin_centre_then_xray_centre_plan.py

@@ -6,13 +6,17 @@
 from dodal.devices.eiger import EigerDetector
 from dodal.devices.oav.oav_parameters import OAV_CONFIG_JSON, OAVParameters
 
+from mx_bluesky.hyperion.device_setup_plans.manipulate_sample import move_phi_chi_omega
 from mx_bluesky.hyperion.device_setup_plans.utils import (
     start_preparing_data_collection_then_do_plan,
 )
 from mx_bluesky.hyperion.experiment_plans.grid_detect_then_xray_centre_plan import (
     GridDetectThenXRayCentreComposite,
     detect_grid_and_do_gridscan,
 )
+from mx_bluesky.hyperion.experiment_plans.oav_snapshot_plan import (
+    setup_beamline_for_OAV,
+)
 from mx_bluesky.hyperion.experiment_plans.pin_tip_centring_plan import (
     PinTipCentringComposite,
     pin_tip_centre_plan,
@@ -65,6 +69,17 @@ def pin_centre_then_xray_centre_plan(
     )
 
     def _pin_centre_then_xray_centre_plan():
+        yield from setup_beamline_for_OAV(
+            composite.smargon, composite.backlight, composite.aperture_scatterguard
+        )
+
+        yield from move_phi_chi_omega(
+            composite.smargon,
+            parameters.phi_start_deg,
+            parameters.chi_start_deg,
+            group=CONST.WAIT.READY_FOR_OAV,
+        )
+
         yield from pin_tip_centre_plan(
             pin_tip_centring_composite,
             parameters.tip_offset_um,

src/mx_bluesky/hyperion/experiment_plans/rotation_scan_plan.py

@@ -49,7 +49,7 @@
 from mx_bluesky.hyperion.experiment_plans.oav_snapshot_plan import (
     OavSnapshotComposite,
     oav_snapshot_plan,
-    setup_oav_snapshot_plan,
+    setup_beamline_for_OAV,
 )
 from mx_bluesky.hyperion.log import LOGGER
 from mx_bluesky.hyperion.parameters.constants import CONST
@@ -324,8 +324,8 @@ def _div_by_1000_if_not_none(num: float | None):
     )
     if params.take_snapshots:
         yield from bps.wait(CONST.WAIT.MOVE_GONIO_TO_START)
-        yield from setup_oav_snapshot_plan(
-            composite, params, motion_values.max_velocity_deg_s
+        yield from setup_beamline_for_OAV(
+            composite.smargon, composite.backlight, composite.aperture_scatterguard
         )
         yield from oav_snapshot_plan(composite, params, oav_params)
     yield from rotation_scan_plan(

src/mx_bluesky/hyperion/parameters/constants.py

@@ -42,6 +42,7 @@ class PlanGroupCheckpointConstants:
     GRID_READY_FOR_DC = "grid_ready_for_data_collection"
     ROTATION_READY_FOR_DC = "rotation_ready_for_data_collection"
     MOVE_GONIO_TO_START = "move_gonio_to_start"
+    READY_FOR_OAV = "ready_for_oav"
 
 
 @dataclass(frozen=True)

tests/conftest.py

@@ -345,8 +345,18 @@ def oav(test_config_files):
     parameters = OAVConfigParams(
         test_config_files["zoom_params_file"], test_config_files["display_config"]
     )
+    parameters.micronsPerXPixel = 2.87
+    parameters.micronsPerYPixel = 2.87
     oav = i03.oav(fake_with_ophyd_sim=True, params=parameters)
     oav.snapshot.trigger = MagicMock(return_value=NullStatus())
+    oav.zoom_controller.zrst.set("1.0x")
+    oav.zoom_controller.onst.set("2.0x")
+    oav.zoom_controller.twst.set("3.0x")
+    oav.zoom_controller.thst.set("5.0x")
+    oav.zoom_controller.frst.set("7.0x")
+    oav.zoom_controller.fvst.set("9.0x")
+    oav.proc.port_name.sim_put("proc")  # type: ignore
+    oav.cam.port_name.sim_put("CAM")  # type: ignore
     oav.grid_snapshot.trigger = MagicMock(return_value=NullStatus())
     return oav
 

tests/unit_tests/hyperion/device_setup_plans/test_setup_oav.py

@@ -3,8 +3,7 @@
 import pytest
 from bluesky import plan_stubs as bps
 from bluesky.run_engine import RunEngine
-from dodal.beamlines import i03
-from dodal.devices.oav.oav_detector import OAV, OAVConfigParams
+from dodal.devices.oav.oav_detector import OAV
 from dodal.devices.oav.oav_parameters import OAVParameters
 from dodal.devices.oav.pin_image_recognition import PinTipDetection
 from ophyd.signal import Signal
@@ -14,26 +13,7 @@
     pre_centring_setup_oav,
 )
 
-ZOOM_LEVELS_XML = "tests/test_data/test_jCameraManZoomLevels.xml"
 OAV_CENTRING_JSON = "tests/test_data/test_OAVCentring.json"
-DISPLAY_CONFIGURATION = "tests/test_data/test_display.configuration"
-
-
-@pytest.fixture
-def oav() -> OAV:
-    oav = i03.oav(fake_with_ophyd_sim=True)
-    oav.parameters = OAVConfigParams(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION)
-
-    oav.proc.port_name.sim_put("proc")  # type: ignore
-    oav.cam.port_name.sim_put("CAM")  # type: ignore
-
-    oav.zoom_controller.zrst.set("1.0x")
-    oav.zoom_controller.onst.set("2.0x")
-    oav.zoom_controller.twst.set("3.0x")
-    oav.zoom_controller.thst.set("5.0x")
-    oav.zoom_controller.frst.set("7.0x")
-    oav.zoom_controller.fvst.set("9.0x")
-    return oav
 
 
 @pytest.fixture

tests/unit_tests/hyperion/experiment_plans/test_pin_centre_then_xray_centre_plan.py

@@ -4,7 +4,10 @@
 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 ApertureScatterguard, ApertureValue
+from dodal.devices.backlight import Backlight, BacklightPosition
 from dodal.devices.detector.detector_motion import ShutterState
+from dodal.devices.smargon import Smargon
 from dodal.devices.synchrotron import SynchrotronMode
 
 from mx_bluesky.hyperion.experiment_plans.pin_centre_then_xray_centre_plan import (
@@ -189,3 +192,112 @@ def test_pin_centre_then_xray_centre_plan_activates_ispyb_callback_before_pin_ti
         msgs, lambda msg: msg.command == "detect_grid_and_do_gridscan"
     )
     assert_message_and_return_remaining(msgs, lambda msg: msg.command == "close_run")
+
+
+@patch(
+    "mx_bluesky.hyperion.experiment_plans.pin_centre_then_xray_centre_plan.pin_tip_centre_plan",
+    autospec=True,
+)
+@patch(
+    "mx_bluesky.hyperion.experiment_plans.pin_centre_then_xray_centre_plan.detect_grid_and_do_gridscan",
+    autospec=True,
+)
+def test_pin_centre_then_xray_centre_plan_sets_up_backlight_and_aperture(
+    mock_detect_grid_and_do_gridscan,
+    mock_pin_tip_centre_plan,
+    backlight: Backlight,
+    aperture_scatterguard: ApertureScatterguard,
+    sim_run_engine: RunEngineSimulator,
+    test_pin_centre_then_xray_centre_params: PinTipCentreThenXrayCentre,
+    test_config_files,
+):
+    mock_detect_grid_and_do_gridscan.return_value = iter(
+        [Msg("detect_grid_and_do_gridscan")]
+    )
+    mock_pin_tip_centre_plan.return_value = iter([Msg("pin_tip_centre_plan")])
+
+    mock_composite = MagicMock()
+    mock_composite.aperture_scatterguard = aperture_scatterguard
+    mock_composite.backlight = backlight
+
+    msgs = sim_run_engine.simulate_plan(
+        pin_centre_then_xray_centre_plan(
+            mock_composite,
+            test_pin_centre_then_xray_centre_params,
+            test_config_files["oav_config_json"],
+        )
+    )
+
+    msgs = assert_message_and_return_remaining(
+        msgs,
+        lambda msg: msg.command == "set"
+        and msg.obj.name == "backlight"
+        and msg.args == (BacklightPosition.IN,)
+        and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV,
+    )
+    msgs = assert_message_and_return_remaining(
+        msgs,
+        lambda msg: msg.command == "set"
+        and msg.obj.name == "aperture_scatterguard"
+        and msg.args == (ApertureValue.ROBOT_LOAD,)
+        and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV,
+    )
+
+    msgs = assert_message_and_return_remaining(
+        msgs, lambda msg: msg.command == "pin_tip_centre_plan"
+    )
+
+
+@patch(
+    "mx_bluesky.hyperion.experiment_plans.pin_centre_then_xray_centre_plan.pin_tip_centre_plan",
+    autospec=True,
+)
+@patch(
+    "mx_bluesky.hyperion.experiment_plans.pin_centre_then_xray_centre_plan.detect_grid_and_do_gridscan",
+    autospec=True,
+)
+def test_pin_centre_then_xray_centre_plan_goes_to_the_starting_chi_and_phi(
+    mock_detect_grid_and_do_gridscan,
+    mock_pin_tip_centre_plan,
+    smargon: Smargon,
+    sim_run_engine: RunEngineSimulator,
+    test_pin_centre_then_xray_centre_params: PinTipCentreThenXrayCentre,
+    test_config_files,
+):
+    mock_detect_grid_and_do_gridscan.return_value = iter(
+        [Msg("detect_grid_and_do_gridscan")]
+    )
+    mock_pin_tip_centre_plan.return_value = iter([Msg("pin_tip_centre_plan")])
+
+    mock_composite = MagicMock()
+    mock_composite.smargon = smargon
+
+    test_pin_centre_then_xray_centre_params.phi_start_deg = 30
+    test_pin_centre_then_xray_centre_params.chi_start_deg = 50
+
+    msgs = sim_run_engine.simulate_plan(
+        pin_centre_then_xray_centre_plan(
+            mock_composite,
+            test_pin_centre_then_xray_centre_params,
+            test_config_files["oav_config_json"],
+        )
+    )
+
+    msgs = assert_message_and_return_remaining(
+        msgs,
+        lambda msg: msg.command == "set"
+        and msg.obj.name == "smargon-phi"
+        and msg.args == (test_pin_centre_then_xray_centre_params.phi_start_deg,)
+        and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV,
+    )
+    msgs = assert_message_and_return_remaining(
+        msgs,
+        lambda msg: msg.command == "set"
+        and msg.obj.name == "smargon-chi"
+        and msg.args == (test_pin_centre_then_xray_centre_params.chi_start_deg,)
+        and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV,
+    )
+
+    msgs = assert_message_and_return_remaining(
+        msgs, lambda msg: msg.command == "pin_tip_centre_plan"
+    )

tests/unit_tests/hyperion/experiment_plans/test_pin_tip_centring.py

@@ -318,20 +318,15 @@ def test_given_pin_tip_detect_using_ophyd_when_pin_tip_centre_plan_called_then_e
     mock_move_into_view,
     get_move: MagicMock,
     smargon: Smargon,
+    oav: OAV,
     test_config_files: dict[str, str],
     RE: RunEngine,
 ):
     set_mock_value(smargon.omega.user_readback, 0)
-    mock_oav: OAV = MagicMock(spec=OAV)
-    mock_oav.parameters = OAVConfigParams(
-        test_config_files["zoom_params_file"], test_config_files["display_config"]
-    )
-    mock_oav.parameters.micronsPerXPixel = 2.87
-    mock_oav.parameters.micronsPerYPixel = 2.87
     mock_ophyd_pin_tip_detection = MagicMock()
     composite = PinTipCentringComposite(
         backlight=MagicMock(),
-        oav=mock_oav,
+        oav=oav,
         smargon=smargon,
         pin_tip_detection=mock_ophyd_pin_tip_detection,
     )

tests/unit_tests/hyperion/experiment_plans/test_rotation_scan_plan.py

@@ -20,7 +20,6 @@
 
 from mx_bluesky.hyperion.experiment_plans.oav_snapshot_plan import (
     OAV_SNAPSHOT_GROUP,
-    OAV_SNAPSHOT_SETUP_GROUP,
 )
 from mx_bluesky.hyperion.experiment_plans.rotation_scan_plan import (
     RotationMotionProfile,
@@ -435,7 +434,7 @@ def test_rotation_scan_moves_gonio_to_start_before_snapshots(
     msgs = assert_message_and_return_remaining(
         msgs,
         lambda msg: msg.command == "wait"
-        and msg.kwargs["group"] == OAV_SNAPSHOT_SETUP_GROUP,
+        and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV,
     )
 
 
@@ -505,19 +504,19 @@ def test_rotation_snapshot_setup_called_to_move_backlight_in_aperture_out_before
         lambda msg: msg.command == "set"
         and msg.obj.name == "backlight"
         and msg.args[0] == BacklightPosition.IN
-        and msg.kwargs["group"] == OAV_SNAPSHOT_SETUP_GROUP,
+        and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV,
     )
     msgs = assert_message_and_return_remaining(
         msgs,
         lambda msg: msg.command == "set"
         and msg.obj.name == "aperture_scatterguard"
         and msg.args[0] == ApertureValue.ROBOT_LOAD
-        and msg.kwargs["group"] == OAV_SNAPSHOT_SETUP_GROUP,
+        and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV,
     )
     msgs = assert_message_and_return_remaining(
         msgs,
         lambda msg: msg.command == "wait"
-        and msg.kwargs["group"] == OAV_SNAPSHOT_SETUP_GROUP,
+        and msg.kwargs["group"] == CONST.WAIT.READY_FOR_OAV,
     )
     msgs = assert_message_and_return_remaining(
         msgs, lambda msg: msg.command == "trigger" and msg.obj.name == "oav_snapshot"
@@ -541,7 +540,7 @@ def test_rotation_scan_skips_init_backlight_aperture_and_snapshots_if_snapshot_p
         )
     )
     assert not [
-        msg for msg in msgs if msg.kwargs.get("group", None) == OAV_SNAPSHOT_SETUP_GROUP
+        msg for msg in msgs if msg.kwargs.get("group", None) == CONST.WAIT.READY_FOR_OAV
     ]
     assert not [
         msg for msg in msgs if msg.kwargs.get("group", None) == OAV_SNAPSHOT_GROUP