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 110f518a..3865e7a2 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) | +| `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) | ## 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** — `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 →"*. +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 +`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/controllers.py deleted file mode 100755 index b39937ee..00000000 --- a/src/fastcs/demo/controllers.py +++ /dev/null @@ -1,147 +0,0 @@ -import asyncio -import enum -import json -from dataclasses import KW_ONLY, dataclass -from typing import TypeVar - -import numpy as np - -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW -from fastcs.connections import IPConnection, IPConnectionSettings -from fastcs.controllers import Controller, ControllerVector -from fastcs.datatypes import Enum, Float, Int, Waveform -from fastcs.logging import logger -from fastcs.methods import command, scan - -NumberT = TypeVar("NumberT", int, float) - - -class OnOffEnum(enum.StrEnum): - Off = "0" - On = "1" - - -@dataclass -class TemperatureControllerSettings: - num_ramp_controllers: int - ip_settings: IPConnectionSettings - - -@dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str - _: KW_ONLY - update_period: float | None = 0.2 - - -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): - def __init__(self, connection: IPConnection, suffix: str): - 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") - self.log_event( - "Query for attribute", - topic=attr, - query=query, - response=response, - ) - - await attr.update(attr.dtype(response)) - - -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,))) - - def __init__(self, settings: TemperatureControllerSettings) -> None: - self.connection = IPConnection() - self.suffix = "" - super().__init__( - ios=[TemperatureControllerAttributeIO(self.connection, self.suffix)] - ) - - self._settings = settings - - self.ramps = ControllerVector( - { - index: TemperatureRampController(index, self.connection) - for index in range(1, settings.num_ramp_controllers + 1) - } - ) - - @command() - async def cancel_all(self) -> None: - for rc in self.ramps.values(): - await rc.enabled.put(OnOffEnum.Off, sync_setpoint=True) - # TODO: The requests all get concatenated and the sim doesn't handle it - await asyncio.sleep(0.1) - - async def connect(self) -> None: - await self.connection.connect(self._settings.ip_settings) - - async def reconnect(self): - 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() - - @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") - ) - - await self.voltages.update(voltages) - - for index, controller in self.ramps.items(): - self.log_event( - "Update voltages", - topic=controller.voltage, - query=query, - response=voltages, - ) - await controller.voltage.update(float(voltages[index - 1])) - - -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.connection = conn diff --git a/src/fastcs/demo/temperature_attr.py b/src/fastcs/demo/temperature_attr.py new file mode 100755 index 00000000..f91b35e8 --- /dev/null +++ b/src/fastcs/demo/temperature_attr.py @@ -0,0 +1,253 @@ +"""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 Any, TypeVar + +import numpy as np + +from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.controllers import Controller, ControllerVector +from fastcs.datatypes import Enum, Float, Int, Waveform +from fastcs.logging import logger +from fastcs.methods import command, scan + +NumberT = TypeVar("NumberT", int, float) + + +class OnOffEnum(enum.StrEnum): + Off = "0" + On = "1" + + +@dataclass +class TemperatureControllerSettings: + num_ramp_controllers: int + 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 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 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 + + 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, + query=query, + response=response, + ) + + 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): + def __init__(self, settings: TemperatureControllerSettings) -> 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, + ), + ) + 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( + { + index: TemperatureRampController(index, self.connection) + for index in range(1, settings.num_ramp_controllers + 1) + } + ) + + @command() + async def cancel_all(self) -> None: + for rc in self.ramps.values(): + await rc.enabled.put(OnOffEnum.Off, sync_setpoint=True) + # TODO: The requests all get concatenated and the sim doesn't handle it + await asyncio.sleep(0.1) + + async def connect(self) -> None: + await self.connection.connect(self._settings.ip_settings) + + async def reconnect(self): + 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() + + @scan(0.1) + async def update_voltages(self): + query = self._protocol.get_voltages() + voltages = json.loads((await self.connection.send_query(query)).strip("\r\n")) + + await self.voltages.update(voltages) + + for index, controller in self.ramps.items(): + self.log_event( + "Update voltages", + topic=controller.voltage, + query=query, + response=voltages, + ) + await controller.voltage.update(float(voltages[index - 1])) + + +class TemperatureRampController(Controller): + def __init__(self, index: int, conn: IPConnection) -> None: + 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/tests/demo/test_controllers.py b/tests/demo/test_controllers.py deleted file mode 100644 index bd7775bd..00000000 --- a/tests/demo/test_controllers.py +++ /dev/null @@ -1,60 +0,0 @@ -from unittest.mock import AsyncMock - -import numpy as np -import pytest - -from fastcs.connections import IPConnectionSettings -from fastcs.controllers import ControllerVector -from fastcs.demo.controllers import ( - OnOffEnum, - TemperatureController, - TemperatureControllerSettings, - TemperatureRampController, -) - - -@pytest.fixture -def controller() -> TemperatureController: - settings = TemperatureControllerSettings( - num_ramp_controllers=4, - ip_settings=IPConnectionSettings(ip="localhost", port=25565), - ) - controller = TemperatureController(settings) - controller.post_initialise() - return controller - - -def test_ramps_is_controller_vector(controller: TemperatureController): - assert isinstance(controller.ramps, ControllerVector) - assert list(controller.ramps) == [1, 2, 3, 4] - for index, ramp in controller.ramps.items(): - assert isinstance(ramp, TemperatureRampController) - assert controller.ramps[index] is ramp - - -@pytest.mark.asyncio -async def test_cancel_all_disables_every_ramp(controller: TemperatureController): - puts = {} - for index, ramp in controller.ramps.items(): - puts[index] = AsyncMock() - ramp.enabled.put = puts[index] # type: ignore[method-assign] - - await controller.cancel_all() - - for put in puts.values(): - put.assert_awaited_once_with(OnOffEnum.Off, sync_setpoint=True) - - -@pytest.mark.asyncio -async def test_update_voltages_updates_waveform_and_each_ramp( - controller: TemperatureController, -): - controller.connection.send_query = AsyncMock(return_value="[1, 2, 3, 4]\r\n") - - await controller.update_voltages() - - np.testing.assert_array_equal( - controller.voltages.get(), np.array([1, 2, 3, 4], dtype=np.int32) - ) - for index, ramp in controller.ramps.items(): - assert ramp.voltage.get() == pytest.approx(float(index)) diff --git a/tests/demo/test_temperature_attr.py b/tests/demo/test_temperature_attr.py new file mode 100644 index 00000000..43703420 --- /dev/null +++ b/tests/demo/test_temperature_attr.py @@ -0,0 +1,144 @@ +from unittest.mock import AsyncMock + +import numpy as np +import pytest + +from fastcs.connections import IPConnectionSettings +from fastcs.controllers import ControllerVector +from fastcs.demo.temperature_attr import ( + OnOffEnum, + TemperatureController, + TemperatureControllerSettings, + TemperatureRampController, +) + + +@pytest.fixture +def controller() -> TemperatureController: + settings = TemperatureControllerSettings( + num_ramp_controllers=4, + ip_settings=IPConnectionSettings(ip="localhost", port=25565), + ) + controller = TemperatureController(settings) + controller.post_initialise() + return controller + + +@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] + for index, ramp in controller.ramps.items(): + assert isinstance(ramp, TemperatureRampController) + 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 = {} + for index, ramp in controller.ramps.items(): + puts[index] = AsyncMock() + ramp.enabled.put = puts[index] # type: ignore[method-assign] + + await controller.cancel_all() + + for put in puts.values(): + put.assert_awaited_once_with(OnOffEnum.Off, sync_setpoint=True) + + +@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.get(), np.array([1, 2, 3, 4], dtype=np.int32) + ) + for index, ramp in controller.ramps.items(): + assert ramp.voltage.get() == pytest.approx(float(index))