WIP: detector 5 stack CI run (do not merge)

CLAUDE.md

@@ -51,7 +51,9 @@ tox -p                  # all envs in parallel (CI equivalent)
 ## Code conventions
 
 - **No `ABC` base needed** — `@abstractmethod` without `ABC`/`ABCMeta` is intentional; pyright enforces implementation statically.
-- **A new `TypeVar` needs an entry in `docs/conf.py`.** Docs build with `nitpicky = True` and warnings-as-errors, and TypeVars never resolve as references, so every one is listed by qualified name (`ophyd_async.core._utils.T`, …) in the `nitpick_ignore` list near the end of `conf.py`. Miss it and `tox -e docs` fails on `reference target not found` while every other env stays green. Prefer declaring TypeVars in the `_utils.py` of their package, next to the existing ones, so the list stays in one place.
+- **A new `TypeVar` needs an entry in `docs/conf.py`.** Docs build with `nitpicky = True` and warnings-as-errors, and TypeVars never resolve as references, so every one is listed by qualified name (`ophyd_async.core._utils.T`, …) in the `nitpick_ignore` list near the end of `conf.py`. Miss it and `tox -e docs` fails on `reference target not found` while every other env stays green. Generic TypeVars (`T`, `V`, …) live in the `_utils.py` of their package; a TypeVar specific to one class is declared in that class's module, next to it.
+- **Plain `Enum` members take their own name as their value** (`UNSTAGED = "UNSTAGED"`), never `auto()`. This is for enums that do not represent a control system value. `StrictEnum`, `SubsetEnum` and `SupersetEnum` are different: their values are the strings the control system uses (e.g. `ON = "On"`), so they keep those.
+- **A lone marker value is a `typing_extensions.Sentinel`** (`KICKED_OFF = Sentinel("KICKED_OFF")`, usable as its own type in hints), not a one-member `Enum`, unless it crosses a serialisation boundary or is compared `==` to plain values: `CALCULATE_TIMEOUT` stays a string because pydantic fields and YAML/JSON plan arguments carry it.
 
 ## Updating this guide
 

docs/conf.py

@@ -157,6 +157,7 @@ def setup(app: application.Sphinx):
     "ophyd_async.core._standard_base._StandardBase",
     "ophyd_async.core._flyable.PrepareT",
     "ophyd_async.core._flyable.CtxT",
+    "ophyd_async.core._detector.ProviderT",
     "ophyd_async.core._utils.P",
     "ophyd_async.core._utils.P.args",
     "ophyd_async.core._utils.P.kwargs",
@@ -171,9 +172,11 @@ def setup(app: application.Sphinx):
     "ophyd_async.epics.adcore._io.ADBaseIOT",
     "ophyd_async.epics.adcore._io.NDPluginBaseIOT",
     "ophyd_async.epics.adcore._io.NDPluginFileIOT",
-    "ophyd_async.epics.motor.MotorFlyCtx",
+    "ophyd_async.epics.motor.MotorPrepareCtx",
     "ophyd_async.epics.motor.MotorFlyableMovableLogic",
-    "ophyd_async.epics.pmac._pmac_trajectory.PmacFlyCtx",
+    "ophyd_async.epics.pmac._pmac_trajectory.PmacPrepareCtx",
+    "ophyd_async.sim._motor.SimMotorLogic",
+    "ophyd_async.sim._motor.SimMotorPrepareCtx",
     "ophyd_async.tango.core._base_device.T",
     "ophyd_async.tango.core._tango_transport.P",
     "ophyd_async.tango.core._tango_transport.R",
@@ -191,6 +194,16 @@ def setup(app: application.Sphinx):
     "0.001",
     "1.0",
     "bluesky.protocols.T_co",
+    # TypeVars documented on StandardFlyable, added in #1359
+    "PrepareT",
+    "CtxT",
+    # event_model publishes no intersphinx entry for this TypedDict
+    "event_model.documents.PartialEventPage",
+    # TypeVar for the plugin an ADDataFactory builds a data logic around
+    # The context DetectorLogic threads from prepare to unstage, private
+    # because nothing outside StandardDetector constructs or inspects one
+    "ophyd_async.core._detector.DetectorCtx",
+    "ophyd_async.core._detector._DetectorData",
 ]
 nitpick_ignore = []
 for var in obj_ignore:

