From 49c4ab1c4e02c54adb37c14e7853836ba2fe566c Mon Sep 17 00:00:00 2001 From: rtuck99 Date: Mon, 8 Jul 2024 15:26:30 +0100 Subject: [PATCH] 601 convert smargon to ophyd async (#656) * (#601) Initial changes to convert smargon to ophyd_async * (#601) Fixes to remaining unit tests, parameter validation for smargon * (#601) Remove vestigial limit checking features that were never used * Remove I23 gonio * Remove unused XYZLimitBundle and related classes * Remove validate_against_hardware as the original is_valid() method it is based on was never used * (#601) Fix failing dual_backlight unit test, mypy errors * (#601) Minor changes following PR review --- src/dodal/beamlines/i23.py | 12 -- src/dodal/devices/fast_grid_scan.py | 30 ---- src/dodal/devices/i23/__init__.py | 0 src/dodal/devices/i23/gonio.py | 29 ---- src/dodal/devices/motors.py | 48 ------ src/dodal/devices/smargon.py | 150 +++++++++++------ src/dodal/devices/util/epics_util.py | 29 ++-- tests/common/beamlines/test_beamline_utils.py | 9 +- .../system_tests/test_smargon_system.py | 8 +- tests/devices/unit_tests/conftest.py | 23 +++ .../unit_tests/i24/test_dual_backlight.py | 2 +- tests/devices/unit_tests/test_gridscan.py | 153 ++++-------------- tests/devices/unit_tests/test_motors.py | 37 ----- tests/devices/unit_tests/test_oav.py | 34 +--- tests/devices/unit_tests/test_smargon.py | 73 +++++---- tests/devices/unit_tests/test_utils.py | 27 ++-- 16 files changed, 250 insertions(+), 414 deletions(-) delete mode 100644 src/dodal/devices/i23/__init__.py delete mode 100644 src/dodal/devices/i23/gonio.py delete mode 100644 tests/devices/unit_tests/test_motors.py diff --git a/src/dodal/beamlines/i23.py b/src/dodal/beamlines/i23.py index ce2a72d9f0..4cbbf549f6 100644 --- a/src/dodal/beamlines/i23.py +++ b/src/dodal/beamlines/i23.py @@ -1,6 +1,5 @@ from dodal.common.beamlines.beamline_utils import device_instantiation from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline -from dodal.devices.i23.gonio import Gonio from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.log import set_beamline as set_log_beamline from dodal.utils import get_beamline_name, get_hostname, skip_device @@ -19,17 +18,6 @@ def _is_i23_machine(): return hostname.startswith("i23-ws") or hostname.startswith("i23-control") -def gonio(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> Gonio: - """Get the i23 goniometer device""" - return device_instantiation( - Gonio, - "Gonio", - "-MO-GONIO-01:", - wait_for_connection, - fake_with_ophyd_sim, - ) - - @skip_device(lambda: not _is_i23_machine()) def oav_pin_tip_detection( wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False diff --git a/src/dodal/devices/fast_grid_scan.py b/src/dodal/devices/fast_grid_scan.py index 340e48a39c..2e1b2cce50 100644 --- a/src/dodal/devices/fast_grid_scan.py +++ b/src/dodal/devices/fast_grid_scan.py @@ -23,7 +23,6 @@ from pydantic import validator from pydantic.dataclasses import dataclass -from dodal.devices.motors import XYZLimitBundle from dodal.log import LOGGER from dodal.parameters.experiment_parameter_base import AbstractExperimentWithBeamParams @@ -112,35 +111,6 @@ def _get_y_axis(cls, y_axis: GridAxis, values: dict[str, Any]) -> GridAxis: def _get_z_axis(cls, z_axis: GridAxis, values: dict[str, Any]) -> GridAxis: return GridAxis(values["z2_start"], values["z_step_size"], values["z_steps"]) - def is_valid(self, limits: XYZLimitBundle) -> bool: - """ - Validates scan parameters - - :param limits: The motor limits against which to validate - the parameters - :return: True if the scan is valid - """ - x_in_limits = limits.x.is_within(self.x_axis.start) and limits.x.is_within( - self.x_axis.end - ) - y_in_limits = limits.y.is_within(self.y_axis.start) and limits.y.is_within( - self.y_axis.end - ) - - first_grid_in_limits = ( - x_in_limits and y_in_limits and limits.z.is_within(self.z1_start) - ) - - z_in_limits = limits.z.is_within(self.z_axis.start) and limits.z.is_within( - self.z_axis.end - ) - - second_grid_in_limits = ( - x_in_limits and z_in_limits and limits.y.is_within(self.y2_start) - ) - - return first_grid_in_limits and second_grid_in_limits - def get_num_images(self): return self.x_steps * self.y_steps + self.x_steps * self.z_steps diff --git a/src/dodal/devices/i23/__init__.py b/src/dodal/devices/i23/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/dodal/devices/i23/gonio.py b/src/dodal/devices/i23/gonio.py deleted file mode 100644 index 08336401f7..0000000000 --- a/src/dodal/devices/i23/gonio.py +++ /dev/null @@ -1,29 +0,0 @@ -from ophyd import Component as Cpt -from ophyd import EpicsMotor -from ophyd.epics_motor import MotorBundle - -from dodal.devices.motors import MotorLimitHelper, XYZLimitBundle - - -class Gonio(MotorBundle): - x = Cpt(EpicsMotor, "X") - y = Cpt(EpicsMotor, "Y") - z = Cpt(EpicsMotor, "Z") - kappa = Cpt(EpicsMotor, "KAPPA") - phi = Cpt(EpicsMotor, "PHI") - omega = Cpt(EpicsMotor, "OMEGA") - - def get_xyz_limits(self) -> XYZLimitBundle: - """Get the limits for the x, y and z axes. - - Note that these limits may not yet be valid until wait_for_connection is called - on this MotorBundle. - - Returns: - XYZLimitBundle: The limits for the underlying motors. - """ - return XYZLimitBundle( - MotorLimitHelper(self.x), - MotorLimitHelper(self.y), - MotorLimitHelper(self.z), - ) diff --git a/src/dodal/devices/motors.py b/src/dodal/devices/motors.py index 253026d07d..5c9a77c967 100644 --- a/src/dodal/devices/motors.py +++ b/src/dodal/devices/motors.py @@ -1,8 +1,3 @@ -from dataclasses import dataclass -from typing import List, Tuple - -import numpy as np -from ophyd import EpicsMotor from ophyd_async.core import Device from ophyd_async.epics.motion import Motor @@ -13,46 +8,3 @@ def __init__(self, prefix: str, name: str): self.y = Motor(prefix + "Y") self.z = Motor(prefix + "Z") super().__init__(name) - - -@dataclass -class MotorLimitHelper: - """ - Represents motor limit(s) - """ - - motor: EpicsMotor - - def is_within(self, position: float) -> bool: - """Checks position against limits - - :param position: The position to check - :return: True if position is within the limits - """ - low = float(self.motor.low_limit_travel.get()) - high = float(self.motor.high_limit_travel.get()) - return low <= position <= high - - -@dataclass -class XYZLimitBundle: - """ - Holder for limits reflecting an x, y, z bundle - """ - - x: MotorLimitHelper - y: MotorLimitHelper - z: MotorLimitHelper - - def position_valid( - self, position: np.ndarray | List[float] | Tuple[float, float, float] - ): - if len(position) != 3: - raise ValueError( - f"Position valid expects a 3-vector, got {position} instead" - ) - return ( - self.x.is_within(position[0]) - & self.y.is_within(position[1]) - & self.z.is_within(position[2]) - ) diff --git a/src/dodal/devices/smargon.py b/src/dodal/devices/smargon.py index c4ea5c421e..4d5bf14c66 100644 --- a/src/dodal/devices/smargon.py +++ b/src/dodal/devices/smargon.py @@ -1,14 +1,16 @@ +from collections.abc import Generator +from dataclasses import dataclass from enum import Enum +from math import isclose +from typing import Collection, cast -from ophyd import Component as Cpt -from ophyd import Device, EpicsMotor, EpicsSignal -from ophyd.epics_motor import MotorBundle -from ophyd.status import StatusBase +from bluesky import plan_stubs as bps +from bluesky.utils import Msg +from ophyd_async.core import AsyncStatus, Device, StandardReadable, wait_for_value +from ophyd_async.epics.motion import Motor +from ophyd_async.epics.signal import epics_signal_r -from dodal.devices.motors import MotorLimitHelper, XYZLimitBundle -from dodal.devices.status import await_approx_value from dodal.devices.util.epics_util import SetWhenEnabled -from dodal.devices.util.motor_utils import ExtendedEpicsMotor class StubPosition(Enum): @@ -16,6 +18,13 @@ class StubPosition(Enum): RESET_TO_ROBOT_LOAD = 1 +def approx_equal_to(target, deadband: float = 1e-9): + def approx_equal_to_target(value): + return isclose(target, value, rel_tol=0, abs_tol=deadband) + + return approx_equal_to_target + + class StubOffsets(Device): """Stub offsets are used to change the internal co-ordinate system of the smargon by adding an offset to x, y, z. @@ -25,23 +34,64 @@ class StubOffsets(Device): set them so that the current position is zero or to pre-defined positions. """ - parent: "Smargon" + def __init__(self, name: str = "", prefix: str = ""): + self.center_at_current_position = SetWhenEnabled(prefix=prefix + "CENTER_CS") + self.to_robot_load = SetWhenEnabled(prefix=prefix + "SET_STUBS_TO_RL") + super().__init__(name) - center_at_current_position = Cpt(SetWhenEnabled, "CENTER_CS") - to_robot_load = Cpt(SetWhenEnabled, "SET_STUBS_TO_RL") - - def set(self, pos: StubPosition) -> StatusBase: + @AsyncStatus.wrap + async def set(self, pos: StubPosition): if pos == StubPosition.CURRENT_AS_CENTER: - status = self.center_at_current_position.set(1) - status &= await_approx_value(self.parent.x, 0.0, deadband=0.1) - status &= await_approx_value(self.parent.y, 0.0, deadband=0.1) - status &= await_approx_value(self.parent.z, 0.0, deadband=0.1) - return status + await self.center_at_current_position.set(1) + smargon = cast(Smargon, self.parent) + await wait_for_value( + smargon.x.user_readback, approx_equal_to(0.0, 0.1), None + ) + await wait_for_value( + smargon.y.user_readback, approx_equal_to(0.0, 0.1), None + ) + await wait_for_value( + smargon.z.user_readback, approx_equal_to(0.0, 0.1), None + ) else: - return self.to_robot_load.set(1) + await self.to_robot_load.set(1) + + +@dataclass +class AxisLimit: + """Represents the minimum and maximum allowable values on an axis""" + + min_value: float + max_value: float + + def contains(self, pos: float): + """Determine if the specified value is within limits. + + Args: + pos: the value to check + + Returns: + True if the value does not exceed the limits + """ + return self.min_value <= pos <= self.max_value -class Smargon(MotorBundle): +@dataclass +class XYZLimits: + """The limits of the smargon x, y, z axes.""" + + x: AxisLimit + y: AxisLimit + z: AxisLimit + + def position_valid(self, pos: Collection[float]) -> bool: + return all( + axis_limits.contains(value) + for axis_limits, value in zip([self.x, self.y, self.z], pos) + ) + + +class Smargon(StandardReadable): """ Real motors added to allow stops following pin load (e.g. real_x1.stop() ) X1 and X2 real motors provide compound chi motion as well as the compound X travel, @@ -49,35 +99,39 @@ class Smargon(MotorBundle): Robot loading can nudge these and lead to errors. """ - x = Cpt(ExtendedEpicsMotor, "X") - y = Cpt(EpicsMotor, "Y") - z = Cpt(EpicsMotor, "Z") - chi = Cpt(EpicsMotor, "CHI") - phi = Cpt(EpicsMotor, "PHI") - omega = Cpt(ExtendedEpicsMotor, "OMEGA") - - real_x1 = Cpt(EpicsMotor, "MOTOR_3") - real_x2 = Cpt(EpicsMotor, "MOTOR_4") - real_y = Cpt(EpicsMotor, "MOTOR_1") - real_z = Cpt(EpicsMotor, "MOTOR_2") - real_phi = Cpt(EpicsMotor, "MOTOR_5") - real_chi = Cpt(EpicsMotor, "MOTOR_6") - - stub_offsets = Cpt(StubOffsets, "") - - disabled = Cpt(EpicsSignal, "DISABLED") - - def get_xyz_limits(self) -> XYZLimitBundle: - """Get the limits for the x, y and z axes. - - Note that these limits may not yet be valid until wait_for_connection is called - on this MotorBundle. + def __init__(self, prefix: str = "", name: str = ""): + with self.add_children_as_readables(): + self.x = Motor(prefix + "X") + self.y = Motor(prefix + "Y") + self.z = Motor(prefix + "Z") + self.chi = Motor(prefix + "CHI") + self.phi = Motor(prefix + "PHI") + self.omega = Motor(prefix + "OMEGA") + self.real_x1 = Motor(prefix + "MOTOR_3") + self.real_x2 = Motor(prefix + "MOTOR_4") + self.real_y = Motor(prefix + "MOTOR_1") + self.real_z = Motor(prefix + "MOTOR_2") + self.real_phi = Motor(prefix + "MOTOR_5") + self.real_chi = Motor(prefix + "MOTOR_6") + self.stub_offsets = StubOffsets(prefix=prefix) + self.disabled = epics_signal_r(int, prefix + "DISABLED") + + super().__init__(name) + + def get_xyz_limits(self) -> Generator[Msg, None, XYZLimits]: + """Obtain a plan stub that returns the smargon XYZ axis limits + + Yields: + Bluesky messages Returns: - XYZLimitBundle: The limits for the underlying motors. + the axis limits """ - return XYZLimitBundle( - MotorLimitHelper(self.x), - MotorLimitHelper(self.y), - MotorLimitHelper(self.z), - ) + limits = {} + for name, pv in [ + (attr_name, getattr(self, attr_name)) for attr_name in ["x", "y", "z"] + ]: + min_value = yield from bps.rd(pv.low_limit_travel) + max_value = yield from bps.rd(pv.high_limit_travel) + limits[name] = AxisLimit(min_value, max_value) + return XYZLimits(**limits) diff --git a/src/dodal/devices/util/epics_util.py b/src/dodal/devices/util/epics_util.py index 083460b9fd..2da39c59a8 100644 --- a/src/dodal/devices/util/epics_util.py +++ b/src/dodal/devices/util/epics_util.py @@ -1,10 +1,14 @@ from functools import partial from typing import Callable -from ophyd import Component, Device, EpicsSignal +from bluesky.protocols import Movable +from ophyd import Component, EpicsSignal +from ophyd import Device as OphydDevice from ophyd.status import Status, StatusBase +from ophyd_async.core import AsyncStatus, wait_for_value +from ophyd_async.core import Device as OphydAsyncDevice +from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw -from dodal.devices.status import await_value from dodal.log import LOGGER @@ -24,7 +28,7 @@ def epics_signal_put_wait(pv_name: str, wait: float = 3.0) -> Component[EpicsSig def run_functions_without_blocking( functions_to_chain: list[Callable[[], StatusBase]], timeout: float = 60.0, - associated_obj: Device | None = None, + associated_obj: OphydDevice | None = None, ) -> Status: """Creates and initiates an asynchronous chaining of functions which return a status @@ -112,16 +116,15 @@ def call_func(func: Callable[[], StatusBase]) -> StatusBase: return func() -class SetWhenEnabled(Device): +class SetWhenEnabled(OphydAsyncDevice, Movable): """A device that sets the proc field of a PV when it becomes enabled.""" - proc = Component(EpicsSignal, ".PROC") - disp = Component(EpicsSignal, ".DISP") + def __init__(self, name: str = "", prefix: str = ""): + self.proc = epics_signal_rw(int, prefix + ".PROC") + self.disp = epics_signal_r(int, prefix + ".DISP") + super().__init__(name) - def set(self, proc: int) -> Status: - return run_functions_without_blocking( - [ - lambda: await_value(self.disp, 0), - lambda: self.proc.set(proc), - ] - ) + @AsyncStatus.wrap + async def set(self, value: int): + await wait_for_value(self.disp, 0, None) + await self.proc.set(value) diff --git a/tests/common/beamlines/test_beamline_utils.py b/tests/common/beamlines/test_beamline_utils.py index 71400355a1..1113a16e11 100644 --- a/tests/common/beamlines/test_beamline_utils.py +++ b/tests/common/beamlines/test_beamline_utils.py @@ -11,6 +11,7 @@ from dodal.beamlines import i03 from dodal.common.beamlines import beamline_utils from dodal.devices.aperturescatterguard import ApertureScatterguard +from dodal.devices.qbpm1 import QBPM1 from dodal.devices.smargon import Smargon from dodal.devices.zebra import Zebra from dodal.utils import make_all_devices @@ -52,11 +53,11 @@ def test_instantiating_different_device_with_same_name(): def test_instantiate_v1_function_fake_makes_fake(): - smargon: Smargon = beamline_utils.device_instantiation( - i03.Smargon, "smargon", "", True, True, None + qbpm: QBPM1 = beamline_utils.device_instantiation( + QBPM1, "qbpm", "", True, True, None ) - assert isinstance(smargon, Device) - assert isinstance(smargon.disabled, FakeEpicsSignal) + assert isinstance(qbpm, Device) + assert isinstance(qbpm.intensity, FakeEpicsSignal) def test_instantiate_v2_function_fake_makes_fake(): diff --git a/tests/devices/system_tests/test_smargon_system.py b/tests/devices/system_tests/test_smargon_system.py index 1df6a276fb..9996407f14 100644 --- a/tests/devices/system_tests/test_smargon_system.py +++ b/tests/devices/system_tests/test_smargon_system.py @@ -1,10 +1,14 @@ +from os import environ + import pytest from dodal.devices.smargon import Smargon @pytest.mark.s03 -def test_when_smargon_created_against_s03_then_can_connect(): +@pytest.mark.skip(reason="S03 and I03 smargon DISABLED types differ") +async def test_when_smargon_created_against_s03_then_can_connect(): + print(f"EPICS_CA_REPEATER_PORT is {environ['EPICS_CA_REPEATER_PORT']}") smargon = Smargon("BL03S-MO-SGON-01:", name="smargon") - smargon.wait_for_connection() + await smargon.connect() diff --git a/tests/devices/unit_tests/conftest.py b/tests/devices/unit_tests/conftest.py index be6a7179d3..3f6f1d82d4 100644 --- a/tests/devices/unit_tests/conftest.py +++ b/tests/devices/unit_tests/conftest.py @@ -1,3 +1,4 @@ +from functools import partial from pathlib import Path import pytest @@ -6,10 +7,14 @@ DirectoryProvider, StaticDirectoryProvider, callback_on_mock_put, + get_mock_put, set_mock_value, ) from ophyd_async.epics.motion import Motor +from dodal.beamlines import i03 +from dodal.common.beamlines.beamline_utils import clear_devices + DIRECTORY_INFO_FOR_TESTING: DirectoryInfo = DirectoryInfo( root=Path("/does/not/exist"), resource_dir=Path("/on/this/filesystem"), @@ -31,3 +36,21 @@ def patch_motor(motor: Motor, initial_position=0): @pytest.fixture def static_directory_provider(tmp_path: Path) -> DirectoryProvider: return StaticDirectoryProvider(tmp_path) + + +@pytest.fixture +def smargon(): + smargon = i03.smargon(fake_with_ophyd_sim=True) + + def mock_set(motor, value, *args, **kwargs): + set_mock_value(motor.user_readback, value) + + def patch_motor(motor): + get_mock_put(motor.user_setpoint).side_effect = partial(mock_set, motor) + + for motor in [smargon.omega, smargon.x, smargon.y, smargon.z]: + patch_motor(motor) + + yield smargon + + clear_devices() diff --git a/tests/devices/unit_tests/i24/test_dual_backlight.py b/tests/devices/unit_tests/i24/test_dual_backlight.py index 097c953c53..9101b2de7c 100644 --- a/tests/devices/unit_tests/i24/test_dual_backlight.py +++ b/tests/devices/unit_tests/i24/test_dual_backlight.py @@ -52,7 +52,7 @@ async def test_when_backlight_out_it_switches_off( async def test_when_backlight_not_out_it_switches_on( fake_backlight: DualBacklight, RE: RunEngine ): - RE(bps.abs_set(fake_backlight, "OAV2")) + RE(bps.abs_set(fake_backlight, "OAV2", wait=True)) assert await fake_backlight.backlight_state.get_value() == "ON" diff --git a/tests/devices/unit_tests/test_gridscan.py b/tests/devices/unit_tests/test_gridscan.py index 268c255419..f69d829488 100644 --- a/tests/devices/unit_tests/test_gridscan.py +++ b/tests/devices/unit_tests/test_gridscan.py @@ -1,11 +1,11 @@ from asyncio import wait_for +from dataclasses import dataclass import numpy as np import pytest from bluesky import plan_stubs as bps from bluesky import preprocessors as bpp from bluesky.run_engine import RunEngine -from ophyd.sim import make_fake_device from ophyd.status import DeviceStatus, Status from ophyd_async.core import DeviceCollector, get_mock_put, set_mock_value @@ -152,18 +152,27 @@ async def test_running_finished_with_all_images_done_then_complete_status_finish assert complete_status.exception() is None -def create_motor_bundle_with_limits(low_limit, high_limit) -> Smargon: - FakeSmargon = make_fake_device(Smargon) - grid_scan_motor_bundle: Smargon = FakeSmargon(name="test fake Smargon") - grid_scan_motor_bundle.wait_for_connection() +@pytest.fixture +def motor_bundle_with_limits(smargon) -> Smargon: for axis in [ - grid_scan_motor_bundle.x, - grid_scan_motor_bundle.y, - grid_scan_motor_bundle.z, + smargon.x, + smargon.y, + smargon.z, ]: - axis.low_limit_travel.sim_put(low_limit) # type: ignore - axis.high_limit_travel.sim_put(high_limit) # type: ignore - return grid_scan_motor_bundle + set_mock_value(axis.low_limit_travel, 0) + set_mock_value(axis.high_limit_travel, 10) + + return smargon + + +@dataclass +class CompositeWithSmargon: + smargon: Smargon + + +@pytest.fixture +def composite_with_smargon(motor_bundle_with_limits): + return CompositeWithSmargon(motor_bundle_with_limits) @pytest.mark.parametrize( @@ -174,9 +183,14 @@ def create_motor_bundle_with_limits(low_limit, high_limit) -> Smargon: (5, True), ], ) -def test_within_limits_check(position, expected_in_limit): - limits = create_motor_bundle_with_limits(0.0, 10).get_xyz_limits() - assert limits.x.is_within(position) == expected_in_limit +async def test_within_limits_check( + RE: RunEngine, motor_bundle_with_limits, position, expected_in_limit +): + def check_limits_plan(): + limits = yield from motor_bundle_with_limits.get_xyz_limits() + assert limits.x.contains(position) == expected_in_limit + + RE(check_limits_plan()) PASSING_LINE_1 = (1, 5, 1) @@ -186,111 +200,12 @@ def test_within_limits_check(position, expected_in_limit): FAILING_CONST = 15 -@pytest.mark.parametrize( - "start, steps, size, expected_in_limits", - [ - (*PASSING_LINE_1, True), - (*PASSING_LINE_2, True), - (*FAILING_LINE_1, False), - (-1, 5, 1, False), - (-1, 10, 2, False), - (0, 10, 0.1, True), - (5, 10, 0.5, True), - (5, 20, 0.6, False), - ], -) -def test_scan_within_limits_1d(start, steps, size, expected_in_limits): - motor_bundle = create_motor_bundle_with_limits(0.0, 10.0) - grid_params = ZebraGridScanParams( - transmission_fraction=0.01, x_start=start, x_steps=steps, x_step_size=size - ) - assert grid_params.is_valid(motor_bundle.get_xyz_limits()) == expected_in_limits - - -@pytest.mark.parametrize( - "x_start, x_steps, x_size, y1_start, y_steps, y_size, z1_start, expected_in_limits", - [ - (*PASSING_LINE_1, *PASSING_LINE_2, PASSING_CONST, True), - (*PASSING_LINE_1, *FAILING_LINE_1, PASSING_CONST, False), - (*PASSING_LINE_1, *PASSING_LINE_2, FAILING_CONST, False), - ], -) -def test_scan_within_limits_2d( - x_start, x_steps, x_size, y1_start, y_steps, y_size, z1_start, expected_in_limits -): - motor_bundle = create_motor_bundle_with_limits(0.0, 10.0) - grid_params = ZebraGridScanParams( - transmission_fraction=0.01, - x_start=x_start, - x_steps=x_steps, - x_step_size=x_size, - y1_start=y1_start, - y_steps=y_steps, - y_step_size=y_size, - z1_start=z1_start, - ) - assert grid_params.is_valid(motor_bundle.get_xyz_limits()) == expected_in_limits - - -@pytest.mark.parametrize( - "x_start, x_steps, x_size, y1_start, y_steps, y_size, z1_start, z2_start, z_steps, z_size, y2_start, expected_in_limits", - [ - ( - *PASSING_LINE_1, - *PASSING_LINE_2, - PASSING_CONST, - *PASSING_LINE_1, - PASSING_CONST, - True, - ), - ( - *PASSING_LINE_1, - *PASSING_LINE_2, - PASSING_CONST, - *PASSING_LINE_1, - FAILING_CONST, - False, - ), - ( - *PASSING_LINE_1, - *PASSING_LINE_2, - PASSING_CONST, - *FAILING_LINE_1, - PASSING_CONST, - False, - ), - ], -) -def test_scan_within_limits_3d( - x_start, - x_steps, - x_size, - y1_start, - y_steps, - y_size, - z1_start, - z2_start, - z_steps, - z_size, - y2_start, - expected_in_limits, -): - motor_bundle = create_motor_bundle_with_limits(0.0, 10.0) - grid_params = ZebraGridScanParams( - transmission_fraction=0.01, - x_start=x_start, - x_steps=x_steps, - x_step_size=x_size, - y1_start=y1_start, - y_steps=y_steps, - y_step_size=y_size, - z1_start=z1_start, - z2_start=z2_start, - z_steps=z_steps, - z_step_size=z_size, - y2_start=y2_start, - ) - assert grid_params.is_valid(motor_bundle.get_xyz_limits()) == expected_in_limits +def check_parameter_validation(params, composite, expected_in_limits): + if expected_in_limits: + yield from params.validate_against_hardware(composite) + else: + with pytest.raises(ValueError): + yield from params.validate_against_hardware(composite) @pytest.fixture diff --git a/tests/devices/unit_tests/test_motors.py b/tests/devices/unit_tests/test_motors.py deleted file mode 100644 index f1576af5f6..0000000000 --- a/tests/devices/unit_tests/test_motors.py +++ /dev/null @@ -1,37 +0,0 @@ -from unittest.mock import MagicMock - -import pytest - -from dodal.devices.motors import XYZLimitBundle - - -def test_given_position_in_limits_then_position_valid_returns_true(): - mock_limits = MagicMock(), MagicMock(), MagicMock() - bundle = XYZLimitBundle(*mock_limits) - for mock in mock_limits: - mock.is_within.return_value = True - - assert bundle.position_valid([0, 0, 0]) - - -@pytest.mark.parametrize( - "axes", - [[1], [2], [0, 2], [0, 1, 2]], -) -def test_given_axes_not_in_limits_then_position_valid_returns_false(axes): - mock_limits = MagicMock(), MagicMock(), MagicMock() - bundle = XYZLimitBundle(*mock_limits) - for mock in mock_limits: - mock.is_within.return_value = True - for axis in axes: - mock_limits[axis].is_within.return_value = False - - assert not bundle.position_valid([0, 0, 0]) - - -def test_when_position_valid_called_without_3_vector_the_raises(): - mock_limits = MagicMock(), MagicMock(), MagicMock() - bundle = XYZLimitBundle(*mock_limits) - - with pytest.raises(ValueError): - raise bundle.position_valid([0, 0, 0, 0]) diff --git a/tests/devices/unit_tests/test_oav.py b/tests/devices/unit_tests/test_oav.py index 8147c0eafc..ae42fe5a51 100644 --- a/tests/devices/unit_tests/test_oav.py +++ b/tests/devices/unit_tests/test_oav.py @@ -1,4 +1,3 @@ -from functools import partial from unittest.mock import AsyncMock, MagicMock, call, patch import numpy as np @@ -6,12 +5,11 @@ from bluesky import plan_stubs as bps from bluesky.run_engine import RunEngine from ophyd.sim import instantiate_fake_device, make_fake_device -from ophyd.status import Status +from ophyd_async.core import set_mock_value from PIL import Image from requests import HTTPError, Response import dodal.devices.oav.utils as oav_utils -from dodal.beamlines import i03 from dodal.devices.oav.oav_detector import OAV, OAVConfigParams from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.devices.oav.pin_image_recognition.utils import SampleLocation @@ -26,34 +24,6 @@ ZOOM_LEVELS_XML = "tests/devices/unit_tests/test_jCameraManZoomLevels.xml" -def fake_smargon() -> Smargon: - 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 - - def mock_set(motor, val): - motor.user_readback.sim_put(val) # type: ignore - return Status(done=True, success=True) - - def patch_motor(motor): - return patch.object(motor, "set", partial(mock_set, motor)) - - with ( - patch_motor(smargon.omega), - patch_motor(smargon.x), - patch_motor(smargon.y), - patch_motor(smargon.z), - ): - return smargon - - -@pytest.fixture -def smargon(): - yield fake_smargon() - - @pytest.fixture def fake_oav() -> OAV: oav_params = OAVConfigParams(ZOOM_LEVELS_XML, DISPLAY_CONFIGURATION) @@ -271,7 +241,7 @@ def test_values_for_move_so_that_beam_is_at_pixel( fake_oav.parameters.beam_centre_i = beam_centre[0] fake_oav.parameters.beam_centre_j = beam_centre[1] - smargon.omega.user_readback.sim_put(angle) # type: ignore + set_mock_value(smargon.omega.user_readback, angle) RE = RunEngine(call_returns_result=True) pos = RE( diff --git a/tests/devices/unit_tests/test_smargon.py b/tests/devices/unit_tests/test_smargon.py index 628acd2abc..d65aca29e1 100644 --- a/tests/devices/unit_tests/test_smargon.py +++ b/tests/devices/unit_tests/test_smargon.py @@ -1,66 +1,83 @@ -from time import sleep from typing import Tuple import pytest -from ophyd.sim import make_fake_device +from bluesky import RunEngine +from bluesky import plan_stubs as bps +from ophyd_async.core import DeviceCollector, observe_value, set_mock_value from dodal.devices.smargon import Smargon, StubPosition @pytest.fixture -def smargon() -> Smargon: - return make_fake_device(Smargon)(name="smargon") +async def smargon() -> Smargon: + async with DeviceCollector(mock=True): + smargon = Smargon(name="smargon") + return smargon def set_smargon_pos(smargon: Smargon, pos: Tuple[float, float, float]): - smargon.x.user_readback.sim_put(pos[0]) # type: ignore - smargon.y.user_readback.sim_put(pos[1]) # type: ignore - smargon.z.user_readback.sim_put(pos[2]) # type: ignore + set_mock_value(smargon.x.user_readback, pos[0]) + set_mock_value(smargon.y.user_readback, pos[1]) + set_mock_value(smargon.z.user_readback, pos[2]) -def test_given_to_robot_disp_low_when_stub_offsets_set_to_robot_load_then_proc_set( +async def test_given_to_robot_disp_low_when_stub_offsets_set_to_robot_load_then_proc_set( + RE: RunEngine, smargon: Smargon, ): - smargon.stub_offsets.to_robot_load.disp.sim_put(0) # type: ignore + set_mock_value(smargon.stub_offsets.to_robot_load.disp, 0) - status = smargon.stub_offsets.set(StubPosition.RESET_TO_ROBOT_LOAD) - status.wait() + def plan(): + yield from bps.abs_set( + smargon.stub_offsets, StubPosition.RESET_TO_ROBOT_LOAD, wait=True + ) + robot_load_proc = yield from bps.rd(smargon.stub_offsets.to_robot_load.proc) + assert robot_load_proc == 1 + current_pos_proc = yield from bps.rd( + smargon.stub_offsets.center_at_current_position.proc + ) + assert current_pos_proc == 0 - assert smargon.stub_offsets.to_robot_load.proc.get() == 1 - assert smargon.stub_offsets.center_at_current_position.proc.get() == 0 + RE(plan()) -def test_given_center_disp_low_and_at_centre_when_stub_offsets_set_to_center_then_proc_set( +async def test_given_center_disp_low_and_at_centre_when_stub_offsets_set_to_center_then_proc_set( + RE: RunEngine, smargon: Smargon, ): - smargon.stub_offsets.center_at_current_position.disp.sim_put(0) # type: ignore + set_mock_value(smargon.stub_offsets.center_at_current_position.disp, 0) set_smargon_pos(smargon, (0, 0, 0)) - status = smargon.stub_offsets.set(StubPosition.CURRENT_AS_CENTER) - status.wait() + def plan(): + yield from bps.abs_set( + smargon.stub_offsets, StubPosition.CURRENT_AS_CENTER, wait=True + ) + assert (yield from bps.rd(smargon.stub_offsets.to_robot_load.proc)) == 0 + assert ( + yield from bps.rd(smargon.stub_offsets.center_at_current_position.proc) + ) == 1 - assert smargon.stub_offsets.to_robot_load.proc.get() == 0 - assert smargon.stub_offsets.center_at_current_position.proc.get() == 1 + RE(plan()) -def test_given_center_disp_low_when_stub_offsets_set_to_center_and_moved_to_0_0_0_then_proc_set( +async def test_given_center_disp_low_when_stub_offsets_set_to_center_and_moved_to_0_0_0_then_proc_set( smargon: Smargon, ): - smargon.stub_offsets.center_at_current_position.disp.sim_put(0) # type: ignore + set_mock_value(smargon.stub_offsets.center_at_current_position.disp, 0) set_smargon_pos(smargon, (1.5, 0.5, 3.4)) - status = smargon.stub_offsets.set(StubPosition.CURRENT_AS_CENTER) + current_proc_values = observe_value( + smargon.stub_offsets.center_at_current_position.proc + ) + assert await anext(current_proc_values) == 0 - sleep(0.01) + status = smargon.stub_offsets.set(StubPosition.CURRENT_AS_CENTER) - assert smargon.stub_offsets.center_at_current_position.proc.get() == 1 + assert await anext(current_proc_values) == 1 assert not status.done set_smargon_pos(smargon, (0, 0, 0)) - status.wait() - - assert smargon.stub_offsets.to_robot_load.proc.get() == 0 - assert smargon.stub_offsets.center_at_current_position.proc.get() == 1 + assert await smargon.stub_offsets.to_robot_load.proc.get_value() == 0 diff --git a/tests/devices/unit_tests/test_utils.py b/tests/devices/unit_tests/test_utils.py index 4d2f78440c..55b0359a6e 100644 --- a/tests/devices/unit_tests/test_utils.py +++ b/tests/devices/unit_tests/test_utils.py @@ -2,9 +2,10 @@ from unittest.mock import MagicMock, patch import pytest -from ophyd.sim import NullStatus, make_fake_device +from ophyd.sim import NullStatus from ophyd.status import Status from ophyd.utils.errors import StatusTimeoutError, WaitTimeoutError +from ophyd_async.core import AsyncStatus, get_mock_put, set_mock_value from dodal.devices.util.epics_util import SetWhenEnabled, run_functions_without_blocking from dodal.log import LOGGER, GELFTCPHandler, logging, set_up_all_logging_handlers @@ -108,16 +109,20 @@ def test_status_points_to_provided_device_object(): assert returned_status.obj == expected_obj -def test_given_disp_high_when_set_SetWhenEnabled_then_proc_not_set_until_disp_low(): - signal: SetWhenEnabled = make_fake_device(SetWhenEnabled)(name="test") - signal.disp.sim_put(1) # type: ignore - signal.proc.set = MagicMock(return_value=Status(done=True, success=True)) - - status = signal.set(1) - signal.proc.set.assert_not_called() - signal.disp.sim_put(0) # type: ignore - status.wait() - signal.proc.set.assert_called_once() +async def test_given_disp_high_when_set_SetWhenEnabled_then_proc_not_set_until_disp_low(): + device: SetWhenEnabled = SetWhenEnabled(name="test") + await device.connect(True) + set_mock_value(device.disp, 1) + proc_mock = get_mock_put(device.proc) + proc_mock.return_value = NullStatus() + status: AsyncStatus = device.set(1) + assert not status.done + proc_mock.assert_not_called() + set_mock_value(device.disp, 0) + + await status + proc_mock.assert_called_once() + assert status.success def test_if_one_status_errors_then_later_functions_not_called():