From abc0b30e64bf12005ed008e693b5c0752a5f4beb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 00:17:21 +0000 Subject: [PATCH 1/3] demo: SCPI example - annotated attributes with per-attribute filler data Example 4 of the tutorial sequence, and the declarative rung: a device that does NOT describe itself, whose attributes are hand-annotated in the class body and provisioned from that static metadata by a `ControllerFiller`. `SCPIParam` is one spec object per attribute - the command token plus all generic metadata in the same place, because they describe the same thing: ramp_rate: Annotated[ AttrRW[float], SCPIParam("R", precision=3, units="K/s") ] Both new modules live in the demo (protocol) layer, not core FastCS: ADR 0014 decision 3 says core ships no extras vocabulary for 1.0. Core defines the mechanism - the extras an `Annotated` hint carries through to the filler - and this is the worked example of a third party building on it. `SCPIParam` is a sibling of ophyd-async's `PvSuffix`, not a FastCS type; hence the name, rather than `SCPIMeta`, since the `*Meta` suffix is reserved for the `Unpack`-able typed dicts. - `demo/scpi.py`: `SCPIParam` and `SCPIController`. The controller reads its own declarations, builds each attribute's getter and setter from the token, and passes the param's `.meta` to `fill_attribute`, which runtime-validates it against the datatype the hint declared. The access mode comes from the hint: an `AttrR[...]` gets no setter. A `suffix` addresses one channel of a device, so a per-ramp sub controller is the same class with `01` appended to every token. - `demo/temperature_scpi.py`: the full multi-ramp temperature controller declared this way, running on the existing temperature sim - its text protocol is already SCPI-shaped, so no new sim is invented and, crucially, no introspection is. It carries the composition and methods rungs too: a `ControllerVector` of ramp sub controllers, the voltages `@scan`, and the cancel-all `@command`. Two attributes deliberately carry no token: `voltages` and each ramp's `voltage` are updated by the parent's scan in one query rather than one per ramp, so the driver fills them itself. That is what shows `SCPIController` filling what it owns and leaving the rest alone. Tests cover the wire format, the per-ramp suffix, the enum datatype coming from the hint, and the negative case the issue asks for: `precision` on a `str` attribute raises, naming the attribute and the field. Closes #405 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BymsiALDVCGv1nJWK8DyAV --- src/fastcs/demo/scpi.py | 184 ++++++++++++++++++++ src/fastcs/demo/temperature_scpi.py | 173 +++++++++++++++++++ tests/demo/test_temperature_scpi.py | 254 ++++++++++++++++++++++++++++ 3 files changed, 611 insertions(+) create mode 100644 src/fastcs/demo/scpi.py create mode 100644 src/fastcs/demo/temperature_scpi.py create mode 100644 tests/demo/test_temperature_scpi.py diff --git a/src/fastcs/demo/scpi.py b/src/fastcs/demo/scpi.py new file mode 100644 index 00000000..5392fb7a --- /dev/null +++ b/src/fastcs/demo/scpi.py @@ -0,0 +1,184 @@ +"""Example 4's protocol layer - a declarative vocabulary built on the filler. + +A SCPI device does not describe itself. There is no parameter tree to walk and no +metadata to read off the wire, so the attributes and everything about them are +written by hand, in the class body, as annotated hints:: + + class Thermostat(SCPIController): + ramp_rate: Annotated[AttrRW[float], SCPIParam("R", precision=3, units="K/s")] + +That is the declarative half of ADR 0013 for a device that cannot be introspected: +the class body says what exists and `SCPIController` provisions it, rather than +`__init__` wiring each getter and setter by hand as +:mod:`fastcs.demo.temperature_attr` does. + +**This lives in the demo, not core FastCS** (ADR 0014 decision 3). Core defines no +extras vocabulary for 1.0 - it defines the *mechanism*, which is the ``extras`` an +``Annotated`` hint carries through to `ControllerFiller`. What a protocol package +puts in there is its own business, and this module is the worked example of one +doing it: `SCPIParam` is a sibling of ophyd-async's ``PvSuffix``, not a FastCS type. +""" + +from collections.abc import Awaitable, Callable +from typing import Any, Unpack, cast + +from fastcs.attributes import AttrW, Polled +from fastcs.connections import IPConnection +from fastcs.controllers import Controller +from fastcs.controllers.filler import Declaration +from fastcs.datatypes import DType_T, Meta +from fastcs.logging import logger + + +class SCPIParam: + """One attribute's whole spec: its command token, plus its metadata. + + The command token and the metadata are written in the same place because they + describe the same thing, and splitting them across two extras would let one be + updated without the other:: + + target: Annotated[AttrR[float], SCPIParam("T", precision=3, units="degC")] + + It is the **exclusive** spec source for its attribute: `SCPIController` does not + also merge a separate ``FloatMeta`` extra on the same hint, so there is one + place to look for what an attribute is. + + Named ``SCPIParam`` rather than ``SCPIMeta``: the ``*Meta`` suffix is reserved + for the ``Unpack``-able typed dicts, and this is a binding extra you + instantiate. + + Args: + param: The device's command token - ``"R"`` for a device answering + ``R?`` and accepting ``R=1.5`` + meta: Metadata for the attribute, validated against the datatype the hint + declared when `ControllerFiller.fill_attribute` applies it + + """ + + def __init__(self, param: str, **meta: Unpack[Meta]) -> None: + self.param = param + self.meta: Meta = meta + + def __repr__(self) -> str: + return f"SCPIParam({self.param!r}, {self.meta})" + + +class SCPIController(Controller): + """A controller whose attributes are declared as `SCPIParam` hints. + + Provisions each declared attribute's getter and setter from its command token, + and applies the metadata that came with it. The wire format is the text protocol + the demo's temperature sim speaks - ``R?`` to read, ``R=1.5`` to write - which is + SCPI-shaped, so no new simulation is needed to demonstrate this. + + A sub controller addressing one channel of a device passes a ``suffix``, which is + appended to every token: the same class serves ramp 1 as ``S01?`` and ramp 2 as + ``S02?`` without dispatching on which at IO time. + + Args: + connection: The link to talk over. Held as ``self.connection`` too, so + scans are gated on it. + suffix: Appended to every command token, for a per-channel sub controller + description: Passed to `Controller` + + """ + + poll_period: float = 0.2 + """Seconds between reads of every polled attribute this controller declares.""" + + def __init__( + self, + connection: IPConnection, + suffix: str = "", + description: str | None = None, + ) -> None: + self._connection = connection + self._suffix = suffix + + super().__init__(description) + + self.connection = connection + + async def initialise(self) -> None: + """Provision every attribute the class body declared with a `SCPIParam`. + + Nothing here talks to the device: a SCPI device has nothing to ask. The + work is turning static declarations into IO, which is why this reads only + the class body and the connection it was given. + """ + for declaration in self.filler.declarations.values(): + param = self._param_of(declaration) + if param is None: + # Not ours - a hint some other mechanism fills, or a promise the + # driver provisions itself. + continue + + self._fill(declaration, param) + + self.filler.check_filled() + + @staticmethod + def _param_of(declaration: Declaration) -> SCPIParam | None: + """The `SCPIParam` an ``Annotated`` hint carried, if it carried one. + + ``declaration.hint.extras`` is the ``(child, extras)`` the filler yields, + looked up by name so there is something to call ``fill_attribute`` with. + """ + for extra in declaration.hint.extras: + if isinstance(extra, SCPIParam): + return extra + return None + + def _fill(self, declaration: Declaration, param: SCPIParam) -> None: + datatype = declaration.hint.datatype + if datatype is None: + # ``state: AttrR`` is a promise for introspection to satisfy, and a + # SCPI device introspects nothing - so there is no datatype to parse + # the device's answer into and nothing this layer can build. + raise TypeError( + f"{type(self).__name__}.{declaration.raw_name} declares a " + f"{param!r} but does not say what it holds. A SCPI device cannot " + "be asked, so the hint must name its datatype - " + f"`{declaration.raw_name}: Annotated[AttrRW[float], ...]`." + ) + + setter = None + if isinstance(declaration.child, AttrW): + setter = self._setter(param.param) + + self.filler.fill_attribute( + declaration.name, + # The hint's datatype doubles as the parser for the device's text + # answer, which every datatype FastCS serves happens to support - + # ``float("1.5")``, ``OnOffEnum("1")`` - but nothing in its type says + # so, hence the cast. + getter=Polled( + self._getter(param.param, cast(Callable[[str], Any], datatype)), + period=self.poll_period, + ), + setter=setter, + **param.meta, + ) + + def _getter( + self, param: str, datatype: Callable[[str], DType_T] + ) -> Callable[[], Awaitable[DType_T]]: + """A zero-argument coroutine reading one parameter, parsed into its type.""" + + async def get() -> DType_T: + query = f"{param}{self._suffix}?\r\n" + response = (await self._connection.send_query(query)).strip("\r\n") + logger.trace("Query for attribute", query=query, response=response) + return datatype(response) + + return get + + def _setter(self, param: str) -> Callable[[Any], Awaitable[None]]: + """A one-argument coroutine writing one parameter.""" + + async def put(value: Any) -> None: + command = f"{param}{self._suffix}={value}\r\n" + await self._connection.send_command(command) + logger.trace("Send command for attribute", command=command) + + return put diff --git a/src/fastcs/demo/temperature_scpi.py b/src/fastcs/demo/temperature_scpi.py new file mode 100644 index 00000000..9b1eb94e --- /dev/null +++ b/src/fastcs/demo/temperature_scpi.py @@ -0,0 +1,173 @@ +"""Example 4 - declarative: a SCPI device's attributes written as annotated hints. + +The same multi-ramp temperature controller as :mod:`fastcs.demo.temperature_attr`, and +the same device behind it, declared rather than constructed. Every attribute is a +class-body hint carrying a `SCPIParam` that says both what to send and what the +value means:: + + ramp_rate: Annotated[AttrRW[float], SCPIParam("R", precision=3, units="K/s")] + +Contrast with the two examples either side of it: + +- :mod:`fastcs.demo.temperature_attr` wires the same device **procedurally** - each + getter and setter handed to a constructor in ``__init__``. Use that when the + attributes depend on data only ``__init__`` has. +- :mod:`fastcs.demo.eiger` builds its attributes from what the device **reports**. Use + that when the device describes itself. + +A SCPI device does neither: it does not describe itself, which is exactly why its +attributes are hand-annotated, and the metadata it cannot report - precision, units, +a human-readable description - is written where the attribute is declared. + +This module also carries the composition and methods rungs: a `ControllerVector` of +ramp sub controllers, a ``@scan`` deriving values the device reports in one go, and +a ``@command`` acting across the whole vector. +""" + +import asyncio +import enum +import json +from dataclasses import dataclass +from typing import Annotated + +import numpy as np + +from fastcs.attributes import AttrR, AttrRW +from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.controllers import ControllerVector +from fastcs.datatypes import Array1D +from fastcs.demo.scpi import SCPIController, SCPIParam +from fastcs.logging import logger +from fastcs.methods import command, scan + + +class OnOffEnum(enum.StrEnum): + Off = "0" + On = "1" + + +@dataclass +class TemperatureControllerSettings: + num_ramp_controllers: int + ip_settings: IPConnectionSettings + + +class TemperatureRampController(SCPIController): + """One ramp, addressed by a two-digit suffix on every command token. + + The suffix is the whole of what distinguishes ramp 1 from ramp 2, so the + declarations below are written once and serve every ramp. + """ + + start: Annotated[ + AttrRW[int], + SCPIParam("S", description="Temperature the ramp starts from"), + ] + end: Annotated[ + AttrRW[int], + SCPIParam("E", description="Temperature the ramp finishes at"), + ] + enabled: Annotated[ + AttrRW[OnOffEnum], + SCPIParam("N", description="Whether this ramp is running"), + ] + # `precision` is the kind of thing a return annotation cannot carry: `-> float` + # says nothing about how many decimal places are meaningful. + target: Annotated[ + AttrR[float], + SCPIParam("T", precision=3, units="degC", description="Where the ramp is at"), + ] + actual: Annotated[ + AttrR[float], + SCPIParam("A", precision=3, units="degC", description="Measured temperature"), + ] + # Updated by the parent's `update_voltages` scan, which reads every ramp's + # voltage in one query, so this one has no command token of its own. + voltage: AttrR[float] + + def __init__(self, index: int, connection: IPConnection) -> None: + super().__init__( + connection, suffix=f"{index:02d}", description=f"Ramp {index}" + ) + + async def initialise(self) -> None: + await super().initialise() + + # No `SCPIParam`, so `SCPIController` left it alone: filled here with the + # metadata it wants and no IO, because the parent updates it. + self.filler.fill_attribute("voltage", precision=3, units="V") + + +class TemperatureController(SCPIController): + """The whole device: its own parameters, plus a ramp sub controller each.""" + + ramp_rate: Annotated[ + AttrRW[float], + SCPIParam("R", precision=3, units="K/s", description="Rate of change"), + ] + power: Annotated[ + AttrR[float], + SCPIParam("P", precision=3, units="W", description="Power draw"), + ] + # Read by the scan below in a single query rather than one per ramp, so it + # carries no command token either. + voltages: AttrR[Array1D[np.int32]] + + ramps: ControllerVector[TemperatureRampController] + + def __init__(self, settings: TemperatureControllerSettings) -> None: + self._settings = settings + + super().__init__(IPConnection()) + + self.ramps = ControllerVector( + { + index: TemperatureRampController(index, self.connection) + for index in range(1, settings.num_ramp_controllers + 1) + } + ) + + async def initialise(self) -> None: + await super().initialise() + + self.filler.fill_attribute("voltages", shape=(len(self.ramps),)) + + async def connect(self) -> None: + await self.connection.connect(self._settings.ip_settings) + + async def reconnect(self) -> None: + try: + await self.connection.close() + await self.connection.connect(self._settings.ip_settings) + except BaseException: + logger.exception("Reconnect failed") + return + + self._connected = True + + async def close(self) -> None: + await self.connection.close() + + @command() + async def cancel_all(self) -> None: + for ramp in self.ramps.values(): + await ramp.enabled.set(OnOffEnum.Off) + # TODO: The requests all get concatenated and the sim doesn't handle it + await asyncio.sleep(0.1) + + @scan(0.1) + async def update_voltages(self) -> None: + """One query for every ramp's voltage, fanned out to each of them. + + The device reports them together, so reading them together is one round + trip rather than one per ramp - which is why `voltage` is declared without + a command token of its own. + """ + response = (await self.connection.send_query("V?\r\n")).strip("\r\n") + voltages = np.array(json.loads(response), dtype=np.int32) + + await self.voltages.update(voltages) + + for index, ramp in self.ramps.items(): + self.log_event("Update voltages", topic=ramp.voltage, response=voltages) + await ramp.voltage.update(float(voltages[index - 1])) diff --git a/tests/demo/test_temperature_scpi.py b/tests/demo/test_temperature_scpi.py new file mode 100644 index 00000000..d8a80856 --- /dev/null +++ b/tests/demo/test_temperature_scpi.py @@ -0,0 +1,254 @@ +from typing import Annotated +from unittest.mock import AsyncMock + +import numpy as np +import pytest +import pytest_asyncio + +from fastcs.attributes import AttrR, AttrRW, AttrW +from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.controllers import ControllerVector +from fastcs.demo.scpi import SCPIController, SCPIParam +from fastcs.demo.temperature_scpi import ( + OnOffEnum, + TemperatureController, + TemperatureControllerSettings, + TemperatureRampController, +) + + +@pytest_asyncio.fixture +async def controller() -> TemperatureController: + settings = TemperatureControllerSettings( + num_ramp_controllers=4, + ip_settings=IPConnectionSettings(ip="localhost", port=25565), + ) + controller = TemperatureController(settings) + + # What `ControllerRunner` does before anything is served: the declarations + # are provisioned by `initialise`, not by construction. + await controller.initialise() + for ramp in controller.ramps.values(): + await ramp.initialise() + + return controller + + +@pytest_asyncio.fixture +async def ramp_controller( + controller: TemperatureController, +) -> TemperatureRampController: + return controller.ramps[1] + + +@pytest.mark.asyncio +async 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_every_declaration_was_provisioned(controller: TemperatureController): + controller.check_filled() + + assert set(controller.attributes) == {"ramp_rate", "power", "voltages"} + assert set(controller.ramps[1].attributes) == { + "start", + "end", + "enabled", + "target", + "actual", + "voltage", + } + + +@pytest.mark.asyncio +async def test_the_hint_decides_the_access_mode(controller: TemperatureController): + assert isinstance(controller.ramp_rate, AttrW) + # `power: AttrR[float]` has no setter half for the filler to provision + assert not isinstance(controller.power, AttrW) + + +@pytest.mark.asyncio +async def test_metadata_comes_from_the_param(controller: TemperatureController): + """The command token and the metadata are written in one place, and both end + up on the attribute.""" + assert controller.ramp_rate.meta == { + "precision": 3, + "units": "K/s", + "description": "Rate of change", + } + + +@pytest.mark.asyncio +async def test_an_attribute_without_a_param_is_left_to_the_driver( + controller: TemperatureController, +): + """`voltages` is updated by the scan, so it has no command token - the + controller fills it itself with the shape only it knows.""" + assert controller.voltages.meta.get("shape") == (4,) + assert not controller.voltages.has_getter() + + +@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.poll() + + controller.connection.send_query.assert_awaited_once_with("R?\r\n") + assert controller.ramp_rate.readback == 1.5 + + +@pytest.mark.asyncio +async def test_ramp_rate_written_to_device(controller: TemperatureController): + controller.connection.send_command = AsyncMock() + + await controller.ramp_rate.set(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.poll() + + controller.connection.send_query.assert_awaited_once_with("P?\r\n") + assert controller.power.readback == 10.25 + + +@pytest.mark.asyncio +async def test_a_ramp_suffixes_every_command( + ramp_controller: TemperatureRampController, +): + ramp_controller.connection.send_query = AsyncMock(return_value="7\r\n") + + await ramp_controller.start.poll() + + ramp_controller.connection.send_query.assert_awaited_once_with("S01?\r\n") + assert ramp_controller.start.readback == 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.set(42) + + ramp_controller.connection.send_command.assert_awaited_once_with("E01=42\r\n") + + +@pytest.mark.asyncio +async def test_an_enum_datatype_comes_from_the_hint( + ramp_controller: TemperatureRampController, +): + """`AttrRW[OnOffEnum]` is what parses the device's `0`/`1` into a member.""" + ramp_controller.connection.send_query = AsyncMock(return_value="1\r\n") + + await ramp_controller.enabled.poll() + + assert ramp_controller.enabled.readback is OnOffEnum.On + + +@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.set(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_cancel_all_disables_every_ramp(controller: TemperatureController): + sets = {} + for index, ramp in controller.ramps.items(): + sets[index] = AsyncMock() + ramp.enabled.set = sets[index] # type: ignore[method-assign] + + await controller.cancel_all() + + for set_ in sets.values(): + set_.assert_awaited_once_with(OnOffEnum.Off) + + +@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() + + controller.connection.send_query.assert_awaited_once_with("V?\r\n") + np.testing.assert_array_equal( + controller.voltages.readback, np.array([1, 2, 3, 4], dtype=np.int32) + ) + for index, ramp in controller.ramps.items(): + assert ramp.voltage.readback == pytest.approx(float(index)) + + +@pytest.mark.asyncio +async def test_metadata_is_validated_against_the_declared_datatype(): + """The runtime counterpart to the static `Unpack[FloatMeta]` check: a spec + object collects whatever it was told, so what it holds is only checkable when + it meets the datatype the hint declared.""" + + class Mislabelled(SCPIController): + serial: Annotated[AttrR[str], SCPIParam("SN", precision=3)] + + controller = Mislabelled(IPConnection()) + + with pytest.raises(TypeError) as error: + await controller.initialise() + + assert "'precision' is not valid metadata for str" in str(error.value) + assert "serial" in str(error.value) + + +@pytest.mark.asyncio +async def test_a_param_on_a_hint_that_names_no_datatype(): + """A SCPI device cannot be asked what it holds, so the hint has to say.""" + + class Underspecified(SCPIController): + state: Annotated[AttrR, SCPIParam("ST")] + + controller = Underspecified(IPConnection()) + + with pytest.raises(TypeError, match="does not say what it holds"): + await controller.initialise() + + +@pytest.mark.asyncio +async def test_a_hint_with_no_param_is_reported_as_unfilled(): + """`SCPIController` fills what carries a token; `check_filled` still holds + the driver to everything else its class body promised.""" + + class Forgetful(SCPIController): + discovered: AttrR + + controller = Forgetful(IPConnection()) + + with pytest.raises(RuntimeError, match="discovered"): + await controller.initialise() + + +@pytest.mark.asyncio +async def test_the_poll_period_is_a_class_attribute(): + class Slow(SCPIController): + poll_period = 5.0 + + reading: Annotated[AttrRW[float], SCPIParam("X")] + + controller = Slow(IPConnection()) + await controller.initialise() + + assert controller.reading.poll_period == 5.0 From 0141c3defeac5173e6cc920307d2a08a54cbc749 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 00:31:33 +0000 Subject: [PATCH 2/3] fix CI: ruff-format the new modules, and two ambiguous docstring roles Both failures were in what I could not see locally rather than in the code. `pre-commit run --all-files` only sees files git knows about, so the three new modules were invisible to it until they were staged - ruff-format then wanted `TemperatureRampController.__init__`'s super() call on one line. Staged first this time, and verified. The docs job builds with a network, so `default_role = "any"` resolves against the real intersphinx inventories; two single-backtick spans in the new docstrings - ``__init__`` and ``voltage`` - are prose rather than cross-references, and are now literals. Offline they were hidden among the intersphinx misses that having no network produces. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BymsiALDVCGv1nJWK8DyAV --- src/fastcs/demo/scpi.py | 2 +- src/fastcs/demo/temperature_scpi.py | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/fastcs/demo/scpi.py b/src/fastcs/demo/scpi.py index 5392fb7a..b13bc0a1 100644 --- a/src/fastcs/demo/scpi.py +++ b/src/fastcs/demo/scpi.py @@ -9,7 +9,7 @@ class Thermostat(SCPIController): That is the declarative half of ADR 0013 for a device that cannot be introspected: the class body says what exists and `SCPIController` provisions it, rather than -`__init__` wiring each getter and setter by hand as +``__init__`` wiring each getter and setter by hand as :mod:`fastcs.demo.temperature_attr` does. **This lives in the demo, not core FastCS** (ADR 0014 decision 3). Core defines no diff --git a/src/fastcs/demo/temperature_scpi.py b/src/fastcs/demo/temperature_scpi.py index 9b1eb94e..5e4b401c 100644 --- a/src/fastcs/demo/temperature_scpi.py +++ b/src/fastcs/demo/temperature_scpi.py @@ -86,9 +86,7 @@ class TemperatureRampController(SCPIController): voltage: AttrR[float] def __init__(self, index: int, connection: IPConnection) -> None: - super().__init__( - connection, suffix=f"{index:02d}", description=f"Ramp {index}" - ) + super().__init__(connection, suffix=f"{index:02d}", description=f"Ramp {index}") async def initialise(self) -> None: await super().initialise() @@ -160,7 +158,7 @@ async def update_voltages(self) -> None: """One query for every ramp's voltage, fanned out to each of them. The device reports them together, so reading them together is one round - trip rather than one per ramp - which is why `voltage` is declared without + trip rather than one per ramp - which is why ``voltage`` is declared without a command token of its own. """ response = (await self.connection.send_query("V?\r\n")).strip("\r\n") From 49be9940c3b3a6d6620a39815fc40685337847f7 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 14 Sep 2026 10:53:20 +0000 Subject: [PATCH 3/3] feat: add backend and factory for attrs to simplify filling getters/setters --- .../0021-attr-backend-and-factory.md | 85 ++++++++++ src/fastcs/attributes/__init__.py | 2 + src/fastcs/attributes/backend.py | 32 ++++ src/fastcs/attributes/factory.py | 102 ++++++++++++ src/fastcs/controllers/filler.py | 68 ++++++-- src/fastcs/demo/eiger.py | 2 +- src/fastcs/demo/scpi.py | 93 ++++++----- src/fastcs/demo/temperature_scpi.py | 8 +- tests/conftest.py | 20 +++ tests/test_attr_factory.py | 147 ++++++++++++++++++ tests/test_controller_filler.py | 76 +++++++-- tests/test_controllers.py | 25 ++- 12 files changed, 579 insertions(+), 81 deletions(-) create mode 100644 docs/explanations/decisions/0021-attr-backend-and-factory.md create mode 100644 src/fastcs/attributes/backend.py create mode 100644 src/fastcs/attributes/factory.py create mode 100644 tests/test_attr_factory.py diff --git a/docs/explanations/decisions/0021-attr-backend-and-factory.md b/docs/explanations/decisions/0021-attr-backend-and-factory.md new file mode 100644 index 00000000..d958c74a --- /dev/null +++ b/docs/explanations/decisions/0021-attr-backend-and-factory.md @@ -0,0 +1,85 @@ +# 21. AttrBackend and AttrFactory + +Date: 2026-09-10 + +**Related:** [ADR 9](0009-handler-to-attribute-io-pattern.md), +[ADR 13](0013-declarative-procedural-split-and-controller-filler.md), +[ADR 14](0014-attribute-io-rw-rework.md) + +## Status + +Accepted + +## Context + +`SCPIController` ([ADR 13](0013-declarative-procedural-split-and-controller-filler.md)'s +worked example of a filler-based protocol layer) hand-built a getter/setter closure +per declared attribute, each one closing over the attribute's command token and, +for a getter, the datatype that parses the device's text answer: + +```python +self.filler.fill_attribute( + declaration.name, + getter=Polled(self._getter(param.param, datatype), period=self.poll_period), + setter=setter, + **param.meta, +) +``` + +Every controller that binds many attributes to one wire protocol re-implements this +same shape - a closure factory, `Polled` wrapping, and an `isinstance` check to skip +building a setter for a read-only attribute - with nothing shared between them. There +was no protocol-agnostic concept of "a thing that knows how to get/set a value given +some protocol-specific arguments" for `ControllerFiller` to build on. + +## Decision + +Introduce `AttrBackend` (`fastcs/attributes/backend.py`), a `Protocol` describing +exactly that: + +```python +class AttrBackend(Protocol[*Ts, DType_T]): + async def get(self, *args: *Ts) -> DType_T: ... + async def set(self, value: DType_T, *args: *Ts) -> None: ... +``` + +and `AttrFactory` (`fastcs/attributes/factory.py`), built from one, with two halves: + +- **Construction** - `attr_r`/`attr_w`/`attr_rw` build a *new* attribute with the + backend's IO bound, for a controller that discovers attributes at runtime (no + `Declaration` to fill). +- **Filling** - `fill(attr, *args, polled=...)` binds IO onto an attribute a + `ControllerFiller` already created from a class-body hint, dispatching on the + attribute's actual kind (`AttrR`/`AttrW`/`AttrRW`) so callers don't branch + themselves. + +`ControllerFiller.fill_from_backend(declaration, backend, *args, polled=..., **meta)` +is the entry point a protocol layer like `SCPIController` actually calls: it looks up +`declaration.child`, delegates IO-binding to `AttrFactory.fill`, and applies `**meta` +through the existing `fill_attribute` (unchanged - `fill_attribute` already treats a +missing `getter`/`setter` as "nothing to bind here", so no new code path was needed +in it for the meta-only call `fill_from_backend` makes). + +**This is not `AttributeIO`/`AttributeIORef` (ADR 9) come back.** ADR 14 deleted that +pattern because its `AttributeIORef` existed solely to carry inert per-attribute data +until a class-scope `Attribute` instance's `AttributeIO` could be found *by type* at +`post_initialise()` - a problem [ADR 13](0013-declarative-procedural-split-and-controller-filler.md) +had already made obsolete by moving every attribute's construction into `__init__`/ +`initialise()`, where a live connection is already in scope. `AttrBackend` has no ref +object, no type-based dispatch registry, and no controller-wide "list of IOs to +connect": a controller constructs its backend and hands it directly to +`AttrFactory`/`fill_from_backend` itself, the same way it would hand a bare getter/ +setter to `fill_attribute` today. `SCPIController` now builds one `SCPIBackend` per +instance (mnemonic + parser as call arguments, not per-attribute construction state) +instead of one closure pair per attribute. + + +## Consequences + +- `SCPIController` migrates from two hand-built closure factories (`_getter`/ + `_setter`) to one `SCPIBackend` instance and a single `fill_from_backend` call; + behaviour is unchanged (every existing SCPI demo test passes unmodified). +- A future protocol layer (REST, say) gets the same shared mechanism for free: + implement `get`/`set`, hand the backend to `fill_from_backend`. +- `fill_attribute`'s existing raw-getter/setter path is untouched and still the right + choice for a controller that does not want a shared backend abstraction. diff --git a/src/fastcs/attributes/__init__.py b/src/fastcs/attributes/__init__.py index 326cfb65..db815320 100644 --- a/src/fastcs/attributes/__init__.py +++ b/src/fastcs/attributes/__init__.py @@ -13,5 +13,7 @@ from .attr_w import UnboundSetter as UnboundSetter from .attribute import Attribute as Attribute from .attribute import AttributeAccessMode as AttributeAccessMode +from .backend import AttrBackend as AttrBackend +from .factory import AttrFactory as AttrFactory from .severity import Severity as Severity from .update import Update as Update diff --git a/src/fastcs/attributes/backend.py b/src/fastcs/attributes/backend.py new file mode 100644 index 00000000..fa3a182c --- /dev/null +++ b/src/fastcs/attributes/backend.py @@ -0,0 +1,32 @@ +"""`AttrBackend` - a protocol-agnostic get/set for one attribute's value.""" + +from __future__ import annotations + +from typing import Protocol, TypeVarTuple + +from fastcs.attributes.update import Update +from fastcs.datatypes import DType_T + +Ts = TypeVarTuple("Ts") +"""The positional arguments a backend's `get`/`set` identify a resource by""" + + +class AttrBackend(Protocol[*Ts, DType_T]): + """Something that can `get`/`set` a value given protocol-specific arguments. + + One instance typically serves every attribute a controller declares. `args` is how + a single call says *which* attribute it means. + """ + + async def get(self, *args: *Ts) -> DType_T: + """Read the value identified by `args`.""" + ... + + async def set(self, value: DType_T, *args: *Ts) -> DType_T | Update[DType_T] | None: + """Write `value` to the resource identified by `args`. + + Mirrors `Setter[DType_T]` (ADR 0014): `None` is fire-and-forget, and a + returned value or `Update` is the device's accepted/clamped value, applied + to the attribute's readback and setpoint immediately. + """ + ... diff --git a/src/fastcs/attributes/factory.py b/src/fastcs/attributes/factory.py new file mode 100644 index 00000000..0ffea6b3 --- /dev/null +++ b/src/fastcs/attributes/factory.py @@ -0,0 +1,102 @@ +"""`AttrFactory` - binds an `AttrBackend`'s IO onto attributes.""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Generic, cast + +from fastcs.attributes.attr_r import AttrR, Getter, NotPolled, Polled, Schedule +from fastcs.attributes.attr_rw import AttrRW +from fastcs.attributes.attr_w import AttrW, Setter +from fastcs.attributes.attribute import Attribute +from fastcs.attributes.backend import AttrBackend, Ts +from fastcs.attributes.update import Update +from fastcs.datatypes import DType_T + + +@dataclass +class AttrFactory(Generic[*Ts, DType_T]): + """Binds a backend's IO onto attributes - new ones, or ones already declared.""" + + backend: AttrBackend[*Ts, DType_T] + + def attr_r( + self, + datatype: type[DType_T], + *args: *Ts, + schedule: Schedule[DType_T] | None = None, + ) -> AttrR[DType_T]: + """Build a new read-only attribute, its getter bound to this backend.""" + getter = self._schedule(*args, schedule=schedule) + attribute = AttrR(cast(Any, datatype), getter=getter) + return cast(AttrR[DType_T], attribute) + + def attr_w(self, datatype: type[DType_T], *args: *Ts) -> AttrW[DType_T]: + """Build a new write-only attribute, its setter bound to this backend.""" + attribute = AttrW(cast(Any, datatype), setter=self._setter(*args)) + return cast(AttrW[DType_T], attribute) + + def attr_rw( + self, + datatype: type[DType_T], + *args: *Ts, + schedule: Schedule[DType_T] | None = None, + ) -> AttrRW[DType_T]: + """Build a new read-write attribute, its getter/setter bound to this backend.""" + attribute = AttrRW( + cast(Any, datatype), + getter=self._schedule(*args, schedule=schedule), + setter=self._setter(*args), + ) + return cast(AttrRW[DType_T], attribute) + + def fill( + self, + attr: Attribute[DType_T], + *args: *Ts, + schedule: Schedule[DType_T] | None = None, + ) -> None: + """Bind this backend's get/set onto an already-constructed attribute. + + Args: + attr: The already-constructed attribute to bind IO onto + args: Forwarded to the backend's `get`/`set` to identify the resource + schedule: A bare `Polled(period=...)`/`NotPolled()` to read the getter + on, or `None` to read once, at connect + + """ + if isinstance(attr, AttrR): + attr.set_getter(self._schedule(*args, schedule=schedule)) + if isinstance(attr, AttrW): + attr.set_setter(self._setter(*args)) + + def _schedule( + self, *args: *Ts, schedule: Schedule[DType_T] | None + ) -> Getter[DType_T] | Schedule[DType_T]: + """This backend's getter, with `schedule` applied. + + `Polled`/`NotPolled` bind a getter onto themselves when called (the + same mechanism `AttrR.declare`'s own `schedule` argument uses), so + there is no `Polled`/`NotPolled` branch to write here. + """ + getter = self._getter(*args) + + if isinstance(schedule, Polled | NotPolled) and schedule.getter is not None: + raise TypeError( + "The schedule given to `AttrFactory` already has a getter; pass " + "a bare Polled(period=...) or NotPolled()" + ) + + return getter if schedule is None else schedule(getter) + + def _getter(self, *args: *Ts) -> Getter[DType_T]: + async def get() -> DType_T: + return await self.backend.get(*args) + + return get + + def _setter(self, *args: *Ts) -> Setter[DType_T]: + async def set_(value: DType_T) -> DType_T | Update[DType_T] | None: + return await self.backend.set(value, *args) + + return set_ diff --git a/src/fastcs/controllers/filler.py b/src/fastcs/controllers/filler.py index 3a53495a..f02cd0d6 100644 --- a/src/fastcs/controllers/filler.py +++ b/src/fastcs/controllers/filler.py @@ -14,7 +14,9 @@ class OdinDetector(Controller): frames: AttrRW[int] # exists as soon as __init__ returns async def initialise(self) -> None: - self.filler.fill_attribute("frames", getter=..., setter=...) + self.filler.fill_attribute( + self.filler.declarations["frames"], getter=..., setter=... + ) so ``self.frames`` can be referenced by the rest of ``__init__`` - the rule ADR 0013 takes from ophyd-async, and what makes ``initialise`` safe to run in @@ -47,6 +49,8 @@ async def initialise(self) -> None: from fastcs.attributes import Attribute, AttrR, AttrW from fastcs.attributes.attr_r import Getter, Schedule from fastcs.attributes.attr_w import Setter +from fastcs.attributes.backend import AttrBackend, Ts +from fastcs.attributes.factory import AttrFactory from fastcs.datatypes import DType, DType_T, Meta, validate_meta from fastcs.methods import Method @@ -231,7 +235,7 @@ def __iter__(self) -> Iterator[tuple[Attribute | None, tuple[Any, ...]]]: def fill_attribute( self, - name: str, + declaration: Declaration, getter: Getter[DType_T] | Schedule[DType_T] | None = None, setter: Setter[DType_T] | None = None, datatype: type[DType_T] | None = None, @@ -240,7 +244,11 @@ def fill_attribute( """Provision a declared attribute with its IO and metadata. Args: - name: The name the class body declared + declaration: The class-body declaration to fill, as yielded by + `declarations` or iteration over this filler. Look one up by + name with ``self.filler.declarations[name]`` if you don't + already have it - iterating declarations to find the one you + want is the common case this signature is for. getter: IO to read the value with, optionally wrapped in a `Polled`/`NotPolled` schedule setter: IO to write the value with @@ -255,31 +263,25 @@ def fill_attribute( The attribute, which is the same object the hint created Raises: - KeyError: If nothing of that name was declared + KeyError: If the declaration names a promise rather than a built + attribute TypeError: If the attribute has no half the given IO would fill, the datatype disagrees with the hint, or the metadata does not suit the datatype """ - declaration = self._declarations.get(name) - if declaration is None: - raise KeyError( - f"{type(self._controller).__name__} has no attribute declaration " - f"named '{name}' to fill. Declare it as a class-body hint with its " - "datatype, or add the attribute with `add_attribute`." - ) - if declaration.child is None: # A hint that does not name its datatype - `state: AttrR` - is a # promise rather than something the filler could build, so there is # no attribute here to provision. raise KeyError( - f"{type(self._controller).__name__} declared '{name}' as " + f"{type(self._controller).__name__} declared '{declaration.name}' as " f"{declaration.hint.type_} without a datatype, so there is no " "attribute to fill. Subscript the hint with the datatype it holds, " "or add the attribute with `add_attribute`." ) + name = declaration.name attribute = declaration.child # The whole request is checked before any of it is applied, so that a @@ -333,13 +335,49 @@ def fill_attribute( return attribute - def fill_meta(self, name: str, meta: Meta) -> Attribute: + def fill_from_backend( + self, + declaration: Declaration, + backend: AttrBackend[*Ts, DType_T], + *args: *Ts, + schedule: Schedule[DType_T] | None = None, + **meta: Unpack[Meta], + ) -> Attribute: + """Fill a declared attribute's IO from a backend, then apply its metadata. + + Args: + declaration: The class-body declaration to fill + args: Forwarded to the backend's `get`/`set` to identify the resource + schedule: A bare `Polled(period=...)`/`NotPolled()` to read the getter + on, or `None` to read once, at connect - see `AttrFactory.fill` + meta: Metadata for the attribute, validated against the datatype the + hint declared + + Returns: + The attribute, which is the same object the hint created + + Raises: + KeyError: If the declaration names a promise rather than a built + attribute - see `fill_attribute` + + """ + # Meta first, IO second: `fill_attribute` is what raises for a promise + # or bad metadata, and it should do so before any IO is bound, not after + # - otherwise a rejected fill would leave the attribute's getter/setter + # already set from the backend, refusing a corrected retry. + attribute = self.fill_attribute(declaration, **meta) + + AttrFactory(backend).fill(attribute, *args, schedule=schedule) + + return attribute + + def fill_meta(self, declaration: Declaration, meta: Meta) -> Attribute: """Fill a declared attribute's metadata from an extras object. The shape a protocol layer wants: ``SCPIParam(...).meta`` in one go, validated against the datatype the hint declared. """ - return self.fill_attribute(name, **meta) + return self.fill_attribute(declaration, **meta) def check_filled(self) -> None: """Raise if anything the class body promised does not exist. diff --git a/src/fastcs/demo/eiger.py b/src/fastcs/demo/eiger.py index afebc0c7..e7c2d9ed 100644 --- a/src/fastcs/demo/eiger.py +++ b/src/fastcs/demo/eiger.py @@ -171,7 +171,7 @@ async def initialise(self) -> None: # access mode and datatype the hint promised against what # the device turned out to report. self.filler.fill_attribute( - param, datatype=datatype, getter=getter, setter=setter + declaration, datatype=datatype, getter=getter, setter=setter ) elif setter is None: self.add_attribute(param, AttrR(datatype, getter=getter)) diff --git a/src/fastcs/demo/scpi.py b/src/fastcs/demo/scpi.py index b13bc0a1..cce40410 100644 --- a/src/fastcs/demo/scpi.py +++ b/src/fastcs/demo/scpi.py @@ -12,17 +12,18 @@ class Thermostat(SCPIController): ``__init__`` wiring each getter and setter by hand as :mod:`fastcs.demo.temperature_attr` does. -**This lives in the demo, not core FastCS** (ADR 0014 decision 3). Core defines no -extras vocabulary for 1.0 - it defines the *mechanism*, which is the ``extras`` an -``Annotated`` hint carries through to `ControllerFiller`. What a protocol package -puts in there is its own business, and this module is the worked example of one -doing it: `SCPIParam` is a sibling of ophyd-async's ``PvSuffix``, not a FastCS type. + +Every declared attribute reads and writes through the same wire protocol, differing +only in its command token and (for a getter) the datatype that parses the device's +text answer - the shared `AttrBackend`/`AttrFactory` mechanism this module's +`SCPIBackend` is a worked example of, rather than a getter/setter closure hand-built +per attribute. """ -from collections.abc import Awaitable, Callable +from collections.abc import Callable from typing import Any, Unpack, cast -from fastcs.attributes import AttrW, Polled +from fastcs.attributes import Polled from fastcs.connections import IPConnection from fastcs.controllers import Controller from fastcs.controllers.filler import Declaration @@ -63,6 +64,37 @@ def __repr__(self) -> str: return f"SCPIParam({self.param!r}, {self.meta})" +class SCPIBackend: + """Reads and writes one SCPI parameter, given its mnemonic and datatype. + + Shared across every attribute an `SCPIController` declares, rather than a + getter/setter closure hand-built per attribute: `SCPIController._fill` supplies + the mnemonic and the datatype that parses the device's text answer as call + arguments (see `fastcs.attributes.factory.AttrFactory`), not as construction + state closed over per attribute. + """ + + def __init__(self, connection: IPConnection, suffix: str) -> None: + self._connection = connection + self._suffix = suffix + + async def get(self, param: str, datatype: Callable[[str], DType_T]) -> DType_T: + query = f"{param}{self._suffix}?\r\n" + response = (await self._connection.send_query(query)).strip("\r\n") + # The hint's datatype doubles as the parser for the device's text answer, + # which every datatype FastCS serves happens to support - ``float("1.5")``, + # ``OnOffEnum("1")`` - but nothing in its type says so, hence the cast at + # the call site that built this `datatype` argument. + return datatype(response) + + async def set( + self, value: DType_T, param: str, _datatype: Callable[[str], DType_T] + ) -> None: + command = f"{param}{self._suffix}={value}\r\n" + await self._connection.send_command(command) + logger.trace("Send command for attribute", command=command) + + class SCPIController(Controller): """A controller whose attributes are declared as `SCPIParam` hints. @@ -92,8 +124,7 @@ def __init__( suffix: str = "", description: str | None = None, ) -> None: - self._connection = connection - self._suffix = suffix + self._backend = SCPIBackend(connection, suffix) super().__init__(description) @@ -142,43 +173,11 @@ def _fill(self, declaration: Declaration, param: SCPIParam) -> None: f"`{declaration.raw_name}: Annotated[AttrRW[float], ...]`." ) - setter = None - if isinstance(declaration.child, AttrW): - setter = self._setter(param.param) - - self.filler.fill_attribute( - declaration.name, - # The hint's datatype doubles as the parser for the device's text - # answer, which every datatype FastCS serves happens to support - - # ``float("1.5")``, ``OnOffEnum("1")`` - but nothing in its type says - # so, hence the cast. - getter=Polled( - self._getter(param.param, cast(Callable[[str], Any], datatype)), - period=self.poll_period, - ), - setter=setter, + self.filler.fill_from_backend( + declaration, + self._backend, + param.param, + cast(Callable[[str], Any], datatype), + schedule=Polled(period=self.poll_period), **param.meta, ) - - def _getter( - self, param: str, datatype: Callable[[str], DType_T] - ) -> Callable[[], Awaitable[DType_T]]: - """A zero-argument coroutine reading one parameter, parsed into its type.""" - - async def get() -> DType_T: - query = f"{param}{self._suffix}?\r\n" - response = (await self._connection.send_query(query)).strip("\r\n") - logger.trace("Query for attribute", query=query, response=response) - return datatype(response) - - return get - - def _setter(self, param: str) -> Callable[[Any], Awaitable[None]]: - """A one-argument coroutine writing one parameter.""" - - async def put(value: Any) -> None: - command = f"{param}{self._suffix}={value}\r\n" - await self._connection.send_command(command) - logger.trace("Send command for attribute", command=command) - - return put diff --git a/src/fastcs/demo/temperature_scpi.py b/src/fastcs/demo/temperature_scpi.py index 5e4b401c..17017f5a 100644 --- a/src/fastcs/demo/temperature_scpi.py +++ b/src/fastcs/demo/temperature_scpi.py @@ -93,7 +93,9 @@ async def initialise(self) -> None: # No `SCPIParam`, so `SCPIController` left it alone: filled here with the # metadata it wants and no IO, because the parent updates it. - self.filler.fill_attribute("voltage", precision=3, units="V") + self.filler.fill_attribute( + self.filler.declarations["voltage"], precision=3, units="V" + ) class TemperatureController(SCPIController): @@ -128,7 +130,9 @@ def __init__(self, settings: TemperatureControllerSettings) -> None: async def initialise(self) -> None: await super().initialise() - self.filler.fill_attribute("voltages", shape=(len(self.ramps),)) + self.filler.fill_attribute( + self.filler.declarations["voltages"], shape=(len(self.ramps),) + ) async def connect(self) -> None: await self.connection.connect(self._settings.ip_settings) diff --git a/tests/conftest.py b/tests/conftest.py index 569b863e..d28c8930 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -40,6 +40,26 @@ def clear_softioc_records(): builder.ClearRecords() +class FakeBackend: + """Records every `get`/`set` call an `AttrBackend` receives, and returns + fixed values - shared by the `AttrFactory`/`ControllerFiller.fill_from_backend` + tests.""" + + def __init__(self, value: float = 1.5, set_result: float | None = None) -> None: + self.value = value + self.set_result = set_result + self.gets: list[tuple[object, ...]] = [] + self.sets: list[tuple[float, tuple[object, ...]]] = [] + + async def get(self, *args: object) -> float: + self.gets.append(args) + return self.value + + async def set(self, value: float, *args: object) -> float | None: + self.sets.append((value, args)) + return self.set_result + + class BackendTestController(MyTestController): read_int: AttrR[int] read_write_int: AttrRW[int] diff --git a/tests/test_attr_factory.py b/tests/test_attr_factory.py new file mode 100644 index 00000000..fc313211 --- /dev/null +++ b/tests/test_attr_factory.py @@ -0,0 +1,147 @@ +import pytest + +from fastcs.attributes import AttrFactory, AttrR, AttrRW, AttrW, NotPolled, Polled +from fastcs.util import ONCE +from tests.conftest import FakeBackend + + +@pytest.mark.asyncio +async def test_attr_r_builds_a_readable_attribute_bound_to_the_backend(): + backend = FakeBackend(value=2.5) + + attr = AttrFactory(backend).attr_r(float, "R") + + assert isinstance(attr, AttrR) + assert await attr.poll() == 2.5 + assert backend.gets == [("R",)] + + +@pytest.mark.parametrize( + "schedule, expected_poll_period", + [(None, ONCE), (Polled(period=0.5), 0.5), (NotPolled(), None)], + ids=["no-schedule", "polled", "not-polled"], +) +def test_attr_r_applies_the_given_schedule( + schedule: Polled[float] | NotPolled[float] | None, expected_poll_period +): + attr = AttrFactory(FakeBackend()).attr_r(float, "R", schedule=schedule) + + assert attr.poll_period == expected_poll_period + + +@pytest.mark.parametrize( + "schedule, expected_poll_period", + [(None, ONCE), (Polled(period=0.5), 0.5), (NotPolled(), None)], + ids=["no-schedule", "polled", "not-polled"], +) +def test_attr_rw_applies_the_given_schedule( + schedule: Polled[float] | NotPolled[float] | None, expected_poll_period +): + attr = AttrFactory(FakeBackend()).attr_rw(float, "R", schedule=schedule) + + assert attr.poll_period == expected_poll_period + + +@pytest.mark.asyncio +async def test_attr_w_builds_a_writable_attribute_bound_to_the_backend(): + backend = FakeBackend() + + attr = AttrFactory(backend).attr_w(float, "R") + + assert isinstance(attr, AttrW) + await attr.set(3.5) + assert backend.sets == [(3.5, ("R",))] + + +@pytest.mark.asyncio +async def test_attr_rw_binds_both_getter_and_setter(): + backend = FakeBackend(value=4.5) + + attr = AttrFactory(backend).attr_rw(float, "R") + + assert isinstance(attr, AttrRW) + assert await attr.poll() == 4.5 + await attr.set(5.5) + assert backend.sets == [(5.5, ("R",))] + + +@pytest.mark.asyncio +async def test_fill_binds_a_getter_onto_a_read_only_attribute(): + backend = FakeBackend(value=6.5) + attr = AttrR(float) + + AttrFactory(backend).fill(attr, "R") + + assert await attr.poll() == 6.5 + assert backend.gets == [("R",)] + + +@pytest.mark.asyncio +async def test_fill_binds_a_setter_onto_a_write_only_attribute(): + backend = FakeBackend() + attr = AttrW(float) + + AttrFactory(backend).fill(attr, "R") + + await attr.set(7.5) + assert backend.sets == [(7.5, ("R",))] + + +@pytest.mark.asyncio +async def test_fill_setter_return_value_updates_the_readback(): + """A setter returning a value is the device's accepted/clamped value.""" + backend = FakeBackend(set_result=6.0) + attr = AttrRW(float) + + AttrFactory(backend).fill(attr, "R") + await attr.set(5.0) + + assert attr.readback == 6.0 + + +@pytest.mark.asyncio +async def test_fill_binds_both_halves_onto_a_read_write_attribute(): + backend = FakeBackend(value=8.5) + attr = AttrRW(float) + + AttrFactory(backend).fill(attr, "R") + + assert await attr.poll() == 8.5 + await attr.set(9.5) + assert backend.sets == [(9.5, ("R",))] + + +@pytest.mark.parametrize( + "schedule, expected_poll_period", + [(None, ONCE), (Polled(period=0.2), 0.2), (NotPolled(), None)], + ids=["no-schedule", "polled", "not-polled"], +) +def test_fill_applies_the_given_schedule( + schedule: Polled[float] | NotPolled[float] | None, expected_poll_period +): + attr = AttrR(float) + + AttrFactory(FakeBackend()).fill(attr, "R", schedule=schedule) + + assert attr.poll_period == expected_poll_period + + +def test_fill_with_a_schedule_that_already_has_a_getter_raises(): + attr = AttrR(float) + + async def get() -> float: + return 0.0 + + with pytest.raises(TypeError, match="already has a getter"): + AttrFactory(FakeBackend()).fill(attr, "R", schedule=Polled(get, period=0.2)) + + +@pytest.mark.asyncio +async def test_fill_forwards_every_positional_argument_to_the_backend(): + backend = FakeBackend() + attr = AttrR(float) + + AttrFactory(backend).fill(attr, "R", "01") + await attr.poll() + + assert backend.gets == [("R", "01")] diff --git a/tests/test_controller_filler.py b/tests/test_controller_filler.py index f52cd9a8..9d87b786 100644 --- a/tests/test_controller_filler.py +++ b/tests/test_controller_filler.py @@ -10,6 +10,7 @@ from fastcs.attributes import AttrR, AttrRW, AttrW, NotPolled from fastcs.controllers import BaseController, Controller, ControllerVector from fastcs.datatypes import Array1D +from tests.conftest import FakeBackend class Colour(enum.Enum): @@ -214,7 +215,9 @@ class Declared(Controller): async def get() -> float: return 2.5 - controller.filler.fill_attribute("reading", getter=NotPolled(get)) + controller.filler.fill_attribute( + controller.filler.declarations["reading"], getter=NotPolled(get) + ) assert controller.reading.poll_period is None assert await controller.reading.poll() == 2.5 @@ -230,7 +233,9 @@ async def put(value: float) -> None: pass with pytest.raises(TypeError, match="nothing to write"): - controller.filler.fill_attribute("reading", setter=put) + controller.filler.fill_attribute( + controller.filler.declarations["reading"], setter=put + ) def test_filling_a_getter_on_a_write_only_attribute_raises(): @@ -243,7 +248,9 @@ async def get() -> float: return 0.0 with pytest.raises(TypeError, match="nothing to read"): - controller.filler.fill_attribute("demand", getter=get) + controller.filler.fill_attribute( + controller.filler.declarations["demand"], getter=get + ) def test_filling_twice_raises(): @@ -255,10 +262,11 @@ class Declared(Controller): async def get() -> float: return 0.0 - controller.filler.fill_attribute("reading", getter=get) + declaration = controller.filler.declarations["reading"] + controller.filler.fill_attribute(declaration, getter=get) with pytest.raises(ValueError, match="already has a getter"): - controller.filler.fill_attribute("reading", getter=get) + controller.filler.fill_attribute(declaration, getter=get) @pytest.mark.asyncio @@ -275,10 +283,12 @@ async def get() -> float: async def put(value: float) -> None: pass + declaration = controller.filler.declarations["reading"] + with pytest.raises(TypeError, match="nothing to write"): - controller.filler.fill_attribute("reading", getter=get, setter=put) + controller.filler.fill_attribute(declaration, getter=get, setter=put) - controller.filler.fill_attribute("reading", getter=get) + controller.filler.fill_attribute(declaration, getter=get) assert await controller.reading.poll() == 3.5 @@ -291,7 +301,7 @@ class Declared(Controller): with pytest.raises(TypeError, match="not valid metadata"): controller.filler.fill_attribute( - "reading", + controller.filler.declarations["reading"], units="mm", structured_dtype=[("index", np.int32)], # pyright: ignore[reportCallIssue] ) @@ -304,11 +314,59 @@ class Declared(Controller): reading: AttrR[float] controller = Declared() - controller.filler.fill_meta("reading", {"units": "mm", "precision": 2}) + controller.filler.fill_meta( + controller.filler.declarations["reading"], {"units": "mm", "precision": 2} + ) + + assert controller.reading.meta == {"units": "mm", "precision": 2} + + +@pytest.mark.asyncio +async def test_fill_from_backend_binds_getter_and_setter_from_the_backend(): + class Declared(Controller): + reading: AttrRW[float] + + controller = Declared() + backend = FakeBackend(value=2.5) + + controller.filler.fill_from_backend( + controller.filler.declarations["reading"], backend, "R" + ) + + assert await controller.reading.poll() == 2.5 + await controller.reading.set(3.5) + assert backend.sets == [(3.5, ("R",))] + + +def test_fill_from_backend_applies_metadata(): + class Declared(Controller): + reading: AttrR[float] + + controller = Declared() + + controller.filler.fill_from_backend( + controller.filler.declarations["reading"], + FakeBackend(), + "R", + units="mm", + precision=2, + ) assert controller.reading.meta == {"units": "mm", "precision": 2} +def test_fill_from_backend_on_a_promise_raises(): + class Declared(Controller): + state: AttrR + + controller = Declared() + + with pytest.raises(KeyError, match="without a datatype"): + controller.filler.fill_from_backend( + controller.filler.declarations["state"], FakeBackend(), "R" + ) + + def test_hinted_attributes_are_not_shared_between_instances(): class Declared(Controller): count: AttrRW[int] diff --git a/tests/test_controllers.py b/tests/test_controllers.py index fb8575c2..b6f1e246 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -187,7 +187,10 @@ async def put(value: int) -> None: pass controller.filler.fill_attribute( - "read_write_int", getter=Polled(get, period=0.5), setter=put, units="counts" + controller.filler.declarations["read_write_int"], + getter=Polled(get, period=0.5), + setter=put, + units="counts", ) # Filled in place, so a reference taken during __init__ is still the one @@ -205,7 +208,9 @@ class HintedController(Controller): controller = HintedController() with pytest.raises(TypeError, match="wrong datatype"): - controller.filler.fill_attribute("read_write_int", datatype=float) + controller.filler.fill_attribute( + controller.filler.declarations["read_write_int"], datatype=float + ) def test_filling_metadata_the_datatype_has_no_use_for_raises(): @@ -215,7 +220,9 @@ class HintedController(Controller): controller = HintedController() with pytest.raises(TypeError, match="'precision' is not valid metadata for str"): - controller.filler.fill_attribute("label", precision=3) + controller.filler.fill_attribute( + controller.filler.declarations["label"], precision=3 + ) def test_filling_something_that_was_never_declared_raises(): @@ -224,8 +231,8 @@ class HintedController(Controller): controller = HintedController() - with pytest.raises(KeyError, match="no attribute declaration"): - controller.filler.fill_attribute("not_declared") + with pytest.raises(KeyError, match="not_declared"): + controller.filler.declarations["not_declared"] class PromisedAttrController(Controller): @@ -285,13 +292,17 @@ def test_filling_an_enum_attribute_with_another_enum_raises(): controller = EnumHintedController() with pytest.raises(TypeError, match="wrong datatype"): - controller.filler.fill_attribute("colour", datatype=BadEnum) + controller.filler.fill_attribute( + controller.filler.declarations["colour"], datatype=BadEnum + ) def test_filling_an_enum_attribute_with_the_declared_enum_is_accepted(): controller = EnumHintedController() - controller.filler.fill_attribute("colour", datatype=GoodEnum) + controller.filler.fill_attribute( + controller.filler.declarations["colour"], datatype=GoodEnum + ) assert controller.colour.dtype is GoodEnum