Rework StandardMovable and StandardFlyable around a single logic, add fly() and ensure_stopped()

CHANGELOG.md

@@ -2,6 +2,51 @@
 
 ## Unreleased
 
+### Breaking changes
+
+- `StandardMovable.movable_logic` and `StandardFlyable.flyable_logic` are replaced by a single `logic` property, declared by each mix-in with its own type, so a Device that moves and flies implements it once with a logic inheriting both `MovableLogic` and `FlyableLogic` (see ADR 0021). Rename your `movable_logic`/`flyable_logic` `@cached_property` to `logic`; no deprecated aliases are left. Forgetting it is now a `TypeError` at instantiation rather than an error the first time it is used. `StandardFlyable` is now abstract, use `FlyableLogic.with_device()` for an ephemeral flyer.
+- `FlyableLogic` hooks are renamed and reshaped; a subclass that still defines `on_prepare`, `on_kickoff`, `on_complete`, `on_stage`, `on_unstage` or (unless it is also a `MovableLogic`) `stop` raises `TypeError` naming the replacement:
+  - `on_prepare(value)` is now `prepare_to_fly(name, value, previous_ctx)`, an async generator that yields the context first, then optionally `WatcherUpdate`s that report the slow part (such as moving to the start) to the watchers of `prepare()`. A logic with no context yields `None`. `name` is the name of the device and `previous_ctx` is the context of the last finished `prepare()` since `stage()`, or `None` for the first. `prepare()` keeps the context only once the generator has finished; it raises `TypeError` if the first yield is a `WatcherUpdate` or a later one is not, and `RuntimeError` if it yields nothing. The generator is closed if `prepare()` fails or is cancelled, so it must own anything it starts (for example with `async with AsyncStatus(...)`).
+  - `on_kickoff(ctx)` and `on_complete(ctx)` are replaced by one `fly(ctx)`, an async generator that `StandardFlyable` runs in the task of the status `complete()` returns. It yields `KICKED_OFF` once the scan is up to speed, which finishes `kickoff()`, then optionally `WatcherUpdate`s, which go to the watchers of `complete()`; `complete()` finishes when it returns. A failure before `KICKED_OFF` fails `kickoff()`, one after it fails `complete()`. It is cancelled on `unstage()`, when `complete()`'s status is cancelled and when `kickoff()` is cancelled before `KICKED_OFF`, so `fly` must wait for everything it starts (for example with `async with AsyncStatus(...)`).
+  - `stop()` is now the abstract `ensure_stopped()`, which must wait until the hardware has stopped. `on_stage()` is now `ensure_ready()`, which defaults to `ensure_stopped()` and is called on `stage()`. `unstage()` cancels a `prepare()` or `fly` in progress, then always calls `ensure_stopped()`.
+- "Device <name> was stopped." is now raised as `StoppedError`, a `RuntimeError`, so `except RuntimeError` still catches it. It is raised by `StandardMovable` and `StandardFlyable` themselves, not by the logic: a `MovableLogic.move()` may end early without an error when stopped, as `Motor` and `SimMotor` now do.
+- `StandardMovable.stop()` ends the `set()` in progress, then calls `ensure_stopped()` without waiting for it to finish failing. `stop(success=True)` lets it finish normally, `stop(success=False)` fails it with `StoppedError`, even if its move has not started yet, in which case the hardware is not touched. A second `stop()` only calls `ensure_stopped()` again. It does not end a `prepare()` or fly, only `unstage()` does, but `ensure_stopped()` still halts the hardware, so a `stop()` during the run-up of `prepare()` can leave it finishing short of the start; a future `pause()` hook is the place to fix this. `StandardMovable.set()` is now a plain method returning a `WatchableAsyncStatus`.
+- A new `set()` on a `StandardMovable` ends the one in progress, which fails with `StoppedError` saying it was superseded; the hardware is not stopped, the new move retargets it.
+- `StandardMovable` is now stageable, but a `StandardMovable` alone does nothing on `stage()` or `unstage()`. A Device that also flies runs the `FlyableLogic`'s `ensure_ready()` on `stage()` and `ensure_stopped()` on `unstage()`, so it stops a `set()` in progress at both; the RunEngine already stops everything it set when a run ends or pauses.
+- `Motor` sends `.STOP` and waits for `.DMOV` every time `ensure_stopped()` is called, even if the motor looks stopped, as `.DMOV` can flicker at standstill on some motors; so every staged `Motor` gets one `.STOP` at `unstage()`. `Motor.stage()` stops a motion in progress rather than waiting for it.
+- `FlyableLogic.prepare_to_fly` and `FlyableLogic.fly` are annotated as returning `AsyncGenerator`; annotate overrides the same way.
+- `complete()` fails with `TypeError` if `fly` yields `KICKED_OFF` more than once, as for any other yield that is not a `WatcherUpdate`.
+- `MovableLogic.stop()` is now the abstract `ensure_stopped()`, which must wait until the motion has stopped, and `StandardMovable.stop()` returns once it has. A subclass that still defines `stop()` gets a `DeprecationWarning`, and its `stop()` is used as `ensure_stopped()`. `Motor` now waits for `.DMOV` after writing `.STOP`, allowing its acceleration time to decelerate, and `InstantMotorMock` finishes a move on `.STOP`.
+- `ophyd_async` now needs `typing-extensions>=4.14`, for `KICKED_OFF`. Type-checking a `fly` annotated with `KICKED_OFF` needs pyright 1.1.410 or later.
+- Renamed `MotorFlyCtx` to `MotorPrepareCtx`, `PmacFlyCtx` and `PmacPrepareContext` to `PmacPrepareCtx`, and `SimMotorMoveLogic` to `SimMotorLogic`.
+- A flyer must now be staged before it is prepared: `prepare()` is only allowed after `stage()`, `kickoff()` after `prepare()` and `complete()` after `kickoff()`, and a verb in the wrong state raises `TransitionError` (a `RuntimeError`). Staging an already-staged flyer, even one with a verb running, used to silently reset it and now raises (see below). A fly that fails leaves the flyer staged. `complete()` returns the same status each time it is called after a `kickoff()`, until the next `prepare()` or `stage()`. After `complete()` a flyer can be prepared again without restaging. Stock bluesky plans that stage their devices (`bp.count`, `bp.scan`) keep working; `bp.fly` neither stages nor prepares, so stage the flyer and prepare it before `bp.fly`, for example with `bpp.stage_wrapper`.
+- Staging any already-staged Standard device (`StandardReadable`, `StandardMovable`, `StandardFlyable`, ...) now raises `TransitionError`, as ophyd raises `RedundantStaging`. For example, nesting `bpp.stage_wrapper` around a plan that also stages the same device now fails.
+- `unstage()` now cancels the `prepare()` or fly that is running. `unstage()` waits for it before calling `ensure_stopped()`, then raises any error it raised while being cancelled.
+- `MovableLogic` and `FlyableLogic` now share the base `Logic`, an `ABC`, so a subclass missing an abstract hook (`ensure_stopped`, `prepare_to_fly` or `fly`) raises `TypeError` when it is constructed. See ADR 0022 for the reasoning behind these hooks.
+- `SimMotor` is now a `StandardFlyable`, so like any flyer it must be staged before `prepare()`.
+- `SimMotor.stage()` now calls `SimMotorLogic.ensure_ready()`, which calls `ensure_stopped()` and so stops the simulated motion, waits for it to finish, and sets the setpoint to the current readback. `unstage()` cancels a fly in progress, then calls `ensure_stopped()`. The simulated motion runs in the task of the verb that moves, so `stop()` during a `prepare()` or fly, or `stage()` during a `set()`, cancels that verb, which raises `CancelledError`.
+- The progress of `StandardFlyable.prepare()` now comes from the `WatcherUpdate`s the logic's `prepare_to_fly` yields, replacing the heuristic that watched setpoint writes (#1448).
+- `WatchableAsyncStatus` closes the async generator it drives as soon as it is cancelled or fails, rather than when the generator is collected, and a watcher that raises is logged and ignored rather than failing the status.
+
 ### Added
 
