From e90bd1d66009693d32b6df0ba101b715363b86f9 Mon Sep 17 00:00:00 2001 From: Relm-Arrowny Date: Thu, 16 Jul 2026 14:27:21 +0000 Subject: [PATCH 1/7] add temperature controller --- src/dodal/beamlines/i10_1.py | 58 ++++++++++ .../temperature_controller.py | 105 ++++++++++++++++++ 2 files changed, 163 insertions(+) create mode 100644 src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py diff --git a/src/dodal/beamlines/i10_1.py b/src/dodal/beamlines/i10_1.py index ef0a282fb47..7e63a4f60d2 100644 --- a/src/dodal/beamlines/i10_1.py +++ b/src/dodal/beamlines/i10_1.py @@ -8,6 +8,12 @@ HighFieldMagnet, I10JScalerCard, ) +from dodal.devices.beamlines.i10_1.high_field_magnet.temperature_controller import ( + BasePID, + HighFieldMagnetHeater, + HighFieldMagnetTemperatureSensor, + TemperatureController, +) from dodal.devices.common_mirror import XYZPiezoCollimatingMirror from dodal.devices.current_amplifiers import ( SR570, @@ -254,3 +260,55 @@ def high_field_magnet() -> HighFieldMagnet: return HighFieldMagnet( prefix=f"{PREFIX.beamline_prefix}-EA-SMC-01:", ) + + +@devices.factory() +def hfm_temp() -> TemperatureController: + sensor = HighFieldMagnetTemperatureSensor( + prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-01:" + ) + heater = HighFieldMagnetHeater(prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-01:") + pid = BasePID( + prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-01:", + ) + return TemperatureController( + prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-01:", + suffix="TTEMP:SET", + sensor=sensor, + heater=heater, + pid=pid, + ) + + +@devices.factory() +def hfm_vti() -> TemperatureController: + prefix = f"{PREFIX.beamline_prefix}-EA-TCTRL-02:" + sensor = HighFieldMagnetTemperatureSensor(prefix=prefix) + heater = HighFieldMagnetHeater(prefix=prefix) + pid = BasePID( + prefix=prefix, + ) + return TemperatureController( + prefix=prefix, + suffix="TTEMP:SET", + sensor=sensor, + heater=heater, + pid=pid, + ) + + +@devices.factory() +def hfm_he3() -> TemperatureController: + prefix = f"{PREFIX.beamline_prefix}-EA-TCTRL-03:" + sensor = HighFieldMagnetTemperatureSensor(prefix=prefix) + heater = HighFieldMagnetHeater(prefix=prefix) + pid = BasePID( + prefix=prefix, + ) + return TemperatureController( + prefix=prefix, + suffix="TTEMP:SET", + sensor=sensor, + heater=heater, + pid=pid, + ) diff --git a/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py b/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py new file mode 100644 index 00000000000..7414edb136b --- /dev/null +++ b/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py @@ -0,0 +1,105 @@ +from dataclasses import dataclass +from functools import cached_property + +from ophyd_async.core import ( + SignalR, + SignalRW, + StandardMovable, + StandardReadable, + StandardReadableFormat, + StrictEnum, + soft_signal_rw, +) +from ophyd_async.epics.core import epics_signal_r, epics_signal_rw + +from dodal.devices.movable import MovableWithToleranceLogic + + +@dataclass +class TemperatureMovableLogic(MovableWithToleranceLogic): + async def stop(self): + current_val = await self.readback.get_value() + await self.setpoint.set(current_val) + + +class BasePID(StandardReadable): + def __init__( + self, + prefix: str, + suffix_p: str = "P", + suffix_i: str = "I", + suffix_d: str = "D", + name: str = "", + ): + with self.add_children_as_readables(): + self.p = epics_signal_rw(float, prefix + suffix_p) + self.i = epics_signal_rw(float, prefix + suffix_i) + self.d = epics_signal_rw(float, prefix + suffix_d) + super().__init__(name=name) + + +class BaseHeater(StandardReadable): + setpoint: SignalRW[float] + output: SignalR[float] + + +class BaseTemperatureSensor(StandardReadable): + temperature: SignalR[float] + + +class HeaterMode(StrictEnum): + MANUAL = "Manual" + AUTO = "Auto" + + +class HighFieldMagnetTemperatureSensor(BaseTemperatureSensor): + def __init__(self, prefix: str, suffix: str = "STEMP", name: str = ""): + with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): + self.temperature = epics_signal_r(float, prefix + suffix) + with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): + self.config_readback_2 = epics_signal_r(float, prefix + suffix + "2") + self.config_readback_3 = epics_signal_r(float, prefix + suffix + "3") + + super().__init__(name=name) + + +class HighFieldMagnetHeater(BaseHeater): + def __init__(self, prefix: str, name: str = ""): + with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): + self.setpoint = epics_signal_rw(float, prefix + "MANV:SET") + with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): + self.output = epics_signal_r(float, prefix + "HEATERP") + self.mode = epics_signal_rw( + HeaterMode, + read_pv=prefix + "ACTIVITY", + write_pv=prefix + "ACTIVITY:SET", + ) + super().__init__(name=name) + + +class TemperatureController(StandardReadable, StandardMovable): + def __init__( + self, + prefix: str, + suffix: str, + sensor: BaseTemperatureSensor, + heater: BaseHeater, + pid: BasePID, + name: str = "", + ): + with self.add_children_as_readables(): + self.heater = heater + self.pid = pid + self.sensor = sensor + with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): + self.tolerance = soft_signal_rw(float, initial_value=0.1) + self.user_setpoint = epics_signal_rw(float, prefix + suffix) + super().__init__(name=name) + + @cached_property + def movable_logic(self) -> TemperatureMovableLogic: + return TemperatureMovableLogic( + setpoint=self.user_setpoint, + readback=self.sensor.temperature, + tolerance=self.tolerance, + ) From 0e94bc12f15789b8ecc471ac1e919a7f31da8a20 Mon Sep 17 00:00:00 2001 From: Relm-Arrowny Date: Thu, 16 Jul 2026 15:02:18 +0000 Subject: [PATCH 2/7] clean up --- src/dodal/beamlines/i10_1.py | 44 ++----------- .../temperature_controller.py | 61 ++++++++++++++++--- 2 files changed, 59 insertions(+), 46 deletions(-) diff --git a/src/dodal/beamlines/i10_1.py b/src/dodal/beamlines/i10_1.py index 7e63a4f60d2..a501cf7cd11 100644 --- a/src/dodal/beamlines/i10_1.py +++ b/src/dodal/beamlines/i10_1.py @@ -9,9 +9,6 @@ I10JScalerCard, ) from dodal.devices.beamlines.i10_1.high_field_magnet.temperature_controller import ( - BasePID, - HighFieldMagnetHeater, - HighFieldMagnetTemperatureSensor, TemperatureController, ) from dodal.devices.common_mirror import XYZPiezoCollimatingMirror @@ -264,51 +261,20 @@ def high_field_magnet() -> HighFieldMagnet: @devices.factory() def hfm_temp() -> TemperatureController: - sensor = HighFieldMagnetTemperatureSensor( + return TemperatureController.from_prefix( prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-01:" ) - heater = HighFieldMagnetHeater(prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-01:") - pid = BasePID( - prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-01:", - ) - return TemperatureController( - prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-01:", - suffix="TTEMP:SET", - sensor=sensor, - heater=heater, - pid=pid, - ) @devices.factory() def hfm_vti() -> TemperatureController: - prefix = f"{PREFIX.beamline_prefix}-EA-TCTRL-02:" - sensor = HighFieldMagnetTemperatureSensor(prefix=prefix) - heater = HighFieldMagnetHeater(prefix=prefix) - pid = BasePID( - prefix=prefix, - ) - return TemperatureController( - prefix=prefix, - suffix="TTEMP:SET", - sensor=sensor, - heater=heater, - pid=pid, + return TemperatureController.from_prefix( + prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-02:" ) @devices.factory() def hfm_he3() -> TemperatureController: - prefix = f"{PREFIX.beamline_prefix}-EA-TCTRL-03:" - sensor = HighFieldMagnetTemperatureSensor(prefix=prefix) - heater = HighFieldMagnetHeater(prefix=prefix) - pid = BasePID( - prefix=prefix, - ) - return TemperatureController( - prefix=prefix, - suffix="TTEMP:SET", - sensor=sensor, - heater=heater, - pid=pid, + return TemperatureController.from_prefix( + prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-03:" ) diff --git a/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py b/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py index 7414edb136b..07d6846d97d 100644 --- a/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py +++ b/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py @@ -1,3 +1,6 @@ +from __future__ import annotations + +from abc import ABC, abstractmethod from dataclasses import dataclass from functools import cached_property @@ -43,8 +46,11 @@ class BaseHeater(StandardReadable): output: SignalR[float] -class BaseTemperatureSensor(StandardReadable): - temperature: SignalR[float] +class BaseTemperatureSensor(StandardReadable, ABC): + @property + @abstractmethod + def temperature(self) -> SignalR[float]: + pass class HeaterMode(StrictEnum): @@ -53,15 +59,33 @@ class HeaterMode(StrictEnum): class HighFieldMagnetTemperatureSensor(BaseTemperatureSensor): - def __init__(self, prefix: str, suffix: str = "STEMP", name: str = ""): + def __init__( + self, + prefix: str, + suffix: str = "STEMP", + config_suffixes: list[str] | None = None, + name: str = "", + ): + config_suffixes = config_suffixes or ["2", "3"] + with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): - self.temperature = epics_signal_r(float, prefix + suffix) - with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): - self.config_readback_2 = epics_signal_r(float, prefix + suffix + "2") - self.config_readback_3 = epics_signal_r(float, prefix + suffix + "3") + self._temperature = epics_signal_r(float, prefix + suffix) + with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): + for sfx in config_suffixes: + signal_name = f"config_{sfx}" + if hasattr(self, signal_name): + raise AttributeError( + f"Cannot add configuration signal '{signal_name}': attribute already exists." + ) + signal = epics_signal_r(float, prefix + suffix + sfx) + setattr(self, signal_name, signal) super().__init__(name=name) + @property + def temperature(self) -> SignalR[float]: + return self._temperature + class HighFieldMagnetHeater(BaseHeater): def __init__(self, prefix: str, name: str = ""): @@ -103,3 +127,26 @@ def movable_logic(self) -> TemperatureMovableLogic: readback=self.sensor.temperature, tolerance=self.tolerance, ) + + @classmethod + def from_prefix( + cls, + prefix: str, + suffix: str = "TTEMP:SET", + config_suffixes: list[str] | None = None, + name: str = "", + ) -> TemperatureController: + """Helper factory to instantiate the entire stack from a single prefix.""" + sensor = HighFieldMagnetTemperatureSensor( + prefix=prefix, config_suffixes=config_suffixes + ) + heater = HighFieldMagnetHeater(prefix=prefix) + pid = BasePID(prefix=prefix) + return cls( + prefix=prefix, + suffix=suffix, + sensor=sensor, + heater=heater, + pid=pid, + name=name, + ) From 7a54be35e9fb6dfee6079bdcf411de55613c0fe7 Mon Sep 17 00:00:00 2001 From: Relm-Arrowny Date: Fri, 17 Jul 2026 12:10:20 +0000 Subject: [PATCH 3/7] test --- .../temperature_controller.py | 20 +++--- .../test_temperature_controller.py | 71 +++++++++++++++++++ 2 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py diff --git a/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py b/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py index 07d6846d97d..54c47bf5e46 100644 --- a/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py +++ b/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py @@ -25,7 +25,7 @@ async def stop(self): await self.setpoint.set(current_val) -class BasePID(StandardReadable): +class PID(StandardReadable): def __init__( self, prefix: str, @@ -90,14 +90,15 @@ def temperature(self) -> SignalR[float]: class HighFieldMagnetHeater(BaseHeater): def __init__(self, prefix: str, name: str = ""): with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): - self.setpoint = epics_signal_rw(float, prefix + "MANV:SET") - with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): - self.output = epics_signal_r(float, prefix + "HEATERP") self.mode = epics_signal_rw( HeaterMode, read_pv=prefix + "ACTIVITY", write_pv=prefix + "ACTIVITY:SET", ) + self.setpoint = epics_signal_rw(float, prefix + "MANV:SET") + with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): + self.output = epics_signal_r(float, prefix + "HEATERP") + super().__init__(name=name) @@ -108,16 +109,19 @@ def __init__( suffix: str, sensor: BaseTemperatureSensor, heater: BaseHeater, - pid: BasePID, + pid: PID, name: str = "", ): + + self.pid = pid + self.heater = heater with self.add_children_as_readables(): - self.heater = heater - self.pid = pid self.sensor = sensor + with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): self.tolerance = soft_signal_rw(float, initial_value=0.1) self.user_setpoint = epics_signal_rw(float, prefix + suffix) + super().__init__(name=name) @cached_property @@ -141,7 +145,7 @@ def from_prefix( prefix=prefix, config_suffixes=config_suffixes ) heater = HighFieldMagnetHeater(prefix=prefix) - pid = BasePID(prefix=prefix) + pid = PID(prefix=prefix) return cls( prefix=prefix, suffix=suffix, diff --git a/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py b/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py new file mode 100644 index 00000000000..19820fb7a85 --- /dev/null +++ b/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py @@ -0,0 +1,71 @@ +import asyncio + +import pytest +from ophyd_async.core import init_devices, set_mock_value +from ophyd_async.testing import ( + assert_configuration, + assert_reading, + partial_reading, +) + +from dodal.devices.beamlines.i10_1.high_field_magnet.temperature_controller import ( + TemperatureController, +) + + +@pytest.fixture +def hfm_temp_controller() -> TemperatureController: + with init_devices(mock=True): + hfm_temp_controller = TemperatureController.from_prefix( + prefix="I10J-MAGNET-01:TEMP:", + suffix="TTEMP:SET", + ) + return hfm_temp_controller + + +async def test_temperature_controller_readback( + hfm_temp_controller: TemperatureController, +): + await hfm_temp_controller.set(1.0) + + await assert_reading( + hfm_temp_controller, + { + "hfm_temp_controller": partial_reading(1.0), + }, + ) + await assert_configuration( + hfm_temp_controller, + { + "hfm_temp_controller-sensor-config_2": partial_reading(0.0), + "hfm_temp_controller-sensor-config_3": partial_reading(0.0), + "hfm_temp_controller-tolerance": partial_reading(0.1), + "hfm_temp_controller-user_setpoint": partial_reading(1.0), + }, + ) + await asyncio.gather( + hfm_temp_controller.pid.p.set(1.0), + hfm_temp_controller.pid.i.set(2.0), + hfm_temp_controller.pid.d.set(3.0), + ) + await assert_reading( + hfm_temp_controller.pid, + { + "hfm_temp_controller-pid-p": partial_reading(1.0), + "hfm_temp_controller-pid-i": partial_reading(2.0), + "hfm_temp_controller-pid-d": partial_reading(3.0), + }, + ) + + +async def test_temperature_controller_stop(hfm_temp_controller: TemperatureController): + + set_mock_value(hfm_temp_controller.user_setpoint, 1.0) + set_mock_value(hfm_temp_controller.sensor.temperature, 0.5) + await hfm_temp_controller.stop() + await assert_reading( + hfm_temp_controller, + { + "hfm_temp_controller": partial_reading(0.5), + }, + ) From 5edb33136b2af98f69f310b36efbd348ce846a2e Mon Sep 17 00:00:00 2001 From: Relm-Arrowny Date: Fri, 17 Jul 2026 12:27:00 +0000 Subject: [PATCH 4/7] move base devices to common --- .../temperature_controller.py | 87 ++---------------- .../devices/temperture_controller/__init__.py | 18 +++- .../basic_temperature_controller.py | 89 +++++++++++++++++++ .../test_temperature_controller.py | 12 +-- 4 files changed, 122 insertions(+), 84 deletions(-) create mode 100644 src/dodal/devices/temperture_controller/basic_temperature_controller.py diff --git a/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py b/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py index 54c47bf5e46..afc47ef3b11 100644 --- a/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py +++ b/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py @@ -1,56 +1,18 @@ from __future__ import annotations -from abc import ABC, abstractmethod -from dataclasses import dataclass -from functools import cached_property - from ophyd_async.core import ( SignalR, - SignalRW, - StandardMovable, - StandardReadable, StandardReadableFormat, StrictEnum, - soft_signal_rw, ) from ophyd_async.epics.core import epics_signal_r, epics_signal_rw -from dodal.devices.movable import MovableWithToleranceLogic - - -@dataclass -class TemperatureMovableLogic(MovableWithToleranceLogic): - async def stop(self): - current_val = await self.readback.get_value() - await self.setpoint.set(current_val) - - -class PID(StandardReadable): - def __init__( - self, - prefix: str, - suffix_p: str = "P", - suffix_i: str = "I", - suffix_d: str = "D", - name: str = "", - ): - with self.add_children_as_readables(): - self.p = epics_signal_rw(float, prefix + suffix_p) - self.i = epics_signal_rw(float, prefix + suffix_i) - self.d = epics_signal_rw(float, prefix + suffix_d) - super().__init__(name=name) - - -class BaseHeater(StandardReadable): - setpoint: SignalRW[float] - output: SignalR[float] - - -class BaseTemperatureSensor(StandardReadable, ABC): - @property - @abstractmethod - def temperature(self) -> SignalR[float]: - pass +from dodal.devices.temperture_controller import ( + PID, + BaseHeater, + BaseTemperatureSensor, + TemperatureController, +) class HeaterMode(StrictEnum): @@ -102,51 +64,20 @@ def __init__(self, prefix: str, name: str = ""): super().__init__(name=name) -class TemperatureController(StandardReadable, StandardMovable): +class HFMTemperatureController(TemperatureController): def __init__( self, prefix: str, - suffix: str, - sensor: BaseTemperatureSensor, - heater: BaseHeater, - pid: PID, - name: str = "", - ): - - self.pid = pid - self.heater = heater - with self.add_children_as_readables(): - self.sensor = sensor - - with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): - self.tolerance = soft_signal_rw(float, initial_value=0.1) - self.user_setpoint = epics_signal_rw(float, prefix + suffix) - - super().__init__(name=name) - - @cached_property - def movable_logic(self) -> TemperatureMovableLogic: - return TemperatureMovableLogic( - setpoint=self.user_setpoint, - readback=self.sensor.temperature, - tolerance=self.tolerance, - ) - - @classmethod - def from_prefix( - cls, - prefix: str, suffix: str = "TTEMP:SET", config_suffixes: list[str] | None = None, name: str = "", - ) -> TemperatureController: - """Helper factory to instantiate the entire stack from a single prefix.""" + ): sensor = HighFieldMagnetTemperatureSensor( prefix=prefix, config_suffixes=config_suffixes ) heater = HighFieldMagnetHeater(prefix=prefix) pid = PID(prefix=prefix) - return cls( + super().__init__( prefix=prefix, suffix=suffix, sensor=sensor, diff --git a/src/dodal/devices/temperture_controller/__init__.py b/src/dodal/devices/temperture_controller/__init__.py index 5391f25dd66..1a99df13388 100644 --- a/src/dodal/devices/temperture_controller/__init__.py +++ b/src/dodal/devices/temperture_controller/__init__.py @@ -1,3 +1,19 @@ +from .basic_temperature_controller import ( + PID, + BaseHeater, + BaseTemperatureSensor, + HeaterMode, + TemperatureController, +) from .lakeshore.lakeshore import Lakeshore, Lakeshore336, Lakeshore340 -__all__ = ["Lakeshore336", "Lakeshore340", "Lakeshore"] +__all__ = [ + "Lakeshore336", + "Lakeshore340", + "Lakeshore", + "BaseHeater", + "BaseTemperatureSensor", + "HeaterMode", + "PID", + "TemperatureController", +] diff --git a/src/dodal/devices/temperture_controller/basic_temperature_controller.py b/src/dodal/devices/temperture_controller/basic_temperature_controller.py new file mode 100644 index 00000000000..01672c833ea --- /dev/null +++ b/src/dodal/devices/temperture_controller/basic_temperature_controller.py @@ -0,0 +1,89 @@ +from __future__ import annotations + +from abc import ABC, abstractmethod +from dataclasses import dataclass +from functools import cached_property + +from ophyd_async.core import ( + SignalR, + SignalRW, + StandardMovable, + StandardReadable, + StandardReadableFormat, + StrictEnum, + soft_signal_rw, +) +from ophyd_async.epics.core import epics_signal_rw + +from dodal.devices.movable import MovableWithToleranceLogic + + +@dataclass +class TemperatureMovableLogic(MovableWithToleranceLogic): + async def stop(self): + current_val = await self.readback.get_value() + await self.setpoint.set(current_val) + + +class PID(StandardReadable): + def __init__( + self, + prefix: str, + suffix_p: str = "P", + suffix_i: str = "I", + suffix_d: str = "D", + name: str = "", + ): + with self.add_children_as_readables(): + self.p = epics_signal_rw(float, prefix + suffix_p) + self.i = epics_signal_rw(float, prefix + suffix_i) + self.d = epics_signal_rw(float, prefix + suffix_d) + super().__init__(name=name) + + +class BaseHeater(StandardReadable): + setpoint: SignalRW[float] + output: SignalR[float] + + +class BaseTemperatureSensor(StandardReadable, ABC): + @property + @abstractmethod + def temperature(self) -> SignalR[float]: + pass + + +class HeaterMode(StrictEnum): + MANUAL = "Manual" + AUTO = "Auto" + + +class TemperatureController(StandardReadable, StandardMovable): + def __init__( + self, + prefix: str, + suffix: str, + sensor: BaseTemperatureSensor, + heater: BaseHeater, + pid: PID, + name: str = "", + ): + + self.pid = pid + self.heater = heater + with self.add_children_as_readables(): + self.sensor = sensor + + with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): + self.tolerance = soft_signal_rw(float, initial_value=0.1) + self.user_setpoint = epics_signal_rw(float, prefix + suffix) + + super().__init__(name=name) + + @cached_property + def movable_logic(self) -> TemperatureMovableLogic: + return TemperatureMovableLogic( + setpoint=self.user_setpoint, + readback=self.sensor.temperature, + tolerance=self.tolerance, + ) diff --git a/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py b/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py index 19820fb7a85..4bb0fad1bde 100644 --- a/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py +++ b/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py @@ -9,14 +9,14 @@ ) from dodal.devices.beamlines.i10_1.high_field_magnet.temperature_controller import ( - TemperatureController, + HFMTemperatureController, ) @pytest.fixture -def hfm_temp_controller() -> TemperatureController: +def hfm_temp_controller() -> HFMTemperatureController: with init_devices(mock=True): - hfm_temp_controller = TemperatureController.from_prefix( + hfm_temp_controller = HFMTemperatureController( prefix="I10J-MAGNET-01:TEMP:", suffix="TTEMP:SET", ) @@ -24,7 +24,7 @@ def hfm_temp_controller() -> TemperatureController: async def test_temperature_controller_readback( - hfm_temp_controller: TemperatureController, + hfm_temp_controller: HFMTemperatureController, ): await hfm_temp_controller.set(1.0) @@ -58,7 +58,9 @@ async def test_temperature_controller_readback( ) -async def test_temperature_controller_stop(hfm_temp_controller: TemperatureController): +async def test_temperature_controller_stop( + hfm_temp_controller: HFMTemperatureController, +): set_mock_value(hfm_temp_controller.user_setpoint, 1.0) set_mock_value(hfm_temp_controller.sensor.temperature, 0.5) From 81f3fce6445577d5992143ae6aacc2c1965694d0 Mon Sep 17 00:00:00 2001 From: Relm-Arrowny Date: Fri, 17 Jul 2026 12:34:19 +0000 Subject: [PATCH 5/7] fix i10_1 --- src/dodal/beamlines/i10_1.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/dodal/beamlines/i10_1.py b/src/dodal/beamlines/i10_1.py index a501cf7cd11..5a76a8cff34 100644 --- a/src/dodal/beamlines/i10_1.py +++ b/src/dodal/beamlines/i10_1.py @@ -9,7 +9,7 @@ I10JScalerCard, ) from dodal.devices.beamlines.i10_1.high_field_magnet.temperature_controller import ( - TemperatureController, + HFMTemperatureController, ) from dodal.devices.common_mirror import XYZPiezoCollimatingMirror from dodal.devices.current_amplifiers import ( @@ -260,21 +260,15 @@ def high_field_magnet() -> HighFieldMagnet: @devices.factory() -def hfm_temp() -> TemperatureController: - return TemperatureController.from_prefix( - prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-01:" - ) +def hfm_temp() -> HFMTemperatureController: + return HFMTemperatureController(prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-01:") @devices.factory() -def hfm_vti() -> TemperatureController: - return TemperatureController.from_prefix( - prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-02:" - ) +def hfm_vti() -> HFMTemperatureController: + return HFMTemperatureController(prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-02:") @devices.factory() -def hfm_he3() -> TemperatureController: - return TemperatureController.from_prefix( - prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-03:" - ) +def hfm_he3() -> HFMTemperatureController: + return HFMTemperatureController(prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-03:") From 7b1fbca2b5030e072b2114e0a78a9861332c2133 Mon Sep 17 00:00:00 2001 From: Relm-Arrowny Date: Fri, 17 Jul 2026 14:31:25 +0000 Subject: [PATCH 6/7] move test --- .../devices/temperture_controller/__init__.py | 4 +- ...ller.py => base_temperature_controller.py} | 15 ++- .../test_temperature_controller.py | 17 +-- .../test_base_temperature_controller.py | 121 ++++++++++++++++++ 4 files changed, 131 insertions(+), 26 deletions(-) rename src/dodal/devices/temperture_controller/{basic_temperature_controller.py => base_temperature_controller.py} (87%) create mode 100644 tests/devices/temperature_controller/test_base_temperature_controller.py diff --git a/src/dodal/devices/temperture_controller/__init__.py b/src/dodal/devices/temperture_controller/__init__.py index 1a99df13388..d126a7344f9 100644 --- a/src/dodal/devices/temperture_controller/__init__.py +++ b/src/dodal/devices/temperture_controller/__init__.py @@ -1,8 +1,7 @@ -from .basic_temperature_controller import ( +from .base_temperature_controller import ( PID, BaseHeater, BaseTemperatureSensor, - HeaterMode, TemperatureController, ) from .lakeshore.lakeshore import Lakeshore, Lakeshore336, Lakeshore340 @@ -13,7 +12,6 @@ "Lakeshore", "BaseHeater", "BaseTemperatureSensor", - "HeaterMode", "PID", "TemperatureController", ] diff --git a/src/dodal/devices/temperture_controller/basic_temperature_controller.py b/src/dodal/devices/temperture_controller/base_temperature_controller.py similarity index 87% rename from src/dodal/devices/temperture_controller/basic_temperature_controller.py rename to src/dodal/devices/temperture_controller/base_temperature_controller.py index 01672c833ea..76397fdc29a 100644 --- a/src/dodal/devices/temperture_controller/basic_temperature_controller.py +++ b/src/dodal/devices/temperture_controller/base_temperature_controller.py @@ -3,6 +3,7 @@ from abc import ABC, abstractmethod from dataclasses import dataclass from functools import cached_property +from typing import Generic, TypeVar from ophyd_async.core import ( SignalR, @@ -10,7 +11,6 @@ StandardMovable, StandardReadable, StandardReadableFormat, - StrictEnum, soft_signal_rw, ) from ophyd_async.epics.core import epics_signal_rw @@ -53,18 +53,19 @@ def temperature(self) -> SignalR[float]: pass -class HeaterMode(StrictEnum): - MANUAL = "Manual" - AUTO = "Auto" +SensorT = TypeVar("SensorT", bound=BaseTemperatureSensor) +HeaterT = TypeVar("HeaterT", bound=BaseHeater) -class TemperatureController(StandardReadable, StandardMovable): +class TemperatureController( + StandardReadable, StandardMovable, Generic[SensorT, HeaterT] +): def __init__( self, prefix: str, suffix: str, - sensor: BaseTemperatureSensor, - heater: BaseHeater, + sensor: SensorT, + heater: HeaterT, pid: PID, name: str = "", ): diff --git a/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py b/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py index 4bb0fad1bde..0af721160c9 100644 --- a/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py +++ b/tests/devices/beamlines/i10_1/high_field_magnet/test_temperature_controller.py @@ -1,7 +1,7 @@ import asyncio import pytest -from ophyd_async.core import init_devices, set_mock_value +from ophyd_async.core import init_devices from ophyd_async.testing import ( assert_configuration, assert_reading, @@ -56,18 +56,3 @@ async def test_temperature_controller_readback( "hfm_temp_controller-pid-d": partial_reading(3.0), }, ) - - -async def test_temperature_controller_stop( - hfm_temp_controller: HFMTemperatureController, -): - - set_mock_value(hfm_temp_controller.user_setpoint, 1.0) - set_mock_value(hfm_temp_controller.sensor.temperature, 0.5) - await hfm_temp_controller.stop() - await assert_reading( - hfm_temp_controller, - { - "hfm_temp_controller": partial_reading(0.5), - }, - ) diff --git a/tests/devices/temperature_controller/test_base_temperature_controller.py b/tests/devices/temperature_controller/test_base_temperature_controller.py new file mode 100644 index 00000000000..ec540c833de --- /dev/null +++ b/tests/devices/temperature_controller/test_base_temperature_controller.py @@ -0,0 +1,121 @@ +import asyncio + +import pytest +from ophyd_async.core import ( + SignalR, + StandardReadableFormat, + init_devices, + set_mock_value, +) +from ophyd_async.epics.core import epics_signal_r, epics_signal_rw +from ophyd_async.testing import assert_configuration, assert_reading, partial_reading + +from dodal.devices.temperture_controller import ( + PID, + BaseHeater, + BaseTemperatureSensor, + TemperatureController, +) + + +class MinimalMockSensor(BaseTemperatureSensor): + def __init__(self, name: str = ""): + with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): + self._temperature = epics_signal_r(float, "prefix + suffix)") + super().__init__(name=name) + + @property + def temperature(self) -> SignalR[float]: + return self._temperature + + +class MockHeater(BaseHeater): + def __init__(self, name: str = ""): + self.setpoint = epics_signal_rw(float, "prefix + suffix:SET") + self.output = epics_signal_r(float, "prefix + suffix") + super().__init__(name=name) + + +class MockTemperatureController(TemperatureController): + def __init__(self, name: str = ""): + sensor = MinimalMockSensor() + heater = MockHeater() + pid = PID(prefix="TEST-PID:") + super().__init__( + prefix="TEST-CTRL:", + suffix="SET", + sensor=sensor, + heater=heater, + pid=pid, + name=name, + ) + + +@pytest.fixture +def mock_controller() -> MockTemperatureController: + with init_devices(mock=True): + mock_controller = MockTemperatureController() + return mock_controller + + +async def test_temperature_movable(mock_controller: MockTemperatureController): + + set_mock_value(mock_controller.tolerance, 0.5) + set_mock_value(mock_controller.sensor.temperature, 10.0) + + status = mock_controller.set(20.0) + assert not status.done + set_mock_value(mock_controller.sensor.temperature, 19.4) + + assert not status.done + set_mock_value(mock_controller.sensor.temperature, 19.8) + await status + assert status.done + assert status.success + + +async def test_temperature_controller_readback( + mock_controller: MockTemperatureController, +): + await mock_controller.set(1.0) + await assert_reading( + mock_controller, + { + "mock_controller": partial_reading(1.0), + }, + ) + await assert_configuration( + mock_controller, + { + "mock_controller-tolerance": partial_reading(0.1), + "mock_controller-user_setpoint": partial_reading(1.0), + }, + ) + await asyncio.gather( + mock_controller.pid.p.set(1.0), + mock_controller.pid.i.set(2.0), + mock_controller.pid.d.set(3.0), + ) + await assert_reading( + mock_controller.pid, + { + "mock_controller-pid-p": partial_reading(1.0), + "mock_controller-pid-i": partial_reading(2.0), + "mock_controller-pid-d": partial_reading(3.0), + }, + ) + + +async def test_temperature_controller_stop( + mock_controller: MockTemperatureController, +): + + set_mock_value(mock_controller.user_setpoint, 1.0) + set_mock_value(mock_controller.sensor.temperature, 0.5) + await mock_controller.stop() + await assert_reading( + mock_controller, + { + "mock_controller": partial_reading(0.5), + }, + ) From dc0581eb8d7623939215688140a43064450a3dcb Mon Sep 17 00:00:00 2001 From: Relm-Arrowny Date: Fri, 17 Jul 2026 14:35:03 +0000 Subject: [PATCH 7/7] remove abc requirement for sensor --- .../i10_1/high_field_magnet/temperature_controller.py | 7 +------ .../temperture_controller/base_temperature_controller.py | 8 ++------ .../test_base_temperature_controller.py | 7 +------ 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py b/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py index afc47ef3b11..0aaafd7ae5b 100644 --- a/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py +++ b/src/dodal/devices/beamlines/i10_1/high_field_magnet/temperature_controller.py @@ -1,7 +1,6 @@ from __future__ import annotations from ophyd_async.core import ( - SignalR, StandardReadableFormat, StrictEnum, ) @@ -31,7 +30,7 @@ def __init__( config_suffixes = config_suffixes or ["2", "3"] with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): - self._temperature = epics_signal_r(float, prefix + suffix) + self.temperature = epics_signal_r(float, prefix + suffix) with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): for sfx in config_suffixes: @@ -44,10 +43,6 @@ def __init__( setattr(self, signal_name, signal) super().__init__(name=name) - @property - def temperature(self) -> SignalR[float]: - return self._temperature - class HighFieldMagnetHeater(BaseHeater): def __init__(self, prefix: str, name: str = ""): diff --git a/src/dodal/devices/temperture_controller/base_temperature_controller.py b/src/dodal/devices/temperture_controller/base_temperature_controller.py index 76397fdc29a..db16f1e2b04 100644 --- a/src/dodal/devices/temperture_controller/base_temperature_controller.py +++ b/src/dodal/devices/temperture_controller/base_temperature_controller.py @@ -1,6 +1,5 @@ from __future__ import annotations -from abc import ABC, abstractmethod from dataclasses import dataclass from functools import cached_property from typing import Generic, TypeVar @@ -46,11 +45,8 @@ class BaseHeater(StandardReadable): output: SignalR[float] -class BaseTemperatureSensor(StandardReadable, ABC): - @property - @abstractmethod - def temperature(self) -> SignalR[float]: - pass +class BaseTemperatureSensor(StandardReadable): + temperature: SignalR[float] SensorT = TypeVar("SensorT", bound=BaseTemperatureSensor) diff --git a/tests/devices/temperature_controller/test_base_temperature_controller.py b/tests/devices/temperature_controller/test_base_temperature_controller.py index ec540c833de..36bbbf1167b 100644 --- a/tests/devices/temperature_controller/test_base_temperature_controller.py +++ b/tests/devices/temperature_controller/test_base_temperature_controller.py @@ -2,7 +2,6 @@ import pytest from ophyd_async.core import ( - SignalR, StandardReadableFormat, init_devices, set_mock_value, @@ -21,13 +20,9 @@ class MinimalMockSensor(BaseTemperatureSensor): def __init__(self, name: str = ""): with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): - self._temperature = epics_signal_r(float, "prefix + suffix)") + self.temperature = epics_signal_r(float, "prefix + suffix)") super().__init__(name=name) - @property - def temperature(self) -> SignalR[float]: - return self._temperature - class MockHeater(BaseHeater): def __init__(self, name: str = ""):