docs/explanations/decisions/0012-detector-rewrite.md

@@ -5,6 +5,9 @@ Date: 2026-01-20
 
 Accepted
 
+Partly superseded by [](./0025-runtime-readable-format.md), which made `StandardDetector` a
+`StandardReadable` and removed the `prepare_single` tier along with `ReadableDataProvider`.
+
 ## Context
 
 The original `StandardDetector` implementation had several architectural issues:
@@ -52,7 +55,7 @@ We will restructure `StandardDetector` to use composition with three separate lo
 
 - **`StreamableDataProvider`** - For streaming data (appears in StreamDatum documents)
   - `make_datakeys(collections_per_event)` - Generate DataKey descriptions
-  - `make_stream_docs(collections_written, collections_per_event)` - Emit StreamAsset documents
+  - `make_stream_docs(collections_from, collections_to, collections_per_event)` - Emit StreamAsset documents
   - `collections_written_signal` - Signal tracking write progress
 
 ### Detector Changes

docs/explanations/decisions/0017-standard-movable.md

@@ -6,6 +6,10 @@ Date: 2026-03-10
 
 Accepted
 
+Partly superseded by [ADR 0021](0021-single-logic-property.md), which replaced `movable_logic` with a single `logic` property.
+
+The `stop()` hook below has since been replaced by an abstract `ensure_stopped()` that waits until the motion has stopped (see [ADR 0022](0022-fly-generator-and-ensure-stopped.md)).
+
 ## Context
 
 Several device types share a common movement pattern: write a setpoint, then observe a

docs/explanations/decisions/0021-single-logic-property.md

@@ -0,0 +1,78 @@
+# 21. One `logic` property per Device
+
+Date: 2026-08-18
+
+## Status
+
+Accepted
+
+Supersedes part of ADR 0017, which introduced `movable_logic`.
+
+## Context
+
+A Device that both moves and flies had to expose the same logic object twice. `Motor`
+built one `MotorFlyableMovableLogic` and returned it from a `movable_logic` property for
+`StandardMovable` and a `flyable_logic` property for `StandardFlyable`, plus a
+private `_logic` to avoid building it twice. Every future mix-in would add another name.
+
+The obstacle is that Python has no intersection type: there is no way to write "the logic
+must be both a `MovableLogic[float]` and a `FlyableLogic[FlyMotorInfo, MotorFlyCtx]`" as a
+single annotation.
+
+ADR 0017 also chose `@cached_property` stacked on `@abstractmethod` over an
+`add_movable_logic()` method, on the grounds that the property is "checkable by static
+analysis". That turned out not to hold. `functools.cached_property` does not forward
+`__isabstractmethod__` from the function it wraps, so `StandardMovable.__abstractmethods__`
+was empty, pyright reported nothing, and a Device that forgot its logic failed with
+`AttributeError: 'NoneType' object has no attribute 'readback'` from inside
+`Device.__init__`.
+
+## Decision
+
+Each mix-in declares the **same** attribute name, `logic`, with **its own**
+required type:
+
+```python
+class StandardMovable(Device, ..., Generic[SignalDatatypeT]):
+    @abstract_cached_property
+    def logic(self) -> MovableLogic[SignalDatatypeT]: ...
+
+class StandardFlyable(_StandardBase, ..., Generic[PrepareT, CtxT]):
+    @abstract_cached_property
+    def logic(self) -> FlyableLogic[PrepareT, CtxT]: ...
+```
+
+A Device inheriting both provides one implementation, and the type checker verifies it
+against each base independently. The intersection is expressed by the logic class's own
+MRO: `MotorFlyableMovableLogic` inherits both, so it satisfies both declarations, and the
+concrete type is what callers see back.
+
+This catches, at type-check time, a logic that satisfies one mix-in but not the other, a
+wrong datatype parameter, and a Device with two mix-ins and no implementation. A Device
+with a *single* mix-in and no implementation is caught at instantiation instead, by
+`abstract_cached_property`, a `cached_property` that sets `__isabstractmethod__` on the
+descriptor so that `Device`'s metaclass collects it. It is declared as returning
+`cached_property[T]` rather than as a subclass, because a subclass breaks the covariance
+check on the override and would reject the correct case too.
+
+Because `StandardFlyable` is now genuinely abstract, `FlyableLogic.with_device` builds
+a concrete `_EphemeralFlyable`, which takes the logic in its `__init__`.
+
+An alternative of a single `logics` collection searched by type was rejected: membership
+cannot be proved statically, so it would reintroduce exactly the runtime-only failure
+this change fixes.
+
+## Consequences
+
+- One property per Device however many mix-ins it combines; `Motor` loses two properties.
+- Where the move and fly logic are genuinely unrelated implementations they must be
+  combined into one class, since one property returns one object. Nothing in-tree pays
+  this cost, as `MotorFlyableMovableLogic` was already that shape.
+- Forgetting the logic now fails with `TypeError: Can't instantiate abstract class ...
+  with abstract method logic` rather than an `AttributeError` on `None`.
+- `movable_logic` and `flyable_logic` are removed rather than deprecated. A shim would
+  only have helped code that *reads* the attribute: a subclass that still *defines*
+  `movable_logic` would override the shim and leave `logic` unimplemented, so the
+  Device would fail at instantiation anyway.
+- The same mechanism is available to `StandardDetector` if it later inherits
+  `StandardFlyable`; its logic would need to satisfy `FlyableLogic` too.

