diff --git a/CHANGELOG.md b/CHANGELOG.md index 908c10b569..dae311903c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,9 @@ - `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. +- `StandardReadable` now keeps one format per registered child, keyed by the child, so registering a child again replaces its format instead of adding a second contribution, and the order of `hints` is the order children were first registered. The private `_describe_config_funcs`, `_read_config_funcs` and `_has_hints` tuples are gone: a subclass that added to them now adds a hook to `_describe_configuration_funcs`, `_read_configuration_funcs` or `_hints_funcs` (see ADR 0023). +- The deprecated `ConfigSignal` and `HintedSignal` aliases are removed from `ophyd_async.core`; use `StandardReadableFormat.CONFIG_SIGNAL`, `HINTED_SIGNAL` or `HINTED_UNCACHED_SIGNAL`. `StandardReadable.add_readables()` is deprecated in favour of `set_readable_format()` on each device. +- `store_settings()` writes the readable format changes of the Device tree under the reserved key ``, and `apply_settings()` resets the tree to its class formats before applying them. Settings files stored before this have no such key and leave formats alone. `retrieve_settings()` warns about and drops any other `<...>` key. ### Added @@ -38,6 +41,9 @@ - `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. +- `StandardReadable.set_readable_format()`, `reset_readable_formats()`, `get_readable_formats()` and `get_readable_format_changes()` (which yield each child with its format), to change how a child contributes to `read()`, `read_configuration()` and `hints` at runtime, the equivalent of `Kind` in ophyd v1 (see "How to change what a Device reports at runtime"). +- `ReadableEntry`, `ReadableFormats`, `walk_readable_formats()`, `apply_readable_formats()` and `READABLE_FORMATS_KEY` in `ophyd_async.core`, and a `readable_formats` field on `Settings`, so formats are stored and retrieved with the signal values. +- pyright now runs with `reportMatchNotExhaustive`. ### Fixed diff --git a/docs/explanations/decisions/0023-runtime-readable-format.md b/docs/explanations/decisions/0023-runtime-readable-format.md new file mode 100644 index 0000000000..ee037a7d65 --- /dev/null +++ b/docs/explanations/decisions/0023-runtime-readable-format.md @@ -0,0 +1,125 @@ +# 23. Make readable format a runtime setting keyed by Device + +Date: 2026-08-18 + +## Status + +Accepted + +## Context + +In ophyd v1 a `Component`'s `kind` could be changed at runtime, so one Device could serve +several techniques: a monochromator's energy is configuration for one and a hinted read for +another. `StandardReadable` had no equivalent: `add_readables` appended bound methods into +tuples that did not record which device each came from, so nothing could be removed or +re-formatted afterwards (#1394). + +In #1395 `StandardDetector` had its own `add_config_signals`, and reviewers would not have +areaDetector plugins registered automatically: a detector carries far more plugins than are +wired into its chain, so reporting all of them hides which values mean anything. Per-plugin +opt-in after construction is the primitive #1394 asks for. + +Three designs were considered: + +- **Chain parsing**, registering what the plugin graph wires up. Rejected as too large, and + it fails once plugins cross IOCs, as every plugin would have to be exposed to ophyd. +- **One registry for formats and logic objects.** Rejected: a format is keyed by child, + unbounded in number and merged by union, where a logic is keyed by role, one per role, and + a conflict is an error, so the registry would branch on entry kind at every insert. + Changing a format alters `describe()`; changing a logic, the protocols the Device + satisfies. +- **A format registry keyed by device**, leaving logic objects alone. Chosen. + +## Decision + +### The registry is a `{device: format}` dict + +`StandardReadable` maps each registered child to its format, in registration order, and +derives each verb's callables on demand, so the verbs always see the current registry. +`set_readable_format(device, format)` sets one entry, and `None` removes it. + +- Keying by `Device` is identity keying, which a registry of children needs: `Device` has no + `__eq__`, and `DeviceVector` and `DeviceMap` hash by `id()`. +- A tuple of pairs was tried for immutability, but rebuilding it per registration made + construction O(n²): 77 ms for 2000 signals against 13 ms for the dict. +- It is a `cached_property`, because `AreaDetector` and `add_children_as_readables` register + children *before* `super().__init__()`, which would discard anything `__init__` assigned. +- Only a `SignalR` takes a signal format, so the verbs get entries already typed. +- Registering a device twice replaces its format, as the old double contribution was an + accident. +- The deprecated `ConfigSignal` and `HintedSignal` markers go: they compared equal to their + target, which only worked while `add_readables` matched at registration. + +### Runtime changes can be undone, and the baseline is sealed by a metaclass + +@oliwenmandiamond pointed out that a runtime change makes the class definition misleading, +and two safeguards were proposed: class-declared formats immutable unless opted out, or a +separate `StandardDynamicReadable`. Both were rejected, as they block the case this ADR +exists for: #1394's monochromator *declares* `energy` as configuration, and locking it would +make a beamline get the class changed to switch technique. @jwlodek added that ophyd v1 has +always been mutable, NSLS-II codebases rely on it, and differences between the libraries are +a cost. + +Adopted instead, from both: `StandardReadable.reset_readable_formats` puts a Device back +to what its class declared, so a plan can undo a retune without recording what it changed. +Its baseline is the class, so it discards a stored technique too. + +The baseline is sealed by a metaclass. Formats are declared in annotations, by registration +before `super().__init__()` (all 24 in-tree Devices) and by registration after it (legal, +though unused). `type.__call__` is the only point after all three; a snapshot at the end of +`__init__` was measured to miss the third, so the reset would silently drop a declared +child. `Device` already has a metaclass, so this adds no machinery. + +### Format changes apply between runs, not within one + +A change part way through a run would make `describe()` disagree with the descriptor emitted +at its start. This is documented rather than enforced, as a Device cannot see run +boundaries. + +### Formats are stored with settings, under a reserved key + +A technique change wants values and formats together, so they share one file and one +`apply_settings`. Values stay flat, and one reserved key holds the format changes: + +```yaml +energy: 7.0 +: + : + energy: HINTED_SIGNAL + temperature: null +``` + +- No version marker: a file without the key changes no formats. +- `<` cannot begin a Python identifier, so no attribute path collides with a reserved key. + Unlike `*FORMATS*` it needs no YAML quoting, and `` avoids the `? '' :` form + PyYAML emits for an empty key. `retrieve_settings` warns and drops an unknown `<...>` key, + so a newer file loads in an older version. +- Formats are keyed by owner, then child, because a format belongs to a pair: one child can + be registered on two owners with different formats. Recording the owner needs no "direct + child" rule, so `driver.acquire_time` and `DeviceVector` elements work. +- Formats are stored even by a config-only `store_settings`, as they decide which signals + are configuration. + +### Only changes are stored, and applying resets to the class defaults first + +`walk_readable_formats` stores how each `StandardReadable` differs from its construction, +and `apply_readable_formats` resets the tree to its class defaults, then applies that. +Rejected: + +- **Store everything, apply by replacing**: an old file silently unregisters a signal added + in a later version, and undoes a release's change of default. +- **Store everything, apply by merging**: cannot return from technique B to A, as what B + changed and A does not mention stays. + +Every path is resolved before anything changes, so a bad path cannot half apply. +`Settings.partition` copies the formats to both halves, because plans such as +`apply_panda_settings` apply only the halves, and a copy on one would let the plan decide +whether they are restored. Formats go after values, so a failed write leaves them untouched. + +## Consequences + +- A Device can switch, save and restore techniques at runtime, closing #1394. +- `add_readables` keeps working but gains "set" semantics, so it is deprecated in favour of + `set_readable_format`, which says what it does. +- A runtime change to a device registered outside its owner has no stable path, so + `walk_readable_formats` warns and skips it. diff --git a/docs/how-to/change-readable-format.md b/docs/how-to/change-readable-format.md new file mode 100644 index 0000000000..cabc2dceda --- /dev/null +++ b/docs/how-to/change-readable-format.md @@ -0,0 +1,88 @@ +# How to change what a Device reports at runtime + +A [](#StandardReadable) decides what appears in `read()`, `read_configuration()` and +`hints` from the [](#StandardReadableFormat) each of its children is registered with. +Normally that is fixed when the Device class is written, but it can also be changed on a +live Device with [](#StandardReadable.set_readable_format). + +This is the equivalent of changing a `Component`'s `kind` in ophyd v1. + +## Changing one signal + +A beamline with two techniques might scan a monochromator's energy in one and hold it +fixed in the other. The same Device serves both: + +```python +from ophyd_async.core import StandardReadableFormat as Format + +# Scanning technique: energy is a hinted read +mono.set_readable_format(mono.energy, Format.HINTED_SIGNAL) + +# Fixed technique: energy is configuration, recorded once per run +mono.set_readable_format(mono.energy, Format.CONFIG_SIGNAL) + +# Not interesting for this technique at all +mono.set_readable_format(mono.energy, None) +``` + +[](#StandardReadable.get_readable_formats) yields each child Device that is currently registered +with its format, so a child that does not contribute is simply absent: + +```python +for child, format in mono.get_readable_formats(): + print(child.name, format) +``` + +## Putting it back + +[](#StandardReadable.reset_readable_formats) undoes every runtime change, back to +how the class declared things: annotations, `add_children_as_readables`, and +anything the Device's own `__init__` registered: + +```python +mono.set_readable_format(mono.energy, Format.HINTED_SIGNAL) # retune for a technique +... +mono.reset_readable_formats() # back to the class defaults +``` + +So a plan that retunes a Device can put it back without having to record what it +changed. It resets that Device only, not its children. To do a whole tree: + +```python +from ophyd_async.core import apply_readable_formats + +apply_readable_formats(detector, {}) +``` + +```{note} +The baseline is what the Device class declares, so this also discards formats that came +from a stored settings file (see [](./store-and-retrieve.md)). +``` + +```{note} +Change formats **between runs**, not inside one. A run's descriptor is emitted when the +run starts, and [](#StandardReadableFormat.HINTED_SIGNAL) begins monitoring in `stage()`, +so a format changed part way through a run would leave `describe()` and `read()` +disagreeing. +``` + +## Opting a detector plugin in + +A [](#StandardDetector) is a [](#StandardReadable), so the same call works on one. This +matters most for areaDetector, where a detector often carries many more plugins than are +wired into its chain, so plugins are never registered automatically, and you opt in to +the ones that are actually producing meaningful values: + +```python +det.set_readable_format(det.stats.total, Format.HINTED_UNCACHED_SIGNAL) +``` + +Data produced by a [](#DetectorDataLogic) is added on top of these, so a detector can +write a file *and* report a plugin scalar in a step scan. + +```{seealso} +[](./store-and-retrieve.md) for saving a set of formats alongside signal values and +loading them back, and +[](../explanations/decisions/0023-runtime-readable-format.md) for why the registry is +keyed by Device. +``` diff --git a/docs/how-to/store-and-retrieve.md b/docs/how-to/store-and-retrieve.md index e91ec08dd5..4d51cf349e 100644 --- a/docs/how-to/store-and-retrieve.md +++ b/docs/how-to/store-and-retrieve.md @@ -43,3 +43,26 @@ def load_panda(panda1: HDFPanda): settings = yield from retrieve_settings(provider, "yaml_file_name", panda1) yield from apply_settings_if_different(settings, apply_panda_settings) ``` + +## Storing readable formats too + +As well as signal values, [](#store_settings) records how each child contributes to the Device's bluesky verbs (which signals are hinted, which are configuration) and [](#apply_settings) restores that alongside the values. The two change on the same cadence, so they live in one file and are applied by one call. This means a stored file can switch a Device between techniques as well as putting its values back; see [](./change-readable-format.md) for changing a format directly. + +Values stay flat in the yaml. Formats go under the reserved [](#READABLE_FORMATS_KEY), and only what has changed since the Device was constructed is recorded, so the file stays short and can be edited by hand: + +```yaml +energy: 7.0 +temperature: 20.0 +: + : + energy: HINTED_SIGNAL + temperature: null + stats: + total: HINTED_UNCACHED_SIGNAL +``` + +The outer key of each formats entry is the [](#StandardReadable) the child is registered on, with `` standing for the Device you stored. The inner key is the child's path from that owner. The owner is recorded rather than worked out from the child's path, because the same child can be registered on more than one Device with a different format each time. A null means the child has been dropped. + +Applying first resets every Device in the tree to the formats its class declares, then applies the changes in the file. So a file stored with nothing changed (`: {}`) puts the class defaults back, and a signal added to a Device class after the file was stored keeps the format the class gives it. + +A file with no `` key changes no formats, so settings files saved before formats were storable keep working untouched. A file with *only* that key switches technique without writing a single value to hardware. diff --git a/pyproject.toml b/pyproject.toml index d9d7ca5652..849b961408 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -92,6 +92,7 @@ version_file = "src/ophyd_async/_version.py" [tool.pyright] typeCheckingMode = "standard" reportMissingImports = false # Ignore missing stubs in imported modules +reportMatchNotExhaustive = true [tool.pytest.ini_options] # Run pytest with all our checkers, and don't spam us with massive tracebacks on error diff --git a/src/ophyd_async/core/__init__.py b/src/ophyd_async/core/__init__.py index 74e1115149..9022728341 100644 --- a/src/ophyd_async/core/__init__.py +++ b/src/ophyd_async/core/__init__.py @@ -99,10 +99,13 @@ Watcher, ) from ._readable import ( - ConfigSignal, - HintedSignal, + READABLE_FORMATS_KEY, + ReadableEntry, + ReadableFormats, StandardReadable, StandardReadableFormat, + apply_readable_formats, + walk_readable_formats, ) from ._settings import Settings, SettingsProvider from ._signal import ( @@ -279,6 +282,11 @@ def __getattr__(name): # Readable "StandardReadable", "StandardReadableFormat", + "ReadableEntry", + "ReadableFormats", + "walk_readable_formats", + "apply_readable_formats", + "READABLE_FORMATS_KEY", # Detector "DetectorTrigger", "TriggerInfo", @@ -343,8 +351,6 @@ def __getattr__(name): "DerivedSignalFactory", "merge_gathered_dicts", # Back compat - delete before 1.0 - "ConfigSignal", - "HintedSignal", # Standard enums "EnabledDisabled", "EnableDisable", diff --git a/src/ophyd_async/core/_readable.py b/src/ophyd_async/core/_readable.py index 2e378bcb80..7ea972fd92 100644 --- a/src/ophyd_async/core/_readable.py +++ b/src/ophyd_async/core/_readable.py @@ -1,15 +1,25 @@ +import asyncio +import itertools import warnings -from collections.abc import Awaitable, Callable, Generator, Sequence +from collections.abc import ( + Awaitable, + Callable, + Generator, + Iterator, + MutableMapping, + Sequence, +) from contextlib import contextmanager from enum import Enum -from typing import Any, cast +from functools import cached_property +from typing import Literal from bluesky.protocols import HasHints, Hints, Reading from event_model import DataKey from ._device import Device, DeviceMap, DeviceVector from ._protocol import AsyncConfigurable, AsyncReadable, AsyncStageable -from ._signal import SignalR +from ._signal import SignalR, walk_devices from ._standard_base import _StandardBase from ._utils import merge_gathered_dicts @@ -49,38 +59,86 @@ class StandardReadableFormat(Enum): def __call__(self, parent: Device, child: Device): if not isinstance(parent, StandardReadable): raise TypeError(f"Expected parent to be StandardReadable, got {parent}") - parent.add_readables([child], self) + parent.set_readable_format(child, self) -# Back compat -class _WarningMatcher: - def __init__(self, name: str, target: StandardReadableFormat): - self._name = name - self._target = target +# Short alias for the case statements below +Format = StandardReadableFormat - def __eq__(self, value: object) -> bool: - warnings.warn( - DeprecationWarning( - f"Use `StandardReadableFormat.{self._target.name}` " - f"instead of `{self._name}`" - ), - stacklevel=2, - ) - return value == self._target +SignalFormat = Literal[ + Format.CONFIG_SIGNAL, + Format.HINTED_SIGNAL, + Format.UNCACHED_SIGNAL, + Format.HINTED_UNCACHED_SIGNAL, +] +#: A registered child and its format. Only a `SignalR` has a signal format. +ReadableEntry = tuple[Device, Literal[Format.CHILD]] | tuple[SignalR, SignalFormat] -def _compat_format(name: str, target: StandardReadableFormat) -> StandardReadableFormat: - return cast(StandardReadableFormat, _WarningMatcher(name, target)) +class _Readables(MutableMapping[Device, StandardReadableFormat]): + """The registered children and their formats, in registration order. -ConfigSignal = _compat_format("ConfigSignal", StandardReadableFormat.CONFIG_SIGNAL) -HintedSignal: Any = _compat_format("HintedSignal", StandardReadableFormat.HINTED_SIGNAL) -HintedSignal.uncached = _compat_format( - "HintedSignal.uncached", StandardReadableFormat.HINTED_UNCACHED_SIGNAL -) + Only a `SignalR` may be given a signal format. + """ + + def __init__(self) -> None: + self._entries: dict[Device, ReadableEntry] = {} + + def __getitem__(self, device: Device) -> StandardReadableFormat: + return self._entries[device][1] + + def __setitem__(self, device: Device, format: StandardReadableFormat) -> None: + if not isinstance(format, StandardReadableFormat): + raise TypeError(f"{format} is not a StandardReadableFormat") + match format: + case Format.CHILD: + self._entries[device] = (device, format) + case ( + Format.CONFIG_SIGNAL + | Format.HINTED_SIGNAL + | Format.UNCACHED_SIGNAL + | Format.HINTED_UNCACHED_SIGNAL + ): + if not isinstance(device, SignalR): + raise TypeError(f"{device} is not a SignalR") + self._entries[device] = (device, format) + + def __delitem__(self, device: Device) -> None: + del self._entries[device] + + def __iter__(self) -> Iterator[Device]: + return iter(self._entries) + + def __len__(self) -> int: + return len(self._entries) + + def entries(self) -> list[ReadableEntry]: + """A snapshot of each child and its format, in registration order.""" + return list(self._entries.values()) + + +class _SealDefaultFormatsMeta(type(_StandardBase)): # type: ignore[misc] + """Record the formats a `StandardReadable` was constructed with. + + The snapshot is taken after the whole `__init__` chain has returned, so it + holds the formats from annotations and from registration before or after + `super().__init__()`. + """ + def __call__(cls, *args, **kwargs): + self = super().__call__(*args, **kwargs) + self._default_readables = dict(self._readables) + return self -class StandardReadable(_StandardBase, AsyncReadable, AsyncConfigurable, HasHints): + +class StandardReadable( + _StandardBase, + AsyncReadable, + AsyncConfigurable, + HasHints, + metaclass=_SealDefaultFormatsMeta, +): """Device that provides selected child Device values in `read()`. Provides the ability for children to be registered to: @@ -89,24 +147,184 @@ class StandardReadable(_StandardBase, AsyncReadable, AsyncConfigurable, HasHints - Provide their value in `read_configuration()` and `describe_configuration() - Select a value to appear in `hints` - The behavior is customized with a [](#StandardReadableFormat) + The behavior is customized with a [](#StandardReadableFormat), which can be + changed at runtime with [](#StandardReadable.set_readable_format). """ - # These must be immutable types to avoid accidental sharing between - # different instances of the class - _describe_config_funcs: tuple[Callable[[], Awaitable[dict[str, DataKey]]], ...] = () - _read_config_funcs: tuple[Callable[[], Awaitable[dict[str, Reading]]], ...] = () - _describe_funcs: tuple[Callable[[], Awaitable[dict[str, DataKey]]], ...] = () + @cached_property + def _readables(self) -> _Readables: + """The registered children and their formats, in registration order. + + A `cached_property`, so `add_children_as_readables` and + `set_readable_format` can be called before `super().__init__()`. + """ + return _Readables() + + # The formats declared during construction, sealed by the metaclass once + # __init__ has fully returned. Only ever replaced, never mutated in place, + # so the shared class level default is safe. + _default_readables: dict[Device, StandardReadableFormat] = {} + + # What contributes to each verb, gathered in parallel by the verb methods. + # Immutable defaults to avoid accidental sharing between instances. A + # subclass producing data from somewhere other than a registered child adds + # to these rather than overriding the verbs, so registered children keep + # working; `StandardDetector` does it for its data logics. Each registry + # is named after the method that gathers it, and each hook added to it is + # `__on_`, name-mangled so mix-ins in one MRO never collide. _read_funcs: tuple[Callable[[], Awaitable[dict[str, Reading]]], ...] = () - _has_hints: tuple[HasHints, ...] = () + _describe_funcs: tuple[Callable[[], Awaitable[dict[str, DataKey]]], ...] = () + _read_configuration_funcs: tuple[ + Callable[[], Awaitable[dict[str, Reading]]], ... + ] = () + _describe_configuration_funcs: tuple[ + Callable[[], Awaitable[dict[str, DataKey]]], ... + ] = () + _hints_funcs: tuple[Callable[[], Iterator[Hints]], ...] = () + + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + # Each of these derives from _readables on every call rather than being + # registered up-front, so a format changed between runs is picked up on + # the next stage() or the next descriptor. + self._stage_funcs += (self.__on_stage,) + self._unstage_funcs += (self.__on_unstage,) + self._read_funcs += (self.__on_read,) + self._describe_funcs += (self.__on_describe,) + self._read_configuration_funcs += (self.__on_read_configuration,) + self._describe_configuration_funcs += (self.__on_describe_configuration,) + self._hints_funcs += (self.__on_hints,) + + def _signals_to_stage(self) -> Iterator[AsyncStageable]: + # HINTED_SIGNAL stages so that caching is set up and read() is fast; a + # CHILD stages because it may have staging of its own. The uncached and + # config formats deliberately do not. + for entry in self._readables.entries(): + match entry: + case (device, Format.CHILD): + if isinstance(device, AsyncStageable): + yield device + case (signal, Format.HINTED_SIGNAL): + yield signal + case ( + _, + Format.CONFIG_SIGNAL + | Format.UNCACHED_SIGNAL + | Format.HINTED_UNCACHED_SIGNAL, + ): + pass + + async def __on_stage(self) -> None: + await asyncio.gather(*(sig.stage().task for sig in self._signals_to_stage())) + + async def __on_unstage(self) -> None: + await asyncio.gather(*(sig.unstage().task for sig in self._signals_to_stage())) + + async def __on_describe_configuration(self) -> dict[str, DataKey]: + """What the registered children contribute to `describe_configuration()`.""" + coros = [] + for entry in self._readables.entries(): + match entry: + case (device, Format.CHILD): + if isinstance(device, AsyncConfigurable): + coros.append(device.describe_configuration()) + case (signal, Format.CONFIG_SIGNAL): + coros.append(signal.describe()) + case ( + _, + Format.HINTED_SIGNAL + | Format.UNCACHED_SIGNAL + | Format.HINTED_UNCACHED_SIGNAL, + ): + pass + return await merge_gathered_dicts(coros) + + async def __on_read_configuration(self) -> dict[str, Reading]: + """What the registered children contribute to `read_configuration()`.""" + coros = [] + for entry in self._readables.entries(): + match entry: + case (device, Format.CHILD): + if isinstance(device, AsyncConfigurable): + coros.append(device.read_configuration()) + case (signal, Format.CONFIG_SIGNAL): + coros.append(signal.read()) + case ( + _, + Format.HINTED_SIGNAL + | Format.UNCACHED_SIGNAL + | Format.HINTED_UNCACHED_SIGNAL, + ): + pass + return await merge_gathered_dicts(coros) + + async def __on_describe(self) -> dict[str, DataKey]: + """What the registered children contribute to `describe()`.""" + coros = [] + for entry in self._readables.entries(): + match entry: + case (device, Format.CHILD): + if isinstance(device, AsyncReadable): + coros.append(device.describe()) + case ( + signal, + Format.HINTED_SIGNAL + | Format.UNCACHED_SIGNAL + | Format.HINTED_UNCACHED_SIGNAL, + ): + coros.append(signal.describe()) + case (_, Format.CONFIG_SIGNAL): + pass + return await merge_gathered_dicts(coros) + + async def __on_read(self) -> dict[str, Reading]: + """What the registered children contribute to `read()`.""" + coros = [] + for entry in self._readables.entries(): + match entry: + case (device, Format.CHILD): + if isinstance(device, AsyncReadable): + coros.append(device.read()) + case (signal, Format.HINTED_SIGNAL): + coros.append(signal.read()) + case ( + signal, + Format.UNCACHED_SIGNAL | Format.HINTED_UNCACHED_SIGNAL, + ): + coros.append(signal.read(cached=False)) + case (_, Format.CONFIG_SIGNAL): + pass + return await merge_gathered_dicts(coros) + + def __on_hints(self) -> Iterator[Hints]: + """What the registered children contribute to `hints`.""" + for entry in self._readables.entries(): + match entry: + case (device, Format.CHILD): + if isinstance(device, HasHints): + yield device.hints + case ( + signal, + Format.HINTED_SIGNAL | Format.HINTED_UNCACHED_SIGNAL, + ): + # An unnamed signal has nothing to put in `fields`, but is + # still hinted, so it contributes the key and no name + yield {"fields": [signal.name] if signal.name else []} + case ( + _, + Format.CONFIG_SIGNAL | Format.UNCACHED_SIGNAL, + ): + pass async def describe_configuration(self) -> dict[str, DataKey]: return await merge_gathered_dicts( - [func() for func in self._describe_config_funcs] + [func() for func in self._describe_configuration_funcs] ) async def read_configuration(self) -> dict[str, Reading]: - return await merge_gathered_dicts([func() for func in self._read_config_funcs]) + return await merge_gathered_dicts( + [func() for func in self._read_configuration_funcs] + ) async def describe(self) -> dict[str, DataKey]: return await merge_gathered_dicts([func() for func in self._describe_funcs]) @@ -122,14 +340,16 @@ def hints(self) -> Hints: flattened_dimensions = [] hints: Hints = {} - for new_hint in self._has_hints: + for new_hint in itertools.chain.from_iterable( + func() for func in self._hints_funcs + ): # If a hints key in a subdevice isn't given in bluesky protocols, then it # doesn't carry over to the device holding it. - for key, value in new_hint.hints.items(): + for key, value in new_hint.items(): if key in ("fields", "dimensions"): if isinstance(value, str) or not isinstance(value, Sequence): raise RuntimeError( - f"{new_hint.name}: '{key}' key must be a Sequence, " + f"{self.name}: '{key}' key must be a Sequence, " f"received {type(value)}" ) already_parsed = ( @@ -158,17 +378,16 @@ def add_children_as_readables( self, format: StandardReadableFormat = StandardReadableFormat.CHILD, ) -> Generator[None, None, None]: - """Context manager that calls [](#add_readables) on child Devices added within. + """Register child Devices added within with the given format. Scans `self.children()` on entry and exit to context manager, and calls - `add_readables()` on any that are added with the provided - `StandardReadableFormat`. + [](#StandardReadable.set_readable_format) on any that are added with the + provided `StandardReadableFormat`. """ dict_copy = dict(self.children()) yield - # Set symmetric difference operator gives all newly added keys. new_dict = dict(self.children()) for key, value in new_dict.items(): # Check if key already exists in dict_copy and if the value has changed. @@ -193,9 +412,90 @@ def add_children_as_readables( else: flattened_values.append(value) - new_devices = list(filter(lambda x: isinstance(x, Device), flattened_values)) - self.add_readables(new_devices, format) + for device in flattened_values: + if isinstance(device, Device): + self.set_readable_format(device, format) + + def set_readable_format( + self, device: Device, format: StandardReadableFormat | None + ) -> None: + """Set how a child Device contributes to this Device's bluesky verbs. + This is the runtime equivalent of the `Kind` attribute in ophyd v1: it + can be called after construction to move a child between configuration, + hinted and uncached reads, or to drop it from the verbs entirely. + + Call it **between runs**. The descriptor for a run is emitted at its + start, and [](#StandardReadableFormat.HINTED_SIGNAL) sets up caching in + `stage()`, so changing a format part way through a run would make + `describe()` and `read()` disagree. + + :param device: The Device to set the format of, normally a child of this one + :param format: + The format to give it, or `None` to stop it contributing at all. + Replaces any format the device already has, rather than adding a + second contribution. + :raises TypeError: If a signal-only format is given a non-`SignalR` + """ + if format is None: + self._readables.pop(device, None) + else: + # Re-formatting an already registered child keeps its position, so + # changing a format does not reorder `hints` + self._readables[device] = format + + def reset_readable_formats(self) -> None: + """Undo every runtime format change, back to how the class declared them. + + The baseline is everything registered by the time construction + finished: annotations, `add_children_as_readables`, and any + [](#StandardReadable.set_readable_format) an `__init__` made. Anything + done after that is undone: switching a child between config and hinted, + dropping one with a format of `None`, or registering a new one. + + Intended for a Device that is retuned per technique, so a scan can put + it back without knowing what it changed. + + This resets **this Device only**, not its children. To reset a whole + tree use `apply_readable_formats(device, {})`. + + The baseline is the *class declaration*, not the last stored settings + file, so this discards a technique loaded by [](#apply_settings) too. + """ + # Mutated rather than replaced, so the cached_property stays the same + # object and nothing holding a reference sees a stale registry + self._readables.clear() + self._readables.update(self._default_readables) + + def get_readable_formats(self) -> Iterator[ReadableEntry]: + """Yield each registered child and its format, in registration order. + + A Device that does not contribute is not yielded. It is safe to call + [](#StandardReadable.set_readable_format) while iterating. + """ + return iter(self._readables.entries()) + + def get_readable_format_changes( + self, + ) -> Iterator[tuple[Device, StandardReadableFormat | None]]: + """Yield the formats that differ from those declared at construction. + + A child registered at construction but since dropped has a format of `None`. + It is safe to call [](#StandardReadable.set_readable_format) while iterating. + """ + changes: list[tuple[Device, StandardReadableFormat | None]] = [ + (child, format) + for child, format in self._readables.items() + if self._default_readables.get(child) is not format + ] + changes.extend( + (child, None) + for child in self._default_readables + if child not in self._readables + ) + return iter(changes) + + # Back compat - delete before 1.0 def add_readables( self, devices: Sequence[Device], @@ -203,78 +503,140 @@ def add_readables( ) -> None: """Add devices to contribute to various bluesky verbs. - Use output from the given devices to contribute to the verbs of the following - interfaces: - - - [](#bluesky.protocols.Readable) - - [](#bluesky.protocols.Configurable) - - [](#bluesky.protocols.Stageable) - - [](#bluesky.protocols.HasHints) - :param devices: The devices to be added :param format: Determines which of the devices functions are added to which verb as per the [](#StandardReadableFormat) documentation """ + warnings.warn( + DeprecationWarning( + "Use `set_readable_format(device, format)` on each device " + "instead of `add_readables(devices, format)`" + ), + stacklevel=2, + ) + for device in devices: + self.set_readable_format(device, format) - def assert_device_is_signalr(device: Device) -> SignalR: - if not isinstance(device, SignalR): - raise TypeError(f"{device} is not a SignalR") - return device - for device in devices: - match format: - case StandardReadableFormat.CHILD: - if isinstance(device, AsyncConfigurable): - self._describe_config_funcs += (device.describe_configuration,) - self._read_config_funcs += (device.read_configuration,) - if isinstance(device, AsyncReadable): - self._describe_funcs += (device.describe,) - self._read_funcs += (device.read,) - if isinstance(device, AsyncStageable): - self._stage_funcs += (device.stage,) - self._unstage_funcs += (device.unstage,) - if isinstance(device, HasHints): - self._has_hints += (device,) - case StandardReadableFormat.CONFIG_SIGNAL: - signalr_device = assert_device_is_signalr(device=device) - self._describe_config_funcs += (signalr_device.describe,) - self._read_config_funcs += (signalr_device.read,) - case StandardReadableFormat.HINTED_SIGNAL: - signalr_device = assert_device_is_signalr(device=device) - self._describe_funcs += (signalr_device.describe,) - self._read_funcs += (signalr_device.read,) - self._stage_funcs += (signalr_device.stage,) - self._unstage_funcs += (signalr_device.unstage,) - self._has_hints += (_HintsFromName(signalr_device),) - case StandardReadableFormat.UNCACHED_SIGNAL: - signalr_device = assert_device_is_signalr(device=device) - self._describe_funcs += (signalr_device.describe,) - self._read_funcs += (_UncachedRead(signalr_device),) - case StandardReadableFormat.HINTED_UNCACHED_SIGNAL: - signalr_device = assert_device_is_signalr(device=device) - self._describe_funcs += (signalr_device.describe,) - self._read_funcs += (_UncachedRead(signalr_device),) - self._has_hints += (_HintsFromName(signalr_device),) - - -class _UncachedRead: - def __init__(self, signal: SignalR) -> None: - self.signal = signal - - async def __call__(self) -> dict[str, Reading]: - return await self.signal.read(cached=False) - - -class _HintsFromName(HasHints): - def __init__(self, device: Device) -> None: - self.device = device +#: Reserved key under which [](#store_settings) writes readable formats. Chosen +#: so that it cannot be a dotted attribute path: `<` cannot start a Python +#: identifier, so no attribute assignment can produce a colliding key, and +#: unlike `*FORMATS*` it needs no quoting in yaml. +READABLE_FORMATS_KEY = "" - @property - def name(self) -> str: - return self.device.name +#: The key standing in for the root Device itself in a [](#ReadableFormats), +#: where every other key is a dotted attribute path. Not a valid Python +#: identifier, so no attribute assignment can produce a colliding path, and +#: unlike `""` it does not make yaml fall back to its hard-to-read explicit +#: key syntax. +ROOT_DEVICE_KEY = "" - @property - def hints(self) -> Hints: - fields = [self.name] if self.name else [] - return {"fields": fields} +#: The changes to the readable formats of a Device tree since construction, in +#: a form that can be stored and retrieved: +#: `{path of the StandardReadable: {path of the child: format}}`, where a format +#: of `None` means "stop this child contributing". +#: The owner path is a dotted attribute path from the root Device, as produced +#: by [](#walk_devices), with `ROOT_DEVICE_KEY` meaning the root Device +#: itself. The child path is a dotted attribute path from the owner. +ReadableFormats = dict[str, dict[str, StandardReadableFormat | None]] + + +def _paths_to_devices(device: Device) -> dict[str, Device]: + return {ROOT_DEVICE_KEY: device, **walk_devices(device)} + + +def walk_readable_formats(device: Device) -> ReadableFormats | None: + """Retrieve how the readable formats in a Device tree differ from construction. + + Used as part of saving a Device, so that which signals are hinted or + config can be restored later. Only the differences from what each + `StandardReadable` registered during construction are returned, with a + child it has since dropped given a format of `None`. + + :param device: The root Device to walk. + :return: + A [](#ReadableFormats) suitable for storing, or `None` if there is no + `StandardReadable` in the tree. + """ + paths_to_devices = _paths_to_devices(device) + readables = { + path: dev + for path, dev in paths_to_devices.items() + if isinstance(dev, StandardReadable) + } + if not readables: + return None + formats: ReadableFormats = {} + for path, dev in readables.items(): + device_to_path = {child: p for p, child in walk_devices(dev).items()} + entries: dict[str, StandardReadableFormat | None] = {} + for child, format in dev.get_readable_format_changes(): + child_path = device_to_path.get(child) + if child_path is None: + # Not below its owner, so it has no stable path to store it + # against. See https://github.com/bluesky/ophyd-async/issues/1402 + warnings.warn( + f"{dev.name or dev}: {child.name or child} is not within " + f"{dev.name or dev}, so the change to its readable format " + "will not be stored", + stacklevel=2, + ) + else: + entries[child_path] = format + if entries: + formats[path] = entries + return formats + + +def apply_readable_formats(device: Device, formats: ReadableFormats) -> None: + """Reset the readable formats in a Device tree, then apply the given changes. + + Every `StandardReadable` in the tree is first put back to what it + registered during construction, as + [](#StandardReadable.reset_readable_formats) does, then the changes in + `formats` are applied on top. So an empty `formats` restores the class + defaults, and a child added to a Device since `formats` was stored keeps + the format its class gives it. + + To stop a child contributing, give it a format of `None`, which in a stored + file is a null: + + ```yaml + : + : + energy: HINTED_SIGNAL + temperature: null + ``` + + :param device: The root Device the owner paths in `formats` are relative to. + :param formats: A [](#ReadableFormats), e.g. from [](#walk_readable_formats). + :raises KeyError: If a path does not name a Device in the tree. + :raises TypeError: If an owner path does not name a `StandardReadable`. + """ + devices = _paths_to_devices(device) + resolved: list[ + tuple[StandardReadable, list[tuple[Device, StandardReadableFormat | None]]] + ] = [] + # Resolve everything before changing anything, so a bad path cannot leave + # the tree half applied + for owner_path, entries in formats.items(): + owner = devices[owner_path] + if not isinstance(owner, StandardReadable): + raise TypeError(f"{owner_path!r} is not a StandardReadable, got {owner}") + children = walk_devices(owner) + readables: list[tuple[Device, StandardReadableFormat | None]] = [] + for child_path, format in entries.items(): + readables.append( + ( + children[child_path], + None if format is None else StandardReadableFormat(format), + ) + ) + resolved.append((owner, readables)) + for dev in devices.values(): + if isinstance(dev, StandardReadable): + dev.reset_readable_formats() + for owner, readables in resolved: + for child, format in readables: + owner.set_readable_format(child, format) diff --git a/src/ophyd_async/core/_settings.py b/src/ophyd_async/core/_settings.py index 4edc87b2e5..ded01b24c8 100644 --- a/src/ophyd_async/core/_settings.py +++ b/src/ophyd_async/core/_settings.py @@ -5,6 +5,7 @@ from typing import Any, Generic from ._device import Device, DeviceT +from ._readable import ReadableFormats from ._signal import SignalRW from ._signal_backend import SignalDatatypeT @@ -33,11 +34,26 @@ class Settings(MutableMapping[SignalRW[Any], Any], Generic[DeviceT]): """ def __init__( - self, device: DeviceT, settings: MutableMapping[SignalRW, Any] | None = None + self, + device: DeviceT, + settings: MutableMapping[SignalRW, Any] | None = None, + readable_formats: ReadableFormats | None = None, ): self.device = device self._settings = {} self.update(settings or {}) + self.readable_formats: ReadableFormats | None = ( + None + if readable_formats is None + else {owner: dict(entries) for owner, entries in readable_formats.items()} + ) + """How the Device's children contribute to its bluesky verbs. + + Stored and applied alongside the signal values, as the two change on + the same cadence. Applied by [](#apply_settings) with + [](#apply_readable_formats), so a dict (even an empty one) first resets + the tree to its class defaults, while `None` leaves formats alone. + """ def __getitem__(self, key: SignalRW[SignalDatatypeT]) -> SignalDatatypeT: return self._settings[key] @@ -70,10 +86,37 @@ def __len__(self) -> int: return len(self._settings) def __or__(self, other: MutableMapping[SignalRW, Any]) -> Settings[DeviceT]: - """Create a new Settings that is the union of self overridden by other.""" - if isinstance(other, Settings) and not self._is_in_device(other.device): - raise ValueError(f"{other.device} is not a child of {self.device}") - return Settings(self.device, self._settings | dict(other)) + """Create a new Settings that is the union of self overridden by other. + + Readable formats merge per (owner, child), the same granularity that + values merge at, so `other` only replaces the children it mentions. + + :raises ValueError: + If `other` is a `Settings` for a Device outside this one, or for a + Device below this one while carrying readable formats. + """ + formats = self.readable_formats + if isinstance(other, Settings): + if other.device is not self.device: + if not self._is_in_device(other.device): + msg = f"{other.device} is not a child of {self.device}" + raise ValueError(msg) + if other.readable_formats is not None: + # Re-rooting the owner paths is easy, but applying resets + # the whole tree, so a sub-device's formats would reset + # Devices it knows nothing about + msg = ( + f"Cannot merge readable formats from {other.device}, whose " + f"paths are relative to it rather than to {self.device}" + ) + raise ValueError(msg) + if other.readable_formats is not None: + formats = { + owner: dict(entries) for owner, entries in (formats or {}).items() + } + for owner, entries in other.readable_formats.items(): + formats.setdefault(owner, {}).update(entries) + return Settings(self.device, self._settings | dict(other), formats) def partition( self, predicate: Callable[[SignalRW], bool] @@ -88,13 +131,17 @@ def partition( The first contains the signals for which the predicate returned True, and the second contains the signals for which the predicate returned False. + Both halves carry the same `readable_formats`. Applying them twice is + harmless: it writes to no hardware and gives the same result. + :example: ```python settings = Settings(device, {device.special: 1, device.sig: 2}) specials, others = settings.partition(lambda sig: "special" in sig.name) ``` """ - where_true, where_false = Settings(self.device), Settings(self.device) + where_true = Settings(self.device, readable_formats=self.readable_formats) + where_false = Settings(self.device, readable_formats=self.readable_formats) for signal, value in self.items(): dest = where_true if predicate(signal) else where_false dest[signal] = value diff --git a/src/ophyd_async/plan_stubs/_settings.py b/src/ophyd_async/plan_stubs/_settings.py index 5e75c6eb83..2d86fb4da4 100644 --- a/src/ophyd_async/plan_stubs/_settings.py +++ b/src/ophyd_async/plan_stubs/_settings.py @@ -9,12 +9,17 @@ from bluesky.utils import MsgGenerator, plan from ophyd_async.core import ( + READABLE_FORMATS_KEY, Device, + ReadableFormats, Settings, SettingsProvider, SignalRW, + StandardReadableFormat, Table, + apply_readable_formats, walk_config_signals, + walk_readable_formats, walk_rw_signals, ) @@ -53,21 +58,37 @@ def get_current_settings( def store_settings( provider: SettingsProvider, name: str, device: Device, only_config: bool = False ) -> MsgGenerator[None]: - """Walk a Device for SignalRWs and store their values. + """Walk a Device for SignalRWs and store their values and readable formats. If `only_config` is True, store only configuration settings on `Configurable`. + Readable formats are stored in both cases. + + Values are stored flat, keyed by dotted attribute path. The changes to + readable formats since construction go under the reserved + [](#READABLE_FORMATS_KEY). It is written, even if empty, whenever the tree + contains a `StandardReadable`, as applying it resets formats to the class + defaults. :param provider: The provider to store the settings with. :param name: The name to store the settings under. :param device: The Device to walk for SignalRWs. - :param only_config: If True, store only configuration settings. + :param only_config: If True, store only configuration signal values. """ if only_config: signals = yield from wait_for_awaitable(walk_config_signals(device)) else: signals = walk_rw_signals(device) - named_values = yield from _get_values_of_signals(signals) - yield from wait_for_awaitable(provider.store(name, named_values)) + data: dict[str, Any] = yield from _get_values_of_signals(signals) + formats = walk_readable_formats(device) + if formats is not None: + data[READABLE_FORMATS_KEY] = { + owner: { + child: None if format is None else format.value + for child, format in entries.items() + } + for owner, entries in formats.items() + } + yield from wait_for_awaitable(provider.store(name, data)) @plan @@ -78,26 +99,54 @@ def retrieve_settings( If `only_config` is True, retrieve only configuration settings on `Configurable`. + A file with no [](#READABLE_FORMATS_KEY), such as one stored before + readable formats existed, gives Settings that leave formats alone. + :param provider: The provider to retrieve the settings from. :param name: The name of the settings to retrieve. :param device: The Device to retrieve the settings for. :param only_config: If True, retrieve only configuration settings. """ - named_values = yield from wait_for_awaitable(provider.retrieve(name)) + # Copy, as the provider may hand back a dict it holds on to and the reserved + # keys are popped out of it below + data = dict((yield from wait_for_awaitable(provider.retrieve(name)))) + # Reserved keys first, so what is left is signal paths. + # Present but null, as a hand written ":" would be, still + # means reset to the class defaults + stored_formats = ( + data.pop(READABLE_FORMATS_KEY) or {} if READABLE_FORMATS_KEY in data else None + ) + readable_formats: ReadableFormats | None = ( + None + if stored_formats is None + else { + owner: { + # A null means "stop this child contributing", so is not coerced + child: None if format is None else StandardReadableFormat(format) + for child, format in entries.items() + } + for owner, entries in stored_formats.items() + } + ) if only_config: signals = yield from wait_for_awaitable(walk_config_signals(device)) else: signals = walk_rw_signals(device) - unknown_names = set(named_values) - set(signals) + unknown_names = set(data) - set(signals) if unknown_names: raise NameError(f"Unknown signal names {sorted(unknown_names)}") - signal_values = {signals[name]: value for name, value in named_values.items()} - return Settings(device, signal_values) + signal_values = {signals[name]: value for name, value in data.items()} + return Settings(device, signal_values, readable_formats) @plan def apply_settings(settings: Settings) -> MsgGenerator[None]: - """Set every SignalRW to the given value in Settings. If value is None ignore it.""" + """Set every SignalRW to the given value in Settings. If value is None ignore it. + + Then apply any readable formats the Settings carries with + [](#apply_readable_formats). Formats go last, so a failed value write leaves + them untouched. + """ signal_values = { signal: value for signal, value in settings.items() if value is not None } @@ -105,6 +154,8 @@ def apply_settings(settings: Settings) -> MsgGenerator[None]: for signal, value in signal_values.items(): yield from bps.abs_set(signal, value, group="apply_settings") yield from bps.wait("apply_settings") + if settings.readable_formats is not None: + apply_readable_formats(settings.device, settings.readable_formats) @plan diff --git a/src/ophyd_async/tango/core/_tango_transport.py b/src/ophyd_async/tango/core/_tango_transport.py index 3585337884..d063458cce 100644 --- a/src/ophyd_async/tango/core/_tango_transport.py +++ b/src/ophyd_async/tango/core/_tango_transport.py @@ -752,6 +752,8 @@ def make_converter(info: AttributeConfig | CommandConfig, datatype) -> TangoConv AttrDataFormat.IMAGE, ]: return TangoDevStateArrayConverter() + case _: + pass else: # command info match info.in_type: case CmdArgType.DevState: @@ -768,6 +770,8 @@ def make_converter(info: AttributeConfig | CommandConfig, datatype) -> TangoConv return TangoLongStringTableConverter() case CmdArgType.DevVarDoubleStringArray: return TangoDoubleStringTableConverter() + case _: + pass # default case return trivial converter return TangoConverter() diff --git a/tests/system_tests/tango/core/test_yaml_save.yaml b/tests/system_tests/tango/core/test_yaml_save.yaml index c7103cfd3b..a6b0ac7f08 100644 --- a/tests/system_tests/tango/core/test_yaml_save.yaml +++ b/tests/system_tests/tango/core/test_yaml_save.yaml @@ -1,3 +1,4 @@ +: {} a_bool: true a_str: test_string float64: 1.234 diff --git a/tests/unit_tests/core/test_readable.py b/tests/unit_tests/core/test_readable.py index 5071553b69..40ef5f3387 100644 --- a/tests/unit_tests/core/test_readable.py +++ b/tests/unit_tests/core/test_readable.py @@ -1,17 +1,13 @@ +from collections.abc import Sequence from unittest.mock import MagicMock import pytest from bluesky.protocols import HasHints from ophyd_async.core import ( - AsyncConfigurable, - AsyncReadable, - AsyncStageable, - ConfigSignal, Device, DeviceMap, DeviceVector, - HintedSignal, MockSignalBackend, SignalR, StandardReadable, @@ -22,11 +18,10 @@ from ophyd_async.core import StandardReadableFormat as Format -@pytest.mark.parametrize("wrapper", [HintedSignal, HintedSignal.uncached, ConfigSignal]) -def test_standard_readable_wrappers_raise_deprecation_warning(wrapper): +def test_standard_readable_rejects_a_non_format(): sr = StandardReadable() - with pytest.deprecated_call(): - sr.add_readables([soft_signal_rw(int)], wrapper) + with pytest.raises(TypeError, match="is not a StandardReadableFormat"): + sr.set_readable_format(soft_signal_rw(int), "CONFIG_SIGNAL") # type: ignore def test_standard_readable_hints(): @@ -43,7 +38,9 @@ def test_standard_readable_hints(): hint3 = MagicMock(spec=HasHints) hint3.hints = {"fields": ["jkl"], "gridding": "rectilinear_nonsequential"} - sr.add_readables([hint1, hint2, hint3]) + sr.set_readable_format(hint1, Format.CHILD) + sr.set_readable_format(hint2, Format.CHILD) + sr.set_readable_format(hint3, Format.CHILD) assert sr.hints == { "fields": ["abc", "def", "ghi", "jkl"], @@ -60,10 +57,8 @@ def test_standard_readable_hints_raises_when_overriding_string_literal(): hint2 = MagicMock(spec=HasHints) hint2.hints = {"gridding": "a different string"} - sr._has_hints = ( - hint1, - hint2, - ) + sr.set_readable_format(hint1, Format.CHILD) + sr.set_readable_format(hint2, Format.CHILD) with pytest.raises(RuntimeError, match=r"Hint key .* may not be overridden"): sr.hints # noqa: B018 @@ -88,7 +83,7 @@ def test_incorrect_fields_and_dimensions_in_hints(sub_hints: dict, error: str): sr = StandardReadable() has_hints = MagicMock(spec=HasHints) has_hints.hints = sub_hints - sr._has_hints = (has_hints,) + sr.set_readable_format(has_hints, Format.CHILD) with pytest.raises(RuntimeError, match=error): sr.hints # noqa: B018 @@ -115,10 +110,8 @@ def test_standard_readable_arbitrary_hints(): "gridding": "rectilinear_nonsequential", } - sr._has_hints = ( - hint1, - hint2, - ) + sr.set_readable_format(hint1, Format.CHILD) + sr.set_readable_format(hint2, Format.CHILD) assert sr.hints == { "gridding": "rectilinear_nonsequential", "fields": ["field1", "field2"], @@ -135,10 +128,8 @@ def test_standard_readable_hints_raises_when_overriding_sequence(): hint2 = MagicMock(spec=HasHints) hint2.hints = {"fields": ["field2"]} - sr._has_hints = ( - hint1, - hint2, - ) + sr.set_readable_format(hint1, Format.CHILD) + sr.set_readable_format(hint2, Format.CHILD) with pytest.raises(RuntimeError, match=r"Hint 'fields' .* override existing hint"): sr.hints # noqa: B018 @@ -146,14 +137,26 @@ def test_standard_readable_hints_raises_when_overriding_sequence(): def test_standard_readable_add_children_context_manager(): sr = StandardReadable() - mock = MagicMock() - sr.add_readables = mock with sr.add_children_as_readables(): sr.a = MagicMock(spec=SignalR) sr.b = MagicMock(spec=SignalR) sr.c = MagicMock(spec=SignalR) - mock.assert_called_once_with([sr.a, sr.b, sr.c], Format.CHILD) + assert list(sr.get_readable_formats()) == [ + (sr.a, Format.CHILD), + (sr.b, Format.CHILD), + (sr.c, Format.CHILD), + ] + + +async def test_add_children_as_readables_registers_in_declaration_order(): + sr = StandardReadable(name="sr") + names = ["zeta", "alpha", "mu", "beta", "omega", "gamma", "kappa", "delta"] + with sr.add_children_as_readables(Format.HINTED_SIGNAL): + for name in names: + setattr(sr, name, soft_signal_r_and_setter(int, 0)[0]) + + assert sr.hints == {"fields": [f"sr-{name}" for name in names]} @pytest.mark.parametrize( @@ -165,23 +168,19 @@ def test_standard_readable_add_children_context_manager(): ) def test_standard_readable_add_children_cm_device_with_mappings(device_type, keys): sr = StandardReadable() - mock = MagicMock() - sr.add_readables = mock - # Create a mock for the DeviceVector/DeviceMap.children() call + # A DeviceVector/DeviceMap registers its values, not itself devices = [MagicMock(spec=SignalR) for _ in range(3)] device = device_type(dict(zip(keys, devices, strict=True))) with sr.add_children_as_readables(): sr.a = device - mock.assert_called_once_with(devices, Format.CHILD) + assert list(sr.get_readable_formats()) == [(d, Format.CHILD) for d in devices] def test_standard_readable_add_children_cm_filters_non_devices(): sr = StandardReadable() - mock = MagicMock() - sr.add_readables = mock with sr.add_children_as_readables(): sr.a = MagicMock(spec=SignalR) @@ -190,73 +189,76 @@ def test_standard_readable_add_children_cm_filters_non_devices(): sr.d = "abc" sr.e = MagicMock(spec=MockSignalBackend) - mock.assert_called_once_with([sr.a, sr.b], Format.CHILD) - - -def assert_sr_has_attrs(sr: StandardReadable, expected_attrs: dict[str, tuple]): - attrs_to_check = ( - "_describe_config_funcs", - "_read_config_funcs", - "_describe_funcs", - "_read_funcs", - "_stage_funcs", - "_unstage_funcs", - "_has_hints", - ) - actual = {attr: getattr(sr, attr) for attr in attrs_to_check} - expected = {attr: expected_attrs.get(attr, ()) for attr in attrs_to_check} - assert actual == expected + # Only the Devices are registered + assert list(sr.get_readable_formats()) == [ + (sr.a, Format.CHILD), + (sr.b, Format.CHILD), + ] -signal_r = MagicMock(spec=SignalR) -async_readable = MagicMock(spec=AsyncReadable) -async_configurable = MagicMock(spec=AsyncConfigurable) -async_stageable = MagicMock(spec=AsyncStageable) -has_hints = MagicMock(spec=HasHints) +async def assert_contributes( + sr: StandardReadable, + *, + read: Sequence[str] = (), + config: Sequence[str] = (), + hints: Sequence[str] = (), +) -> None: + """Assert which keys a StandardReadable contributes to each verb.""" + assert list(await sr.read()) == list(read) + assert list(await sr.describe()) == list(read) + assert list(await sr.read_configuration()) == list(config) + assert list(await sr.describe_configuration()) == list(config) + assert list(sr.hints.get("fields", [])) == list(hints) @pytest.mark.parametrize( - "readable, expected_attrs", + "format, in_read, in_config, in_hints", [ - ( - signal_r, - { - "_read_funcs": (signal_r.read,), - "_describe_funcs": (signal_r.describe,), - "_stage_funcs": (signal_r.stage,), - "_unstage_funcs": (signal_r.unstage,), - }, - ), - ( - async_readable, - { - "_read_funcs": (async_readable.read,), - "_describe_funcs": (async_readable.describe,), - }, - ), - ( - async_configurable, - { - "_read_config_funcs": (async_configurable.read_configuration,), - "_describe_config_funcs": (async_configurable.describe_configuration,), - }, - ), - ( - async_stageable, - { - "_stage_funcs": (async_stageable.stage,), - "_unstage_funcs": (async_stageable.unstage,), - }, - ), - (has_hints, {"_has_hints": (has_hints,)}), + (Format.CONFIG_SIGNAL, False, True, False), + (Format.HINTED_SIGNAL, True, False, True), + (Format.UNCACHED_SIGNAL, True, False, False), + (Format.HINTED_UNCACHED_SIGNAL, True, False, True), ], ) -def test_standard_readable_add_readables_adds_to_expected_attrs( - readable, expected_attrs: dict[str, tuple] +async def test_standard_readable_signal_formats_route_to_expected_verbs( + format, in_read, in_config, in_hints ): - sr = StandardReadable() - sr.add_readables([readable]) - assert_sr_has_attrs(sr, expected_attrs) + sr = StandardReadable(name="sr") + sr.sig, _ = soft_signal_r_and_setter(int, 0, name="sig") + sr.set_readable_format(sr.sig, format) + name = [sr.sig.name] + await assert_contributes( + sr, + read=name if in_read else [], + config=name if in_config else [], + hints=name if in_hints else [], + ) + + +async def test_standard_readable_child_format_uses_child_verbs(): + inner = StandardReadable(name="inner") + inner.conf, _ = soft_signal_r_and_setter(int, 0, name="conf") + inner.val, _ = soft_signal_r_and_setter(int, 0, name="val") + inner.set_readable_format(inner.conf, Format.CONFIG_SIGNAL) + inner.set_readable_format(inner.val, Format.HINTED_SIGNAL) + + outer = StandardReadable(name="outer") + outer.inner = inner + outer.set_readable_format(inner, Format.CHILD) + + await assert_contributes( + outer, + read=[inner.val.name], + config=[inner.conf.name], + hints=[inner.val.name], + ) + + +async def test_standard_readable_child_format_ignores_plain_device(): + sr = StandardReadable(name="sr") + sr.child = Device(name="child") + sr.set_readable_format(sr.child, Format.CHILD) + await assert_contributes(sr) @pytest.mark.parametrize( @@ -275,43 +277,48 @@ def test_standard_readable_add_readables_raises_signalr_typeerror(format) -> Non # Ensure it raises TypeError with pytest.raises(TypeError, match=f"{mock_device} is not a SignalR"): - sr.add_readables([mock_device], format=format) - - -def test_standard_readable_config_signal(): - signal_r = MagicMock(spec=SignalR) - sr = StandardReadable() - sr.add_readables([signal_r], Format.CONFIG_SIGNAL) - assert sr._describe_config_funcs == (signal_r.describe,) - assert sr._read_config_funcs == (signal_r.read,) + sr.set_readable_format(mock_device, format) -def test_standard_readable_hinted_signal(): - signal_r = MagicMock(spec=SignalR) +def test_add_readables_is_deprecated() -> None: sr = StandardReadable() - sr.add_readables([signal_r], Format.HINTED_SIGNAL) - assert sr._describe_funcs == (signal_r.describe,) - assert sr._read_funcs == (signal_r.read,) - assert sr._stage_funcs == (signal_r.stage,) - assert sr._unstage_funcs == (signal_r.unstage,) - assert sr._has_hints[0].device == signal_r + sig = soft_signal_rw(int) + with pytest.warns(DeprecationWarning, match="set_readable_format"): + sr.add_readables([sig], Format.CONFIG_SIGNAL) + # Still does the job it always did + assert list(sr.get_readable_formats()) == [(sig, Format.CONFIG_SIGNAL)] -def test_standard_readable_uncached_signal(): - signal_r = MagicMock(spec=SignalR) - sr = StandardReadable() - sr.add_readables([signal_r], Format.UNCACHED_SIGNAL) - assert sr._describe_funcs == (signal_r.describe,) - assert sr._read_funcs[0].signal == signal_r +@pytest.mark.parametrize( + "format, staged", + [ + (Format.HINTED_SIGNAL, True), + (Format.CHILD, True), + (Format.CONFIG_SIGNAL, False), + (Format.UNCACHED_SIGNAL, False), + (Format.HINTED_UNCACHED_SIGNAL, False), + ], +) +async def test_standard_readable_stage_starts_caching_only_for_cached_formats( + format, staged +): + # stage() exists to start monitoring, and reading cached raises if it isn't + sr = StandardReadable(name="sr") + sr.sig, _ = soft_signal_r_and_setter(int, 0, name="sig") + await sr.sig.connect(mock=False) + sr.set_readable_format(sr.sig, format) + await sr.stage() + if staged: + assert await sr.sig.read(cached=True) + else: + with pytest.raises(RuntimeError, match="not being monitored"): + await sr.sig.read(cached=True) -def test_standard_readable_hinted_uncached_signal(): - signal_r = MagicMock(spec=SignalR) - sr = StandardReadable() - sr.add_readables([signal_r], Format.HINTED_UNCACHED_SIGNAL) - assert sr._describe_funcs == (signal_r.describe,) - assert sr._read_funcs[0].signal == signal_r - assert sr._has_hints[0].device == signal_r + # unstage tears the monitor back down + await sr.unstage() + with pytest.raises(RuntimeError, match="not being monitored"): + await sr.sig.read(cached=True) def test_standard_readable_add_children_multi_nested(): @@ -350,3 +357,199 @@ async def test_standard_readable_stage_twice_raises(): await sr.stage() await sr.unstage() await sr.stage() + + +async def test_set_readable_format_changes_verb_at_runtime(): + # The ophyd v1 `kind` use case from #1394: a signal that is config for one + # technique and hinted for another, without redefining the Device. + sr = StandardReadable(name="sr") + sr.energy, _ = soft_signal_r_and_setter(float, 0.0, name="energy") + + sr.set_readable_format(sr.energy, Format.CONFIG_SIGNAL) + await assert_contributes(sr, config=[sr.energy.name]) + + sr.set_readable_format(sr.energy, Format.HINTED_SIGNAL) + await assert_contributes(sr, read=[sr.energy.name], hints=[sr.energy.name]) + + sr.set_readable_format(sr.energy, None) + await assert_contributes(sr) + + +async def test_set_readable_format_replaces_rather_than_duplicates(): + sr = StandardReadable(name="sr") + sr.sig, _ = soft_signal_r_and_setter(int, 0, name="sig") + + sr.set_readable_format(sr.sig, Format.HINTED_SIGNAL) + sr.set_readable_format(sr.sig, Format.HINTED_SIGNAL) + + assert sr.hints == {"fields": [sr.sig.name]} + assert list(sr.get_readable_formats()) == [(sr.sig, Format.HINTED_SIGNAL)] + + +async def test_get_readable_formats_omits_unregistered_children(): + sr = StandardReadable(name="sr") + sr.sig, _ = soft_signal_r_and_setter(int, 0, name="sig") + sr.other, _ = soft_signal_r_and_setter(int, 0, name="other") + assert list(sr.get_readable_formats()) == [] + sr.set_readable_format(sr.sig, Format.CONFIG_SIGNAL) + sr.set_readable_format(sr.other, Format.HINTED_SIGNAL) + # Registration order is preserved, and an unregistered child is absent + assert list(sr.get_readable_formats()) == [ + (sr.sig, Format.CONFIG_SIGNAL), + (sr.other, Format.HINTED_SIGNAL), + ] + sr.set_readable_format(sr.sig, None) + assert list(sr.get_readable_formats()) == [(sr.other, Format.HINTED_SIGNAL)] + + +async def test_set_readable_format_while_iterating_get_readable_formats(): + sr = StandardReadable(name="sr") + sr.sig, _ = soft_signal_r_and_setter(int, 0, name="sig") + sr.other, _ = soft_signal_r_and_setter(int, 0, name="other") + sr.set_readable_format(sr.sig, Format.CONFIG_SIGNAL) + sr.set_readable_format(sr.other, Format.CONFIG_SIGNAL) + + for signal, _format in sr.get_readable_formats(): + sr.set_readable_format(signal, Format.HINTED_SIGNAL) + for signal, _format in sr.get_readable_format_changes(): + sr.set_readable_format(signal, None) + + assert list(sr.get_readable_formats()) == [] + + +async def test_reset_readable_formats_undoes_runtime_changes(): + class Dev(StandardReadable): + def __init__(self, name=""): + self.energy, _ = soft_signal_r_and_setter(float, 0.0, name="energy") + self.temp, _ = soft_signal_r_and_setter(float, 0.0, name="temp") + self.set_readable_format(self.energy, Format.CONFIG_SIGNAL) + self.set_readable_format(self.temp, Format.CONFIG_SIGNAL) + super().__init__(name=name) + + dev = Dev(name="dev") + dev.extra, _ = soft_signal_r_and_setter(float, 0.0, name="extra") + declared = list(dev.get_readable_formats()) + + # Change one, drop one, add one that was never declared + dev.set_readable_format(dev.energy, Format.HINTED_SIGNAL) + dev.set_readable_format(dev.temp, None) + dev.set_readable_format(dev.extra, Format.HINTED_SIGNAL) + assert list(dev.get_readable_formats()) == [ + (dev.energy, Format.HINTED_SIGNAL), + (dev.extra, Format.HINTED_SIGNAL), + ] + assert list(dev.get_readable_format_changes()) == [ + (dev.energy, Format.HINTED_SIGNAL), + (dev.extra, Format.HINTED_SIGNAL), + (dev.temp, None), + ] + + dev.reset_readable_formats() + assert list(dev.get_readable_formats()) == declared + assert list(dev.get_readable_format_changes()) == [] + # Repeatable, and the baseline is not consumed + dev.set_readable_format(dev.energy, None) + dev.reset_readable_formats() + assert list(dev.get_readable_formats()) == declared + + +async def test_reset_readable_formats_keeps_registrations_after_super_init(): + # The reason the baseline is sealed by the metaclass rather than at the end + # of StandardReadable.__init__: this registration happens after super() and + # is still part of what the class declares + class Dev(StandardReadable): + def __init__(self, name=""): + self.before, _ = soft_signal_r_and_setter(int, 0, name="before") + self.set_readable_format(self.before, Format.CONFIG_SIGNAL) + super().__init__(name=name) + self.after, _ = soft_signal_r_and_setter(int, 0, name="after") + self.set_readable_format(self.after, Format.HINTED_SIGNAL) + + dev = Dev(name="dev") + expected = [(dev.before, Format.CONFIG_SIGNAL), (dev.after, Format.HINTED_SIGNAL)] + assert list(dev.get_readable_formats()) == expected + + dev.set_readable_format(dev.after, None) + dev.reset_readable_formats() + assert list(dev.get_readable_formats()) == expected + + +async def test_reset_readable_formats_restores_annotated_declarations(): + sr = StandardReadable(name="sr") + sr.sig, _ = soft_signal_r_and_setter(int, 0, name="sig") + # Nothing declared, so a runtime registration resets away entirely + sr.set_readable_format(sr.sig, Format.HINTED_SIGNAL) + sr.reset_readable_formats() + assert list(sr.get_readable_formats()) == [] + + +async def test_default_readable_formats_are_not_shared_between_instances(): + class Dev(StandardReadable): + def __init__(self, name=""): + self.sig, _ = soft_signal_r_and_setter(int, 0, name="sig") + self.set_readable_format(self.sig, Format.CONFIG_SIGNAL) + super().__init__(name=name) + + a, b = Dev(name="a"), Dev(name="b") + a.set_readable_format(a.sig, None) + a.reset_readable_formats() + b.reset_readable_formats() + assert list(a.get_readable_formats()) == [(a.sig, Format.CONFIG_SIGNAL)] + assert list(b.get_readable_formats()) == [(b.sig, Format.CONFIG_SIGNAL)] + assert StandardReadable._default_readables == {} + + +async def test_readables_registered_before_super_init_survive(): + # AreaDetector registers config signals before calling super().__init__(), + # so the registry must not be created there. It must also be per instance. + class Dev(StandardReadable): + def __init__(self, name=""): + self.sig, _ = soft_signal_r_and_setter(int, 0, name="sig") + self.set_readable_format(self.sig, Format.HINTED_SIGNAL) + super().__init__(name=name) + + a, b = Dev(name="a"), Dev(name="b") + assert list(a.get_readable_formats()) == [(a.sig, Format.HINTED_SIGNAL)] + assert list(b.get_readable_formats()) == [(b.sig, Format.HINTED_SIGNAL)] + + b.set_readable_format(b.sig, None) + assert list(a.get_readable_formats()) == [(a.sig, Format.HINTED_SIGNAL)] + assert list(b.get_readable_formats()) == [] + + +async def test_set_readable_format_keeps_position_when_reformatting(): + # Changing a format must not reorder hints + sr = StandardReadable(name="sr") + sr.one, _ = soft_signal_r_and_setter(int, 0, name="one") + sr.two, _ = soft_signal_r_and_setter(int, 0, name="two") + sr.set_readable_format(sr.one, Format.HINTED_SIGNAL) + sr.set_readable_format(sr.two, Format.HINTED_SIGNAL) + assert sr.hints == {"fields": [sr.one.name, sr.two.name]} + + sr.set_readable_format(sr.one, Format.HINTED_UNCACHED_SIGNAL) + assert sr.hints == {"fields": [sr.one.name, sr.two.name]} + + +async def test_set_readable_format_affects_staging_of_later_runs(): + sr = StandardReadable(name="sr") + sr.sig, _ = soft_signal_r_and_setter(int, 0, name="sig") + await sr.sig.connect(mock=False) + sr.set_readable_format(sr.sig, Format.HINTED_SIGNAL) + + await sr.stage() + assert await sr.sig.read(cached=True) + await sr.unstage() + + # Dropping the signal means the next run does not monitor it + sr.set_readable_format(sr.sig, None) + await sr.stage() + with pytest.raises(RuntimeError, match="not being monitored"): + await sr.sig.read(cached=True) + await sr.unstage() + + +def test_set_readable_format_rejects_non_signal_for_signal_format(): + sr = StandardReadable(name="sr") + sr.child = Device(name="child") + with pytest.raises(TypeError, match="is not a SignalR"): + sr.set_readable_format(sr.child, Format.CONFIG_SIGNAL) diff --git a/tests/unit_tests/epics/pvi/test_pvi.py b/tests/unit_tests/epics/pvi/test_pvi.py index 7dcafdd3e4..80d963d15a 100644 --- a/tests/unit_tests/epics/pvi/test_pvi.py +++ b/tests/unit_tests/epics/pvi/test_pvi.py @@ -155,7 +155,7 @@ async def test_device_create_children_from_annotations_with_device_vectors(): await device.connect(mock=True) block_1_device = device.device - assert block_1_device in device._has_hints + assert (block_1_device, Format.CHILD) in device.get_readable_formats() block_2_device_vector = device.device_vector assert device.device_vector[1].name == "test_device-device_vector-1" diff --git a/tests/unit_tests/plan_stubs/test_settings.py b/tests/unit_tests/plan_stubs/test_settings.py index c64f2a36c3..4510173d7f 100644 --- a/tests/unit_tests/plan_stubs/test_settings.py +++ b/tests/unit_tests/plan_stubs/test_settings.py @@ -6,7 +6,17 @@ import pytest import yaml -from ophyd_async.core import Settings, YamlSettingsProvider, get_mock +from ophyd_async.core import ( + Device, + Settings, + StandardReadable, + YamlSettingsProvider, + apply_readable_formats, + get_mock, + soft_signal_rw, + walk_readable_formats, +) +from ophyd_async.core import StandardReadableFormat as Format from ophyd_async.plan_stubs import ( apply_settings, apply_settings_if_different, @@ -179,3 +189,417 @@ def my_plan(): assert m.mock_calls == [call.sig_rw.put("foo")] RE(my_plan()) + + +@pytest.fixture +def technique_device() -> StandardReadable: + """A Device whose energy signal is config for one technique, hinted for another.""" + device = StandardReadable(name="mono") + device.energy = soft_signal_rw(float, 7.0) + device.temperature = soft_signal_rw(float, 20.0) + device.set_name("mono") + device.set_readable_format(device.energy, Format.CONFIG_SIGNAL) + device.set_readable_format(device.temperature, Format.CONFIG_SIGNAL) + return device + + +async def test_store_settings_writes_formats_under_reserved_key( + RE, technique_device, tmp_path +): + provider = YamlSettingsProvider(tmp_path) + + def my_plan(): + yield from store_settings(provider, "fixed_energy", technique_device) + with open(tmp_path / "fixed_energy.yaml") as f: + stored = yaml.safe_load(f) + # Values stay flat, formats live under a key that cannot be a path + assert stored == { + "energy": 7.0, + "temperature": 20.0, + "": { + "": { + "energy": "CONFIG_SIGNAL", + "temperature": "CONFIG_SIGNAL", + } + }, + } + + RE(my_plan()) + + +async def test_reserved_keys_need_no_quoting_in_yaml(RE, technique_device, tmp_path): + provider = YamlSettingsProvider(tmp_path) + + def my_plan(): + yield from store_settings(provider, "fixed_energy", technique_device) + text = (tmp_path / "fixed_energy.yaml").read_text() + assert ":" in text + assert ":" in text + # An empty string root would make yaml fall back to "? ''" explicit keys + assert "?" not in text + + RE(my_plan()) + + +async def test_settings_round_trip_switches_technique(RE, technique_device, tmp_path): + provider = YamlSettingsProvider(tmp_path) + energy, temperature = technique_device.energy, technique_device.temperature + + def my_plan(): + yield from store_settings(provider, "fixed_energy", technique_device) + + # Switch to a technique that scans energy and drops temperature entirely + technique_device.set_readable_format(energy, Format.HINTED_SIGNAL) + technique_device.set_readable_format(temperature, None) + yield from bps.abs_set(energy, 9.0, wait=True) + yield from store_settings(provider, "scan_energy", technique_device) + + # Going back restores both the value and the formats in one apply + fixed = yield from retrieve_settings(provider, "fixed_energy", technique_device) + yield from apply_settings(fixed) + assert list(technique_device.get_readable_formats()) == [ + (energy, Format.CONFIG_SIGNAL), + (temperature, Format.CONFIG_SIGNAL), + ] + assert (yield from bps.rd(energy)) == 7.0 + + # And forward again drops temperature rather than merging the two + scanning = yield from retrieve_settings( + provider, "scan_energy", technique_device + ) + yield from apply_settings(scanning) + assert list(technique_device.get_readable_formats()) == [ + (energy, Format.HINTED_SIGNAL) + ] + assert (yield from bps.rd(energy)) == 9.0 + + RE(my_plan()) + + +async def test_settings_file_without_formats_leaves_them_alone( + RE, technique_device, tmp_path +): + """A file stored before formats existed must not clear them.""" + provider = YamlSettingsProvider(tmp_path) + energy = technique_device.energy + (tmp_path / "old_style.yaml").write_text("energy: 3.0\ntemperature: 4.0\n") + + def my_plan(): + settings = yield from retrieve_settings(provider, "old_style", technique_device) + assert settings.readable_formats is None + yield from apply_settings(settings) + # Values applied, formats untouched + assert (yield from bps.rd(energy)) == 3.0 + assert (energy, Format.CONFIG_SIGNAL) in technique_device.get_readable_formats() + + RE(my_plan()) + + +async def test_formats_only_file_applies_without_touching_values( + RE, technique_device, tmp_path +): + """A hand written formats only profile changes no hardware. + + Also covers the explicit null, which is the only way to unregister a child, + since applying otherwise merges. + """ + provider = YamlSettingsProvider(tmp_path) + energy = technique_device.energy + (tmp_path / "hinted.yaml").write_text( + ":\n" + " :\n" + " energy: HINTED_SIGNAL\n" + " temperature: null\n" + ) + + def my_plan(): + settings = yield from retrieve_settings(provider, "hinted", technique_device) + assert dict(settings) == {} + yield from apply_settings(settings) + # An explicit null drops temperature, where omitting it would have left + # it alone + assert list(technique_device.get_readable_formats()) == [ + (energy, Format.HINTED_SIGNAL) + ] + assert (yield from bps.rd(energy)) == 7.0 + + RE(my_plan()) + + +class _Mono(StandardReadable): + def __init__(self, name=""): + with self.add_children_as_readables(Format.CONFIG_SIGNAL): + self.energy = soft_signal_rw(float, 7.0) + self.temperature = soft_signal_rw(float, 20.0) + super().__init__(name=name) + + +async def test_store_settings_writes_only_changes_from_class_defaults(RE, tmp_path): + provider = YamlSettingsProvider(tmp_path) + mono = _Mono(name="mono") + + def stored_formats(name: str): + with open(tmp_path / f"{name}.yaml") as f: + return yaml.safe_load(f)[""] + + def my_plan(): + yield from store_settings(provider, "defaults", mono) + mono.set_readable_format(mono.energy, Format.HINTED_SIGNAL) + mono.set_readable_format(mono.temperature, None) + yield from store_settings(provider, "scan", mono) + assert stored_formats("defaults") == {} + # A dropped default is written as a null + assert stored_formats("scan") == { + "": {"energy": "HINTED_SIGNAL", "temperature": None} + } + # The defaults file undoes changes it does not mention + defaults = yield from retrieve_settings(provider, "defaults", mono) + yield from apply_settings(defaults) + assert list(mono.get_readable_formats()) == [ + (mono.energy, Format.CONFIG_SIGNAL), + (mono.temperature, Format.CONFIG_SIGNAL), + ] + scan = yield from retrieve_settings(provider, "scan", mono) + yield from apply_settings(scan) + assert list(mono.get_readable_formats()) == [ + (mono.energy, Format.HINTED_SIGNAL) + ] + + RE(my_plan()) + + +async def test_null_formats_key_resets_to_class_defaults(RE, tmp_path): + provider = YamlSettingsProvider(tmp_path) + mono = _Mono(name="mono") + mono.set_readable_format(mono.energy, Format.HINTED_SIGNAL) + (tmp_path / "reset.yaml").write_text(":\n") + + def my_plan(): + settings = yield from retrieve_settings(provider, "reset", mono) + assert settings.readable_formats == {} + yield from apply_settings(settings) + assert (mono.energy, Format.CONFIG_SIGNAL) in mono.get_readable_formats() + + RE(my_plan()) + + +@pytest.mark.parametrize( + "content, unknown", + [ + ("energyy: 3.0\n", r"\['energyy'\]"), + ( + "energy: 3.0\n:\n : {}\n: 1\n", + r"\['', ''\]", + ), + ], + ids=["typo", "unknown_reserved_keys"], +) +async def test_retrieve_settings_rejects_unknown_names( + RE, technique_device, tmp_path, content, unknown +): + provider = YamlSettingsProvider(tmp_path) + (tmp_path / "bad.yaml").write_text(content) + + def my_plan(): + with pytest.raises(NameError, match=rf"Unknown signal names {unknown}"): + yield from retrieve_settings(provider, "bad", technique_device) + + RE(my_plan()) + + +async def test_store_settings_walks_nested_devices(RE, tmp_path): + provider = YamlSettingsProvider(tmp_path) + inner = StandardReadable(name="inner") + inner.sig = soft_signal_rw(float, 1.0) + outer = StandardReadable(name="outer") + outer.inner = inner + outer.top = soft_signal_rw(float, 2.0) + outer.set_name("outer") + inner.set_readable_format(inner.sig, Format.HINTED_SIGNAL) + outer.set_readable_format(outer.top, Format.CONFIG_SIGNAL) + outer.set_readable_format(inner, Format.CHILD) + # The same signal registered on a second owner, from deeper in its tree + outer.set_readable_format(inner.sig, Format.CONFIG_SIGNAL) + + def my_plan(): + yield from store_settings(provider, "nested", outer) + with open(tmp_path / "nested.yaml") as f: + assert yaml.safe_load(f) == { + "top": 2.0, + "inner.sig": 1.0, + "": { + "": { + "top": "CONFIG_SIGNAL", + "inner": "CHILD", + "inner.sig": "CONFIG_SIGNAL", + }, + # Child paths are relative to their owner + "inner": {"sig": "HINTED_SIGNAL"}, + }, + } + + RE(my_plan()) + + +def test_apply_readable_formats_rejects_unknown_path(technique_device): + with pytest.raises(KeyError, match="nope"): + apply_readable_formats(technique_device, {"nope": {}}) + + +def test_apply_readable_formats_rejects_non_readable(technique_device): + with pytest.raises(TypeError, match="is not a StandardReadable"): + apply_readable_formats(technique_device, {"energy": {}}) + + +def test_settings_or_merges_formats_per_child(technique_device): + # Values merge per signal and apply_readable_formats merges per child, so + # `other` must only replace the children it mentions: overriding a whole + # owner would drop children it has nothing to say about + base = Settings( + technique_device, + {technique_device.energy: 1.0}, + { + "": { + "energy": Format.CONFIG_SIGNAL, + "temperature": Format.CHILD, + } + }, + ) + override = Settings( + technique_device, + {technique_device.temperature: 2.0}, + {"": {"energy": Format.HINTED_SIGNAL}}, + ) + merged = base | override + assert merged.readable_formats == { + "": { + "energy": Format.HINTED_SIGNAL, # overridden + "temperature": Format.CHILD, # untouched, not dropped + } + } + assert dict(merged) == { + technique_device.energy: 1.0, + technique_device.temperature: 2.0, + } + # Neither operand is mutated + assert base.readable_formats[""]["energy"] is Format.CONFIG_SIGNAL + assert override.readable_formats[""] == { + "energy": Format.HINTED_SIGNAL + } + + +def test_settings_or_keeps_owners_the_other_does_not_mention(technique_device): + base = Settings(technique_device, {}, {"a": {"x": Format.CONFIG_SIGNAL}}) + override = Settings(technique_device, {}, {"b": {"y": Format.HINTED_SIGNAL}}) + assert (base | override).readable_formats == { + "a": {"x": Format.CONFIG_SIGNAL}, + "b": {"y": Format.HINTED_SIGNAL}, + } + + +def test_settings_or_rejects_formats_from_a_sub_device(technique_device): + # A sub-device's paths are relative to itself, so would silently come + # to mean the parent + parent = StandardReadable(name="parent") + parent.child = technique_device + parent.set_name("parent") + base = Settings(parent) + sub = Settings( + technique_device, {}, {"": {"energy": Format.HINTED_SIGNAL}} + ) + with pytest.raises(ValueError, match="paths are relative to it"): + base | sub + # Without formats it is just a value merge, which is fine + assert (base | Settings(technique_device)).readable_formats is None + + +def test_settings_partition_carries_formats_to_both_halves(technique_device): + # apply_panda_settings applies only the halves, never the original, so + # formats must survive on whichever half is applied + settings = Settings( + technique_device, + {technique_device.energy: 1.0, technique_device.temperature: 2.0}, + {"": {"energy": Format.CONFIG_SIGNAL}}, + ) + a, b = settings.partition(lambda sig: "energy" in sig.name) + assert a.readable_formats == settings.readable_formats + assert b.readable_formats == settings.readable_formats + + +async def test_store_settings_writes_empty_formats_for_an_unchanged_readable( + RE, tmp_path +): + """The key is still written, as applying it resets runtime changes. + + This is the shape of TangoTestDevice. + """ + provider = YamlSettingsProvider(tmp_path) + device = StandardReadable(name="dev") + device.sig = soft_signal_rw(float, 1.0) + device.set_name("dev") + + def my_plan(): + yield from store_settings(provider, "empty", device) + with open(tmp_path / "empty.yaml") as f: + assert yaml.safe_load(f) == {"sig": 1.0, "": {}} + + RE(my_plan()) + + +async def test_store_settings_warns_and_skips_a_readable_outside_the_tree(RE, tmp_path): + """A child registered from outside the tree has no path to store it against. + + Its value is not stored either, since store_settings only walks the tree, so + it cannot survive a round trip at all. See + https://github.com/bluesky/ophyd-async/issues/1402. + """ + provider = YamlSettingsProvider(tmp_path) + device = StandardReadable(name="dev") + device.sig = soft_signal_rw(float, 1.0) + device.set_name("dev") + outsider = soft_signal_rw(float, 2.0, name="outsider") + device.set_readable_format(device.sig, Format.CONFIG_SIGNAL) + device.set_readable_format(outsider, Format.CONFIG_SIGNAL) + + match = "dev: outsider is not within dev" + with pytest.warns(UserWarning, match=match): + assert walk_readable_formats(device) == { + "": {"sig": Format.CONFIG_SIGNAL} + } + + def my_plan(): + with pytest.warns(UserWarning, match=match): + yield from store_settings(provider, "outsider", device) + with open(tmp_path / "outsider.yaml") as f: + # Neither the outsider's format nor its value is stored + assert yaml.safe_load(f) == { + "sig": 1.0, + "": {"": {"sig": "CONFIG_SIGNAL"}}, + } + + RE(my_plan()) + + +async def test_store_settings_omits_the_key_for_a_device_with_no_readables( + RE, tmp_path +): + """A Device with no StandardReadable in its tree has nothing to clear. + + This is the shape of EpicsTestCaDevice, and is why the EPICS golden files + needed no regeneration. + """ + provider = YamlSettingsProvider(tmp_path) + + class Plain(Device): + pass + + device = Plain(name="dev") + device.sig = soft_signal_rw(float, 1.0) + device.set_name("dev") + + def my_plan(): + yield from store_settings(provider, "plain", device) + with open(tmp_path / "plain.yaml") as f: + assert yaml.safe_load(f) == {"sig": 1.0} + + RE(my_plan()) diff --git a/tests/unit_tests/test_data/test_yaml_config_save.yaml b/tests/unit_tests/test_data/test_yaml_config_save.yaml index 99fb4ac43c..1c981f1ef5 100644 --- a/tests/unit_tests/test_data/test_yaml_config_save.yaml +++ b/tests/unit_tests/test_data/test_yaml_config_save.yaml @@ -1,3 +1,4 @@ +: {} a_bool: true a_enum: Bbb a_float: 1.234 diff --git a/tests/unit_tests/test_data/test_yaml_save.yaml b/tests/unit_tests/test_data/test_yaml_save.yaml index 900837389f..1d2d7047bb 100644 --- a/tests/unit_tests/test_data/test_yaml_save.yaml +++ b/tests/unit_tests/test_data/test_yaml_save.yaml @@ -1,3 +1,4 @@ +: {} _sig_rw: Top level private SignalRW child.a_bool: true child.a_enum: Bbb