From e4e51225ac944b56ac9cbada80c3351cd026c6b1 Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Wed, 20 Mar 2024 15:41:04 +0000 Subject: [PATCH 01/25] ophyd-async aperture and scatterguard first attempt --- src/dodal/devices/aperture.py | 21 +++++++++++---------- src/dodal/devices/scatterguard.py | 14 ++++++++------ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/dodal/devices/aperture.py b/src/dodal/devices/aperture.py index fa2b0d1d8d..7852b2276a 100644 --- a/src/dodal/devices/aperture.py +++ b/src/dodal/devices/aperture.py @@ -1,12 +1,13 @@ -from ophyd import Component, Device, EpicsSignalRO +from ophyd_async.core import StandardReadable +from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw -from dodal.devices.util.motor_utils import ExtendedEpicsMotor - -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 = epics_signal_rw(float, prefix + "X") + self.y = epics_signal_rw(float, prefix + "Y") + self.z = epics_signal_rw(float, 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) diff --git a/src/dodal/devices/scatterguard.py b/src/dodal/devices/scatterguard.py index b29b148bd7..b2a087ec6d 100644 --- a/src/dodal/devices/scatterguard.py +++ b/src/dodal/devices/scatterguard.py @@ -1,9 +1,11 @@ -from ophyd import Component as Cpt -from ophyd import Device +from ophyd_async.core import StandardReadable +from ophyd_async.epics.signal import epics_signal_rw -from dodal.devices.util.motor_utils import ExtendedEpicsMotor +class Scatterguard(StandardReadable): + """The scatterguard device.""" -class Scatterguard(Device): - x = Cpt(ExtendedEpicsMotor, "X") - y = Cpt(ExtendedEpicsMotor, "Y") + def __init__(self, prefix: str, name: str = "") -> None: + self.x = epics_signal_rw(float, prefix + "X") + self.y = epics_signal_rw(float, prefix + "Y") + super().__init__(name) From adaaf77f304b45a5cd431e8ff446062104f16c12 Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Tue, 26 Mar 2024 14:05:11 +0000 Subject: [PATCH 02/25] WIP help.. --- src/dodal/devices/aperture.py | 4 +- src/dodal/devices/aperturescatterguard.py | 41 ++--- src/dodal/devices/util/motor_utils.py | 9 ++ .../unit_tests/test_aperture_scatterguard.py | 149 +++++++++--------- 4 files changed, 110 insertions(+), 93 deletions(-) diff --git a/src/dodal/devices/aperture.py b/src/dodal/devices/aperture.py index 7852b2276a..ab976b4c25 100644 --- a/src/dodal/devices/aperture.py +++ b/src/dodal/devices/aperture.py @@ -1,11 +1,13 @@ from ophyd_async.core import StandardReadable from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw +from dodal.devices.util.motor_utils import ExtendedMotor + class Aperture(StandardReadable): def __init__(self, prefix: str, name: str = "") -> None: self.x = epics_signal_rw(float, prefix + "X") - self.y = epics_signal_rw(float, prefix + "Y") + self.y = ExtendedMotor(prefix + "Y") self.z = epics_signal_rw(float, prefix + "Z") self.small = epics_signal_r(int, prefix + "Y:SMALL_CALC") self.medium = epics_signal_r(int, prefix + "Y:MEDIUM_CALC") diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 66cf56e459..cfc331cf5a 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -4,14 +4,13 @@ from functools import reduce from typing import List, Optional, Sequence -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.epics.motion import Motor from dodal.devices.aperture import Aperture -from dodal.devices.logging_ophyd_device import InfoLoggingDevice from dodal.devices.scatterguard import Scatterguard +from dodal.devices.util.motor_utils import ExtendedMotor from dodal.log import LOGGER @@ -83,18 +82,20 @@ def as_list(self) -> List[SingleAperturePosition]: ] -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 + super().__init__(name) - class SelectedAperture(SignalRO): + class SelectedAperture(SignalR): def get(self): assert isinstance(self.parent, ApertureScatterguard) return self.parent._get_current_aperture_position() - selected_aperture = Cpt(SelectedAperture) + selected_aperture = SelectedAperture() # look at software devices def load_aperture_positions(self, positions: AperturePositions): LOGGER.info(f"{self.name} loaded in {positions}") @@ -117,7 +118,7 @@ def _get_motor_list(self): ] def _set_raw_unsafe(self, positions: ApertureFiveDimensionalLocation) -> AndStatus: - motors: Sequence[EpicsMotor] = self._get_motor_list() + motors: Sequence[ExtendedMotor] = self._get_motor_list() return reduce( operator.and_, [motor.set(pos) for motor, pos in zip(motors, positions)] ) @@ -130,14 +131,16 @@ def _get_current_aperture_position(self) -> SingleAperturePosition: 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 = self.aperture.y.get_value() 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 * self.aperture.y.motor_resolution.get) + # extendedepicsmotor class has tolerance fields added + # ophyd-async epics motor may need to do the same thing - epics motor + if self.aperture.large.get_value() == 1: return self.aperture_positions.LARGE - elif int(self.aperture.medium.get()) == 1: + elif self.aperture.medium.get_value() == 1: return self.aperture_positions.MEDIUM - elif int(self.aperture.small.get()) == 1: + elif self.aperture.small.get_value() == 1: return self.aperture_positions.SMALL elif current_ap_y <= robot_load_ap_y + tolerance: return self.aperture_positions.ROBOT_LOAD @@ -170,7 +173,7 @@ def _safe_move_within_datacollection_range( ) return status - current_ap_z = self.aperture.z.user_setpoint.get() + current_ap_z = self.aperture.z.read() tolerance = self.TOLERANCE_STEPS * self.aperture.z.motor_resolution.get() diff_on_z = abs(current_ap_z - aperture_z) if diff_on_z > tolerance: @@ -180,7 +183,7 @@ def _safe_move_within_datacollection_range( 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 = self.aperture.y.read() if aperture_y > current_ap_y: sg_status: AndStatus = self.scatterguard.x.set( scatterguard_x diff --git a/src/dodal/devices/util/motor_utils.py b/src/dodal/devices/util/motor_utils.py index 07638ba3f6..d67aa47594 100644 --- a/src/dodal/devices/util/motor_utils.py +++ b/src/dodal/devices/util/motor_utils.py @@ -1,6 +1,15 @@ 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") + super().__init__(name) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index b119571062..621d6d27f1 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -1,9 +1,11 @@ -from unittest.mock import MagicMock, call +# from unittest.mock import MagicMock, call import pytest from ophyd.sim import make_fake_device from ophyd.status import StatusBase +# from ophyd_async.core import StandardReadable +# from ophyd_async.core.sim_signal_backend import SimSignalBackend from dodal.devices.aperturescatterguard import ( ApertureFiveDimensionalLocation, AperturePositions, @@ -83,70 +85,70 @@ def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): ) -def test_aperture_scatterguard_select_bottom_moves_sg_down_then_assembly_up( - aperture_positions: AperturePositions, - aperture_in_medium_pos: ApertureScatterguard, -): - aperture_scatterguard = aperture_in_medium_pos - call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) - - aperture_scatterguard.set(aperture_positions.SMALL) - - call_logger.assert_has_calls( - [ - call._mock_sg_x(5.3375), - call._mock_sg_y(-3.55), - call._mock_ap_x(2.43), - call._mock_ap_y(48.974), - call._mock_ap_z(15.8), - ] - ) - - -def test_aperture_unsafe_move( - aperture_positions: AperturePositions, - aperture_in_medium_pos: ApertureScatterguard, -): - (a, b, c, d, e) = (0.2, 3.4, 5.6, 7.8, 9.0) - aperture_scatterguard = aperture_in_medium_pos - call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) - aperture_scatterguard._set_raw_unsafe((a, b, c, d, e)) # type: ignore - - call_logger.assert_has_calls( - [ - call._mock_ap_x(a), - call._mock_ap_y(b), - call._mock_ap_z(c), - call._mock_sg_x(d), - call._mock_sg_y(e), - ] - ) - - -def test_aperture_scatterguard_select_top_moves_assembly_down_then_sg_up( - aperture_positions: AperturePositions, aperture_in_medium_pos: ApertureScatterguard -): - aperture_scatterguard = aperture_in_medium_pos - call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) - - aperture_scatterguard.set(aperture_positions.LARGE) - - call_logger.assert_has_calls( - [ - call._mock_ap_x(2.389), - call._mock_ap_y(40.986), - call._mock_ap_z(15.8), - call._mock_sg_x(5.25), - call._mock_sg_y(4.43), - ] - ) +# def test_aperture_scatterguard_select_bottom_moves_sg_down_then_assembly_up( +# aperture_positions: AperturePositions, +# aperture_in_medium_pos: ApertureScatterguard, +# ): +# aperture_scatterguard = aperture_in_medium_pos +# call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) + +# aperture_scatterguard.set(aperture_positions.SMALL) + +# call_logger.assert_has_calls( +# [ +# call._mock_sg_x(5.3375), +# call._mock_sg_y(-3.55), +# call._mock_ap_x(2.43), +# call._mock_ap_y(48.974), +# call._mock_ap_z(15.8), +# ] +# ) + + +# def test_aperture_unsafe_move( +# aperture_positions: AperturePositions, +# aperture_in_medium_pos: ApertureScatterguard, +# ): +# (a, b, c, d, e) = (0.2, 3.4, 5.6, 7.8, 9.0) +# aperture_scatterguard = aperture_in_medium_pos +# call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) +# aperture_scatterguard._set_raw_unsafe((a, b, c, d, e)) # type: ignore + +# call_logger.assert_has_calls( +# [ +# call._mock_ap_x(a), +# call._mock_ap_y(b), +# call._mock_ap_z(c), +# call._mock_sg_x(d), +# call._mock_sg_y(e), +# ] +# ) + + +# def test_aperture_scatterguard_select_top_moves_assembly_down_then_sg_up( +# aperture_positions: AperturePositions, aperture_in_medium_pos: ApertureScatterguard +# ): +# aperture_scatterguard = aperture_in_medium_pos +# call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) + +# aperture_scatterguard.set(aperture_positions.LARGE) + +# call_logger.assert_has_calls( +# [ +# call._mock_ap_x(2.389), +# call._mock_ap_y(40.986), +# call._mock_ap_z(15.8), +# call._mock_sg_x(5.25), +# call._mock_sg_y(4.43), +# ] +# ) def test_aperture_scatterguard_throws_error_if_outside_tolerance( ap_sg: ApertureScatterguard, ): - ap_sg.aperture.z.motor_resolution.sim_put(0.001) # type: ignore - ap_sg.aperture.z.user_setpoint.sim_put(1) # type: ignore + ap_sg.aperture.z.motor_resolution._set_value(0.001) # type: ignore + ap_sg.aperture.z._set_value(1) # type: ignore ap_sg.aperture.z.motor_done_move.sim_put(1) # type: ignore with pytest.raises(InvalidApertureMove): @@ -265,16 +267,17 @@ def test_when_aperture_set_and_device_read_then_position_returned( ) -def install_logger_for_aperture_and_scatterguard(aperture_scatterguard): - parent_mock = MagicMock() - mock_ap_x = aperture_scatterguard.aperture.x.set - mock_ap_y = aperture_scatterguard.aperture.y.set - mock_ap_z = aperture_scatterguard.aperture.z.set - mock_sg_x = aperture_scatterguard.scatterguard.x.set - mock_sg_y = aperture_scatterguard.scatterguard.y.set - parent_mock.attach_mock(mock_ap_x, "_mock_ap_x") - parent_mock.attach_mock(mock_ap_y, "_mock_ap_y") - parent_mock.attach_mock(mock_ap_z, "_mock_ap_z") - parent_mock.attach_mock(mock_sg_x, "_mock_sg_x") - parent_mock.attach_mock(mock_sg_y, "_mock_sg_y") - return parent_mock +# ensure movements happen correctly to avoid collision - find another way +# def install_logger_for_aperture_and_scatterguard(aperture_scatterguard): +# parent_mock = MagicMock() +# mock_ap_x = aperture_scatterguard.aperture.x.set +# mock_ap_y = aperture_scatterguard.aperture.y.set +# mock_ap_z = aperture_scatterguard.aperture.z.set +# mock_sg_x = aperture_scatterguard.scatterguard.x.set +# mock_sg_y = aperture_scatterguard.scatterguard.y.set +# parent_mock.attach_mock(mock_ap_x, "_mock_ap_x") +# parent_mock.attach_mock(mock_ap_y, "_mock_ap_y") +# parent_mock.attach_mock(mock_ap_z, "_mock_ap_z") +# parent_mock.attach_mock(mock_sg_x, "_mock_sg_x") +# parent_mock.attach_mock(mock_sg_y, "_mock_sg_y") +# return parent_mock From 7b376bec440983b3af12dfd76deb9a7eaa407a35 Mon Sep 17 00:00:00 2001 From: David Perl Date: Tue, 26 Mar 2024 14:49:26 +0000 Subject: [PATCH 03/25] ... --- src/dodal/devices/aperture.py | 6 +++--- src/dodal/devices/aperturescatterguard.py | 19 ++++++++++++------- .../unit_tests/test_aperture_scatterguard.py | 5 +++++ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/dodal/devices/aperture.py b/src/dodal/devices/aperture.py index ab976b4c25..81c4c7f51c 100644 --- a/src/dodal/devices/aperture.py +++ b/src/dodal/devices/aperture.py @@ -1,14 +1,14 @@ from ophyd_async.core import StandardReadable -from ophyd_async.epics.signal import epics_signal_r, epics_signal_rw +from ophyd_async.epics.signal import epics_signal_r from dodal.devices.util.motor_utils import ExtendedMotor class Aperture(StandardReadable): def __init__(self, prefix: str, name: str = "") -> None: - self.x = epics_signal_rw(float, prefix + "X") + self.x = ExtendedMotor(prefix + "X") self.y = ExtendedMotor(prefix + "Y") - self.z = epics_signal_rw(float, prefix + "Z") + 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") diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index cfc331cf5a..c970377b82 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -6,6 +6,7 @@ from ophyd.status import AndStatus, Status, StatusBase from ophyd_async.core import SignalR, StandardReadable +from ophyd_async.core.sim_signal_backend import SimSignalBackend from ophyd_async.epics.motion import Motor from dodal.devices.aperture import Aperture @@ -91,11 +92,13 @@ def __init__(self, prefix: str = "", name: str = "") -> None: super().__init__(name) class SelectedAperture(SignalR): - def get(self): + async def get(self): assert isinstance(self.parent, ApertureScatterguard) - return self.parent._get_current_aperture_position() + await self._backend.put(self.parent._get_current_aperture_position()) - selected_aperture = SelectedAperture() # look at software devices + selected_aperture = SelectedAperture( + backend=SimSignalBackend(datatype=SingleAperturePosition, source="") + ) # look at software devices def load_aperture_positions(self, positions: AperturePositions): LOGGER.info(f"{self.name} loaded in {positions}") @@ -123,7 +126,7 @@ def _set_raw_unsafe(self, positions: ApertureFiveDimensionalLocation) -> AndStat operator.and_, [motor.set(pos) for motor, pos in zip(motors, positions)] ) - 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 @@ -131,9 +134,11 @@ def _get_current_aperture_position(self) -> SingleAperturePosition: If no position is found then raises InvalidApertureMove. """ assert isinstance(self.aperture_positions, AperturePositions) - current_ap_y = self.aperture.y.get_value() + current_ap_y = await self.aperture.y.readback.get_value() robot_load_ap_y = self.aperture_positions.ROBOT_LOAD.location.aperture_y - tolerance = self.TOLERANCE_STEPS * self.aperture.y.motor_resolution.get) + 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 self.aperture.large.get_value() == 1: @@ -162,7 +167,7 @@ def _safe_move_within_datacollection_range( # 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 = self.aperture.z. if not ap_z_in_position: status: Status = Status(obj=self) status.set_exception( diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 621d6d27f1..5b13b8a206 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -76,6 +76,11 @@ def aperture_positions(): return aperture_positions +def test_create_aperturescatterguard(): + fake_aperture_scatterguard = make_fake_device(ApertureScatterguard) + ap_sg = fake_aperture_scatterguard() + + def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): position_to_reject = ApertureFiveDimensionalLocation(0, 0, 0, 0, 0) From 241414bb302c4c135c57a054a4d861f395a239fa Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Tue, 26 Mar 2024 15:23:11 +0000 Subject: [PATCH 04/25] work in progress, scatterguard extended motor, async/await --- src/dodal/devices/aperturescatterguard.py | 53 ++++++++++--------- src/dodal/devices/scatterguard.py | 7 +-- src/dodal/devices/util/motor_utils.py | 1 + .../unit_tests/test_aperture_scatterguard.py | 2 +- 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index c970377b82..3781bcec9d 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -1,3 +1,4 @@ +import asyncio import operator from collections import namedtuple from dataclasses import dataclass @@ -5,9 +6,8 @@ from typing import List, Optional, Sequence from ophyd.status import AndStatus, Status, StatusBase -from ophyd_async.core import SignalR, StandardReadable +from ophyd_async.core import AsyncStatus, SignalR, StandardReadable from ophyd_async.core.sim_signal_backend import SimSignalBackend -from ophyd_async.epics.motion import Motor from dodal.devices.aperture import Aperture from dodal.devices.scatterguard import Scatterguard @@ -152,7 +152,7 @@ async def _get_current_aperture_position(self) -> SingleAperturePosition: raise InvalidApertureMove("Current aperture/scatterguard state unrecognised") - def _safe_move_within_datacollection_range( + async def _safe_move_within_datacollection_range( self, pos: ApertureFiveDimensionalLocation ) -> StatusBase: """ @@ -167,7 +167,7 @@ def _safe_move_within_datacollection_range( # unpacking the position aperture_x, aperture_y, aperture_z, scatterguard_x, scatterguard_y = pos - ap_z_in_position = self.aperture.z. + ap_z_in_position = await self.aperture.z.done_with_move.get_value() if not ap_z_in_position: status: Status = Status(obj=self) status.set_exception( @@ -178,8 +178,10 @@ def _safe_move_within_datacollection_range( ) return status - current_ap_z = self.aperture.z.read() - 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( @@ -188,28 +190,29 @@ def _safe_move_within_datacollection_range( f"Current aperture z ({current_ap_z}), outside of tolerance ({tolerance}) from target ({aperture_z})." ) - current_ap_y = self.aperture.y.read() + 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) + sg_status: AsyncStatus = asyncio.gather( + self.scatterguard.x._move(scatterguard_x), + self.scatterguard.y._move(scatterguard_y), + ) + await sg_status + return asyncio.gather( + sg_status, + self.aperture.x._move(aperture_x), + self.aperture.y._move(aperture_y), + self.aperture.z._move(aperture_z), ) - ap_status: AndStatus = ( - self.aperture.x.set(aperture_x) - & self.aperture.y.set(aperture_y) - & self.aperture.z.set(aperture_z) + ap_status: AsyncStatus = 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 ap_status + final_status: AsyncStatus = asyncio.gather( + ap_status, + self.scatterguard.x._move(scatterguard_x), + self.scatterguard.y._move(scatterguard_y), ) return final_status diff --git a/src/dodal/devices/scatterguard.py b/src/dodal/devices/scatterguard.py index b2a087ec6d..8a567242d0 100644 --- a/src/dodal/devices/scatterguard.py +++ b/src/dodal/devices/scatterguard.py @@ -1,11 +1,12 @@ from ophyd_async.core import StandardReadable -from ophyd_async.epics.signal import epics_signal_rw + +from dodal.devices.util.motor_utils import ExtendedMotor class Scatterguard(StandardReadable): """The scatterguard device.""" def __init__(self, prefix: str, name: str = "") -> None: - self.x = epics_signal_rw(float, prefix + "X") - self.y = epics_signal_rw(float, prefix + "Y") + self.x = ExtendedMotor(prefix + "X") + self.y = ExtendedMotor(prefix + "Y") super().__init__(name) diff --git a/src/dodal/devices/util/motor_utils.py b/src/dodal/devices/util/motor_utils.py index d67aa47594..1bb14ea4da 100644 --- a/src/dodal/devices/util/motor_utils.py +++ b/src/dodal/devices/util/motor_utils.py @@ -12,4 +12,5 @@ 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.done_with_move = epics_signal_r(float, prefix + ".DMOV") super().__init__(name) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 5b13b8a206..db6b80de9d 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -186,7 +186,7 @@ def set_underlying_motors( def test_aperture_positions_large( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): - ap_sg.aperture.large.sim_put(1) # type: ignore + ap_sg.aperture.large._backend._set_value(1) # type: ignore assert ap_sg._get_current_aperture_position() == aperture_positions.LARGE From 23097907c37dfe35135bba743afbb4f3bca3afe0 Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Tue, 26 Mar 2024 16:35:06 +0000 Subject: [PATCH 05/25] Work in progress async --- src/dodal/devices/aperturescatterguard.py | 36 ++++++++++------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 3781bcec9d..97b06e4d9c 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -5,7 +5,6 @@ from functools import reduce from typing import List, Optional, Sequence -from ophyd.status import AndStatus, Status, StatusBase from ophyd_async.core import AsyncStatus, SignalR, StandardReadable from ophyd_async.core.sim_signal_backend import SimSignalBackend @@ -104,7 +103,7 @@ 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}") @@ -120,7 +119,9 @@ def _get_motor_list(self): self.scatterguard.y, ] - def _set_raw_unsafe(self, positions: ApertureFiveDimensionalLocation) -> AndStatus: + def _set_raw_unsafe( + self, positions: ApertureFiveDimensionalLocation + ) -> AsyncStatus: motors: Sequence[ExtendedMotor] = self._get_motor_list() return reduce( operator.and_, [motor.set(pos) for motor, pos in zip(motors, positions)] @@ -154,7 +155,7 @@ async def _get_current_aperture_position(self) -> SingleAperturePosition: 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 @@ -169,14 +170,10 @@ async def _safe_move_within_datacollection_range( ap_z_in_position = await self.aperture.z.done_with_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 = await self.aperture.z.readback.get_value() tolerance = ( @@ -191,28 +188,25 @@ async def _safe_move_within_datacollection_range( ) current_ap_y = await self.aperture.y.readback.get_value() + if aperture_y > current_ap_y: - sg_status: AsyncStatus = asyncio.gather( + await asyncio.gather( self.scatterguard.x._move(scatterguard_x), self.scatterguard.y._move(scatterguard_y), ) - await sg_status - return asyncio.gather( - sg_status, + await asyncio.gather( self.aperture.x._move(aperture_x), self.aperture.y._move(aperture_y), self.aperture.z._move(aperture_z), ) - - ap_status: AsyncStatus = asyncio.gather( + return + await asyncio.gather( self.aperture.x._move(aperture_x), self.aperture.y._move(aperture_y), self.aperture.z._move(aperture_z), ) - await ap_status - final_status: AsyncStatus = asyncio.gather( - ap_status, + + await asyncio.gather( self.scatterguard.x._move(scatterguard_x), self.scatterguard.y._move(scatterguard_y), ) - return final_status From 661ec59aab2d66b38f038c314ffe0399d958d235 Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Wed, 27 Mar 2024 11:52:44 +0000 Subject: [PATCH 06/25] WIP ophyd-asyncio .connect(sim=True) for unit-tests --- src/dodal/devices/aperturescatterguard.py | 8 +- src/dodal/devices/util/motor_utils.py | 2 +- tests/devices/unit_tests/conftest.py | 37 +++++-- .../unit_tests/test_aperture_scatterguard.py | 100 ++++++++++-------- 4 files changed, 86 insertions(+), 61 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 97b06e4d9c..77dba513b4 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -142,11 +142,11 @@ async def _get_current_aperture_position(self) -> SingleAperturePosition: ) # extendedepicsmotor class has tolerance fields added # ophyd-async epics motor may need to do the same thing - epics motor - if self.aperture.large.get_value() == 1: + if await self.aperture.large.get_value() == 1: return self.aperture_positions.LARGE - elif self.aperture.medium.get_value() == 1: + elif await self.aperture.medium.get_value() == 1: return self.aperture_positions.MEDIUM - elif self.aperture.small.get_value() == 1: + elif await self.aperture.small.get_value() == 1: return self.aperture_positions.SMALL elif current_ap_y <= robot_load_ap_y + tolerance: return self.aperture_positions.ROBOT_LOAD @@ -168,7 +168,7 @@ async def _safe_move_within_datacollection_range( # unpacking the position aperture_x, aperture_y, aperture_z, scatterguard_x, scatterguard_y = pos - ap_z_in_position = await self.aperture.z.done_with_move.get_value() + ap_z_in_position = await self.aperture.z.motor_done_move.get_value() if not ap_z_in_position: raise InvalidApertureMove( "ApertureScatterguard z is still moving. Wait for it to finish " diff --git a/src/dodal/devices/util/motor_utils.py b/src/dodal/devices/util/motor_utils.py index 1bb14ea4da..8becc6c0e8 100644 --- a/src/dodal/devices/util/motor_utils.py +++ b/src/dodal/devices/util/motor_utils.py @@ -12,5 +12,5 @@ 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.done_with_move = epics_signal_r(float, prefix + ".DMOV") + self.motor_done_move = epics_signal_r(float, prefix + ".DMOV") super().__init__(name) diff --git a/tests/devices/unit_tests/conftest.py b/tests/devices/unit_tests/conftest.py index f27893fbed..6c094378bc 100644 --- a/tests/devices/unit_tests/conftest.py +++ b/tests/devices/unit_tests/conftest.py @@ -4,21 +4,36 @@ 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 ExtendedEpicsMotor, 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) -def mock_set(motor: EpicsMotor, val): - motor.user_setpoint.sim_put(val) # type: ignore - motor.user_readback.sim_put(val) # type: ignore + +# 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_set(motor: Union[ExtendedMotor, Motor], val): + motor.setpoint._backend._set_value(val) # type: ignore + motor.readback._backend._set_value(val) # type: ignore return Status(done=True, success=True) -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 +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 + motor.motor_done_move._backend._set_value(1) # type: ignore + if isinstance(motor, ExtendedMotor): + motor.motor_resolution._backend._set_value(0.001) # type: ignore return patch.object(motor, "set", MagicMock(side_effect=partial(mock_set, motor))) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index db6b80de9d..d960b10fda 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -2,7 +2,7 @@ import pytest from ophyd.sim import make_fake_device -from ophyd.status import StatusBase +from ophyd_async.core import AsyncStatus # from ophyd_async.core import StandardReadable # from ophyd_async.core.sim_signal_backend import SimSignalBackend @@ -18,9 +18,10 @@ @pytest.fixture -def ap_sg(aperture_positions: AperturePositions): +async def ap_sg(aperture_positions: AperturePositions): FakeApertureScatterguard = make_fake_device(ApertureScatterguard) ap_sg: ApertureScatterguard = FakeApertureScatterguard(name="test_ap_sg") + await ap_sg.connect(sim=True) ap_sg.load_aperture_positions(aperture_positions) with ( patch_motor(ap_sg.aperture.x), @@ -43,7 +44,7 @@ def aperture_in_medium_pos( ap_sg.aperture.z.set(medium.aperture_z) ap_sg.scatterguard.x.set(medium.scatterguard_x) ap_sg.scatterguard.y.set(medium.scatterguard_y) - ap_sg.aperture.medium.sim_put(1) # type: ignore + ap_sg.aperture.medium._backend._set_value(1) # type: ignore yield ap_sg @@ -81,6 +82,13 @@ def test_create_aperturescatterguard(): ap_sg = fake_aperture_scatterguard() +def test_get_aperturescatterguard_aperture(): + fake_aperture_scatterguard = make_fake_device(ApertureScatterguard) + ap_sg = fake_aperture_scatterguard() + value = ap_sg.aperture.y.readback.get_value() + print(value) + + def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): position_to_reject = ApertureFiveDimensionalLocation(0, 0, 0, 0, 0) @@ -149,28 +157,27 @@ def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): # ) -def test_aperture_scatterguard_throws_error_if_outside_tolerance( +async def test_aperture_scatterguard_throws_error_if_outside_tolerance( ap_sg: ApertureScatterguard, ): - ap_sg.aperture.z.motor_resolution._set_value(0.001) # type: ignore - ap_sg.aperture.z._set_value(1) # type: ignore - ap_sg.aperture.z.motor_done_move.sim_put(1) # type: ignore + ap_sg.aperture.z.motor_resolution._backend._set_value(0.001) # type: ignore + ap_sg.aperture.z.readback._backend._set_value(1) # type: ignore + ap_sg.aperture.z.motor_done_move._backend._set_value(1) # type: ignore with pytest.raises(InvalidApertureMove): pos = ApertureFiveDimensionalLocation(0, 0, 1.1, 0, 0) - ap_sg._safe_move_within_datacollection_range(pos) + await ap_sg._safe_move_within_datacollection_range(pos) -def test_aperture_scatterguard_returns_status_if_within_tolerance( +async def test_aperture_scatterguard_returns_status_if_within_tolerance( ap_sg: ApertureScatterguard, ): - ap_sg.aperture.z.motor_resolution.sim_put(0.001) # type: ignore - ap_sg.aperture.z.user_setpoint.sim_put(1) # type: ignore - ap_sg.aperture.z.motor_done_move.sim_put(1) # type: ignore + ap_sg.aperture.z.motor_resolution._backend._set_value(0.001) # type: ignore + ap_sg.aperture.z.readback._backend._set_value(1) # type: ignore + ap_sg.aperture.z.motor_done_move._backend._set_value(1) # type: ignore pos = ApertureFiveDimensionalLocation(0, 0, 1, 0, 0) - status = ap_sg._safe_move_within_datacollection_range(pos) - assert isinstance(status, StatusBase) + await ap_sg._safe_move_within_datacollection_range(pos) def set_underlying_motors( @@ -183,71 +190,75 @@ def set_underlying_motors( ap_sg.scatterguard.y.set(position.scatterguard_y) -def test_aperture_positions_large( +async def test_aperture_positions_large( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): ap_sg.aperture.large._backend._set_value(1) # type: ignore - assert ap_sg._get_current_aperture_position() == aperture_positions.LARGE + assert await ap_sg._get_current_aperture_position() == aperture_positions.LARGE -def test_aperture_positions_medium( +async def test_aperture_positions_medium( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): - ap_sg.aperture.medium.sim_put(1) # type: ignore - assert ap_sg._get_current_aperture_position() == aperture_positions.MEDIUM + ap_sg.aperture.medium._backend._set_value(1) # type: ignore + assert await ap_sg._get_current_aperture_position() == aperture_positions.MEDIUM -def test_aperture_positions_small( +async def test_aperture_positions_small( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): - ap_sg.aperture.small.sim_put(1) # type: ignore - assert ap_sg._get_current_aperture_position() == aperture_positions.SMALL + ap_sg.aperture.small._backend._set_value(1) # type: ignore + assert await ap_sg._get_current_aperture_position() == aperture_positions.SMALL -def test_aperture_positions_robot_load( +async def test_aperture_positions_robot_load( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): - ap_sg.aperture.large.sim_put(0) # type: ignore - ap_sg.aperture.medium.sim_put(0) # type: ignore - ap_sg.aperture.small.sim_put(0) # type: ignore + ap_sg.aperture.large._backend._set_value(0) # type: ignore + ap_sg.aperture.medium._backend._set_value(0) # type: ignore + ap_sg.aperture.small._backend._set_value(0) # type: ignore ap_sg.aperture.y.set(aperture_positions.ROBOT_LOAD.location.aperture_y) # type: ignore - assert ap_sg._get_current_aperture_position() == aperture_positions.ROBOT_LOAD + assert await ap_sg._get_current_aperture_position() == aperture_positions.ROBOT_LOAD -def test_aperture_positions_robot_load_within_tolerance( +async def test_aperture_positions_robot_load_within_tolerance( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): robot_load_ap_y = aperture_positions.ROBOT_LOAD.location.aperture_y - tolerance = ap_sg.TOLERANCE_STEPS * ap_sg.aperture.y.motor_resolution.get() - ap_sg.aperture.large.sim_put(0) # type: ignore - ap_sg.aperture.medium.sim_put(0) # type: ignore - ap_sg.aperture.small.sim_put(0) # type: ignore + tolerance = ( + ap_sg.TOLERANCE_STEPS * await ap_sg.aperture.y.motor_resolution.get_value() + ) + ap_sg.aperture.large._backend._set_value(0) # type: ignore + ap_sg.aperture.medium._backend._set_value(0) # type: ignore + ap_sg.aperture.small._backend._set_value(0) # type: ignore ap_sg.aperture.y.set(robot_load_ap_y + tolerance) # type: ignore - assert ap_sg._get_current_aperture_position() == aperture_positions.ROBOT_LOAD + assert await ap_sg._get_current_aperture_position() == aperture_positions.ROBOT_LOAD -def test_aperture_positions_robot_load_outside_tolerance( +async def test_aperture_positions_robot_load_outside_tolerance( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): robot_load_ap_y = aperture_positions.ROBOT_LOAD.location.aperture_y - tolerance = (ap_sg.TOLERANCE_STEPS + 1) * ap_sg.aperture.y.motor_resolution.get() - ap_sg.aperture.large.sim_put(0) # type: ignore - ap_sg.aperture.medium.sim_put(0) # type: ignore - ap_sg.aperture.small.sim_put(0) # type: ignore + tolerance = ( + ap_sg.TOLERANCE_STEPS + 1 + ) * await ap_sg.aperture.y.motor_resolution.get_value() + ap_sg.aperture.large._backend._set_value(0) # type: ignore + ap_sg.aperture.medium._backend._set_value(0) # type: ignore + ap_sg.aperture.small._backend._set_value(0) # type: ignore ap_sg.aperture.y.set(robot_load_ap_y + tolerance) # type: ignore with pytest.raises(InvalidApertureMove): - ap_sg._get_current_aperture_position() + await ap_sg._get_current_aperture_position() -def test_aperture_positions_robot_load_unsafe( +async def test_aperture_positions_robot_load_unsafe( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): - ap_sg.aperture.large.sim_put(0) # type: ignore - ap_sg.aperture.medium.sim_put(0) # type: ignore - ap_sg.aperture.small.sim_put(0) # type: ignore + ap_sg.aperture.large._backend._set_value(0) # type: ignore + ap_sg.aperture.medium._backend._set_value(0) # type: ignore + ap_sg.aperture.small._backend._set_value(0) # type: ignore ap_sg.aperture.y.set(50.0) # type: ignore with pytest.raises(InvalidApertureMove): - ap_sg._get_current_aperture_position() + await ap_sg._get_current_aperture_position() def test_given_aperture_not_set_through_device_but_motors_in_position_when_device_read_then_position_returned( @@ -264,7 +275,6 @@ def test_when_aperture_set_and_device_read_then_position_returned( aperture_in_medium_pos: ApertureScatterguard, aperture_positions: AperturePositions ): set_status = aperture_in_medium_pos.set(aperture_positions.MEDIUM) - set_status.wait() selected_aperture = aperture_in_medium_pos.read() assert ( selected_aperture["test_ap_sg_selected_aperture"]["value"] From 7d0f304860c573e65c86eb28b345814c6ea88e7c Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Wed, 27 Mar 2024 15:38:06 +0000 Subject: [PATCH 07/25] WIP more tests passing now we are updating position values - thanks David! --- src/dodal/devices/aperturescatterguard.py | 35 +++++++++++-------- tests/devices/unit_tests/conftest.py | 20 ++--------- tests/devices/unit_tests/test_aperture.py | 7 ++-- .../unit_tests/test_aperture_scatterguard.py | 16 +++++---- 4 files changed, 36 insertions(+), 42 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 77dba513b4..34299f82b4 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -32,10 +32,12 @@ class InvalidApertureMove(Exception): @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( @@ -88,16 +90,21 @@ def __init__(self, prefix: str = "", name: str = "") -> None: 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(SignalR): - async def get(self): + async def read(self, *args, **kwargs): assert isinstance(self.parent, ApertureScatterguard) - await self._backend.put(self.parent._get_current_aperture_position()) - - selected_aperture = SelectedAperture( - backend=SimSignalBackend(datatype=SingleAperturePosition, source="") - ) # look at software devices + 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}") @@ -135,18 +142,18 @@ async def _get_current_aperture_position(self) -> SingleAperturePosition: If no position is found then raises InvalidApertureMove. """ assert isinstance(self.aperture_positions, AperturePositions) - current_ap_y = await self.aperture.y.readback.get_value() + 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 * 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() == 1: + if await self.aperture.large.get_value(cached=False) == 1: return self.aperture_positions.LARGE - elif await self.aperture.medium.get_value() == 1: + elif await self.aperture.medium.get_value(cached=False) == 1: return self.aperture_positions.MEDIUM - elif await self.aperture.small.get_value() == 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 diff --git a/tests/devices/unit_tests/conftest.py b/tests/devices/unit_tests/conftest.py index 6c094378bc..2df82c6cc0 100644 --- a/tests/devices/unit_tests/conftest.py +++ b/tests/devices/unit_tests/conftest.py @@ -2,26 +2,10 @@ from typing import Union from unittest.mock import MagicMock, 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, 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) - - -# 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))) +from dodal.devices.util.motor_utils import ExtendedMotor def mock_set(motor: Union[ExtendedMotor, Motor], val): @@ -33,7 +17,7 @@ def mock_set(motor: Union[ExtendedMotor, Motor], val): 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 - motor.motor_done_move._backend._set_value(1) # 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, "set", MagicMock(side_effect=partial(mock_set, motor))) diff --git a/tests/devices/unit_tests/test_aperture.py b/tests/devices/unit_tests/test_aperture.py index fc0c155656..0e3b734b7f 100644 --- a/tests/devices/unit_tests/test_aperture.py +++ b/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) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index d960b10fda..ef7f7848ce 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -261,23 +261,25 @@ async def test_aperture_positions_robot_load_unsafe( await ap_sg._get_current_aperture_position() -def test_given_aperture_not_set_through_device_but_motors_in_position_when_device_read_then_position_returned( +async def test_given_aperture_not_set_through_device_but_motors_in_position_when_device_read_then_position_returned( aperture_in_medium_pos: ApertureScatterguard, aperture_positions: AperturePositions ): - selected_aperture = aperture_in_medium_pos.read() + selected_aperture = await aperture_in_medium_pos.read() + assert isinstance(selected_aperture, dict) assert ( - selected_aperture["test_ap_sg_selected_aperture"]["value"] + selected_aperture["test_ap_sg-selected_aperture"]["value"] == aperture_positions.MEDIUM ) -def test_when_aperture_set_and_device_read_then_position_returned( +async def test_when_aperture_set_and_device_read_then_position_returned( aperture_in_medium_pos: ApertureScatterguard, aperture_positions: AperturePositions ): - set_status = aperture_in_medium_pos.set(aperture_positions.MEDIUM) - selected_aperture = aperture_in_medium_pos.read() + set_status = await aperture_in_medium_pos.set(aperture_positions.MEDIUM) + print(set_status) + selected_aperture = await aperture_in_medium_pos.read() assert ( - selected_aperture["test_ap_sg_selected_aperture"]["value"] + selected_aperture["test_ap_sg-selected_aperture"]["value"] == aperture_positions.MEDIUM ) From ed2b34b5758b6758fa2c4a863ac01d1f1b08c021 Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Thu, 4 Apr 2024 12:58:59 +0000 Subject: [PATCH 08/25] WIP working on tests --- src/dodal/devices/aperturescatterguard.py | 3 +- tests/devices/unit_tests/conftest.py | 21 ++- .../unit_tests/test_aperture_scatterguard.py | 127 +++++++++++------- 3 files changed, 97 insertions(+), 54 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 34299f82b4..ce87d9c40b 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -131,7 +131,8 @@ def _set_raw_unsafe( ) -> AsyncStatus: motors: Sequence[ExtendedMotor] = self._get_motor_list() return reduce( - operator.and_, [motor.set(pos) for motor, pos in zip(motors, positions)] + operator.and_, + [motor.set(pos) for motor, pos in zip(motors, positions)], ) async def _get_current_aperture_position(self) -> SingleAperturePosition: diff --git a/tests/devices/unit_tests/conftest.py b/tests/devices/unit_tests/conftest.py index 2df82c6cc0..c3bfcd9ba5 100644 --- a/tests/devices/unit_tests/conftest.py +++ b/tests/devices/unit_tests/conftest.py @@ -1,17 +1,32 @@ from functools import partial from typing import Union -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch from ophyd.status import Status +from ophyd_async.core.async_status import AsyncStatus from ophyd_async.epics.motion import Motor from dodal.devices.util.motor_utils import ExtendedMotor +# def mock_set(motor: Union[ExtendedMotor, Motor], val): +# motor.setpoint._backend._set_value(val) # type: ignore +# motor.readback._backend._set_value(val) # type: ignore +# return Status(done=True, success=True) + + +# 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, "set", MagicMock(side_effect=partial(mock_set, motor))) + def mock_set(motor: Union[ExtendedMotor, Motor], val): motor.setpoint._backend._set_value(val) # type: ignore motor.readback._backend._set_value(val) # type: ignore - return Status(done=True, success=True) + return AsyncStatus(awaitable=True) # type: ignore def patch_motor(motor: Union[ExtendedMotor, Motor], initial_position=0): @@ -20,4 +35,4 @@ def patch_motor(motor: Union[ExtendedMotor, Motor], initial_position=0): 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, "set", MagicMock(side_effect=partial(mock_set, motor))) + return patch.object(motor, "set", AsyncMock(side_effect=partial(mock_set, motor))) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index ef7f7848ce..5aabdb4d28 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -1,11 +1,11 @@ -# from unittest.mock import MagicMock, call +import asyncio +from unittest.mock import AsyncMock, call, patch import pytest from ophyd.sim import make_fake_device -from ophyd_async.core import AsyncStatus +from ophyd_async.core import AsyncStatus, StandardReadable +from ophyd_async.core.sim_signal_backend import SimSignalBackend -# from ophyd_async.core import StandardReadable -# from ophyd_async.core.sim_signal_backend import SimSignalBackend from dodal.devices.aperturescatterguard import ( ApertureFiveDimensionalLocation, AperturePositions, @@ -105,7 +105,7 @@ def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): # aperture_scatterguard = aperture_in_medium_pos # call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) -# aperture_scatterguard.set(aperture_positions.SMALL) +# aperture_scatterguard.set(aperture_positions.SMALL) # type: ignore # call_logger.assert_has_calls( # [ @@ -118,43 +118,70 @@ def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): # ) -# def test_aperture_unsafe_move( -# aperture_positions: AperturePositions, -# aperture_in_medium_pos: ApertureScatterguard, -# ): -# (a, b, c, d, e) = (0.2, 3.4, 5.6, 7.8, 9.0) -# aperture_scatterguard = aperture_in_medium_pos -# call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) -# aperture_scatterguard._set_raw_unsafe((a, b, c, d, e)) # type: ignore +@patch("dodal.devices.aperturescatterguard.asyncio.gather") +async def test_aperture_scatterguard_select_bottom_moves_sg_down_then_assembly_up( + asyncio_gather: AsyncMock, + aperture_positions: AperturePositions, + aperture_in_medium_pos: ApertureScatterguard, +): + aperture_scatterguard = aperture_in_medium_pos + + await aperture_scatterguard.set(aperture_positions.SMALL) + + asyncio_gather.assert_has_calls( + # [ + # + # + # ] + [ + asyncio.gather( + call._mock_sg_x(5.3375), call._mock_sg_y(-3.55), call._mock_ap_x(2.43) + ), + asyncio.gather(call._mock_ap_y(48.974), call._mock_ap_z(15.8)), + ] + ) -# call_logger.assert_has_calls( -# [ -# call._mock_ap_x(a), -# call._mock_ap_y(b), -# call._mock_ap_z(c), -# call._mock_sg_x(d), -# call._mock_sg_y(e), -# ] -# ) +async def test_aperture_unsafe_move( + aperture_positions: AperturePositions, + aperture_in_medium_pos: ApertureScatterguard, +): + (a, b, c, d, e) = (0.2, 3.4, 5.6, 7.8, 9.0) + aperture_scatterguard = aperture_in_medium_pos + call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) + await aperture_scatterguard._set_raw_unsafe((a, b, c, d, e)) # type: ignore + + call_logger.assert_has_calls( + [ + call._mock_ap_x(a), + call._mock_ap_y(b), + call._mock_ap_z(c), + call._mock_sg_x(d), + call._mock_sg_y(e), + ] + ) -# def test_aperture_scatterguard_select_top_moves_assembly_down_then_sg_up( -# aperture_positions: AperturePositions, aperture_in_medium_pos: ApertureScatterguard -# ): -# aperture_scatterguard = aperture_in_medium_pos -# call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) -# aperture_scatterguard.set(aperture_positions.LARGE) +async def test_aperture_scatterguard_select_top_moves_assembly_down_then_sg_up( + aperture_positions: AperturePositions, aperture_in_medium_pos: ApertureScatterguard +): + aperture_scatterguard = aperture_in_medium_pos + call_logger = await install_logger_for_aperture_and_scatterguard( + aperture_scatterguard + ) -# call_logger.assert_has_calls( -# [ -# call._mock_ap_x(2.389), -# call._mock_ap_y(40.986), -# call._mock_ap_z(15.8), -# call._mock_sg_x(5.25), -# call._mock_sg_y(4.43), -# ] -# ) + await aperture_scatterguard.set(aperture_positions.LARGE) # type: ignore + + call_logger.assert_has_calls( + [ + call._mock_ap_x(2.389), + call._mock_ap_y(40.986), + call._mock_ap_z(15.8), + call._mock_sg_x(5.25), + call._mock_sg_y(4.43), + ], + any_order=False, + ) async def test_aperture_scatterguard_throws_error_if_outside_tolerance( @@ -285,16 +312,16 @@ async def test_when_aperture_set_and_device_read_then_position_returned( # ensure movements happen correctly to avoid collision - find another way -# def install_logger_for_aperture_and_scatterguard(aperture_scatterguard): -# parent_mock = MagicMock() -# mock_ap_x = aperture_scatterguard.aperture.x.set -# mock_ap_y = aperture_scatterguard.aperture.y.set -# mock_ap_z = aperture_scatterguard.aperture.z.set -# mock_sg_x = aperture_scatterguard.scatterguard.x.set -# mock_sg_y = aperture_scatterguard.scatterguard.y.set -# parent_mock.attach_mock(mock_ap_x, "_mock_ap_x") -# parent_mock.attach_mock(mock_ap_y, "_mock_ap_y") -# parent_mock.attach_mock(mock_ap_z, "_mock_ap_z") -# parent_mock.attach_mock(mock_sg_x, "_mock_sg_x") -# parent_mock.attach_mock(mock_sg_y, "_mock_sg_y") -# return parent_mock +def install_logger_for_aperture_and_scatterguard(aperture_scatterguard): + parent_mock = AsyncMock() + mock_ap_x = aperture_scatterguard.aperture.x._move + mock_ap_y = aperture_scatterguard.aperture.y._move + mock_ap_z = aperture_scatterguard.aperture.z._move + mock_sg_x = aperture_scatterguard.scatterguard.x._move + mock_sg_y = aperture_scatterguard.scatterguard.y._move + parent_mock.attach_mock(mock_ap_x, "_mock_ap_x") + parent_mock.attach_mock(mock_ap_y, "_mock_ap_y") + parent_mock.attach_mock(mock_ap_z, "_mock_ap_z") + parent_mock.attach_mock(mock_sg_x, "_mock_sg_x") + parent_mock.attach_mock(mock_sg_y, "_mock_sg_y") + return parent_mock From 9650bea5d002b467057534691968903528bb0de3 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 4 Apr 2024 14:50:52 +0100 Subject: [PATCH 09/25] (#391) mock _move instead of set in ap_sg --- src/dodal/devices/aperturescatterguard.py | 1 + tests/devices/unit_tests/conftest.py | 13 +++++-- .../unit_tests/test_aperture_scatterguard.py | 34 ++++++++----------- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index ce87d9c40b..3f3ffeb101 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -134,6 +134,7 @@ def _set_raw_unsafe( operator.and_, [motor.set(pos) for motor, pos in zip(motors, positions)], ) + # return AsyncStatus(asyncio.gather(a._move(...),b._move(...),...)) async def _get_current_aperture_position(self) -> SingleAperturePosition: """ diff --git a/tests/devices/unit_tests/conftest.py b/tests/devices/unit_tests/conftest.py index c3bfcd9ba5..9d015ec76d 100644 --- a/tests/devices/unit_tests/conftest.py +++ b/tests/devices/unit_tests/conftest.py @@ -1,3 +1,4 @@ +import asyncio from functools import partial from typing import Union from unittest.mock import AsyncMock, MagicMock, patch @@ -23,10 +24,14 @@ # return patch.object(motor, "set", MagicMock(side_effect=partial(mock_set, motor))) -def mock_set(motor: Union[ExtendedMotor, Motor], val): +async def mock_good_coroutine(): + return asyncio.sleep(0) + + +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 AsyncStatus(awaitable=True) # type: ignore + return mock_good_coroutine() # type: ignore def patch_motor(motor: Union[ExtendedMotor, Motor], initial_position=0): @@ -35,4 +40,6 @@ def patch_motor(motor: Union[ExtendedMotor, Motor], initial_position=0): 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, "set", AsyncMock(side_effect=partial(mock_set, motor))) + return patch.object( + motor, "_move", AsyncMock(side_effect=partial(mock_move, motor)) + ) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 5aabdb4d28..94622d8d10 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -34,16 +34,16 @@ async def ap_sg(aperture_positions: AperturePositions): @pytest.fixture -def aperture_in_medium_pos( +async def aperture_in_medium_pos( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions, ): medium = aperture_positions.MEDIUM.location - ap_sg.aperture.x.set(medium.aperture_x) - ap_sg.aperture.y.set(medium.aperture_y) - ap_sg.aperture.z.set(medium.aperture_z) - ap_sg.scatterguard.x.set(medium.scatterguard_x) - ap_sg.scatterguard.y.set(medium.scatterguard_y) + await ap_sg.aperture.x.set(medium.aperture_x) + await ap_sg.aperture.y.set(medium.aperture_y) + await ap_sg.aperture.z.set(medium.aperture_z) + await ap_sg.scatterguard.x.set(medium.scatterguard_x) + await ap_sg.scatterguard.y.set(medium.scatterguard_y) ap_sg.aperture.medium._backend._set_value(1) # type: ignore yield ap_sg @@ -118,27 +118,23 @@ def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): # ) -@patch("dodal.devices.aperturescatterguard.asyncio.gather") async def test_aperture_scatterguard_select_bottom_moves_sg_down_then_assembly_up( - asyncio_gather: AsyncMock, aperture_positions: AperturePositions, aperture_in_medium_pos: ApertureScatterguard, ): + call_logger = install_logger_for_aperture_and_scatterguard(aperture_in_medium_pos) aperture_scatterguard = aperture_in_medium_pos await aperture_scatterguard.set(aperture_positions.SMALL) - asyncio_gather.assert_has_calls( - # [ - # - # - # ] - [ - asyncio.gather( - call._mock_sg_x(5.3375), call._mock_sg_y(-3.55), call._mock_ap_x(2.43) - ), - asyncio.gather(call._mock_ap_y(48.974), call._mock_ap_z(15.8)), - ] + call_logger.assert_has_calls( + calls=[ + call._mock_sg_x(5.3375), + call._mock_sg_y(-3.55), + call._mock_ap_x(2.43), + call._mock_ap_y(48.974), + call._mock_ap_z(15.8), + ], ) From 41ca74f2725068cc0a81be5b537623c189e24e86 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 4 Apr 2024 14:56:40 +0100 Subject: [PATCH 10/25] (#391) remove stray await --- tests/devices/unit_tests/test_aperture_scatterguard.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 94622d8d10..8d21663656 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -162,9 +162,7 @@ async def test_aperture_scatterguard_select_top_moves_assembly_down_then_sg_up( aperture_positions: AperturePositions, aperture_in_medium_pos: ApertureScatterguard ): aperture_scatterguard = aperture_in_medium_pos - call_logger = await install_logger_for_aperture_and_scatterguard( - aperture_scatterguard - ) + call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) await aperture_scatterguard.set(aperture_positions.LARGE) # type: ignore From 41436a3ab1e2f777ccb6938d1b784b745832c74e Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Thu, 4 Apr 2024 15:36:20 +0000 Subject: [PATCH 11/25] _set_raw_unsafe to asyncio.gather --- src/dodal/devices/aperturescatterguard.py | 20 +++++++++++-------- .../unit_tests/test_aperture_scatterguard.py | 20 ------------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 3f3ffeb101..f007c6d976 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -126,15 +126,19 @@ def _get_motor_list(self): self.scatterguard.y, ] - def _set_raw_unsafe( - self, positions: ApertureFiveDimensionalLocation - ) -> AsyncStatus: - motors: Sequence[ExtendedMotor] = 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), ) - # return AsyncStatus(asyncio.gather(a._move(...),b._move(...),...)) async def _get_current_aperture_position(self) -> SingleAperturePosition: """ diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 8d21663656..bd35c63ca5 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -98,26 +98,6 @@ def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): ) -# def test_aperture_scatterguard_select_bottom_moves_sg_down_then_assembly_up( -# aperture_positions: AperturePositions, -# aperture_in_medium_pos: ApertureScatterguard, -# ): -# aperture_scatterguard = aperture_in_medium_pos -# call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) - -# aperture_scatterguard.set(aperture_positions.SMALL) # type: ignore - -# call_logger.assert_has_calls( -# [ -# call._mock_sg_x(5.3375), -# call._mock_sg_y(-3.55), -# call._mock_ap_x(2.43), -# call._mock_ap_y(48.974), -# call._mock_ap_z(15.8), -# ] -# ) - - async def test_aperture_scatterguard_select_bottom_moves_sg_down_then_assembly_up( aperture_positions: AperturePositions, aperture_in_medium_pos: ApertureScatterguard, From 32be1b52b4cb512334d0616d571f84af8910588c Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Thu, 4 Apr 2024 15:38:07 +0000 Subject: [PATCH 12/25] tidy up unused imports and old commented code block --- src/dodal/devices/aperturescatterguard.py | 7 ++----- tests/devices/unit_tests/conftest.py | 18 +----------------- .../unit_tests/test_aperture_scatterguard.py | 5 +---- 3 files changed, 4 insertions(+), 26 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index f007c6d976..6f46bd1d4c 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -1,16 +1,13 @@ import asyncio -import operator 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_async.core import AsyncStatus, SignalR, StandardReadable +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.scatterguard import Scatterguard -from dodal.devices.util.motor_utils import ExtendedMotor from dodal.log import LOGGER diff --git a/tests/devices/unit_tests/conftest.py b/tests/devices/unit_tests/conftest.py index 9d015ec76d..5213cbadb8 100644 --- a/tests/devices/unit_tests/conftest.py +++ b/tests/devices/unit_tests/conftest.py @@ -1,28 +1,12 @@ import asyncio from functools import partial from typing import Union -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch -from ophyd.status import Status -from ophyd_async.core.async_status import AsyncStatus from ophyd_async.epics.motion import Motor from dodal.devices.util.motor_utils import ExtendedMotor -# def mock_set(motor: Union[ExtendedMotor, Motor], val): -# motor.setpoint._backend._set_value(val) # type: ignore -# motor.readback._backend._set_value(val) # type: ignore -# return Status(done=True, success=True) - - -# 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, "set", MagicMock(side_effect=partial(mock_set, motor))) - async def mock_good_coroutine(): return asyncio.sleep(0) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index bd35c63ca5..1ddbf7b6e0 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -1,10 +1,7 @@ -import asyncio -from unittest.mock import AsyncMock, call, patch +from unittest.mock import AsyncMock, call import pytest from ophyd.sim import make_fake_device -from ophyd_async.core import AsyncStatus, StandardReadable -from ophyd_async.core.sim_signal_backend import SimSignalBackend from dodal.devices.aperturescatterguard import ( ApertureFiveDimensionalLocation, From 19a03bd68b0b124eac68e9cd7066540f9f3e3f9e Mon Sep 17 00:00:00 2001 From: David Perl Date: Fri, 5 Apr 2024 08:48:13 +0100 Subject: [PATCH 13/25] placate linter --- .pre-commit-config.yaml | 2 +- tests/devices/unit_tests/test_aperture_scatterguard.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c3b62b737e..f08d80fc8a 100644 --- a/.pre-commit-config.yaml +++ b/.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 diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 1ddbf7b6e0..ca9bc4cc20 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -76,7 +76,7 @@ def aperture_positions(): def test_create_aperturescatterguard(): fake_aperture_scatterguard = make_fake_device(ApertureScatterguard) - ap_sg = fake_aperture_scatterguard() + _ = fake_aperture_scatterguard() def test_get_aperturescatterguard_aperture(): From 2de7fa1264ccb539cff68037aeac0fc6e84ec533 Mon Sep 17 00:00:00 2001 From: David Perl Date: Fri, 5 Apr 2024 09:01:46 +0100 Subject: [PATCH 14/25] (#391) fix logging and LUT tests --- .../unit_tests/util/test_adjuster_plans.py | 2 +- tests/plans/test_topup_plan.py | 2 +- tests/unit_tests/test_log.py | 15 ++++++--------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/devices/unit_tests/util/test_adjuster_plans.py b/tests/devices/unit_tests/util/test_adjuster_plans.py index 3c1802aa18..72dd11c9f9 100644 --- a/tests/devices/unit_tests/util/test_adjuster_plans.py +++ b/tests/devices/unit_tests/util/test_adjuster_plans.py @@ -10,7 +10,7 @@ @pytest.fixture def fake_aperture(): FakeAperture = make_fake_device(Aperture) - fake_aperture: Aperture = FakeAperture(name="aperture") + fake_aperture: Aperture = FakeAperture(prefix="", name="aperture") return fake_aperture diff --git a/tests/plans/test_topup_plan.py b/tests/plans/test_topup_plan.py index 4cb079af1c..bcc73cc03d 100644 --- a/tests/plans/test_topup_plan.py +++ b/tests/plans/test_topup_plan.py @@ -14,7 +14,7 @@ @pytest.fixture -def synchrotron() -> Synchrotron: +def synchrotron(RE) -> Synchrotron: return i03.synchrotron(fake_with_ophyd_sim=True) diff --git a/tests/unit_tests/test_log.py b/tests/unit_tests/test_log.py index d0ed3205f6..deca8ab19c 100644 --- a/tests/unit_tests/test_log.py +++ b/tests/unit_tests/test_log.py @@ -4,6 +4,7 @@ import pytest from graypy import GELFTCPHandler +from ophyd import log as ophyd_log from dodal import log from dodal.log import ( @@ -130,6 +131,10 @@ def test_various_messages_to_graylog_get_beamline_filter( ): from os import environ + from bluesky.run_engine import RunEngine + + RE = RunEngine() + if environ.get("BEAMLINE"): del environ["BEAMLINE"] log.beamline_filter = log.BeamlineFilter() @@ -157,19 +162,11 @@ def mock_set_up_graylog_handler(logger, host, port): mock_GELFTCPHandler.emit.assert_called() assert mock_GELFTCPHandler.emit.call_args.args[0].beamline == "dev" - from dodal.beamlines import i03 - - _aperture_scatterguard = i03.aperture_scatterguard( - fake_with_ophyd_sim=True, wait_for_connection=True - ) + ophyd_log.logger.info("Ophyd log message") assert mock_GELFTCPHandler.emit.call_args.args[0].name == "ophyd" assert mock_GELFTCPHandler.emit.call_args.args[0].beamline == "dev" - from bluesky.run_engine import RunEngine - - RE = RunEngine() RE.log.logger.info("RunEngine log message") - assert mock_GELFTCPHandler.emit.call_args.args[0].name == "bluesky" assert mock_GELFTCPHandler.emit.call_args.args[0].beamline == "dev" From 49fa71654f54570412984b5df3a9fd5974312d07 Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Wed, 10 Apr 2024 10:00:21 +0000 Subject: [PATCH 15/25] WIP AsyncStatus bluseky tests --- src/dodal/devices/aperturescatterguard.py | 13 +++-- .../unit_tests/test_aperture_scatterguard.py | 52 ++++++++++++++----- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 8d56f36687..22289f7ed8 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -2,7 +2,8 @@ from collections import namedtuple from dataclasses import dataclass -from ophyd_async.core import SignalR, StandardReadable +from bluesky.protocols import Movable +from ophyd_async.core import AsyncStatus, SignalR, StandardReadable from ophyd_async.core.sim_signal_backend import SimSignalBackend from dodal.devices.aperture import Aperture @@ -80,7 +81,7 @@ def as_list(self) -> list[SingleAperturePosition]: ] -class ApertureScatterguard(StandardReadable): +class ApertureScatterguard(StandardReadable, Movable): def __init__(self, prefix: str = "", name: str = "") -> None: self.aperture = Aperture(prefix="-MO-MAPT-01:") self.scatterguard = Scatterguard(prefix="-MO-SCAT-01:") @@ -106,12 +107,12 @@ def load_aperture_positions(self, positions: AperturePositions): LOGGER.info(f"{self.name} loaded in {positions}") self.aperture_positions = positions - def set(self, pos: SingleAperturePosition): + def set(self, pos: SingleAperturePosition) -> AsyncStatus: assert isinstance(self.aperture_positions, AperturePositions) if pos not in self.aperture_positions.as_list(): raise InvalidApertureMove(f"Unknown aperture: {pos}") - return self._safe_move_within_datacollection_range(pos.location) + return AsyncStatus(self._safe_move_within_datacollection_range(pos.location)) def _get_motor_list(self): return [ @@ -208,6 +209,10 @@ async def _safe_move_within_datacollection_range( self.aperture.y._move(aperture_y), self.aperture.z._move(aperture_z), ) + new_ap_x = await self.aperture.x.readback.get_value() + new_ap_y = await self.aperture.y.readback.get_value() + + print(new_ap_x, new_ap_y) return await asyncio.gather( self.aperture.x._move(aperture_x), diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index ca9bc4cc20..980fa1d913 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -1,6 +1,8 @@ from unittest.mock import AsyncMock, call +import bluesky.plan_stubs as bps import pytest +from bluesky.run_engine import RunEngine from ophyd.sim import make_fake_device from dodal.devices.aperturescatterguard import ( @@ -74,6 +76,21 @@ def aperture_positions(): return aperture_positions +def install_logger_for_aperture_and_scatterguard(aperture_scatterguard): + parent_mock = AsyncMock() + mock_ap_x = aperture_scatterguard.aperture.x._move + mock_ap_y = aperture_scatterguard.aperture.y._move + mock_ap_z = aperture_scatterguard.aperture.z._move + mock_sg_x = aperture_scatterguard.scatterguard.x._move + mock_sg_y = aperture_scatterguard.scatterguard.y._move + parent_mock.attach_mock(mock_ap_x, "_mock_ap_x") + parent_mock.attach_mock(mock_ap_y, "_mock_ap_y") + parent_mock.attach_mock(mock_ap_z, "_mock_ap_z") + parent_mock.attach_mock(mock_sg_x, "_mock_sg_x") + parent_mock.attach_mock(mock_sg_y, "_mock_sg_y") + return parent_mock + + def test_create_aperturescatterguard(): fake_aperture_scatterguard = make_fake_device(ApertureScatterguard) _ = fake_aperture_scatterguard() @@ -282,17 +299,24 @@ async def test_when_aperture_set_and_device_read_then_position_returned( ) -# ensure movements happen correctly to avoid collision - find another way -def install_logger_for_aperture_and_scatterguard(aperture_scatterguard): - parent_mock = AsyncMock() - mock_ap_x = aperture_scatterguard.aperture.x._move - mock_ap_y = aperture_scatterguard.aperture.y._move - mock_ap_z = aperture_scatterguard.aperture.z._move - mock_sg_x = aperture_scatterguard.scatterguard.x._move - mock_sg_y = aperture_scatterguard.scatterguard.y._move - parent_mock.attach_mock(mock_ap_x, "_mock_ap_x") - parent_mock.attach_mock(mock_ap_y, "_mock_ap_y") - parent_mock.attach_mock(mock_ap_z, "_mock_ap_z") - parent_mock.attach_mock(mock_sg_x, "_mock_sg_x") - parent_mock.attach_mock(mock_sg_y, "_mock_sg_y") - return parent_mock +async def test_ap_sg_in_runengine( + aperture_in_medium_pos: ApertureScatterguard, + RE: RunEngine, +): + test_position = ( + aperture_in_medium_pos.aperture_positions.SMALL # type: ignore + ) # I'll leave this to you + RE( + bps.abs_set( + aperture_in_medium_pos, + test_position, + wait=True, + ) + ) + # position = await aperture_in_medium_pos._get_current_aperture_position() + # medium = aperture_in_medium_pos.aperture_positions.MEDIUM # type: ignore + # assert medium == position + # assert that positions afterwards are equal to specified in the test position + assert ( + await aperture_in_medium_pos._get_current_aperture_position() == test_position + ) From 7325b3b0e6e6821730e0eff8b25d0f9997f0eea6 Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 10 Apr 2024 11:16:59 +0100 Subject: [PATCH 16/25] (#391) fix test --- src/dodal/devices/aperturescatterguard.py | 4 --- .../unit_tests/test_aperture_scatterguard.py | 32 +++++++++++-------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 22289f7ed8..1c7fdb3c16 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -209,10 +209,6 @@ async def _safe_move_within_datacollection_range( self.aperture.y._move(aperture_y), self.aperture.z._move(aperture_z), ) - new_ap_x = await self.aperture.x.readback.get_value() - new_ap_y = await self.aperture.y.readback.get_value() - - print(new_ap_x, new_ap_y) return await asyncio.gather( self.aperture.x._move(aperture_x), diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 980fa1d913..268ac5d71d 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -303,20 +303,24 @@ async def test_ap_sg_in_runengine( aperture_in_medium_pos: ApertureScatterguard, RE: RunEngine, ): - test_position = ( - aperture_in_medium_pos.aperture_positions.SMALL # type: ignore - ) # I'll leave this to you - RE( - bps.abs_set( - aperture_in_medium_pos, - test_position, - wait=True, - ) + ap_sg = aperture_in_medium_pos + assert ap_sg.aperture_positions + test_position = ap_sg.aperture_positions.SMALL + RE(bps.abs_set(ap_sg, test_position, wait=True)) + assert ( + await ap_sg.aperture.x.readback.get_value() == test_position.location.aperture_x + ) + assert ( + await ap_sg.aperture.y.readback.get_value() == test_position.location.aperture_y + ) + assert ( + await ap_sg.aperture.z.readback.get_value() == test_position.location.aperture_z + ) + assert ( + await ap_sg.scatterguard.x.readback.get_value() + == test_position.location.scatterguard_x ) - # position = await aperture_in_medium_pos._get_current_aperture_position() - # medium = aperture_in_medium_pos.aperture_positions.MEDIUM # type: ignore - # assert medium == position - # assert that positions afterwards are equal to specified in the test position assert ( - await aperture_in_medium_pos._get_current_aperture_position() == test_position + await ap_sg.scatterguard.y.readback.get_value() + == test_position.location.scatterguard_y ) From cedcfd257b0f58f313051233d1b68a2a50a7fc60 Mon Sep 17 00:00:00 2001 From: David Perl Date: Fri, 12 Apr 2024 08:40:45 +0100 Subject: [PATCH 17/25] (#391) update i03 test --- tests/beamlines/unit_tests/test_i03.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/beamlines/unit_tests/test_i03.py b/tests/beamlines/unit_tests/test_i03.py index 227634811f..33af943be6 100644 --- a/tests/beamlines/unit_tests/test_i03.py +++ b/tests/beamlines/unit_tests/test_i03.py @@ -14,7 +14,7 @@ def test_list(): ] -def test_getting_second_aperture_scatterguard_gives_valid_device(): +def test_getting_second_aperture_scatterguard_gives_valid_device(RE): beamline_utils.clear_devices() test_positions = AperturePositions( (0, 1, 2, 3, 4), (5, 6, 7, 8, 9), (10, 11, 12, 13, 14), (15, 16, 17, 18, 19) @@ -24,4 +24,4 @@ def test_getting_second_aperture_scatterguard_gives_valid_device(): ) assert ap_sg.aperture_positions is not None ap_sg = i03.aperture_scatterguard(fake_with_ophyd_sim=True) - assert ap_sg.aperture_positions is not None + assert ap_sg.aperture_positions == test_positions From 5c5c97e337f73a8a34db2db9b90788bf5903a43b Mon Sep 17 00:00:00 2001 From: David Perl Date: Fri, 12 Apr 2024 08:54:00 +0100 Subject: [PATCH 18/25] (#391) tidy up tests a little --- .../unit_tests/test_aperture_scatterguard.py | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 268ac5d71d..aa1c10abe1 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -99,8 +99,7 @@ def test_create_aperturescatterguard(): def test_get_aperturescatterguard_aperture(): fake_aperture_scatterguard = make_fake_device(ApertureScatterguard) ap_sg = fake_aperture_scatterguard() - value = ap_sg.aperture.y.readback.get_value() - print(value) + _ = ap_sg.aperture.y.readback.get_value() def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): @@ -304,23 +303,14 @@ async def test_ap_sg_in_runengine( RE: RunEngine, ): ap_sg = aperture_in_medium_pos + ap = ap_sg.aperture + sg = ap_sg.scatterguard assert ap_sg.aperture_positions test_position = ap_sg.aperture_positions.SMALL + test_loc = test_position.location RE(bps.abs_set(ap_sg, test_position, wait=True)) - assert ( - await ap_sg.aperture.x.readback.get_value() == test_position.location.aperture_x - ) - assert ( - await ap_sg.aperture.y.readback.get_value() == test_position.location.aperture_y - ) - assert ( - await ap_sg.aperture.z.readback.get_value() == test_position.location.aperture_z - ) - assert ( - await ap_sg.scatterguard.x.readback.get_value() - == test_position.location.scatterguard_x - ) - assert ( - await ap_sg.scatterguard.y.readback.get_value() - == test_position.location.scatterguard_y - ) + assert await ap.x.readback.get_value() == test_loc.aperture_x + assert await ap.y.readback.get_value() == test_loc.aperture_y + assert await ap.z.readback.get_value() == test_loc.aperture_z + assert await sg.x.readback.get_value() == test_loc.scatterguard_x + assert await sg.y.readback.get_value() == test_loc.scatterguard_y From 965271835c3ca48c718cac590b50eb605775fc6e Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Mon, 15 Apr 2024 11:46:01 +0000 Subject: [PATCH 19/25] Remove print from tests and obseleted comment --- src/dodal/devices/aperturescatterguard.py | 2 -- tests/devices/unit_tests/test_aperture_scatterguard.py | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 1c7fdb3c16..7825bf7b6a 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -150,8 +150,6 @@ async def _get_current_aperture_position(self) -> SingleAperturePosition: 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 await self.aperture.medium.get_value(cached=False) == 1: diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index aa1c10abe1..5a9383c987 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -289,8 +289,7 @@ async def test_given_aperture_not_set_through_device_but_motors_in_position_when async def test_when_aperture_set_and_device_read_then_position_returned( aperture_in_medium_pos: ApertureScatterguard, aperture_positions: AperturePositions ): - set_status = await aperture_in_medium_pos.set(aperture_positions.MEDIUM) - print(set_status) + await aperture_in_medium_pos.set(aperture_positions.MEDIUM) selected_aperture = await aperture_in_medium_pos.read() assert ( selected_aperture["test_ap_sg-selected_aperture"]["value"] From 211703f131822a11f6259a02eb3aebd253350d73 Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Tue, 16 Apr 2024 07:55:08 +0000 Subject: [PATCH 20/25] update for compatability with ophyd-async breaking change 0e7e45b --- src/dodal/devices/aperturescatterguard.py | 6 +++--- tests/devices/unit_tests/conftest.py | 8 ++++---- .../unit_tests/test_aperture_scatterguard.py | 16 ++++++++-------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 7825bf7b6a..2e5468387d 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -145,7 +145,7 @@ async def _get_current_aperture_position(self) -> SingleAperturePosition: If no position is found then raises InvalidApertureMove. """ assert isinstance(self.aperture_positions, AperturePositions) - current_ap_y = await self.aperture.y.readback.get_value(cached=False) + current_ap_y = await self.aperture.y.user_readback.get_value(cached=False) robot_load_ap_y = self.aperture_positions.ROBOT_LOAD.location.aperture_y tolerance = ( self.TOLERANCE_STEPS * await self.aperture.y.motor_resolution.get_value() @@ -183,7 +183,7 @@ async def _safe_move_within_datacollection_range( "before triggering another move." ) - current_ap_z = await self.aperture.z.readback.get_value() + current_ap_z = await self.aperture.z.user_readback.get_value() tolerance = ( self.TOLERANCE_STEPS * await self.aperture.z.motor_resolution.get_value() ) @@ -195,7 +195,7 @@ async def _safe_move_within_datacollection_range( f"Current aperture z ({current_ap_z}), outside of tolerance ({tolerance}) from target ({aperture_z})." ) - current_ap_y = await self.aperture.y.readback.get_value() + current_ap_y = await self.aperture.y.user_readback.get_value() if aperture_y > current_ap_y: await asyncio.gather( diff --git a/tests/devices/unit_tests/conftest.py b/tests/devices/unit_tests/conftest.py index f21fadafa4..84ad70d1d8 100644 --- a/tests/devices/unit_tests/conftest.py +++ b/tests/devices/unit_tests/conftest.py @@ -15,14 +15,14 @@ async def mock_good_coroutine(): def mock_move(motor: ExtendedMotor | Motor, val, *args, **kwargs): - motor.setpoint._backend._set_value(val) # type: ignore - motor.readback._backend._set_value(val) # type: ignore + motor.user_setpoint._backend._set_value(val) # type: ignore + motor.user_readback._backend._set_value(val) # type: ignore return mock_good_coroutine() # type: ignore def patch_motor(motor: ExtendedMotor | Motor, initial_position=0): - motor.setpoint._backend._set_value(initial_position) # type: ignore - motor.readback._backend._set_value(initial_position) # type: ignore + motor.user_setpoint._backend._set_value(initial_position) # type: ignore + motor.user_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 diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 5a9383c987..12bdaccf10 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -99,7 +99,7 @@ def test_create_aperturescatterguard(): def test_get_aperturescatterguard_aperture(): fake_aperture_scatterguard = make_fake_device(ApertureScatterguard) ap_sg = fake_aperture_scatterguard() - _ = ap_sg.aperture.y.readback.get_value() + _ = ap_sg.aperture.y.user_readback.get_value() def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): @@ -175,7 +175,7 @@ async def test_aperture_scatterguard_throws_error_if_outside_tolerance( ap_sg: ApertureScatterguard, ): ap_sg.aperture.z.motor_resolution._backend._set_value(0.001) # type: ignore - ap_sg.aperture.z.readback._backend._set_value(1) # type: ignore + ap_sg.aperture.z.user_readback._backend._set_value(1) # type: ignore ap_sg.aperture.z.motor_done_move._backend._set_value(1) # type: ignore with pytest.raises(InvalidApertureMove): @@ -187,7 +187,7 @@ async def test_aperture_scatterguard_returns_status_if_within_tolerance( ap_sg: ApertureScatterguard, ): ap_sg.aperture.z.motor_resolution._backend._set_value(0.001) # type: ignore - ap_sg.aperture.z.readback._backend._set_value(1) # type: ignore + ap_sg.aperture.z.user_readback._backend._set_value(1) # type: ignore ap_sg.aperture.z.motor_done_move._backend._set_value(1) # type: ignore pos = ApertureFiveDimensionalLocation(0, 0, 1, 0, 0) @@ -308,8 +308,8 @@ async def test_ap_sg_in_runengine( test_position = ap_sg.aperture_positions.SMALL test_loc = test_position.location RE(bps.abs_set(ap_sg, test_position, wait=True)) - assert await ap.x.readback.get_value() == test_loc.aperture_x - assert await ap.y.readback.get_value() == test_loc.aperture_y - assert await ap.z.readback.get_value() == test_loc.aperture_z - assert await sg.x.readback.get_value() == test_loc.scatterguard_x - assert await sg.y.readback.get_value() == test_loc.scatterguard_y + assert await ap.x.user_readback.get_value() == test_loc.aperture_x + assert await ap.y.user_readback.get_value() == test_loc.aperture_y + assert await ap.z.user_readback.get_value() == test_loc.aperture_z + assert await sg.x.user_readback.get_value() == test_loc.scatterguard_x + assert await sg.y.user_readback.get_value() == test_loc.scatterguard_y From 991aaf018c075e2f74f2b8ec20e549d6e9f4b156 Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Wed, 17 Apr 2024 19:42:18 +0000 Subject: [PATCH 21/25] Removed ExtendedMotor for Motor and changed motor_resolution to deadband --- src/dodal/devices/aperture.py | 8 ++++---- src/dodal/devices/aperturescatterguard.py | 8 ++------ src/dodal/devices/scatterguard.py | 6 +++--- src/dodal/devices/util/motor_utils.py | 8 -------- tests/devices/unit_tests/conftest.py | 10 ++++------ tests/devices/unit_tests/test_aperture_scatterguard.py | 10 ++++------ 6 files changed, 17 insertions(+), 33 deletions(-) diff --git a/src/dodal/devices/aperture.py b/src/dodal/devices/aperture.py index 81c4c7f51c..7ed63b6114 100644 --- a/src/dodal/devices/aperture.py +++ b/src/dodal/devices/aperture.py @@ -1,14 +1,14 @@ from ophyd_async.core import StandardReadable from ophyd_async.epics.signal import epics_signal_r -from dodal.devices.util.motor_utils import ExtendedMotor +from dodal.devices.util.motor_utils import Motor 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.x = Motor(prefix + "X") + self.y = Motor(prefix + "Y") + self.z = Motor(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") diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 2e5468387d..785b55c4f1 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -147,9 +147,7 @@ async def _get_current_aperture_position(self) -> SingleAperturePosition: assert isinstance(self.aperture_positions, AperturePositions) current_ap_y = await self.aperture.y.user_readback.get_value(cached=False) robot_load_ap_y = self.aperture_positions.ROBOT_LOAD.location.aperture_y - tolerance = ( - self.TOLERANCE_STEPS * await self.aperture.y.motor_resolution.get_value() - ) + tolerance = self.TOLERANCE_STEPS * await self.aperture.y.deadband.get_value() if await self.aperture.large.get_value(cached=False) == 1: return self.aperture_positions.LARGE elif await self.aperture.medium.get_value(cached=False) == 1: @@ -184,9 +182,7 @@ async def _safe_move_within_datacollection_range( ) current_ap_z = await self.aperture.z.user_readback.get_value() - tolerance = ( - self.TOLERANCE_STEPS * await self.aperture.z.motor_resolution.get_value() - ) + tolerance = self.TOLERANCE_STEPS * await self.aperture.z.deadband.get_value() diff_on_z = abs(current_ap_z - aperture_z) if diff_on_z > tolerance: raise InvalidApertureMove( diff --git a/src/dodal/devices/scatterguard.py b/src/dodal/devices/scatterguard.py index 8a567242d0..9996fb3609 100644 --- a/src/dodal/devices/scatterguard.py +++ b/src/dodal/devices/scatterguard.py @@ -1,12 +1,12 @@ from ophyd_async.core import StandardReadable -from dodal.devices.util.motor_utils import ExtendedMotor +from dodal.devices.util.motor_utils import Motor class Scatterguard(StandardReadable): """The scatterguard device.""" def __init__(self, prefix: str, name: str = "") -> None: - self.x = ExtendedMotor(prefix + "X") - self.y = ExtendedMotor(prefix + "Y") + self.x = Motor(prefix + "X") + self.y = Motor(prefix + "Y") super().__init__(name) diff --git a/src/dodal/devices/util/motor_utils.py b/src/dodal/devices/util/motor_utils.py index 8becc6c0e8..9742baa47c 100644 --- a/src/dodal/devices/util/motor_utils.py +++ b/src/dodal/devices/util/motor_utils.py @@ -6,11 +6,3 @@ 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) diff --git a/tests/devices/unit_tests/conftest.py b/tests/devices/unit_tests/conftest.py index 84ad70d1d8..ab72a20080 100644 --- a/tests/devices/unit_tests/conftest.py +++ b/tests/devices/unit_tests/conftest.py @@ -7,24 +7,22 @@ from ophyd_async.core import DirectoryInfo, DirectoryProvider, StaticDirectoryProvider from ophyd_async.epics.motion import Motor -from dodal.devices.util.motor_utils import ExtendedMotor - async def mock_good_coroutine(): return asyncio.sleep(0) -def mock_move(motor: ExtendedMotor | Motor, val, *args, **kwargs): +def mock_move(motor: Motor, val, *args, **kwargs): motor.user_setpoint._backend._set_value(val) # type: ignore motor.user_readback._backend._set_value(val) # type: ignore return mock_good_coroutine() # type: ignore -def patch_motor(motor: ExtendedMotor | Motor, initial_position=0): +def patch_motor(motor: Motor, initial_position=0): motor.user_setpoint._backend._set_value(initial_position) # type: ignore motor.user_readback._backend._set_value(initial_position) # type: ignore - if isinstance(motor, ExtendedMotor): - motor.motor_resolution._backend._set_value(0.001) # type: ignore + if isinstance(motor, Motor): + motor.deadband._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)) diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index 12bdaccf10..d0be3cf8b2 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -174,7 +174,7 @@ async def test_aperture_scatterguard_select_top_moves_assembly_down_then_sg_up( async def test_aperture_scatterguard_throws_error_if_outside_tolerance( ap_sg: ApertureScatterguard, ): - ap_sg.aperture.z.motor_resolution._backend._set_value(0.001) # type: ignore + ap_sg.aperture.z.deadband._backend._set_value(0.001) # type: ignore ap_sg.aperture.z.user_readback._backend._set_value(1) # type: ignore ap_sg.aperture.z.motor_done_move._backend._set_value(1) # type: ignore @@ -186,7 +186,7 @@ async def test_aperture_scatterguard_throws_error_if_outside_tolerance( async def test_aperture_scatterguard_returns_status_if_within_tolerance( ap_sg: ApertureScatterguard, ): - ap_sg.aperture.z.motor_resolution._backend._set_value(0.001) # type: ignore + ap_sg.aperture.z.deadband._backend._set_value(0.001) # type: ignore ap_sg.aperture.z.user_readback._backend._set_value(1) # type: ignore ap_sg.aperture.z.motor_done_move._backend._set_value(1) # type: ignore @@ -239,9 +239,7 @@ async def test_aperture_positions_robot_load_within_tolerance( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): robot_load_ap_y = aperture_positions.ROBOT_LOAD.location.aperture_y - tolerance = ( - ap_sg.TOLERANCE_STEPS * await ap_sg.aperture.y.motor_resolution.get_value() - ) + tolerance = ap_sg.TOLERANCE_STEPS * await ap_sg.aperture.y.deadband.get_value() ap_sg.aperture.large._backend._set_value(0) # type: ignore ap_sg.aperture.medium._backend._set_value(0) # type: ignore ap_sg.aperture.small._backend._set_value(0) # type: ignore @@ -255,7 +253,7 @@ async def test_aperture_positions_robot_load_outside_tolerance( robot_load_ap_y = aperture_positions.ROBOT_LOAD.location.aperture_y tolerance = ( ap_sg.TOLERANCE_STEPS + 1 - ) * await ap_sg.aperture.y.motor_resolution.get_value() + ) * await ap_sg.aperture.y.deadband.get_value() ap_sg.aperture.large._backend._set_value(0) # type: ignore ap_sg.aperture.medium._backend._set_value(0) # type: ignore ap_sg.aperture.small._backend._set_value(0) # type: ignore From 73c1361fa8fb535cd3f4acb330701e51c1e260a0 Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Thu, 18 Apr 2024 10:28:51 +0000 Subject: [PATCH 22/25] remove old comment about deadband not in epics motor --- src/dodal/devices/aperturescatterguard.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index 785b55c4f1..41313f2e82 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -167,9 +167,6 @@ async def _safe_move_within_datacollection_range( See https://github.com/DiamondLightSource/hyperion/wiki/Aperture-Scatterguard-Collisions for why this is required. """ - # EpicsMotor does not have deadband/MRES field, so the way to check if we are - # in a datacollection position is to see if we are "ready" (DMOV) and the target - # position is correct # unpacking the position aperture_x, aperture_y, aperture_z, scatterguard_x, scatterguard_y = pos From a991d2fe7a96a8899d1a52b4893539a8dd32e265 Mon Sep 17 00:00:00 2001 From: Kate Smith Date: Fri, 19 Apr 2024 08:08:10 +0000 Subject: [PATCH 23/25] remove unnecessary if statement (no more ExtendedMotor) --- tests/devices/unit_tests/conftest.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/devices/unit_tests/conftest.py b/tests/devices/unit_tests/conftest.py index ab72a20080..a68f85f8cf 100644 --- a/tests/devices/unit_tests/conftest.py +++ b/tests/devices/unit_tests/conftest.py @@ -21,9 +21,8 @@ def mock_move(motor: Motor, val, *args, **kwargs): def patch_motor(motor: Motor, initial_position=0): motor.user_setpoint._backend._set_value(initial_position) # type: ignore motor.user_readback._backend._set_value(initial_position) # type: ignore - if isinstance(motor, Motor): - motor.deadband._backend._set_value(0.001) # type: ignore - motor.motor_done_move._backend._set_value(1) # type: ignore + motor.deadband._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)) ) From e173bbec3eef87e03605da9f1ae6d0873cd14919 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 29 Apr 2024 19:02:04 +0100 Subject: [PATCH 24/25] Remove unused imports --- src/dodal/devices/util/motor_utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/dodal/devices/util/motor_utils.py b/src/dodal/devices/util/motor_utils.py index 9742baa47c..07638ba3f6 100644 --- a/src/dodal/devices/util/motor_utils.py +++ b/src/dodal/devices/util/motor_utils.py @@ -1,6 +1,4 @@ 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): From 1142f8b6872bafcdf530bc6ad34e5153fe204acb Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 29 Apr 2024 19:34:06 +0100 Subject: [PATCH 25/25] Tidy up tests a bit --- src/dodal/devices/aperture.py | 3 +- src/dodal/devices/aperturescatterguard.py | 10 ++- src/dodal/devices/scatterguard.py | 3 +- .../unit_tests/test_aperture_scatterguard.py | 70 ++++++++----------- 4 files changed, 39 insertions(+), 47 deletions(-) diff --git a/src/dodal/devices/aperture.py b/src/dodal/devices/aperture.py index 7ed63b6114..99cb6ff1a4 100644 --- a/src/dodal/devices/aperture.py +++ b/src/dodal/devices/aperture.py @@ -1,8 +1,7 @@ from ophyd_async.core import StandardReadable +from ophyd_async.epics.motion import Motor from ophyd_async.epics.signal import epics_signal_r -from dodal.devices.util.motor_utils import Motor - class Aperture(StandardReadable): def __init__(self, prefix: str, name: str = "") -> None: diff --git a/src/dodal/devices/aperturescatterguard.py b/src/dodal/devices/aperturescatterguard.py index b98ff61715..35ae65016b 100644 --- a/src/dodal/devices/aperturescatterguard.py +++ b/src/dodal/devices/aperturescatterguard.py @@ -29,9 +29,11 @@ class InvalidApertureMove(Exception): @dataclass class SingleAperturePosition: + # Default values are needed as ophyd_async sim does not respect initial_values of + # soft signal backends see https://github.com/bluesky/ophyd-async/issues/266 name: str = "" GDA_name: str = "" - radius_microns: float | None = 0 + radius_microns: float | None = None location: ApertureFiveDimensionalLocation = ApertureFiveDimensionalLocation( 0, 0, 0, 0, 0 ) @@ -63,6 +65,10 @@ class AperturePositions: SMALL: SingleAperturePosition ROBOT_LOAD: SingleAperturePosition + UNKNOWN = SingleAperturePosition( + "Unknown", "UNKNOWN", None, ApertureFiveDimensionalLocation(0, 0, 0, 0, 0) + ) + @classmethod def from_gda_beamline_params(cls, params): return cls( @@ -88,7 +94,7 @@ def __init__(self, prefix: str = "", name: str = "") -> None: self.aperture_positions: AperturePositions | None = None self.TOLERANCE_STEPS = 3 # Number of MRES steps self.selected_aperture = self.SelectedAperture( - backend=SimSignalBackend(datatype=SingleAperturePosition) + backend=SimSignalBackend(SingleAperturePosition, AperturePositions.UNKNOWN), ) self.set_readable_signals( read=[ diff --git a/src/dodal/devices/scatterguard.py b/src/dodal/devices/scatterguard.py index 9996fb3609..3df94b9ba4 100644 --- a/src/dodal/devices/scatterguard.py +++ b/src/dodal/devices/scatterguard.py @@ -1,6 +1,5 @@ from ophyd_async.core import StandardReadable - -from dodal.devices.util.motor_utils import Motor +from ophyd_async.epics.motion import Motor class Scatterguard(StandardReadable): diff --git a/tests/devices/unit_tests/test_aperture_scatterguard.py b/tests/devices/unit_tests/test_aperture_scatterguard.py index d0be3cf8b2..26695a696b 100644 --- a/tests/devices/unit_tests/test_aperture_scatterguard.py +++ b/tests/devices/unit_tests/test_aperture_scatterguard.py @@ -3,7 +3,7 @@ import bluesky.plan_stubs as bps import pytest from bluesky.run_engine import RunEngine -from ophyd.sim import make_fake_device +from ophyd_async.core import set_sim_value from dodal.devices.aperturescatterguard import ( ApertureFiveDimensionalLocation, @@ -18,8 +18,7 @@ @pytest.fixture async def ap_sg(aperture_positions: AperturePositions): - FakeApertureScatterguard = make_fake_device(ApertureScatterguard) - ap_sg: ApertureScatterguard = FakeApertureScatterguard(name="test_ap_sg") + ap_sg = ApertureScatterguard(name="test_ap_sg") await ap_sg.connect(sim=True) ap_sg.load_aperture_positions(aperture_positions) with ( @@ -43,7 +42,7 @@ async def aperture_in_medium_pos( await ap_sg.aperture.z.set(medium.aperture_z) await ap_sg.scatterguard.x.set(medium.scatterguard_x) await ap_sg.scatterguard.y.set(medium.scatterguard_y) - ap_sg.aperture.medium._backend._set_value(1) # type: ignore + set_sim_value(ap_sg.aperture.medium, 1) yield ap_sg @@ -91,17 +90,6 @@ def install_logger_for_aperture_and_scatterguard(aperture_scatterguard): return parent_mock -def test_create_aperturescatterguard(): - fake_aperture_scatterguard = make_fake_device(ApertureScatterguard) - _ = fake_aperture_scatterguard() - - -def test_get_aperturescatterguard_aperture(): - fake_aperture_scatterguard = make_fake_device(ApertureScatterguard) - ap_sg = fake_aperture_scatterguard() - _ = ap_sg.aperture.y.user_readback.get_value() - - def test_aperture_scatterguard_rejects_unknown_position(aperture_in_medium_pos): position_to_reject = ApertureFiveDimensionalLocation(0, 0, 0, 0, 0) @@ -157,7 +145,7 @@ async def test_aperture_scatterguard_select_top_moves_assembly_down_then_sg_up( aperture_scatterguard = aperture_in_medium_pos call_logger = install_logger_for_aperture_and_scatterguard(aperture_scatterguard) - await aperture_scatterguard.set(aperture_positions.LARGE) # type: ignore + await aperture_scatterguard.set(aperture_positions.LARGE) call_logger.assert_has_calls( [ @@ -174,9 +162,9 @@ async def test_aperture_scatterguard_select_top_moves_assembly_down_then_sg_up( async def test_aperture_scatterguard_throws_error_if_outside_tolerance( ap_sg: ApertureScatterguard, ): - ap_sg.aperture.z.deadband._backend._set_value(0.001) # type: ignore - ap_sg.aperture.z.user_readback._backend._set_value(1) # type: ignore - ap_sg.aperture.z.motor_done_move._backend._set_value(1) # type: ignore + set_sim_value(ap_sg.aperture.z.deadband, 0.001) + set_sim_value(ap_sg.aperture.z.user_readback, 1) + set_sim_value(ap_sg.aperture.z.motor_done_move, 1) with pytest.raises(InvalidApertureMove): pos = ApertureFiveDimensionalLocation(0, 0, 1.1, 0, 0) @@ -186,9 +174,9 @@ async def test_aperture_scatterguard_throws_error_if_outside_tolerance( async def test_aperture_scatterguard_returns_status_if_within_tolerance( ap_sg: ApertureScatterguard, ): - ap_sg.aperture.z.deadband._backend._set_value(0.001) # type: ignore - ap_sg.aperture.z.user_readback._backend._set_value(1) # type: ignore - ap_sg.aperture.z.motor_done_move._backend._set_value(1) # type: ignore + set_sim_value(ap_sg.aperture.z.deadband, 0.001) + set_sim_value(ap_sg.aperture.z.user_readback, 1) + set_sim_value(ap_sg.aperture.z.motor_done_move, 1) pos = ApertureFiveDimensionalLocation(0, 0, 1, 0, 0) await ap_sg._safe_move_within_datacollection_range(pos) @@ -207,31 +195,31 @@ def set_underlying_motors( async def test_aperture_positions_large( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): - ap_sg.aperture.large._backend._set_value(1) # type: ignore + set_sim_value(ap_sg.aperture.large, 1) assert await ap_sg._get_current_aperture_position() == aperture_positions.LARGE async def test_aperture_positions_medium( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): - ap_sg.aperture.medium._backend._set_value(1) # type: ignore + set_sim_value(ap_sg.aperture.medium, 1) assert await ap_sg._get_current_aperture_position() == aperture_positions.MEDIUM async def test_aperture_positions_small( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): - ap_sg.aperture.small._backend._set_value(1) # type: ignore + set_sim_value(ap_sg.aperture.small, 1) assert await ap_sg._get_current_aperture_position() == aperture_positions.SMALL async def test_aperture_positions_robot_load( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): - ap_sg.aperture.large._backend._set_value(0) # type: ignore - ap_sg.aperture.medium._backend._set_value(0) # type: ignore - ap_sg.aperture.small._backend._set_value(0) # type: ignore - ap_sg.aperture.y.set(aperture_positions.ROBOT_LOAD.location.aperture_y) # type: ignore + set_sim_value(ap_sg.aperture.large, 0) + set_sim_value(ap_sg.aperture.medium, 0) + set_sim_value(ap_sg.aperture.small, 0) + await ap_sg.aperture.y.set(aperture_positions.ROBOT_LOAD.location.aperture_y) assert await ap_sg._get_current_aperture_position() == aperture_positions.ROBOT_LOAD @@ -240,10 +228,10 @@ async def test_aperture_positions_robot_load_within_tolerance( ): robot_load_ap_y = aperture_positions.ROBOT_LOAD.location.aperture_y tolerance = ap_sg.TOLERANCE_STEPS * await ap_sg.aperture.y.deadband.get_value() - ap_sg.aperture.large._backend._set_value(0) # type: ignore - ap_sg.aperture.medium._backend._set_value(0) # type: ignore - ap_sg.aperture.small._backend._set_value(0) # type: ignore - ap_sg.aperture.y.set(robot_load_ap_y + tolerance) # type: ignore + set_sim_value(ap_sg.aperture.large, 0) + set_sim_value(ap_sg.aperture.medium, 0) + set_sim_value(ap_sg.aperture.small, 0) + await ap_sg.aperture.y.set(robot_load_ap_y + tolerance) assert await ap_sg._get_current_aperture_position() == aperture_positions.ROBOT_LOAD @@ -254,10 +242,10 @@ async def test_aperture_positions_robot_load_outside_tolerance( tolerance = ( ap_sg.TOLERANCE_STEPS + 1 ) * await ap_sg.aperture.y.deadband.get_value() - ap_sg.aperture.large._backend._set_value(0) # type: ignore - ap_sg.aperture.medium._backend._set_value(0) # type: ignore - ap_sg.aperture.small._backend._set_value(0) # type: ignore - ap_sg.aperture.y.set(robot_load_ap_y + tolerance) # type: ignore + set_sim_value(ap_sg.aperture.large, 0) + set_sim_value(ap_sg.aperture.medium, 0) + set_sim_value(ap_sg.aperture.small, 0) + await ap_sg.aperture.y.set(robot_load_ap_y + tolerance) with pytest.raises(InvalidApertureMove): await ap_sg._get_current_aperture_position() @@ -265,10 +253,10 @@ async def test_aperture_positions_robot_load_outside_tolerance( async def test_aperture_positions_robot_load_unsafe( ap_sg: ApertureScatterguard, aperture_positions: AperturePositions ): - ap_sg.aperture.large._backend._set_value(0) # type: ignore - ap_sg.aperture.medium._backend._set_value(0) # type: ignore - ap_sg.aperture.small._backend._set_value(0) # type: ignore - ap_sg.aperture.y.set(50.0) # type: ignore + set_sim_value(ap_sg.aperture.large, 0) + set_sim_value(ap_sg.aperture.medium, 0) + set_sim_value(ap_sg.aperture.small, 0) + await ap_sg.aperture.y.set(50.0) with pytest.raises(InvalidApertureMove): await ap_sg._get_current_aperture_position()