(#391) Create an ophyd-async ApertureScatterguard device

.pre-commit-config.yaml

@@ -21,7 +21,7 @@ repos:
         name: Run ruff
         stages: [commit]
         language: system
-        entry: ruff
+        entry: ruff check
         types: [python]
 
   # Re-enable after https://github.com/DiamondLightSource/dodal/issues/311

src/dodal/devices/aperture.py

@@ -1,12 +1,15 @@
-from ophyd import Component, Device, EpicsSignalRO
+from ophyd_async.core import StandardReadable
+from ophyd_async.epics.signal import epics_signal_r
 
-from dodal.devices.util.motor_utils import ExtendedEpicsMotor
+from dodal.devices.util.motor_utils import ExtendedMotor
 
 
-class Aperture(Device):
-    x = Component(ExtendedEpicsMotor, "X")
-    y = Component(ExtendedEpicsMotor, "Y")
-    z = Component(ExtendedEpicsMotor, "Z")
-    small = Component(EpicsSignalRO, "Y:SMALL_CALC")
-    medium = Component(EpicsSignalRO, "Y:MEDIUM_CALC")
-    large = Component(EpicsSignalRO, "Y:LARGE_CALC")
+class Aperture(StandardReadable):
+    def __init__(self, prefix: str, name: str = "") -> None:
+        self.x = ExtendedMotor(prefix + "X")
+        self.y = ExtendedMotor(prefix + "Y")
+        self.z = ExtendedMotor(prefix + "Z")
+        self.small = epics_signal_r(int, prefix + "Y:SMALL_CALC")
+        self.medium = epics_signal_r(int, prefix + "Y:MEDIUM_CALC")
+        self.large = epics_signal_r(int, prefix + "Y:LARGE_CALC")
+        super().__init__(name)

src/dodal/devices/aperturescatterguard.py

@@ -1,16 +1,12 @@
-import operator
+import asyncio
 from collections import namedtuple
 from dataclasses import dataclass
-from functools import reduce
-from typing import List, Optional, Sequence
+from typing import List, Optional
 
-from ophyd import Component as Cpt
-from ophyd import SignalRO
-from ophyd.epics_motor import EpicsMotor
-from ophyd.status import AndStatus, Status, StatusBase
+from ophyd_async.core import SignalR, StandardReadable
+from ophyd_async.core.sim_signal_backend import SimSignalBackend
 
 from dodal.devices.aperture import Aperture
-from dodal.devices.logging_ophyd_device import InfoLoggingDevice
 from dodal.devices.scatterguard import Scatterguard
 from dodal.log import LOGGER
 
@@ -33,10 +29,12 @@
 
 @dataclass
 class SingleAperturePosition:
-    name: str
-    GDA_name: str
-    radius_microns: Optional[float]
-    location: ApertureFiveDimensionalLocation
+    name: str = ""
+    GDA_name: str = ""
+    radius_microns: Optional[float] = 0
+    location: ApertureFiveDimensionalLocation = ApertureFiveDimensionalLocation(
+        0, 0, 0, 0, 0
+    )
 
 
 def position_from_params(
@@ -83,24 +81,33 @@
         ]
 
 
-class ApertureScatterguard(InfoLoggingDevice):
-    aperture = Cpt(Aperture, "-MO-MAPT-01:")
-    scatterguard = Cpt(Scatterguard, "-MO-SCAT-01:")
-    aperture_positions: Optional[AperturePositions] = None
-    TOLERANCE_STEPS = 3  # Number of MRES steps
+class ApertureScatterguard(StandardReadable):
+    def __init__(self, prefix: str = "", name: str = "") -> None:
+        self.aperture = Aperture(prefix="-MO-MAPT-01:")
+        self.scatterguard = Scatterguard(prefix="-MO-SCAT-01:")
+        self.aperture_positions: Optional[AperturePositions] = None
+        self.TOLERANCE_STEPS = 3  # Number of MRES steps
+        self.selected_aperture = self.SelectedAperture(
+            backend=SimSignalBackend(datatype=SingleAperturePosition, source="")
+        )
+        self.set_readable_signals(
+            read=[
+                self.selected_aperture,
+            ]
+        )
+        super().__init__(name)
 
-    class SelectedAperture(SignalRO):
-        def get(self):
+    class SelectedAperture(SignalR):
+        async def read(self, *args, **kwargs):
             assert isinstance(self.parent, ApertureScatterguard)
-            return self.parent._get_current_aperture_position()
-
-    selected_aperture = Cpt(SelectedAperture)
+            await self._backend.put(await self.parent._get_current_aperture_position())
+            return {self.name: await self._backend.get_reading()}
 
     def load_aperture_positions(self, positions: AperturePositions):
         LOGGER.info(f"{self.name} loaded in {positions}")
         self.aperture_positions = positions
 
-    def set(self, pos: SingleAperturePosition) -> StatusBase:
+    def set(self, pos: SingleAperturePosition):
         assert isinstance(self.aperture_positions, AperturePositions)
         if pos not in self.aperture_positions.as_list():
             raise InvalidApertureMove(f"Unknown aperture: {pos}")
@@ -116,37 +123,49 @@
             self.scatterguard.y,
         ]
 
