diff --git a/docs/conf.py b/docs/conf.py index 3ee2ad966..edfd712dd 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -89,6 +89,9 @@ ("py:class", "p4p.nt.enum.NTEnum"), ("py:class", "p4p.nt.ndarray.NTNDArray"), ("py:class", "p4p.nt.NTTable"), + # httpx and fastapi don't have intersphinx mappings + ("py:class", "httpx.AsyncBaseTransport"), + ("py:class", "fastapi.applications.FastAPI"), # Problems in FastCS itself ("py:class", "BaseController"), ("py:class", "AttrIOUpdateCallback"), @@ -98,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/docs/explanations/controllers.md b/docs/explanations/controllers.md index 2378144d2..3d18defd5 100644 --- a/docs/explanations/controllers.md +++ b/docs/explanations/controllers.md @@ -28,8 +28,9 @@ lifecycle, if required. ### Scan task behaviour When used as the root controller, FastCS collects all `@scan` methods and readable -attributes with `update_period` set, across the whole controller hierarchy to be run as -background tasks by FastCS. Scan tasks are gated on the `_connected` flag: if a scan +attributes whose `getter` is wrapped in `Polled`, across the whole controller +hierarchy, to be run as background tasks by FastCS. Scan tasks are gated on the +`_connected` flag: if a scan raises an exception, `_connected` is set to `False` and tasks pause until `reconnect` sets it back to `True`. @@ -154,7 +155,7 @@ distinct components with different types or roles. `BaseController` is the common base class for both `Controller` and `ControllerVector`. It handles the creation and validation of attributes, scan methods, command methods, and -sub controllers, including type hint introspection and IO connection. +sub controllers, including type hint introspection. `BaseController` is public for use in **type hints only**. It should not be subclassed directly when implementing a device driver. Use `Controller` or `ControllerVector` diff --git a/docs/explanations/datatypes.md b/docs/explanations/datatypes.md index c614deb1d..fb1d81740 100644 --- a/docs/explanations/datatypes.md +++ b/docs/explanations/datatypes.md @@ -175,7 +175,7 @@ float_type.validate(42) # Returns 42.0 (int -> float) Validation runs automatically when: 1. **Attribute update**: `await attr.update(value)` validates before storing -2. **Put request**: `await attr.put(value)` validates before sending to device +2. **Set request**: `await attr.set(value)` validates before sending to device 3. **Initial value**: Values passed to `initial_value` are validated on creation ```python @@ -188,9 +188,9 @@ attr = AttrRW(Int(min=0, max=10), initial_value=5) await attr.update(7) # OK await attr.update(15) # Raises ValueError -# Puts are validated -await attr.put(3) # OK -await attr.put(-1) # Raises ValueError +# Sets are validated +await attr.set(3) # OK +await attr.set(-1) # Raises ValueError ``` ## Transport Handling 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..f9c2ce865 --- /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=Polled(get_start, period=0.2), setter=set_start) +``` + +**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..5911b3ec8 --- /dev/null +++ b/docs/explanations/decisions/0014-attribute-io-rw-rework.md @@ -0,0 +1,404 @@ +# 14. Per-Attribute IO as getter/setter Callables + +Date: 2026-07-20 (revised 2026-08-03, after the #412 review) + +**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), [ADR 20](0020-transport-setpoint-mirroring.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 setpoint cache immediately — the sanctioned replacement for + `fastcs-secop`'s private `_call_sync_setpoint_callbacks`. +- **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). +- 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=Polled(get_ramp_rate, period=0.2), + setter=set_ramp_rate, + units="deg", + ) +``` + +### The reading schedule travels with the getter + +There is no `poll_period` constructor argument. A getter carries its own +schedule, so the two cannot drift apart and the pair can be passed around as +one value: + +```python +self.config = AttrR(float, getter=self._get_config) # once, at connect +self.reading = AttrR(float, getter=Polled(self._get_reading, period=0.2)) # every 0.2s +self.label = AttrR(str, getter=NotPolled(self._get_label)) # never; poll() only +self.computed = AttrR(float) # soft, no getter +``` + +`Polled` and `NotPolled` take an optional getter and bind one when called, so +the same objects serve the declarative spelling in +[ADR 18](0018-attr-decorator-sugar.md) — where the getter arrives by decoration +and there is no argument to wrap — giving one vocabulary across both: + +| Schedule | Procedural | Declarative | +|---|---|---| +| Once, at connect | `AttrR(t, getter=g)` | `@attr(units="V")` | +| Every 0.5s | `AttrR(t, getter=Polled(g, period=0.5))` | `@attr(Polled(0.5), units="V")` | +| Never; `poll()` only | `AttrR(t, getter=NotPolled(g))` | `@attr(NotPolled(), units="V")` | + +**A bare getter means "read once, at connect"**, not "never read". Three +defaults were considered: + +1. *Bare = once* (chosen). Fails safe: an attribute always shows a real value, + and polling is opted into per attribute rather than being something you must + remember to switch off. +2. *Bare = never read.* Restores the pre-refactor `AttributeIORef.update_period + = None` default and makes all scheduling explicit — but fails **silently**: + an unpolled `AttrRW` sits at the datatype default and, under + [ADR 20](0020-transport-setpoint-mirroring.md), never establishes a setpoint + either, so every transport shows `0`/`""`/`False` until someone writes to it. +3. *No default; always require a wrapper.* Rejected because ADR 18 promises a + bare `@attr`, which must resolve to some schedule. A constructor that refused + to default while the decorator defaulted would reintroduce the asymmetry + these wrappers exist to remove. + +`ONCE` (`float("inf")`) survives internally as what `poll_period` reports for +the bare case, but a driver author never spells it: `Polled(getter, +period=ONCE)` would read as a contradiction, and the once-only case is the one +with no wrapper at all. `NotPolled(g)` is distinct from having no getter — the +former is still readable via `await attr.poll()` and by transports on demand, +the latter has nothing to read. + +### `Update[T]` + +`Update` is what a getter or setter returns when a bare value is not enough: + +```python +@dataclass +class Update(Generic[T]): + readback: T + timestamp: float | None = None # None ⇒ framework stamps receive-time + setpoint: T | None = None # None ⇒ leave the cached setpoint alone +``` + +- `readback` is the value, named for the cache it feeds. +- `setpoint` is how a device that reports its own setpoint drives one, and how + a setter distinguishes "the device clamped the value it will *report*" from + "the device clamped what I *asked for*". A **bare** value returned from a + setter means both — it is equivalent to `Update(readback=v, setpoint=v)`. +- `severity` is **not** on `Update` yet; native timestamps and the severity + enum are [ADR 16](0016-setpoint-cache-timestamps-and-controller-runner.md)'s + scope and land with it. `timestamp` is accepted here so the field ordering is + settled, but is not yet persisted. + +### 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) | +| `update_setpoint(value)` | async method | — | ✓ | ✓ | no (cache push) | +| `set(value)` | async method | — | ✓ | ✓ | **yes** (setter) | +| `add_readback_callback()` | method | ✓ | — | ✓ | no | +| `add_setpoint_callback()` | method | — | ✓ | ✓ | no | + +- **`.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` is now a read-only property reporting the schedule resolved from + the getter's wrapper, not a constructor argument. 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. + `update_setpoint(value)` is its setpoint-side counterpart. +- **`set(value)` replaces `put()`** (the bluesky/ophyd verb): it caches + `.setpoint` immediately (decision 10a) and publishes it to the setpoint + callbacks, then runs the setter; the setter's return feeds `.readback` via + `update()`. The old `sync_setpoint=` kwarg and `_call_sync_setpoint_callbacks` + are gone. +- **The two callback registrars are symmetric.** `add_readback_callback()` + (formerly `add_on_update_callback()`) and `add_setpoint_callback()` are how + transports publish each cache. Transports must not track a setpoint of their + own — see [ADR 20](0020-transport-setpoint-mirroring.md), which also removes + the per-transport "seeding" of a setpoint display by making the first readback + on an `AttrRW` establish the setpoint. + +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=Polled(get_ramp, period=0.2), setter=set_ramp) +``` + +The old ref's `update_period=0.2` becomes the `Polled(..., period=0.2)` wrapper; +a ref that left `update_period` at its `None` default becomes `NotPolled(...)` +if it really should never be read, or a bare getter if a connect-time read was +what it wanted. + +`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. +- Drivers that relied on the old ref default of `update_period=None` change + behaviour if they migrate to a bare getter: they gain a connect-time read. + This is intended (see the three options above) but is the one migration step + that is not purely mechanical. + +## Questions resolved in review (#402, #412) + +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 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.) It is now. Caching `.setpoint` before running + the setter was originally an *attribute-cache* guarantee only, with the + remote-client view left transport-dependent: **PVA** posted the setpoint as + soon as it was written, whereas **CA** posted only *after* the update callback + completed, so a long-running setter delayed the CA-visible setpoint. The + follow-up this left open is closed by + [ADR 20](0020-transport-setpoint-mirroring.md): every transport now mirrors + the attribute's setpoint through `add_setpoint_callback()`, which fires + before the setter runs, so CA and PVA agree and the ordering is a property of + the attribute rather than of each transport. +7. **Should `poll_period` be a second constructor argument?** No — merged into + the getter as `Polled`/`NotPolled` wrappers, so a getter and its schedule are + one value and the same vocabulary works in the `@attr` decorator, where there + is no getter argument to pair it with. 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..196775cbf --- /dev/null +++ b/docs/explanations/decisions/0018-attr-decorator-sugar.md @@ -0,0 +1,156 @@ +# 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(Polled(0.5), units="V") # 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(Polled(0.5), precision=3, units="V")`. The keyword arguments map onto + the same `*Meta` fields (typed with `Unpack[…Meta]`, validated against the + getter's return type). The optional leading positional is a **schedule** - + the same `Polled`/`NotPolled` objects the procedural form wraps its getter in + ([ADR 14](0014-attribute-io-rw-rework.md), amendment 2026-08-03) - so the two + spellings share one vocabulary rather than the decorator taking a + `poll_period=` kwarg the constructor no longer has. Sugar over that + mechanism, not a parallel one. +- **Bare `@attr` means the same as a bare `getter=`**: read once, when the + controller connects. This symmetry is why the constructor keeps a default + instead of demanding a wrapper - a bare decorator has to resolve to some + schedule, so both sides default to the same safe one: + + | Schedule | Procedural | Declarative | + |---|---|---| + | Once, at connect | `AttrR(t, getter=g)` | `@attr(units="V")` | + | Every 0.5s | `AttrR(t, getter=Polled(g, period=0.5))` | `@attr(Polled(0.5), units="V")` | + | Never; `poll()` only | `AttrR(t, getter=NotPolled(g))` | `@attr(NotPolled(), units="V")` | +- `@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/docs/explanations/decisions/0020-transport-setpoint-mirroring.md b/docs/explanations/decisions/0020-transport-setpoint-mirroring.md new file mode 100644 index 000000000..5d83520f3 --- /dev/null +++ b/docs/explanations/decisions/0020-transport-setpoint-mirroring.md @@ -0,0 +1,69 @@ +# 20. Transports mirror the attribute setpoint rather than tracking their own + +Date: 2026-08-03 + +## Status + +Accepted + +## Context + +An `AttrRW` has two values a transport must present: the readback (what the device +reports) and the setpoint (what was last asked of it). Readbacks were already +published by callback - the attribute calls back on every change and each transport +posts it - but setpoints were not. + +Instead, each transport maintained its own setpoint display and updated it directly +in its write path, on the assumption that the only thing that could change a +setpoint was a write arriving through that same transport. That assumption is wrong +as soon as there is more than one transport, or a device that reports its own +setpoint. + +It also left a visible gap at startup. A setpoint display starts at the datatype's +default, which is usually not the device's actual value, so each transport grew a +one-shot "seeding" hack: subscribe to the *readback* callback, and the first time a +value arrives, copy it into the setpoint display and unsubscribe. Two transports had +near-identical copies of this, and it only worked for `AttrRW` (a pure `AttrW` has no +readback to seed from). + +The two EPICS transports had also drifted apart on ordering. PVA posted the setpoint +as soon as the put arrived, before the setter ran; CA posted it only after the update +callback completed, so a slow setter left the CA setpoint stale for the duration of +the write. + +## Decision + +The attribute owns the setpoint, and transports mirror it. + +- `AttrW` gains `add_setpoint_callback()`, the setpoint-side counterpart of + `AttrR.add_readback_callback()` (renamed from `add_on_update_callback()` for the + symmetry). Every transport registers one and posts whatever it is given. +- `AttrW.update_setpoint()` caches a setpoint and fires those callbacks. `set()` + calls it before running the setter, so the requested value is visible immediately; + a value returned by the setter goes through it again, so a clamped or rejected + value replaces it. +- A getter or setter can also drive the setpoint by returning + `Update(readback=..., setpoint=...)` - the mechanism for a device that reports its + own setpoint. `setpoint=None` (the default) leaves the cached setpoint alone. +- Seeding is gone. An `AttrRW` starts with no known setpoint, and the first readback + to arrive - from a poll, a scan, or anything else calling `update()` - establishes + it. Subsequent readbacks do not, so a readback that disagrees with the setpoint + does not silently rewrite what the user asked for. + +Transports must not update their own setpoint display directly in their write path. + +## Consequences + +Every transport shows the same setpoint, whichever transport was written through, and +CA and PVA now agree on when it appears: at the start of the write, before the setter +runs. That is the behaviour PVA already had, and it is the one that gives GUIs +immediate feedback. + +Attribution is lost at the transport layer - a client cannot tell from the setpoint +alone which transport originated a write. This is deliberate: consistency between +transports is worth more than attribution, and attribution is recoverable from the +logs, which record the originating transport for every `set()`. + +The one-shot seeding blocks in the CA and PVA transports are deleted, along with the +`isinstance(attribute, AttrR)` checks that guarded them, since the mechanism now works +for a pure `AttrW` too. diff --git a/docs/explanations/transports.md b/docs/explanations/transports.md index bf34a2fcc..5a99c9ff5 100644 --- a/docs/explanations/transports.md +++ b/docs/explanations/transports.md @@ -98,14 +98,15 @@ layer. | Callback | Registered with | Triggered By | Direction | Purpose | |----------|-----------------|--------------|-----------|---------| -| On Update | `add_on_update_callback()` | `attr.update(value)` | Publish ↑ | Update protocol representation when attribute value changes | -| Sync Setpoint | `add_sync_setpoint_callback()` | `attr.put(value, sync_setpoint=True)` | Publish ↑ | Update transport's setpoint display without device communication | +| Readback | `add_readback_callback()` | `attr.update(value)` | Publish ↑ | Update protocol representation when the attribute's readback changes | +| Setpoint | `add_setpoint_callback()` | `attr.set(value)` | Publish ↑ | Update protocol representation when the attribute's setpoint changes | | Update Datatype | `add_update_datatype_callback()` | `datatype` property changes | Publish ↑ | Update protocol metadata when datatype changes | -| Put | `attr.put(value)` | Transport receives user input | Put ↓ | Forward write requests from protocol to attribute | +| Set | `attr.set(value)` | Transport receives user input | Set ↓ | Forward write requests from protocol to attribute | -### On Update Callbacks +### Readback Callbacks -Use `add_on_update_callback()` to update the protocol layer when an attribute's value changes. +Use `add_readback_callback()` to update the protocol layer when an attribute's +readback changes. ```python def create_read(name, attribute): @@ -114,7 +115,7 @@ def create_read(name, attribute): async def update_protocol_value(value): protocol_read.post(value) - attribute.add_on_update_callback(update_protocol_value) + attribute.add_readback_callback(update_protocol_value) ``` The callback receives the new value and should update the protocol-specific @@ -129,7 +130,7 @@ Use `add_update_datatype_callback()` to update protocol metadata when an attribu def create_read(name, attribute): ... - attribute.add_on_update_callback(update_protocol_value) + attribute.add_readback_callback(update_protocol_value) def update_protocol_metadata(datatype: DataType): protocol_read.set_units(datatype.units) @@ -140,49 +141,47 @@ def create_read(name, attribute): The callback receives the new `DataType` instance and should update the protocol's metadata representation (e.g., EPICS record fields like `EGU`, `HOPR`, `LOPR`). -### Put +### Setpoint Callbacks -When the transport receives a write request from the protocol, call `await -attribute.put(value)` to forward it to the attribute. This triggers validation and -propagates the value to the device via the IO layer. The transport should also update -its own setpoint display directly rather than relying on the sync setpoint callback -being called. +Use `add_setpoint_callback()` to update the protocol layer when an attribute's +setpoint changes. A transport must **not** update its own setpoint display directly - +it registers a callback and lets the attribute drive it, so that every transport +agrees on the setpoint however it was changed (see +[](./decisions/0020-transport-setpoint-mirroring)). ```python def create_write(name, attribute): protocol_setpoint = Protocol(name) - async def handle_write(value): + async def update_protocol_setpoint(value): protocol_setpoint.post(value) - await attribute.put(value) -``` - -### Sync Setpoint Callbacks -Use `add_sync_setpoint_callback()` to update the protocol layer's setpoint -representation when the transport receives a write request. This is called when -`AttrW.put` is called with `sync_setpoint=True`. - -Each transport is responsible for updating its own setpoint display while actioning the -change and should not rely on its sync setpoint callback being called by the attribute, -nor should it call `AttrW.put` with `sync_setpoint=True`. Setpoints should not be synced -between transports in this case - this is intentional to show which transport the change -came from. + async def handle_write(value): + await attribute.set(value) -```python -def create_write(name, attribute): - ... + attribute.add_setpoint_callback(update_protocol_setpoint) +``` - async def update_setpoint_display(value): - protocol_setpoint.post(value) +The callback fires when: - attribute.add_sync_setpoint_callback(update_setpoint_display) -``` +- a write arrives through *any* transport - `set()` caches the requested value and + publishes it before running the setter, so the display updates immediately rather + than waiting for a slow device; +- the setter returns a value, which replaces it with the device's accepted or clamped + value; +- a getter or setter returns `Update(readback=..., setpoint=...)`, for a device that + reports its own setpoint; +- the first readback arrives on an `AttrRW` that has never been written. An `AttrRW` + starts with no known setpoint, so this is what stops a setpoint display sitting at + the datatype's default until someone writes to it. No seeding is required in the + transport. -Sync setpoint callbacks are used in specific cases: +### Set -- When an attribute delegates to other attributes that actually communicate with the device -- During the first update of an `AttrRW`, to initialize the setpoint with the first readback value +When the transport receives a write request from the protocol, call `await +attribute.set(value)` to forward it to the attribute. This triggers validation, caches +the value as the attribute's `.setpoint` (firing the setpoint callbacks above), and (if +the attribute has one) runs its `setter` to propagate the value to the device. ## Commands diff --git a/docs/explanations/what-is-fastcs.md b/docs/explanations/what-is-fastcs.md index 501a54284..da10059e9 100644 --- a/docs/explanations/what-is-fastcs.md +++ b/docs/explanations/what-is-fastcs.md @@ -22,9 +22,8 @@ without modification. A FastCS application has three layers: **Controller** - a Python class that models the device. It holds attributes and -commands, implements connection logic, and creates periodic polling tasks. The -controller can create `AttributeIO`s to handle `update` and `send` operations between -attributes and the device. +commands, implements connection logic, and creates periodic polling tasks. Attributes +take `getter`/`setter` callables that read and write values on the device. **Attributes and commands** - typed values (`AttrR`, `AttrW`, `AttrRW`) and callable actions (`@command`) declared on the controller. Attributes represent the device's diff --git a/docs/how-to/table-waveform-data.md b/docs/how-to/table-waveform-data.md index d7a7572bc..8a768604f 100644 --- a/docs/how-to/table-waveform-data.md +++ b/docs/how-to/table-waveform-data.md @@ -129,7 +129,7 @@ await controller.channel_data.update(data) ```python # Get the table -table = controller.results.get() +table = controller.results.readback # Access by column name names = table["name"] diff --git a/docs/how-to/update-attributes-from-device.md b/docs/how-to/update-attributes-from-device.md index 895eb5f2a..f76fbc04e 100644 --- a/docs/how-to/update-attributes-from-device.md +++ b/docs/how-to/update-attributes-from-device.md @@ -3,129 +3,121 @@ There are different patterns for pushing values from a device into attributes to suit different use cases. Choose the pattern that fits how the device API delivers data. -## Update Tasks via `AttributeIO.update` +## Poll via a Getter -Use this pattern when each attribute maps to an independent request to the device. The -`AttributeIO.update` method is called periodically as a background task, once per -attribute, at the rate set by `update_period` in the attribute's `AttributeIORef`. +Use this pattern when each attribute maps to an independent request to the device. Give +the attribute a `getter` wrapped in `Polled` and FastCS will call it periodically as a +background task, at the period given. -Define an `AttributeIORef` with an `update_period` and implement `AttributeIO.update` -to query the device and call `attr.update` with the result: +Write a getter that queries the device and returns the value - the framework caches it +and calls any update callbacks; there's no need to call `attr.update` yourself: ```python -from dataclasses import KW_ONLY, dataclass - -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, NotPolled, Polled from fastcs.controllers import Controller from fastcs.datatypes import Float, String -@dataclass -class MyDeviceIORef(AttributeIORef): - register: str - _: KW_ONLY - update_period: float | None = 0.5 - - -class MyDeviceIO(AttributeIO[float, MyDeviceIORef]): +class MyController(Controller): def __init__(self, connection): - super().__init__() self._connection = connection + super().__init__() - async def update(self, attr: AttrR[float, MyDeviceIORef]): - response = await self._connection.send_query(f"{attr.io_ref.register}?\r\n") - await attr.update(float(response.strip())) + self.temperature = AttrR( + Float(), getter=Polled(self._get_temperature, period=0.5) + ) + self.setpoint = AttrRW( + Float(), + getter=Polled(self._get_setpoint, period=1.0), + setter=self._set_setpoint, + ) + self.label = AttrR(String(), getter=NotPolled(self._get_label)) - async def send(self, attr: AttrW[float, MyDeviceIORef], value: float): - await self._connection.send_command(f"{attr.io_ref.register}={value}\r\n") + async def _get_temperature(self) -> float: + response = await self._connection.send_query("T?\r\n") + return float(response.strip()) + async def _get_setpoint(self) -> float: + response = await self._connection.send_query("S?\r\n") + return float(response.strip()) -class MyController(Controller): - temperature = AttrR(Float(), io_ref=MyDeviceIORef("T")) - setpoint = AttrRW(Float(), io_ref=MyDeviceIORef("S", update_period=1.0)) - label = AttrR(String(), io_ref=MyDeviceIORef("L", update_period=None)) + async def _set_setpoint(self, value: float) -> None: + await self._connection.send_command(f"S={value}\r\n") - def __init__(self, connection): - super().__init__(ios=[MyDeviceIO(connection)]) + async def _get_label(self) -> str: + response = await self._connection.send_query("L?\r\n") + return response.strip() ``` -Setting `update_period` to: +How the getter is passed decides when it is called: -- A positive `float` — polls at that interval in seconds. -- `None` — no automatic updates; the attribute value is only set explicitly (e.g. from a - scan method or subscription callback). -- `ONCE` (imported from `fastcs`) — called once on startup and not again. +- A bare getter (`getter=self._get_label`) — the `ONCE` schedule: read when the + controller connects, and not again. Use it for values that only change because + you changed them, such as writable configuration the device holds for you. +- `Polled(getter, period=0.5)` — polls at that interval in seconds. Use it for + values the device changes on its own, such as readings and status. +- `NotPolled(getter)` — never read on a schedule; the attribute value is only set + explicitly (e.g. from a scan method or subscription callback), or read on demand + via `await attr.poll()`. This differs from giving no getter at all, which leaves + nothing to read on demand. -## Initial Read with Event-Driven Updates from Puts +`ONCE` is the default when a getter is given, so polling is opted into per +attribute rather than being something you have to remember to switch off. -Use this pattern when attributes need their initial value read on startup, but subsequent -updates arrive as side-effects of write operations rather than on a fixed poll cycle. -This is common for devices that echo back related parameter values in their response to a -set command. +## Initial Read with Event-Driven Updates from Sets -Set `update_period=ONCE` on the `AttributeIORef` so that `AttributeIO.update` is called -once when the application starts. Then, in `AttributeIO.send`, parse the device's -response to the put and call `attr.update` on any attributes whose values have changed: +Use this pattern when attributes need their initial value read on startup, but +subsequent updates arrive as side-effects of write operations rather than on a fixed +poll cycle. This is common for devices that echo back related parameter values in their +response to a set command. -```python -from collections.abc import Awaitable, Callable -from dataclasses import KW_ONLY, dataclass +Pass the getter bare, without `Polled`, so it runs once on startup and not again. +Then, in the setter, parse the device's response and call `.update()` directly on any +sibling attributes whose values have changed: -from fastcs import ONCE -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +```python +from fastcs.attributes import AttrR, AttrRW from fastcs.controllers import Controller from fastcs.datatypes import Float -@dataclass -class MyDeviceIORef(AttributeIORef): - register: str - _: KW_ONLY - update_period: float | None = ONCE - - - -PutResponseCallback = Callable[[str], Awaitable[None]] - - -class MyDeviceIO(AttributeIO[float, MyDeviceIORef]): - def __init__(self, connection, on_put_response: PutResponseCallback | None = None): - super().__init__() +class MyController(Controller): + def __init__(self, connection): self._connection = connection - self._on_put_response = on_put_response - - async def update(self, attr: AttrR[float, MyDeviceIORef]): - response = await self._connection.send_query(f"{attr.io_ref.register}?\r\n") - await attr.update(float(response.strip())) + super().__init__() - async def send(self, attr: AttrW[float, MyDeviceIORef], value: float): - # Device responds with a snapshot of all current values after a set - response = await self._connection.send_query( - f"{attr.io_ref.register}={value}\r\n" + self.setpoint = AttrRW( + Float(), getter=self._get_setpoint, setter=self._set_setpoint ) - if self._on_put_response is not None: - await self._on_put_response(response) + self.actual_temperature = AttrR(Float(), getter=self._get_actual_temperature) + self.power = AttrR(Float(), getter=self._get_power) + self.status = AttrR(Float(), getter=self._get_status) + async def _get_setpoint(self) -> float: + return float((await self._connection.send_query("S?\r\n")).strip()) -class MyController(Controller): - setpoint = AttrRW(Float(), io_ref=MyDeviceIORef("S")) - actual_temperature = AttrR(Float(), io_ref=MyDeviceIORef("T")) - power = AttrR(Float(), io_ref=MyDeviceIORef("P")) - status = AttrR(Float(), io_ref=MyDeviceIORef("X")) - - def __init__(self, connection): - super().__init__(ios=[MyDeviceIO(connection, self._handle_put_response)]) - - async def _handle_put_response(self, response: str) -> None: + async def _set_setpoint(self, value: float) -> None: + # Device responds with a snapshot of all current values after a set + response = await self._connection.send_query(f"S={value}\r\n") actual, power, status = response.strip().split(",") await self.actual_temperature.update(float(actual)) await self.power.update(float(power)) await self.status.update(float(status)) + + async def _get_actual_temperature(self) -> float: + return float((await self._connection.send_query("T?\r\n")).strip()) + + async def _get_power(self) -> float: + return float((await self._connection.send_query("P?\r\n")).strip()) + + async def _get_status(self) -> float: + return float((await self._connection.send_query("X?\r\n")).strip()) ``` -Attributes that are updated as side-effects of puts can still carry `update_period=ONCE` -so they also get their initial value on startup. Set `update_period=None` instead if the -device response to the put is the only source of truth and no initial poll is needed. +Attributes that are updated as a side-effect of a set can still take a bare getter, +so they also get their initial value on startup. Use `NotPolled(getter)` instead if +the device's response to the set is the only source of truth and no initial poll is +needed. ## Batched Updates via a Scan Method @@ -133,8 +125,9 @@ Use this pattern when the device returns values for multiple attributes in a sin response. A `@scan` method runs periodically on the controller and distributes the results by calling `attr.update` directly on each attribute. -Attributes that are updated this way do not need an `io_ref` with an `update_period` -because the scan method drives the updates rather than individual IO tasks. +Attributes that are updated this way do not need a `getter` at all, because +the scan method drives the updates directly, rather than each attribute polling +independently. ```python import json @@ -146,7 +139,7 @@ from fastcs.methods import scan class ChannelController(Controller): - voltage = AttrR(Float()) # No io_ref — updated by parent scan method + voltage = AttrR(Float()) # No getter — updated by parent scan method def __init__(self, index: int, connection): super().__init__(f"Ch{index:02d}") @@ -178,66 +171,53 @@ class MultiChannelController(Controller): The scan period (here `0.1` seconds) sets how often the batched query runs. Scans that raise an exception will pause and wait for `reconnect()` to be called before resuming. -### Scan as a cache for `AttributeIO.update` +### Scan as a cache for getters When there are many attributes to update from a batched response, calling `attr.update` for each one inside the scan method becomes verbose. Instead, the scan can populate a -cache on the `AttributeIO`, and each attribute's regular update task reads from that -cache rather than querying the device while the device is still only queried once per -cycle. +shared cache, and each attribute's own getter (polled independently) reads from that +cache rather than querying the device - the device is still only queried once per cycle. ```python import json -from dataclasses import KW_ONLY, dataclass -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR +from fastcs.attributes import AttrR from fastcs.controllers import Controller from fastcs.datatypes import Float from fastcs.methods import scan -@dataclass -class ChannelIORef(AttributeIORef): - index: int - _: KW_ONLY - update_period: float | None = 0.1 - - -class ChannelIO(AttributeIO[float, ChannelIORef]): - def __init__(self): - super().__init__() - self._cache: dict[int, float] = {} - - def update_cache(self, values: dict[int, float]) -> None: - self._cache = values +class ChannelController(Controller): + def __init__(self, index: int, cache: dict[int, float]): + self._index = index + self._cache = cache + super().__init__(f"Ch{index:02d}") - async def update(self, attr: AttrR[float, ChannelIORef]): - cached = self._cache.get(attr.io_ref.index) - if cached is not None: - await attr.update(cached) + self.voltage = AttrR(Float(), getter=Polled(self._get_voltage, period=0.1)) - -class ChannelController(Controller): - def __init__(self, index: int, io: ChannelIO): - super().__init__(f"Ch{index:02d}", ios=[io]) - self.voltage = AttrR(Float(), io_ref=ChannelIORef(index)) + async def _get_voltage(self) -> float: + return self._cache.get(self._index, 0.0) class MultiChannelController(Controller): def __init__(self, channel_count: int, connection): self._connection = connection - self._channel_io = ChannelIO() + self._cache: dict[int, float] = {} super().__init__() + self._channels: list[ChannelController] = [] for i in range(channel_count): - self.add_sub_controller(f"Ch{i:02d}", ChannelController(i, self._channel_io)) + ch = ChannelController(i, self._cache) + self._channels.append(ch) + self.add_sub_controller(f"Ch{i:02d}", ch) @scan(0.1) async def fetch_voltages(self): voltages = json.loads( (await self._connection.send_query("V?\r\n")).strip() ) - self._channel_io.update_cache(dict(enumerate(map(float, voltages)))) + self._cache.clear() + self._cache.update(enumerate(map(float, voltages))) ``` ## Subscription Callbacks diff --git a/docs/how-to/wait-methods.md b/docs/how-to/wait-methods.md index d61fb8bec..e9ef09b94 100644 --- a/docs/how-to/wait-methods.md +++ b/docs/how-to/wait-methods.md @@ -20,7 +20,7 @@ class MotorController(Controller): @command() async def move_and_wait(self): """Move to target and wait until we arrive.""" - target = self.target.get() + target = self.target.readback # Start the move (implementation depends on your device) await self._start_move(target) diff --git a/docs/snippets/dynamic.py b/docs/snippets/dynamic.py index 7dde6dfe5..9a5cc4f55 100644 --- a/docs/snippets/dynamic.py +++ b/docs/snippets/dynamic.py @@ -1,23 +1,31 @@ import json -from dataclasses import KW_ONLY, dataclass from typing import Any, Literal, TypeVar from pydantic import BaseModel, ConfigDict, ValidationError -from fastcs.attributes import ( - Attribute, - AttributeIO, - AttributeIORef, - AttrR, - AttrRW, - AttrW, -) +from fastcs.attributes import Attribute, AttrR, AttrRW from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import Bool, DataType, Float, Int, String from fastcs.launch import FastCS from fastcs.transports.epics.ca import EpicsCATransport +ValueT = TypeVar("ValueT") + + +class TemperatureProtocol: + def __init__(self, connection: IPConnection): + self._connection = connection + + async def send_command(self, param: str, value: ValueT, dtype: type[ValueT]): + command = f"{param}={dtype(value)}" # type: ignore[call-arg] + await self._connection.send_command(f"{command}\r\n") + + async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: + query = f"{param}?" + response = await self._connection.send_query(f"{query}\r\n") + return dtype(response.strip("\r\n")) # type: ignore[call-arg] + class TemperatureControllerParameter(BaseModel): model_config = ConfigDict(extra="forbid") @@ -39,7 +47,9 @@ def fastcs_datatype(self) -> DataType: return String() -def create_attributes(parameters: dict[str, Any]) -> dict[str, Attribute]: +def create_attributes( + parameters: dict[str, Any], protocol: TemperatureProtocol +) -> dict[str, Attribute]: attributes: dict[str, Attribute] = {} for name, parameter in parameters.items(): name = name.replace(" ", "_").lower() @@ -50,46 +60,23 @@ def create_attributes(parameters: dict[str, Any]) -> dict[str, Attribute]: print(f"Failed to validate parameter '{parameter}'\n{e}") continue - io_ref = TemperatureControllerAttributeIORef(parameter.command) + datatype = parameter.fastcs_datatype + command = parameter.command + + async def getter(command=command, dtype=datatype.dtype): + return await protocol.send_query(command, dtype) + match parameter.access_mode: case "r": - attributes[name] = AttrR(parameter.fastcs_datatype, io_ref=io_ref) + attributes[name] = AttrR(datatype, getter=getter) case "rw": - attributes[name] = AttrRW(parameter.fastcs_datatype, io_ref=io_ref) - - return attributes - -NumberT = TypeVar("NumberT", int, float) + async def setter(value, command=command, dtype=datatype.dtype): + await protocol.send_command(command, value, dtype) + attributes[name] = AttrRW(datatype, getter=getter, setter=setter) -@dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str - _: KW_ONLY - update_period: float | None = 0.2 - - -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): - def __init__(self, connection: IPConnection): - super().__init__() - - self._connection = connection - - async def update(self, attr: AttrR[NumberT, TemperatureControllerAttributeIORef]): - query = f"{attr.io_ref.name}?" - response = await self._connection.send_query(f"{query}\r\n") - value = response.strip("\r\n") - - await attr.update(attr.dtype(value)) - - async def send( - self, attr: AttrW[NumberT, TemperatureControllerAttributeIORef], value: NumberT - ) -> None: - command = f"{attr.io_ref.name}={attr.dtype(value)}" - await self._connection.send_command(f"{command}\r\n") + return attributes class TemperatureRampController(Controller): @@ -97,13 +84,16 @@ def __init__( self, index: int, parameters: dict[str, TemperatureControllerParameter], - io: TemperatureControllerAttributeIO, + protocol: TemperatureProtocol, ): self._parameters = parameters - super().__init__(f"Ramp{index}", ios=[io]) + self._protocol = protocol + super().__init__(f"Ramp{index}") async def initialise(self): - for name, attribute in create_attributes(self._parameters).items(): + for name, attribute in create_attributes( + self._parameters, self._protocol + ).items(): self.add_attribute(name, attribute) @@ -111,9 +101,9 @@ class TemperatureController(Controller): def __init__(self, settings: IPConnectionSettings): self._ip_settings = settings self._connection = IPConnection() + self._protocol = TemperatureProtocol(self._connection) - self._io = TemperatureControllerAttributeIO(self._connection) - super().__init__(ios=[self._io]) + super().__init__() async def connect(self): await self._connection.connect(self._ip_settings) @@ -125,12 +115,12 @@ async def initialise(self): ramps_api = api.pop("Ramps") - for name, attribute in create_attributes(api).items(): + for name, attribute in create_attributes(api, self._protocol).items(): self.add_attribute(name, attribute) for idx, ramp_parameters in enumerate(ramps_api): ramp_controller = TemperatureRampController( - idx + 1, ramp_parameters, self._io + idx + 1, ramp_parameters, self._protocol ) await ramp_controller.initialise() self.add_sub_controller(f"Ramp{idx + 1:02d}", ramp_controller) diff --git a/docs/snippets/static07.py b/docs/snippets/static07.py index cac5549d3..3bd0e04f3 100644 --- a/docs/snippets/static07.py +++ b/docs/snippets/static07.py @@ -1,8 +1,6 @@ -from dataclasses import dataclass from pathlib import Path -from typing import TypeVar -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR +from fastcs.attributes import AttrR, Polled from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import String @@ -10,35 +8,19 @@ from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport -NumberT = TypeVar("NumberT", int, float) - - -@dataclass -class IDAttributeIORef(AttributeIORef): - update_period: float | None = 0.2 - - -class IDAttributeIO(AttributeIO[NumberT, IDAttributeIORef]): - def __init__(self, connection: IPConnection): - super().__init__() - - self._connection = connection - - async def update(self, attr: AttrR[NumberT, IDAttributeIORef]): - response = await self._connection.send_query("ID?\r\n") - value = response.strip("\r\n") - - await attr.update(attr.dtype(value)) - class TemperatureController(Controller): - device_id = AttrR(String(), io_ref=IDAttributeIORef()) - def __init__(self, settings: IPConnectionSettings): self._ip_settings = settings self._connection = IPConnection() - super().__init__(ios=[IDAttributeIO(self._connection)]) + super().__init__() + + self.device_id = AttrR(String(), getter=Polled(self._get_device_id, period=0.2)) + + async def _get_device_id(self) -> str: + response = await self._connection.send_query("ID?\r\n") + return response.strip("\r\n") async def connect(self): await self._connection.connect(self._ip_settings) diff --git a/docs/snippets/static08.py b/docs/snippets/static08.py index 5382fa3c9..f2483679e 100644 --- a/docs/snippets/static08.py +++ b/docs/snippets/static08.py @@ -1,8 +1,7 @@ -from dataclasses import KW_ONLY, dataclass from pathlib import Path from typing import TypeVar -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR +from fastcs.attributes import AttrR, Polled from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import Float, String @@ -10,41 +9,40 @@ from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport -NumberT = TypeVar("NumberT", int, float) +ValueT = TypeVar("ValueT") -@dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str - _: KW_ONLY - update_period: float | None = 0.2 - - -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): - def __init__(self, connection: IPConnection): - super().__init__() - +class TemperatureProtocol: + def __init__(self, connection: IPConnection, suffix: str = ""): self._connection = connection + self._suffix = suffix - async def update(self, attr: AttrR[NumberT, TemperatureControllerAttributeIORef]): - query = f"{attr.io_ref.name}?" - response = await self._connection.send_query(f"{query}\r\n") - value = response.strip("\r\n") + async def send_command(self, param: str, value: ValueT, dtype: type[ValueT]): + command = f"{param}{self._suffix}={dtype(value)}" # type: ignore[call-arg] + await self._connection.send_command(f"{command}\r\n") - await attr.update(attr.dtype(value)) + async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: + query = f"{param}{self._suffix}?" + response = await self._connection.send_query(f"{query}\r\n") + return dtype(response.strip("\r\n")) # type: ignore[call-arg] class TemperatureController(Controller): - device_id = AttrR(String(), io_ref=TemperatureControllerAttributeIORef("ID")) - power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) - def __init__(self, settings: IPConnectionSettings): self._ip_settings = settings self._connection = IPConnection() + self._protocol = TemperatureProtocol(self._connection) + + super().__init__() + + self.device_id = AttrR(String(), getter=Polled(self._get_device_id, period=0.2)) + self.power = AttrR(Float(), getter=Polled(self._get_power, period=0.2)) + + async def _get_device_id(self) -> str: + return await self._protocol.send_query("ID", str) - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + async def _get_power(self) -> float: + return await self._protocol.send_query("P", float) async def connect(self): await self._connection.connect(self._ip_settings) diff --git a/docs/snippets/static09.py b/docs/snippets/static09.py index dc5bb9d54..1a75ea678 100644 --- a/docs/snippets/static09.py +++ b/docs/snippets/static09.py @@ -1,8 +1,7 @@ -from dataclasses import KW_ONLY, dataclass from pathlib import Path from typing import TypeVar -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, Polled from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import Float, String @@ -10,48 +9,51 @@ from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport -NumberT = TypeVar("NumberT", int, float) +ValueT = TypeVar("ValueT") -@dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str - _: KW_ONLY - update_period: float | None = 0.2 - - -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): - def __init__(self, connection: IPConnection): - super().__init__() - +class TemperatureProtocol: + def __init__(self, connection: IPConnection, suffix: str = ""): self._connection = connection + self._suffix = suffix - async def update(self, attr: AttrR[NumberT, TemperatureControllerAttributeIORef]): - query = f"{attr.io_ref.name}?" - response = await self._connection.send_query(f"{query}\r\n") - value = response.strip("\r\n") - - await attr.update(attr.dtype(value)) - - async def send( - self, attr: AttrW[NumberT, TemperatureControllerAttributeIORef], value: NumberT - ) -> None: - command = f"{attr.io_ref.name}={attr.dtype(value)}" + async def send_command(self, param: str, value: ValueT, dtype: type[ValueT]): + command = f"{param}{self._suffix}={dtype(value)}" # type: ignore[call-arg] await self._connection.send_command(f"{command}\r\n") + async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: + query = f"{param}{self._suffix}?" + response = await self._connection.send_query(f"{query}\r\n") + return dtype(response.strip("\r\n")) # type: ignore[call-arg] + class TemperatureController(Controller): - device_id = AttrR(String(), io_ref=TemperatureControllerAttributeIORef("ID")) - power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) - ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) - def __init__(self, settings: IPConnectionSettings): self._ip_settings = settings self._connection = IPConnection() + self._protocol = TemperatureProtocol(self._connection) + + super().__init__() + + self.device_id = AttrR(String(), getter=Polled(self._get_device_id, period=0.2)) + self.power = AttrR(Float(), getter=Polled(self._get_power, period=0.2)) + self.ramp_rate = AttrRW( + Float(), + getter=Polled(self._get_ramp_rate, period=0.2), + setter=self._set_ramp_rate, + ) + + async def _get_device_id(self) -> str: + return await self._protocol.send_query("ID", str) + + async def _get_power(self) -> float: + return await self._protocol.send_query("P", float) + + async def _get_ramp_rate(self) -> float: + return await self._protocol.send_query("R", float) - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + async def _set_ramp_rate(self, value: float) -> None: + await self._protocol.send_command("R", value, float) async def connect(self): await self._connection.connect(self._ip_settings) diff --git a/docs/snippets/static10.py b/docs/snippets/static10.py index 24f6d523d..3c02c402e 100644 --- a/docs/snippets/static10.py +++ b/docs/snippets/static10.py @@ -1,8 +1,7 @@ -from dataclasses import KW_ONLY, dataclass from pathlib import Path from typing import TypeVar -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, Polled from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import Float, Int, String @@ -10,60 +9,69 @@ from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport -NumberT = TypeVar("NumberT", int, float) +ValueT = TypeVar("ValueT") -@dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str - _: KW_ONLY - update_period: float | None = 0.2 - - -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): +class TemperatureProtocol: def __init__(self, connection: IPConnection, suffix: str = ""): - super().__init__() - self._connection = connection self._suffix = suffix - async def update(self, attr: AttrR[NumberT, TemperatureControllerAttributeIORef]): - query = f"{attr.io_ref.name}{self._suffix}?" - response = await self._connection.send_query(f"{query}\r\n") - value = response.strip("\r\n") - - await attr.update(attr.dtype(value)) - - async def send( - self, attr: AttrW[NumberT, TemperatureControllerAttributeIORef], value: NumberT - ) -> None: - command = f"{attr.io_ref.name}{self._suffix}={attr.dtype(value)}" + async def send_command(self, param: str, value: ValueT, dtype: type[ValueT]): + command = f"{param}{self._suffix}={dtype(value)}" # type: ignore[call-arg] await self._connection.send_command(f"{command}\r\n") + async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: + query = f"{param}{self._suffix}?" + response = await self._connection.send_query(f"{query}\r\n") + return dtype(response.strip("\r\n")) # type: ignore[call-arg] -class TemperatureRampController(Controller): - start = AttrRW(Int(), io_ref=TemperatureControllerAttributeIORef(name="S")) - end = AttrRW(Int(), io_ref=TemperatureControllerAttributeIORef(name="E")) +class TemperatureRampController(Controller): def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" - super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + self._protocol = TemperatureProtocol(connection, suffix) + super().__init__(f"Ramp{suffix}") + + self.start = AttrRW( + Int(), + getter=Polled(self._get_start, period=0.2), + setter=self._set_start, + ) + self.end = AttrRW( + Int(), + getter=Polled(self._get_end, period=0.2), + setter=self._set_end, ) + async def _get_start(self) -> int: + return await self._protocol.send_query("S", int) -class TemperatureController(Controller): - device_id = AttrR(String(), io_ref=TemperatureControllerAttributeIORef("ID")) - power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) - ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) + async def _set_start(self, value: int) -> None: + await self._protocol.send_command("S", value, int) + + async def _get_end(self) -> int: + return await self._protocol.send_query("E", int) + + async def _set_end(self, value: int) -> None: + await self._protocol.send_command("E", value, int) + +class TemperatureController(Controller): def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ip_settings = settings self._connection = IPConnection() + self._protocol = TemperatureProtocol(self._connection) + + super().__init__() - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + self.device_id = AttrR(String(), getter=Polled(self._get_device_id, period=0.2)) + self.power = AttrR(Float(), getter=Polled(self._get_power, period=0.2)) + self.ramp_rate = AttrRW( + Float(), + getter=Polled(self._get_ramp_rate, period=0.2), + setter=self._set_ramp_rate, + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): @@ -71,6 +79,18 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) + async def _get_device_id(self) -> str: + return await self._protocol.send_query("ID", str) + + async def _get_power(self) -> float: + return await self._protocol.send_query("P", float) + + async def _get_ramp_rate(self) -> float: + return await self._protocol.send_query("R", float) + + async def _set_ramp_rate(self, value: float) -> None: + await self._protocol.send_command("R", value, float) + async def connect(self): await self._connection.connect(self._ip_settings) diff --git a/docs/snippets/static11.py b/docs/snippets/static11.py index e07ed5a41..6dd4359f0 100644 --- a/docs/snippets/static11.py +++ b/docs/snippets/static11.py @@ -1,9 +1,8 @@ import enum -from dataclasses import KW_ONLY, dataclass from pathlib import Path from typing import TypeVar -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, Polled from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import Enum, Float, Int, String @@ -11,38 +10,23 @@ from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport -NumberT = TypeVar("NumberT", int, float) +ValueT = TypeVar("ValueT") -@dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str - _: KW_ONLY - update_period: float | None = 0.2 - - -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): +class TemperatureProtocol: def __init__(self, connection: IPConnection, suffix: str = ""): - super().__init__() - self._connection = connection self._suffix = suffix - async def update(self, attr: AttrR[NumberT, TemperatureControllerAttributeIORef]): - query = f"{attr.io_ref.name}{self._suffix}?" - response = await self._connection.send_query(f"{query}\r\n") - value = response.strip("\r\n") - - await attr.update(attr.dtype(value)) - - async def send( - self, attr: AttrW[NumberT, TemperatureControllerAttributeIORef], value: NumberT - ) -> None: - command = f"{attr.io_ref.name}{self._suffix}={attr.dtype(value)}" + async def send_command(self, param: str, value: ValueT, dtype: type[ValueT]): + command = f"{param}{self._suffix}={dtype(value)}" # type: ignore[call-arg] await self._connection.send_command(f"{command}\r\n") + async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: + query = f"{param}{self._suffix}?" + response = await self._connection.send_query(f"{query}\r\n") + return dtype(response.strip("\r\n")) # type: ignore[call-arg] + class OnOffEnum(enum.StrEnum): Off = "0" @@ -50,27 +34,61 @@ class OnOffEnum(enum.StrEnum): 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("N")) - def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" - super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + self._protocol = TemperatureProtocol(connection, suffix) + super().__init__(f"Ramp{suffix}") + + self.start = AttrRW( + Int(), + getter=Polled(self._get_start, period=0.2), + setter=self._set_start, + ) + self.end = AttrRW( + Int(), + getter=Polled(self._get_end, period=0.2), + setter=self._set_end, + ) + self.enabled = AttrRW( + Enum(OnOffEnum), + getter=Polled(self._get_enabled, period=0.2), + setter=self._set_enabled, ) + async def _get_start(self) -> int: + return await self._protocol.send_query("S", int) -class TemperatureController(Controller): - device_id = AttrR(String(), io_ref=TemperatureControllerAttributeIORef("ID")) - power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) - ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) + async def _set_start(self, value: int) -> None: + await self._protocol.send_command("S", value, int) + async def _get_end(self) -> int: + return await self._protocol.send_query("E", int) + + async def _set_end(self, value: int) -> None: + await self._protocol.send_command("E", value, int) + + async def _get_enabled(self) -> OnOffEnum: + return OnOffEnum(await self._protocol.send_query("N", str)) + + async def _set_enabled(self, value: OnOffEnum) -> None: + await self._protocol.send_command("N", value.value, str) + + +class TemperatureController(Controller): def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ip_settings = settings self._connection = IPConnection() + self._protocol = TemperatureProtocol(self._connection) - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__() + + self.device_id = AttrR(String(), getter=Polled(self._get_device_id, period=0.2)) + self.power = AttrR(Float(), getter=Polled(self._get_power, period=0.2)) + self.ramp_rate = AttrRW( + Float(), + getter=Polled(self._get_ramp_rate, period=0.2), + setter=self._set_ramp_rate, + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): @@ -78,6 +96,18 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) + async def _get_device_id(self) -> str: + return await self._protocol.send_query("ID", str) + + async def _get_power(self) -> float: + return await self._protocol.send_query("P", float) + + async def _get_ramp_rate(self) -> float: + return await self._protocol.send_query("R", float) + + async def _set_ramp_rate(self, value: float) -> None: + await self._protocol.send_command("R", value, float) + async def connect(self): await self._connection.connect(self._ip_settings) diff --git a/docs/snippets/static12.py b/docs/snippets/static12.py index 9f1f2af14..818f0337c 100644 --- a/docs/snippets/static12.py +++ b/docs/snippets/static12.py @@ -1,10 +1,9 @@ import enum import json -from dataclasses import KW_ONLY, dataclass from pathlib import Path from typing import TypeVar -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, Polled from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import Enum, Float, Int, String @@ -13,38 +12,23 @@ from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport -NumberT = TypeVar("NumberT", int, float) +ValueT = TypeVar("ValueT") -@dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str - _: KW_ONLY - update_period: float | None = 0.2 - - -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): +class TemperatureProtocol: def __init__(self, connection: IPConnection, suffix: str = ""): - super().__init__() - self._connection = connection self._suffix = suffix - async def update(self, attr: AttrR[NumberT, TemperatureControllerAttributeIORef]): - query = f"{attr.io_ref.name}{self._suffix}?" - response = await self._connection.send_query(f"{query}\r\n") - value = response.strip("\r\n") - - await attr.update(attr.dtype(value)) - - async def send( - self, attr: AttrW[NumberT, TemperatureControllerAttributeIORef], value: NumberT - ) -> None: - command = f"{attr.io_ref.name}{self._suffix}={attr.dtype(value)}" + async def send_command(self, param: str, value: ValueT, dtype: type[ValueT]): + command = f"{param}{self._suffix}={dtype(value)}" # type: ignore[call-arg] await self._connection.send_command(f"{command}\r\n") + async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: + query = f"{param}{self._suffix}?" + response = await self._connection.send_query(f"{query}\r\n") + return dtype(response.strip("\r\n")) # type: ignore[call-arg] + class OnOffEnum(enum.StrEnum): Off = "0" @@ -52,30 +36,70 @@ class OnOffEnum(enum.StrEnum): 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("N")) - target = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("T")) - actual = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("A")) - voltage = AttrR(Float()) - def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" - super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + self._protocol = TemperatureProtocol(connection, suffix) + super().__init__(f"Ramp{suffix}") + + self.start = AttrRW( + Int(), + getter=Polled(self._get_start, period=0.2), + setter=self._set_start, + ) + self.end = AttrRW( + Int(), + getter=Polled(self._get_end, period=0.2), + setter=self._set_end, ) + self.enabled = AttrRW( + Enum(OnOffEnum), + getter=Polled(self._get_enabled, period=0.2), + setter=self._set_enabled, + ) + self.target = AttrR(Float(), getter=Polled(self._get_target, period=0.2)) + self.actual = AttrR(Float(), getter=Polled(self._get_actual, period=0.2)) + self.voltage = AttrR(Float()) + async def _get_start(self) -> int: + return await self._protocol.send_query("S", int) -class TemperatureController(Controller): - device_id = AttrR(String(), io_ref=TemperatureControllerAttributeIORef("ID")) - power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) - ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) + async def _set_start(self, value: int) -> None: + await self._protocol.send_command("S", value, int) + + async def _get_end(self) -> int: + return await self._protocol.send_query("E", int) + async def _set_end(self, value: int) -> None: + await self._protocol.send_command("E", value, int) + + async def _get_enabled(self) -> OnOffEnum: + return OnOffEnum(await self._protocol.send_query("N", str)) + + async def _set_enabled(self, value: OnOffEnum) -> None: + await self._protocol.send_command("N", value.value, str) + + async def _get_target(self) -> float: + return await self._protocol.send_query("T", float) + + async def _get_actual(self) -> float: + return await self._protocol.send_query("A", float) + + +class TemperatureController(Controller): def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ip_settings = settings self._connection = IPConnection() + self._protocol = TemperatureProtocol(self._connection) - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__() + + self.device_id = AttrR(String(), getter=Polled(self._get_device_id, period=0.2)) + self.power = AttrR(Float(), getter=Polled(self._get_power, period=0.2)) + self.ramp_rate = AttrRW( + Float(), + getter=Polled(self._get_ramp_rate, period=0.2), + setter=self._set_ramp_rate, + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): @@ -83,6 +107,18 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) + async def _get_device_id(self) -> str: + return await self._protocol.send_query("ID", str) + + async def _get_power(self) -> float: + return await self._protocol.send_query("P", float) + + async def _get_ramp_rate(self) -> float: + return await self._protocol.send_query("R", float) + + async def _set_ramp_rate(self, value: float) -> None: + await self._protocol.send_command("R", value, float) + async def connect(self): await self._connection.connect(self._ip_settings) diff --git a/docs/snippets/static13.py b/docs/snippets/static13.py index b2036c66f..9313a873a 100644 --- a/docs/snippets/static13.py +++ b/docs/snippets/static13.py @@ -1,11 +1,10 @@ import asyncio import enum import json -from dataclasses import KW_ONLY, dataclass from pathlib import Path from typing import TypeVar -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, Polled from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import Enum, Float, Int, String @@ -14,38 +13,23 @@ from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport -NumberT = TypeVar("NumberT", int, float) +ValueT = TypeVar("ValueT") -@dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str - _: KW_ONLY - update_period: float | None = 0.2 - - -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): +class TemperatureProtocol: def __init__(self, connection: IPConnection, suffix: str = ""): - super().__init__() - self._connection = connection self._suffix = suffix - async def update(self, attr: AttrR[NumberT, TemperatureControllerAttributeIORef]): - query = f"{attr.io_ref.name}{self._suffix}?" - response = await self._connection.send_query(f"{query}\r\n") - value = response.strip("\r\n") - - await attr.update(attr.dtype(value)) - - async def send( - self, attr: AttrW[NumberT, TemperatureControllerAttributeIORef], value: NumberT - ) -> None: - command = f"{attr.io_ref.name}{self._suffix}={attr.dtype(value)}" + async def send_command(self, param: str, value: ValueT, dtype: type[ValueT]): + command = f"{param}{self._suffix}={dtype(value)}" # type: ignore[call-arg] await self._connection.send_command(f"{command}\r\n") + async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: + query = f"{param}{self._suffix}?" + response = await self._connection.send_query(f"{query}\r\n") + return dtype(response.strip("\r\n")) # type: ignore[call-arg] + class OnOffEnum(enum.StrEnum): Off = "0" @@ -53,30 +37,70 @@ class OnOffEnum(enum.StrEnum): 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("N")) - target = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("T")) - actual = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("A")) - voltage = AttrR(Float()) - def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" - super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + self._protocol = TemperatureProtocol(connection, suffix) + super().__init__(f"Ramp{suffix}") + + self.start = AttrRW( + Int(), + getter=Polled(self._get_start, period=0.2), + setter=self._set_start, + ) + self.end = AttrRW( + Int(), + getter=Polled(self._get_end, period=0.2), + setter=self._set_end, ) + self.enabled = AttrRW( + Enum(OnOffEnum), + getter=Polled(self._get_enabled, period=0.2), + setter=self._set_enabled, + ) + self.target = AttrR(Float(), getter=Polled(self._get_target, period=0.2)) + self.actual = AttrR(Float(), getter=Polled(self._get_actual, period=0.2)) + self.voltage = AttrR(Float()) + async def _get_start(self) -> int: + return await self._protocol.send_query("S", int) -class TemperatureController(Controller): - device_id = AttrR(String(), io_ref=TemperatureControllerAttributeIORef("ID")) - power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) - ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) + async def _set_start(self, value: int) -> None: + await self._protocol.send_command("S", value, int) + + async def _get_end(self) -> int: + return await self._protocol.send_query("E", int) + async def _set_end(self, value: int) -> None: + await self._protocol.send_command("E", value, int) + + async def _get_enabled(self) -> OnOffEnum: + return OnOffEnum(await self._protocol.send_query("N", str)) + + async def _set_enabled(self, value: OnOffEnum) -> None: + await self._protocol.send_command("N", value.value, str) + + async def _get_target(self) -> float: + return await self._protocol.send_query("T", float) + + async def _get_actual(self) -> float: + return await self._protocol.send_query("A", float) + + +class TemperatureController(Controller): def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ip_settings = settings self._connection = IPConnection() + self._protocol = TemperatureProtocol(self._connection) - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__() + + self.device_id = AttrR(String(), getter=Polled(self._get_device_id, period=0.2)) + self.power = AttrR(Float(), getter=Polled(self._get_power, period=0.2)) + self.ramp_rate = AttrRW( + Float(), + getter=Polled(self._get_ramp_rate, period=0.2), + setter=self._set_ramp_rate, + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): @@ -84,6 +108,18 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) + async def _get_device_id(self) -> str: + return await self._protocol.send_query("ID", str) + + async def _get_power(self) -> float: + return await self._protocol.send_query("P", float) + + async def _get_ramp_rate(self) -> float: + return await self._protocol.send_query("R", float) + + async def _set_ramp_rate(self, value: float) -> None: + await self._protocol.send_command("R", value, float) + async def connect(self): await self._connection.connect(self._ip_settings) @@ -98,7 +134,7 @@ async def update_voltages(self): @command() async def disable_all(self) -> None: for rc in self._ramp_controllers: - await rc.enabled.put(OnOffEnum.Off, sync_setpoint=True) + await rc.enabled.set(OnOffEnum.Off) # TODO: The requests all get concatenated and the sim doesn't handle it await asyncio.sleep(0.1) diff --git a/docs/snippets/static14.py b/docs/snippets/static14.py index f54c93d3d..8af326fbd 100644 --- a/docs/snippets/static14.py +++ b/docs/snippets/static14.py @@ -1,11 +1,10 @@ import asyncio import enum import json -from dataclasses import KW_ONLY, dataclass from pathlib import Path from typing import TypeVar -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, Polled from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import Enum, Float, Int, String @@ -15,40 +14,26 @@ from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport -NumberT = TypeVar("NumberT", int, float) +ValueT = TypeVar("ValueT") -@dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str - _: KW_ONLY - update_period: float | None = 0.2 - - -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): +class TemperatureProtocol: def __init__(self, connection: IPConnection, suffix: str = ""): - super().__init__() - self._connection = connection self._suffix = suffix - async def update(self, attr: AttrR[NumberT, TemperatureControllerAttributeIORef]): - query = f"{attr.io_ref.name}{self._suffix}?" - response = await self._connection.send_query(f"{query}\r\n") - value = response.strip("\r\n") - await attr.update(attr.dtype(value)) - - async def send( - self, attr: AttrW[NumberT, TemperatureControllerAttributeIORef], value: NumberT - ) -> None: - command = f"{attr.io_ref.name}{self._suffix}={attr.dtype(value)}" + async def send_command(self, param: str, value: ValueT, dtype: type[ValueT]): + command = f"{param}{self._suffix}={dtype(value)}" # type: ignore[call-arg] - logger.info("Sending attribute value", command=command, attribute=attr) + logger.info("Sending attribute value", command=command) await self._connection.send_command(f"{command}\r\n") + async def send_query(self, param: str, dtype: type[ValueT]) -> ValueT: + query = f"{param}{self._suffix}?" + response = await self._connection.send_query(f"{query}\r\n") + return dtype(response.strip("\r\n")) # type: ignore[call-arg] + class OnOffEnum(enum.StrEnum): Off = "0" @@ -56,30 +41,70 @@ class OnOffEnum(enum.StrEnum): 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("N")) - target = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("T")) - actual = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("A")) - voltage = AttrR(Float()) - def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" - super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + self._protocol = TemperatureProtocol(connection, suffix) + super().__init__(f"Ramp{suffix}") + + self.start = AttrRW( + Int(), + getter=Polled(self._get_start, period=0.2), + setter=self._set_start, + ) + self.end = AttrRW( + Int(), + getter=Polled(self._get_end, period=0.2), + setter=self._set_end, ) + self.enabled = AttrRW( + Enum(OnOffEnum), + getter=Polled(self._get_enabled, period=0.2), + setter=self._set_enabled, + ) + self.target = AttrR(Float(), getter=Polled(self._get_target, period=0.2)) + self.actual = AttrR(Float(), getter=Polled(self._get_actual, period=0.2)) + self.voltage = AttrR(Float()) + async def _get_start(self) -> int: + return await self._protocol.send_query("S", int) -class TemperatureController(Controller): - device_id = AttrR(String(), io_ref=TemperatureControllerAttributeIORef("ID")) - power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) - ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) + async def _set_start(self, value: int) -> None: + await self._protocol.send_command("S", value, int) + + async def _get_end(self) -> int: + return await self._protocol.send_query("E", int) + async def _set_end(self, value: int) -> None: + await self._protocol.send_command("E", value, int) + + async def _get_enabled(self) -> OnOffEnum: + return OnOffEnum(await self._protocol.send_query("N", str)) + + async def _set_enabled(self, value: OnOffEnum) -> None: + await self._protocol.send_command("N", value.value, str) + + async def _get_target(self) -> float: + return await self._protocol.send_query("T", float) + + async def _get_actual(self) -> float: + return await self._protocol.send_query("A", float) + + +class TemperatureController(Controller): def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ip_settings = settings self._connection = IPConnection() + self._protocol = TemperatureProtocol(self._connection) - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__() + + self.device_id = AttrR(String(), getter=Polled(self._get_device_id, period=0.2)) + self.power = AttrR(Float(), getter=Polled(self._get_power, period=0.2)) + self.ramp_rate = AttrRW( + Float(), + getter=Polled(self._get_ramp_rate, period=0.2), + setter=self._set_ramp_rate, + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): @@ -87,6 +112,18 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) + async def _get_device_id(self) -> str: + return await self._protocol.send_query("ID", str) + + async def _get_power(self) -> float: + return await self._protocol.send_query("P", float) + + async def _get_ramp_rate(self) -> float: + return await self._protocol.send_query("R", float) + + async def _set_ramp_rate(self, value: float) -> None: + await self._protocol.send_command("R", value, float) + async def connect(self): await self._connection.connect(self._ip_settings) @@ -102,7 +139,7 @@ async def update_voltages(self): async def disable_all(self) -> None: self.log_event("Disabling all ramps") for rc in self._ramp_controllers: - await rc.enabled.put(OnOffEnum.Off, sync_setpoint=True) + await rc.enabled.set(OnOffEnum.Off) # TODO: The requests all get concatenated and the sim doesn't handle it await asyncio.sleep(0.1) diff --git a/docs/snippets/static15.py b/docs/snippets/static15.py index aa2d53a92..35a244115 100644 --- a/docs/snippets/static15.py +++ b/docs/snippets/static15.py @@ -1,56 +1,46 @@ import asyncio import enum import json -from dataclasses import KW_ONLY, dataclass from pathlib import Path from typing import TypeVar -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, Polled from fastcs.connections import IPConnection, IPConnectionSettings from fastcs.controllers import Controller from fastcs.datatypes import Enum, Float, Int, String from fastcs.launch import FastCS from fastcs.logging import LogLevel, configure_logging, logger from fastcs.methods import command, scan +from fastcs.tracer import Tracer from fastcs.transports.epics import EpicsGUIOptions from fastcs.transports.epics.ca import EpicsCATransport -NumberT = TypeVar("NumberT", int, float) +ValueT = TypeVar("ValueT") -@dataclass -class TemperatureControllerAttributeIORef(AttributeIORef): - name: str - _: KW_ONLY - update_period: float | None = 0.2 - - -class TemperatureControllerAttributeIO( - AttributeIO[NumberT, TemperatureControllerAttributeIORef] -): +class TemperatureProtocol(Tracer): def __init__(self, connection: IPConnection, suffix: str = ""): super().__init__() - self._connection = connection self._suffix = suffix - async def update(self, attr: AttrR[NumberT, TemperatureControllerAttributeIORef]): - query = f"{attr.io_ref.name}{self._suffix}?" - response = await self._connection.send_query(f"{query}\r\n") - value = response.strip("\r\n") + async def send_command(self, param: str, value: ValueT, dtype: type[ValueT]): + command = f"{param}{self._suffix}={dtype(value)}" # type: ignore[call-arg] - self.log_event("Query for attribute", query=query, response=value, topic=attr) + logger.info("Sending attribute value", command=command) - await attr.update(attr.dtype(value)) + await self._connection.send_command(f"{command}\r\n") - async def send( - self, attr: AttrW[NumberT, TemperatureControllerAttributeIORef], value: NumberT - ) -> None: - command = f"{attr.io_ref.name}{self._suffix}={attr.dtype(value)}" + async def send_query( + self, param: str, dtype: type[ValueT], topic: Tracer | None = None + ) -> ValueT: + query = f"{param}{self._suffix}?" + response = await self._connection.send_query(f"{query}\r\n") + value = dtype(response.strip("\r\n")) # type: ignore[call-arg] - logger.info("Sending attribute value", command=command, attribute=attr) + self.log_event("Query for attribute", topic=topic, query=query, response=value) - await self._connection.send_command(f"{command}\r\n") + return value class OnOffEnum(enum.StrEnum): @@ -59,30 +49,70 @@ class OnOffEnum(enum.StrEnum): 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("N")) - target = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("T")) - actual = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("A")) - voltage = AttrR(Float()) - def __init__(self, index: int, connection: IPConnection) -> None: suffix = f"{index:02d}" - super().__init__( - f"Ramp{suffix}", ios=[TemperatureControllerAttributeIO(connection, suffix)] + self._protocol = TemperatureProtocol(connection, suffix) + super().__init__(f"Ramp{suffix}") + + self.start = AttrRW( + Int(), + getter=Polled(self._get_start, period=0.2), + setter=self._set_start, + ) + self.end = AttrRW( + Int(), + getter=Polled(self._get_end, period=0.2), + setter=self._set_end, + ) + self.enabled = AttrRW( + Enum(OnOffEnum), + getter=Polled(self._get_enabled, period=0.2), + setter=self._set_enabled, ) + self.target = AttrR(Float(), getter=Polled(self._get_target, period=0.2)) + self.actual = AttrR(Float(), getter=Polled(self._get_actual, period=0.2)) + self.voltage = AttrR(Float()) + async def _get_start(self) -> int: + return await self._protocol.send_query("S", int, topic=self.start) -class TemperatureController(Controller): - device_id = AttrR(String(), io_ref=TemperatureControllerAttributeIORef("ID")) - power = AttrR(Float(), io_ref=TemperatureControllerAttributeIORef("P")) - ramp_rate = AttrRW(Float(), io_ref=TemperatureControllerAttributeIORef("R")) + async def _set_start(self, value: int) -> None: + await self._protocol.send_command("S", value, int) + + async def _get_end(self) -> int: + return await self._protocol.send_query("E", int, topic=self.end) + + async def _set_end(self, value: int) -> None: + await self._protocol.send_command("E", value, int) + async def _get_enabled(self) -> OnOffEnum: + return OnOffEnum(await self._protocol.send_query("N", str, topic=self.enabled)) + + async def _set_enabled(self, value: OnOffEnum) -> None: + await self._protocol.send_command("N", value.value, str) + + async def _get_target(self) -> float: + return await self._protocol.send_query("T", float, topic=self.target) + + async def _get_actual(self) -> float: + return await self._protocol.send_query("A", float, topic=self.actual) + + +class TemperatureController(Controller): def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ip_settings = settings self._connection = IPConnection() + self._protocol = TemperatureProtocol(self._connection) - super().__init__(ios=[TemperatureControllerAttributeIO(self._connection)]) + super().__init__() + + self.device_id = AttrR(String(), getter=Polled(self._get_device_id, period=0.2)) + self.power = AttrR(Float(), getter=Polled(self._get_power, period=0.2)) + self.ramp_rate = AttrRW( + Float(), + getter=Polled(self._get_ramp_rate, period=0.2), + setter=self._set_ramp_rate, + ) self._ramp_controllers: list[TemperatureRampController] = [] for index in range(1, ramp_count + 1): @@ -90,6 +120,18 @@ def __init__(self, ramp_count: int, settings: IPConnectionSettings): self._ramp_controllers.append(controller) self.add_sub_controller(f"R{index}", controller) + async def _get_device_id(self) -> str: + return await self._protocol.send_query("ID", str, topic=self.device_id) + + async def _get_power(self) -> float: + return await self._protocol.send_query("P", float, topic=self.power) + + async def _get_ramp_rate(self) -> float: + return await self._protocol.send_query("R", float, topic=self.ramp_rate) + + async def _set_ramp_rate(self, value: float) -> None: + await self._protocol.send_command("R", value, float) + async def connect(self): await self._connection.connect(self._ip_settings) @@ -105,7 +147,7 @@ async def update_voltages(self): async def disable_all(self) -> None: self.log_event("Disabling all ramps") for rc in self._ramp_controllers: - await rc.enabled.put(OnOffEnum.Off, sync_setpoint=True) + await rc.enabled.set(OnOffEnum.Off) # TODO: The requests all get concatenated and the sim doesn't handle it await asyncio.sleep(0.1) diff --git a/docs/tutorials/dynamic-drivers.md b/docs/tutorials/dynamic-drivers.md index 8b02b1e7b..ae55dc608 100644 --- a/docs/tutorials/dynamic-drivers.md +++ b/docs/tutorials/dynamic-drivers.md @@ -43,27 +43,27 @@ implement an `initialise` method to create these dynamically instead. Create a pydantic model to validate the response from the device :::{literalinclude} /snippets/dynamic.py -:lines: 5,18-35 +:lines: 4,30-47 ::: Create a function to parse the dictionary, validate the entries against the model and -create `Attributes`. +create `Attributes`. Each attribute gets a `getter` (and, if writable, a `setter`) built +as a small closure over its `command` and the shared `TemperatureProtocol` instance, +rather than an IO reference - dynamically-created attributes need their IO wired up at +construction time just like statically-declared ones do. :::{literalinclude} /snippets/dynamic.py -:lines: 38-56 +:lines: 50-79 ::: Update the controllers to not define attributes statically and implement initialise -methods to create these attributes dynamically. +methods to create these attributes dynamically, passing the shared `TemperatureProtocol` +down to `create_attributes` so the dynamically-created getters/setters can use it. :::{literalinclude} /snippets/dynamic.py -:lines: 91-131 +:lines: 82-128 ::: -The `suffix` field should also be removed from `TemperatureController` and -`TemperatureRampController` and then not used in `TemperatureControllerAttributeIO` -because the `command` field on `TemperatureControllerParameter` includes this. - TODO: Add `enabled` back in to `TemperatureRampController` and recreate `disable_all` to demonstrate validation of introspected Attributes. diff --git a/docs/tutorials/static-drivers.md b/docs/tutorials/static-drivers.md index f28f9b603..aa447c13f 100644 --- a/docs/tutorials/static-drivers.md +++ b/docs/tutorials/static-drivers.md @@ -82,7 +82,7 @@ In [1]: controller.device_id Out[1]: AttrR(String()) -In [2]: controller.device_id.get() +In [2]: controller.device_id.readback Out[2]: '' ::: @@ -139,8 +139,10 @@ The `demo.bob` will have been created in the directory the application was run f ## FastCS Device Connection The `Attributes` of a FastCS `Controller` need some IO with the device in order to get -and set values. This is implemented with `AttributeIO`s and connections. Generally each -driver implements its own IO and connection logic, but there are some built in options. +and set values. This is implemented with plain `getter`/`setter` callables passed to the +`Attribute` constructor, together with a connection. Generally each driver implements +its own getter/setter logic and connection, but there are some built in connection +options. Update the controller to create an `IPConnection` to communicate with the simulator over TCP and implement a `connect` method that establishes the connection. The `connect` @@ -165,27 +167,29 @@ The application will now fail to connect if the demo simulation is not running. ::: The `Controller` has now established a connection with the simulator. This connection -can be passed to an `AttributeIO` to enable it to query the device API and update the -value in the `device_id` attribute. Create a `TemperatureControllerAttributeIO` child -class and implement the `update` method to query the device and set the value of the -attribute, and create a `TemperatureControllerAttributeIORef` and pass an instance of -it to the `device_id` attribute to tell the controller what io to use to update it. +can be used by a `getter` callable to query the device API and update the value in the +`device_id` attribute. Note that the `Attribute` now has to be created in `__init__`, +after the connection exists, rather than as a class body instance - a getter needs to +close over a live connection, which doesn't exist yet when the class body is evaluated. +Write a `_get_device_id` method that queries the device and returns its value, and pass +it to `device_id` as `getter`. :::{note} -The `update_period` property tells the base class how often to call `update` +Passing the getter bare, as here, means it is called once at start up. Wrap it in +`Polled(getter, period=...)` to have the base class call it repeatedly instead. ::: ::::{admonition} Code 7 :class: dropdown, hint :::{literalinclude} /snippets/static07.py -:emphasize-lines: 1,3,5-6,15-33,37,43 +:emphasize-lines: 13-19,21-23 ::: :::: :::{note} -In the `update` method, errors won't crash the application, but it prints them to the +If a getter raises, it won't crash the application, but it prints the error to the terminal. - `Update loop ... stopped:` ::: @@ -201,26 +205,28 @@ DEMO:DeviceId SIMTCONT123 The simulator supports many other commands, for example it reports the total power currently being drawn with the `P` command. This can be exposed by adding another -`AttrR` with a `Float` datatype, but the IO only supports the `ID` command to get the -device ID. This new attribute could have its own IO, but it is similar enough that the -existing IO can be support both. +`AttrR` with a `Float` datatype, but so far the getter for `device_id` only knows how to +send the `ID` command. This new attribute could get its own bespoke getter, but the +query-building logic is similar enough between commands that it is worth factoring out. -Modify the IO ref to take a `name` string and update the IO to use it in the query -string sent to the device. Create a new attribute to read the power usage using this. +Extract a small `TemperatureProtocol` class that knows how to send a query or a command +for a given parameter name, casting the response to the right python type. Each +attribute then gets a thin getter method that just names the parameter and delegates to +the protocol. :::{note} All responses from the `IPConnection` are strings. This is fine for the `ID` command -because the value is actually a string, but for `P` the value is a float, so the -`update` methods needs to explicitly cast to the correct type. It can use -`Attribute.dtype` to call the builtin for its datatype - e.g. `int`, `float`, `str`, -etc. +because the value is actually a string, but for `P` the value is a float, so +`TemperatureProtocol.send_query` needs to explicitly cast to the correct type. It takes +the target python type as an argument (e.g. `int`, `float`, `str`) and calls it as a +constructor to perform the cast. ::: :::{admonition} Code 8 :class: dropdown, hint :::{literalinclude} /snippets/static08.py -:emphasize-lines: 10,19-21,33-38,42-43 +:emphasize-lines: 12,15-27,34,38-39,41-45 ::: :::: @@ -229,14 +235,14 @@ Now the IOC has two PVs being polled periodically. The new PV will be visible in Phoebus UI on refresh (right-click). `DEMO:Power` will read as `0` because the simulator is not currently running a ramp. To do that the controller needs to be able to set values on the device, as well as read them back. The ramp rate of the temperature can be -read with the `R` command and set with the `R=...` command. This means the IO also needs -a `send` method to send values to the device. +read with the `R` command and set with the `R=...` command. This means the protocol also +needs a way to send values to the device, which `send_command` already provides. -Update the IO to implement `send` and then add a new `AttrRW` with type `Float` to get -and set the ramp rate. +Add a new `AttrRW` with type `Float` to get and set the ramp rate, giving it both a +`getter` and a `setter`. :::{note} -The set commands do not return a response, so use the `send_command` method instead of +The set commands do not return a response, so the setter uses `send_command` instead of `send_query`. ::: @@ -244,7 +250,7 @@ The set commands do not return a response, so use the `send_command` method inst :class: dropdown, hint :::{literalinclude} /snippets/static09.py -:emphasize-lines: 7,40-44,48-50 +:emphasize-lines: 4,40-45,53-57 ::: :::: @@ -279,16 +285,17 @@ has. This can be done with the use of sub controllers. Controllers can be arbitr nested to match the structure of a device and this structure is then mirrored to the transport layer for the visibility of the user. -Create a `TemperatureRampController` with two `AttrRW`s the ramp start and end, update -the IO to include an optional suffix for the commands so that it can be shared with -the parent `TemperatureController` and add an argument to define how many ramps there -are, which is used to register the correct number of ramp controllers with the parent. +Create a `TemperatureRampController` with two `AttrRW`s for the ramp start and end, give +`TemperatureProtocol` an optional suffix so an instance can be shared with the parent +`TemperatureController` while still addressing an individual ramp, and add an argument +to define how many ramps there are, which is used to register the correct number of ramp +controllers with the parent. ::::{admonition} Code 10 :class: dropdown, hint :::{literalinclude} /snippets/static10.py -:emphasize-lines: 10,28,32,35,44,48-56,64,70-74,83 +:emphasize-lines: 30-53,57,73-77 ::: :::: @@ -313,7 +320,7 @@ Add an `AttrRW` to the `TemperatureRampController`s with an `Enum` type, using a :class: dropdown, hint :::{literalinclude} /snippets/static11.py -:emphasize-lines: 1,11,49-51,57 +:emphasize-lines: 1,31-33,48-53,67-71 ::: :::: @@ -355,39 +362,41 @@ The applied voltage for each ramp is also available with the `V?` command, but t is an array with each element corresponding to a ramp. Here it will be simplest to manually fetch the array in the parent controller and pass each value into ramp controller. This can be done with a `scan` method - these are called at a defined rate, -similar to the `update` method of an `AttributeIO`. +similar to how each attribute's getter is polled. -Add an `AttrR` for the voltage to the `TemperatureRampController`, but do not pass it an -IO ref. Then add a method to the `TemperatureController` with a `@scan` decorator that -gets the array of voltages and sets each ramp controller with its value. Also add -`AttrR`s for the target and actual temperature for each ramp as described above. +Add an `AttrR` for the voltage to the `TemperatureRampController`, but do not give it a +`getter` - it is a soft attribute, pushed to directly by the parent controller's scan +method instead. Then add a method to the `TemperatureController` with a `@scan` +decorator that gets the array of voltages and sets each ramp controller with its value. +Also add `AttrR`s for the target and actual temperature for each ramp as described +above. ::::{admonition} Code 12 :class: dropdown, hint :::{literalinclude} /snippets/static12.py -:emphasize-lines: 2,16,60-62,91-97 +:emphasize-lines: 11,56-58,78-82,123-129 ::: :::: Creating attributes is intended to be a simple API covering most use cases, but where more flexibility is needed wrapped controller methods can be useful to avoid adding -complexity to the IO to handle a small subset of attributes. It is also useful for -implementing higher level logic on top of the attributes that expose the API of a device -directly. For example, it would be useful to have a single button to stop all of the -ramps at the same time. This can be done with a `command` method. These are similar to -`scan` methods except that they create an API in transport layer in the same way an +complexity to a getter/setter to handle a small subset of attributes. It is also useful +for implementing higher level logic on top of the attributes that expose the API of a +device directly. For example, it would be useful to have a single button to stop all of +the ramps at the same time. This can be done with a `command` method. These are similar +to `scan` methods except that they create an API in transport layer in the same way an attribute does. Add a method with a `@command` decorator to set enabled to false in every ramp -controller. +controller by calling `set` on each `enabled` attribute. ::::{admonition} Code 13 :class: dropdown, hint :::{literalinclude} /snippets/static13.py -:emphasize-lines: 1,17,100-105 +:emphasize-lines: 1,132-137 ::: :::: @@ -412,14 +421,14 @@ application. To enable logging from the core framework call `configure_logging` arguments (the default logging level is INFO). To log messages from a driver, import the singleton `logger` directly. -Create a module-level logger to log status of the application start up. Create a class -logger for `TemperatureControllerAttributeIO` to log the commands it sends. +Create a module-level logger to log status of the application start up, and use it +inside `TemperatureProtocol.send_command` to log the commands it sends. ::::{admonition} Code 14 :class: dropdown, hint :::{literalinclude} /snippets/static14.py -:emphasize-lines: 13,48,110,115 +:emphasize-lines: 12,28,145,150 ::: :::: @@ -427,55 +436,48 @@ logger for `TemperatureControllerAttributeIO` to log the commands it sends. Try setting a PV and check the console for the log message it prints. ``` -[2025-11-18 11:26:41.065+0000 I] Sending attribute value [TemperatureControllerAttributeIO] command=E01=70, attribute=AttrRW(path=R1.end, datatype=Int, io_ref=TemperatureControllerAttributeIORef(update_period=0.2, name='E')) +[2026-01-01 11:26:41.065+0000 I] Sending attribute value [fastcs] command=E01=70 ``` -A similar log message could be added for the update method of the IO, but this would be -very verbose. For this use case FastCS provides the `Tracer` class, which is inherited -by `AttributeIO`, among other core FastCS classes. This enables the logging of `TRACE` -level log messages that are disabled by default, but can be enabled at runtime. +A similar log message could be added for the getters, but this would be very verbose. +For this use case FastCS provides the `Tracer` class, which can be inherited by anything +that wants to support selective, per-instance logging - `Attribute` and `BaseController` +already do. This enables the logging of `TRACE` level log messages that are disabled by +default, but can be enabled at runtime. -Update the `send` method of the IO to log a message showing the query that was sent and -the response from the device. Update the `configure_logging` call to pass -`LogLevel.TRACE` as the log level, so that when tracing is enabled the messages are -visible. +Make `TemperatureProtocol` inherit `Tracer` too, and update `send_query` to take a +`topic` argument and log a message showing the query that was sent and the response +from the device via `self.log_event`, passing through the attribute doing the query as +the `topic`. Update each getter to pass its own attribute as `topic`. Update the +`configure_logging` call to pass `LogLevel.TRACE` as the log level, so that when tracing +is enabled the messages are visible. ::::{admonition} Code 15 :class: dropdown, hint :::{literalinclude} /snippets/static15.py -:emphasize-lines: 13,49-51,118 +:emphasize-lines: 12,14,21,34-36,41,125,153 ::: :::: Enable tracing on the `power` attribute by calling `enable_tracing` and then enable a -ramp so that the value updates. Check the console to see the messages. Call +ramp so that the value updates. Check the console to see the messages. Call `disable_tracing` to disable the log messages for `power`. ``` In [1]: controller.power.enable_tracing() -[2025-11-18 11:11:12.060+0000 T] Query for attribute [TemperatureControllerAttributeIO] query=P?, response=0.0 -[2025-11-18 11:11:12.060+0000 T] Attribute set [AttrR] attribute=AttrR(path=power, datatype=Float, io_ref=TemperatureControllerAttributeIORef(update_period=0.2, name='P')), value=0.0 -[2025-11-18 11:11:12.060+0000 T] PV set from attribute [fastcs.transports.epics.ca.ioc] pv=DEMO:Power, value=0.0 -[2025-11-18 11:11:12.194+0000 I] PV put: DEMO:R1:Enabled = 1 [fastcs.transports.epics.ca.ioc] pv=DEMO:R1:Enabled, value=1 -[2025-11-18 11:11:12.195+0000 I] Sending attribute value [TemperatureControllerAttributeIO] command=N01=1, attribute=AttrRW(path=R1.enabled, datatype=Enum, io_ref=TemperatureControllerAttributeIORef(update_period=0.2, name='N')) -[2025-11-18 11:11:12.261+0000 T] Update attribute [AttrR] -[2025-11-18 11:11:12.262+0000 T] Query for attribute [TemperatureControllerAttributeIO] query=P?, response=29.040181873093132 -[2025-11-18 11:11:12.262+0000 T] Attribute set [AttrR] attribute=AttrR(path=power, datatype=Float, io_ref=TemperatureControllerAttributeIORef(update_period=0.2, name='P')), value=29.040181873093132 -[2025-11-18 11:11:12.262+0000 T] PV set from attribute [fastcs.transports.epics.ca.ioc] pv=DEMO:Power, value=29.04 -[2025-11-18 11:11:12.463+0000 T] Update attribute [AttrR] -[2025-11-18 11:11:12.464+0000 T] Query for attribute [TemperatureControllerAttributeIO] query=P?, response=30.452524641833854 -[2025-11-18 11:11:12.464+0000 T] Attribute set [AttrR] attribute=AttrR(path=power, datatype=Float, io_ref=TemperatureControllerAttributeIORef(update_period=0.2, name='P')), value=30.452524641833854 -[2025-11-18 11:11:12.465+0000 T] PV set from attribute [fastcs.transports.epics.ca.ioc] pv=DEMO:Power, value=30.45 +[2026-01-01 11:11:12.060+0000 T] Query for attribute [fastcs] query=P?, response=0.0 +[2026-01-01 11:11:12.194+0000 I] PV put: DEMO:R1:Enabled = 1 [fastcs.transports.epics.ca.ioc] pv=DEMO:R1:Enabled, value=1 +[2026-01-01 11:11:12.195+0000 I] Sending attribute value [fastcs] command=N01=1 +[2026-01-01 11:11:12.262+0000 T] Query for attribute [fastcs] query=P?, response=29.040181873093132 +[2026-01-01 11:11:12.463+0000 T] Query for attribute [fastcs] query=P?, response=30.452524641833854 In [2]: controller.power.disable_tracing() ``` -These log messages include other trace loggers that log messages with `power` as the -`topic`, so they also appear automatically, so the log messages show changes to the -attribute throughout the stack: the query to the device and its response, the value the -attribute is set to, and the value that the PV in the EPICS CA transport is set to. - +Only messages with `power` as their topic appear, even though every attribute's getter +is querying the device on the same period - other attributes' queries stay silent until +tracing is enabled on them too. :::{note} The `Tracer` can also be used as a module-level instance for use in free functions. diff --git a/pyproject.toml b/pyproject.toml index f178a375c..95129fb4a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -92,7 +92,21 @@ addopts = """ """ # https://iscinumpy.gitlab.io/post/bound-version-constraints/#watch-for-warnings # https://github.com/DiamondLightSource/FastCS/issues/230 -filterwarnings = "error" +# +# The two pytest-generated warnings are downgraded to "report but do not fail". +# They are raised for events that happen *outside* any test - an exception during +# garbage collection, or in a non-main thread - and pytest attributes them to +# whichever test happens to be running at the time. This suite leaves objects +# alive in its subprocess and multiprocessing fixtures (run_ioc_as_subprocess's +# forkserver and Queues, the tickit Popen in test_docs_snippets), so the +# resulting ResourceWarning lands on an unrelated test, and which one varies by +# Python version and by run. They are still printed, so real leaks stay visible; +# track them down with PYTHONTRACEMALLOC=25, which adds the allocation traceback. +filterwarnings = [ + "error", + "default::pytest.PytestUnraisableExceptionWarning", + "default::pytest.PytestUnhandledThreadExceptionWarning", +] # Doctest python code in docs, python code in src docstrings, test functions in tests testpaths = "docs src tests" timeout = 5 diff --git a/src/fastcs/attributes/__init__.py b/src/fastcs/attributes/__init__.py index d0f5e59f0..e968192b2 100644 --- a/src/fastcs/attributes/__init__.py +++ b/src/fastcs/attributes/__init__.py @@ -1,10 +1,12 @@ from .attr_r import AttrR as AttrR +from .attr_r import Getter as Getter +from .attr_r import NotPolled as NotPolled +from .attr_r import Polled as Polled +from .attr_r import Schedule as Schedule from .attr_rw import AttrRW as AttrRW from .attr_w import AttrW as AttrW +from .attr_w import Setter as Setter from .attribute import Attribute as Attribute from .attribute import AttributeAccessMode as AttributeAccessMode -from .attribute_io import AnyAttributeIO as AnyAttributeIO -from .attribute_io import AttributeIO as AttributeIO -from .attribute_io_ref import AttributeIORef as AttributeIORef -from .attribute_io_ref import AttributeIORefT as AttributeIORefT from .hinted_attribute import HintedAttribute as HintedAttribute +from .update import Update as Update diff --git a/src/fastcs/attributes/_infer_datatype.py b/src/fastcs/attributes/_infer_datatype.py new file mode 100644 index 000000000..a601781d6 --- /dev/null +++ b/src/fastcs/attributes/_infer_datatype.py @@ -0,0 +1,52 @@ +from __future__ import annotations + +import enum +import inspect +from collections.abc import Callable +from typing import Any, get_args, get_origin + +from fastcs.attributes.update import Update +from fastcs.datatypes import Bool, DataType, Enum, Float, Int, String + +_DEFAULT_DATATYPES: dict[type, Callable[[], DataType]] = { + int: Int, + float: Float, + bool: Bool, + str: String, +} + + +def _unwrap_update_annotation(annotation: Any) -> Any: + if get_origin(annotation) is Update: + args = get_args(annotation) + return args[0] if args else annotation + return annotation + + +def _datatype_for_type(py_type: Any) -> DataType | None: + if py_type in _DEFAULT_DATATYPES: + return _DEFAULT_DATATYPES[py_type]() + if isinstance(py_type, type) and issubclass(py_type, enum.Enum): + return Enum(py_type) + return None + + +def infer_datatype_from_getter(getter: Callable) -> DataType | None: + """Infer a default ``DataType`` from a getter's return type annotation.""" + signature = inspect.signature(getter, eval_str=True) + annotation = signature.return_annotation + if annotation is inspect.Signature.empty: + return None + return _datatype_for_type(_unwrap_update_annotation(annotation)) + + +def infer_datatype_from_setter(setter: Callable) -> DataType | None: + """Infer a default ``DataType`` from a setter's value parameter annotation.""" + signature = inspect.signature(setter, eval_str=True) + parameters = list(signature.parameters.values()) + if not parameters: + return None + annotation = parameters[0].annotation + if annotation is inspect.Signature.empty: + return None + return _datatype_for_type(annotation) diff --git a/src/fastcs/attributes/attr_r.py b/src/fastcs/attributes/attr_r.py index 28d66c07b..3e4243d5a 100644 --- a/src/fastcs/attributes/attr_r.py +++ b/src/fastcs/attributes/attr_r.py @@ -1,75 +1,149 @@ from __future__ import annotations import asyncio -from collections.abc import Callable, Coroutine -from typing import Any +from collections.abc import Awaitable, Callable, Coroutine +from dataclasses import KW_ONLY, dataclass, replace +from typing import Any, Generic +from fastcs.attributes._infer_datatype import infer_datatype_from_getter from fastcs.attributes.attribute import Attribute, AttributeAccessMode -from fastcs.attributes.attribute_io_ref import AttributeIORefT +from fastcs.attributes.update import Update from fastcs.attributes.util import AttrValuePredicate, PredicateEvent from fastcs.datatypes import DataType, DType_T from fastcs.logging import logger +from fastcs.util import ONCE -AttrIOUpdateCallback = Callable[["AttrR[DType_T, Any]"], Coroutine[None, None, None]] -"""An AttributeIO callback that takes an AttrR and updates its value""" -AttrUpdateCallback = Callable[[], Coroutine[None, None, None]] -"""A callback to be called periodically to update an attribute""" -AttrOnUpdateCallback = Callable[[DType_T], Coroutine[None, None, None]] -"""A callback to be called when the value of the attribute is updated""" +Getter = Callable[[], Awaitable[DType_T | Update[DType_T]]] +"""A callable that fetches a fresh value for an attribute from its source""" +AttrReadbackCallback = Callable[[DType_T], Coroutine[None, None, None]] +"""A callback to be called when the readback of the attribute updates""" -class AttrR(Attribute[DType_T, AttributeIORefT]): +@dataclass +class Polled(Generic[DType_T]): + """A getter to be read repeatedly, every ``period`` seconds:: + + AttrR(getter=Polled(protocol.get_temperature, period=0.1)) + + Use this for values the device changes on its own, such as readings and status. + A getter passed without a schedule is read once, when the controller connects. + """ + + getter: Getter[DType_T] | None = None + _: KW_ONLY + period: float + + def __call__(self, getter: Getter[DType_T]) -> Polled[DType_T]: + """Bind a getter, so a schedule can also be applied as a decorator.""" + return replace(self, getter=getter) + + +@dataclass +class NotPolled(Generic[DType_T]): + """A getter that is never read on a schedule:: + + AttrR(getter=NotPolled(protocol.get_label)) + + The value is only set explicitly - from a ``@scan`` or a subscription calling + ``attr.update()`` - or read on demand with ``await attr.poll()``. This is not the + same as an attribute with no getter at all, which has nothing to read. + """ + + getter: Getter[DType_T] | None = None + + def __call__(self, getter: Getter[DType_T]) -> NotPolled[DType_T]: + """Bind a getter, so a schedule can also be applied as a decorator.""" + return replace(self, getter=getter) + + +Schedule = Polled[DType_T] | NotPolled[DType_T] +"""A getter with a reading schedule attached""" + + +class AttrR(Attribute[DType_T]): """A read-only ``Attribute``""" def __init__( self, - datatype: DataType[DType_T], - io_ref: AttributeIORefT | None = None, - group: str | None = None, + datatype: DataType[DType_T] | None = None, + getter: Getter[DType_T] | Schedule[DType_T] | None = None, initial_value: DType_T | None = None, - description: str | None = None, + **kwargs: Any, ) -> None: - super().__init__(datatype, io_ref, group, description=description) + match getter: + case Polled() | NotPolled(): + if getter.getter is None: + raise ValueError( + f"{type(getter).__name__} was given no getter to schedule" + ) + resolved_getter = getter.getter + poll_period = getter.period if isinstance(getter, Polled) else None + case None: + resolved_getter, poll_period = None, None + case _: + # A getter with no schedule is read once, when the controller + # connects - the safe default, and what a bare ``@attr`` means. + resolved_getter, poll_period = getter, ONCE + + if datatype is None and resolved_getter is not None: + datatype = infer_datatype_from_getter(resolved_getter) + + # Pass the datatype on rather than validating it here: in an ``AttrRW`` the + # setter may still supply it, and ``Attribute`` makes the final check. + super().__init__(datatype, **kwargs) + self._value: DType_T = ( - datatype.initial_value if initial_value is None else initial_value + self._datatype.initial_value if initial_value is None else initial_value ) - self._update_callback: AttrIOUpdateCallback[DType_T] | None = None - """Callback to update the value of the attribute with an IO to the source""" - self._on_update_callbacks: ( - list[tuple[AttrOnUpdateCallback[DType_T], bool]] | None + self._getter = resolved_getter + self._poll_period: float | None = poll_period + """Period in seconds between calls to poll(), or ONCE, or None (on-demand)""" + self._readback_callbacks: ( + list[tuple[AttrReadbackCallback[DType_T], bool]] | None ) = None - """Callbacks to publish changes to the value of the attribute""" + """Callbacks to publish changes to the readback of the attribute""" self._on_update_events: set[PredicateEvent[DType_T]] = set() """Events to set when the value satisifies some predicate""" - def get(self) -> DType_T: - """Get the cached value of the attribute.""" + @property + def readback(self) -> DType_T: + """The last known value of the attribute.""" return self._value + def has_getter(self) -> bool: + return self._getter is not None + + @property + def poll_period(self) -> float | None: + return self._poll_period + @property def access_mode(self) -> AttributeAccessMode: return "r" - async def update(self, value: Any) -> None: - """Update the value of the attibute + async def update(self, value: DType_T | Update[DType_T]) -> None: + """Update the value of the attribute This sets the cached value of the attribute presented in the API. It should - generally only be called from an IO or a controller that is updating the value - from some underlying source. + generally only be called from a getter or a controller that is updating the + value from some underlying source. Any update callbacks will be called with the new value and any update events with predicates satisfied by the new value will be set. - To request a change to the setpoint of the attribute, use the ``put`` method, + To request a change to the setpoint of the attribute, use the ``set`` method, which will attempt to apply the change to the underlying source. Args: - value: The new value of the attribute + value: The new value of the attribute, or an ``Update`` wrapping it Raises: ValueError: If the value fails to be validated to DType_T """ + if isinstance(value, Update): + value = value.readback + self.log_event("Attribute set", value=repr(value), attribute=self) _previous_value = self._value @@ -85,57 +159,53 @@ async def update(self, value: Any) -> None: e for e in self._on_update_events if e.set(self._value) } - if self._on_update_callbacks is not None: - callbacks_to_call: list[AttrOnUpdateCallback[DType_T]] = [ + if self._readback_callbacks is not None: + callbacks_to_call: list[AttrReadbackCallback[DType_T]] = [ cb - for cb, always in self._on_update_callbacks + for cb, always in self._readback_callbacks if always or not self.datatype.equal(self._value, _previous_value) ] try: await asyncio.gather(*[cb(self._value) for cb in callbacks_to_call]) except Exception as e: logger.opt(exception=e).error( - "On update callbacks failed", + "Readback callbacks failed", attribute=self, value=repr(self._value), ) raise - def add_on_update_callback( - self, callback: AttrOnUpdateCallback[DType_T], always: bool = False - ) -> None: - """Add a callback to be called when the value of the attribute is updated + async def poll(self) -> DType_T: + """Fetch a fresh value from the getter, cache it, and return it.""" + if self._getter is None: + raise RuntimeError(f"{self} has no getter") - The callback will be called with the updated value. + self.log_event("Poll attribute", topic=self) + result = await self._getter() + await self.update(result) + return self._value - """ - if self._on_update_callbacks is None: - self._on_update_callbacks = [] - self._on_update_callbacks.append((callback, always)) + def add_readback_callback( + self, callback: AttrReadbackCallback[DType_T], always: bool = False + ) -> None: + """Add a callback to be called when the readback of the attribute updates - def set_update_callback(self, callback: AttrIOUpdateCallback[DType_T]): - """Set the callback to update the value of the attribute from the source + The callback will be called with the updated readback value. Transports + should use this to publish the attribute's readback, and + ``AttrW.add_setpoint_callback`` to publish its setpoint. - The callback will be converted to an async task and called periodically. + Args: + callback: The callback to call with the updated readback value + always: Whether to call the callback on every ``update``, rather than + only when the new value differs from the cached one. Defaults to + ``False``, so an update that does not change the value is not + published. Pass ``True`` for a callback that must see every update + - one that timestamps it, or counts it, rather than displaying it. """ - if self._update_callback is not None: - raise RuntimeError("Attribute already has an IO update callback") - - self._update_callback = callback - - def bind_update_callback(self) -> AttrUpdateCallback: - """Bind self into the registered IO update callback""" - if self._update_callback is None: - raise RuntimeError("Attribute has no update callback") - else: - update_callback = self._update_callback - - async def update_attribute(): - self.log_event("Update attribute", topic=self) - await update_callback(self) - - return update_attribute + if self._readback_callbacks is None: + self._readback_callbacks = [] + self._readback_callbacks.append((callback, always)) async def wait_for_predicate( self, predicate: AttrValuePredicate[DType_T], *, timeout: float diff --git a/src/fastcs/attributes/attr_rw.py b/src/fastcs/attributes/attr_rw.py index 5f0c2edbd..4214254e2 100644 --- a/src/fastcs/attributes/attr_rw.py +++ b/src/fastcs/attributes/attr_rw.py @@ -1,42 +1,84 @@ -from fastcs.attributes.attr_r import AttrR -from fastcs.attributes.attr_w import AttrW +from __future__ import annotations + +from typing import Any + +from fastcs.attributes.attr_r import AttrR, Getter, Schedule +from fastcs.attributes.attr_w import AttrW, Setter from fastcs.attributes.attribute import AttributeAccessMode -from fastcs.attributes.attribute_io_ref import AttributeIORefT +from fastcs.attributes.update import Update from fastcs.datatypes import DataType, DType_T +from fastcs.logging import logger -class AttrRW(AttrR[DType_T, AttributeIORefT], AttrW[DType_T, AttributeIORefT]): +class AttrRW(AttrR[DType_T], AttrW[DType_T]): """A read-write ``Attribute``.""" def __init__( self, - datatype: DataType[DType_T], - io_ref: AttributeIORefT | None = None, - group: str | None = None, + datatype: DataType[DType_T] | None = None, + getter: Getter[DType_T] | Schedule[DType_T] | None = None, + setter: Setter[DType_T] | None = None, initial_value: DType_T | None = None, - description: str | None = None, + **kwargs: Any, ): - super().__init__(datatype, io_ref, group, initial_value, description) - - self._setpoint_initialised = False - - if io_ref is None: - self.set_on_put_callback(self._internal_update) + # There is no datatype handling to do here. ``AttrR`` infers it from the + # getter and ``AttrW`` from the setter; the MRO runs both in turn, so + # whichever can resolve it does, and ``Attribute`` makes the final check. + super().__init__( + datatype, + getter=getter, + setter=setter, + initial_value=initial_value, + **kwargs, + ) @property def access_mode(self) -> AttributeAccessMode: return "rw" - async def _internal_update( - self, attr: AttrW[DType_T, AttributeIORefT], value: DType_T - ): - """Update value directly when Attribute has no IO""" - assert attr is self - await self.update(value) + async def update(self, value: DType_T | Update[DType_T]) -> None: + """Update the readback of the attribute, and its setpoint if appropriate. + + An ``Update`` carrying a ``setpoint`` publishes that too - the mechanism for + a device that reports its own setpoint. Otherwise, the first readback to + arrive establishes the setpoint, so that a setpoint display shows the + device's value rather than the datatype's default until first written. - async def update(self, value: DType_T): + """ await super().update(value) - if not self._setpoint_initialised: - await self._call_sync_setpoint_callbacks(self._value) - self._setpoint_initialised = True + if isinstance(value, Update) and value.setpoint is not None: + await self.update_setpoint(value.setpoint) + elif not self._setpoint_known: + await self.update_setpoint(self._value) + + async def set(self, value: DType_T) -> None: + """Request a new value for the attribute. + + With no setter, this is a soft attribute: the requested value is pushed + straight to the readback. With a setter, a returned value is treated as the + device's accepted/clamped value and is applied to the readback as well as + the setpoint. + + """ + await self.update_setpoint(value) + + if self._setter is None: + await self.update(self._setpoint) + else: + try: + result = await self._setter(self._setpoint) + except Exception as e: + logger.opt(exception=e).error( + "Set failed", attribute=self, setpoint=self._setpoint + ) + else: + if isinstance(result, Update): + await self.update(result) + elif result is not None: + # A bare value is the device's accepted/clamped value - both the + # new readback and what it understood us to ask for. + await self.update_setpoint(result) + await self.update(result) + + self.log_event("Set complete", setpoint=self._setpoint, attribute=self) diff --git a/src/fastcs/attributes/attr_w.py b/src/fastcs/attributes/attr_w.py index 3e6a4517d..696d66dc7 100644 --- a/src/fastcs/attributes/attr_w.py +++ b/src/fastcs/attributes/attr_w.py @@ -1,98 +1,129 @@ +from __future__ import annotations + import asyncio -from collections.abc import Awaitable, Callable +from collections.abc import Awaitable, Callable, Coroutine from typing import Any +from fastcs.attributes._infer_datatype import infer_datatype_from_setter from fastcs.attributes.attribute import Attribute, AttributeAccessMode -from fastcs.attributes.attribute_io_ref import AttributeIORefT +from fastcs.attributes.update import Update from fastcs.datatypes import DataType, DType_T from fastcs.logging import logger -AttrOnPutCallback = Callable[["AttrW[DType_T, Any]", DType_T], Awaitable[None]] -"""Callbacks to be called when the setpoint of an attribute is changed""" -AttrSyncSetpointCallback = Callable[[DType_T], Awaitable[None]] -"""Callbacks to be called when the setpoint of an attribute is changed""" +Setter = Callable[[DType_T], Awaitable[None | DType_T | Update[DType_T]]] +"""A callable that applies a new setpoint to an attribute's source""" +AttrSetpointCallback = Callable[[DType_T], Coroutine[None, None, None]] +"""A callback to be called when the setpoint of the attribute updates""" -class AttrW(Attribute[DType_T, AttributeIORefT]): +class AttrW(Attribute[DType_T]): """A write-only ``Attribute``.""" def __init__( self, - datatype: DataType[DType_T], - io_ref: AttributeIORefT | None = None, - group: str | None = None, - description: str | None = None, + datatype: DataType[DType_T] | None = None, + setter: Setter[DType_T] | None = None, + **kwargs: Any, ) -> None: - super().__init__( - datatype, # type: ignore - io_ref, - group, - description=description, - ) - self._on_put_callback: AttrOnPutCallback[DType_T] | None = None - """Callback to action a change to the setpoint of the attribute""" - self._sync_setpoint_callbacks: list[AttrSyncSetpointCallback[DType_T]] = [] + if datatype is None and setter is not None: + datatype = infer_datatype_from_setter(setter) + + super().__init__(datatype, **kwargs) + + self._setter = setter + self._setpoint: DType_T = self._datatype.initial_value + self._setpoint_known = False + """Whether the setpoint reflects a real value rather than the datatype default + + Until something establishes it - a write, or (for an ``AttrRW``) the first + readback - the setpoint is just the datatype's default and means nothing. + """ + self._setpoint_callbacks: list[AttrSetpointCallback[DType_T]] = [] """Callbacks to publish changes to the setpoint of the attribute""" + @property + def setpoint(self) -> DType_T: + """The last-requested value of the attribute.""" + return self._setpoint + + def has_setter(self) -> bool: + return self._setter is not None + @property def access_mode(self) -> AttributeAccessMode: return "w" - async def put(self, setpoint: DType_T, sync_setpoint: bool = False) -> None: - """Set the setpoint of the attribute + def add_setpoint_callback(self, callback: AttrSetpointCallback[DType_T]) -> None: + """Add a callback to be called when the setpoint of the attribute updates + + The callback will be called with the updated setpoint. Transports should + use this to publish the attribute's setpoint rather than tracking their own, + so that every transport agrees on it however the change was made. + + """ + self._setpoint_callbacks.append(callback) - This should be called by clients to the attribute such as transports to apply a - change to the attribute. The ``_on_put_callback`` will be called with this new - setpoint, which may or may not take effect depending on the validity of the new - value. For example, if the attribute has an IO to some device, the value might - be rejected. + async def update_setpoint(self, value: DType_T) -> None: + """Cache a new setpoint and publish it to the setpoint callbacks. - To directly change the value of the attribute, for example from an update loop - that has read a new value from some underlying source, call `AttrR.update`. + This does no IO - it is the setpoint-side counterpart of ``AttrR.update``. """ - setpoint = self._datatype.validate(setpoint) - if self._on_put_callback is not None: + self._setpoint = self._datatype.validate(value) + self._setpoint_known = True + + if self._setpoint_callbacks: try: - await self._on_put_callback(self, setpoint) + await asyncio.gather( + *[cb(self._setpoint) for cb in self._setpoint_callbacks] + ) except Exception as e: logger.opt(exception=e).error( - "Put failed", attribute=self, setpoint=setpoint + "Setpoint callbacks failed", + attribute=self, + setpoint=repr(self._setpoint), ) + raise + + def _setter_result_setpoint( + self, result: DType_T | Update[DType_T] + ) -> DType_T | None: + """The setpoint a setter's return value asks for, if any.""" + if isinstance(result, Update): + # A bare readback with no setpoint leaves the cached setpoint alone. + return result.setpoint + # A bare value is the device's accepted/clamped value - both what it will + # report and what it understood us to ask for. + return result + + async def set(self, value: DType_T) -> None: + """Request a new value for the attribute + + This should be called by clients to the attribute such as transports to apply + a change to the attribute. ``value`` is cached as the setpoint, then the + setter (if any) is called to apply it to the underlying source - the value + might be rejected or clamped, depending on the validity of the new value. If + the setter returns a value, that is treated as the source's accepted/clamped + value and becomes the new cached setpoint. + + To directly change the readback of an attribute, for example from an update + loop that has read a new value from some underlying source, call + ``AttrR.update``. - if sync_setpoint: + """ + await self.update_setpoint(value) + + if self._setter is not None: try: - await self._call_sync_setpoint_callbacks(setpoint) + result = await self._setter(self._setpoint) except Exception as e: logger.opt(exception=e).error( - "Sync setpoint failed", attribute=self, setpoint=setpoint + "Set failed", attribute=self, setpoint=self._setpoint ) + else: + if result is not None: + accepted = self._setter_result_setpoint(result) + if accepted is not None: + await self.update_setpoint(accepted) - self.log_event("Put complete", setpoint=setpoint, attribute=self) - - async def _call_sync_setpoint_callbacks(self, setpoint: DType_T) -> None: - if self._sync_setpoint_callbacks: - await asyncio.gather( - *[cb(setpoint) for cb in self._sync_setpoint_callbacks] - ) - - def set_on_put_callback(self, callback: AttrOnPutCallback[DType_T]) -> None: - """Set the callback to call when the setpoint is changed - - The callback will be called with the attribute and the new setpoint. - - """ - if self._on_put_callback is not None: - raise RuntimeError("Attribute already has an on put callback") - - self._on_put_callback = callback - - def add_sync_setpoint_callback( - self, callback: AttrSyncSetpointCallback[DType_T] - ) -> None: - """Add a callback to publish changes to the setpoint of the attribute - - The callback will be called with the new setpoint. - - """ - self._sync_setpoint_callbacks.append(callback) + self.log_event("Set complete", setpoint=self._setpoint, attribute=self) diff --git a/src/fastcs/attributes/attribute.py b/src/fastcs/attributes/attribute.py index ca4955b53..f98e0ebab 100644 --- a/src/fastcs/attributes/attribute.py +++ b/src/fastcs/attributes/attribute.py @@ -2,14 +2,13 @@ from collections.abc import Callable from typing import Generic, Literal -from fastcs.attributes.attribute_io_ref import AttributeIORefT from fastcs.datatypes import DataType, DType, DType_T from fastcs.tracer import Tracer AttributeAccessMode = Literal["r", "w", "rw"] -class Attribute(Generic[DType_T, AttributeIORefT], Tracer, ABC): +class Attribute(Generic[DType_T], Tracer, ABC): """Base FastCS attribute. Instances of this class added to a ``Controller`` will be used by the FastCS class. @@ -17,17 +16,24 @@ class Attribute(Generic[DType_T, AttributeIORefT], Tracer, ABC): def __init__( self, - datatype: DataType[DType_T], - io_ref: AttributeIORefT | None = None, + datatype: DataType[DType_T] | None = None, group: str | None = None, description: str | None = None, ) -> None: super().__init__() + # Subclasses may infer the datatype from a getter's return annotation or a + # setter's value annotation and pass the result down; by the time it reaches + # here it must be resolved. + if datatype is None: + raise ValueError( + "datatype must be given explicitly, or be inferable from the " + "getter's return annotation or the setter's value annotation" + ) + assert issubclass(datatype.dtype, DType), ( f"Attr type must be one of {DType}, received type {datatype.dtype}" ) - self._io_ref = io_ref self._datatype: DataType[DType_T] = datatype self._group = group self.enabled = True @@ -41,15 +47,6 @@ def __init__( self._name = "" self._path = [] - @property - def io_ref(self) -> AttributeIORefT: - if self._io_ref is None: - raise RuntimeError(f"{self} has no AttributeIORef") - return self._io_ref - - def has_io_ref(self): - return self._io_ref is not None - @property def datatype(self) -> DataType[DType_T]: return self._datatype @@ -115,4 +112,4 @@ def __repr__(self): full_name = self.full_name or None datatype = self._datatype.__class__.__name__ - return f"{name}(name={full_name}, datatype={datatype}, io_ref={self._io_ref})" + return f"{name}(name={full_name}, datatype={datatype})" diff --git a/src/fastcs/attributes/attribute_io.py b/src/fastcs/attributes/attribute_io.py deleted file mode 100644 index bc2749770..000000000 --- a/src/fastcs/attributes/attribute_io.py +++ /dev/null @@ -1,60 +0,0 @@ -from typing import Any, Generic, cast, get_args - -from fastcs.attributes.attr_r import AttrR -from fastcs.attributes.attr_w import AttrW -from fastcs.attributes.attribute_io_ref import AttributeIORef, AttributeIORefT -from fastcs.datatypes import DType_T -from fastcs.tracer import Tracer - - -class AttributeIO(Generic[DType_T, AttributeIORefT], Tracer): - """Base class for performing IO for an `Attribute` - - This class should be inherited to implement reading and writing values from - ``Attributes`` via some API. For read, ``Attribute``s implement the ``update`` - method and for write, ``Attribute`` implement the ``send`` method. - - Concrete implementations of this class must be parameterised with a specific - ``AttributeIORef`` that defines exactly what part of the API the ``Attribute`` - corresponds to. See the docstring for `AttributeIORef` for more information. - """ - - ref_type = AttributeIORef - - def __init_subclass__(cls) -> None: - # sets ref_type from subclass generic args - # from python 3.12 we can use types.get_original_bases - args = get_args(cast(Any, cls).__orig_bases__[0]) - cls.ref_type = args[1] - - def __init__(self): - super().__init__() - - async def update(self, attr: AttrR[DType_T, AttributeIORefT]) -> None: - """Update `AttrR` value from device - - This method will be called in `AttrR.update` in a background task. - - Exceptions raised by this method will be caught and logged with a full stack - trace. If using targeted try-except blocks to log more specific errors, this - should be done with stack trace and exceptions should be re-raised to be handled - by FastCS. - - """ - raise NotImplementedError() - - async def send(self, attr: AttrW[DType_T, AttributeIORefT], value: DType_T) -> None: - """Send `Attribute` value to device - - This method will be called in `AttrW.put`, generally from a `Transport`. - - Exceptions raised by this method will be caught and logged with a full stack - trace. If using targetted try-except blocks to log more specific errors, this - should be done with stack trace and exceptions should be re-raised to be handled - by FastCS.. - - """ - raise NotImplementedError() - - -AnyAttributeIO = AttributeIO[Any] diff --git a/src/fastcs/attributes/attribute_io_ref.py b/src/fastcs/attributes/attribute_io_ref.py deleted file mode 100644 index 575025822..000000000 --- a/src/fastcs/attributes/attribute_io_ref.py +++ /dev/null @@ -1,26 +0,0 @@ -from dataclasses import KW_ONLY, dataclass - -from typing_extensions import TypeVar - - -@dataclass -class AttributeIORef: - """Base for references to define IO for an ``Attribute`` over an API. - - This object acts as a specification of the API that its corresponding - ``AttributeIO`` should access for a given ``Attribute``. The fields necessary to - distinguish between different ``Attributes`` is an implementation detail of the IO, - but some examples are a string to send over a TCP port, or URI within an HTTP - server. - """ - - # Make fields keyword-only so that child classes can have fields without defaults - _: KW_ONLY - update_period: float | None = None - """Period in seconds between attribute updates, or `ONCE`""" - - -AttributeIORefT = TypeVar( - "AttributeIORefT", bound=AttributeIORef, default=AttributeIORef, covariant=True -) -"""An `AttributeIORef` for an `Attribute`""" diff --git a/src/fastcs/attributes/update.py b/src/fastcs/attributes/update.py new file mode 100644 index 000000000..be84f4be2 --- /dev/null +++ b/src/fastcs/attributes/update.py @@ -0,0 +1,28 @@ +from __future__ import annotations + +from dataclasses import dataclass +from typing import Generic + +from fastcs.datatypes import DType_T + + +@dataclass +class Update(Generic[DType_T]): + """A value returned from a getter or setter, with optional metadata. + + A getter or setter may return a bare value, or wrap it in ``Update`` to say + more about it: + + - ``timestamp`` - when the value was obtained. ``None`` means the framework + should stamp it with the time the update was received. + - ``setpoint`` - a setpoint to publish alongside the readback. ``None`` leaves + the cached setpoint untouched. + + A bare value returned from a setter is equivalent to + ``Update(readback=value, setpoint=value)`` - the device's accepted or clamped + value, which is both what it will report and what was asked of it. + """ + + readback: DType_T + timestamp: float | None = None + setpoint: DType_T | None = None diff --git a/src/fastcs/controllers/base_controller.py b/src/fastcs/controllers/base_controller.py index 8b29ed6d2..725b02e53 100755 --- a/src/fastcs/controllers/base_controller.py +++ b/src/fastcs/controllers/base_controller.py @@ -1,7 +1,5 @@ from __future__ import annotations -from collections import Counter -from collections.abc import Sequence from copy import deepcopy from typing import ( TypeVar, @@ -11,7 +9,7 @@ get_type_hints, ) -from fastcs.attributes import AnyAttributeIO, Attribute, AttrR, AttrW, HintedAttribute +from fastcs.attributes import Attribute, HintedAttribute from fastcs.controllers.controller_api import ControllerAPI from fastcs.logging import logger from fastcs.methods import Command, Method, Scan, UnboundCommand, UnboundScan @@ -41,7 +39,6 @@ def __init__( self, path: list[str] | None = None, description: str | None = None, - ios: Sequence[AnyAttributeIO] | None = None, ) -> None: super().__init__() @@ -64,10 +61,6 @@ def __init__( self._bind_attrs() - ios = ios or [] - self._attribute_ref_io_map = {io.ref_type: io for io in ios} - self._validate_io(ios) - def _find_type_hints(self): """Find `Attribute` and `Controller` type hints for introspection validation""" for name, hint in get_type_hints(type(self)).items(): @@ -81,7 +74,7 @@ def _find_type_hints(self): if args is None: dtype = None else: - if len(args) == 2: + if len(args) == 1: dtype = args[0] else: raise TypeError( @@ -156,16 +149,6 @@ class method and a controller instance, so that it can be called from any ): self.add_scan(attr_name, unbound_scan.bind(self)) - def _validate_io(self, ios: Sequence[AnyAttributeIO]): - """Validate that there is exactly one AttributeIO class registered to the - controller for each type of AttributeIORef belonging to the attributes of the - controller""" - for ref_type, count in Counter([io.ref_type for io in ios]).items(): - if count > 1: - raise RuntimeError( - f"More than one AttributeIO class handles {ref_type.__name__}" - ) - def __repr__(self): name = self.__class__.__name__ path = ".".join(self.path) or None @@ -192,7 +175,6 @@ async def initialise(self): def post_initialise(self): """Hook to call after all attributes added, before serving the application""" self._validate_type_hints() - self._connect_attribute_ios() def _validate_type_hints(self): """Validate all type-hints were introspected""" @@ -260,28 +242,6 @@ def _validate_hinted_controller(self, name: str): sub_controller=controller, ) - def _connect_attribute_ios(self) -> None: - """Connect ``Attribute`` callbacks to ``AttributeIO``s""" - for attr in self.__attributes.values(): - ref = attr.io_ref if attr.has_io_ref() else None - if ref is None: - continue - - io = self._attribute_ref_io_map.get(type(ref)) - if io is None: - raise ValueError( - f"{self.__class__.__name__} does not have an AttributeIO " - f"to handle {attr.io_ref.__class__.__name__}" - ) - - if isinstance(attr, AttrW): - attr.set_on_put_callback(io.send) - if isinstance(attr, AttrR): - attr.set_update_callback(io.update) - - for controller in self.sub_controllers.values(): - controller._connect_attribute_ios() # noqa: SLF001 - @property def path(self) -> list[str]: """Path prefix of attributes, recursively including parent Controllers.""" diff --git a/src/fastcs/controllers/controller.py b/src/fastcs/controllers/controller.py index a6c726027..b03793db6 100755 --- a/src/fastcs/controllers/controller.py +++ b/src/fastcs/controllers/controller.py @@ -2,9 +2,7 @@ from collections import defaultdict from collections.abc import Sequence -from fastcs.attributes import AnyAttributeIO from fastcs.attributes.attr_r import AttrR -from fastcs.attributes.attribute_io_ref import AttributeIORef from fastcs.controllers.base_controller import BaseController from fastcs.controllers.controller_api import ControllerAPI from fastcs.logging import logger @@ -18,9 +16,8 @@ class Controller(BaseController): def __init__( self, description: str | None = None, - ios: Sequence[AnyAttributeIO] | None = None, ) -> None: - super().__init__(description=description, ios=ios) + super().__init__(description=description) self._connected = False def add_sub_controller(self, name: str, sub_controller: BaseController): @@ -83,14 +80,18 @@ def create_api_and_tasks( scan_dict[method.period].append(method.fn) for attribute in api.attributes.values(): - match attribute: - case AttrR(_io_ref=AttributeIORef(update_period=update_period)): - if update_period is ONCE: - initial_coros.append(attribute.bind_update_callback()) - elif update_period is not None: - scan_dict[update_period].append( - attribute.bind_update_callback() - ) + if not (isinstance(attribute, AttrR) and attribute.has_getter()): + continue + + poll_period = attribute.poll_period + + async def poll_attribute(attribute: AttrR = attribute) -> None: + await attribute.poll() + + if poll_period is ONCE: + initial_coros.append(poll_attribute) + elif poll_period is not None: + scan_dict[poll_period].append(poll_attribute) periodic_scan_coros: list[ScanCallback] = [] for period, methods in scan_dict.items(): diff --git a/src/fastcs/controllers/controller_vector.py b/src/fastcs/controllers/controller_vector.py index 119258272..739952fc2 100755 --- a/src/fastcs/controllers/controller_vector.py +++ b/src/fastcs/controllers/controller_vector.py @@ -1,6 +1,5 @@ -from collections.abc import Iterator, Mapping, MutableMapping, Sequence +from collections.abc import Iterator, Mapping, MutableMapping -from fastcs.attributes import AnyAttributeIO from fastcs.controllers.base_controller import BaseController from fastcs.controllers.controller import Controller from fastcs.util import Controller_T @@ -18,9 +17,8 @@ def __init__( self, children: Mapping[int, Controller_T], description: str | None = None, - ios: Sequence[AnyAttributeIO] | None = None, ) -> None: - super().__init__(description=description, ios=ios) + super().__init__(description=description) self._children: dict[int, Controller_T] = {} for index, child in children.items(): self[index] = child diff --git a/src/fastcs/demo/README.md b/src/fastcs/demo/README.md new file mode 100644 index 000000000..3865e7a2a --- /dev/null +++ b/src/fastcs/demo/README.md @@ -0,0 +1,76 @@ +# `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=…)`), 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 + +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`; 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: + +- **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` 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. diff --git a/src/fastcs/demo/__main__.py b/src/fastcs/demo/__main__.py index ff4548063..467be4f81 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 5926fc8ce..000000000 --- a/src/fastcs/demo/controllers.py +++ /dev/null @@ -1,146 +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 -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._ramp_controllers: list[TemperatureRampController] = [] - for index in range(1, settings.num_ramp_controllers + 1): - controller = TemperatureRampController(index, self.connection) - self._ramp_controllers.append(controller) - self.add_sub_controller(f"R{index}", controller) - - @command() - async def cancel_all(self) -> None: - for rc in self._ramp_controllers: - 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 enumerate(self._ramp_controllers): - self.log_event( - "Update voltages", - topic=controller.voltage, - query=query, - response=voltages, - ) - await controller.voltage.update(float(voltages[index])) - - -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/eiger.py b/src/fastcs/demo/eiger.py new file mode 100644 index 000000000..4d473b9af --- /dev/null +++ b/src/fastcs/demo/eiger.py @@ -0,0 +1,178 @@ +"""Example 5 - introspectable controller: a cut-down Eiger over the fake REST sim. + +Half the attributes (``count_time``, ``state``) are declared as type hints and +checked by the current ``HintedAttribute`` introspection-validation mechanism; the +rest of the parameter tree is discovered at ``initialise()`` time by walking the +sim's ``keys`` endpoints and is added dynamically, with no static check. A device +that describes itself over the wire is exactly the case where introspection earns +its complexity - contrast with the (deliberately non-introspectable) SCPI/temperature +examples. +""" + +import enum +from dataclasses import dataclass +from typing import Any, cast + +import httpx + +from fastcs.attributes import AttrR, AttrRW, Polled +from fastcs.controllers import Controller +from fastcs.datatypes import Bool, DataType, Enum, Float, Int, String +from fastcs.demo.simulation.eiger import API_PREFIX, Subsystem, ValueType + +_DATATYPES: dict[ValueType, type[DataType]] = { + "float": Float, + "int": Int, + "string": String, + "bool": Bool, +} + +# Poll period (seconds) for read-only status params that change on the device. +UPDATE_PERIOD = 0.2 + + +def _datatype(param: str, data: dict[str, Any]) -> DataType: + """Build a datatype for a parameter from the metadata the device reports. + + A parameter that reports ``allowed_values`` is discrete, so it becomes an `Enum` + over an enum class built from those values. The members are only knowable over the + wire, which is exactly the case introspection exists for. + """ + allowed_values = data.get("allowed_values") + if allowed_values is None: + return _DATATYPES[data["value_type"]]() + + name = "".join(part.title() for part in param.split("_")) + # The functional API builds a class; type checkers only see the instance signature. + enum_cls = cast( + type[enum.Enum], enum.Enum(name, {value: value for value in allowed_values}) + ) + return Enum(enum_cls) + + +@dataclass +class EigerConnectionSettings: + base_url: str = "http://localhost:8000" + + +class EigerConnection: + """Thin async HTTP client wrapper for the Eiger REST sim. + + A ``transport`` can be supplied to point directly at an in-process ASGI app + (e.g. in tests), bypassing the network entirely. + """ + + def __init__(self, transport: httpx.AsyncBaseTransport | None = None): + self._transport = transport + self._client: httpx.AsyncClient | None = None + + async def connect(self, settings: EigerConnectionSettings) -> None: + self._client = httpx.AsyncClient( + base_url=settings.base_url, transport=self._transport + ) + + async def close(self) -> None: + if self._client is not None: + await self._client.aclose() + self._client = None + + @property + def client(self) -> httpx.AsyncClient: + if self._client is None: + raise RuntimeError("EigerConnection is not connected") + return self._client + + async def keys(self, subsystem: Subsystem) -> list[str]: + response = await self.client.get(f"{API_PREFIX}/{subsystem}/keys") + response.raise_for_status() + return response.json() + + async def get(self, subsystem: Subsystem, param: str) -> dict: + response = await self.client.get(f"{API_PREFIX}/{subsystem}/{param}") + response.raise_for_status() + return response.json() + + async def put(self, subsystem: Subsystem, param: str, value) -> None: + response = await self.client.put( + f"{API_PREFIX}/{subsystem}/{param}", json={"value": value} + ) + response.raise_for_status() + + +class EigerDetector(Controller): + """Cut-down Eiger controller: half declared, half introspected.""" + + # Declared (checked): must exist, with this access mode and dtype, after + # initialise() introspects the parameter tree. ``state`` is discrete, and its + # enum class is built from the ``allowed_values`` the device reports, so there + # is no author-time type to hint - only the access mode can be pinned here. + count_time: AttrRW[float] + state: AttrR + + # Derived (soft): built on top of the introspected ``state`` param. Declaring + # ``state`` as a checked attribute is what lets us reference it in code and + # publish something computed from it - here, whether the detector is idle. + idle = AttrR(Bool()) + + def __init__( + self, + settings: EigerConnectionSettings | None = None, + transport: httpx.AsyncBaseTransport | None = None, + ) -> None: + self.connection = EigerConnection(transport=transport) + super().__init__() + + self._settings = settings or EigerConnectionSettings() + + def _getter(self, subsystem: Subsystem, param: str): + async def get() -> Any: + data = await self.connection.get(subsystem, param) + # No cast here - ``update`` validates against the datatype, which is the + # one place a bad value from the device should be coerced or complained + # about. + return data["value"] + + return get + + def _setter(self, subsystem: Subsystem, param: str): + async def put(value: Any) -> None: + await self.connection.put(subsystem, param, value) + + return put + + async def connect(self) -> None: + await self.connection.connect(self._settings) + self._connected = True + + async def disconnect(self) -> None: + await self.connection.close() + + async def initialise(self) -> None: + for subsystem in ("config", "status"): + for param in await self.connection.keys(subsystem): + data = await self.connection.get(subsystem, param) + datatype = _datatype(param, data) + + if data["access_mode"] == "rw": + attr = AttrRW( + datatype, + getter=self._getter(subsystem, param), + setter=self._setter(subsystem, param), + ) + else: + # Read-only params are status values that change on the device, + # so poll them periodically rather than reading once. + attr = AttrR( + datatype, + getter=Polled( + self._getter(subsystem, param), period=UPDATE_PERIOD + ), + ) + + self.add_attribute(param, attr) + + # Keep the derived ``idle`` flag in sync with the introspected ``state``. + self.state.add_readback_callback(self._update_idle) + + async def _update_idle(self, state: enum.Enum) -> None: + await self.idle.update(state.value == "idle") diff --git a/src/fastcs/demo/simulation/eiger.py b/src/fastcs/demo/simulation/eiger.py new file mode 100644 index 000000000..b70488d7e --- /dev/null +++ b/src/fastcs/demo/simulation/eiger.py @@ -0,0 +1,141 @@ +"""A cut-down, Eiger-shaped fake REST device for the introspectable controller demo. + +Mimics the shape of a real Eiger detector's parameter-tree REST API (subsystems of +named parameters, a ``keys`` listing endpoint, per-parameter GET/PUT) without any of +the real detector logic. Introspection earns its complexity only when a device's +parameters aren't knowable at author time - this sim exists to give that a genuine, +self-describing backend to introspect. +""" + +import asyncio +from collections.abc import AsyncIterator +from contextlib import asynccontextmanager +from dataclasses import dataclass +from typing import Any, Literal + +from fastapi import FastAPI, HTTPException + +ValueType = Literal["float", "int", "string", "bool"] +AccessMode = Literal["r", "rw"] +Subsystem = Literal["config", "status"] + +API_PREFIX = "/detector/api/1.8.0" + +# The sim flips its temperature between these two values so the front end has +# something visibly changing to poll. +TEMPERATURES = (20.0, 30.0) + + +@dataclass +class EigerParameter: + value: Any + value_type: ValueType + access_mode: AccessMode = "r" + allowed_values: list[str] | None = None + """The permitted values of a discrete parameter, as the real detector reports them. + + Only discrete parameters carry this, and it is the metadata a client needs to + introspect the parameter as an enum rather than a bare string. + """ + + +def _initial_state() -> dict[Subsystem, dict[str, EigerParameter]]: + return { + "config": { + "count_time": EigerParameter(0.1, "float", "rw"), + "frame_time": EigerParameter(0.1, "float", "rw"), + "nimages": EigerParameter(1, "int", "rw"), + "description": EigerParameter("Simulated Eiger", "string", "r"), + }, + "status": { + "state": EigerParameter( + "idle", "string", "r", allowed_values=["idle", "ready", "acquire"] + ), + "temperature": EigerParameter(22.5, "float", "r"), + "humidity": EigerParameter(32.1, "float", "r"), + }, + } + + +async def _oscillate_temperature( + parameter: EigerParameter, period: float = 0.5 +) -> None: + """Flip a temperature parameter between two known values forever. + + Runs as a background task under the app's lifespan (started by a real server, + e.g. uvicorn). The in-process ASGI transport used by the controller in tests + does not start lifespan events, so a test that wants the task running drives + the lifespan explicitly. + """ + index = 0 + while True: + await asyncio.sleep(period) + index = 1 - index + parameter.value = TEMPERATURES[index] + + +def create_eiger_sim_app() -> FastAPI: + """Create a FastAPI app simulating a cut-down Eiger detector REST API.""" + state = _initial_state() + + @asynccontextmanager + async def lifespan(app: FastAPI) -> AsyncIterator[None]: + task = asyncio.create_task( + _oscillate_temperature(state["status"]["temperature"]) + ) + try: + yield + finally: + task.cancel() + + app = FastAPI(lifespan=lifespan) + # Backdoor: expose the parameter tree so tests can set read-only values (e.g. + # ``state``, which has no PUT route) and then poll them through the controller. + app.state.sim = state + + def _subsystem(subsystem: str) -> dict[str, EigerParameter]: + try: + return state[subsystem] # type: ignore[index] + except KeyError: + raise HTTPException( + status_code=404, detail=f"Unknown subsystem '{subsystem}'" + ) from None + + def _parameter(subsystem: str, param: str) -> EigerParameter: + try: + return _subsystem(subsystem)[param] + except KeyError: + raise HTTPException( + status_code=404, detail=f"Unknown parameter '{param}'" + ) from None + + @app.get(API_PREFIX + "/{subsystem}/keys") + async def get_keys(subsystem: str) -> list[str]: + return list(_subsystem(subsystem)) + + @app.get(API_PREFIX + "/{subsystem}/{param}") + async def get_parameter(subsystem: str, param: str) -> dict[str, Any]: + parameter = _parameter(subsystem, param) + data: dict[str, Any] = { + "value": parameter.value, + "value_type": parameter.value_type, + "access_mode": parameter.access_mode, + } + # Only discrete parameters report their options, as on the real detector. + if parameter.allowed_values is not None: + data["allowed_values"] = parameter.allowed_values + return data + + @app.put(API_PREFIX + "/{subsystem}/{param}") + async def put_parameter( + subsystem: str, param: str, body: dict[str, Any] + ) -> dict[str, Any]: + parameter = _parameter(subsystem, param) + if parameter.access_mode != "rw": + raise HTTPException( + status_code=403, detail=f"Parameter '{param}' is read-only" + ) + parameter.value = body["value"] + return {"value": parameter.value} + + return app diff --git a/src/fastcs/demo/temperature_attr.py b/src/fastcs/demo/temperature_attr.py new file mode 100755 index 000000000..756559bc8 --- /dev/null +++ b/src/fastcs/demo/temperature_attr.py @@ -0,0 +1,219 @@ +"""Example 2 - getter/setter: per-attribute IO wired via callables in ``__init__``. + +The device's protocol is written as a plain class with one ``async`` method per +command, each doing its own IO and returning a typed value - the shape a +manufacturer's own library usually already has. Those methods *are* the getters and +setters:: + + self.ramp_rate = AttrRW( + getter=Polled(protocol.get_ramp_rate, period=0.2), + setter=protocol.set_ramp_rate, + ) + +Nothing sits between the protocol and the attribute: no IO class hierarchy, no +per-attribute ref object, no adapter. Because each method annotates its types, the +datatype is inferred from them, so most attributes do not restate it - only the ones +that want metadata the annotation cannot carry, like ``Float(prec=3)``. + +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 dataclass + +import numpy as np + +from fastcs.attributes import AttrR, AttrRW, Polled +from fastcs.connections import IPConnection, IPConnectionSettings +from fastcs.controllers import Controller, ControllerVector +from fastcs.datatypes import DType_T, Float, Waveform +from fastcs.logging import logger +from fastcs.methods import command, scan + + +class OnOffEnum(enum.StrEnum): + Off = "0" + On = "1" + + +@dataclass +class TemperatureControllerSettings: + num_ramp_controllers: int + ip_settings: IPConnectionSettings + + +class TemperatureProtocol: + """The device's wire protocol - one async method per command, doing its own IO. + + This is the layer a manufacturer would ship: it knows how to talk to the device + and nothing about FastCS. Each method is a zero- or one-argument coroutine + returning an annotated type, which is exactly what an attribute's ``getter`` and + ``setter`` are, so they can be handed over as-is. + """ + + def __init__(self, connection: IPConnection, suffix: str = "") -> None: + self._connection = connection + self._suffix = suffix + + async def _query(self, param: str, dtype: Callable[[str], DType_T]) -> DType_T: + query = f"{param}{self._suffix}?\r\n" + response = (await self._connection.send_query(query)).strip("\r\n") + logger.trace("Query for attribute", query=query, response=response) + return dtype(response) + + async def _command(self, param: str, value: object) -> None: + command = f"{param}{self._suffix}={value}\r\n" + await self._connection.send_command(command) + logger.trace("Send command for attribute", command=command) + + async def get_ramp_rate(self) -> float: + return await self._query("R", float) + + async def set_ramp_rate(self, value: float) -> None: + await self._command("R", value) + + async def get_power(self) -> float: + return await self._query("P", float) + + async def get_voltages(self) -> np.ndarray: + query = "V?\r\n" + response = (await self._connection.send_query(query)).strip("\r\n") + logger.trace("Query for attribute", query=query, response=response) + return np.array(json.loads(response), dtype=np.int32) + + +class TemperatureRampProtocol(TemperatureProtocol): + """The 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 - no dispatching + on which ramp is being addressed at IO time. + """ + + def __init__(self, connection: IPConnection, index: int) -> None: + super().__init__(connection, suffix=f"{index:02d}") + + async def get_start(self) -> int: + return await self._query("S", int) + + async def set_start(self, value: int) -> None: + await self._command("S", value) + + async def get_end(self) -> int: + return await self._query("E", int) + + async def set_end(self, value: int) -> None: + await self._command("E", value) + + async def get_enabled(self) -> OnOffEnum: + return await self._query("N", OnOffEnum) + + async def set_enabled(self, value: OnOffEnum) -> None: + await self._command("N", value) + + async def get_target(self) -> float: + return await self._query("T", float) + + async def get_actual(self) -> float: + return await self._query("A", float) + + +class TemperatureController(Controller): + def __init__(self, settings: TemperatureControllerSettings) -> None: + self.connection = IPConnection() + self._settings = settings + self._protocol = TemperatureProtocol(self.connection) + + super().__init__() + + # No datatype: inferred from get_ramp_rate's `-> float` annotation. + self.ramp_rate = AttrRW( + getter=Polled(self._protocol.get_ramp_rate, period=0.2), + setter=self._protocol.set_ramp_rate, + ) + self.power = AttrR(getter=Polled(self._protocol.get_power, period=0.2)) + # 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.set(OnOffEnum.Off) + # 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): + voltages = await self._protocol.get_voltages() + + await self.voltages.update(voltages) + + for index, controller in self.ramps.items(): + self.log_event( + "Update voltages", topic=controller.voltage, response=voltages + ) + await controller.voltage.update(float(voltages[index - 1])) + + +class TemperatureRampController(Controller): + def __init__(self, index: int, conn: IPConnection) -> None: + self._protocol = TemperatureRampProtocol(conn, index) + + super().__init__(f"Ramp{index:02d}") + + self.connection = conn + + # Datatypes inferred from the protocol methods' annotations - including the + # enum, whose members come from OnOffEnum via get_enabled's return type. + self.start = AttrRW( + getter=Polled(self._protocol.get_start, period=0.2), + setter=self._protocol.set_start, + ) + self.end = AttrRW( + getter=Polled(self._protocol.get_end, period=0.2), + setter=self._protocol.set_end, + ) + self.enabled = AttrRW( + getter=Polled(self._protocol.get_enabled, period=0.2), + setter=self._protocol.set_enabled, + ) + # Stated explicitly, to carry metadata the annotation cannot: `-> float` + # says nothing about display precision. + self.target = AttrR( + Float(prec=3), getter=Polled(self._protocol.get_target, period=0.2) + ) + self.actual = AttrR( + Float(prec=3), getter=Polled(self._protocol.get_actual, period=0.2) + ) + # Updated by the parent controller's update_voltages scan + self.voltage = AttrR(Float(prec=3)) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index 536cdcfa5..29a89d3a2 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -214,7 +214,7 @@ async def async_record_set(value: DType_T): _add_attr_pvi_info(record, pv_prefix, attr_name, "r") - attribute.add_on_update_callback(async_record_set) + attribute.add_readback_callback(async_record_set) def _create_and_link_write_pv( @@ -229,7 +229,7 @@ def _create_and_link_write_pv( async def on_update(value): logger.info("PV put: {pv} = {value}", pv=pv, value=repr(value)) - await attribute.put(cast_from_epics_type(attribute.datatype, value)) + await attribute.set(cast_from_epics_type(attribute.datatype, value)) async def set_setpoint_without_process(value: DType_T): tracer.log_event( @@ -244,7 +244,10 @@ async def set_setpoint_without_process(value: DType_T): _add_attr_pvi_info(record, pv_prefix, attr_name, "w") - attribute.add_sync_setpoint_callback(set_setpoint_without_process) + # Mirror the attribute's setpoint whenever it changes, however it changed - + # a put on this PV, a put on another transport, or the device reporting its + # own setpoint. See ADR 0020. + attribute.add_setpoint_callback(set_setpoint_without_process) def _create_and_link_command_pvs( diff --git a/src/fastcs/transports/epics/ca/util.py b/src/fastcs/transports/epics/ca/util.py index 6a3e6dd83..c6473afbf 100644 --- a/src/fastcs/transports/epics/ca/util.py +++ b/src/fastcs/transports/epics/ca/util.py @@ -73,7 +73,7 @@ def validate_ca_id(controller_api: ControllerAPI) -> None: def _make_in_record(pv: str, attribute: AttrR) -> RecordWrapper: common_fields = { "DESC": attribute.description, - "initial_value": cast_to_epics_type(attribute.datatype, attribute.get()), + "initial_value": cast_to_epics_type(attribute.datatype, attribute.readback), } match attribute.datatype: @@ -139,7 +139,7 @@ def _make_out_record(pv: str, attribute: AttrW, on_update: Callable) -> RecordWr "DESC": attribute.description, "initial_value": cast_to_epics_type( attribute.datatype, - attribute.get() + attribute.readback if isinstance(attribute, AttrRW) else attribute.datatype.initial_value, ), diff --git a/src/fastcs/transports/epics/pva/_pv_handlers.py b/src/fastcs/transports/epics/pva/_pv_handlers.py index 5ba819f98..5668ce0d4 100644 --- a/src/fastcs/transports/epics/pva/_pv_handlers.py +++ b/src/fastcs/transports/epics/pva/_pv_handlers.py @@ -53,7 +53,7 @@ async def put(self, pv: SharedPV, op: ServerOperation): else: pv.post(value) - await self._attr_w.put(cast_value) + await self._attr_w.set(cast_value) op.done() @@ -121,7 +121,7 @@ def _wrap(value: dict): def make_shared_read_pv(attribute: AttrR) -> SharedPV: shared_pv = SharedPV( - initial=cast_to_p4p_value(attribute, attribute.get()), + initial=cast_to_p4p_value(attribute, attribute.readback), **_make_shared_pv_arguments(attribute), ) @@ -129,7 +129,7 @@ async def set_readback(value): tracer.log_event("PV set readback", topic=attribute, value=value) shared_pv.post(cast_to_p4p_value(attribute, value)) - attribute.add_on_update_callback(set_readback) + attribute.add_readback_callback(set_readback) return shared_pv @@ -145,7 +145,10 @@ async def set_setpoint(value): tracer.log_event("PV set setpoint", topic=attribute, value=value) shared_pv.post(cast_to_p4p_value(attribute, value)) - attribute.add_sync_setpoint_callback(set_setpoint) + # Mirror the attribute's setpoint whenever it changes, however it changed - a + # put on this PV, a put on another transport, or the device reporting its own + # setpoint. See ADR 0020. + attribute.add_setpoint_callback(set_setpoint) return shared_pv diff --git a/src/fastcs/transports/epics/pva/ioc.py b/src/fastcs/transports/epics/pva/ioc.py index 5b2e29611..a3816fe77 100644 --- a/src/fastcs/transports/epics/pva/ioc.py +++ b/src/fastcs/transports/epics/pva/ioc.py @@ -11,7 +11,7 @@ from .pvi import add_pvi_info -async def parse_attributes(root_controller_api: ControllerAPI) -> StaticProvider: +def parse_attributes(root_controller_api: ControllerAPI) -> StaticProvider: """Parses `Attribute` s into p4p signals in handlers.""" provider = StaticProvider(pv_prefix_from_path(root_controller_api.path)) @@ -56,13 +56,16 @@ class P4PIOC: def __init__(self, controller_apis: list[ControllerAPI]): self._controller_apis = controller_apis - - async def _build_providers(self) -> list[StaticProvider]: - return [await parse_attributes(api) for api in self._controller_apis] + # Build the PVs up front rather than in ``run``. Creating a PV is what + # registers its readback/setpoint callbacks on the attribute, and ``run`` + # is awaited after the initial polls have already fired - so a PV built + # there would miss the first readback, and the setpoint an ``AttrRW`` + # seeds from it (ADR 0020), leaving the served value at the datatype + # default. ``EpicsCAIOC`` builds its records in ``__init__`` for the + # same reason. + self._providers = [parse_attributes(api) for api in self._controller_apis] async def run(self): - providers = await self._build_providers() - endless_event = asyncio.Event() - with Server(providers): + with Server(self._providers): await endless_event.wait() diff --git a/src/fastcs/transports/graphql/graphql.py b/src/fastcs/transports/graphql/graphql.py index 871e5c905..0c74ad27d 100644 --- a/src/fastcs/transports/graphql/graphql.py +++ b/src/fastcs/transports/graphql/graphql.py @@ -144,7 +144,7 @@ def _wrap_attr_set( """Wrap an attribute in a function with annotations for strawberry""" async def _dynamic_f(value): - await attribute.put(value) + await attribute.set(value) return value # Add type annotations for validation, schema, conversions @@ -161,7 +161,7 @@ def _wrap_attr_get( """Wrap an attribute in a function with annotations for strawberry""" async def _dynamic_f() -> DType_T: - return attribute.get() + return attribute.readback _dynamic_f.__name__ = attr_name _dynamic_f.__annotations__["return"] = attribute.datatype.dtype diff --git a/src/fastcs/transports/rest/rest.py b/src/fastcs/transports/rest/rest.py index 522246b2e..a8ca359d8 100644 --- a/src/fastcs/transports/rest/rest.py +++ b/src/fastcs/transports/rest/rest.py @@ -69,7 +69,7 @@ def _wrap_attr_put( attribute: AttrW[DType_T], ) -> Callable[[DType_T], Coroutine[Any, Any, None]]: async def attr_put(request): - await attribute.put(cast_from_rest_type(attribute.datatype, request.value)) + await attribute.set(cast_from_rest_type(attribute.datatype, request.value)) # Fast api uses type annotations for validation, schema, conversions attr_put.__annotations__["request"] = _put_request_body(attribute) @@ -95,7 +95,7 @@ def _wrap_attr_get( attribute: AttrR[DType_T], ) -> Callable[[], Coroutine[Any, Any, dict[str, object]]]: async def attr_get() -> dict[str, object]: - value = attribute.get() + value = attribute.readback return {"value": cast_to_rest_type(attribute.datatype, value)} return attr_get diff --git a/src/fastcs/transports/tango/dsr.py b/src/fastcs/transports/tango/dsr.py index 93aaab1c8..553fe3d92 100644 --- a/src/fastcs/transports/tango/dsr.py +++ b/src/fastcs/transports/tango/dsr.py @@ -30,7 +30,7 @@ def _wrap_updater_fget( ) -> Callable[[Any], Any]: async def fget(tango_device: Device): tango_device.info_stream(f"called fget method: {attr_name}") - return cast_to_tango_type(attribute.datatype, attribute.get()) + return cast_to_tango_type(attribute.datatype, attribute.readback) return fget @@ -54,7 +54,7 @@ def _wrap_updater_fset( ) -> Callable[[Any, Any], Any]: async def fset(tango_device: Device, value): tango_device.info_stream(f"called fset method: {attr_name}") - coro = attribute.put(cast_from_tango_type(attribute.datatype, value)) + coro = attribute.set(cast_from_tango_type(attribute.datatype, value)) await _run_threadsafe_blocking(coro, loop) return fset diff --git a/tests/assertable_controller.py b/tests/assertable_controller.py index c57916134..d429abd7a 100644 --- a/tests/assertable_controller.py +++ b/tests/assertable_controller.py @@ -1,44 +1,24 @@ import copy from contextlib import contextmanager -from dataclasses import dataclass from typing import Literal from pytest_mock import MockerFixture, MockType -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR from fastcs.controllers import Controller, ControllerAPI -from fastcs.datatypes import DType_T, Int +from fastcs.datatypes import Int from fastcs.methods import command, scan -@dataclass -class MyTestAttributeIORef(AttributeIORef): - update_period = 1 - - -class MyTestAttributeIO(AttributeIO[DType_T, MyTestAttributeIORef]): - async def update(self, attr: AttrR[DType_T, MyTestAttributeIORef]): - print(f"update {attr}") - - async def send(self, attr: AttrW[DType_T, MyTestAttributeIORef], value: DType_T): - print(f"sending {attr} = {value}") - if isinstance(attr, AttrRW): - await attr.update(value) - - -test_attribute_io = MyTestAttributeIO() # instance - - class TestSubController(Controller): - read_int: AttrR = AttrR(Int(), io_ref=MyTestAttributeIORef()) - def __init__(self) -> None: - super().__init__(ios=[test_attribute_io]) + super().__init__() + self.read_int = AttrR(Int()) class MyTestController(Controller): def __init__(self) -> None: - super().__init__(ios=[test_attribute_io]) + super().__init__() self._sub_controllers: list[TestSubController] = [] for index in range(1, 3): @@ -97,29 +77,67 @@ def __init__( @contextmanager def assert_read_here(self, path: list[str]): - yield from self._assert_method(path, "get") + yield from self._assert_readback(path) @contextmanager def assert_write_here(self, path: list[str]): - yield from self._assert_method(path, "put") + yield from self._assert_method(path, "set") @contextmanager def assert_execute_here(self, path: list[str]): yield from self._assert_method(path, "") - def _assert_method(self, path: list[str], method: Literal["get", "put", ""]): + def _navigate(self, path: list[str]) -> tuple[ControllerAPI, str]: + queue = copy.deepcopy(path) + controller_api: ControllerAPI = self + item_name = queue.pop(-1) + for item in queue: + controller_api = controller_api.sub_apis[item] + return controller_api, item_name + + def _assert_readback(self, path: list[str]): + """Confirm that an attribute's ``readback`` property is read exactly once + within a context block. + + ``readback`` is a read-only property, so it can't be spied on with + ``mocker.spy`` (which needs to reassign the instance attribute). Instead, + temporarily replace the property on the attribute's class with a counting + wrapper, scoped to just this one instance. + """ + controller_api, item_name = self._navigate(path) + attr = controller_api.attributes[item_name] + assert isinstance(attr, AttrR) + cls = type(attr) + original = cls.readback + assert original.fget is not None + original_fget = original.fget + call_count = {"n": 0} + + def fget(self): + if self is attr: + call_count["n"] += 1 + return original_fget(self) + + cls.readback = property(fget) # type: ignore[misc] + try: + yield # Enter context + except Exception as e: + raise e + else: # Exit context + assert call_count["n"] == 1, ( + f"Expected {'.'.join(path + ['readback'])} to be read once, " + f"but it was read {call_count['n']} times." + ) + finally: + cls.readback = original # type: ignore[misc] + + def _assert_method(self, path: list[str], method: Literal["set", ""]): """ This context manager can be used to confirm that a fastcs controller's respective attribute or command methods are called a single time within a context block """ - queue = copy.deepcopy(path) - - # Navigate to sub controller - controller_api = self - item_name = queue.pop(-1) - for item in queue: - controller_api = controller_api.sub_apis[item] + controller_api, item_name = self._navigate(path) # Get spy if method: diff --git a/tests/conftest.py b/tests/conftest.py index 818c7d178..9c6563dfe 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -24,7 +24,7 @@ from fastcs.logging._logging import LogLevel from fastcs.transports.tango.dsr import FASTCS_TANGO_SERVER_NAME, register_dev from fastcs.transports.tango.util import tango_dev_class_name, tango_dev_name -from tests.assertable_controller import MyTestAttributeIORef, MyTestController +from tests.assertable_controller import MyTestController from tests.example_p4p_ioc import run as _run_p4p_ioc from tests.example_softioc import run as _run_softioc @@ -42,11 +42,11 @@ def clear_softioc_records(): class BackendTestController(MyTestController): - read_int: AttrR = AttrR(Int(), io_ref=MyTestAttributeIORef()) - read_write_int: AttrRW = AttrRW(Int(), io_ref=MyTestAttributeIORef()) + read_int: AttrR = AttrR(Int()) + read_write_int: AttrRW = AttrRW(Int()) read_write_float: AttrRW = AttrRW(Float()) read_bool: AttrR = AttrR(Bool()) - write_bool: AttrW = AttrW(Bool(), io_ref=MyTestAttributeIORef()) + write_bool: AttrW = AttrW(Bool()) read_string: AttrRW = AttrRW(String()) diff --git a/tests/demo/test_eiger.py b/tests/demo/test_eiger.py new file mode 100644 index 000000000..df35f758d --- /dev/null +++ b/tests/demo/test_eiger.py @@ -0,0 +1,141 @@ +import asyncio +import enum + +import httpx +import pytest +import pytest_asyncio + +from fastcs.attributes import AttrR, AttrRW +from fastcs.datatypes import Enum +from fastcs.demo.eiger import UPDATE_PERIOD, EigerDetector +from fastcs.demo.simulation.eiger import EigerParameter, create_eiger_sim_app +from fastcs.util import ONCE + +# Backdoor to the sim's parameter tree, keyed by subsystem then param name. +SimState = dict[str, dict[str, EigerParameter]] + + +@pytest_asyncio.fixture +async def _eiger(): + app = create_eiger_sim_app() + controller = EigerDetector(transport=httpx.ASGITransport(app=app)) + await controller.connect() + await controller.initialise() + controller.post_initialise() + yield controller, app.state.sim + await controller.disconnect() + + +@pytest_asyncio.fixture +async def detector(_eiger) -> EigerDetector: + return _eiger[0] + + +@pytest_asyncio.fixture +async def sim(_eiger) -> SimState: + return _eiger[1] + + +@pytest.mark.asyncio +async def test_hinted_attributes_are_introspected(detector: EigerDetector): + assert isinstance(detector.count_time, AttrRW) + assert detector.count_time.datatype.dtype is float + + assert isinstance(detector.state, AttrR) + # ``state`` reports ``allowed_values``, so it is introspected as an enum whose + # members come from the device rather than as a bare string. + assert isinstance(detector.state.datatype, Enum) + assert detector.state.datatype.names == ["idle", "ready", "acquire"] + + +@pytest.mark.asyncio +async def test_enum_attribute_reads_as_member(detector: EigerDetector, sim: SimState): + sim["status"]["state"].value = "acquire" + await detector.state.poll() + + state = detector.state.readback + assert isinstance(state, enum.Enum) + assert state.value == "acquire" + + +@pytest.mark.asyncio +async def test_unhinted_attributes_are_also_introspected(detector: EigerDetector): + for name in ("frame_time", "nimages", "description", "temperature", "humidity"): + assert name in detector.attributes + + +@pytest.mark.asyncio +async def test_read_attribute_from_device(detector: EigerDetector): + await detector.count_time.poll() + assert detector.count_time.readback == 0.1 + + humidity = detector.attributes["humidity"] + assert isinstance(humidity, AttrR) + await humidity.poll() + assert humidity.readback == 32.1 + + +@pytest.mark.asyncio +async def test_write_attribute_to_device(detector: EigerDetector): + await detector.count_time.set(0.5) + + # Read it back through the attribute to confirm the round-trip to the device. + await detector.count_time.poll() + assert detector.count_time.readback == 0.5 + + +@pytest.mark.asyncio +async def test_idle_derived_from_state(detector: EigerDetector, sim: SimState): + # ``idle`` is soft and starts at its default, tracking ``state`` once polled. + assert detector.idle.readback is False + + # Poke the read-only ``state`` via the sim backdoor, then poll the attribute. + sim["status"]["state"].value = "acquire" + await detector.state.poll() + assert detector.idle.readback is False + + sim["status"]["state"].value = "idle" + await detector.state.poll() + assert detector.idle.readback is True + + +@pytest.mark.asyncio +async def test_read_only_params_poll_but_rw_read_once(detector: EigerDetector): + for name in ("state", "temperature", "humidity", "description"): + attr = detector.attributes[name] + assert isinstance(attr, AttrR) and not isinstance(attr, AttrRW) + assert attr.poll_period == UPDATE_PERIOD + + assert detector.count_time.poll_period is ONCE + + +@pytest.mark.asyncio +async def test_temperature_oscillation_seen_via_subscribe(): + # The oscillation task runs under the app lifespan, so drive the lifespan here + # (the bare ASGI transport used elsewhere does not start it). Observe it through + # the controller's temperature attribute, subscribing for updates. + app = create_eiger_sim_app() + async with app.router.lifespan_context(app): + controller = EigerDetector(transport=httpx.ASGITransport(app=app)) + await controller.connect() + await controller.initialise() + controller.post_initialise() + + temperature = controller.attributes["temperature"] + assert isinstance(temperature, AttrR) + + seen: list[float] = [] + + async def record(value: float) -> None: + seen.append(value) + + temperature.add_readback_callback(record) + + # Poll across several sim flips (every 0.5s) so the value changes under us. + for _ in range(8): + await temperature.poll() + await asyncio.sleep(0.2) + + await controller.disconnect() + + assert len(set(seen)) > 1, f"temperature did not change: {seen}" diff --git a/tests/demo/test_temperature_attr.py b/tests/demo/test_temperature_attr.py new file mode 100644 index 000000000..bab260065 --- /dev/null +++ b/tests/demo/test_temperature_attr.py @@ -0,0 +1,147 @@ +from unittest.mock import AsyncMock + +import numpy as np +import pytest + +from fastcs.attributes import AttrW +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.poll() + + controller.connection.send_query.assert_awaited_once_with("R?\r\n") + assert controller.ramp_rate.readback == 1.5 + + +@pytest.mark.asyncio +async def test_ramp_rate_written_to_device(controller: TemperatureController): + controller.connection.send_command = AsyncMock() + + await controller.ramp_rate.set(2.5) + + controller.connection.send_command.assert_awaited_once_with("R=2.5\r\n") + + +@pytest.mark.asyncio +async def test_power_read_from_device(controller: TemperatureController): + controller.connection.send_query = AsyncMock(return_value="10.25\r\n") + + await controller.power.poll() + + controller.connection.send_query.assert_awaited_once_with("P?\r\n") + assert controller.power.readback == 10.25 + + +@pytest.mark.asyncio +async def test_ramp_start_read_from_device(ramp_controller: TemperatureRampController): + ramp_controller.connection.send_query = AsyncMock(return_value="7\r\n") + + await ramp_controller.start.poll() + + ramp_controller.connection.send_query.assert_awaited_once_with("S01?\r\n") + assert ramp_controller.start.readback == 7 + + +@pytest.mark.asyncio +async def test_ramp_end_written_to_device(ramp_controller: TemperatureRampController): + ramp_controller.connection.send_command = AsyncMock() + + await ramp_controller.end.set(42) + + ramp_controller.connection.send_command.assert_awaited_once_with("E01=42\r\n") + + +@pytest.mark.asyncio +async def test_ramp_enabled_written_to_device( + ramp_controller: TemperatureRampController, +): + ramp_controller.connection.send_command = AsyncMock() + + await ramp_controller.enabled.set(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.set(index) + + assert [ + call.args[0] for call in controller.connection.send_command.await_args_list + ] == ["S01=1\r\n", "S02=2\r\n", "S03=3\r\n", "S04=4\r\n"] + + +@pytest.mark.asyncio +async def test_read_only_attribute_has_no_setter( + ramp_controller: TemperatureRampController, +): + # Access mode is structural now: no setter means it is not an AttrW at all. + assert not isinstance(ramp_controller.target, AttrW) + assert ramp_controller.start.has_setter() + + +@pytest.mark.asyncio +async def test_cancel_all_disables_every_ramp(controller: TemperatureController): + sets = {} + for index, ramp in controller.ramps.items(): + sets[index] = AsyncMock() + ramp.enabled.set = sets[index] # type: ignore[method-assign] + + await controller.cancel_all() + + for set_ in sets.values(): + set_.assert_awaited_once_with(OnOffEnum.Off) + + +@pytest.mark.asyncio +async def test_update_voltages_updates_waveform_and_each_ramp( + controller: TemperatureController, +): + controller.connection.send_query = AsyncMock(return_value="[1, 2, 3, 4]\r\n") + + await controller.update_voltages() + + controller.connection.send_query.assert_awaited_once_with("V?\r\n") + np.testing.assert_array_equal( + controller.voltages.readback, np.array([1, 2, 3, 4], dtype=np.int32) + ) + for index, ramp in controller.ramps.items(): + assert ramp.voltage.readback == pytest.approx(float(index)) diff --git a/tests/example_p4p_ioc.py b/tests/example_p4p_ioc.py index 95cc8e70b..ce83fe43f 100644 --- a/tests/example_p4p_ioc.py +++ b/tests/example_p4p_ioc.py @@ -1,28 +1,16 @@ import asyncio import enum -from dataclasses import dataclass import numpy as np -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, AttrW from fastcs.controllers import Controller, ControllerVector -from fastcs.datatypes import Bool, DType_T, Enum, Float, Int, Table, Waveform +from fastcs.datatypes import Bool, Enum, Float, Int, Table, Waveform from fastcs.launch import FastCS from fastcs.methods import command, scan from fastcs.transports.epics.pva import EpicsPVATransport -@dataclass -class SimpleAttributeIORef(AttributeIORef): - pass - - -class SimpleAttributeIO(AttributeIO[DType_T, SimpleAttributeIORef]): - async def send(self, attr: AttrW[DType_T, SimpleAttributeIORef], value): - if isinstance(attr, AttrRW): - await attr.update(value) - - class FEnum(enum.Enum): A = 0 B = 1 @@ -33,39 +21,47 @@ class FEnum(enum.Enum): class ParentController(Controller): description = "some controller" - a: AttrRW = AttrRW( - Int(max=400_000, max_alarm=40_000), io_ref=SimpleAttributeIORef() - ) - b: AttrW = AttrW(Float(min=-1, min_alarm=-0.5), io_ref=SimpleAttributeIORef()) + a: AttrRW = AttrRW(Int(max=400_000, max_alarm=40_000)) + b: AttrW = AttrW(Float(min=-1, min_alarm=-0.5)) table: AttrRW = AttrRW( Table([("A", np.int32), ("B", "i"), ("C", "?"), ("D", np.float64)]), - io_ref=SimpleAttributeIORef(), ) - def __init__(self, description=None, ios=None): - super().__init__(description, ios) - class ChildController(Controller): fail_on_next_e = True - c: AttrW = AttrW(Int(), io_ref=SimpleAttributeIORef()) + c: AttrW = AttrW(Int()) + + def __init__(self, description: str | None = None): + super().__init__(description=description) + + # A getter/setter pair against an in-memory "device", doing what an + # AttributeIO used to do. The setter clamps the requested value and + # returns what it accepted, which becomes both the readback and the + # setpoint; the getter seeds the setpoint when the controller connects. + self._clamped = 5 + self.clamped = AttrRW(Int(), getter=self.get_clamped, setter=self.set_clamped) + + async def get_clamped(self) -> int: + return self._clamped - def __init__(self, description=None, ios=None): - super().__init__(description, ios) + async def set_clamped(self, value: int) -> int: + self._clamped = min(max(value, 0), 100) + return self._clamped @command() async def d(self): print("D: RUNNING") await asyncio.sleep(0.1) print("D: FINISHED") - await self.j.update(self.j.get() + 1) + await self.j.update(self.j.readback + 1) - e: AttrR = AttrR(Bool(), io_ref=SimpleAttributeIORef()) + e: AttrR = AttrR(Bool()) @scan(1) async def flip_flop(self): - await self.e.update(not self.e.get()) + await self.e.update(not self.e.readback) f: AttrRW = AttrRW(Enum(FEnum)) g: AttrRW = AttrRW(Waveform(np.int64, shape=(3,))) @@ -81,15 +77,14 @@ async def i(self): else: self.fail_on_next_e = True print("I: FINISHED") - await self.j.update(self.j.get() + 1) + await self.j.update(self.j.readback + 1) j: AttrR = AttrR(Int()) def run(id="P4P_TEST_DEVICE"): - simple_attribute_io = SimpleAttributeIO() p4p_options = EpicsPVATransport() - controller = ParentController(ios=[simple_attribute_io]) + controller = ParentController() controller.set_path([id]) class ChildVector(ControllerVector): @@ -100,12 +95,8 @@ def __init__(self, children, description=None): sub_controller = ChildVector( { - 1: ChildController( - description="some sub controller", ios=[simple_attribute_io] - ), - 2: ChildController( - description="another sub controller", ios=[simple_attribute_io] - ), + 1: ChildController(description="some sub controller"), + 2: ChildController(description="another sub controller"), }, description="some child vector", ) diff --git a/tests/example_softioc.py b/tests/example_softioc.py index 14c1a0278..d58011d71 100644 --- a/tests/example_softioc.py +++ b/tests/example_softioc.py @@ -16,6 +16,18 @@ class ParentController(Controller): a: AttrR = AttrR(Int()) b: AttrRW = AttrRW(Int()) + def __init__(self, description: str | None = None) -> None: + super().__init__(description) + self._clamped = 5 + self.clamped = AttrRW(Int(), getter=self.get_clamped, setter=self.set_clamped) + + async def get_clamped(self) -> int: + return self._clamped + + async def set_clamped(self, value: int) -> int: + self._clamped = min(max(value, 0), 100) + return self._clamped + class ChildController(Controller): c: AttrW = AttrW(Int()) @@ -30,7 +42,7 @@ def run(id="SOFTIOC_TEST_DEVICE"): controller.set_path([id]) vector = ControllerVector({i: ChildController() for i in range(2)}) controller.add_sub_controller("ChildVector", vector) - gui_options = EpicsGUIOptions(output_dir=Path("."), title="Demo Vector") + gui_options = EpicsGUIOptions(output_dir=Path("./opis"), title="Demo Vector") fastcs = FastCS( controller, [ diff --git a/tests/test_attribute_logging.py b/tests/test_attribute_logging.py index e24a47db4..4f54d5d3b 100644 --- a/tests/test_attribute_logging.py +++ b/tests/test_attribute_logging.py @@ -33,7 +33,7 @@ async def test_attr_r_update_logs_validation_error(loguru_caplog): attr = AttrR(Int()) with pytest.raises(ValueError): - await attr.update("not_an_int") + await attr.update("not_an_int") # type: ignore[arg-type] assert "Failed to validate value" in loguru_caplog.text @@ -45,9 +45,9 @@ async def test_attr_r_update_logs_callback_failure(loguru_caplog): async def failing_callback(_value: int): raise RuntimeError("callback failed") - attr.add_on_update_callback(failing_callback) + attr.add_readback_callback(failing_callback) with pytest.raises(RuntimeError): await attr.update(42) - assert "On update callbacks failed" in loguru_caplog.text + assert "Readback callbacks failed" in loguru_caplog.text diff --git a/tests/test_attributes.py b/tests/test_attributes.py index cdd428911..e78d5c59e 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -1,14 +1,13 @@ import asyncio -from dataclasses import dataclass from functools import partial -from typing import Generic, TypeVar import pytest from pytest_mock import MockerFixture -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR, AttrRW, AttrW +from fastcs.attributes import AttrR, AttrRW, AttrW, NotPolled, Polled, Update from fastcs.controllers import Controller from fastcs.datatypes import Float, Int, String +from fastcs.util import ONCE def test_attribute_access_mode(): @@ -26,10 +25,8 @@ def test_attribute_access_mode(): def test_attr_r(): attr = AttrR(String(), group="test group") - with pytest.raises(RuntimeError): - _ = attr.io_ref - - assert not attr.has_io_ref() + assert not attr.has_getter() + assert attr.poll_period is None assert isinstance(attr.datatype, String) assert attr.dtype == str assert attr.group == "test group" @@ -42,39 +39,123 @@ def test_attr_r(): assert attr.name == "test_name" assert attr.path == ["test_path"] - with pytest.raises(RuntimeError): + with pytest.raises(RuntimeError, match="already registered with a controller as"): attr.set_name("test_name") - with pytest.raises(RuntimeError): + with pytest.raises(RuntimeError, match="already registered with a controller at"): attr.set_path(["test_path"]) - assert attr.get() == "" + assert attr.readback == "" + + +def test_datatype_inferred_from_getter_annotation(): + async def get_value() -> float: + return 1.5 + + attr = AttrR(getter=get_value) + assert isinstance(attr.datatype, Float) + + +def test_datatype_inferred_from_setter_annotation(): + async def set_value(value: int) -> None: + pass + + attr = AttrW(setter=set_value) + assert isinstance(attr.datatype, Int) + + +def test_datatype_required_when_not_inferable(): + expected_message = "datatype must be given explicitly" + + with pytest.raises(ValueError, match=expected_message): + AttrR() + + with pytest.raises(ValueError, match=expected_message): + AttrW() + + with pytest.raises(ValueError, match=expected_message): + AttrRW() @pytest.mark.asyncio -async def test_attr_update(mocker: MockerFixture): +async def test_attr_update(): attr = AttrRW(Int()) await attr.update(42) - assert attr.get() == 42 + assert attr.readback == 42 await attr.update("100") # type: ignore - assert attr.get() == 100 + assert attr.readback == 100 with pytest.raises(ValueError, match="Failed to cast"): await attr.update("not_an_int") # type: ignore - attr = AttrRW(Int()) - sync_setpoint_mock = mocker.AsyncMock() - attr.add_sync_setpoint_callback(sync_setpoint_mock) + # update() also accepts an Update wrapper, unwrapping to just the value + await attr.update(Update(7, timestamp=123.0)) + assert attr.readback == 7 + + +@pytest.mark.asyncio +async def test_poll(): + async def do_update(): + return 5 + + attr = AttrR(Int(), getter=do_update) + assert attr.has_getter() + + value = await attr.poll() + assert value == 5 + assert attr.readback == 5 + + +@pytest.mark.asyncio +async def test_poll_unwraps_update_wrapper(): + async def do_update(): + return Update(9, timestamp=123.0) + + attr = AttrR(Int(), getter=do_update) + value = await attr.poll() + assert value == 9 + assert attr.readback == 9 + + +@pytest.mark.asyncio +async def test_poll_with_no_getter_raises(): + attr = AttrR(Int()) + + with pytest.raises(RuntimeError, match="has no getter"): + await attr.poll() + + +@pytest.mark.asyncio +async def test_poll_exception_propagates(): + async def do_update(): + raise ValueError("do_update failed") + + attr = AttrR(Int(), getter=do_update) - await attr.update("200") # type: ignore - assert attr.get() == 200 - sync_setpoint_mock.assert_called_once_with(200) + with pytest.raises(ValueError, match="do_update failed"): + await attr.poll() - sync_setpoint_mock.reset_mock() - await attr.update(20) - assert attr.get() == 20 - sync_setpoint_mock.assert_not_called() + +def test_poll_period_comes_from_the_getter(): + async def do_update(): + return 1 + + # A bare getter is read once, when the controller connects. + attr = AttrR(Int(), getter=do_update) + assert attr.poll_period == ONCE + + # Wrapping it in Polled schedules it instead. + attr_explicit = AttrR(Int(), getter=Polled(do_update, period=0.5)) + assert attr_explicit.poll_period == 0.5 + + # NotPolled is never scheduled - on-demand poll() only. + attr_on_demand = AttrR(Int(), getter=NotPolled(do_update)) + assert attr_on_demand.poll_period is None + assert attr_on_demand.has_getter() + + attr_no_getter = AttrR(Int()) + assert attr_no_getter.poll_period is None @pytest.mark.asyncio @@ -84,7 +165,7 @@ async def test_wait_for_predicate(mocker: MockerFixture): async def update(attr: AttrR): while True: await asyncio.sleep(0.1) - await attr.update(attr.get() + 3) # 3, 6, 9, 12 != 10 + await attr.update(attr.readback + 3) # 3, 6, 9, 12 != 10 asyncio.create_task(update(attr)) @@ -93,7 +174,7 @@ def predicate(v: int) -> bool: return v > 10 wait_mock = mocker.spy(asyncio, "wait_for") - with pytest.raises(TimeoutError): + with pytest.raises(TimeoutError, match="Timeout waiting 0.2s for .* predicate"): await attr.wait_for_predicate(predicate, timeout=0.2) await attr.wait_for_predicate(predicate, timeout=1) @@ -116,7 +197,7 @@ async def update(attr: AttrR): asyncio.create_task(update(attr)) wait_mock = mocker.spy(asyncio, "wait_for") - with pytest.raises(TimeoutError): + with pytest.raises(TimeoutError, match="Timeout waiting 0.2s for .* value 10"): await attr.wait_for_value(10, timeout=0.2) await attr.wait_for_value(1, timeout=1) @@ -131,20 +212,18 @@ async def update(attr: AttrR): @pytest.mark.asyncio async def test_attributes(): device = {"state": "Idle", "number": 1, "count": False} - ui = {"state": "", "number": 0, "count": False, "update_count": 0} + ui = {"state": "", "number": 0, "update_count": 0} async def update_ui(value, key): ui[key] = value ui["update_count"] += 1 - async def send(_attr, value, key): + async def send(value, key): device[key] = value - - async def device_add(): - device["number"] += 1 + return value # accepted value echoes straight back to the readback attr_r = AttrR(String()) - attr_r.add_on_update_callback(partial(update_ui, key="state"), always=False) + attr_r.add_readback_callback(partial(update_ui, key="state"), always=False) await attr_r.update(device["state"]) assert ui["state"] == "Idle" # Update with new value triggers callback @@ -153,50 +232,83 @@ async def device_add(): # Identical update does not trigger callback as always=False assert ui["update_count"] == 1 - attr_rw = AttrRW(Int()) - attr_rw._on_put_callback = partial(send, key="number") - attr_rw.add_sync_setpoint_callback(partial(update_ui, key="number")) - await attr_rw.put(2, sync_setpoint=True) + attr_rw = AttrRW(Int(), setter=partial(send, key="number")) + attr_rw.add_readback_callback(partial(update_ui, key="number")) + await attr_rw.set(2) assert device["number"] == 2 assert ui["number"] == 2 @pytest.mark.asyncio -async def test_attribute_io(): - @dataclass - class MyAttributeIORef(AttributeIORef): - cool: int +async def test_soft_attribute_self_wires(): + """With no getter/setter, AttrRW.set() pushes straight to readback.""" + attr = AttrRW(Int()) + assert not attr.has_getter() + assert not attr.has_setter() + + await attr.set(40) + assert attr.setpoint == 40 + assert attr.readback == 40 + - class MyAttributeIO(AttributeIO[int, MyAttributeIORef]): - async def update(self, attr: AttrR[int, MyAttributeIORef]): - print("I am updating", self.ref_type, attr.io_ref.cool) +@pytest.mark.asyncio +async def test_setter_return_value_updates_readback(): + accepted = {} - class MyController(Controller): - my_attr = AttrR(Int(), io_ref=MyAttributeIORef(cool=5)) - your_attr = AttrR(Int(), io_ref=MyAttributeIORef(cool=10)) + async def setter(value): + accepted["value"] = value + return value + 1 # device clamps/accepts a different value - def __init__(self): - super().__init__(ios=[MyAttributeIO()]) + attr = AttrRW(Int(), setter=setter) - c = MyController() + await attr.set(10) + assert accepted["value"] == 10 + assert attr.setpoint == 11 + assert attr.readback == 11 - class ControllerNoIO(Controller): - my_attr = AttrR(Int(), io_ref=MyAttributeIORef(cool=5)) - @dataclass - class OtherAttributeIORef(AttributeIORef): - not_cool: int +@pytest.mark.asyncio +async def test_setter_with_no_return_leaves_readback_untouched(): + async def setter(value): + return None - class MissingIOController(Controller): - my_attr = AttrR(Int(), io_ref=OtherAttributeIORef(not_cool=5)) + attr = AttrRW(Int(), setter=setter) - with pytest.raises(ValueError, match="does not have an AttributeIO to handle"): - controller = MissingIOController() - controller._connect_attribute_ios() + await attr.set(5) + assert attr.setpoint == 5 + assert attr.readback == 0 # unchanged - no getter/poll has happened - await c.initialise() - c._connect_attribute_ios() - await c.my_attr.bind_update_callback()() + +@pytest.mark.asyncio +async def test_attrw_setter_return_value_updates_setpoint_cache(): + async def setter(value): + return value + 1 + + attr = AttrW(Int(), setter=setter) + + await attr.set(5) + assert attr.setpoint == 6 + + +@pytest.mark.asyncio +async def test_set_setter_exception_is_caught_and_logged(mocker: MockerFixture): + async def do_set(value): + raise ValueError("do_set failed") + + attr = AttrW(Int(), setter=do_set) + mock_logger = mocker.patch("fastcs.attributes.attr_w.logger") + + # exception is caught, not raised, and the setpoint is still cached + await attr.set(5) + assert attr.setpoint == 5 + + # the setter's exception is the one logged, at error level + logged_exception = mock_logger.opt.call_args.kwargs["exception"] + assert isinstance(logged_exception, ValueError) + assert str(logged_exception) == "do_set failed" + mock_logger.opt.return_value.error.assert_called_once_with( + "Set failed", attribute=attr, setpoint=5 + ) class DummyConnection: @@ -264,38 +376,8 @@ async def set(self, uri: str, value: float | int): self._float_value = value -NumberT = TypeVar("NumberT", int, float) - - @pytest.mark.asyncio() -async def test_dynamic_attribute_io_specification(): - @dataclass - class DemoParameterAttributeIORef(AttributeIORef, Generic[NumberT]): - name: str - subsystem: str - connection: DummyConnection - - @property - def uri(self): - return f"{self.subsystem}/{self.name}" - - class DemoParameterAttributeIO(AttributeIO[NumberT, DemoParameterAttributeIORef]): - async def update( - self, - attr: AttrR[NumberT, DemoParameterAttributeIORef], - ): - value = await attr.io_ref.connection.get(attr.io_ref.uri) - await attr.update(value) # type: ignore - - async def send( - self, - attr: AttrW[NumberT, DemoParameterAttributeIORef], - value: NumberT, - ) -> None: - await attr.io_ref.connection.set(attr.io_ref.uri, value) - if isinstance(attr, AttrRW): - await self.update(attr) - +async def test_dynamic_attribute_getter_setter_specification(): class DemoParameterController(Controller): ro_int_parameter: AttrR int_parameter: AttrRW @@ -312,22 +394,36 @@ async def initialise(self): for parameter_response in example_introspection_response: try: ro = parameter_response["read_only"] - ref = DemoParameterAttributeIORef( - name=parameter_response["name"], - subsystem=parameter_response["subsystem"], - connection=self._connection, - ) - attr_class = AttrR if ro else AttrRW - attr = attr_class( - datatype=dtype_mapping[parameter_response["dtype"]]( - min=parameter_response.get("min", None), - max=parameter_response.get("max", None), - ), - io_ref=ref, - initial_value=parameter_response.get("value", None), + name = parameter_response["name"] + uri = f"{parameter_response['subsystem']}/{name}" + datatype = dtype_mapping[parameter_response["dtype"]]( + min=parameter_response.get("min", None), + max=parameter_response.get("max", None), ) - self.add_attribute(ref.name, attr) + async def getter(uri=uri) -> int | float: + return await self._connection.get(uri) # type: ignore[return-value] + + if ro: + attr = AttrR( + datatype, + getter=getter, + initial_value=parameter_response.get("value", None), + ) + else: + + async def setter(value, uri=uri): + await self._connection.set(uri, value) + return value + + attr = AttrRW( + datatype, + getter=getter, + setter=setter, + initial_value=parameter_response.get("value", None), + ) + + self.add_attribute(name, attr) except Exception as e: print( "Exception constructing attribute from parameter response:", @@ -335,130 +431,11 @@ async def initialise(self): e, ) - c = DemoParameterController(ios=[DemoParameterAttributeIO()]) + c = DemoParameterController() await c.initialise() - c._connect_attribute_ios() - await c.ro_int_parameter.bind_update_callback()() - assert c.ro_int_parameter.get() == 10 - await c.ro_int_parameter.bind_update_callback()() - assert c.ro_int_parameter.get() == 11 - - await c.int_parameter.put(20) - assert c.int_parameter.get() == 20 - - -@pytest.mark.asyncio -async def test_attribute_no_io(mocker: MockerFixture): - class MyController(Controller): - no_ref = AttrRW(Int()) - base_class_ref = AttrRW(Int(), io_ref=AttributeIORef()) - - with pytest.raises( - ValueError, - match="MyController does not have an AttributeIO to handle AttributeIORef", - ): - c = MyController() - c._connect_attribute_ios() - - class SimpleAttributeIO(AttributeIO[int]): - async def update(self, attr): - await attr.update(100) - - with pytest.raises( - RuntimeError, match="More than one AttributeIO class handles AttributeIORef" - ): - MyController(ios=[SimpleAttributeIO(), SimpleAttributeIO()]) - - # we need to explicitly pass an AttributeIO if we want to handle instances of - # the AttributeIORef base class - c = MyController(ios=[SimpleAttributeIO()]) - assert not c.no_ref.has_io_ref() - assert c.base_class_ref.has_io_ref() - - await c.initialise() - c._connect_attribute_ios() - - # There is a difference between providing an AttributeIO for the default - # AttributeIORef class and not specifying the io_ref for an Attribute - # default callbacks are not provided by AttributeIO subclasses - - sync_setpoint_mock = mocker.AsyncMock() - c.no_ref.add_sync_setpoint_callback(sync_setpoint_mock) - - await c.no_ref.put(40) - sync_setpoint_mock.assert_called_once_with(40) # sync setpoint called on first set - sync_setpoint_mock.reset_mock() - await c.no_ref.put(41) # sync setpoint callback not called without flag - await c.no_ref.put(42, sync_setpoint=True) - sync_setpoint_mock.assert_called_once_with(42) - - c2 = MyController(ios=[SimpleAttributeIO()]) - - await c2.initialise() - c2._connect_attribute_ios() - - assert c2.base_class_ref.get() == 0 - await c2.base_class_ref.bind_update_callback()() - assert c2.base_class_ref.get() == 100 - - -def test_add_update_callback_twice_raises(): - async def do_update(attr: AttrR[int]): - pass - - attr = AttrRW(Int()) - attr.set_update_callback(do_update) - - with pytest.raises(RuntimeError): - attr.set_update_callback(do_update) - - -@pytest.mark.asyncio -async def test_bind_update(): - attr = AttrRW(Int()) - - with pytest.raises(RuntimeError): - attr.bind_update_callback() - - async def do_update(attr: AttrR[int]): - await attr.update(5) - - attr.set_update_callback(do_update) - callback = attr.bind_update_callback() - - await callback() - assert attr.get() == 5 - - -@pytest.mark.asyncio -async def test_bind_update_exception(): - attr = AttrRW(Int()) - - async def do_update(attr: AttrR[int]): - raise ValueError("do_update failed") - - attr.set_update_callback(do_update) - - callback = attr.bind_update_callback() - - with pytest.raises(ValueError): - await callback() - - -@pytest.mark.asyncio -async def test_put(): - attr = AttrW(Int()) - - async def do_put(attr: AttrW[int], value: int): - raise ValueError("do_put failed") - - async def do_sync_setpoint(setpoint: int): - raise ValueError("do_sync_setpoint failed") - - attr.set_on_put_callback(do_put) - attr.add_sync_setpoint_callback(do_sync_setpoint) - await attr.put(5) + assert await c.ro_int_parameter.poll() == 10 + assert await c.ro_int_parameter.poll() == 11 - with pytest.raises(RuntimeError): - attr.set_on_put_callback(do_put) + await c.int_parameter.set(20) + assert c.int_parameter.readback == 20 diff --git a/tests/test_control_system.py b/tests/test_control_system.py index ca151cc02..66b78b192 100644 --- a/tests/test_control_system.py +++ b/tests/test_control_system.py @@ -1,9 +1,8 @@ import asyncio -from dataclasses import dataclass import pytest -from fastcs.attributes import AttributeIO, AttributeIORef, AttrR +from fastcs.attributes import AttrR, NotPolled, Polled from fastcs.control_system import FastCS from fastcs.controllers import Controller from fastcs.datatypes import Int @@ -59,40 +58,42 @@ async def do_nothing_static(self): @pytest.mark.asyncio async def test_update_periods(): - @dataclass - class AttributeIORefTimesCalled(AttributeIORef): - update_period: float | None = None - _times_called = 0 + times_called = {"once": 0, "quickly": 0, "never": 0} - class AttributeIOTimesCalled(AttributeIO[int, AttributeIORefTimesCalled]): - async def update(self, attr: AttrR[int, AttributeIORefTimesCalled]): - attr.io_ref._times_called += 1 - await attr.update(attr.io_ref._times_called) + async def get_once(): + times_called["once"] += 1 + return times_called["once"] + + async def get_quickly(): + times_called["quickly"] += 1 + return times_called["quickly"] + + async def get_never(): + times_called["never"] += 1 + return times_called["never"] class MyController(Controller): - update_once = AttrR(Int(), io_ref=AttributeIORefTimesCalled(update_period=ONCE)) - update_quickly = AttrR( - Int(), io_ref=AttributeIORefTimesCalled(update_period=0.1) - ) - update_never = AttrR( - Int(), io_ref=AttributeIORefTimesCalled(update_period=None) - ) - - controller = MyController(ios=[AttributeIOTimesCalled()]) + def __init__(self): + super().__init__() + self.update_once = AttrR(Int(), getter=Polled(get_once, period=ONCE)) + self.update_quickly = AttrR(Int(), getter=Polled(get_quickly, period=0.1)) + self.update_never = AttrR(Int(), getter=NotPolled(get_never)) + + controller = MyController() loop = asyncio.get_event_loop() fastcs = FastCS(controller, [], loop) - assert controller.update_quickly.get() == 0 - assert controller.update_once.get() == 0 - assert controller.update_never.get() == 0 + assert controller.update_quickly.readback == 0 + assert controller.update_once.readback == 0 + assert controller.update_never.readback == 0 asyncio.create_task(fastcs.serve(interactive=False)) await asyncio.sleep(0.5) - assert controller.update_quickly.get() > 1 - assert controller.update_once.get() == 1 - assert controller.update_never.get() == 0 + assert controller.update_quickly.readback > 1 + assert controller.update_once.readback == 1 + assert controller.update_never.readback == 0 assert len(fastcs._scan_tasks) == 1 assert len(fastcs._initial_coros) == 1 diff --git a/tests/test_multi_controller.py b/tests/test_multi_controller.py index 9e27f62e5..bde9b2f38 100644 --- a/tests/test_multi_controller.py +++ b/tests/test_multi_controller.py @@ -166,7 +166,7 @@ async def test_pva_transport_serves_two_controllers_with_distinct_pvi_roots(): transport = EpicsPVATransport() transport.connect([api1, api2], asyncio.get_event_loop()) - providers = await transport._ioc._build_providers() + providers = transport._ioc._providers pv_names = {name for provider in providers for name in provider.keys()} assert "ALPHA:PVI" in pv_names diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 8b7c12205..ece949116 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -8,7 +8,6 @@ from softioc import softioc from tests.assertable_controller import ( AssertableControllerAPI, - MyTestAttributeIORef, MyTestController, ) from tests.util import ColourEnum @@ -54,7 +53,7 @@ async def test_create_and_link_read_pv(mocker: MockerFixture): record = make_record.return_value attribute = AttrR(Int()) - attribute.add_on_update_callback = mocker.MagicMock() + attribute.add_readback_callback = mocker.MagicMock() _create_and_link_read_pv("PREFIX", "PV", "attr", None, attribute) @@ -62,8 +61,8 @@ async def test_create_and_link_read_pv(mocker: MockerFixture): add_attr_pvi_info.assert_called_once_with(record, "PREFIX", "attr", "r") # Extract the callback generated and set in the function and call it - attribute.add_on_update_callback.assert_called_once_with(mocker.ANY) - record_set_callback = attribute.add_on_update_callback.call_args[0][0] + attribute.add_readback_callback.assert_called_once_with(mocker.ANY) + record_set_callback = attribute.add_readback_callback.call_args[0][0] await record_set_callback(1) record.set.assert_called_once_with(1) @@ -228,27 +227,32 @@ async def test_create_and_link_write_pv(mocker: MockerFixture): ) record = make_record.return_value - attribute = AttrW(Int()) - attribute.put = mocker.AsyncMock() - attribute.add_sync_setpoint_callback = mocker.MagicMock() + attribute = AttrRW(Int()) + attribute.set = mocker.AsyncMock() + attribute.add_setpoint_callback = mocker.MagicMock() _create_and_link_write_pv("PREFIX", "PV", "attr", None, attribute) make_record.assert_called_once_with("PREFIX:PV", attribute, on_update=mocker.ANY) add_attr_pvi_info.assert_called_once_with(record, "PREFIX", "attr", "w") - # Extract the write update callback generated and set in the function and call it - attribute.add_sync_setpoint_callback.assert_called_once_with(mocker.ANY) - sync_setpoint_callback = attribute.add_sync_setpoint_callback.call_args[0][0] - await sync_setpoint_callback(1) + # Extract the setpoint callback generated and set in the function + attribute.add_setpoint_callback.assert_called_once_with(mocker.ANY) + set_setpoint_callback = attribute.add_setpoint_callback.call_args[0][0] + await set_setpoint_callback(1) record.set.assert_called_once_with(1, process=False) + # Unlike the old one-shot seeding, every setpoint change is mirrored. + record.set.reset_mock() + await set_setpoint_callback(2) + record.set.assert_called_once_with(2, process=False) + # Extract the on update callback generated and set in the function and call it on_update_callback = make_record.call_args[1]["on_update"] await on_update_callback(1) - attribute.put.assert_called_once_with(1) + attribute.set.assert_called_once_with(1) class LongEnum(enum.Enum): @@ -343,11 +347,11 @@ def test_get_output_record_raises(mocker: MockerFixture): class EpicsController(MyTestController): - read_int = AttrR(Int(), io_ref=MyTestAttributeIORef()) - read_write_int = AttrRW(Int(), io_ref=MyTestAttributeIORef()) + read_int = AttrR(Int()) + read_write_int = AttrRW(Int()) read_write_float = AttrRW(Float()) read_bool = AttrR(Bool()) - write_bool = AttrW(Bool(), io_ref=MyTestAttributeIORef()) + write_bool = AttrW(Bool()) read_string = AttrRW(String()) enum = AttrRW(Enum(enum.IntEnum("Enum", {"RED": 0, "GREEN": 1, "BLUE": 2}))) one_d_waveform = AttrRW(Waveform(np.int32, (10,))) diff --git a/tests/transports/epics/ca/test_softioc_system.py b/tests/transports/epics/ca/test_softioc_system.py index 7544f5308..fd1b1ddd0 100644 --- a/tests/transports/epics/ca/test_softioc_system.py +++ b/tests/transports/epics/ca/test_softioc_system.py @@ -16,6 +16,7 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): assert parent_pvi["value"] == { "a": {"r": f"{pv_prefix}:A"}, "b": {"r": f"{pv_prefix}:B_RBV", "w": f"{pv_prefix}:B"}, + "clamped": {"r": f"{pv_prefix}:Clamped_RBV", "w": f"{pv_prefix}:Clamped"}, "childvector": {"d": f"{pv_prefix}:ChildVector:PVI"}, } @@ -45,6 +46,10 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): "d": {"x": f"{pv_prefix}:ChildVector:0:D"}, } + initial_value = ctxt.get(f"{pv_prefix}:Clamped_RBV") + assert initial_value # Clamped initial value is truthy + assert ctxt.get(f"{pv_prefix}:Clamped") == initial_value # Setpoint is synced + # Assert alias. Aliases do not show up in PVI structure assert ctxt.get(f"{pv_prefix}:B") == ctxt.get(f"{pv_prefix}:AliasB") == 0 ctxt.put(f"{pv_prefix}:B", 10, wait=True) diff --git a/tests/transports/epics/pva/test_p4p.py b/tests/transports/epics/pva/test_p4p.py index 947ade79d..098c3cfc6 100644 --- a/tests/transports/epics/pva/test_p4p.py +++ b/tests/transports/epics/pva/test_p4p.py @@ -12,6 +12,7 @@ from p4p.client.asyncio import Context from p4p.client.thread import Context as ThreadContext from p4p.nt import NTTable +from pytest_mock import MockerFixture from fastcs.attributes import AttrR, AttrRW, AttrW from fastcs.controllers import Controller, ControllerVector @@ -62,6 +63,7 @@ async def test_ioc(p4p_subprocess: tuple[str, Queue]): assert child_pvi["display"] == {"description": "some sub controller"} assert child_pvi["value"] == { "c": {"w": f"{pv_prefix}:Child:1:C"}, + "clamped": {"rw": f"{pv_prefix}:Child:1:Clamped"}, "d": {"x": f"{pv_prefix}:Child:1:D"}, "e": {"r": f"{pv_prefix}:Child:1:E"}, "f": {"rw": f"{pv_prefix}:Child:1:F"}, @@ -71,6 +73,12 @@ async def test_ioc(p4p_subprocess: tuple[str, Queue]): "j": {"r": f"{pv_prefix}:Child:1:J"}, } + initial_value = await ctxt.get(f"{pv_prefix}:Child:1:Clamped_RBV") + assert initial_value # Clamped initial value is truthy + assert ( + await ctxt.get(f"{pv_prefix}:Child:1:Clamped") == initial_value + ) # Setpoint is synced + @pytest.mark.asyncio async def test_scan_method(p4p_subprocess: tuple[str, Queue]): @@ -654,3 +662,55 @@ async def put_pvs(): assert ( pytest.approx((end - start).total_seconds(), abs=0.1) == expected_duration ) + + +@pytest.mark.asyncio +async def test_setpoint_seeded_by_initial_poll_reaches_transport( + mocker: MockerFixture, +): + """The PVs must exist by the end of ``connect()``, not ``serve()``. + + An ``AttrRW`` seeds its setpoint from its first readback (ADR 0020), and that + readback comes from the initial poll - which ``FastCS.serve`` runs *before* it + gathers the transports' ``serve()`` coroutines. A PV built in ``serve()`` would + miss the seed and keep serving the datatype default, so the setpoint PV read + ``0`` while ``attribute.setpoint`` read the seeded value. + """ + + class SeedController(Controller): + def __init__(self): + super().__init__() + self.a = AttrRW(Int(), getter=self.get_a) + + async def get_a(self) -> int: + return 10 + + controller = SeedController() + controller.set_path([str(uuid4())]) + await controller.initialise() + controller.post_initialise() + controller_api, _, initial_coros = controller.create_api_and_tasks() + + attribute = controller_api.attributes["a"] + assert isinstance(attribute, AttrRW) + published: list[int] = [] + register_callback = attribute.add_setpoint_callback + + def record_setpoints(callback): + async def wrapper(value): + published.append(value) + await callback(value) + + register_callback(wrapper) + + mocker.patch.object(attribute, "add_setpoint_callback", record_setpoints) + + transport = EpicsPVATransport() + transport.connect(controller_apis=[controller_api], loop=asyncio.get_running_loop()) + + # Nothing has awaited transport.serve() at this point - as in FastCS.serve() + for coro in initial_coros: + await coro() + + assert attribute.setpoint == 10 + assert published == [10] diff --git a/tests/transports/graphQL/test_graphql.py b/tests/transports/graphQL/test_graphql.py index 46d2fc8a9..193d081cd 100644 --- a/tests/transports/graphQL/test_graphql.py +++ b/tests/transports/graphQL/test_graphql.py @@ -8,7 +8,6 @@ from pytest_mock import MockerFixture from tests.assertable_controller import ( AssertableControllerAPI, - MyTestAttributeIORef, MyTestController, ) @@ -18,11 +17,11 @@ class GraphQLController(MyTestController): - read_int = AttrR(Int(), io_ref=MyTestAttributeIORef()) - read_write_int = AttrRW(Int(), io_ref=MyTestAttributeIORef()) + read_int = AttrR(Int()) + read_write_int = AttrRW(Int()) read_write_float = AttrRW(Float()) read_bool = AttrR(Bool()) - write_bool = AttrW(Bool(), io_ref=MyTestAttributeIORef()) + write_bool = AttrW(Bool()) read_string = AttrRW(String()) diff --git a/tests/transports/rest/test_rest.py b/tests/transports/rest/test_rest.py index 80af6698b..2f458cb05 100644 --- a/tests/transports/rest/test_rest.py +++ b/tests/transports/rest/test_rest.py @@ -98,8 +98,8 @@ def test_enum( enum_attr = rest_controller_api.attributes["enum"] assert isinstance(enum_attr, AttrRW) enum_cls = enum_attr.datatype.dtype - assert isinstance(enum_attr.get(), enum_cls) - assert enum_attr.get() == enum_cls(0) + assert isinstance(enum_attr.readback, enum_cls) + assert enum_attr.readback == enum_cls(0) expect = 0 with rest_controller_api.assert_read_here(["enum"]): response = test_client.get("/enum") @@ -109,8 +109,8 @@ def test_enum( with rest_controller_api.assert_write_here(["enum"]): response = test_client.put("/enum", json={"value": new}) assert test_client.get("/enum").json()["value"] == new - assert isinstance(enum_attr.get(), enum_cls) - assert enum_attr.get() == enum_cls(2) + assert isinstance(enum_attr.readback, enum_cls) + assert enum_attr.readback == enum_cls(2) def test_1d_waveform( self, rest_controller_api: AssertableControllerAPI, test_client: TestClient @@ -118,8 +118,8 @@ def test_1d_waveform( attribute = rest_controller_api.attributes["one_d_waveform"] expect = np.zeros((10,), dtype=np.int32) assert isinstance(attribute, AttrRW) - assert np.array_equal(attribute.get(), expect) - assert isinstance(attribute.get(), np.ndarray) + assert np.array_equal(attribute.readback, expect) + assert isinstance(attribute.readback, np.ndarray) with rest_controller_api.assert_read_here(["one_d_waveform"]): response = test_client.get("one-d-waveform") @@ -131,8 +131,8 @@ def test_1d_waveform( result = test_client.get("/one-d-waveform") assert np.array_equal(result.json()["value"], new) - assert np.array_equal(attribute.get(), new) - assert isinstance(attribute.get(), np.ndarray) + assert np.array_equal(attribute.readback, new) + assert isinstance(attribute.readback, np.ndarray) def test_2d_waveform( self, rest_controller_api: AssertableControllerAPI, test_client: TestClient @@ -140,8 +140,8 @@ def test_2d_waveform( attribute = rest_controller_api.attributes["two_d_waveform"] assert isinstance(attribute, AttrRW) expect = np.zeros((10, 10), dtype=np.int32) - assert np.array_equal(attribute.get(), expect) - assert isinstance(attribute.get(), np.ndarray) + assert np.array_equal(attribute.readback, expect) + assert isinstance(attribute.readback, np.ndarray) with rest_controller_api.assert_read_here(["two_d_waveform"]): result = test_client.get("/two-d-waveform") @@ -152,8 +152,8 @@ def test_2d_waveform( result = test_client.get("/two-d-waveform") assert np.array_equal(result.json()["value"], new) - assert np.array_equal(attribute.get(), new) - assert isinstance(attribute.get(), np.ndarray) + assert np.array_equal(attribute.readback, new) + assert isinstance(attribute.readback, np.ndarray) def test_go( self, rest_controller_api: AssertableControllerAPI, test_client: TestClient diff --git a/tests/transports/tango/test_dsr.py b/tests/transports/tango/test_dsr.py index 61a8f73ef..1eef2c242 100644 --- a/tests/transports/tango/test_dsr.py +++ b/tests/transports/tango/test_dsr.py @@ -149,8 +149,8 @@ def test_enum(self, tango_controller_api: AssertableControllerAPI, tango_context enum_attr = tango_controller_api.attributes["enum"] assert isinstance(enum_attr, AttrRW) enum_cls = enum_attr.datatype.dtype - assert isinstance(enum_attr.get(), enum_cls) - assert enum_attr.get() == enum_cls(0) + assert isinstance(enum_attr.readback, enum_cls) + assert enum_attr.readback == enum_cls(0) expect = 0 with tango_controller_api.assert_read_here(["enum"]): result = tango_context.read_attribute("Enum").value @@ -159,8 +159,8 @@ def test_enum(self, tango_controller_api: AssertableControllerAPI, tango_context with tango_controller_api.assert_write_here(["enum"]): tango_context.write_attribute("Enum", new) assert tango_context.read_attribute("Enum").value == new - assert isinstance(enum_attr.get(), enum_cls) - assert enum_attr.get() == enum_cls(1) + assert isinstance(enum_attr.readback, enum_cls) + assert enum_attr.readback == enum_cls(1) def test_1d_waveform( self, tango_controller_api: AssertableControllerAPI, tango_context