From 96cb4bc7e71849dcf3fe4ce9fd1f52b938efcabf Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Jul 2026 22:13:42 +0000 Subject: [PATCH 1/3] demo: use ControllerVector for temperature ramp sub-controllers Evolve the demo temperature controller composition example onto the documented ControllerVector pattern instead of a manual list + add_sub_controller loop, and add unit tests exercising cancel_all and the voltage-distributing scan against a mocked IPConnection. Closes #390 --- src/fastcs/demo/controllers.py | 19 +++++------ tests/demo/test_controllers.py | 59 ++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 tests/demo/test_controllers.py diff --git a/src/fastcs/demo/controllers.py b/src/fastcs/demo/controllers.py index 5926fc8ce..3546fea20 100755 --- a/src/fastcs/demo/controllers.py +++ b/src/fastcs/demo/controllers.py @@ -8,7 +8,7 @@ from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW from fastcs.connections import IPConnection, IPConnectionSettings -from fastcs.controllers import Controller +from fastcs.controllers import Controller, ControllerVector from fastcs.datatypes import Enum, Float, Int, Waveform from fastcs.logging import logger from fastcs.methods import command, scan @@ -80,15 +80,16 @@ def __init__(self, settings: TemperatureControllerSettings) -> None: self._settings = settings - self._ramp_controllers: list[TemperatureRampController] = [] - for index in range(1, settings.num_ramp_controllers + 1): - controller = TemperatureRampController(index, self.connection) - self._ramp_controllers.append(controller) - self.add_sub_controller(f"R{index}", controller) + self.ramps: ControllerVector[TemperatureRampController] = ControllerVector( + { + index: TemperatureRampController(index, self.connection) + for index in range(1, settings.num_ramp_controllers + 1) + } + ) @command() async def cancel_all(self) -> None: - for rc in self._ramp_controllers: + for rc in self.ramps.values(): await rc.enabled.put(OnOffEnum.Off, sync_setpoint=True) # TODO: The requests all get concatenated and the sim doesn't handle it await asyncio.sleep(0.1) @@ -118,14 +119,14 @@ async def update_voltages(self): await self.voltages.update(voltages) - for index, controller in enumerate(self._ramp_controllers): + for index, controller in self.ramps.items(): self.log_event( "Update voltages", topic=controller.voltage, query=query, response=voltages, ) - await controller.voltage.update(float(voltages[index])) + await controller.voltage.update(float(voltages[index - 1])) class TemperatureRampController(Controller): diff --git a/tests/demo/test_controllers.py b/tests/demo/test_controllers.py new file mode 100644 index 000000000..dd0adab82 --- /dev/null +++ b/tests/demo/test_controllers.py @@ -0,0 +1,59 @@ +from unittest.mock import AsyncMock + +import numpy as np +import pytest + +from fastcs.connections import IPConnectionSettings +from fastcs.controllers import ControllerVector +from fastcs.demo.controllers import ( + TemperatureController, + TemperatureControllerSettings, + TemperatureRampController, +) + + +@pytest.fixture +def controller() -> TemperatureController: + settings = TemperatureControllerSettings( + num_ramp_controllers=4, + ip_settings=IPConnectionSettings(ip="localhost", port=25565), + ) + controller = TemperatureController(settings) + controller.post_initialise() + return controller + + +def test_ramps_is_controller_vector(controller: TemperatureController): + assert isinstance(controller.ramps, ControllerVector) + assert list(controller.ramps) == [1, 2, 3, 4] + for index, ramp in controller.ramps.items(): + assert isinstance(ramp, TemperatureRampController) + assert controller.ramps[index] is ramp + + +@pytest.mark.asyncio +async def test_cancel_all_disables_every_ramp(controller: TemperatureController): + controller.connection.send_command = AsyncMock() # type: ignore[method-assign] + + await controller.cancel_all() + + sent_commands = [ + call.args[0] for call in controller.connection.send_command.call_args_list + ] + for index in controller.ramps: + assert f"N{index:02d}=0\r\n" in sent_commands + + +@pytest.mark.asyncio +async def test_update_voltages_updates_waveform_and_each_ramp( + controller: TemperatureController, +): + controller.connection.send_query = AsyncMock(return_value="[1, 2, 3, 4]\r\n") + + await controller.update_voltages() + + np.testing.assert_array_equal( + controller.voltages.get(), np.array([1, 2, 3, 4], dtype=np.int32) + ) + for index, ramp in controller.ramps.items(): + assert ramp.voltage.get() == pytest.approx(float(index)) From 1932e5e22da15a3485a8f6250e4ebe8914e7e54f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Jul 2026 12:09:10 +0000 Subject: [PATCH 2/3] demo: drop redundant type hint on ramps assignment Address review comment: pyright already infers ControllerVector[TemperatureRampController] from the dict literal, so the explicit annotation was redundant. --- src/fastcs/demo/controllers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastcs/demo/controllers.py b/src/fastcs/demo/controllers.py index 3546fea20..b39937eee 100755 --- a/src/fastcs/demo/controllers.py +++ b/src/fastcs/demo/controllers.py @@ -80,7 +80,7 @@ def __init__(self, settings: TemperatureControllerSettings) -> None: self._settings = settings - self.ramps: ControllerVector[TemperatureRampController] = ControllerVector( + self.ramps = ControllerVector( { index: TemperatureRampController(index, self.connection) for index in range(1, settings.num_ramp_controllers + 1) From 8f3be18f16a2f495c1b94c79558c107e0ab85977 Mon Sep 17 00:00:00 2001 From: Tom Cobb Date: Thu, 30 Jul 2026 16:14:32 +0000 Subject: [PATCH 3/3] test(demo): assert cancel_all puts Off on each ramp's enabled attr Check the behaviour cancel_all is responsible for (disabling every ramp via its `enabled` attribute) rather than the wire-format strings the attribute IO layer happens to emit. Co-Authored-By: Claude Opus 5 --- tests/demo/test_controllers.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/demo/test_controllers.py b/tests/demo/test_controllers.py index dd0adab82..bd7775bdf 100644 --- a/tests/demo/test_controllers.py +++ b/tests/demo/test_controllers.py @@ -6,6 +6,7 @@ from fastcs.connections import IPConnectionSettings from fastcs.controllers import ControllerVector from fastcs.demo.controllers import ( + OnOffEnum, TemperatureController, TemperatureControllerSettings, TemperatureRampController, @@ -33,15 +34,15 @@ def test_ramps_is_controller_vector(controller: TemperatureController): @pytest.mark.asyncio async def test_cancel_all_disables_every_ramp(controller: TemperatureController): - controller.connection.send_command = AsyncMock() # type: ignore[method-assign] + puts = {} + for index, ramp in controller.ramps.items(): + puts[index] = AsyncMock() + ramp.enabled.put = puts[index] # type: ignore[method-assign] await controller.cancel_all() - sent_commands = [ - call.args[0] for call in controller.connection.send_command.call_args_list - ] - for index in controller.ramps: - assert f"N{index:02d}=0\r\n" in sent_commands + for put in puts.values(): + put.assert_awaited_once_with(OnOffEnum.Off, sync_setpoint=True) @pytest.mark.asyncio