Dodal 601 convert smargon to ophyd async (#1469)

* (DiamondLightSource/dodal#601) Fix broken unit tests as a result of dodal changes

* Bump dodal commit hash

setup.cfg

@@ -46,7 +46,7 @@ install_requires =
     ophyd-async >= 0.3a5
     bluesky >= 1.13.0a3
     blueapi >= 0.4.3-rc1
-    dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@a9a116d289abc9e67ce8db08f978ff502705b464
+    dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git
 
 [options.entry_points]
 console_scripts =

src/hyperion/device_setup_plans/smargon.py

@@ -0,0 +1,25 @@
+import numpy as np
+from bluesky import plan_stubs as bps
+from dodal.devices.smargon import Smargon
+
+from hyperion.exceptions import WarningException
+
+
+def move_smargon_warn_on_out_of_range(
+    smargon: Smargon, position: np.ndarray | list[float] | tuple[float, float, float]
+):
+    """Throws a WarningException if the specified position is out of range for the
+    smargon. Otherwise moves to that position."""
+    limits = yield from smargon.get_xyz_limits()
+    if not limits.position_valid(position):
+        raise WarningException(
+            "Pin tip centring failed - pin too long/short/bent and out of range"
+        )
+    yield from bps.mv(
+        smargon.x,
+        position[0],
+        smargon.y,
+        position[1],
+        smargon.z,
+        position[2],
+    )

src/hyperion/experiment_plans/pin_tip_centring_plan.py

@@ -2,7 +2,6 @@
 from typing import Generator
 
 import bluesky.plan_stubs as bps
-import numpy as np
 from blueapi.core import BlueskyContext
 from bluesky.utils import Msg
 from dodal.devices.backlight import Backlight
@@ -17,6 +16,7 @@
 from dodal.devices.smargon import Smargon
 
 from hyperion.device_setup_plans.setup_oav import pre_centring_setup_oav
+from hyperion.device_setup_plans.smargon import move_smargon_warn_on_out_of_range
 from hyperion.exceptions import WarningException
 from hyperion.log import LOGGER
 from hyperion.parameters.constants import CONST
@@ -87,9 +87,9 @@ def pin_tip_valid(pin_x: float):
 
         smargon_x = yield from bps.rd(smargon.x.user_readback)
         ideal_move_to_find_pin = float(smargon_x) + step_size_mm
-        move_within_limits = max(
-            min(ideal_move_to_find_pin, smargon.x.high_limit), smargon.x.low_limit
-        )
+        high_limit = yield from bps.rd(smargon.x.high_limit_travel)
+        low_limit = yield from bps.rd(smargon.x.low_limit_travel)
+        move_within_limits = max(min(ideal_move_to_find_pin, high_limit), low_limit)
         if move_within_limits != ideal_move_to_find_pin:
             LOGGER.warning(
                 f"Pin tip is off screen, and moving {step_size_mm} mm would cross limits, "
@@ -110,25 +110,6 @@ def pin_tip_valid(pin_x: float):
         return (tip_x_px, tip_y_px)
 
 
-def move_smargon_warn_on_out_of_range(
-    smargon: Smargon, position: np.ndarray | list[float] | tuple[float, float, float]
-):
-    """Throws a WarningException if the specified position is out of range for the
-    smargon. Otherwise moves to that position."""
-    if not smargon.get_xyz_limits().position_valid(position):
-        raise WarningException(
-            "Pin tip centring failed - pin too long/short/bent and out of range"
-        )
-    yield from bps.mv(
-        smargon.x,
-        position[0],
-        smargon.y,
-        position[1],
-        smargon.z,
-        position[2],
-    )
-
-
 def pin_tip_centre_plan(
     composite: PinTipCentringComposite,
     tip_offset_microns: float,

src/hyperion/experiment_plans/rotation_scan_plan.py

@@ -274,7 +274,9 @@ def rotation_scan(
     def rotation_scan_plan_with_stage_and_cleanup(
         params: RotationScan,
     ):
-        motor_time_to_speed = yield from bps.rd(composite.smargon.omega.acceleration)
+        motor_time_to_speed = yield from bps.rd(
+            composite.smargon.omega.acceleration_time
+        )
         max_vel = (
             yield from bps.rd(composite.smargon.omega.max_velocity)
             or DEFAULT_MAX_VELOCITY

src/hyperion/external_interaction/callbacks/grid_detection_callback.py

@@ -43,9 +43,9 @@ def event(self, doc: Event):
         top_left_y_px = data["oav_grid_snapshot_top_left_y"]
         y_of_centre_of_first_box_px = top_left_y_px + box_width_px / 2
 
-        smargon_omega = data["smargon_omega"]
+        smargon_omega = data["smargon-omega"]
         current_xyz = np.array(
-            [data["smargon_x"], data["smargon_y"], data["smargon_z"]]
+            [data["smargon-x"], data["smargon-y"], data["smargon-z"]]
         )
 
         centre_of_first_box = (

src/hyperion/external_interaction/callbacks/ispyb_callback_base.py

@@ -118,9 +118,9 @@ def _handle_ispyb_hardware_read(self, doc) -> Sequence[ScanDataInfo]:
             slitgap_vertical=doc["data"]["s4_slit_gaps_ygap"],
         )
         hwscan_position_info = DataCollectionPositionInfo(
-            pos_x=doc["data"]["smargon_x"],
-            pos_y=doc["data"]["smargon_y"],
-            pos_z=doc["data"]["smargon_z"],
+            pos_x=doc["data"]["smargon-x"],
+            pos_y=doc["data"]["smargon-y"],
+            pos_z=doc["data"]["smargon-z"],
         )
         scan_data_infos = self.populate_info_for_update(
             hwscan_data_collection_info, hwscan_position_info, self.params

src/hyperion/external_interaction/callbacks/rotation/ispyb_callback.py

@@ -130,9 +130,9 @@ def _handle_ispyb_hardware_read(self, doc: Event):
         )
 
         motor_positions = [
-            doc["data"]["smargon_x"],
-            doc["data"]["smargon_y"],
-            doc["data"]["smargon_z"],
+            doc["data"]["smargon-x"],
+            doc["data"]["smargon-y"],
+            doc["data"]["smargon-z"],
         ]
         assert (
             self.params

src/hyperion/external_interaction/callbacks/xray_centre/ispyb_callback.py

@@ -207,7 +207,7 @@ def _handle_oav_grid_snapshot_triggered(self, doc) -> Sequence[ScanDataInfo]:
             data_collection_id = self.ispyb_ids.data_collection_ids[
                 self._oav_snapshot_event_idx
             ]
-        self._populate_axis_info(data_collection_info, doc["data"]["smargon_omega"])
+        self._populate_axis_info(data_collection_info, doc["data"]["smargon-omega"])
 
         scan_data_info = ScanDataInfo(
             data_collection_info=data_collection_info,

src/hyperion/utils/validation.py

@@ -3,12 +3,11 @@
 import os
 import shutil
 from pathlib import Path
-from unittest.mock import MagicMock, patch
+from unittest.mock import patch
 
 import bluesky.preprocessors as bpp
 from bluesky.run_engine import RunEngine
 from dodal.beamlines import i03
-from ophyd.status import Status
 from ophyd_async.core import set_mock_value
 
 from hyperion.device_setup_plans.read_hardware_for_setup import (
@@ -86,13 +85,8 @@ def fake_create_rotation_devices():
     s4_slit_gaps = i03.s4_slit_gaps(fake_with_ophyd_sim=True)
     dcm = i03.dcm(fake_with_ophyd_sim=True)
     robot = i03.robot(fake_with_ophyd_sim=True)
-    mock_omega_sets = MagicMock(return_value=Status(done=True, success=True))
-    mock_omega_velocity_sets = MagicMock(return_value=Status(done=True, success=True))
 
-    smargon.omega.velocity.set = mock_omega_velocity_sets
-    smargon.omega.set = mock_omega_sets
-
-    smargon.omega.max_velocity.sim_put(131)  # type: ignore
+    set_mock_value(smargon.omega.max_velocity, 131)
 
     set_mock_value(dcm.energy_in_kev.user_readback, 12700)
 

tests/conftest.py

@@ -17,6 +17,7 @@
 from dodal.common.beamlines.beamline_parameters import (
     GDABeamlineParameters,
 )
+from dodal.common.beamlines.beamline_utils import clear_devices
 from dodal.devices.aperturescatterguard import (
     ApertureFiveDimensionalLocation,
     AperturePositions,
@@ -262,29 +263,26 @@ def eiger(done_status):
 
 
 @pytest.fixture
-def smargon() -> Generator[Smargon, None, None]:
+def smargon(RE: RunEngine) -> Generator[Smargon, None, None]:
     smargon = i03.smargon(fake_with_ophyd_sim=True)
-    smargon.x.user_setpoint._use_limits = False
-    smargon.y.user_setpoint._use_limits = False
-    smargon.z.user_setpoint._use_limits = False
-    smargon.omega.user_setpoint._use_limits = False
-    smargon.omega.velocity._use_limits = False
-
     # Initial positions, needed for stub_offsets
-    smargon.stub_offsets.center_at_current_position.disp.sim_put(0)  # type: ignore
-    smargon.x.user_readback.sim_put(0.0)  # type: ignore
-    smargon.y.user_readback.sim_put(0.0)  # type: ignore
-    smargon.z.user_readback.sim_put(0.0)  # type: ignore
+    set_mock_value(smargon.stub_offsets.center_at_current_position.disp, 0)
+    set_mock_value(smargon.x.user_readback, 0.0)
+    set_mock_value(smargon.y.user_readback, 0.0)
+    set_mock_value(smargon.z.user_readback, 0.0)
+    set_mock_value(smargon.x.high_limit_travel, 2)
+    set_mock_value(smargon.x.low_limit_travel, -2)
 
     with (
-        patch_motor(smargon.omega),
-        patch_motor(smargon.x),
-        patch_motor(smargon.y),
-        patch_motor(smargon.z),
-        patch_motor(smargon.chi),
-        patch_motor(smargon.phi),
+        patch_async_motor(smargon.omega),
+        patch_async_motor(smargon.x),
+        patch_async_motor(smargon.y),
+        patch_async_motor(smargon.z),
+        patch_async_motor(smargon.chi),
+        patch_async_motor(smargon.phi),
     ):
         yield smargon
+    clear_devices()
 
 
 @pytest.fixture
@@ -560,13 +558,7 @@ def fake_create_rotation_devices(
     robot: BartRobot,
     done_status,
 ):
-    mock_omega_sets = MagicMock(return_value=Status(done=True, success=True))
-    mock_omega_velocity_sets = MagicMock(return_value=Status(done=True, success=True))
-
-    smargon.omega.velocity.set = mock_omega_velocity_sets
-    smargon.omega.set = mock_omega_sets
-
-    smargon.omega.max_velocity.sim_put(131)  # type: ignore
+    set_mock_value(smargon.omega.max_velocity, 131)
 
     return RotationScanComposite(
         attenuator=attenuator,
@@ -711,7 +703,7 @@ async def mock_complete(result):
     fake_composite.zocalo.timeout_s = 3
     set_mock_value(fake_composite.zebra_fast_grid_scan.scan_invalid, False)
     set_mock_value(fake_composite.zebra_fast_grid_scan.position_counter, 0)
-    fake_composite.smargon.x.max_velocity.sim_put(10)  # type: ignore
+    set_mock_value(fake_composite.smargon.x.max_velocity, 10)
 
     set_mock_value(fake_composite.robot.barcode, "BARCODE")
 

tests/unit_tests/experiment_plans/conftest.py

@@ -48,9 +48,9 @@ def make_event_doc(data, descriptor="abc123") -> Event:
         "radius_microns": None,
         "location": (15, 16, 2, 18, 19),
     },
-    "smargon_x": 10.0,
-    "smargon_y": 20.0,
-    "smargon_z": 30.0,
+    "smargon-x": 10.0,
+    "smargon-y": 20.0,
+    "smargon-z": 30.0,
 }
 
 BASIC_POST_SETUP_DOC = {

tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py

@@ -440,7 +440,7 @@ def wrapped_gridscan_and_move():
     @patch(
         "hyperion.experiment_plans.flyscan_xray_centre_plan.move_x_y_z", autospec=True
     )
-    def test_when_gridscan_finished_then_smargon_stub_offsets_are_set_and_dev_shm_disabled(
+    async def test_when_gridscan_finished_then_smargon_stub_offsets_are_set_and_dev_shm_disabled(
         self,
         move_xyz: MagicMock,
         run_gridscan: MagicMock,
@@ -469,7 +469,7 @@ def wrapped_gridscan_and_move():
             )
         )
         assert (
-            fake_fgs_composite.smargon.stub_offsets.center_at_current_position.proc.get()
+            await fake_fgs_composite.smargon.stub_offsets.center_at_current_position.proc.get_value()
             == 1
         )
         assert fake_fgs_composite.eiger.odin.fan.dev_shm_enable.get() == 0
@@ -639,9 +639,9 @@ def test_GIVEN_no_results_from_zocalo_WHEN_communicator_wait_for_results_called_
                 random.uniform(-0.5, 0.5),
             ]
         )
-        fake_fgs_composite.smargon.x.user_readback.sim_put(initial_x_y_z[0])  # type: ignore
-        fake_fgs_composite.smargon.y.user_readback.sim_put(initial_x_y_z[1])  # type: ignore
-        fake_fgs_composite.smargon.z.user_readback.sim_put(initial_x_y_z[2])  # type: ignore
+        set_mock_value(fake_fgs_composite.smargon.x.user_readback, initial_x_y_z[0])
+        set_mock_value(fake_fgs_composite.smargon.y.user_readback, initial_x_y_z[1])
+        set_mock_value(fake_fgs_composite.smargon.z.user_readback, initial_x_y_z[2])
 
         def wrapped_gridscan_and_move():
             run_generic_ispyb_handler_setup(ispyb_cb, test_fgs_params_panda_zebra)
@@ -663,7 +663,7 @@ def wrapped_gridscan_and_move():
     @patch(
         "hyperion.experiment_plans.flyscan_xray_centre_plan.move_x_y_z", autospec=True
     )
-    def test_given_gridscan_fails_to_centre_then_stub_offsets_not_set(
+    async def test_given_gridscan_fails_to_centre_then_stub_offsets_not_set(
         self,
         move_xyz: MagicMock,
         run_gridscan: MagicMock,
@@ -688,7 +688,7 @@ class MoveException(Exception):
                 )
             )
         assert (
-            fake_fgs_composite.smargon.stub_offsets.center_at_current_position.proc.get()
+            await fake_fgs_composite.smargon.stub_offsets.center_at_current_position.proc.get_value()
             == 0
         )
 
@@ -698,7 +698,7 @@ class MoveException(Exception):
     @patch(
         "hyperion.experiment_plans.flyscan_xray_centre_plan.move_x_y_z", autospec=True
     )
-    def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set(
+    async def test_given_setting_stub_offsets_disabled_then_stub_offsets_not_set(
         self,
         move_xyz: MagicMock,
         run_gridscan: MagicMock,
@@ -731,7 +731,7 @@ def wrapped_gridscan_and_move():
             )
         )
         assert (
-            fake_fgs_composite.smargon.stub_offsets.center_at_current_position.proc.get()
+            await fake_fgs_composite.smargon.stub_offsets.center_at_current_position.proc.get_value()
             == 0
         )
 

tests/unit_tests/experiment_plans/test_oav_snapshot_plan.py

@@ -89,7 +89,7 @@ def test_oav_snapshot_plan_issues_rotations_and_generates_events(
         msgs = sim_run_engine.assert_message_and_return_remaining(
             msgs,
             lambda msg: msg.command == "set"
-            and msg.obj.name == "smargon_omega"
+            and msg.obj.name == "smargon-omega"
             and msg.args[0] == expected["omega"]
             and msg.kwargs["group"] == OAV_SNAPSHOT_GROUP,
         )