Changes from code review: improved naming, made enabled read/write, a…

…nd used ophyd-async assert_reading utility function for tests.

src/dodal/beamlines/i22.py

@@ -15,7 +15,7 @@
 from dodal.devices.i22.fswitch import FSwitch
 from dodal.devices.slits import Slits
 from dodal.devices.tetramm import TetrammDetector
-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 get_beamline_name, skip_device
 
@@ -277,9 +277,9 @@ def oav(
 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,

src/dodal/devices/watsonmarlow323.py → 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
                ]
           )
 

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,
+            },
+        }
+    )
+