+- `KICKED_OFF`, the sentinel `FlyableLogic.fly` yields once the scan is up to speed.
+- `watch_readback()` reports the progress of a move by watching a `MovableLogic` readback, for use from `FlyableLogic` hooks.
+- `SimMotor.prepare()` and `SimMotor.complete()` report progress to watchers, as `Motor` does.
+- `TransitionError`, a `RuntimeError` raised by Standard devices for a verb called in the wrong state.
+- `StoppedError`, the `RuntimeError` raised when `stop()` or a new `set()` ends a move, prepare or fly early.
+- `cancel_and_wait()` cancels tasks and waits for all of them to end, even through a cancel of its caller, then raises the error of the first task, in the order given, that ended with one. If its caller was cancelled, it raises `CancelledError` instead.
 - `plugins_with_proc()` adds the process plugin made from a `proc_suffix` to a detector's `plugins`, and raises `ValueError` if `plugins` already has a `"proc"`. Every areaDetector vendor detector and `ContAcqDetector` uses it, so `"proc"` is reserved: pass `proc_suffix` instead.
+- `Logic`, the base of `MovableLogic` and `FlyableLogic`, with the abstract `ensure_stopped()`. `FlyableLogic` adds `ensure_ready()`, which defaults to it.
+
+### Fixed
+
+- `complete()` finishing after `unstage()` no longer makes the device look prepared or flown to a new fly.
+- `StandardReadable.add_children_as_readables()` registers the children it adds in the order they were declared, so `hints`, `read()` and `describe()` no longer vary from run to run.
+- `unstage()` now cancels a running fly, so the PMAC trajectory stops appending points and nothing is left running once the hardware is stopped.
+- `unstage()` now ends a running `complete()`, such as for the PandA.
+- A `prepare()` that fails or is cancelled now stops the move to the run-up start of a `Motor` or `SimMotor`.
+- A failed `prepare()` no longer leaves the flyer prepared with the failed context, so there is nothing to kick off.
+- `set_and_wait_for_other_value()` cancelled before the first value arrives, such as by `stop()` during a move, no longer leaves its value-watching task running.
+- A signal `get_value()`, `set()` or `trigger()` cancelled just as it finishes no longer swallows the cancellation on Python 3.11.
+- Leaving an `async with AsyncStatus(...)` block, `set_and_wait_for_other_value()` and the sim `BlobAcquireLogic.ensure_stopped()` no longer swallow a cancellation of the caller that arrives while they wait for the task they cancelled: they wait for that task to end, then raise the cancellation.

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

@@ -171,9 +171,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",

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,82 @@
+# 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 only half held. pyright did report a Device that forgot its logic, but
+`functools.cached_property` does not forward `__isabstractmethod__` from the function it
+wraps, so `StandardMovable.__abstractmethods__` was empty and at runtime the Device 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
+    @abstractmethod
+    def logic(self) -> MovableLogic[SignalDatatypeT]: ...
+
+class StandardFlyable(_StandardBase, ..., Generic[PrepareT, CtxT]):
+    @abstract_cached_property
+    @abstractmethod
+    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 no implementation. That last is also caught at
+instantiation, by stacking `abstract_cached_property` on `@abstractmethod`: pyright reads
+the `@abstractmethod`, and `abstract_cached_property` is 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__`.
+
+`logic` is public, so mock helpers such as `InstantMovableMock` and subclasses can reach
+the signals it holds.
+
+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.