diff --git a/docs/explanations/decisions/0013-declarative-procedural-split-and-controller-filler.md b/docs/explanations/decisions/0013-declarative-procedural-split-and-controller-filler.md new file mode 100644 index 000000000..1b802a5be --- /dev/null +++ b/docs/explanations/decisions/0013-declarative-procedural-split-and-controller-filler.md @@ -0,0 +1,182 @@ +# 13. Declarative/Procedural Split and ControllerFiller + +Date: 2026-07-20 + +**Related:** [Issue #388](https://github.com/DiamondLightSource/fastcs/issues/388) + +## Status + +Proposed + +## Context + +FastCS currently has two mechanisms for declaring the shape of a `Controller`: + +1. **Class-scope `Attribute` instances** (`ramp_rate = AttrRW(Float(), io_ref=...)` + assigned directly in the class body). `BaseController._bind_attrs` + (`src/fastcs/controllers/base_controller.py`) walks the MRO, finds these, and + `deepcopy`s each one onto the instance so that multiple instances of the same + `Controller` subclass do not share mutable state. +2. **Bare type hints** (`frames: AttrRW[int]`) validated, not created, by + `HintedAttribute` (`src/fastcs/attributes/hinted_attribute.py`) via + `_find_type_hints`/`_validate_type_hints`. The actual `Attribute` must be + constructed and assigned by the developer, normally in an `initialise()` + override that introspects a device. + +ophyd-async has the equivalent split (`Device` class-body hints vs. `__init__` +procedural construction, see `docs/explanations/declarative-vs-procedural.md`), +but only one mechanism for the declarative half: hints always *create* children, +provisioned by a `DeviceConnector`-owned `DeviceFiller` +(`ophyd_async/core/_device_filler.py`) either immediately or later via +connect-time introspection. There is no ophyd-async equivalent of FastCS's +class-scope instance style, and there cannot be — a `Signal`'s backend depends on +which `DeviceConnector` the owning `Device` is constructed with, so the backend +cannot be known until connect/construction time chooses the connector. + +Our own downstream drivers show why the FastCS class-scope-instance mechanism is +already the minority case in practice, not the norm: + +- `fastcs-eiger` mixes class-body instances (`trigger_exposure = AttrRW(Float())`) + with bare hints filled by REST-API introspection in `initialise()` + (`eiger_detector_controller.py`) — i.e. it already wants one unified mechanism. +- `fastcs-secop`, `fastcs-PandABlocks`, and `fastcs-catio`'s dynamic path build + **all** of their attributes from wire/YAML-derived data at `initialise()` time; + none of them use class-scope instances at all. + +The `deepcopy` half of `_bind_attrs` exists solely to make class-scope instances +safe to reuse across `Controller` instances. It is fragile (IO objects, bound +callbacks, and connections do not always survive a deepcopy cleanly) and costs +construction time on every instantiation, for a feature none of our real-world +introspecting drivers use. + +## Decision + +Adopt a single declarative mechanism, matching ophyd-async: **class body = +declarations + decorated behaviour; instance scope = construction with data.** +Concretely: + +- Remove class-scope `Attribute` **instances** entirely. `AttrRW(getter=..., + setter=...)` may no longer be assigned directly in a class body. +- Remove the deepcopy half of `_bind_attrs`. Method binding for `@command`/ + `@scan` — and the new `@attr`/`@x.setter` sugar + ([ADR 18](0018-attr-decorator-sugar.md)) — is unaffected and stays, since it + does not require deepcopy: it binds a method to `self` at construction time + via the `UnboundCommand`/`UnboundScan` machinery rather than deepcopying a + prototype. +- Remove `HintedAttribute` and `_validate_type_hints`/`_validate_hinted_*` as + a *separate* validation-only pass. Their job — "this hinted child must + exist with the right type after initialisation" — is subsumed into the new + `ControllerFiller`. +- Introduce `ControllerFiller`, a direct structural port of ophyd-async's + `DeviceFiller`. It scans class-body type hints (`AttrR/W/RW[T]`, + `Command[P, T]` — see [ADR 15](0015-typed-commands.md) — and nested + `Controller` / `ControllerVector[T]`), creates children **unfilled**, and + tracks filled/unfilled state per child. `check_filled(source)` raises, + listing by name, anything a `Controller`'s `initialise()` promised via a hint + but did not provision. +- `ControllerFiller` yields `(child, extras)` for each created child — the + `extras` being anything else found in an `Annotated[...]` hint — so that + protocol libraries (a future SCPI package, for example) can define their + own extras vocabulary the same way ophyd-async's `PvSuffix`/`TangoPolling` + do. Core FastCS defines **no** extras vocabulary for 1.0 (decision 3 of + #388). +- When a child is filled from an `Annotated[AttrRW[T], extras]` hint, the filler + **runtime-validates** the metadata the extras carries (a `FloatMeta`, or a + protocol object's `.meta` such as `SCPIParam(...).meta`) against the datatype + `T` — e.g. `precision` supplied for a `str` raises. This is the runtime + counterpart to the static `Unpack[FloatMeta]` check on the procedural `Attr*` + constructors (see [ADR 14](0014-attribute-io-rw-rework.md)). + +Two patterns follow, and the class body distinguishes them. + +**Procedural, no hint** — the value is fully constructed in `__init__`, so it +needs no class-body declaration at all. Per-attribute IO is a `getter`/`setter` +pair of callables ([ADR 14](0014-attribute-io-rw-rework.md)); the datatype is +inferred from the getter's return annotation: + +```python +class TemperatureRampController(Controller): + def __init__(self, index: int, conn: IPConnection) -> None: + super().__init__() + suffix = f"{index:02d}" + + async def get_start() -> int: + return int(await conn.send_query(f"S{suffix}?\r\n")) + + async def set_start(value: int) -> None: + await conn.send_command(f"S{suffix}={value}\r\n") + + # datatype int is inferred from get_start's return annotation + self.start = AttrRW(getter=get_start, setter=set_start, poll_period=0.2) +``` + +**Declarative hint + filler** — the value is *promised* by a hint; the +`ControllerFiller` (run from `Controller.__init__`) creates it as an +**unfilled** `Attribute` so it **exists as soon as `__init__` returns**, and +`initialise()` later *fills* it (provisions the getter/setter + metadata) by +introspection: + +```python +class OdinDetector(Controller): + frames: AttrRW[int] # created UNFILLED by the filler in __init__; + # self.frames EXISTS after __init__, before initialise() + + async def initialise(self) -> None: + # introspection FILLS the already-created hinted attrs (getter/setter + + # metadata), and may add wholly-undeclared dynamic attrs (no hint) + for name, spec in await self._query_parameter_tree(): + self.filler.fill_attribute(name, spec) # validates meta vs datatype + self.filler.check_filled() +``` + +**The rule** (identical to ophyd-async's rule for `Signal`s): *at the end of +`__init__`, any Attribute referenced in code — and therefore carrying a type +hint — must exist* (the filler guarantees this for hinted children by creating +them unfilled during `__init__`). Only `__init__` is serial; `initialise()` +may then run in parallel across controllers. + +Introspecting controllers keep working as today's `initialise()` + +`add_attribute` pattern. The fully-dynamic case — where the *set* of +attributes is not known until a network round-trip completes +(`fastcs-PandABlocks`, `fastcs-secop`) — needs `ControllerFiller` to fill +children that were never hinted at all. This is **no harder than ophyd-async +already supports**: `Device(connector=PviConnector(prefix))` fills a whole +`Signal` tree from introspection with no hints required, and `ControllerFiller` +mirrors that `DeviceFiller` path directly. + +## Consequences + +- Every existing driver using class-scope `Attribute` instances needs + migration to bare hints + `__init__`/`initialise()` construction — see the + Example 1 (`DRAFT: Example 1 — IORef temperature controller`) and Example + 2 (`DRAFT: Example 2 — introspectable Eiger-style controller`) sub-issues + of #388, and the corresponding downstream repo work. +- `Controller.__init__` no longer needs to run `_bind_attrs`, simplifying + construction and removing a source of deepcopy-related bugs. +- Static typing improves: a bare hint `frames: AttrRW[int]` is exactly the + type a type checker sees, with no deepcopy step that could plausibly + change it. +- `ControllerFiller` becomes a new stable, documented surface — see + [ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md) for how + it interacts with the stable `ControllerAPI` surface consumed by the + embedded ophyd-async connector ([ADR 19](0019-embedded-ophyd-async-connector.md)). + +## Questions resolved in review (#402) + +1. **Must `ControllerFiller` support "no hints at all"?** Yes — it must build + the whole attribute tree from introspected data with nothing declared in the + class body (`fastcs-PandABlocks`, `fastcs-secop`), exactly as + `DeviceFiller` does for a `PviConnector`. +2. **Is `fastcs-catio`'s runtime `type(...)` class-building supported?** No. A + bare `Controller` instead allows attributes to be added onto it from the + outside — which is exactly what the fillers do — so catio moves to + instance-level dynamic attribute construction. +3. **Is there a sibling-ordering mechanism?** No. The rule "any hint-referenced + Attribute must exist by the end of `__init__`" makes `initialise()` + parallelisable; sibling dependencies are an `initialise()` implementation + detail (call `super().initialise()` first). +4. **Are `Optional[X]` hints supported?** Yes — `check_filled` treats an + optional hint as not-required. +5. **Do we follow `DeviceFiller`'s names?** Follow its *structure*, not its + names. Architectural similarity matters; method names match only where + FastCS's vocabulary (`Attribute` vs `Signal`) makes them fit. diff --git a/docs/explanations/decisions/0014-attribute-io-rw-rework.md b/docs/explanations/decisions/0014-attribute-io-rw-rework.md new file mode 100644 index 000000000..0b939687d --- /dev/null +++ b/docs/explanations/decisions/0014-attribute-io-rw-rework.md @@ -0,0 +1,320 @@ +# 14. Per-Attribute IO as getter/setter Callables + +Date: 2026-07-20 + +**Related:** [Issue #388](https://github.com/DiamondLightSource/fastcs/issues/388), +[ADR 9](0009-handler-to-attribute-io-pattern.md), [ADR 12](0012-attribute-io-naming-convention.md), +[ADR 18](0018-attr-decorator-sugar.md) + +## Status + +Proposed + +## Context + +[ADR 9](0009-handler-to-attribute-io-pattern.md) split the old `Handler` +pattern into `AttributeIO` (behaviour, one instance per `Controller`, +shared across attributes) and `AttributeIORef` (per-attribute resource +specification, dispatched to the right `AttributeIO` by type at +`_connect_attribute_ios` time). Its sole structural justification was that +class-scope `Attribute` instances are created before `__init__` runs, so +they cannot close over a live connection — the `AttributeIORef` only needed +to carry inert data (a register name, a URI) until the matching +`AttributeIO` was found by type at `post_initialise()`. + +[ADR 13](0013-declarative-procedural-split-and-controller-filler.md) removes +class-scope `Attribute` instances entirely. Every attribute is now +constructed procedurally, in `__init__` or `initialise()`, where a live +connection is already in scope. The situation `AttributeIORef` was invented +to solve no longer exists. + +We also verified downstream that nothing outside FastCS consumes `io_ref` or +the IO registry directly — every consumer bottoms out in +`attr.set_on_put_callback(io.send)` / `attr.set_update_callback(io.update)` +(`base_controller.py:_connect_attribute_ios`), i.e. the ref/registry split is +a dispatch layer over callbacks that already exist as plain methods. + +The dispatch-by-type registry has real costs in our downstream drivers: + +- `fastcs-catio` registers **three** separate `AttributeIO`/`AttributeIORef` + pairs on one `Controller` (`ios=[poll_io, symbol_io, coe_io]`) purely so + each attribute's `io_ref` type can select the right one at + `_connect_attribute_ios` time — indirection that a direct per-attribute + callable removes outright. +- `fastcs-secop` needs a private escape hatch, + `attr._call_sync_setpoint_callbacks`, to push setpoint echoes from its + `send()` implementation, because the current `AttributeIO.send` signature + has no sanctioned way to do this — flagged in-code as pending a public API. +- `fastcs-PandABlocks`'s `UnitsIO.send` mutates a *sibling* attribute's + datatype (`attribute_to_scale.update_datatype(...)`), reaching outside the + attribute it was invoked for — a pattern the new IO shape should not make + harder, even though it stays an edge case. + +## Decision + +Delete `AttributeIO` and `AttributeIORef` and their whole dispatch machinery. +Per-attribute IO is supplied as plain **`getter`/`setter` callables** on the +`Attr*` constructors — which *is* the procedural spelling of the `@attr` +decorator ([ADR 18](0018-attr-decorator-sugar.md)): + +- `AttrR(getter=g)`, `AttrW(setter=s)`, `AttrRW(getter=g, setter=s)`. Access + mode is enforced by **which parameters exist** (an `AttrR` has no `setter`), + so there is no IO class hierarchy and no abstract-method enforcement to + carry — and `getter=` on a read-only attr is honest where `io=` was a false + friend. +- **The getter returns the value; the framework applies it** — + `getter() -> T | Update[T]` — instead of the old imperative `io.update(attr)`. + Imperative / multi-attribute periodic logic stays with `@scan`, which is + *why* per-attribute IO shrinks to "one value in / out". +- The **setter** returns `None | T | Update[T]`: `None` = fire-and-forget + (readback catches up on the next poll / the setpoint cache); a returned value + is the device's *accepted* value (a clamp or echo) and updates the readback + + the `AttrW` setpoint cache immediately — the sanctioned replacement for + `fastcs-secop`'s private `_call_sync_setpoint_callbacks`. +- `Update[T]` carries `value: T`, `timestamp: float | None` (epoch seconds; + `None` ⇒ framework stamps receive-time), and `severity: Severity = OK` (the + decision-10b severity enum, see + [ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md)). It is + used for both the getter return and a value-returning setter — this is how + device-native timestamps/severity reach `attr.update()`. +- **Datatype is optional when a getter/setter is given** — inferred from the + getter's return annotation (or the setter's parameter), unwrapping `Update[T]` + to `T`, so `AttrR(getter=g)` yields `AttrR[float]` with no restated type + (parity with `@attr`). Only the bare python type is optional; + `precision`/`units`/… stay explicit kwargs, and the per-datatype + `Unpack[*Meta]` static check keys off the inferred return type. Not inferable + (`-> Any`, an unannotated lambda) ⇒ the positional datatype is required + (fail-fast at construction). +- `poll_period` is a read-side kwarg: `ONCE` = read once at connect (the + default when a getter is given); a float = poll at that rate; `None` = + **on-demand only** (read when a client asks, never auto-polled). **No + getter** = soft, value pushed via `attr.update()` from a `@scan`/callback. +- Soft is now simply the *absence* of a getter/setter (`AttrRW(float)` + self-wires setpoint→readback as before, the analogue of ophyd-async's + `soft_signal_rw`); the old `io=None` sentinel is gone. +- The declarative/filler path lowers to the **same** getter/setter (a + `SCPIController`'s filler builds the callables from a `SCPIParam`). getter and + setter are where the old `_connect_attribute_ios` wiring now lives, so + transports and the embedded connector are unaffected. + +`attr` is a **decorator only** (`@attr` / `@attr(precision=3)` + +`@voltage.setter`, [ADR 18](0018-attr-decorator-sugar.md)); there is no +free-function `attr()` factory — the procedural spelling is `AttrR`/`AttrRW` +directly. + +```python +class TemperatureRampController(Controller): + def __init__(self, index: int, conn: IPConnection) -> None: + super().__init__() + name = f"R{index:02d}" + + async def get_ramp_rate() -> float: + return float(await conn.send_query(f"{name}?\r\n")) + + async def set_ramp_rate(value: float) -> None: + await conn.send_command(f"{name}={value}\r\n") + + # datatype float inferred from get_ramp_rate's return annotation + self.ramp_rate = AttrRW( + getter=get_ramp_rate, setter=set_ramp_rate, units="deg", poll_period=0.2 + ) +``` + +### Runtime surface + +The old `get()` / `update(value)` / `put(value)` method trio is renamed and +split so that both **access mode** and **whether a call touches the device** +are legible from the member set: + +| Member | Kind | AttrR | AttrW | AttrRW | Device IO? | +|---|---|---|---|---|---| +| `.readback` | property (sync) | ✓ | — | ✓ | no (cached) | +| `.setpoint` | property (sync) | — | ✓ | ✓ | no (cached) | +| `poll()` | async method | ✓ | — | ✓ | **yes** (getter) | +| `update(value)` | async method | ✓ | — | ✓ | no (cache push) | +| `set(value)` | async method | — | ✓ | ✓ | **yes** (setter) | + +- **`.readback` / `.setpoint` replace `.value`.** Two explicitly-named cached + properties instead of one whose meaning shifted per class. Each class exposes + only the ones it has (`AttrR` has no `.setpoint`, `AttrW` no `.readback`), so + access mode reads off the surface — and the pair mirrors bluesky / + ophyd-async's `Location(setpoint, readback)` exactly, so `AttrRW` maps 1:1 + onto `locate()` and the embedded connector's `get_value`/`get_setpoint`. Both + are **read-only** properties: writes are async (validate + `await` callbacks) + and so cannot be property setters. +- **`poll()` replaces the no-arg `update()`; `update_period` → `poll_period`.** + `poll()` does a live getter read, caches it, and **returns** the value (so an + on-demand read is `await attr.poll()`, mirroring ophyd's live `get_value()`); + `poll_period` (`ONCE` / float / `None`) is only the *schedule* the framework + calls it on. This deletes the `set_update_callback` / `bind_update_callback` + plumbing — the getter lives on the attr and `poll()` calls it. +- **`update(value)` is now purely a cache push** — a `value` or `Update[T]` + from a `@scan`/subscription — with no device IO and no `None` sentinel. +- **`set(value)` replaces `put()`** (the bluesky/ophyd verb): it caches + `.setpoint` immediately (decision 10a), then runs the setter; the setter's + `T | Update[T]` return feeds `.readback` via `update()`. The old + `sync_setpoint=` kwarg and `_call_sync_setpoint_callbacks` are gone. Caching + `.setpoint` first is an *attribute-cache* guarantee only; *when a remote + client sees it* is transport-dependent and differs between CA and PVA — see + the Questions resolved below. + +So `poll()`/`set()` touch the device; `.readback`/`.setpoint`/`update()` do +not. `Attribute` also loses its second generic parameter — +`Attribute[DType_T, AttributeIORefT]` collapses to `Attribute[DType_T]`, making +`AttrRW[float]` structurally isomorphic to ophyd-async's `SignalRW[float]`. + +### Datatype metadata: the `*Meta` TypedDicts + +`DataType` classes are gone ([ADR 15](0015-typed-commands.md) / +[ADR 17](0017-naming-pass.md)). The metadata they carried (precision, units, +nested limits, …) moves to a per-datatype `TypedDict` — `FloatMeta`, `IntMeta`, +`StrMeta`, `BoolMeta`, `EnumMeta`, `Array1DMeta`, `TableMeta` — and **the +resolved metadata is stored on the `Attribute` itself** (`attr.meta`), not on a +separate datatype object. Every transport/connector that read +`attr.datatype.precision`/`.units`/`.limits`/`.choices` now reads `attr.meta` +(enum `choices` come from the python type; `EnumMeta` is display-only). + +Two spellings, two validation layers: + +- **Procedural (statically checked):** the `Attr*` constructors are overloaded + per datatype so the right `*Meta` is unpacked into `**kwargs`: + + ```python + # conceptually, one overload per datatype: + def AttrRW(dtype: type[float], *, getter=..., setter=..., + **kwargs: Unpack[FloatMeta]) -> AttrRW[float]: ... + + self.temperature = AttrRW(float, precision=3, units="deg", setter=apply_temp) + # AttrRW(str, precision=3) is a static type error + ``` + + (The `dtype` positional is only needed when it cannot be inferred from a + getter/setter annotation, as above.) + +- **Declarative (runtime-checked by the filler):** + `Annotated[AttrRW[float], FloatMeta(precision=3)]` (rare) or + `Annotated[AttrRW[float], SCPIParam("P", precision=3)]` (common). Neither + ties the metadata to the `AttrRW[...]` type param statically, so + [ADR 13](0013-declarative-procedural-split-and-controller-filler.md)'s + `ControllerFiller` validates it against the datatype at fill time. A generic + extras object takes the **superset** `Meta` TypedDict — + `SCPIParam(param: str, **kwargs: Unpack[Meta])` (`Meta` being the union of + `FloatMeta`/`StrMeta`/… fields, all optional) — stores a `.meta`, and the + filler passes that `.meta` into the constructed `AttrRW`. + +**One spec object per declaratively-filled attribute.** A protocol extra like +`SCPIParam` is the *single place* an attribute's whole specification is +written: both the protocol binding (the command token, `"P"`) and all its +generic metadata (`description`, `precision`, `units`, limits…) via +`**Unpack[Meta]`. The filler treats that extra as the **exclusive** spec +source for its attribute — it does **not** also merge a separate +`FloatMeta`/`Meta` extra sitting on the same `Annotated[...]` hint, so there +is no precedence question to resolve. The trade this accepts is deliberate: +routing metadata through the superset `Meta` (not a per-datatype +`Unpack[FloatMeta]`) means correctness is the filler's **runtime** job, not a +static check — a separate `Annotated` extra cannot tie its `**Meta` to the +`AttrRW[...]` datatype param, and making the extra generic +(`SCPIParam[float](...)`) only forces the user to restate a type already in +the hint. So the declarative path pays for its ergonomics with runtime +validation; the filler's error must name the attribute and field (e.g. +"`precision` is not valid for `str` attribute `device_id`"). + +Naming: the extra is `SCPIParam` (a binding object you *instantiate* as an +`Annotated` extra), **not** `SCPIMeta` — the `*Meta` suffix is reserved for +the metadata TypedDicts you `Unpack` (`FloatMeta`, `Meta`), a different kind +of Python object. `SCPIParam` is a sibling of ophyd-async's +`PvSuffix`/`TangoPolling`, not of `SignalMetadata`. It is **not** part of core +FastCS (decision 3: core defines no extras vocabulary for 1.0) — it lives in a +protocol layer; the demo package ships an example `SCPIController` + +`SCPIParam` to show how a third party builds one on the filler's +`(child, extras)` mechanism. The `*Meta` module location is deferred to the +public-API-namespace decision (#406); land it provisionally until then. + +### Migration + +Migration collapses an `AttributeIO`/`AttributeIORef` pair into two callables: +the old `update`/`send` method bodies become the `getter`/`setter`, constructed +once per attribute instead of once per controller. + +```python +# Before (ADR 9 shape) +class TempIORef(AttributeIORef): + name: str + +class TempIO(AttributeIO[float, TempIORef]): + async def update(self, attr: AttrR[float, TempIORef]) -> None: + resp = await self._conn.send_query(f"{attr.io_ref.name}?\r\n") + await attr.update(float(resp)) + + async def send(self, attr: AttrW[float, TempIORef], value: float) -> None: + await self._conn.send_command(f"{attr.io_ref.name}={value}\r\n") + +ramp_rate = AttrRW(Float(), io_ref=TempIORef(name="R")) +# ... elsewhere: Controller(ios=[TempIO(conn)]) + +# After +def temp_io(conn: IPConnection, name: str): + async def getter() -> float: + return float(await conn.send_query(f"{name}?\r\n")) + + async def setter(value: float) -> None: + await conn.send_command(f"{name}={value}\r\n") + + return getter, setter + +get_ramp, set_ramp = temp_io(conn, "R") +self.ramp_rate = AttrRW(getter=get_ramp, setter=set_ramp, poll_period=0.2) +``` + +`fastcs-catio`'s three-IO-per-controller pattern becomes per-attribute +callables with no registry needed at all. `fastcs-secop`'s private +`_call_sync_setpoint_callbacks` call is replaced by a value-returning setter. + +## Consequences + +- Every driver that declared `AttributeIO`/`AttributeIORef` subclasses migrates + their `update`/`send` bodies into `getter`/`setter` callables — see the + affected §9 files in the sub-issues of #388 (`attributes/`, + `controllers/base_controller.py`, `controllers/controller.py`) and the + corresponding downstream repo issues. The migration is mechanical. +- `Attribute` loses its second generic parameter, simplifying every type + hint in downstream code (`AttrR[float, MyRef]` → `AttrR[float]`). +- Access-mode compatibility is enforced by the parameter set — an `AttrR` has + no `setter`, so there is no `_validate_io` runtime check and no way to attach + a read-only IO to a write-capable attr statically. For the dynamically-built + `Any`-typed case (`fastcs-secop`, `fastcs-PandABlocks`) a runtime check at + `post_initialise` still catches a missing setter on a write-capable attr. +- The IO no longer has a place to hang per-attribute metadata that + `fastcs-catio` used to read off `attribute.io_ref`; `attr.meta` and the + attribute's own attributes replace that access. + +## Questions resolved in review (#402) + +1. **What replaces the `io=` object and the `ReadIO`/`WriteIO`/`ReadWriteIO` + hierarchy?** Plain `getter`/`setter` callables on the constructors. The IO + class hierarchy and its abstract-method enforcement are dropped entirely; + access mode is enforced by which parameters exist. +2. **Do we still need a runtime access-mode check?** Yes, in addition to the + static shape: a runtime check (e.g. at `post_initialise`) catches a missing + setter on a write-capable `Attr` for the dynamically-built `Any`-typed case. +3. **What is the public replacement for `fastcs-secop`'s + `_call_sync_setpoint_callbacks`?** A `setter` returning `T | Update[T]` *is* + the sanctioned setpoint echo — the returned value updates the readback and + the `AttrW` setpoint cache. +4. **Are there `CallbackReadIO`/`CallbackWriteIO` classes in core?** No. The + one-off callback case folds into `@attr` / `AttrR(getter=…)` + ([ADR 18](0018-attr-decorator-sugar.md)); the same spelling covers the + read-only and read/write cases. +5. **How is per-attribute IO metadata recovered from outside `getter`/`setter`?** + Through `attr.meta` and the attribute's own public members, replacing + `fastcs-catio`'s `attribute.io_ref` access. +6. **Is the setpoint echo a cross-transport "instantly visible" guarantee?** + (@Tom-Willemsen / @shihab-dls.) No — caching `.setpoint` before running the + setter is an *attribute-cache* guarantee (and the sanctioned secop echo); + whether a *remote client* sees it immediately is transport-dependent. **PVA** + posts the setpoint as soon as it is written, then the record may later go + into alarm if the setter rejects it. **CA** posts the PV update only *after* + the update callback (where alarms are set) completes, so a long-running + setter delays the CA-visible setpoint until the send returns. Realigning CA + to PVA's post-before-send ordering is a **transport-layer** follow-up, + tracked separately and not gating this rework. diff --git a/docs/explanations/decisions/0015-typed-commands.md b/docs/explanations/decisions/0015-typed-commands.md new file mode 100644 index 000000000..e59f9017d --- /dev/null +++ b/docs/explanations/decisions/0015-typed-commands.md @@ -0,0 +1,132 @@ +# 15. Typed Commands + +Date: 2026-07-20 + +**Related:** [Issue #388](https://github.com/DiamondLightSource/fastcs/issues/388) + +## Status + +Proposed + +## Context + +FastCS `Command` (`src/fastcs/methods/command.py`) is void/void only: +`Method._validate` requires zero parameters and `None`/empty return type. +`UnboundCommand.bind` produces a `Command` wrapping a zero-arg, no-return +async callable. `Method.__init__` already captures the full +`inspect.Signature` of the wrapped function (`method.py:21`), but `Command`'s +own `_validate` throws that signature away by rejecting anything with +parameters. + +ophyd-async's equivalent, `Command[P, T]` (`ophyd_async/core/_command.py`), +carries a real parameter and return type, exposed via `CommandBackend.signature` +and `CommandBackend.execute(*args, **kwargs) -> T`. `TriggerableCommand = +Command[[], None]` is the void/void case, expressed as a special case of the +general one rather than the only case. + +This gap already shows in our downstream drivers rather than being +speculative: `fastcs-secop` builds command arguments and results dynamically +from SECoP's wire `datainfo` (`_controllers.py:102-110`) — a genuinely typed +(if dynamically-typed) command surface that FastCS's void/void `Command` +cannot represent today, forcing `fastcs-secop` to route command arguments +through attributes on a dedicated `SecopCommandController` instead of a +single typed call. + +## Decision + +Lift the zero-arg/no-return restriction in `Method`/`Command._validate`, and +introduce `Command[P, T]` generic over parameters and return type, keeping +the already-captured `inspect.Signature` as the public surface — +`ControllerAPI` exposes it directly, mirroring `CommandBackend.signature`. + +Transport capability is declared, not assumed uniform: + +- **Tango, REST, GraphQL, and the embedded ophyd-async connector** serve + typed commands fully — arguments and return value round-trip through + each protocol's native typed-call mechanism. +- **EPICS CA/PVA** stay void/void at the wire level (there is no PV + representation of "call with these typed arguments, get this typed + return" that doesn't already exist as separate attributes). They **skip + typed commands with a warning** at start-up rather than failing to serve + the controller at all. A command the user explicitly declares as typed + *and* forces to be served over an EPICS-only transport is a hard error — + matching the existing ophyd-async connector's behaviour, which errors + rather than silently drops when a `Device` requires a capability its + connector cannot provide. + +```python +class Ramp(Controller): + move_to: Command[[float], None] # typed: not served over CA/PVA + stop: Command[[], None] # void/void: served everywhere +``` + +**Argument and return typing are independent** (not all-or-nothing): + +- *Args*: `[]` (none) · `[DT1, DT2, …]` (positional, known types — validated). +- *Returns*: `None` · `DT` (a single typed value). + +There is **no partial `Command[Any, Any]`** — a statically-declared `Command` +always has its parameter and return types fully known. The alternative is not +a half-known command but a fully-dynamic controller: a driver that knows +*nothing* statically (`fastcs-secop`, discovering everything from an +over-the-wire `describe`) does not annotate a `Command` at all — it builds the +whole structure, attributes and commands alike, at runtime. So `P`/`T` are +either **completely known** (static declaration) or the **whole structure is +unknown** (runtime construction); there is no in-between case where you know +something is a command but not its signature. This is the same "hint vs. +no-hint" split as [ADR 13](0013-declarative-procedural-split-and-controller-filler.md) +for attributes — with no partial hint. + +Following the [ADR 17](0017-naming-pass.md) `DataType` drop, command +arguments/returns use plain python types + `*Meta` exactly as attributes do +(no metadata ⇒ "use the python type"), and the serialisation machinery is +**shared** with `Attribute`, not duplicated. **Keyword-argument** commands +need a `TYPE_CHECKING` stub trick and are prototyped separately in the spike +[#403](https://github.com/DiamondLightSource/fastcs/issues/403), not here. + +## Consequences + +- `Command.__call__` gains real `*args`/`**kwargs` forwarding instead of a + bare `await self.fn()`; `UnboundCommand.bind` needs the same treatment. +- `ControllerAPI` (or its per-command entries) needs to expose the + signature to transports, so each transport can decide serve-fully / + serve-with-warning / hard-error per decision above. +- EPICS transports (`transports/epics/ca`, `transports/epics/pva`) need a + capability check at controller-API-build time, producing a startup-time + warning log rather than a runtime failure per typed-command call. +- `fastcs-secop` (and any introspection-driven driver) does **not** get a + `Command[Any, Any]`. Since SECoP's `describe` reveals the entire structure + only at connect time — there is no static declaration of *anything*, let + alone a command of unknown signature — such drivers build their commands + programmatically at runtime, each carrying a concrete signature derived from + the wire `datainfo`. Static `Command[P, T]` is for statically-declared + controllers; fully-dynamic drivers construct commands (or keep the existing + `SecopCommandController` explode-to-PVs workaround) at runtime instead. +- Command args/return values validate through the **same** python-type + + `*Meta` mechanism as attributes (the `DataType` family is removed, ADR 17) — + one shared validation/serialisation path, no command-specific duplicate. + +## Questions resolved in review (#402) + +1. **Do commands need their own type/serialisation mechanism?** No — they share + the attribute path. With `DataType` dropped ([ADR 17](0017-naming-pass.md)), + command args/returns use python types + `*Meta` like attributes, and + complex-type serialisation (arrays, `Enum`, `Table`) is shared with + `Attribute`. +2. **Are args and returns typed all-or-nothing?** No — independently: args `[]` + / `[DT…]`; returns `None` / `DT` (see Decision). Each is fully known — there + is no `Any` middle case. +3. **Is there a partial `Command[Any, Any]`?** No. @Tom-Willemsen confirmed on + [#402](https://github.com/DiamondLightSource/fastcs/pull/402#discussion_r3621453680) + that SECoP devices are discovered entirely from an over-the-wire `describe`: + you never statically know something is a command but not its signature — you + either know the full `Command[P, T]` or you know nothing at all and build the + whole controller at runtime. `P`/`T` are therefore completely known or the + whole structure is unknown, with no in-between. +4. **When does the EPICS skip-with-warning fire?** At IOC startup — post + controller construction, when the fully populated controllers are handed to + the transports to serve. +5. **What about keyword-argument commands?** Deferred to spike + [#403](https://github.com/DiamondLightSource/fastcs/issues/403) + (interactive/Opus; needs a `TYPE_CHECKING` stub). Out of scope for core + typed-command work. diff --git a/docs/explanations/decisions/0016-setpoint-cache-timestamps-and-controller-runner.md b/docs/explanations/decisions/0016-setpoint-cache-timestamps-and-controller-runner.md new file mode 100644 index 000000000..4f67b0528 --- /dev/null +++ b/docs/explanations/decisions/0016-setpoint-cache-timestamps-and-controller-runner.md @@ -0,0 +1,120 @@ +# 16. AttrW Setpoint Cache, Native Timestamps, and ControllerRunner + +Date: 2026-07-20 + +**Related:** [Issue #388](https://github.com/DiamondLightSource/fastcs/issues/388), +[ADR 14](0014-attribute-io-rw-rework.md) + +## Status + +Proposed + +## Context + +Three related gaps block a clean embedded ophyd-async connector +(see [ADR 19](0019-embedded-ophyd-async-connector.md)) and are useful to all +transports independently of embedding: + +1. **No cached setpoint.** The old `AttrW.put` (`src/fastcs/attributes/attr_w.py`) + applied a setpoint via `_on_put_callback` but did not retain it anywhere + queryable. ophyd-async's `SignalBackend.get_setpoint()` — needed for + `locate()` — has no FastCS equivalent to read from. +2. **No FastCS-native timestamps.** The old `AttrR.update` (`attr_r.py`) stamped + nothing; individual transports each did their own thing (EPICS records + get a timestamp from the record subsystem, Tango pushes are unstamped). + An embedded connector currently has no choice but to stamp receive-time + only, which is a real information loss versus what the underlying device + protocol may already provide (Tango event timestamps, EPICS record + timestamps at source). +3. **No documented, extracted runtime.** `FastCS.serve` (`control_system.py`) + inlines the full controller lifecycle — `initialise()` → + `post_initialise()` → `create_api_and_tasks()` → `connect()` → initial + coroutines → scan tasks — as private logic inside the `serve` coroutine. + There is no standalone object an embedding connector can start/stop + without also pulling in `FastCS`'s transport-serving and interactive-shell + concerns. + +Decision 13 of #388 requires this lifecycle, plus `ControllerAPI` and the +attribute/command runtime methods, to be formalised as fastcs-core's single +documented "stable interface" that the ophyd-async connector is restricted +to using — no reaching into `BaseController` internals. + +## Decision + +**Setpoint cache.** `AttrW`/`AttrRW` retain the last-applied setpoint, exposed +via the sync `.setpoint` property from [ADR 14](0014-attribute-io-rw-rework.md)'s +runtime surface (the FastCS analogue of ophyd-async's +`SignalBackend.get_setpoint()`). `set(value)` caches it immediately — before +the setter runs and independent of whether the setter succeeds — so `.setpoint` +is a "what did we last ask for" query, distinct from `.readback` ("what did we +last read back"). This is available to all transports, not just the embedded +connector. + +**Native timestamps (+ severity).** A value entering an `AttrR`/`AttrRW` may +carry a timestamp and severity by arriving as an `Update[T]` +([ADR 14](0014-attribute-io-rw-rework.md)) — from a getter's return, a +value-returning setter, or a `@scan`/subscription `update()` push — defaulting +to framework receive-time when the timestamp is `None`. The timestamp/severity +pair **follows bluesky's `Reading` shape but shares no code** with it, and +severity is a **FastCS enum using the same strings as EPICS** alarm severities. +This is FastCS-native, not EPICS-specific — Tango event pushes and other IO can +supply a device-side timestamp through the same `Update[T]` path a getter +already uses. The embedded connector stamps receive-time only as an interim +measure until this lands, per decision 10 of #388 — this is 1.0 scope, not a +follow-up. + +**ControllerRunner.** Extract the controller lifecycle currently inlined in +`FastCS.serve` into a standalone `ControllerRunner` (or equivalent +`Controller.serve()`/`Controller.stop()` API), independent of the +transport-serving and interactive-shell logic that stays in `FastCS`/ +`control_system.py`. `FastCS.serve` becomes a thin caller of +`ControllerRunner` plus transport wiring. The runner owns: + +- Running `initialise()`/`post_initialise()`/`create_api_and_tasks()` once. +- Running `connect()` and the initial coroutines. +- Starting/stopping the periodic scan tasks. +- The **whole lifecycle including reconnect** — calling `Controller.reconnect()` + on scan-task failure; reconnect is owned by the runner, not left + controller-specific. + +The runner is a class with `start()`/`stop()` (ophyd-async calls `start`/`stop`; +an `async with` context manager is added only if it also suits the `FastCS()` +case). **Idempotency is the caller's responsibility**, not the runner's — the +embedded connector's `connect_real` may run more than once across reconnects +(see [ADR 19](0019-embedded-ophyd-async-connector.md)). + +This, together with `ControllerAPI` and the attribute/command runtime surface +from [ADR 14](0014-attribute-io-rw-rework.md) (`.readback`/`poll()` + +update-callback registration, `set()` + the `.setpoint` cache, +`attr.meta`/`access_mode`/`description`/`group`), becomes the documented stable +surface referenced by decision 13 of #388. + +## Consequences + +- `FastCS.serve` shrinks to transport orchestration; the controller + lifecycle it currently inlines becomes independently testable and + reusable without instantiating a `FastCS` object or any `Transport`. +- Every getter/setter *may* return an `Update[T]` to supply a + timestamp/severity, but a bare value is unaffected — it defaults to + framework receive-time, severity unset. +- Transports gain access to a real setpoint distinct from the readback + value; whether EPICS/Tango/REST/GraphQL surface this as new fields is + transport-specific follow-up work, not part of this ADR. +- The embedded ophyd-async connector becomes buildable against a documented, + narrow surface instead of `BaseController` internals — see + [ADR 19](0019-embedded-ophyd-async-connector.md). + +## Questions resolved in review (#402) + +1. **How is the cached setpoint exposed?** Via the `.setpoint` property from + [ADR 14](0014-attribute-io-rw-rework.md)'s runtime surface (the FastCS + analogue of `SignalBackend.get_setpoint()`), cached by `set()` before the + setter runs. +2. **What shape do timestamp/severity take?** They follow bluesky's `Reading` + shape but **share no code**; severity is a **FastCS enum using the same + strings as EPICS**, carried on `Update[T]`. +3. **What is the runner's shape?** A class with `start()`/`stop()` (context + manager only if it also suits `FastCS()`); **idempotency is the caller's + responsibility**. +4. **Who owns reconnect?** The runner owns the whole lifecycle, including + reconnect. diff --git a/docs/explanations/decisions/0017-naming-pass.md b/docs/explanations/decisions/0017-naming-pass.md new file mode 100644 index 000000000..85af66a07 --- /dev/null +++ b/docs/explanations/decisions/0017-naming-pass.md @@ -0,0 +1,109 @@ +# 17. Naming Pass: precision, Limits Alignment, Array1D/Table Hints + +Date: 2026-07-20 + +**Related:** [Issue #388](https://github.com/DiamondLightSource/fastcs/issues/388), +[ADR 14](0014-attribute-io-rw-rework.md), [ADR 15](0015-typed-commands.md) + +## Status + +Proposed + +## Context + +FastCS and ophyd-async independently arrived at similar concepts with +different names, which is exactly the "false friend" risk #388 opens with: +a developer moving between the two projects can be misled into assuming a +name means the same thing, or reach for a name that does not exist. + +Concretely, `_Numeric` (`src/fastcs/datatypes/_numeric.py`) and `Float` +(`src/fastcs/datatypes/float.py`) use `prec`/`min`/`max`/`min_alarm`/ +`max_alarm`; ophyd-async and the wider bluesky event-model use +`precision` and a `Limits` structure (`Limits(low, high)` per category, e.g. +control/display/alarm/warning) rather than five flat fields. FastCS's +`Waveform(array_dtype, shape)` and `Table` datatypes have no hint-level +spelling analogous to ophyd-async's `Array1D[np.int32]` (a `numpy.ndarray` +subscripted for shape) and `Table` (pydantic-based) hint syntax — under +[ADR 13](0013-declarative-procedural-split-and-controller-filler.md), bare +hints are now load-bearing (they are what `ControllerFiller` scans), so +having ophyd-async-compatible hint spellings for array/table attributes +becomes more valuable than it was when hints were validation-only. + +This pass now lives on python types + `*Meta` typed dicts, not on `DataType` +classes: the `DataType` family is dropped ([ADR 14](0014-attribute-io-rw-rework.md), +[ADR 15](0015-typed-commands.md)), so these renames fold into the per-attribute +IO rework (issue #392) rather than landing as a separate late PR. The concrete +`*Meta` mechanism (per-datatype `TypedDict`s, the superset `Meta` for extras, +`attr.meta` storage, the `Unpack` overloads) is specified in +[ADR 14](0014-attribute-io-rw-rework.md); the module home for these public +names is decided in #406. + +Since this is a pre-1.0 breaking-change window (per #388's framing — +"while breaking pre-1.0"), this is the point to make these renames, not +after 1.0 when they become a deprecation cycle. + +## Decision + +1. **`prec` → `precision`.** Rename across the numeric metadata (`FloatMeta`, + transports, docs, snippets) wherever `prec` appears. `precision` stays an + `int` (decimal places). No behaviour change. + +2. **Limits alignment — nested, not flat.** Replace the flat + `min`/`max`/`min_alarm`/`max_alarm` fields with a nested `Limits` structure + aligned to the bluesky event-model, so alarm/control/display limits read the + same way in FastCS and ophyd-async docs. **All four categories** — control, + display, alarm, warning — are present and **all optional**, with inheritance: + + - supply none ⇒ all unbounded; + - Display but not Control ⇒ Control inherits Display (for a writeable attr); + - Alarm but not Warning ⇒ Warning inherits Alarm; + - both Alarm and Warning ⇒ assert Warning ⊆ Alarm; + - otherwise unspecified ⇒ unbounded. + +3. **`Array1D`/`Table` hint spellings, which are also the runtime structure.** + Adopt `Array1D[np.int32]` and `Table` as the FastCS *hint* spellings a + `ControllerFiller`-scanned class body uses. With `DataType` dropped, these + are **both** the hint and the runtime structure passed around as the + datatype — there is no separate `Waveform`/table `DataType` object to map to. + Procedural construction passes the same types plus `*Meta` (e.g. + `AttrRW(Array1D[np.int32], shape=(4,), getter=...)`), and shape/array + metadata rides on `Array1DMeta` exactly as `precision`/`units` ride on + `FloatMeta`. + +This is explicitly the smallest naming-pass scope agreed in #388 for 1.0. A +`Prec`/`Units`/`Shape` `Annotated` extras vocabulary (letting a hint carry +precision/units/shape without a spec object) is called out in #388 as a +**post-1.0** option enabled by, but not required by, the +[ADR 13](0013-declarative-procedural-split-and-controller-filler.md) extras +mechanism — not part of this ADR. + +## Consequences + +- Every driver using `Float(prec=...)`, `.min`/`.max`/`.min_alarm`/ + `.max_alarm` needs a rename to `precision` and the nested `Limits` + structure. This is a wide diff across all downstream repos (`fastcs-eiger`, + `fastcs-catio`, `fastcs-secop`, `fastcs-PandABlocks` all use numeric + limits somewhere); the flat→nested Limits change is structural, not purely a + rename. +- Transports serving `precision`/limits metadata (EPICS record fields, + Tango attribute properties, REST/GraphQL schema) read these from `attr.meta` + ([ADR 14](0014-attribute-io-rw-rework.md)) and need their field-name mapping + updated to the renamed / nested fields. +- `Array1D`/`Table` become the single array/table representation for both + hinted and procedural attributes, so there is no hint-vs-runtime mapping + layer to keep in sync. + +## Questions resolved in review (#402) + +1. **Flat or nested limits?** Nested — a `Limits` structure, not four flat + fields. +2. **Which limit categories, and how do they combine?** All four + (control/display/alarm/warning), all optional, with the inheritance rules in + Decision point 2 (Control inherits Display, Warning inherits Alarm, assert + Warning ⊆ Alarm, otherwise unbounded). +3. **Is `precision` an int or a float?** An `int` (decimal places). +4. **Do `Array1D`/`Table` map onto a separate runtime `DataType`?** No — with + `DataType` dropped they *are* both the hint and the runtime structure; there + is no `Waveform` object to map to. +5. **Where does this land?** It folds naturally into the per-attribute IO / + `DataType`-drop PR (#392) — the implementer's choice, not a separate late PR. diff --git a/docs/explanations/decisions/0018-attr-decorator-sugar.md b/docs/explanations/decisions/0018-attr-decorator-sugar.md new file mode 100644 index 000000000..923364a25 --- /dev/null +++ b/docs/explanations/decisions/0018-attr-decorator-sugar.md @@ -0,0 +1,142 @@ +# 18. Attr-from-Method Decorator Sugar (`@attr` + `@x.setter`) + +Date: 2026-07-20 + +**Related:** [Issue #388](https://github.com/DiamondLightSource/fastcs/issues/388), +[ADR 13](0013-declarative-procedural-split-and-controller-filler.md), +[ADR 14](0014-attribute-io-rw-rework.md) + +## Status + +Proposed + +## Context + +#388 §7.5 makes the case that FastCS must also sell as a way to write Tango +Device Servers, competing directly with PyTango on the trivial case, not +just on the advanced multi-transport pitch. PyTango's hello-world is one +decorated getter: + +```python +@attribute +def current(self) -> float: + return 2.5 +``` + +Under the [ADR 13](0013-declarative-procedural-split-and-controller-filler.md) +harsh declarative/procedural split (bare hints only in the class body; all +IO wiring procedural), the equivalent trivial case would regress to a method +plus explicit `AttrR(getter=...)` wiring in `__init__` — strictly more +ceremony than PyTango for the simple case that most new users hit first. This +is exactly the kind of "false friend" gap #388 warns about: a PyTango user +evaluating FastCS should not find the *simple* case harder than what they're +moving away from. + +FastCS already has precedent for binding class-body decorated methods to +per-instance callables without any deepcopy hazard: `@command`/`@scan` +(`src/fastcs/methods/command.py`, `scan.py`) use `UnboundCommand`/ +`UnboundScan`, which wrap an unbound function and `.bind(controller)` a +fresh `Command`/`Scan` object per instance at construction time. Because +these are fresh objects constructed per-instance (not deepcopied +prototypes), they carry none of the aliasing hazard that class-scope +`Attribute` *instances* had — which is why +[ADR 13](0013-declarative-procedural-split-and-controller-filler.md) +removes the latter but keeps `@command`/`@scan`. + +## Decision + +Add `@attr` as pure sugar over `AttrR`/`AttrRW` plus generated getter/setter +callables ([ADR 14](0014-attribute-io-rw-rework.md)), built on the same +`Unbound*`-style bind machinery as `@command`/`@scan` — fresh objects per +instance, no prototype/deepcopy hazard, consistent with keeping this a +class-body citizen under +[ADR 13](0013-declarative-procedural-split-and-controller-filler.md). The +decorator mirrors `@property`: `@attr` on the getter, `@voltage.setter` on the +writer. + +```python +class PowerSupply(Controller): + @attr(units="V", poll_period=0.5) # datatype inferred from -> float + async def voltage(self) -> float: + """Output voltage.""" + return await self._conn.query("V?") + + @voltage.setter + async def voltage(self, value: float) -> None: + await self._conn.send(f"V={value}") +``` + +- The datatype is inferred from the return type annotation of the getter + (`-> float` → a `float` attribute), matching how the datatype is inferred + from a getter's annotation on the procedural + `AttrR(getter=…)`/`AttrRW(getter=…, setter=…)` form + ([ADR 14](0014-attribute-io-rw-rework.md)), rather than requiring a `dtype=` + keyword the way PyTango does — decision 14 of #388 explicitly calls this out + as *better* than PyTango's `dtype=` kwarg since it's one real annotation, + checked statically. +- `@attr` comes in two forms: bare `@attr` and parameterised + `@attr(precision=3, units="V", poll_period=0.5)`; the keyword arguments map + onto the same `*Meta` fields (typed with `Unpack[…Meta]`, validated against + the getter's return type) and the `poll_period` read-side kwarg of + `AttrR`/`AttrRW` — sugar over that mechanism, not a parallel one. +- `@attr`'s `.setter` decorator mirrors `@property`/`@x.setter`, giving the + read+write pair a single logical name (`voltage`) with two decorated methods. + There is **no dedicated write-only decorator** — a paired-getter-less `AttrW` + is rare, so it is written longhand as `AttrW(setter=…)`. +- The getter's docstring becomes the attribute's `description`, as + `@command`/`@scan` already do. +- `@attr` supports the [ADR 17](0017-naming-pass.md) `Array1D`/`Table` hint + spellings as the getter's return annotation. +- There is **no** free-function `attr()` factory: the procedural spelling is + `AttrR(getter=…)` / `AttrRW(getter=…, setter=…)` directly. `@attr` degrades + gracefully into that procedural form for protocol families with more complex + needs, and into + [ADR 13](0013-declarative-procedural-split-and-controller-filler.md)'s filler + for introspection-driven attributes — `@attr` is explicitly the *simple* + case, not a replacement for either. +- Refines the class-body rule stated in + [ADR 13](0013-declarative-procedural-split-and-controller-filler.md) to: + *class body = declarations + decorated behaviour; instance scope = + construction with data* — already true today via `@command`/`@scan`, now + extended to attributes. +- Docs gain a "FastCS for PyTango users" page pairing this decorator with the + equivalent PyTango snippet, landing alongside this PR per #388 §8 item 5b. + +Interaction with the filler: an `@attr`-decorated attribute is already defined, +so [ADR 13](0013-declarative-procedural-split-and-controller-filler.md)'s +`ControllerFiller` treats it as filled and does not shadow it; a clash between +an introspected name and a decorated name raises. + +## Consequences + +- New driver code for the common "one attribute, one device call" case gets + noticeably shorter — closing the gap #388 §7.5 identifies against PyTango. +- `@attr` and the procedural `AttrR(getter=…)` / `AttrRW(getter=…, setter=…)` + form are two spellings over one implementation — the generated getter/setter + from [ADR 14](0014-attribute-io-rw-rework.md), with no separate callback-IO + classes. +- There are three ways to declare an attribute (bare hint + filler; explicit + `AttrRW(getter=…, setter=…)`; `@attr` sugar) — the docs need to be clear + about when to reach for which, so this doesn't become three equally-weighted + options with no guidance, undermining the "harsh split" clarity + [ADR 13](0013-declarative-procedural-split-and-controller-filler.md) is + trying to establish. +- Per #388 §8 item 5b, this sits on PR 1 (the per-attribute IO rework) — small + and independent of the `ControllerFiller` work — so it can land early and not + block on [ADR 13](0013-declarative-procedural-split-and-controller-filler.md). + +## Questions resolved in review (#402) + +1. **What is the decorator spelling?** `@attr` + `@x.setter` + (property-mirroring), not `@attr_r`/`@attr_rw`/`.send`. No dedicated + write-only decorator — `AttrW` alone is rare, written longhand. +2. **How is datatype/limits metadata passed?** Via decorator kwargs, typed with + `Unpack[…Meta]` (`precision`, `units`, limits — the + [ADR 14](0014-attribute-io-rw-rework.md)/[ADR 17](0017-naming-pass.md) + `*Meta` fields), validated against the getter's return type. +3. **Does it support the `Array1D`/`Table` hints?** Yes, as the getter's return + annotation. +4. **How does the filler treat a decorated attr?** As already defined — not + shadowed; a clash between an introspected name and a decorated name raises. +5. **Does the getter's docstring become the `description`?** Yes, as + `@command`/`@scan` already do. diff --git a/docs/explanations/decisions/0019-embedded-ophyd-async-connector.md b/docs/explanations/decisions/0019-embedded-ophyd-async-connector.md new file mode 100644 index 000000000..b61c43704 --- /dev/null +++ b/docs/explanations/decisions/0019-embedded-ophyd-async-connector.md @@ -0,0 +1,179 @@ +# 19. Embedded ophyd-async Connector + +Date: 2026-07-20 + +**Related:** [Issue #388](https://github.com/DiamondLightSource/fastcs/issues/388), +[ADR 13](0013-declarative-procedural-split-and-controller-filler.md), +[ADR 14](0014-attribute-io-rw-rework.md), [ADR 15](0015-typed-commands.md), +[ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md) + +## Status + +Proposed + +## Context + +ophyd-async already bridges to FastCS over the network: +`ophyd_async.fastcs.core.fastcs_connector(uri)` is a `PviDeviceConnector` +talking PVA+PVI. #388 proposes an **in-process** embedding as well — running +a FastCS `Controller` directly inside a bluesky/ophyd-async process, with no +network hop, for cases like running a `TemperatureController` straight from +a bluesky plan. + +Researching ophyd-async's `DeviceFiller` +(`ophyd_async/core/_device_filler.py`) as the direct structural reference +for [ADR 13](0013-declarative-procedural-split-and-controller-filler.md)'s +`ControllerFiller` surfaced the exact shape this connector needs to take: + +- `DeviceConnector.create_children_from_annotations` builds a `DeviceFiller` + once (memoised via `hasattr(self, "filler")`), then either fills + immediately or defers to `connect_real`. +- `connect_real` is where PVI and Tango connectors both actually introspect + and fill children — there is no ophyd-async precedent for "fill + everything at construction time" in a connect-time-introspecting + connector; embedding should follow the same connect-time pattern rather + than trying to fill eagerly. +- `SignalBackend`'s methods (`get_value`, `get_setpoint`, `set_callback`, + `put`, `get_datakey`) are the exact surface a `FastCSSignalBackend` needs + to implement in terms of FastCS's `.readback`/`.set()`/setpoint cache/native + timestamps (from [ADR 14](0014-attribute-io-rw-rework.md) and + [ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md)). +- `CommandBackend.execute`/`.signature` is the equivalent surface for typed + commands (from [ADR 15](0015-typed-commands.md)). + +This connector is explicitly the motivating consumer for +[ADR 13](0013-declarative-procedural-split-and-controller-filler.md), +[ADR 15](0015-typed-commands.md), and +[ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md) — it is +what forces those three to define a genuinely stable, documented surface +rather than an implicit one, since it lives in a different package +(ophyd-async) and cannot reach into FastCS internals the way FastCS's own +transports currently can. + +## Decision + +Per decision 6 of #388: no shared package. `FastCSDeviceConnector` lives +entirely on the ophyd-async side, behind an `ophyd-async[fastcs-embed]` +extra, importing only the stable FastCS surface formalised by +[ADR 13](0013-declarative-procedural-split-and-controller-filler.md) +(`ControllerFiller`) and [ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md) +(`ControllerAPI` tree, `ControllerRunner`, and the attribute/command runtime +methods). Convergence is by convention (the two projects agreeing on shape), +not by shared code. + +```python +from ophyd_async.fastcs import embedded_fastcs_connector + +class TempStage(Device): + ramp_rate: SignalRW[float] + power: SignalR[float] + cancel_all: TriggerableCommand + ramps: DeviceVector[TempRamp] + +stage = TempStage(connector=embedded_fastcs_connector(TemperatureController(settings))) +await stage.connect() # runs controller lifecycle in-process +``` + +Mechanics, directly mirroring `PviDeviceConnector`/`TangoDeviceConnector`: + +- `create_children_from_annotations`: builds a `DeviceFiller` with + `FastCSSignalBackend`/`FastCSCommandBackend` factories, `filled=False` — + same lazy pattern as the network connectors. +- `connect_real` (top level): starts the `ControllerRunner` — `initialise()`, + `post_initialise()`, `create_api_and_tasks()`, `Controller.connect()`, + initial coroutines, scan tasks scheduled on the *running* (bluesky) event + loop — then walks the `ControllerAPI` tree filling children via the + `DeviceFiller`, `check_filled()`, `set_name()`. Idempotent across + reconnects, per [ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md)'s + `ControllerRunner` requirement. +- `connect_mock` never touches the controller, so mock-mode ophyd-async + usage stays free (no FastCS controller/connection is instantiated at all). +- Lifecycle (decision 8 of #388): the connector owns the runner; shutdown + via an `atexit` hook plus an explicit `await connector.shutdown()`, which + cancels scan tasks and calls `Controller.disconnect()`. Reconnect is + `Device.connect(force_reconnect=True)`; there is no `Device.disconnect()` + proposal, and the only disconnect we want is `atexit`. +- Errors: FastCS gains a `ConnectionFailedError` (raised when the device + doesn't respond); the connector converts it to `NotConnectedError` and keeps + retrying to connect in the background. All other errors surface unconverted. +- Embedded + transports simultaneously (decision 9 of #388, e.g. a CA GUI + running next to a bluesky plan) is explicitly out of scope for the first + cut, but the `ControllerRunner` is designed so a transport list can be + attached later without redesigning it. + +Backend mappings (from #388 §5, grounded against the researched +`DeviceFiller`/`SignalBackend` surface): + +| ophyd-async | FastCS | +|---|---| +| `SignalBackend.get_value` | `AttrR.readback` | +| `SignalBackend.set_callback` | update-callback registration (`always=True`); stamped per [ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md) | +| `SignalBackend.put` | `AttrW.set(value)` | +| `SignalBackend.get_setpoint` | `AttrW.setpoint` cache, [ADR 14](0014-attribute-io-rw-rework.md)/[ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md) | +| `SignalBackend.get_datakey` | `attr.meta` (units, precision, limits) + python-type/enum choices → `SignalMetadata` + `make_datakey` | +| `CommandBackend.execute`/`.signature` | `Command.__call__` / captured `Signature`, [ADR 15](0015-typed-commands.md) | +| `SignalBackend.source` | e.g. `fastcs://.` | +| child `Device` / `DeviceVector` | sub-`Controller` / `ControllerVector` | +| (not exposed) | `@scan` methods — purely-internal periodic coroutines, bound to no `Attr`, not surfaced to ophyd-async (@shihab-dls, #402) | + +Datatype mapping: `int`/`float`/`bool`/`str` map straight across; the +`Array1D[dtype]` hint/runtime type (per [ADR 17](0017-naming-pass.md)) → +ophyd-async `Array1D[dtype]`; an enum class → the enum class itself. Two cases +needed a decision: + +- **Enums:** un-hinted enum classes introspect at runtime and drop to a string + datatype retaining the choices as metadata; hint-typed enums require the + author to duplicate as a `StrictEnum`/`SubsetEnum`/`SupersetEnum` (as they + would for remote FastCS) for now — revisit once there are use cases. +- **`Table`:** a real bidirectional converter **is in scope for the first + cut**, used as the opportunity to bring the FastCS and ophyd-async `Table` + implementations closer together. + +## Consequences + +- The stable FastCS interface promised in + [ADR 13](0013-declarative-procedural-split-and-controller-filler.md)/ + [ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md) gets + its first external, cross-repo consumer — any accidental internal + dependency this connector picks up is a signal that surface isn't + actually stable yet. +- This is entirely an ophyd-async-side deliverable (§8 items 6-8); + fastcs-core work is a dependency, not part of this repo's PRs. +- Per #388 coordination note: items 1-4 of the §8 work plan land in FastCS + before item 6 is mergeable; prototyping item 6 against a FastCS branch to + validate the stable interface *before* freezing it is the recommended + order, i.e. this connector should be prototyped against the `refactor` + branch here as the other ADRs' implementations land, not written blind + against a spec. +- `fastcs-demo`'s temperature controller simulation + (`fastcs.demo.simulation`) is the existing sim device ophyd-async tests + against — no new simulated device is needed for the first cut. +- Dropping `Device.disconnect()` means #388 §8 item 8 / issue #401 is + rewritten accordingly (reconnect via `force_reconnect=True`, disconnect via + `atexit` only). + +## Questions resolved in review (#402) + +1. **How are enums mapped?** Un-hinted → runtime-introspect, drop to string + keeping choices as metadata; hinted → require + `StrictEnum`/`SubsetEnum`/`SupersetEnum` duplication for now, revisit with + use cases. +2. **Is `Table` supported in the first cut?** Yes — a bidirectional converter, + used to converge the two `Table` implementations. +3. **How are connection errors handled?** FastCS gains a + `ConnectionFailedError`; the connector converts it to `NotConnectedError` and + keeps retrying to connect in the background. All other errors surface + unconverted. +4. **Is there a `Device.disconnect()`?** No — reconnect is + `Device.connect(force_reconnect=True)`; the only disconnect is `atexit`. +5. **Where does `@scan`-derived state that isn't a `Signal` go?** (@shihab-dls.) + Nowhere extra is needed. A `@scan`-decorated method is a **purely internal** + coroutine run periodically; it is *not* bound to an `Attr` and produces **no** + `Signal`. All exposed state already lives in `Attr` instances: a getter-based + `AttrR` schedules its getter as a scan-style task *and* is bound to the Attr + (→ `Signal`), and a soft `AttrR` fed by `@scan` via `update()` is likewise + the exposed Signal — so `@scan` surfaces nothing extra. A `@command` method + **is** different: it creates an `AttrW` and **is** exposed (the + `CommandBackend` row above). Both `@scan` and getter/update coroutines are + collected onto the running loop in `create_api_and_tasks()`; the connector + schedules `@scan` coroutines as internal tasks but never maps them to Signals. diff --git a/src/fastcs/demo/README.md b/src/fastcs/demo/README.md new file mode 100644 index 000000000..110f518a7 --- /dev/null +++ b/src/fastcs/demo/README.md @@ -0,0 +1,78 @@ +# `fastcs.demo` + +The demo package ships FastCS's **living example controllers** for the +ophyd-async / FastCS API-convergence refactor +([issue #388](https://github.com/DiamondLightSource/fastcs/issues/388), ADRs +0013–0019). Consolidated here (rather than a top-level `examples/` package) to +mirror ophyd-async, so the examples install with `fastcs[demo]` and can be run, +imported, and — crucially — used as the **single source of the tutorial code**. + +These modules are the canonical source the tutorials `literalinclude` from +(see the docs `tutorials/`). They are kept green under `uv run --locked tox`, +so the tutorials cannot drift from the framework: every framework PR that +changes an API updates the example(s) it affects in the *same* PR. This +replaces the old "hand-authored `docs/snippets/` that drift" approach — the +examples are the docs. + +## The example modules — a hello-world → complicated-device ladder + +Two hardware backends: a temperature-controller sim and a cut-down Eiger REST +sim. The hello-world is pure-soft (no backend). IO is supplied as plain +`getter`/`setter` callables on `AttrR`/`AttrW`/`AttrRW` (or the `@attr` +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_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 — +`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). +4. **introspectable** — `eiger.py`. + +Notes: + +- **The declarative style is the DRY answer for a real protocol family**, not + a reusable IO object. Recommend it when the shared wire pattern is worth + naming (a protocol you'll reuse); for a handful of bespoke attributes, + getter/setter in `__init__` is lighter and fine. +- **`temperature_scpi.py` is deliberately *not* introspectable.** A SCPI device + does not describe itself, which is exactly why you hand-annotate: the metadata + lives in your Python (`SCPIParam("P", precision=3, …)`), not on the wire. Do + **not** invent SCPI introspection — that would erase the contrast with the + Eiger example. The `SCPIController`/`SCPIParam` vocabulary lives *here in the + demo*, not in core FastCS (decision 3: core ships no extras vocabulary for + 1.0); it demonstrates how a protocol layer builds on the filler's + `(child, extras)` mechanism. +- **`eiger.py` uses a separate REST backend on purpose.** Introspection earns + its complexity only when a device's parameters aren't knowable at author time + (a detector, not a fixed-command temp controller). The backend switch *is* + the lesson — "small & known → declare; large & self-describing → introspect" + — and the REST sim also exercises an HTTP client backend the temp examples + never touch, matching real downstream drivers (`fastcs-eiger`, `fastcs-secop`, + PandABlocks). + +## 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 +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. + +`literalinclude` region markers are added to each module as part of writing its +tutorial (the umbrella docs pass, +[#408](https://github.com/DiamondLightSource/fastcs/issues/408)), not up front.