docs/explanations/decisions/0022-fly-generator-and-ensure-stopped.md

@@ -0,0 +1,44 @@
+# 22. `fly` is a generator, and `ensure_stopped` waits
+
+Date: 2026-10-20
+
+## Status
+
+Accepted
+
+Amends ADR 0017: `MovableLogic.stop()` is now `ensure_stopped()`, and the fly hooks are different from the ones that ADR describes.
+
+## Context
+
+`FlyableLogic` had separate `on_kickoff` and `on_complete` hooks, and `stop()` returned as soon as it had asked the hardware to stop. This allowed three kinds of bug. A scan started in `on_kickoff` could outlive `unstage()`, because nothing owned it. A new `stage()` or `set()` could start while the previous motion was still decelerating. A `set()` caught by `stop()` before its move had started was not ended, and ran on after it.
+
+## Decision
+
+**`prepare_to_fly()` and `fly()` are async generators.** `prepare_to_fly` yields the context first, then any progress; `fly` yields `KICKED_OFF` once flying, then any progress. Everything a scan starts lives inside the generator, so closing or cancelling it is enough to stop it. Writing it as `async with AsyncStatus(...)` plus `yield` makes the ownership visible in the code, and a failure before or after the first yield maps cleanly on to the verb that failed. Two hooks that shared state through the logic, or a hook that returned a `(context, progress)` tuple, could not guarantee this: the progress iterator was not closed if `prepare()` failed, and a context that was itself a tuple was ambiguous. `prepare()` keeps the context only once the generator has finished, so a half-failed prepare leaves nothing to kick off. The first yield is checked at runtime, as a `WatcherUpdate` yielded first is an easy mistake that the type checker cannot see.
+
+**The fly runs in the foreground of the status that `complete()` returns.** `kickoff()` makes a `WatchableAsyncStatus` that drives `fly`, and returns once it has yielded `KICKED_OFF`; `complete()` returns that same status. It already replays the latest update to a late watcher, and the state machine cancels it on `unstage()`, so no separate task, update forwarding or cleanup path is needed. `WatchableAsyncStatus` closes the generator it drives as soon as it is cancelled, rather than when it is collected. Cancelling `complete()`'s status therefore cancels the fly, and a fly cleanup that fails is reported by that status instead of its `CancelledError`.
+
+**`ensure_stopped()` waits until the motion has ended.** `unstage()` cancels `fly`, then calls `ensure_stopped()`, so when `unstage()` returns nothing is running and the hardware is still. Returning early would leave the next `stage()` or `set()` racing the deceleration of the last one.
+
+**One stop future ends a set.** Each `set()` passes a fresh future to the `stop` argument of its `WatchableAsyncStatus`, which races the operation against the future: when the future resolves first the operation is cancelled and the future's value becomes the outcome; statuses still have no public `stop()`, as bluesky treats a status as a task the device stops. The device keeps one future, the latest `set()`'s. `stop()` resolves it with `None` for `stop(success=True)`, so the `set()` finishes normally, or with `StoppedError("Device <name> was stopped.")`, a `RuntimeError`, which it raises. `RunEngine.pause` calls `stop(success=True)` on everything it has set and expects that to finish without an error. A new `set()` resolves the previous future with a `StoppedError` saying it was superseded, so only one `set()` runs at a time; its move is cancelled but the hardware is not stopped, as the new move retargets it. The future is resolved with a value rather than an exception, so a future nobody awaits raises nothing. `stop()` ends only a `set()`, never a `prepare()` or fly: those are ended by `unstage()` through the state machine, so a `stop()` cannot reach a verb it was not meant for. A stop that arrives before the move starts ends the `set()` without touching the hardware, and a second `stop()` finds the future already resolved and only calls `ensure_stopped()` again. `stop()` does not wait for the `set()` to finish failing before `ensure_stopped()`: the hardware should stop as soon as possible. Logics no longer track whether they were stopped: a `move()` may end early without an error when stopped, as an EPICS motor's put completion does, and the stop future decides the outcome.
+
+**Only a flyer stages, and `unstage()` always stops.** A `StandardMovable` alone does nothing on `stage()` or `unstage()`: `stop()` is how it is stopped. `ensure_ready()` is a hook of `FlyableLogic` only, which defaults to `ensure_stopped()`, and only `StandardFlyable` calls it, on `stage()`; a device that moves and flies therefore stops a `set()` in progress at `stage()`, no worse than `caput MOTOR.STOP`. `StandardFlyable.unstage()` cancels a `prepare()` or fly, then always calls `ensure_stopped()`, whether or not the device was prepared. Sparing a `set()` there would only defer its stop: the bluesky RunEngine stops every movable it set when a run ends or pauses. The cost is that every staged `Motor` gets one `.STOP` at `unstage()`, because `Motor.ensure_stopped()` sends it unconditionally: `.DMOV` can flicker at standstill on some motors, so it is not read to decide, and the motor record does not write `.VAL` from `.RBV` on `.STOP`.
+
+The resulting model:
+
+| Verb | Mix-in | Does |
+|------|--------|------|
+| `stage()` | `StandardFlyable` | calls `logic.ensure_ready()` |
+| `unstage()` | `StandardFlyable` | cancels a `prepare()` or fly in progress, then calls `logic.ensure_stopped()` |
+| `stop()` | `StandardMovable` | ends the `set()` via the stop future, then calls `logic.ensure_stopped()` |
+| `set()` | `StandardMovable` | supersedes the `set()` in progress |
+
+The logic hooks `prepare_to_fly` and `fly` are async generators with a first-yield marker: the context for `prepare_to_fly`, `KICKED_OFF` for `fly`.
+
+## Consequences
+
+- The stage and unstage of the previous plan are the only thing that keeps a motion from overlapping the next one. `stage()` can start `ensure_ready()` while the `ensure_stopped()` from the previous `unstage()` is still running, and we accept that. We assume every plan stages its devices with the stage/unstage decorator, as the stock plans do, and the tests run plans back to back and after an interruption to show that this behaves. A plan that does not stage gets none of this.
+- A `FlyableLogic` that also moves gets `ensure_stopped()` called on `stage()` by default, which stops a `set()` in progress. Override `ensure_ready()` if stopping is wrong there.
+- A plan that fails while a `set()` or `complete()` is in flight sees bluesky raise a `FailedStatus` for it from `unstage()`, where it would have seen the plan's own exception first.
+- `StandardFlyable` has no `stop(success)` of its own, and `stop()` on a device that moves and flies does not end a `prepare()` or fly. `ensure_stopped` still halts the hardware, so a `stop()` during the run-up (for example `bps.stop`, or a `RunEngine` pause of a device that was also `set()`, with `rewindable=False`) can leave `prepare()` finishing short of the start. We do not check the position, as motors have a tolerance and none is baked in here. A future `pause()` hook is the better place to fix this.
+- `complete()` returns the fly status even after the fly has failed, until the next `prepare()` or `unstage()`, so the failure is reported rather than a `TransitionError`.