From 0e812fa4e1697cb1f6c302b4e541c2eac2c995a7 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 27 May 2026 14:24:29 +0000 Subject: [PATCH 01/10] Add scaler card device for i09 --- src/dodal/beamlines/i09.py | 52 ++++++++++++++++++++ src/dodal/devices/scaler.py | 93 +++++++++++++++++++++++++++++++++++ tests/devices/test_scaler.py | 94 ++++++++++++++++++++++++++++++++++++ 3 files changed, 239 insertions(+) create mode 100644 src/dodal/devices/scaler.py create mode 100644 tests/devices/test_scaler.py diff --git a/src/dodal/beamlines/i09.py b/src/dodal/beamlines/i09.py index b7c989c4a18..e9f10f0f7c3 100644 --- a/src/dodal/beamlines/i09.py +++ b/src/dodal/beamlines/i09.py @@ -18,6 +18,7 @@ from dodal.devices.hutch_shutter import EXP_SHUTTER_2_INFIX, HutchShutter from dodal.devices.motors import XYZAzimuthPolarStage from dodal.devices.pgm import PlaneGratingMonochromator +from dodal.devices.scaler import ScalerController, SimpleChannelScaler from dodal.devices.selectable_source import SourceSelector from dodal.devices.synchrotron import Synchrotron from dodal.devices.temperture_controller import Lakeshore336 @@ -27,6 +28,7 @@ BL = get_beamline_name("i09") I_PREFIX = BeamlinePrefix(BL, suffix="I") J_PREFIX = BeamlinePrefix(BL, suffix="J") +L_PREFIX = BeamlinePrefix(BL, suffix="L") set_log_beamline(BL) set_utils_beamline(BL) @@ -139,3 +141,53 @@ def intensity_protection() -> SignalRW[IntensityProtection]: return epics_signal_rw( IntensityProtection, f"{I_PREFIX.beamline_prefix}-DI-EAN-01:PROT:ILK" ) + + +@devices.factory +def scaler1() -> ScalerController: + return ScalerController(f"{I_PREFIX.beamline_prefix}-EA-SCLR-01") + + +@devices.factory +def hm3amp20_1(scaler1: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler1, channel=2) + + +@devices.factory +def sm5amp8_1(scaler1: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler1, channel=3) + + +@devices.factory +def smpmamp39_1(scaler1: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler1, channel=4) + + +@devices.factory +def rfdamp10_1(scaler1: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler1, channel=5) + + +@devices.factory +def scaler2() -> ScalerController: + return ScalerController(f"{L_PREFIX}-VA-SCLR-01") + + +@devices.factory +def hm3amp20(scaler1: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler1, channel=2) + + +@devices.factory +def sm5amp8(scaler1: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler1, channel=3) + + +@devices.factory +def smpmamp39(scaler1: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler1, channel=4) + + +@devices.factory +def rfdamp10(scaler1: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler1, channel=5) diff --git a/src/dodal/devices/scaler.py b/src/dodal/devices/scaler.py new file mode 100644 index 00000000000..ab7e6d0f3e2 --- /dev/null +++ b/src/dodal/devices/scaler.py @@ -0,0 +1,93 @@ +import asyncio + +from bluesky.protocols import Triggerable +from ophyd_async.core import ( + AsyncStatus, + DeviceMock, + Reference, + StandardReadable, + StandardReadableFormat, + callback_on_mock_put, + default_mock_class, + set_and_wait_for_value, + set_mock_value, +) +from ophyd_async.epics.core import epics_signal_rw, wait_for_good_state + + +class MockScalerController(DeviceMock["ScalerController"]): + async def connect(self, device: "ScalerController"): + set_mock_value(device.counting, False) + + async def _finish_after_delay(): + await asyncio.sleep(0.2) + set_mock_value(device.counting, False) + + async def _on_put(value): + if value is True: + asyncio.create_task(_finish_after_delay()) + + callback_on_mock_put(device.counting, _on_put) + + +@default_mock_class(MockScalerController) +class ScalerController(StandardReadable, Triggerable): + """Scaler controller that is triggerable. It will set the counting to True and then + waits for it to be False. + """ + + def __init__(self, prefix: str, name: str = ""): + self.counting = epics_signal_rw(bool, prefix + ".CNT") + with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): + self.count_period = epics_signal_rw(float, prefix + ".TP") + + self._acquire_status: AsyncStatus | None = None + # Store the prefix so that the SimpleChannelScaler can reuse. + self.prefix = prefix + + super().__init__(name) + + @AsyncStatus.wrap + async def trigger(self): + self._acquire_status = await set_and_wait_for_value( + self.counting, True, wait_for_set_completion=True + ) + await self._acquire_status + await wait_for_good_state(self.counting, {False}) + + +class SimpleChannelScaler(StandardReadable, Triggerable): + """Create individual channel for a scaler. A ScalerController is used for the + Trigger logic. It will also add this instance signals as readables to the + ScannableController and also add the controllers count_period signal to this + classes read configuration. + """ + + def __init__( + self, + scalar_controller: ScalerController, + channel: int, + name: str = "", + ): + self._scaler_controller_ref = Reference(scalar_controller) + + with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): + self.count = epics_signal_rw( + float, f"{scalar_controller.prefix}.S{channel}" + ) + + super().__init__(name) + + scalar_controller.add_readables([self]) + # Avoid circular read configuration by specifying individual signal + self.add_readables( + [scalar_controller.count_period], StandardReadableFormat.CONFIG_SIGNAL + ) + + @AsyncStatus.wrap + async def set(self, value: float): + await self.count.set(value) + + @AsyncStatus.wrap + async def trigger(self): + await self._scaler_controller_ref().trigger() diff --git a/tests/devices/test_scaler.py b/tests/devices/test_scaler.py new file mode 100644 index 00000000000..ba9c6814ab0 --- /dev/null +++ b/tests/devices/test_scaler.py @@ -0,0 +1,94 @@ +import time + +import pytest +from ophyd_async.core import init_devices +from ophyd_async.testing import assert_configuration, assert_reading, partial_reading + +from dodal.devices.scaler import ScalerController, SimpleChannelScaler + + +@pytest.fixture +def scaler1() -> ScalerController: + with init_devices(mock=True): + scaler1 = ScalerController("TEST:") + return scaler1 + + +@pytest.fixture +def hm3amp20_1(scaler1: ScalerController) -> SimpleChannelScaler: + with init_devices(mock=True): + hm3amp20_1 = SimpleChannelScaler(scaler1, 1) + return hm3amp20_1 + + +@pytest.fixture +def sm5amp8_1(scaler1: ScalerController) -> SimpleChannelScaler: + with init_devices(mock=True): + sm5amp8_1 = SimpleChannelScaler(scaler1, 2) + return sm5amp8_1 + + +async def test_scaler_controller_read(scaler1: ScalerController) -> None: + await assert_reading(scaler1, {}) + + +async def test_scaler_controller_read_with_multiple_channels( + scaler1: ScalerController, + hm3amp20_1: SimpleChannelScaler, + sm5amp8_1: SimpleChannelScaler, +) -> None: + await assert_reading( + scaler1, + { + "hm3amp20_1-count": partial_reading(0), + "sm5amp8_1-count": partial_reading(0), + }, + ) + + +async def test_scaler_controller_read_configuration(scaler1: ScalerController) -> None: + await assert_configuration(scaler1, {"scaler1-count_period": partial_reading(0.0)}) + + +async def test_scaler_controller_trigger_waits_for_counting_to_finish( + scaler1: ScalerController, +) -> None: + start = time.monotonic() + await scaler1.trigger() + elapsed = time.monotonic() - start + assert elapsed >= 0.2 + assert await scaler1.counting.get_value() is False + + +async def test_scaler_controller_trigger_sets_counting_true_then_false( + scaler1: ScalerController, +) -> None: + values = [] + scaler1.counting.subscribe(values.append) + await scaler1.trigger() + + states = [] + expected_states = [False, True, False] + for v in values: + states.append(v[scaler1.counting.name]["value"]) + assert states == expected_states + + +async def test_simple_channel_scaler_read(hm3amp20_1: SimpleChannelScaler) -> None: + await assert_reading( + hm3amp20_1, + { + "hm3amp20_1-count": partial_reading(0), + }, + ) + + +async def test_simple_channel_scaler_read_configuration( + hm3amp20_1: SimpleChannelScaler, +) -> None: + await assert_configuration( + hm3amp20_1, + { + "scaler1-count_period": partial_reading(0), + }, + ) From bfa0064192fee96e610583b1f3254c7dc604c1d2 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 27 May 2026 14:37:07 +0000 Subject: [PATCH 02/10] Add missing tests --- src/dodal/devices/scaler.py | 9 ++++++--- tests/devices/test_scaler.py | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/dodal/devices/scaler.py b/src/dodal/devices/scaler.py index ab7e6d0f3e2..bf31951e6dd 100644 --- a/src/dodal/devices/scaler.py +++ b/src/dodal/devices/scaler.py @@ -23,17 +23,20 @@ async def _finish_after_delay(): await asyncio.sleep(0.2) set_mock_value(device.counting, False) - async def _on_put(value): + async def _on_put(value: bool): if value is True: asyncio.create_task(_finish_after_delay()) + # _on_put is called before the value given is to the signal. Therefore we must + # setup a delay to set the signal back to False once it is True to simulate + # hardware behaviour. callback_on_mock_put(device.counting, _on_put) @default_mock_class(MockScalerController) class ScalerController(StandardReadable, Triggerable): - """Scaler controller that is triggerable. It will set the counting to True and then - waits for it to be False. + """Scaler controller that is triggerable. It will set the counting signal to True + and then waits for it to be False. """ def __init__(self, prefix: str, name: str = ""): diff --git a/tests/devices/test_scaler.py b/tests/devices/test_scaler.py index ba9c6814ab0..6784964fef9 100644 --- a/tests/devices/test_scaler.py +++ b/tests/devices/test_scaler.py @@ -1,4 +1,5 @@ import time +from unittest.mock import AsyncMock, call import pytest from ophyd_async.core import init_devices @@ -92,3 +93,16 @@ async def test_simple_channel_scaler_read_configuration( "scaler1-count_period": partial_reading(0), }, ) + + +async def test_simple_channel_scaler_trigger( + scaler1: ScalerController, hm3amp20_1: SimpleChannelScaler, sm5amp8_1 +) -> None: + mock_trigger = AsyncMock() + scaler1.trigger = mock_trigger + + await hm3amp20_1.trigger() + mock_trigger.assert_awaited_once() + + await sm5amp8_1.trigger() + mock_trigger.assert_has_calls([call(), call()]) From 10c9ce151985a17463a3f35f4f65fd038c7a3f12 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 27 May 2026 14:38:23 +0000 Subject: [PATCH 03/10] Reduce number of lines --- tests/devices/test_scaler.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/devices/test_scaler.py b/tests/devices/test_scaler.py index 6784964fef9..4c7af28f452 100644 --- a/tests/devices/test_scaler.py +++ b/tests/devices/test_scaler.py @@ -76,23 +76,13 @@ async def test_scaler_controller_trigger_sets_counting_true_then_false( async def test_simple_channel_scaler_read(hm3amp20_1: SimpleChannelScaler) -> None: - await assert_reading( - hm3amp20_1, - { - "hm3amp20_1-count": partial_reading(0), - }, - ) + await assert_reading(hm3amp20_1, {"hm3amp20_1-count": partial_reading(0)}) async def test_simple_channel_scaler_read_configuration( hm3amp20_1: SimpleChannelScaler, ) -> None: - await assert_configuration( - hm3amp20_1, - { - "scaler1-count_period": partial_reading(0), - }, - ) + await assert_configuration(hm3amp20_1, {"scaler1-count_period": partial_reading(0)}) async def test_simple_channel_scaler_trigger( From b77454fe287a3099c1d0705fe4b35f03ce06e48a Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 27 May 2026 14:41:31 +0000 Subject: [PATCH 04/10] Add set test --- tests/devices/test_scaler.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/devices/test_scaler.py b/tests/devices/test_scaler.py index 4c7af28f452..26b1f1827b2 100644 --- a/tests/devices/test_scaler.py +++ b/tests/devices/test_scaler.py @@ -2,7 +2,7 @@ from unittest.mock import AsyncMock, call import pytest -from ophyd_async.core import init_devices +from ophyd_async.core import get_mock_put, init_devices from ophyd_async.testing import assert_configuration, assert_reading, partial_reading from dodal.devices.scaler import ScalerController, SimpleChannelScaler @@ -85,6 +85,12 @@ async def test_simple_channel_scaler_read_configuration( await assert_configuration(hm3amp20_1, {"scaler1-count_period": partial_reading(0)}) +async def test_simple_channel_scaler_set(hm3amp20_1: SimpleChannelScaler) -> None: + value = 10 + await hm3amp20_1.set(value) + get_mock_put(hm3amp20_1.count).assert_awaited_once_with(value) + + async def test_simple_channel_scaler_trigger( scaler1: ScalerController, hm3amp20_1: SimpleChannelScaler, sm5amp8_1 ) -> None: From 5830f248983ef9b0725cca060927944be02b2600 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 27 May 2026 15:08:47 +0000 Subject: [PATCH 05/10] Correct the devices to use scaler2 --- src/dodal/beamlines/i09.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/dodal/beamlines/i09.py b/src/dodal/beamlines/i09.py index e9f10f0f7c3..b7c9c0c9d1c 100644 --- a/src/dodal/beamlines/i09.py +++ b/src/dodal/beamlines/i09.py @@ -174,20 +174,20 @@ def scaler2() -> ScalerController: @devices.factory -def hm3amp20(scaler1: ScalerController) -> SimpleChannelScaler: - return SimpleChannelScaler(scaler1, channel=2) +def hm3amp20(scaler2: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler2, channel=2) @devices.factory -def sm5amp8(scaler1: ScalerController) -> SimpleChannelScaler: - return SimpleChannelScaler(scaler1, channel=3) +def sm5amp8(scaler2: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler2, channel=3) @devices.factory -def smpmamp39(scaler1: ScalerController) -> SimpleChannelScaler: - return SimpleChannelScaler(scaler1, channel=4) +def smpmamp39(scaler2: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler2, channel=4) @devices.factory -def rfdamp10(scaler1: ScalerController) -> SimpleChannelScaler: - return SimpleChannelScaler(scaler1, channel=5) +def rfdamp10(scaler2: ScalerController) -> SimpleChannelScaler: + return SimpleChannelScaler(scaler2, channel=5) From b976a8dabf91fe9eefac0e5021383d26f377b2e9 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 27 May 2026 15:44:53 +0000 Subject: [PATCH 06/10] Fix typo --- src/dodal/devices/scaler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dodal/devices/scaler.py b/src/dodal/devices/scaler.py index bf31951e6dd..56d7d595f96 100644 --- a/src/dodal/devices/scaler.py +++ b/src/dodal/devices/scaler.py @@ -27,7 +27,7 @@ async def _on_put(value: bool): if value is True: asyncio.create_task(_finish_after_delay()) - # _on_put is called before the value given is to the signal. Therefore we must + # _on_put is called before the value is given to the signal. Therefore we must # setup a delay to set the signal back to False once it is True to simulate # hardware behaviour. callback_on_mock_put(device.counting, _on_put) From fe2a99f029b848fb0ab138fa171e8acc4eeb79d6 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 27 May 2026 15:59:22 +0000 Subject: [PATCH 07/10] Update mock logic to use subscribe_reading for more correct simulation --- src/dodal/devices/scaler.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/dodal/devices/scaler.py b/src/dodal/devices/scaler.py index 56d7d595f96..044d6693048 100644 --- a/src/dodal/devices/scaler.py +++ b/src/dodal/devices/scaler.py @@ -1,13 +1,12 @@ import asyncio -from bluesky.protocols import Triggerable +from bluesky.protocols import Reading, Triggerable from ophyd_async.core import ( AsyncStatus, DeviceMock, Reference, StandardReadable, StandardReadableFormat, - callback_on_mock_put, default_mock_class, set_and_wait_for_value, set_mock_value, @@ -19,18 +18,15 @@ class MockScalerController(DeviceMock["ScalerController"]): async def connect(self, device: "ScalerController"): set_mock_value(device.counting, False) - async def _finish_after_delay(): + async def _complete(): await asyncio.sleep(0.2) set_mock_value(device.counting, False) - async def _on_put(value: bool): - if value is True: - asyncio.create_task(_finish_after_delay()) + def _on_value(value: dict[str, Reading[bool]]): + if value[device.counting.name]["value"] is True: + asyncio.create_task(_complete()) - # _on_put is called before the value is given to the signal. Therefore we must - # setup a delay to set the signal back to False once it is True to simulate - # hardware behaviour. - callback_on_mock_put(device.counting, _on_put) + device.counting.subscribe_reading(_on_value) @default_mock_class(MockScalerController) @@ -52,12 +48,22 @@ def __init__(self, prefix: str, name: str = ""): @AsyncStatus.wrap async def trigger(self): + print("before set_and_wait") + self._acquire_status = await set_and_wait_for_value( self.counting, True, wait_for_set_completion=True ) + + print("after set_and_wait") + await self._acquire_status + + print("after acquire status") + await wait_for_good_state(self.counting, {False}) + print("after wait_for_good_state") + class SimpleChannelScaler(StandardReadable, Triggerable): """Create individual channel for a scaler. A ScalerController is used for the From a7229db2e3de94ab3cd0b8553a3baff2247a942f Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 27 May 2026 16:02:17 +0000 Subject: [PATCH 08/10] Add comment on why we shouldn't use callback_on_mock_put --- src/dodal/devices/scaler.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/dodal/devices/scaler.py b/src/dodal/devices/scaler.py index 044d6693048..d233fd7c0a3 100644 --- a/src/dodal/devices/scaler.py +++ b/src/dodal/devices/scaler.py @@ -26,6 +26,8 @@ def _on_value(value: dict[str, Reading[bool]]): if value[device.counting.name]["value"] is True: asyncio.create_task(_complete()) + # Can't use callback_on_mock_put as this is called before the mock put, we need + # to simulate after mock put update. device.counting.subscribe_reading(_on_value) From 19792292af5e1849fbb3bab2953ad457428c3803 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 28 May 2026 08:07:01 +0000 Subject: [PATCH 09/10] Remove debug print statements --- src/dodal/devices/scaler.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/dodal/devices/scaler.py b/src/dodal/devices/scaler.py index d233fd7c0a3..6477a548b16 100644 --- a/src/dodal/devices/scaler.py +++ b/src/dodal/devices/scaler.py @@ -50,22 +50,12 @@ def __init__(self, prefix: str, name: str = ""): @AsyncStatus.wrap async def trigger(self): - print("before set_and_wait") - self._acquire_status = await set_and_wait_for_value( self.counting, True, wait_for_set_completion=True ) - - print("after set_and_wait") - await self._acquire_status - - print("after acquire status") - await wait_for_good_state(self.counting, {False}) - print("after wait_for_good_state") - class SimpleChannelScaler(StandardReadable, Triggerable): """Create individual channel for a scaler. A ScalerController is used for the From 424cc6fcc087485998a6c6cf6524d68ead48a93b Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 29 May 2026 14:19:18 +0000 Subject: [PATCH 10/10] Rename count_period to count_time --- src/dodal/devices/scaler.py | 4 ++-- tests/devices/test_scaler.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dodal/devices/scaler.py b/src/dodal/devices/scaler.py index 6477a548b16..9e7aa63d69b 100644 --- a/src/dodal/devices/scaler.py +++ b/src/dodal/devices/scaler.py @@ -40,7 +40,7 @@ class ScalerController(StandardReadable, Triggerable): def __init__(self, prefix: str, name: str = ""): self.counting = epics_signal_rw(bool, prefix + ".CNT") with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): - self.count_period = epics_signal_rw(float, prefix + ".TP") + self.count_time = epics_signal_rw(float, prefix + ".TP") self._acquire_status: AsyncStatus | None = None # Store the prefix so that the SimpleChannelScaler can reuse. @@ -82,7 +82,7 @@ def __init__( scalar_controller.add_readables([self]) # Avoid circular read configuration by specifying individual signal self.add_readables( - [scalar_controller.count_period], StandardReadableFormat.CONFIG_SIGNAL + [scalar_controller.count_time], StandardReadableFormat.CONFIG_SIGNAL ) @AsyncStatus.wrap diff --git a/tests/devices/test_scaler.py b/tests/devices/test_scaler.py index 26b1f1827b2..4539c81ff52 100644 --- a/tests/devices/test_scaler.py +++ b/tests/devices/test_scaler.py @@ -48,7 +48,7 @@ async def test_scaler_controller_read_with_multiple_channels( async def test_scaler_controller_read_configuration(scaler1: ScalerController) -> None: - await assert_configuration(scaler1, {"scaler1-count_period": partial_reading(0.0)}) + await assert_configuration(scaler1, {"scaler1-count_time": partial_reading(0.0)}) async def test_scaler_controller_trigger_waits_for_counting_to_finish( @@ -82,7 +82,7 @@ async def test_simple_channel_scaler_read(hm3amp20_1: SimpleChannelScaler) -> No async def test_simple_channel_scaler_read_configuration( hm3amp20_1: SimpleChannelScaler, ) -> None: - await assert_configuration(hm3amp20_1, {"scaler1-count_period": partial_reading(0)}) + await assert_configuration(hm3amp20_1, {"scaler1-count_time": partial_reading(0)}) async def test_simple_channel_scaler_set(hm3amp20_1: SimpleChannelScaler) -> None: