From 83bcf674b7332ec5b906841eea9cdc55b6d42d23 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Jul 2026 22:22:36 +0000 Subject: [PATCH 1/7] demo: getter/setter-in-init temperature attr example Add temperature_attr.py: a small temperature controller with per-attribute IO (a fresh AttributeIO/AttributeIORef pair per attribute) wired directly in __init__ rather than shared class-body declarations, foreshadowing the AttrRW(getter=, setter=) constructor params landing in #392. Baseline against the current callback-IO API. Closes #404 --- src/fastcs/demo/temperature_attr.py | 79 +++++++++++++++++++++++++++++ tests/demo/test_temperature_attr.py | 48 ++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 src/fastcs/demo/temperature_attr.py create mode 100644 tests/demo/test_temperature_attr.py diff --git a/src/fastcs/demo/temperature_attr.py b/src/fastcs/demo/temperature_attr.py new file mode 100644 index 00000000..84828b95 --- /dev/null +++ b/src/fastcs/demo/temperature_attr.py @@ -0,0 +1,79 @@ +"""Example 2 - getter/setter: per-attribute IO wired directly in ``__init__``. + +Baseline against the CURRENT callback-IO API (deliberately messy): each attribute +gets its own small ``AttributeIO``/``AttributeIORef`` pair, closing directly over the +command it queries/commands on the temperature sim, and attributes are assigned in +``__init__`` rather than declared in the class body. This foreshadows the +``AttrRW(getter=..., setter=...)`` constructor params landing in #392, without a +shared IO class dispatching by name (contrast with the composition example, +``controllers.py``, #390). +""" + +from dataclasses import dataclass + +from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.controllers import Controller +from fastcs.datatypes import Float + + +@dataclass +class TemperatureAttrSettings: + ip_settings: IPConnectionSettings + + +class RampRateIORef(AttributeIORef): + pass + + +class RampRateIO(AttributeIO[float, RampRateIORef]): + """IO for the ramp rate attribute only - a fresh instance per attribute.""" + + def __init__(self, connection: IPConnection): + super().__init__() + self._connection = connection + + async def update(self, attr: AttrR[float, RampRateIORef]) -> None: + response = await self._connection.send_query("R?\r\n") + await attr.update(attr.dtype(response.strip("\r\n"))) + + async def send(self, attr: AttrW[float, RampRateIORef], value: float) -> None: + await self._connection.send_command(f"R={attr.dtype(value)}\r\n") + + +class PowerIORef(AttributeIORef): + pass + + +class PowerIO(AttributeIO[float, PowerIORef]): + """IO for the power attribute only - a fresh instance per attribute.""" + + def __init__(self, connection: IPConnection): + super().__init__() + self._connection = connection + + async def update(self, attr: AttrR[float, PowerIORef]) -> None: + response = await self._connection.send_query("P?\r\n") + await attr.update(attr.dtype(response.strip("\r\n"))) + + +class TemperatureAttrController(Controller): + """A small temperature controller wired attribute-by-attribute in ``__init__``.""" + + def __init__(self, settings: TemperatureAttrSettings) -> None: + self.connection = IPConnection() + self._settings = settings + + super().__init__( + ios=[RampRateIO(self.connection), PowerIO(self.connection)] + ) + + self.ramp_rate = AttrRW(Float(), io_ref=RampRateIORef(update_period=0.2)) + self.power = AttrR(Float(), io_ref=PowerIORef(update_period=0.2)) + + async def connect(self) -> None: + await self.connection.connect(self._settings.ip_settings) + self._connected = True + + async def close(self) -> None: + await self.connection.close() diff --git a/tests/demo/test_temperature_attr.py b/tests/demo/test_temperature_attr.py new file mode 100644 index 00000000..b8a77dc5 --- /dev/null +++ b/tests/demo/test_temperature_attr.py @@ -0,0 +1,48 @@ +from unittest.mock import AsyncMock + +import pytest + +from fastcs.connections import IPConnectionSettings +from fastcs.demo.temperature_attr import ( + TemperatureAttrController, + TemperatureAttrSettings, +) + + +@pytest.fixture +def controller() -> TemperatureAttrController: + settings = TemperatureAttrSettings( + ip_settings=IPConnectionSettings(ip="localhost", port=25565) + ) + controller = TemperatureAttrController(settings) + controller.post_initialise() + return controller + + +@pytest.mark.asyncio +async def test_ramp_rate_read_from_device(controller: TemperatureAttrController): + controller.connection.send_query = AsyncMock(return_value="1.5\r\n") # type: ignore[method-assign] + + await controller.ramp_rate.bind_update_callback()() + + controller.connection.send_query.assert_awaited_once_with("R?\r\n") + assert controller.ramp_rate.get() == 1.5 + + +@pytest.mark.asyncio +async def test_ramp_rate_written_to_device(controller: TemperatureAttrController): + controller.connection.send_command = AsyncMock() # type: ignore[method-assign] + + await controller.ramp_rate.put(2.5) + + controller.connection.send_command.assert_awaited_once_with("R=2.5\r\n") + + +@pytest.mark.asyncio +async def test_power_read_from_device(controller: TemperatureAttrController): + controller.connection.send_query = AsyncMock(return_value="10.25\r\n") # type: ignore[method-assign] + + await controller.power.bind_update_callback()() + + controller.connection.send_query.assert_awaited_once_with("P?\r\n") + assert controller.power.get() == 10.25 From b68e74ad1cde373e5c6864e4f3f63cac6ccc6472 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Jul 2026 22:24:54 +0000 Subject: [PATCH 2/7] fix: ruff-format line-length nit --- src/fastcs/demo/temperature_attr.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/fastcs/demo/temperature_attr.py b/src/fastcs/demo/temperature_attr.py index 84828b95..2313ec3f 100644 --- a/src/fastcs/demo/temperature_attr.py +++ b/src/fastcs/demo/temperature_attr.py @@ -64,9 +64,7 @@ def __init__(self, settings: TemperatureAttrSettings) -> None: self.connection = IPConnection() self._settings = settings - super().__init__( - ios=[RampRateIO(self.connection), PowerIO(self.connection)] - ) + super().__init__(ios=[RampRateIO(self.connection), PowerIO(self.connection)]) self.ramp_rate = AttrRW(Float(), io_ref=RampRateIORef(update_period=0.2)) self.power = AttrR(Float(), io_ref=PowerIORef(update_period=0.2)) From a8db570e1b1dfa32fd8bdbdf5df673ac38725a9c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Jul 2026 12:16:12 +0000 Subject: [PATCH 3/7] demo: reset _connected on close in temperature_attr controller Address CodeRabbit review comment: close() closed the socket but left _connected True, so a subsequent status check would still report connected. --- src/fastcs/demo/temperature_attr.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fastcs/demo/temperature_attr.py b/src/fastcs/demo/temperature_attr.py index 2313ec3f..d351041e 100644 --- a/src/fastcs/demo/temperature_attr.py +++ b/src/fastcs/demo/temperature_attr.py @@ -75,3 +75,4 @@ async def connect(self) -> None: async def close(self) -> None: await self.connection.close() + self._connected = False From 214aaf2c971420bf3b865a619a03dcabc1ddf948 Mon Sep 17 00:00:00 2001 From: Tom Cobb Date: Thu, 23 Jul 2026 12:31:35 +0000 Subject: [PATCH 4/7] demo(#404): single callable-wrapping IO + protocol class (Thorlabs shape) Rewrites the getter/setter baseline to the intended shape: one generic TemperatureIO drives every attribute, and each TemperatureIORef carries the command-building callables (read_cmd/write_cmd) sourced from a single TemperatureProtocol class - mirroring fastcs-thorlabs-mff's MFFAttributeIO/MFFAttributeIORef/ThorlabsAPTProtocol. This is the honest precursor to #392's AttrRW(getter=, setter=): read_cmd/ write_cmd ARE the getter/setter, promoted onto the constructor when the IO/ref wrapper is deleted, while TemperatureProtocol survives unchanged. Replaces the previous per-attribute AttributeIO subclasses (RampRateIO/PowerIO), which hardcoded commands and foreshadowed nothing. Response parsing (float()) is inline in TemperatureIO.update rather than a response_handler callable. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LgnovZ7FWY8YptwqiufTbX --- src/fastcs/demo/temperature_attr.py | 92 ++++++++++++++++++----------- 1 file changed, 59 insertions(+), 33 deletions(-) diff --git a/src/fastcs/demo/temperature_attr.py b/src/fastcs/demo/temperature_attr.py index d351041e..a0511f29 100644 --- a/src/fastcs/demo/temperature_attr.py +++ b/src/fastcs/demo/temperature_attr.py @@ -1,14 +1,18 @@ -"""Example 2 - getter/setter: per-attribute IO wired directly in ``__init__``. - -Baseline against the CURRENT callback-IO API (deliberately messy): each attribute -gets its own small ``AttributeIO``/``AttributeIORef`` pair, closing directly over the -command it queries/commands on the temperature sim, and attributes are assigned in -``__init__`` rather than declared in the class body. This foreshadows the -``AttrRW(getter=..., setter=...)`` constructor params landing in #392, without a -shared IO class dispatching by name (contrast with the composition example, -``controllers.py``, #390). +"""Example 2 - getter/setter: per-attribute IO wired via callables in ``__init__``. + +Baseline against the CURRENT callback-IO API. A **single** generic IO class +(``TemperatureIO``) drives every attribute; the per-attribute behaviour lives in +each attribute's ``TemperatureIORef``, which just carries the command-building +callables (``read_cmd``/``write_cmd``) taken from a single ``TemperatureProtocol`` +class. This is the honest precursor to the ``AttrRW(getter=..., setter=...)`` +constructor params landing in #392: ``read_cmd``/``write_cmd`` *are* the +getter/setter, and #392 simply promotes them onto the constructor and deletes +this IO/ref wrapper, while ``TemperatureProtocol`` survives unchanged. Contrast +with the composition example (``controllers.py``, #390), whose shared IO instead +dispatches on a ``name`` string. """ +from collections.abc import Callable from dataclasses import dataclass from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW @@ -22,39 +26,47 @@ class TemperatureAttrSettings: ip_settings: IPConnectionSettings -class RampRateIORef(AttributeIORef): - pass +class TemperatureProtocol: + """The device wire protocol - one method per command, referenced by the IORefs. + Each getter returns the query string to send; each setter returns the command + string to send for a given value. These are exactly the callables #392 will + pass straight to ``AttrRW(getter=..., setter=...)``. + """ -class RampRateIO(AttributeIO[float, RampRateIORef]): - """IO for the ramp rate attribute only - a fresh instance per attribute.""" + def get_ramp_rate(self) -> str: + return "R?\r\n" - def __init__(self, connection: IPConnection): - super().__init__() - self._connection = connection + def set_ramp_rate(self, value: float) -> str: + return f"R={value}\r\n" - async def update(self, attr: AttrR[float, RampRateIORef]) -> None: - response = await self._connection.send_query("R?\r\n") - await attr.update(attr.dtype(response.strip("\r\n"))) + def get_power(self) -> str: + return "P?\r\n" - async def send(self, attr: AttrW[float, RampRateIORef], value: float) -> None: - await self._connection.send_command(f"R={attr.dtype(value)}\r\n") +@dataclass +class TemperatureIORef(AttributeIORef): + """Per-attribute IO spec: the command-building callables for one attribute.""" -class PowerIORef(AttributeIORef): - pass + read_cmd: Callable[[], str] + write_cmd: Callable[[float], str] | None = None -class PowerIO(AttributeIO[float, PowerIORef]): - """IO for the power attribute only - a fresh instance per attribute.""" +class TemperatureIO(AttributeIO[float, TemperatureIORef]): + """A single generic IO shared by every attribute; behaviour comes from the ref.""" def __init__(self, connection: IPConnection): super().__init__() self._connection = connection - async def update(self, attr: AttrR[float, PowerIORef]) -> None: - response = await self._connection.send_query("P?\r\n") - await attr.update(attr.dtype(response.strip("\r\n"))) + async def update(self, attr: AttrR[float, TemperatureIORef]) -> None: + response = await self._connection.send_query(attr.io_ref.read_cmd()) + await attr.update(float(response.strip("\r\n"))) + + async def send(self, attr: AttrW[float, TemperatureIORef], value: float) -> None: + if attr.io_ref.write_cmd is None: + raise TypeError(f"{attr} is read-only: no write_cmd on its io_ref") + await self._connection.send_command(attr.io_ref.write_cmd(value)) class TemperatureAttrController(Controller): @@ -63,11 +75,25 @@ class TemperatureAttrController(Controller): def __init__(self, settings: TemperatureAttrSettings) -> None: self.connection = IPConnection() self._settings = settings - - super().__init__(ios=[RampRateIO(self.connection), PowerIO(self.connection)]) - - self.ramp_rate = AttrRW(Float(), io_ref=RampRateIORef(update_period=0.2)) - self.power = AttrR(Float(), io_ref=PowerIORef(update_period=0.2)) + self._protocol = TemperatureProtocol() + + super().__init__(ios=[TemperatureIO(self.connection)]) + + self.ramp_rate = AttrRW( + Float(), + io_ref=TemperatureIORef( + read_cmd=self._protocol.get_ramp_rate, + write_cmd=self._protocol.set_ramp_rate, + update_period=0.2, + ), + ) + self.power = AttrR( + Float(), + io_ref=TemperatureIORef( + read_cmd=self._protocol.get_power, + update_period=0.2, + ), + ) async def connect(self) -> None: await self.connection.connect(self._settings.ip_settings) From 42bd98cdfac50b6794d7cab608d3d182dc1388d2 Mon Sep 17 00:00:00 2001 From: Tom Cobb Date: Thu, 30 Jul 2026 16:13:38 +0000 Subject: [PATCH 5/7] test: drop unnecessary # type: ignore[method-assign] in temperature_attr tests The project type-checks with pyright (standard mode), which does not flag assigning an AsyncMock over a bound method here, and `method-assign` is a mypy error code pyright never emits. pyright src tests is clean without them. --- tests/demo/test_temperature_attr.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/demo/test_temperature_attr.py b/tests/demo/test_temperature_attr.py index b8a77dc5..60e7fda9 100644 --- a/tests/demo/test_temperature_attr.py +++ b/tests/demo/test_temperature_attr.py @@ -21,7 +21,7 @@ def controller() -> TemperatureAttrController: @pytest.mark.asyncio async def test_ramp_rate_read_from_device(controller: TemperatureAttrController): - controller.connection.send_query = AsyncMock(return_value="1.5\r\n") # type: ignore[method-assign] + controller.connection.send_query = AsyncMock(return_value="1.5\r\n") await controller.ramp_rate.bind_update_callback()() @@ -31,7 +31,7 @@ async def test_ramp_rate_read_from_device(controller: TemperatureAttrController) @pytest.mark.asyncio async def test_ramp_rate_written_to_device(controller: TemperatureAttrController): - controller.connection.send_command = AsyncMock() # type: ignore[method-assign] + controller.connection.send_command = AsyncMock() await controller.ramp_rate.put(2.5) @@ -40,7 +40,7 @@ async def test_ramp_rate_written_to_device(controller: TemperatureAttrController @pytest.mark.asyncio async def test_power_read_from_device(controller: TemperatureAttrController): - controller.connection.send_query = AsyncMock(return_value="10.25\r\n") # type: ignore[method-assign] + controller.connection.send_query = AsyncMock(return_value="10.25\r\n") await controller.power.bind_update_callback()() From ff9b4b7adbe7f276d837d1219524fce285ac778a Mon Sep 17 00:00:00 2001 From: Tom Cobb Date: Mon, 3 Aug 2026 10:59:05 +0000 Subject: [PATCH 6/7] demo(#404): convert existing temperature controller to getter/setter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than adding a second temperature module, retarget #404 onto the existing `fastcs.demo.controllers` so there is one temperature demo. `TemperatureProtocol`/`TemperatureRampProtocol` carry one method per wire command, `TemperatureIORef` carries the `read_cmd`/`write_cmd` callables, and a single generic `TemperatureIO` just invokes them - the same shape as `fastcs-thorlabs-mff`, and the honest precursor to `AttrRW(getter=…, setter=…)` in #392. Attributes move from the class body into `__init__`, which is what lets each ramp bake its index into its own protocol instance instead of the IO dispatching on a `name` string plus suffix. Composition, `@scan` and `@command` are unchanged, so this module now covers both the getter/setter rung and the composition rung; the README ladder collapses accordingly. Co-Authored-By: Claude Opus 5 --- src/fastcs/demo/README.md | 18 ++- src/fastcs/demo/controllers.py | 200 +++++++++++++++++++++------- src/fastcs/demo/temperature_attr.py | 104 --------------- tests/demo/test_controllers.py | 84 ++++++++++++ tests/demo/test_temperature_attr.py | 48 ------- 5 files changed, 245 insertions(+), 209 deletions(-) delete mode 100644 src/fastcs/demo/temperature_attr.py delete mode 100644 tests/demo/test_temperature_attr.py diff --git a/src/fastcs/demo/README.md b/src/fastcs/demo/README.md index 110f518a..ff4ef631 100644 --- a/src/fastcs/demo/README.md +++ b/src/fastcs/demo/README.md @@ -24,23 +24,22 @@ decorator) — there is no `io=` object and no `DataType`. | Module | Concept | Backend | Issue | |--------|---------|---------|-------| | `hello_world.py` | pure-soft `@attr` decorator over in-memory values | none (soft) | [#398](https://github.com/DiamondLightSource/fastcs/issues/398) | -| `temperature_attr.py` | `getter`/`setter` callables in `__init__` (`AttrRW(getter=…, setter=…)`) | temperature sim | [#404](https://github.com/DiamondLightSource/fastcs/issues/404) | -| `controllers.py` | composition & methods: sub-controllers / `ControllerVector`, `@scan`, `@command` (getter/setter IO) | temperature sim | [#390](https://github.com/DiamondLightSource/fastcs/issues/390) | +| `controllers.py` | `getter`/`setter` callables in `__init__` (`AttrRW(getter=…, setter=…)`), then composition & methods: sub-controllers / `ControllerVector`, `@scan`, `@command` | temperature sim | [#404](https://github.com/DiamondLightSource/fastcs/issues/404), [#390](https://github.com/DiamondLightSource/fastcs/issues/390) | | `temperature_scpi.py` (+ `scpi.py`) | declarative annotated attributes; `ControllerFiller` builds each getter/setter from **static** `SCPIParam` extras metadata | temperature sim | [#405](https://github.com/DiamondLightSource/fastcs/issues/405) | | `eiger.py` (+ `simulation/eiger.py`) | introspectable device: bare hints filled from a **runtime** REST parameter tree | Eiger REST sim | [#391](https://github.com/DiamondLightSource/fastcs/issues/391) | ## The four tutorials -Five modules, **four** tutorials (the old "reusable `io=` object" rung is gone — +Four modules, **four** tutorials (the old "reusable `io=` object" rung is gone — `io=` objects were replaced by getter/setter callables, so there is nothing to factor into): 1. **hello world** — `hello_world.py` (soft `@attr`). -2. **getter/setter** — `temperature_attr.py`; closes with *"when the shared - pattern is worth naming, reach for the declarative style →"*. -3. **declarative** — `temperature_scpi.py` (annotated `SCPIParam` + filler), - and this is where **composition + `@scan` + `@command`** are shown, walking - the full multi-ramp temperature controller (`controllers.py`, #390). +2. **getter/setter** — `controllers.py`; the full multi-ramp temperature + controller, so this is also where **composition + `@scan` + `@command`** + are shown (#390). Closes with *"when the shared pattern is worth naming, + reach for the declarative style →"*. +3. **declarative** — `temperature_scpi.py` (annotated `SCPIParam` + filler). 4. **introspectable** — `eiger.py`. Notes: @@ -67,8 +66,7 @@ Notes: ## Baselines vs framework PRs -`temperature_attr.py`, `controllers.py`, and `eiger.py` have current-API -baselines that can be written **now** (deliberately messy against the +`controllers.py` and `eiger.py` have current-API baselines that can be written **now** (deliberately messy against the pre-refactor API) and are cleaned up as each framework PR lands. `hello_world.py` and `temperature_scpi.py` need framework work first (`@attr` #397; `ControllerFiller` #394). See each issue's `Blocked by:` line. diff --git a/src/fastcs/demo/controllers.py b/src/fastcs/demo/controllers.py index b39937ee..f91b35e8 100755 --- a/src/fastcs/demo/controllers.py +++ b/src/fastcs/demo/controllers.py @@ -1,8 +1,28 @@ +"""Example 2 - getter/setter: per-attribute IO wired via callables in ``__init__``. + +Baseline against the CURRENT callback-IO API. A **single** generic IO class +(``TemperatureIO``) drives every attribute; the per-attribute behaviour lives in +each attribute's ``TemperatureIORef``, which just carries the command-building +callables (``read_cmd``/``write_cmd``) taken from a protocol class with one method +per device command. This is the honest precursor to the +``AttrRW(getter=..., setter=...)`` constructor params landing in #392: +``read_cmd``/``write_cmd`` *are* the getter/setter, and #392 simply promotes them +onto the constructor and deletes this IO/ref wrapper, while the protocol classes +survive unchanged. + +Because the attributes are wired in ``__init__`` rather than the class body, each +one can close over per-instance state - which is what lets a ramp's index be baked +into its protocol instead of dispatched on at IO time. This module also carries the +composition and methods rungs: a ``ControllerVector`` of ``TemperatureRampController`` +sub-controllers, plus ``@scan`` and ``@command``. +""" + import asyncio import enum import json +from collections.abc import Callable from dataclasses import KW_ONLY, dataclass -from typing import TypeVar +from typing import Any, TypeVar import numpy as np @@ -27,35 +47,83 @@ class TemperatureControllerSettings: ip_settings: IPConnectionSettings +class TemperatureProtocol: + """The device wire protocol - one method per command, referenced by the IORefs. + + Each getter returns the query string to send; each setter returns the command + string to send for a given value. These are exactly the callables #392 will pass + straight to ``AttrRW(getter=..., setter=...)``. + """ + + def get_ramp_rate(self) -> str: + return "R?\r\n" + + def set_ramp_rate(self, value: float) -> str: + return f"R={value}\r\n" + + def get_power(self) -> str: + return "P?\r\n" + + def get_voltages(self) -> str: + return "V?\r\n" + + +class TemperatureRampProtocol: + """The wire protocol of a single ramp, whose commands are suffixed by its index. + + The index is baked into the instance, so every command is still a zero- or + one-argument callable that can be handed to an attribute as-is. + """ + + def __init__(self, index: int) -> None: + self.suffix = f"{index:02d}" + + def get_start(self) -> str: + return f"S{self.suffix}?\r\n" + + def set_start(self, value: int) -> str: + return f"S{self.suffix}={value}\r\n" + + def get_end(self) -> str: + return f"E{self.suffix}?\r\n" + + def set_end(self, value: int) -> str: + return f"E{self.suffix}={value}\r\n" + + def get_enabled(self) -> str: + return f"N{self.suffix}?\r\n" + + def set_enabled(self, value: OnOffEnum) -> str: + return f"N{self.suffix}={value}\r\n" + + def get_target(self) -> str: + return f"T{self.suffix}?\r\n" + + def get_actual(self) -> str: + return f"A{self.suffix}?\r\n" + + @dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str +class TemperatureIORef(AttributeIORef): + """Per-attribute IO spec: the command-building callables for one attribute.""" + + read_cmd: Callable[[], str] + write_cmd: Callable[[Any], str] | None = None _: KW_ONLY update_period: float | None = 0.2 -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): - def __init__(self, connection: IPConnection, suffix: str): +class TemperatureIO(AttributeIO[NumberT, TemperatureIORef]): + """A single generic IO shared by every attribute; behaviour comes from the ref.""" + + def __init__(self, connection: IPConnection): super().__init__() self._connection = connection - self.suffix = suffix - - async def send( - self, attr: AttrW[NumberT, TemperatureControllerAttributeIORef], value: NumberT - ) -> None: - command = f"{attr.io_ref.name}{self.suffix}={attr.dtype(value)}" - await self._connection.send_command(f"{command}\r\n") - self.log_event("Send command for attribute", topic=attr, command=command) - async def update( - self, attr: AttrR[NumberT, TemperatureControllerAttributeIORef] - ) -> None: - query = f"{attr.io_ref.name}{self.suffix}?" - response = await self._connection.send_query(f"{query}\r\n") - response = response.strip("\r\n") + async def update(self, attr: AttrR[NumberT, TemperatureIORef]) -> None: + query = attr.io_ref.read_cmd() + response = (await self._connection.send_query(query)).strip("\r\n") self.log_event( "Query for attribute", topic=attr, @@ -65,20 +133,37 @@ async def update( await attr.update(attr.dtype(response)) + async def send( + self, attr: AttrW[NumberT, TemperatureIORef], value: NumberT + ) -> None: + if attr.io_ref.write_cmd is None: + raise TypeError(f"{attr} is read-only: no write_cmd on its io_ref") + + command = attr.io_ref.write_cmd(value) + await self._connection.send_command(command) + self.log_event("Send command for attribute", topic=attr, command=command) -class TemperatureController(Controller): - ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef(name="R")) - power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef(name="P")) - voltages = AttrR(Waveform(np.int32, shape=(4,))) +class TemperatureController(Controller): def __init__(self, settings: TemperatureControllerSettings) -> None: self.connection = IPConnection() - self.suffix = "" - super().__init__( - ios=[TemperatureControllerAttributeIO(self.connection, self.suffix)] - ) - self._settings = settings + self._protocol = TemperatureProtocol() + + super().__init__(ios=[TemperatureIO(self.connection)]) + + self.ramp_rate = AttrRW( + Float(), + io_ref=TemperatureIORef( + read_cmd=self._protocol.get_ramp_rate, + write_cmd=self._protocol.set_ramp_rate, + ), + ) + self.power = AttrR( + Float(), io_ref=TemperatureIORef(read_cmd=self._protocol.get_power) + ) + # Updated by the update_voltages scan below, so no IO of its own + self.voltages = AttrR(Waveform(np.int32, shape=(4,))) self.ramps = ControllerVector( { @@ -112,10 +197,8 @@ async def close(self) -> None: @scan(0.1) async def update_voltages(self): - query = "V?" - voltages = json.loads( - (await self.connection.send_query(f"{query}\r\n")).strip("\r\n") - ) + query = self._protocol.get_voltages() + voltages = json.loads((await self.connection.send_query(query)).strip("\r\n")) await self.voltages.update(voltages) @@ -130,18 +213,41 @@ async def update_voltages(self): class TemperatureRampController(Controller): - start = AttrRW(Int(), io_ref=TemperatureControllerAttributeIORef(name="S")) - end = AttrRW(Int(), io_ref=TemperatureControllerAttributeIORef(name="E")) - enabled = AttrRW( - Enum(OnOffEnum), io_ref=TemperatureControllerAttributeIORef(name="N") - ) - target = AttrR(Float(prec=3), io_ref=TemperatureControllerAttributeIORef(name="T")) - actual = AttrR(Float(prec=3), io_ref=TemperatureControllerAttributeIORef(name="A")) - voltage = AttrR(Float(prec=3)) - def __init__(self, index: int, conn: IPConnection) -> None: - suffix = f"{index:02d}" - super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(conn, suffix)] - ) + self._protocol = TemperatureRampProtocol(index) + + super().__init__(f"Ramp{self._protocol.suffix}", ios=[TemperatureIO(conn)]) + self.connection = conn + + self.start = AttrRW( + Int(), + io_ref=TemperatureIORef( + read_cmd=self._protocol.get_start, + write_cmd=self._protocol.set_start, + ), + ) + self.end = AttrRW( + Int(), + io_ref=TemperatureIORef( + read_cmd=self._protocol.get_end, + write_cmd=self._protocol.set_end, + ), + ) + self.enabled = AttrRW( + Enum(OnOffEnum), + io_ref=TemperatureIORef( + read_cmd=self._protocol.get_enabled, + write_cmd=self._protocol.set_enabled, + ), + ) + self.target = AttrR( + Float(prec=3), + io_ref=TemperatureIORef(read_cmd=self._protocol.get_target), + ) + self.actual = AttrR( + Float(prec=3), + io_ref=TemperatureIORef(read_cmd=self._protocol.get_actual), + ) + # Updated by the parent controller's update_voltages scan + self.voltage = AttrR(Float(prec=3)) diff --git a/src/fastcs/demo/temperature_attr.py b/src/fastcs/demo/temperature_attr.py deleted file mode 100644 index a0511f29..00000000 --- a/src/fastcs/demo/temperature_attr.py +++ /dev/null @@ -1,104 +0,0 @@ -"""Example 2 - getter/setter: per-attribute IO wired via callables in ``__init__``. - -Baseline against the CURRENT callback-IO API. A **single** generic IO class -(``TemperatureIO``) drives every attribute; the per-attribute behaviour lives in -each attribute's ``TemperatureIORef``, which just carries the command-building -callables (``read_cmd``/``write_cmd``) taken from a single ``TemperatureProtocol`` -class. This is the honest precursor to the ``AttrRW(getter=..., setter=...)`` -constructor params landing in #392: ``read_cmd``/``write_cmd`` *are* the -getter/setter, and #392 simply promotes them onto the constructor and deletes -this IO/ref wrapper, while ``TemperatureProtocol`` survives unchanged. Contrast -with the composition example (``controllers.py``, #390), whose shared IO instead -dispatches on a ``name`` string. -""" - -from collections.abc import Callable -from dataclasses import dataclass - -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW -from fastcs.connections import IPConnection, IPConnectionSettings -from fastcs.controllers import Controller -from fastcs.datatypes import Float - - -@dataclass -class TemperatureAttrSettings: - ip_settings: IPConnectionSettings - - -class TemperatureProtocol: - """The device wire protocol - one method per command, referenced by the IORefs. - - Each getter returns the query string to send; each setter returns the command - string to send for a given value. These are exactly the callables #392 will - pass straight to ``AttrRW(getter=..., setter=...)``. - """ - - def get_ramp_rate(self) -> str: - return "R?\r\n" - - def set_ramp_rate(self, value: float) -> str: - return f"R={value}\r\n" - - def get_power(self) -> str: - return "P?\r\n" - - -@dataclass -class TemperatureIORef(AttributeIORef): - """Per-attribute IO spec: the command-building callables for one attribute.""" - - read_cmd: Callable[[], str] - write_cmd: Callable[[float], str] | None = None - - -class TemperatureIO(AttributeIO[float, TemperatureIORef]): - """A single generic IO shared by every attribute; behaviour comes from the ref.""" - - def __init__(self, connection: IPConnection): - super().__init__() - self._connection = connection - - async def update(self, attr: AttrR[float, TemperatureIORef]) -> None: - response = await self._connection.send_query(attr.io_ref.read_cmd()) - await attr.update(float(response.strip("\r\n"))) - - async def send(self, attr: AttrW[float, TemperatureIORef], value: float) -> None: - if attr.io_ref.write_cmd is None: - raise TypeError(f"{attr} is read-only: no write_cmd on its io_ref") - await self._connection.send_command(attr.io_ref.write_cmd(value)) - - -class TemperatureAttrController(Controller): - """A small temperature controller wired attribute-by-attribute in ``__init__``.""" - - def __init__(self, settings: TemperatureAttrSettings) -> None: - self.connection = IPConnection() - self._settings = settings - self._protocol = TemperatureProtocol() - - super().__init__(ios=[TemperatureIO(self.connection)]) - - self.ramp_rate = AttrRW( - Float(), - io_ref=TemperatureIORef( - read_cmd=self._protocol.get_ramp_rate, - write_cmd=self._protocol.set_ramp_rate, - update_period=0.2, - ), - ) - self.power = AttrR( - Float(), - io_ref=TemperatureIORef( - read_cmd=self._protocol.get_power, - update_period=0.2, - ), - ) - - async def connect(self) -> None: - await self.connection.connect(self._settings.ip_settings) - self._connected = True - - async def close(self) -> None: - await self.connection.close() - self._connected = False diff --git a/tests/demo/test_controllers.py b/tests/demo/test_controllers.py index bd7775bd..039c0c3a 100644 --- a/tests/demo/test_controllers.py +++ b/tests/demo/test_controllers.py @@ -24,6 +24,11 @@ def controller() -> TemperatureController: return controller +@pytest.fixture +def ramp_controller(controller: TemperatureController) -> TemperatureRampController: + return controller.ramps[1] + + def test_ramps_is_controller_vector(controller: TemperatureController): assert isinstance(controller.ramps, ControllerVector) assert list(controller.ramps) == [1, 2, 3, 4] @@ -32,6 +37,84 @@ def test_ramps_is_controller_vector(controller: TemperatureController): assert controller.ramps[index] is ramp +@pytest.mark.asyncio +async def test_ramp_rate_read_from_device(controller: TemperatureController): + controller.connection.send_query = AsyncMock(return_value="1.5\r\n") + + await controller.ramp_rate.bind_update_callback()() + + controller.connection.send_query.assert_awaited_once_with("R?\r\n") + assert controller.ramp_rate.get() == 1.5 + + +@pytest.mark.asyncio +async def test_ramp_rate_written_to_device(controller: TemperatureController): + controller.connection.send_command = AsyncMock() + + await controller.ramp_rate.put(2.5) + + controller.connection.send_command.assert_awaited_once_with("R=2.5\r\n") + + +@pytest.mark.asyncio +async def test_power_read_from_device(controller: TemperatureController): + controller.connection.send_query = AsyncMock(return_value="10.25\r\n") + + await controller.power.bind_update_callback()() + + controller.connection.send_query.assert_awaited_once_with("P?\r\n") + assert controller.power.get() == 10.25 + + +@pytest.mark.asyncio +async def test_ramp_start_read_from_device(ramp_controller: TemperatureRampController): + ramp_controller.connection.send_query = AsyncMock(return_value="7\r\n") + + await ramp_controller.start.bind_update_callback()() + + ramp_controller.connection.send_query.assert_awaited_once_with("S01?\r\n") + assert ramp_controller.start.get() == 7 + + +@pytest.mark.asyncio +async def test_ramp_end_written_to_device(ramp_controller: TemperatureRampController): + ramp_controller.connection.send_command = AsyncMock() + + await ramp_controller.end.put(42) + + ramp_controller.connection.send_command.assert_awaited_once_with("E01=42\r\n") + + +@pytest.mark.asyncio +async def test_ramp_enabled_written_to_device( + ramp_controller: TemperatureRampController, +): + ramp_controller.connection.send_command = AsyncMock() + + await ramp_controller.enabled.put(OnOffEnum.On) + + ramp_controller.connection.send_command.assert_awaited_once_with("N01=1\r\n") + + +@pytest.mark.asyncio +async def test_each_ramp_addresses_its_own_index(controller: TemperatureController): + controller.connection.send_command = AsyncMock() + + for index, ramp in controller.ramps.items(): + await ramp.start.put(index) + + assert [ + call.args[0] for call in controller.connection.send_command.await_args_list + ] == ["S01=1\r\n", "S02=2\r\n", "S03=3\r\n", "S04=4\r\n"] + + +@pytest.mark.asyncio +async def test_read_only_attribute_has_no_write_command( + ramp_controller: TemperatureRampController, +): + assert ramp_controller.target.io_ref.write_cmd is None + + @pytest.mark.asyncio async def test_cancel_all_disables_every_ramp(controller: TemperatureController): puts = {} @@ -53,6 +136,7 @@ async def test_update_voltages_updates_waveform_and_each_ramp( await controller.update_voltages() + controller.connection.send_query.assert_awaited_once_with("V?\r\n") np.testing.assert_array_equal( controller.voltages.get(), np.array([1, 2, 3, 4], dtype=np.int32) ) diff --git a/tests/demo/test_temperature_attr.py b/tests/demo/test_temperature_attr.py deleted file mode 100644 index 60e7fda9..00000000 --- a/tests/demo/test_temperature_attr.py +++ /dev/null @@ -1,48 +0,0 @@ -from unittest.mock import AsyncMock - -import pytest - -from fastcs.connections import IPConnectionSettings -from fastcs.demo.temperature_attr import ( - TemperatureAttrController, - TemperatureAttrSettings, -) - - -@pytest.fixture -def controller() -> TemperatureAttrController: - settings = TemperatureAttrSettings( - ip_settings=IPConnectionSettings(ip="localhost", port=25565) - ) - controller = TemperatureAttrController(settings) - controller.post_initialise() - return controller - - -@pytest.mark.asyncio -async def test_ramp_rate_read_from_device(controller: TemperatureAttrController): - controller.connection.send_query = AsyncMock(return_value="1.5\r\n") - - await controller.ramp_rate.bind_update_callback()() - - controller.connection.send_query.assert_awaited_once_with("R?\r\n") - assert controller.ramp_rate.get() == 1.5 - - -@pytest.mark.asyncio -async def test_ramp_rate_written_to_device(controller: TemperatureAttrController): - controller.connection.send_command = AsyncMock() - - await controller.ramp_rate.put(2.5) - - controller.connection.send_command.assert_awaited_once_with("R=2.5\r\n") - - -@pytest.mark.asyncio -async def test_power_read_from_device(controller: TemperatureAttrController): - controller.connection.send_query = AsyncMock(return_value="10.25\r\n") - - await controller.power.bind_update_callback()() - - controller.connection.send_query.assert_awaited_once_with("P?\r\n") - assert controller.power.get() == 10.25 From e73453b17cc878febe0f9d88fdbe5d550ce52887 Mon Sep 17 00:00:00 2001 From: Tom Cobb Date: Mon, 3 Aug 2026 11:41:51 +0000 Subject: [PATCH 7/7] demo(#404): rename controllers.py to temperature_attr.py Match the naming of the other demo modules (hello_world.py, temperature_scpi.py, eiger.py), which are named for the device and the style they demonstrate rather than for the framework concept. Updates the importers: `fastcs.demo.__main__`, the test module, the README ladder and the docs nitpick-ignore entry. The launch `type:` in fastcs.yaml is derived from the top-level package, not the submodule, so `fastcs.TemperatureController` and the checked-in schema.json are unaffected (verified by regenerating the schema). Co-Authored-By: Claude Opus 5 --- docs/conf.py | 2 +- src/fastcs/demo/README.md | 6 +++--- src/fastcs/demo/__main__.py | 2 +- src/fastcs/demo/{controllers.py => temperature_attr.py} | 0 .../demo/{test_controllers.py => test_temperature_attr.py} | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) rename src/fastcs/demo/{controllers.py => temperature_attr.py} (100%) rename tests/demo/{test_controllers.py => test_temperature_attr.py} (99%) diff --git a/docs/conf.py b/docs/conf.py index 99b82e5c..edfd712d 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -101,7 +101,7 @@ ("py:class", "fastcs.logging._graylog.GraylogStaticFields"), ("py:class", "fastcs.logging._graylog.GraylogEnvFields"), ("py:obj", "fastcs.control_system.build_controller_api"), - ("docutils", "fastcs.demo.controllers.TemperatureControllerSettings"), + ("docutils", "fastcs.demo.temperature_attr.TemperatureControllerSettings"), # TypeVar without docstrings still give warnings ("py:class", "strawberry.schema.schema.Schema"), ] diff --git a/src/fastcs/demo/README.md b/src/fastcs/demo/README.md index ff4ef631..3865e7a2 100644 --- a/src/fastcs/demo/README.md +++ b/src/fastcs/demo/README.md @@ -24,7 +24,7 @@ decorator) — there is no `io=` object and no `DataType`. | Module | Concept | Backend | Issue | |--------|---------|---------|-------| | `hello_world.py` | pure-soft `@attr` decorator over in-memory values | none (soft) | [#398](https://github.com/DiamondLightSource/fastcs/issues/398) | -| `controllers.py` | `getter`/`setter` callables in `__init__` (`AttrRW(getter=…, setter=…)`), then composition & methods: sub-controllers / `ControllerVector`, `@scan`, `@command` | temperature sim | [#404](https://github.com/DiamondLightSource/fastcs/issues/404), [#390](https://github.com/DiamondLightSource/fastcs/issues/390) | +| `temperature_attr.py` | `getter`/`setter` callables in `__init__` (`AttrRW(getter=…, setter=…)`), then composition & methods: sub-controllers / `ControllerVector`, `@scan`, `@command` | temperature sim | [#404](https://github.com/DiamondLightSource/fastcs/issues/404), [#390](https://github.com/DiamondLightSource/fastcs/issues/390) | | `temperature_scpi.py` (+ `scpi.py`) | declarative annotated attributes; `ControllerFiller` builds each getter/setter from **static** `SCPIParam` extras metadata | temperature sim | [#405](https://github.com/DiamondLightSource/fastcs/issues/405) | | `eiger.py` (+ `simulation/eiger.py`) | introspectable device: bare hints filled from a **runtime** REST parameter tree | Eiger REST sim | [#391](https://github.com/DiamondLightSource/fastcs/issues/391) | @@ -35,7 +35,7 @@ Four modules, **four** tutorials (the old "reusable `io=` object" rung is gone factor into): 1. **hello world** — `hello_world.py` (soft `@attr`). -2. **getter/setter** — `controllers.py`; the full multi-ramp temperature +2. **getter/setter** — `temperature_attr.py`; the full multi-ramp temperature controller, so this is also where **composition + `@scan` + `@command`** are shown (#390). Closes with *"when the shared pattern is worth naming, reach for the declarative style →"*. @@ -66,7 +66,7 @@ Notes: ## Baselines vs framework PRs -`controllers.py` and `eiger.py` have current-API baselines that can be written **now** (deliberately messy against the +`temperature_attr.py` and `eiger.py` have current-API baselines that can be written **now** (deliberately messy against the pre-refactor API) and are cleaned up as each framework PR lands. `hello_world.py` and `temperature_scpi.py` need framework work first (`@attr` #397; `ControllerFiller` #394). See each issue's `Blocked by:` line. diff --git a/src/fastcs/demo/__main__.py b/src/fastcs/demo/__main__.py index ff454806..467be4f8 100644 --- a/src/fastcs/demo/__main__.py +++ b/src/fastcs/demo/__main__.py @@ -1,6 +1,6 @@ from fastcs import __version__ from fastcs.launch import launch -from .controllers import TemperatureController +from .temperature_attr import TemperatureController launch(TemperatureController, version=__version__) diff --git a/src/fastcs/demo/controllers.py b/src/fastcs/demo/temperature_attr.py similarity index 100% rename from src/fastcs/demo/controllers.py rename to src/fastcs/demo/temperature_attr.py diff --git a/tests/demo/test_controllers.py b/tests/demo/test_temperature_attr.py similarity index 99% rename from tests/demo/test_controllers.py rename to tests/demo/test_temperature_attr.py index 039c0c3a..43703420 100644 --- a/tests/demo/test_controllers.py +++ b/tests/demo/test_temperature_attr.py @@ -5,7 +5,7 @@ from fastcs.connections import IPConnectionSettings from fastcs.controllers import ControllerVector -from fastcs.demo.controllers import ( +from fastcs.demo.temperature_attr import ( OnOffEnum, TemperatureController, TemperatureControllerSettings,