From c2790bf98d3bac7b673000505fea4fa03ffdb599 Mon Sep 17 00:00:00 2001 From: Will Barnett Date: Thu, 30 May 2024 17:36:14 +0100 Subject: [PATCH] Changes from code review: improved naming, made enabled read/write, and used ophyd-async assert_reading utility function for tests. --- src/dodal/beamlines/i22.py | 6 +-- ...onmarlow323.py => watsonmarlow323_pump.py} | 26 ++++++------ .../unit_tests/test_watsonmarlow323.py | 42 ------------------- .../unit_tests/test_watsonmarlow323_pump.py | 40 ++++++++++++++++++ 4 files changed, 57 insertions(+), 57 deletions(-) rename src/dodal/devices/{watsonmarlow323.py => watsonmarlow323_pump.py} (63%) delete mode 100644 tests/devices/unit_tests/test_watsonmarlow323.py create mode 100644 tests/devices/unit_tests/test_watsonmarlow323_pump.py diff --git a/src/dodal/beamlines/i22.py b/src/dodal/beamlines/i22.py index 22a2820e92..6300862ca0 100644 --- a/src/dodal/beamlines/i22.py +++ b/src/dodal/beamlines/i22.py @@ -17,7 +17,7 @@ from dodal.devices.slits import Slits from dodal.devices.tetramm import TetrammDetector from dodal.devices.undulator import Undulator -from dodal.devices.watsonmarlow323 import WatsonMarlow323 +from dodal.devices.watsonmarlow323_pump import WatsonMarlow323Pump from dodal.log import set_beamline as set_log_beamline from dodal.utils import BeamlinePrefix, get_beamline_name, skip_device @@ -315,9 +315,9 @@ def linkam( def ppump( wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False -) -> WatsonMarlow323: +) -> WatsonMarlow323Pump: return device_instantiation( - WatsonMarlow323, + WatsonMarlow323Pump, "ppump", "-EA-PUMP-01:", wait_for_connection, diff --git a/src/dodal/devices/watsonmarlow323.py b/src/dodal/devices/watsonmarlow323_pump.py similarity index 63% rename from src/dodal/devices/watsonmarlow323.py rename to src/dodal/devices/watsonmarlow323_pump.py index 44ac5f9c8c..3d4735d003 100644 --- a/src/dodal/devices/watsonmarlow323.py +++ b/src/dodal/devices/watsonmarlow323_pump.py @@ -1,31 +1,33 @@ from enum import Enum 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_rw -class WatsonMarlow323Enable(str, Enum): +class WatsonMarlow323PumpEnable(str, Enum): DISABLED = "Disabled" ENABLED = "Enabled" -class WatsonMarlow323Direction(str, Enum): - CW = "CW" - CCW = "CCW" +class WatsonMarlow323PumpDirection(str, Enum): + CLOCKWISE = "CW" + COUNTER_CLOCKWISE = "CCW" -class WatsonMarlow323State(str, Enum): +class WatsonMarlow323PumpState(str, Enum): STOPPED = "STOP" STARTED = "START" -class WatsonMarlow323(StandardReadable): +class WatsonMarlow323Pump(StandardReadable): """Watson Marlow 323 Pump device""" def __init__(self, prefix: str, name: str= "") -> None: - self.enabled = epics_signal_r(WatsonMarlow323Enable, prefix + "DISABLE") + self.enabled = epics_signal_rw( + WatsonMarlow323PumpEnable, read_pv = prefix + "DISABLE", write_pv= prefix + "DISABLE" + ) self.direction = epics_signal_rw( - WatsonMarlow323Direction, read_pv = prefix + "INFO:DIR", write_pv= prefix + "SET:DIR" + WatsonMarlow323PumpDirection, read_pv = prefix + "INFO:DIR", write_pv= prefix + "SET:DIR" ) self.state = epics_signal_rw( - WatsonMarlow323State, read_pv = prefix + "INFO:RUN", write_pv = prefix + "SET:RUN" + WatsonMarlow323PumpState, read_pv = prefix + "INFO:RUN", write_pv = prefix + "SET:RUN" ) self.speed = epics_signal_rw( float, read_pv = prefix + "INFO:SPD",write_pv = prefix + "SET:SPD" @@ -39,14 +41,14 @@ def __init__(self, prefix: str, name: str= "") -> None: self.set_readable_signals( read=[ + self.state, self.speed, self.direction ], config=[ - self.state, + self.enabled, self.limit_high, self.limit_low, - self.enabled ] ) diff --git a/tests/devices/unit_tests/test_watsonmarlow323.py b/tests/devices/unit_tests/test_watsonmarlow323.py deleted file mode 100644 index 8e89b60f4c..0000000000 --- a/tests/devices/unit_tests/test_watsonmarlow323.py +++ /dev/null @@ -1,42 +0,0 @@ -from typing import Any, Mapping -from unittest.mock import ANY - -import pytest -from ophyd_async.core import DeviceCollector, StandardReadable, set_mock_value - -from dodal.devices.watsonmarlow323 import WatsonMarlow323, WatsonMarlow323Direction - -@pytest.fixture -async def watsonmarlow323() -> WatsonMarlow323: - async with DeviceCollector(mock=True): - wm_pump = WatsonMarlow323("DEMO-WMPUMP-01:") - - return wm_pump - -async def test_reading_pump_reads_speed_and_direction( watsonmarlow323: WatsonMarlow323): - set_mock_value(watsonmarlow323.speed, 25) - set_mock_value(watsonmarlow323.direction, WatsonMarlow323Direction.CCW) - - await assert_reading( - watsonmarlow323, - { - "wm_pump-speed": { - "alarm_severity": 0, - "timestamp": ANY, - "value": 25, - }, - "wm_pump-direction": { - "alarm_severity": 0, - "timestamp": ANY, - "value": "CCW", - }, - }, - ) - -async def assert_reading( - device: StandardReadable, - expected_reading: Mapping[str, Any], -) -> None: - reading = await device.read() - - assert reading == expected_reading diff --git a/tests/devices/unit_tests/test_watsonmarlow323_pump.py b/tests/devices/unit_tests/test_watsonmarlow323_pump.py new file mode 100644 index 0000000000..f724a061ca --- /dev/null +++ b/tests/devices/unit_tests/test_watsonmarlow323_pump.py @@ -0,0 +1,40 @@ +from unittest.mock import ANY + +import pytest +from ophyd_async.core import DeviceCollector, set_mock_value, assert_reading + +from dodal.devices.watsonmarlow323_pump import WatsonMarlow323Pump, WatsonMarlow323PumpDirection, WatsonMarlow323PumpState + +@pytest.fixture +async def watsonmarlow323() -> WatsonMarlow323Pump: + async with DeviceCollector(mock=True): + wm_pump = WatsonMarlow323Pump("DEMO-WMPUMP-01:") + + return wm_pump + +async def test_reading_pump_reads_state_speed_and_direction( watsonmarlow323: WatsonMarlow323Pump): + set_mock_value(watsonmarlow323.state, WatsonMarlow323PumpState.STOPPED) + set_mock_value(watsonmarlow323.speed, 25) + set_mock_value(watsonmarlow323.direction, WatsonMarlow323PumpDirection.CLOCKWISE) + + await assert_reading( + watsonmarlow323, + { + "wm_pump-state": { + "value": WatsonMarlow323PumpState.STOPPED, + "timestamp": ANY, + "alarm_severity": 0, + }, + "wm_pump-speed": { + "value": 25, + "timestamp": ANY, + "alarm_severity": 0, + }, + "wm_pump-direction": { + "value": WatsonMarlow323PumpDirection.CLOCKWISE, + "timestamp": ANY, + "alarm_severity": 0, + }, + } + ) +