-    def _set_raw_unsafe(self, positions: ApertureFiveDimensionalLocation) -> AndStatus:
-        motors: Sequence[EpicsMotor] = self._get_motor_list()
-        return reduce(
-            operator.and_, [motor.set(pos) for motor, pos in zip(motors, positions)]
+    async def _set_raw_unsafe(self, positions: ApertureFiveDimensionalLocation):
+        """Accept the risks and move in an unsafe way. Collisions possible."""
+
+        # unpacking the position
+        aperture_x, aperture_y, aperture_z, scatterguard_x, scatterguard_y = positions
+
+        await asyncio.gather(
+            self.aperture.x._move(aperture_x),
+            self.aperture.y._move(aperture_y),
+            self.aperture.z._move(aperture_z),
+            self.scatterguard.x._move(scatterguard_x),
+            self.scatterguard.y._move(scatterguard_y),
         )
 
-    def _get_current_aperture_position(self) -> SingleAperturePosition:
+    async def _get_current_aperture_position(self) -> SingleAperturePosition:
         """
         Returns the current aperture position using readback values
         for SMALL, MEDIUM, LARGE. ROBOT_LOAD position defined when
         mini aperture y <= ROBOT_LOAD.location.aperture_y + tolerance.
         If no position is found then raises InvalidApertureMove.
         """
         assert isinstance(self.aperture_positions, AperturePositions)
-        current_ap_y = float(self.aperture.y.user_readback.get())
+        current_ap_y = await self.aperture.y.readback.get_value(cached=False)
         robot_load_ap_y = self.aperture_positions.ROBOT_LOAD.location.aperture_y
-        tolerance = self.TOLERANCE_STEPS * self.aperture.y.motor_resolution.get()
-        if int(self.aperture.large.get()) == 1:
+        tolerance = (
+            self.TOLERANCE_STEPS * await self.aperture.y.motor_resolution.get_value()
+        )
+        # extendedepicsmotor class has tolerance fields added
+        # ophyd-async epics motor may need to do the same thing - epics motor
+        if await self.aperture.large.get_value(cached=False) == 1:
             return self.aperture_positions.LARGE
-        elif int(self.aperture.medium.get()) == 1:
+        elif await self.aperture.medium.get_value(cached=False) == 1:
             return self.aperture_positions.MEDIUM
-        elif int(self.aperture.small.get()) == 1:
+        elif await self.aperture.small.get_value(cached=False) == 1:
             return self.aperture_positions.SMALL
         elif current_ap_y <= robot_load_ap_y + tolerance:
             return self.aperture_positions.ROBOT_LOAD
 
         raise InvalidApertureMove("Current aperture/scatterguard state unrecognised")
 
-    def _safe_move_within_datacollection_range(
+    async def _safe_move_within_datacollection_range(
         self, pos: ApertureFiveDimensionalLocation
-    ) -> StatusBase:
+    ):
         """
         Move the aperture and scatterguard combo safely to a new position.
         See https://github.com/DiamondLightSource/hyperion/wiki/Aperture-Scatterguard-Collisions
@@ -159,19 +178,17 @@
         # unpacking the position
         aperture_x, aperture_y, aperture_z, scatterguard_x, scatterguard_y = pos
 
-        ap_z_in_position = self.aperture.z.motor_done_move.get()
+        ap_z_in_position = await self.aperture.z.motor_done_move.get_value()
         if not ap_z_in_position:
-            status: Status = Status(obj=self)
-            status.set_exception(
-                InvalidApertureMove(
-                    "ApertureScatterguard z is still moving. Wait for it to finish "
-                    "before triggering another move."
-                )
+            raise InvalidApertureMove(
+                "ApertureScatterguard z is still moving. Wait for it to finish "
+                "before triggering another move."
             )
-            return status
 
-        current_ap_z = self.aperture.z.user_setpoint.get()
-        tolerance = self.TOLERANCE_STEPS * self.aperture.z.motor_resolution.get()
+        current_ap_z = await self.aperture.z.readback.get_value()
+        tolerance = (
+            self.TOLERANCE_STEPS * await self.aperture.z.motor_resolution.get_value()
+        )
         diff_on_z = abs(current_ap_z - aperture_z)
         if diff_on_z > tolerance:
             raise InvalidApertureMove(
@@ -180,28 +197,26 @@
                 f"Current aperture z ({current_ap_z}), outside of tolerance ({tolerance}) from target ({aperture_z})."
             )
 
-        current_ap_y = self.aperture.y.user_readback.get()
+        current_ap_y = await self.aperture.y.readback.get_value()
+
         if aperture_y > current_ap_y:
-            sg_status: AndStatus = self.scatterguard.x.set(
-                scatterguard_x
-            ) & self.scatterguard.y.set(scatterguard_y)
-            sg_status.wait()
-            return (
-                sg_status
-                & self.aperture.x.set(aperture_x)
-                & self.aperture.y.set(aperture_y)
-                & self.aperture.z.set(aperture_z)
+            await asyncio.gather(
+                self.scatterguard.x._move(scatterguard_x),
+                self.scatterguard.y._move(scatterguard_y),
             )
-
-        ap_status: AndStatus = (
-            self.aperture.x.set(aperture_x)
-            & self.aperture.y.set(aperture_y)
-            & self.aperture.z.set(aperture_z)
+            await asyncio.gather(
+                self.aperture.x._move(aperture_x),
+                self.aperture.y._move(aperture_y),
+                self.aperture.z._move(aperture_z),
+            )
+            return
+        await asyncio.gather(
+            self.aperture.x._move(aperture_x),
+            self.aperture.y._move(aperture_y),
+            self.aperture.z._move(aperture_z),
         )
-        ap_status.wait()
-        final_status: AndStatus = (
-            ap_status
-            & self.scatterguard.x.set(scatterguard_x)
-            & self.scatterguard.y.set(scatterguard_y)
+
+        await asyncio.gather(
+            self.scatterguard.x._move(scatterguard_x),
+            self.scatterguard.y._move(scatterguard_y),
         )
-        return final_status

src/dodal/devices/scatterguard.py

@@ -1,9 +1,12 @@
-from ophyd import Component as Cpt
-from ophyd import Device
+from ophyd_async.core import StandardReadable
 
-from dodal.devices.util.motor_utils import ExtendedEpicsMotor
+from dodal.devices.util.motor_utils import ExtendedMotor
 
 
-class Scatterguard(Device):
-    x = Cpt(ExtendedEpicsMotor, "X")
-    y = Cpt(ExtendedEpicsMotor, "Y")
+class Scatterguard(StandardReadable):
+    """The scatterguard device."""
+
+    def __init__(self, prefix: str, name: str = "") -> None:
+        self.x = ExtendedMotor(prefix + "X")
+        self.y = ExtendedMotor(prefix + "Y")
+        super().__init__(name)

src/dodal/devices/util/motor_utils.py

@@ -1,6 +1,16 @@
 from ophyd import Component, EpicsMotor, EpicsSignalRO
+from ophyd_async.epics.motion import Motor
+from ophyd_async.epics.signal import epics_signal_r
 
 
 class ExtendedEpicsMotor(EpicsMotor):
     motor_resolution: Component[EpicsSignalRO] = Component(EpicsSignalRO, ".MRES")
     max_velocity: Component[EpicsSignalRO] = Component(EpicsSignalRO, ".VMAX")
+
+
+class ExtendedMotor(Motor):
+    def __init__(self, prefix: str, name: str = ""):
+        self.motor_resolution = epics_signal_r(float, prefix + ".MRES")
+        self.max_velocity = epics_signal_r(float, prefix + ".VMAX")
+        self.motor_done_move = epics_signal_r(float, prefix + ".DMOV")
+        super().__init__(name)

tests/devices/unit_tests/conftest.py

@@ -1,24 +1,29 @@
+import asyncio
 from functools import partial
 from typing import Union
-from unittest.mock import MagicMock, patch
+from unittest.mock import AsyncMock, patch
 
-from ophyd.epics_motor import EpicsMotor
-from ophyd.status import Status
+from ophyd_async.epics.motion import Motor
 
-from dodal.devices.util.motor_utils import ExtendedEpicsMotor
+from dodal.devices.util.motor_utils import ExtendedMotor
 
 
-def mock_set(motor: EpicsMotor, val):
-    motor.user_setpoint.sim_put(val)  # type: ignore
-    motor.user_readback.sim_put(val)  # type: ignore
-    return Status(done=True, success=True)
+async def mock_good_coroutine():
+    return asyncio.sleep(0)
 
 
-def patch_motor(motor: Union[EpicsMotor, ExtendedEpicsMotor], initial_position=0):
-    motor.user_setpoint.sim_put(initial_position)  # type: ignore
-    motor.user_readback.sim_put(initial_position)  # type: ignore
-    motor.motor_done_move.sim_put(1)  # type: ignore
-    motor.user_setpoint._use_limits = False
-    if isinstance(motor, ExtendedEpicsMotor):
-        motor.motor_resolution.sim_put(0.001)  # type: ignore
-    return patch.object(motor, "set", MagicMock(side_effect=partial(mock_set, motor)))
+def mock_move(motor: Union[ExtendedMotor, Motor], val, *args, **kwargs):
+    motor.setpoint._backend._set_value(val)  # type: ignore
+    motor.readback._backend._set_value(val)  # type: ignore
+    return mock_good_coroutine()  # type: ignore
+
+
+def patch_motor(motor: Union[ExtendedMotor, Motor], initial_position=0):
+    motor.setpoint._backend._set_value(initial_position)  # type: ignore
+    motor.readback._backend._set_value(initial_position)  # type: ignore
+    if isinstance(motor, ExtendedMotor):
+        motor.motor_resolution._backend._set_value(0.001)  # type: ignore
+        motor.motor_done_move._backend._set_value(1)  # type: ignore
+    return patch.object(
+        motor, "_move", AsyncMock(side_effect=partial(mock_move, motor))
+    )

tests/devices/unit_tests/test_aperture.py

@@ -5,11 +5,12 @@
 
 
 @pytest.fixture
-def fake_aperture():
+async def fake_aperture():
     FakeAperture = make_fake_device(Aperture)
-    fake_aperture: Aperture = FakeAperture(name="aperture")
+    fake_aperture: Aperture = FakeAperture(prefix="test_ap", name="aperture")
+    await fake_aperture.connect(sim=True)
     return fake_aperture
 
 
 def test_aperture_can_be_created(fake_aperture: Aperture):
-    fake_aperture.wait_for_connection()
+    assert isinstance(fake_aperture, Aperture)