diff --git a/CHANGELOG.md b/CHANGELOG.md index dae311903c..63d284885b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,11 @@ - `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. +- `StandardDetector` is now a `StandardReadable`: signals read in a step scan or reported as configuration are registered with `set_readable_format()`, as on any other `StandardReadable`, and what the data logics produce is added to `read()`, `describe()` and `hints` alongside them. `add_config_signals()` is deprecated in favour of `set_readable_format(signal, StandardReadableFormat.CONFIG_SIGNAL)`. A detector with no hinted fields now reports `hints` as `{}` rather than `{"fields": []}`. +- `DetectorTriggerLogic.config_sigs()` is removed: `get_deadtime()` is given the value of every signal the detector reports as configuration, including those declared on children registered as `CHILD`, so a signal that determines deadtime must be registered as `CONFIG_SIGNAL`. +- The `prepare_single` data logic tier is removed, with `ReadableDataProvider`, `SignalDataProvider` and `adcore.PluginSignalDataLogic`. Register a plugin scalar such as `NDStatsIO.total` with `set_readable_format(signal, StandardReadableFormat.HINTED_SIGNAL)` instead. +- The areaDetector driver and plugin classes (`ADBaseIO`, `NDROIIO`, `NDStatsIO` and `NDROIStatNIO`) are now `StandardReadable`s that declare their own configuration and read signals, so registering a plugin as `CHILD` of an `AreaDetector` puts them in `read()` and `read_configuration()`. +- `ADAcquireLogic` sets `WaitForPlugins` on `stage()`, from its new `wait_for_plugins` argument (default on), rather than `AreaDetector` hiding it, so a step scan reads plugin scalars for the frame it has just taken. ### Added diff --git a/docs/explanations/decisions/0012-detector-rewrite.md b/docs/explanations/decisions/0012-detector-rewrite.md index 3e1145b66b..f1cc30a79a 100644 --- a/docs/explanations/decisions/0012-detector-rewrite.md +++ b/docs/explanations/decisions/0012-detector-rewrite.md @@ -5,6 +5,10 @@ Date: 2026-01-20 Accepted +Partly superseded by [ADR 0024](0024-standard-detector-is-a-standard-readable.md), which +made `StandardDetector` a `StandardReadable` and removed the `prepare_single` tier along +with `ReadableDataProvider`. + ## Context The original `StandardDetector` implementation had several architectural issues: diff --git a/docs/explanations/decisions/0024-standard-detector-is-a-standard-readable.md b/docs/explanations/decisions/0024-standard-detector-is-a-standard-readable.md new file mode 100644 index 0000000000..93f0faac31 --- /dev/null +++ b/docs/explanations/decisions/0024-standard-detector-is-a-standard-readable.md @@ -0,0 +1,50 @@ +# 24. StandardDetector is a StandardReadable + +Date: 2026-10-30 + +## Status + +Accepted + +## Context + +[ADR 0012](0012-detector-rewrite.md) gave `StandardDetector` its own ways to put a signal +in a step scan: `add_config_signals` for configuration, and the `prepare_single` data +logic tier with `ReadableDataProvider` for values read once per event. Every other Device +does both by registering the signal on a `StandardReadable`, and +[ADR 0023](0023-runtime-readable-format.md) made that registry keyed by Device and +changeable at runtime, which is the per-plugin opt-in the #1395 reviewers asked for. + +## Decision + +### `StandardDetector` inherits `StandardReadable` + +A detector's step-scan signals are registered like any other Device's. The detector does not +override `read()`, `describe()` or `hints`: each verb gathers a tuple of callables and the +detector appends its own, as `StandardFlyable` does to `_stage_funcs`, because overriding +meant re-deriving the registered children's contributions in each verb, which would drift. +`add_config_signals` becomes a deprecated wrapper. + +A trigger logic computes deadtime from all the detector's configuration signals rather than +nominating its own, as the values that determine deadtime ought to be recorded anyway. They +come from the registry, not `read_configuration()`, so read-only signals such as +`ADBaseIO.model` count. + +`AreaDetector` registers its driver but no plugin: a plugin is opted in with +`set_readable_format(plugin, StandardReadableFormat.CHILD)` and out with `None`, the +per-plugin, reversible choice the #1395 reviewers asked for. + +### The `prepare_single` data logic tier goes + +Registering a signal now gets one value per event into `read()` for any Device, which was +all [ADR 0012](0012-detector-rewrite.md)'s `prepare_single` tier did. It is deleted with its +providers rather than kept as a second way to do one thing, the one that drops itself with a +warning once a scan asks for more than one collection. + +## Consequences + +- A detector's signals can be switched between techniques, stored and restored like any + other Device's ([ADR 0023](0023-runtime-readable-format.md)). +- A signal that determines deadtime must be registered as configuration. +- Out-of-tree `prepare_single` data logics must register their signals instead; there is no + shim. diff --git a/docs/how-to/implement-ad-detector.md b/docs/how-to/implement-ad-detector.md index 93b0e4c638..469c51ef5f 100644 --- a/docs/how-to/implement-ad-detector.md +++ b/docs/how-to/implement-ad-detector.md @@ -28,11 +28,14 @@ Implement methods for each trigger mode your detector supports: Only implement the prepare methods for trigger modes your detector actually supports. The detector will automatically report which trigger types are available based on which methods are implemented. -If the detector has configuration values that should be captured in the scan then implement: -- `config_sigs()` - Return the set of signals that should appear in read_configuration() +Configuration values that should be captured in the scan are declared on the IO class, +with [](#StandardReadableFormat.CONFIG_SIGNAL), exactly as on any other +[](#StandardReadable). The trigger logic does not nominate them. If you support external triggering you should also implement: -- `get_deadtime(config_values)` - Calculate the minimum time between exposures based on configuration values +- `get_deadtime(config_values)` - Calculate the minimum time between exposures. It is + given the value of *every* signal the detector reports as configuration, so a signal + it needs must be declared with [](#StandardReadableFormat.CONFIG_SIGNAL). To preserve hardware state in plans like [`bp.count`](#bluesky.plans.count) and [`bps.trigger_and_read`](#bluesky.plan_stubs.trigger_and_read) that call `trigger()` @@ -116,12 +119,15 @@ For example, for ADAravis: The composition-based architecture makes it possible to add multiple data outputs to a detector. After creating the detector, you can call `add_detector_logics()` to add additional data sources: ### Reading stats plugins alongside file writing +A plugin scalar needs no data logic: a detector is a [](#StandardReadable), so register +the signal and it appears in `read()` alongside whatever the file writer produces. + ```python -from ophyd_async.epics.adcore import PluginSignalDataLogic +from ophyd_async.core import StandardReadableFormat as Format -det = adaravis.AravisDetector(prefix, path_provider) -# Add stats total as a readable signal in events -det.add_detector_logics(adcore.PluginSignalDataLogic(det.driver, det.stats.total)) +det = adaravis.AravisDetector(prefix, path_provider, plugins={"stats": stats}) +# Add stats total as a hinted signal in events +det.set_readable_format(det.stats.total, Format.HINTED_UNCACHED_SIGNAL) ``` ### Multiple HDF writers for different ROIs @@ -267,4 +273,4 @@ The detector will now support: - Step scanning with `trigger()` - Fly scanning with `kickoff()` and `complete()` - Automatic handling of file writing and StreamAsset document emission -- Configuration signal reporting based on your trigger logic +- Configuration signal reporting from the signals declared with `StandardReadableFormat.CONFIG_SIGNAL` on the IO classes diff --git a/docs/reference/environment-variables.md b/docs/reference/environment-variables.md index 8ab91db8d8..78afba8c51 100644 --- a/docs/reference/environment-variables.md +++ b/docs/reference/environment-variables.md @@ -13,7 +13,7 @@ effect on ophyd-async itself. Controls the implicit prepare performed by [`StandardDetector.trigger()`](#ophyd_async.core.StandardDetector.trigger) when [`prepare()`](#ophyd_async.core.StandardDetector.prepare) has not been called since -the last [`stage()`](#ophyd_async.core.StandardDetector.stage). +the last `stage()`. | Value | Behaviour | |-------|-----------| diff --git a/docs/tutorials/implementing-detectors.md b/docs/tutorials/implementing-detectors.md index 4fa2fe93ea..0c955d4d5d 100644 --- a/docs/tutorials/implementing-detectors.md +++ b/docs/tutorials/implementing-detectors.md @@ -199,8 +199,10 @@ If we wanted to support external triggering, we would also implement: - `prepare_level()` for external level/gate triggering (high level duration determines exposure) We could also implement: -- `get_deadtime()` to calculate the minimum time between exposures based on configuration -- `config_sigs()` to return signals that should appear in read_configuration() +- `get_deadtime()` to calculate the minimum time between exposures. It is given the value + of every signal the detector reports as configuration, so a signal it needs must be + declared with [](#StandardReadableFormat.CONFIG_SIGNAL) like on any other + [](#StandardReadable): the trigger logic does not nominate signals itself - `default_trigger_info()` to return the [](#TriggerInfo) to use when `trigger()` is called without a preceding `prepare()` (governed by [](#OPHYD_ASYNC_PRESERVE_DETECTOR_STATE)). ### `BlobAcquireLogic` diff --git a/src/ophyd_async/core/__init__.py b/src/ophyd_async/core/__init__.py index 9022728341..3583552b48 100644 --- a/src/ophyd_async/core/__init__.py +++ b/src/ophyd_async/core/__init__.py @@ -9,8 +9,6 @@ soft_command, ) from ._data_providers import ( - ReadableDataProvider, - SignalDataProvider, StreamableDataProvider, StreamResourceDataProvider, StreamResourceInfo, @@ -306,9 +304,7 @@ def __getattr__(name): "AutoMaxIncrementingPathProvider", "UUIDFilenameProvider", # Data Providers - "ReadableDataProvider", "StreamableDataProvider", - "SignalDataProvider", "StreamResourceInfo", "StreamResourceDataProvider", # Flyer diff --git a/src/ophyd_async/core/_data_providers.py b/src/ophyd_async/core/_data_providers.py index ad2983cb59..8faa7ba069 100644 --- a/src/ophyd_async/core/_data_providers.py +++ b/src/ophyd_async/core/_data_providers.py @@ -1,39 +1,14 @@ from abc import abstractmethod from collections.abc import AsyncIterator, Sequence -from dataclasses import dataclass from typing import Any -from bluesky.protocols import Reading, StreamAsset +from bluesky.protocols import StreamAsset from event_model import ComposeStreamResource, DataKey, StreamRange from ._signal import SignalR, SignalW from ._utils import ConfinedModel -class ReadableDataProvider: - @abstractmethod - async def make_datakeys(self) -> dict[str, DataKey]: - """Return a DataKey for each Readable that produces a Reading. - - Called before the first exposure is taken. - """ - - @abstractmethod - async def make_readings(self) -> dict[str, Reading]: - """Read the Signals and return their values.""" - - -@dataclass -class SignalDataProvider(ReadableDataProvider): - signal: SignalR - - async def make_datakeys(self) -> dict[str, DataKey]: - return await self.signal.describe() - - async def make_readings(self) -> dict[str, Reading]: - return await self.signal.read(cached=False) - - class StreamableDataProvider: collections_written_signal: SignalR[int] diff --git a/src/ophyd_async/core/_detector.py b/src/ophyd_async/core/_detector.py index ca9f756fe6..6cf7f27a9e 100644 --- a/src/ophyd_async/core/_detector.py +++ b/src/ophyd_async/core/_detector.py @@ -4,8 +4,9 @@ import functools import os import time +import warnings from abc import ABC, abstractmethod -from collections.abc import AsyncIterator, Awaitable, Callable, Sequence +from collections.abc import AsyncIterator, Awaitable, Callable, Iterator, Sequence from dataclasses import dataclass from enum import Enum from functools import cached_property @@ -14,7 +15,6 @@ from bluesky.protocols import ( Collectable, Flyable, - HasHints, Hints, Preparable, Reading, @@ -26,11 +26,12 @@ from event_model import DataKey from pydantic import Field, NonNegativeInt, PositiveInt, computed_field -from ophyd_async.core._log import logger - -from ._data_providers import ReadableDataProvider, StreamableDataProvider -from ._device import Device -from ._protocol import AsyncConfigurable, AsyncReadable +from ._data_providers import StreamableDataProvider +from ._readable import ( + StandardReadable, + StandardReadableFormat, + _config_signals, +) from ._settings import Settings from ._signal import ( SignalDict, @@ -141,19 +142,19 @@ class DetectorTriggerLogic: - Multi-exposure collection batching Subclasses must implement the appropriate `prepare_*` method for any trigger - mode the detector supports, `get_deadtime` if it supports external - triggering, and `config_sigs` if the deadtime would vary according to - detector parameters. + mode the detector supports, and `get_deadtime` if it supports external + triggering. """ - def config_sigs(self) -> set[SignalR]: - """Return the signals that should appear in read_configuration.""" - return set() - def get_deadtime(self, config_values: SignalDict) -> float: """Return the deadtime in seconds for the detector. - :param config_values: the value of each signal in `config_sigs` + :param config_values: + The value of every `Signal` the detector reports as configuration, + i.e. everything registered with + [](#StandardReadableFormat.CONFIG_SIGNAL), including those declared + on children registered as [](#StandardReadableFormat.CHILD). A + logic that needs a signal must declare it as configuration. """ raise NotImplementedError(self) @@ -315,19 +316,16 @@ async def _get_collections_written( class DetectorDataLogic: """Abstract base class for detector data logic and handling. - Implementations must implement either prepare_unbounded for data sources - that work with step scans as well as flyscans, or prepare_single for those - that only work with step scans. + Implementations must override `prepare_unbounded`. A source that produces a + single value per event, like a plugin scalar, does not need a data logic at + all: register its signal with + [](#StandardReadable.set_readable_format) instead. """ #: Add this suffix to the detector name to specify the datakey. These need to be #: different for each DetectorDataLogic added to a detector datakey_suffix: str = "" - async def prepare_single(self, datakey_name: str) -> ReadableDataProvider: - """Provider can only work for a single event.""" - raise NotImplementedError(self) - async def prepare_unbounded(self, datakey_name: str) -> StreamableDataProvider: """Provider can work for an unbounded number of collections.""" raise NotImplementedError(self) @@ -347,7 +345,6 @@ async def stop(self) -> None: @dataclass class _PrepareCtx: trigger_info: TriggerInfo - readable_data_providers: Sequence[ReadableDataProvider] streamable_data_providers: Sequence[StreamableDataProvider] collections_written: int @@ -362,28 +359,34 @@ class _KickoffCtx: class StandardDetector( - Device, + StandardReadable, Stageable, - AsyncConfigurable, - AsyncReadable, Triggerable, Preparable, Flyable, Collectable, WritesStreamAssets, - HasHints, ): """Detector base class for step and fly scanning detectors. Aggregates trigger, arm, reading or stream logic together. + + Signals read during a step scan are registered with + [](#StandardReadable.set_readable_format), exactly as on any other + `StandardReadable`; data produced by a `DetectorDataLogic` is added on top + of those in `read()` and `describe()`. + + `read()` and `describe()` require `prepare()` to have run, and raise + otherwise, so that a descriptor can never be emitted without the detector's + data keys. `trigger()` prepares implicitly, so a step scan never has to do + it explicitly. `read_configuration()` and `describe_configuration()` have no + such requirement. """ # Logic for the detector _trigger_logic: DetectorTriggerLogic | None = None _acquire_logic: DetectorAcquireLogic | None = None _data_logics: Sequence[DetectorDataLogic] = () - # Signals to include in read_configuration - _config_signals: Sequence[SignalR] = () # Context produced by prepare, used by trigger and kickoff _prepare_ctx: _PrepareCtx | None = None # Context produced by kickoff, used by complete @@ -391,6 +394,18 @@ class StandardDetector( # The triggers that are supported by the trigger logic _supported_triggers: set[DetectorTrigger] = {DetectorTrigger.INTERNAL} + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + # Contribute to _StandardBase's fan out rather than replacing stage(), + # so the readable children registered on this detector still get staged + self._stage_funcs += (self._stage_detector,) + self._unstage_funcs += (self._unstage_detector,) + # What the data logics produce, gathered alongside the registered + # children, so the verb methods themselves need no overriding + self._read_funcs += (self.__on_read,) + self._describe_funcs += (self.__on_describe,) + self._hints_funcs += (self.__on_hints,) + # Report the number of events for the next kickoff @cached_property def events_to_kickoff(self) -> SignalRW[int]: @@ -418,8 +433,6 @@ def add_detector_logics( self._trigger_logic = logic # Store the triggers that are supported self._supported_triggers = _get_supported_triggers(logic) - # Add the config signals it needs - self.add_config_signals(*logic.config_sigs()) elif isinstance(logic, DetectorAcquireLogic): if self._acquire_logic is not None: raise RuntimeError("Detector already has acquire logic") @@ -429,18 +442,33 @@ def add_detector_logics( else: raise TypeError(f"Unknown logic type: {type(logic)}") + # Back compat - delete before 1.0 def add_config_signals(self, *signals: SignalR) -> None: """Add a signal to read_configuration(). :param sig: The signal to add """ - self._config_signals = (*self._config_signals, *signals) + warnings.warn( + DeprecationWarning( + "Use `set_readable_format(signal, StandardReadableFormat" + ".CONFIG_SIGNAL)` instead of `add_config_signals(signal)`" + ), + stacklevel=2, + ) + for signal in signals: + self.set_readable_format(signal, StandardReadableFormat.CONFIG_SIGNAL) async def get_trigger_deadtime( self, settings: Settings | None = None ) -> tuple[set[DetectorTrigger], float | None]: """Get supported trigger types and deadtime for the detector. + The trigger logic is given the value of every signal this detector + reports as configuration, rather than nominating the ones it wants: + the values that determine deadtime are exactly the ones that ought to + be recorded for the scan anyway, so a second list to keep in step + earned nothing. + :param settings: Optional settings to use when getting configuration values :return: Tuple of supported trigger types and deadtime in seconds """ @@ -448,23 +476,32 @@ async def get_trigger_deadtime( self._trigger_logic.get_deadtime ): config_values = SignalDict() - for sig in self._trigger_logic.config_sigs(): + to_read: list[SignalR] = [] + for sig in _config_signals(self): if settings and sig in settings: # Use value from settings if it is in there # cast to a SignalRW because settings can only contain those config_values[sig] = settings[cast(SignalRW, sig)] else: - # Get the value live - config_values[sig] = await sig.get_value() + to_read.append(sig) + # Read live values concurrently: this is now every configuration + # signal rather than a handful the logic named, so doing it in + # series would be one round trip per signal + for sig, value in zip( + to_read, + await asyncio.gather(*(sig.get_value() for sig in to_read)), + strict=True, + ): + config_values[sig] = value deadtime = self._trigger_logic.get_deadtime(config_values) else: deadtime = None return self._supported_triggers, deadtime @AsyncStatus.wrap - async def stage(self) -> None: + async def _stage_detector(self) -> None: """Make sure the detector is idle and ready to be used.""" - coros = [data_logic.stop() for data_logic in self._data_logics] + coros: list[Awaitable] = [data_logic.stop() for data_logic in self._data_logics] if self._acquire_logic: coros.append(self._acquire_logic.ensure_ready()) await asyncio.gather(*coros) @@ -483,7 +520,6 @@ async def _update_prepare_context(self, trigger_info: TriggerInfo) -> None: == trigger_info.collections_per_event ): # Reuse the existing data providers - readable_data_providers = self._prepare_ctx.readable_data_providers streamable_data_providers = self._prepare_ctx.streamable_data_providers else: # Stop the existing providers if there is a context and make new ones @@ -492,36 +528,19 @@ async def _update_prepare_context(self, trigger_info: TriggerInfo) -> None: await data_logic.stop() # Setup the data logic for the right number of collections streamable_coros: list[Awaitable[StreamableDataProvider]] = [] - readable_coros: list[Awaitable[ReadableDataProvider]] = [] for dl in self._data_logics: - if _data_logic_supported(dl.prepare_unbounded): - streamable_coros.append( - dl.prepare_unbounded(self.name + dl.datakey_suffix) - ) - elif _data_logic_supported(dl.prepare_single): - if trigger_info.number_of_collections == 1: - readable_coros.append( - dl.prepare_single(self.name + dl.datakey_suffix) - ) - else: - logger.warning( - f"DataLogic {dl} only supports a single collection, but " - "the detector was prepared for " - f"{trigger_info.number_of_collections} collections." - ) - else: + if not _data_logic_supported(dl.prepare_unbounded): msg = f"DataLogic hasn't overridden any prepare_* methods {dl}" raise RuntimeError(msg) - streamable_data_providers, readable_data_providers = await asyncio.gather( - asyncio.gather(*streamable_coros), - asyncio.gather(*readable_coros), - ) + streamable_coros.append( + dl.prepare_unbounded(self.name + dl.datakey_suffix) + ) + streamable_data_providers = await asyncio.gather(*streamable_coros) # Stash the prepare context so we can use it in trigger/kickoff self._prepare_ctx = _PrepareCtx( trigger_info=trigger_info, streamable_data_providers=streamable_data_providers, - readable_data_providers=readable_data_providers, collections_written=await _get_collections_written( streamable_data_providers ), @@ -626,7 +645,7 @@ async def trigger(self) -> AsyncIterator[WatcherUpdate[int]]: """Trigger a single exposure. If [`prepare()`](#StandardDetector.prepare) has not been called since - the last [`stage()`](#StandardDetector.stage), an implicit prepare is + the last `stage()`, an implicit prepare is performed. When [](#OPHYD_ASYNC_PRESERVE_DETECTOR_STATE) is `YES` [](#DetectorTriggerLogic.default_trigger_info) is called to read current hardware state; otherwise a bare [`TriggerInfo()`](#TriggerInfo) is @@ -730,22 +749,30 @@ async def complete(self): ): yield update - async def describe_configuration(self) -> dict[str, DataKey]: - return await merge_gathered_dicts( - sig.describe() for sig in self._config_signals - ) + async def __on_read(self) -> dict[str, Reading]: + """What the data logics contribute to `read()`: nothing, but checks prepare. + + Raising when nothing has been prepared is deliberate, and predates + registered children being readable at all: a detector that has not been + prepared has no data keys. In practice `trigger()` prepares implicitly, + so `trigger_and_read` never reaches this. + """ + error_if_none(self._prepare_ctx, "Prepare not run") + return {} - async def read_configuration(self) -> dict[str, Reading]: - return await merge_gathered_dicts(sig.read() for sig in self._config_signals) + async def __on_describe(self) -> dict[str, DataKey]: + """What the data logics contribute to `describe()`. - async def describe(self) -> dict[str, DataKey]: + Raises when nothing has been prepared, as a `describe()` that quietly + succeeded would emit a descriptor missing the detector's data. + Streamable providers describe their shape for a step scan, but produce + their data through collect_asset_docs rather than read(). + """ ctx = error_if_none(self._prepare_ctx, "Prepare not run") - # Readable and Streamable data providers produce data during read - coros = [dp.make_datakeys() for dp in ctx.readable_data_providers] + [ - dp.make_datakeys(ctx.trigger_info.collections_per_event) - for dp in ctx.streamable_data_providers - ] - return await merge_gathered_dicts(coros) + return await merge_gathered_dicts( + sdp.make_datakeys(ctx.trigger_info.collections_per_event) + for sdp in ctx.streamable_data_providers + ) async def describe_collect(self) -> dict[str, DataKey]: ctx = error_if_none(self._prepare_ctx, "Prepare not run") @@ -756,18 +783,11 @@ async def describe_collect(self) -> dict[str, DataKey]: ] return await merge_gathered_dicts(coros) - @property - def hints(self) -> Hints: - fields: list[str] = [] + def __on_hints(self) -> Iterator[Hints]: + """The data logics' hinted fields, alongside the children's.""" for dl in self._data_logics: - fields.extend(dl.get_hinted_fields(self.name + dl.datakey_suffix)) - return Hints(fields=fields) - - async def read(self) -> dict[str, Reading]: - ctx = error_if_none(self._prepare_ctx, "Prepare not called") - return await merge_gathered_dicts( - dp.make_readings() for dp in ctx.readable_data_providers - ) + if fields := dl.get_hinted_fields(self.name + dl.datakey_suffix): + yield {"fields": list(fields)} async def collect_asset_docs( self, index: int | None = None @@ -793,9 +813,9 @@ async def get_index(self) -> int: return min_collections_written // ctx.trigger_info.collections_per_event @AsyncStatus.wrap - async def unstage(self) -> None: + async def _unstage_detector(self) -> None: """Stop the detector and file writing.""" - coros = [data_logic.stop() for data_logic in self._data_logics] + coros: list[Awaitable] = [data_logic.stop() for data_logic in self._data_logics] if self._acquire_logic: coros.append(self._acquire_logic.ensure_stopped()) await asyncio.gather(*coros) diff --git a/src/ophyd_async/core/_readable.py b/src/ophyd_async/core/_readable.py index 7ea972fd92..55c8e0f3a3 100644 --- a/src/ophyd_async/core/_readable.py +++ b/src/ophyd_async/core/_readable.py @@ -519,6 +519,33 @@ def add_readables( self.set_readable_format(device, format) +def _config_signals(device: Device) -> set[SignalR]: + """Return every `Signal` contributing to a Device's `read_configuration()`. + + Recurses into children registered as [](#StandardReadableFormat.CHILD), so + a Device that registers a sub-Device whole picks up whatever that + sub-Device declares as configuration. A Device that is not a + `StandardReadable` contributes nothing. + + Unlike `walk_config_signals` this reads the registry rather than calling + `read_configuration()`, so it does no I/O and includes read-only `SignalR`s + as well as `SignalRW`s (`ADBaseIO.model` is a `SignalR`, and it is exactly + the signal an areaDetector trigger logic needs for deadtime). + """ + signals: set[SignalR] = set() + if not isinstance(device, StandardReadable): + return signals + for entry in device.get_readable_formats(): + match entry: + case (signal, Format.CONFIG_SIGNAL): + signals.add(signal) + case (child, Format.CHILD): + signals |= _config_signals(child) + case _: + pass + return signals + + #: 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 diff --git a/src/ophyd_async/epics/adaravis.py b/src/ophyd_async/epics/adaravis.py index 2a0d89d176..26595e279c 100644 --- a/src/ophyd_async/epics/adaravis.py +++ b/src/ophyd_async/epics/adaravis.py @@ -63,9 +63,6 @@ class AravisTriggerLogic(DetectorTriggerLogic): process_plugin: NDProcessIO | None override_deadtime: float | None = None - def config_sigs(self) -> set[SignalR]: - return {self.driver.model} - def get_deadtime(self, config_values: SignalDict) -> float: return get_camera_deadtime( model=config_values[self.driver.model], diff --git a/src/ophyd_async/epics/adcore/__init__.py b/src/ophyd_async/epics/adcore/__init__.py index 27a079424b..be4307c00e 100644 --- a/src/ophyd_async/epics/adcore/__init__.py +++ b/src/ophyd_async/epics/adcore/__init__.py @@ -9,7 +9,6 @@ ADMultipartDataLogic, ADWriterFactory, NDArrayDescription, - PluginSignalDataLogic, ) from ._detector import AreaDetector, ContAcqDetector, plugins_with_proc from ._io import ( @@ -95,7 +94,6 @@ "ADContAcqAcquireLogic", # DataLogic "NDArrayDescription", - "PluginSignalDataLogic", "ADHDFDataLogic", "ADMultipartDataLogic", "ADWriterFactory", diff --git a/src/ophyd_async/epics/adcore/_acquire_logic.py b/src/ophyd_async/epics/adcore/_acquire_logic.py index 4d56d9a989..6c6b18e231 100644 --- a/src/ophyd_async/epics/adcore/_acquire_logic.py +++ b/src/ophyd_async/epics/adcore/_acquire_logic.py @@ -16,10 +16,29 @@ class ADAcquireLogic(DetectorAcquireLogic): + """Arm and disarm an areaDetector driver. + + :param driver: The areaDetector driver to arm. + :param driver_armed_signal: + The signal to watch for the driver having armed, if it is not `acquire`. + :param wait_for_plugins: + Whether the detector should wait for its plugins to drain before it + reports acquisition as complete. `WaitForPlugins` makes the driver's + `AcquireBusy` wait for the number of queued arrays to reach zero, and + `wait_for_idle` blocks on the `acquire` put callback, so turning it on + is what makes a step scan read plugin scalars for the frame it just + took rather than the one before. Defaults to on, since racing is + silent and the wait costs nothing when a file writer is the slowest + thing in the chain anyway. Turn it off for a detector whose plugin + chain must not gate acquisition. + :param timeout: How long to wait for the driver to arm and to return to idle. + """ + def __init__( self, driver: ADBaseIO, driver_armed_signal: SignalR[bool] | None = None, + wait_for_plugins: bool = True, timeout: float = DEFAULT_TIMEOUT, ): self.driver = driver @@ -27,9 +46,19 @@ def __init__( self.driver_armed_signal = driver_armed_signal else: self.driver_armed_signal = driver.acquire + self.wait_for_plugins = wait_for_plugins self.timeout = timeout self.acquire_status: AsyncStatus | None = None + async def ensure_ready(self): + # Stop first: WaitForPlugins is latched and has no _RBV, so set it once + # per scan while the driver is known to be idle rather than on the + # latency path before every arm. Always written, never left at whatever + # the IOC happened to boot with, so a scan does not depend on who + # touched the PV last. + await self.ensure_stopped() + await self.driver.wait_for_plugins.set(self.wait_for_plugins) + async def start_acquiring(self): self.acquire_status = await set_and_wait_for_other_value( set_signal=self.driver.acquire, @@ -59,6 +88,12 @@ class ADContAcqAcquireLogic(DetectorAcquireLogic): The CB plugin must read directly from the driver, so `stage()` raises if its `NDArrayPort` is not the driver's port. + Deliberately has no `wait_for_plugins` equivalent to [](#ADAcquireLogic). + That setting gates the driver's `AcquireBusy` going to 0 at the end of an + acquisition, and here the driver is always acquiring, so there is no such + transition to wait on. `wait_for_idle` waits on the circular buffer's + capture instead. + :param driver: The continuously acquiring detector driver :param cb_plugin: The circular buffer plugin gating downstream plugins :param num_frames_to_skip: diff --git a/src/ophyd_async/epics/adcore/_data_logic.py b/src/ophyd_async/epics/adcore/_data_logic.py index 343d7ce219..bc11d4cc8c 100644 --- a/src/ophyd_async/epics/adcore/_data_logic.py +++ b/src/ophyd_async/epics/adcore/_data_logic.py @@ -12,7 +12,6 @@ EnableDisable, PathInfo, PathProvider, - SignalDataProvider, SignalR, StreamableDataProvider, StreamResourceDataProvider, @@ -35,22 +34,6 @@ from ._ndattribute import NDAttributeDataType, NDAttributePvDbrType -@dataclass -class PluginSignalDataLogic(DetectorDataLogic): - driver: ADBaseIO - signal: SignalR - hinted: bool = True - - async def prepare_single(self, datakey_name: str) -> SignalDataProvider: - # Need to wait for all the plugins to have finished before we can read - # the plugin signal - await self.driver.wait_for_plugins.set(True) - return SignalDataProvider(self.signal) - - def get_hinted_fields(self, datakey_name: str) -> Sequence[str]: - return [self.signal.name] if self.hinted else [] - - @dataclass class NDArrayDescription: """Signals that describe the shape and data type of an NDArray frame. diff --git a/src/ophyd_async/epics/adcore/_detector.py b/src/ophyd_async/epics/adcore/_detector.py index 71d500ca6c..2a1cb72b2e 100644 --- a/src/ophyd_async/epics/adcore/_detector.py +++ b/src/ophyd_async/epics/adcore/_detector.py @@ -7,6 +7,7 @@ SignalR, StandardDetector, ) +from ophyd_async.core import StandardReadableFormat as Format from ._acquire_logic import ADContAcqAcquireLogic from ._data_logic import ADWriterFactory @@ -55,9 +56,14 @@ def __init__( writer, data_logic = factory(prefix, driver, plugin_list) setattr(self, factory.writer_name, writer) self.add_detector_logics(data_logic) - self.add_config_signals( - self.driver.acquire_period, self.driver.acquire_time, *config_sigs - ) + # The driver is always wired up, so register it whole and let ADBaseIO + # declare which of its signals are configuration. Plugins are + # deliberately *not* registered: a detector normally carries far more + # plugins than are wired into the chain, so opt in per plugin with + # `set_readable_format(det.stats, Format.CHILD)` instead. + self.set_readable_format(self.driver, Format.CHILD) + for signal in config_sigs: + self.set_readable_format(signal, Format.CONFIG_SIGNAL) super().__init__(name=name) def get_plugin( diff --git a/src/ophyd_async/epics/adcore/_io.py b/src/ophyd_async/epics/adcore/_io.py index 9278cfe706..65ec984eed 100644 --- a/src/ophyd_async/epics/adcore/_io.py +++ b/src/ophyd_async/epics/adcore/_io.py @@ -6,17 +6,15 @@ EnableDisable, SignalR, SignalRW, + StandardReadable, StrictEnum, SubsetEnum, SupersetEnum, TriggerableCommand, non_zero, ) -from ophyd_async.epics.core import ( - EpicsDevice, - EpicsOptions, - PvSuffix, -) +from ophyd_async.core import StandardReadableFormat as Format +from ophyd_async.epics.core import EpicsDevice, EpicsOptions, PvSuffix # Common classes for drivers and plugins @@ -116,7 +114,7 @@ class ADState(StrictEnum): ADBaseIOT = TypeVar("ADBaseIOT", bound="ADBaseIO") -class ADBaseIO(NDArrayBaseIO): +class ADBaseIO(StandardReadable, NDArrayBaseIO): """Base class from which areaDetector drivers are derived. This mirrors the interface provided by ADCore/db/ADBase.template. @@ -127,10 +125,14 @@ class ADBaseIO(NDArrayBaseIO): compatibility with changes made in https://github.com/areaDetector/ADCore/blob/master/RELEASE.md#asynndarraydriver-addriver """ - acquire_time: A[SignalRW[float], PvSuffix.rbv("AcquireTime")] - acquire_period: A[SignalRW[float], PvSuffix.rbv("AcquirePeriod")] - num_images: A[SignalRW[int], PvSuffix.rbv("NumImages")] - image_mode: A[SignalRW[ADImageMode], PvSuffix.rbv("ImageMode")] + acquire_time: A[SignalRW[float], PvSuffix.rbv("AcquireTime"), Format.CONFIG_SIGNAL] + acquire_period: A[ + SignalRW[float], PvSuffix.rbv("AcquirePeriod"), Format.CONFIG_SIGNAL + ] + num_images: A[SignalRW[int], PvSuffix.rbv("NumImages"), Format.CONFIG_SIGNAL] + image_mode: A[ + SignalRW[ADImageMode], PvSuffix.rbv("ImageMode"), Format.CONFIG_SIGNAL + ] detector_state: A[SignalR[ADState], PvSuffix("DetectorState_RBV")] # The following signals have been moved from NDArrayBaseIO for backwards @@ -140,11 +142,13 @@ class ADBaseIO(NDArrayBaseIO): # There is no _RBV for this one wait_for_plugins: A[SignalRW[bool], PvSuffix("WaitForPlugins")] - manufacturer: A[SignalR[str], PvSuffix("Manufacturer_RBV")] - model: A[SignalR[str], PvSuffix("Model_RBV")] - serial_number: A[SignalR[str], PvSuffix("SerialNumber_RBV")] - sdk_version: A[SignalR[str], PvSuffix("SDKVersion_RBV")] - firmware_version: A[SignalR[str], PvSuffix("FirmwareVersion_RBV")] + manufacturer: A[SignalR[str], PvSuffix("Manufacturer_RBV"), Format.CONFIG_SIGNAL] + model: A[SignalR[str], PvSuffix("Model_RBV"), Format.CONFIG_SIGNAL] + serial_number: A[SignalR[str], PvSuffix("SerialNumber_RBV"), Format.CONFIG_SIGNAL] + sdk_version: A[SignalR[str], PvSuffix("SDKVersion_RBV"), Format.CONFIG_SIGNAL] + firmware_version: A[ + SignalR[str], PvSuffix("FirmwareVersion_RBV"), Format.CONFIG_SIGNAL + ] # Classes for plugins @@ -165,15 +169,17 @@ class NDPluginBaseIO(NDArrayBaseIO): queue_size: A[SignalRW[int], PvSuffix.rbv("QueueSize")] -class NDROIIO(NDPluginBaseIO): +class NDROIIO(StandardReadable, NDPluginBaseIO): """Plugin for taking a region of an NDArray. This mirrors the interface provided by ADCore/db/NDROI.template. See HTML docs at https://areadetector.github.io/areaDetector/ADCore/NDPluginROI.html """ - size_x: A[SignalRW[int], PvSuffix.rbv("SizeX")] - size_y: A[SignalRW[int], PvSuffix.rbv("SizeY")] + min_x: A[SignalRW[int], PvSuffix.rbv("MinX"), Format.CONFIG_SIGNAL] + min_y: A[SignalRW[int], PvSuffix.rbv("MinY"), Format.CONFIG_SIGNAL] + size_x: A[SignalRW[int], PvSuffix.rbv("SizeX"), Format.CONFIG_SIGNAL] + size_y: A[SignalRW[int], PvSuffix.rbv("SizeY"), Format.CONFIG_SIGNAL] size_z: A[SignalRW[int], PvSuffix.rbv("SizeZ")] bin_x: A[SignalRW[int], PvSuffix.rbv("BinX")] @@ -181,7 +187,7 @@ class NDROIIO(NDPluginBaseIO): bin_z: A[SignalRW[int], PvSuffix.rbv("BinZ")] -class NDStatsIO(NDPluginBaseIO): +class NDStatsIO(StandardReadable, NDPluginBaseIO): """Plugin for computing statistics from an image or ROI within an image. This mirrors the interface provided by ADCore/db/NDStats.template. @@ -189,59 +195,78 @@ class NDStatsIO(NDPluginBaseIO): """ # Basic statistics - compute_statistics: A[SignalRW[bool], PvSuffix.rbv("ComputeStatistics")] - bgd_width: A[SignalRW[int], PvSuffix.rbv("BgdWidth")] - total: A[SignalR[float], PvSuffix("Total_RBV")] - mean_value: A[SignalR[float], PvSuffix("MeanValue_RBV")] + compute_statistics: A[ + SignalRW[bool], PvSuffix.rbv("ComputeStatistics"), Format.CONFIG_SIGNAL + ] + bgd_width: A[SignalRW[int], PvSuffix.rbv("BgdWidth"), Format.CONFIG_SIGNAL] + min_value: A[ + SignalR[float], PvSuffix("MinValue_RBV"), Format.HINTED_UNCACHED_SIGNAL + ] + min_x: A[SignalR[float], PvSuffix("MinX_RBV"), Format.UNCACHED_SIGNAL] + min_y: A[SignalR[float], PvSuffix("MinY_RBV"), Format.UNCACHED_SIGNAL] + max_value: A[ + SignalR[float], PvSuffix("MaxValue_RBV"), Format.HINTED_UNCACHED_SIGNAL + ] + max_x: A[SignalR[float], PvSuffix("MaxX_RBV"), Format.UNCACHED_SIGNAL] + max_y: A[SignalR[float], PvSuffix("MaxY_RBV"), Format.UNCACHED_SIGNAL] + mean_value: A[ + SignalR[float], PvSuffix("MeanValue_RBV"), Format.HINTED_UNCACHED_SIGNAL + ] + sigma: A[SignalR[float], PvSuffix("Sigma_RBV")] + total: A[SignalR[float], PvSuffix("Total_RBV"), Format.HINTED_UNCACHED_SIGNAL] + net: A[SignalR[float], PvSuffix("Net_RBV")] # Centroid statistics - compute_centroid: A[SignalRW[bool], PvSuffix.rbv("ComputeCentroid")] - centroid_threshold: A[SignalRW[float], PvSuffix.rbv("CentroidThreshold")] + compute_centroid: A[ + SignalRW[bool], PvSuffix.rbv("ComputeCentroid"), Format.CONFIG_SIGNAL + ] + centroid_threshold: A[ + SignalRW[float], PvSuffix.rbv("CentroidThreshold"), Format.CONFIG_SIGNAL + ] + centroid_x: A[SignalR[float], PvSuffix("CentroidX_RBV")] + centroid_y: A[SignalR[float], PvSuffix("CentroidY_RBV")] + sigma_x: A[SignalR[float], PvSuffix("SigmaX_RBV")] + sigma_y: A[SignalR[float], PvSuffix("SigmaY_RBV")] + sigma_xy: A[SignalR[float], PvSuffix("SigmaXY_RBV")] # X and Y Profiles - compute_profiles: A[SignalRW[bool], PvSuffix.rbv("ComputeProfiles")] + compute_profiles: A[ + SignalRW[bool], PvSuffix.rbv("ComputeProfiles"), Format.CONFIG_SIGNAL + ] profile_size_x: A[SignalR[int], PvSuffix("ProfileSizeX_RBV")] profile_size_y: A[SignalR[int], PvSuffix("ProfileSizeY_RBV")] - cursor_x: A[SignalRW[int], PvSuffix.rbv("CursorX")] - cursor_y: A[SignalRW[int], PvSuffix.rbv("CursorY")] + cursor_x: A[SignalRW[int], PvSuffix.rbv("CursorX"), Format.CONFIG_SIGNAL] + cursor_y: A[SignalRW[int], PvSuffix.rbv("CursorY"), Format.CONFIG_SIGNAL] # Array Histogram - compute_histogram: A[SignalRW[bool], PvSuffix.rbv("ComputeHistogram")] - hist_size: A[SignalRW[int], PvSuffix.rbv("HistSize")] - hist_min: A[SignalRW[float], PvSuffix.rbv("HistMin")] - hist_max: A[SignalRW[float], PvSuffix.rbv("HistMax")] + compute_histogram: A[ + SignalRW[bool], PvSuffix.rbv("ComputeHistogram"), Format.CONFIG_SIGNAL + ] + hist_size: A[SignalRW[int], PvSuffix.rbv("HistSize"), Format.CONFIG_SIGNAL] + hist_min: A[SignalRW[float], PvSuffix.rbv("HistMin"), Format.CONFIG_SIGNAL] + hist_max: A[SignalRW[float], PvSuffix.rbv("HistMax"), Format.CONFIG_SIGNAL] -class NDROIStatNIO(EpicsDevice): +class NDROIStatNIO(StandardReadable, EpicsDevice): """Defines the parameters for a single ROI used for statistics calculation. - Each instance represents a single ROI, with attributes for its position - (min_x, min_y) and size (size_x, size_y), as well as a name and use status. - This mirrors the interface provided by ADCore/db/NDROIStatN.template. See definition in ADApp/pluginSrc/NDPluginROIStat.h in https://github.com/areaDetector/ADCore. - - Attributes: - name: The name of the ROI. - use: Flag indicating whether the ROI is used. - min_x: The start X-coordinate of the ROI. - min_y: The start Y-coordinate of the ROI. - size_x: The width of the ROI. - size_y: The height of the ROI. - min_value: Minimum count value in the ROI. - max_value: Maximum count value in the ROI. - mean_value: Mean counts value in the ROI. - total: Total counts in the ROI. """ name_: A[SignalRW[str], PvSuffix("Name")] - use: A[SignalRW[bool], PvSuffix.rbv("Use")] - min_x: A[SignalRW[int], PvSuffix.rbv("MinX")] - min_y: A[SignalRW[int], PvSuffix.rbv("MinY")] - size_x: A[SignalRW[int], PvSuffix.rbv("SizeX")] - size_y: A[SignalRW[int], PvSuffix.rbv("SizeY")] - # stats - min_value: A[SignalR[float], PvSuffix("MinValue_RBV")] - max_value: A[SignalR[float], PvSuffix("MaxValue_RBV")] - mean_value: A[SignalR[float], PvSuffix("MeanValue_RBV")] - total: A[SignalR[float], PvSuffix("Total_RBV")] + use: A[SignalRW[bool], PvSuffix.rbv("Use"), Format.CONFIG_SIGNAL] + min_x: A[SignalRW[int], PvSuffix.rbv("MinX"), Format.CONFIG_SIGNAL] + min_y: A[SignalRW[int], PvSuffix.rbv("MinY"), Format.CONFIG_SIGNAL] + size_x: A[SignalRW[int], PvSuffix.rbv("SizeX"), Format.CONFIG_SIGNAL] + size_y: A[SignalRW[int], PvSuffix.rbv("SizeY"), Format.CONFIG_SIGNAL] + min_value: A[ + SignalR[float], PvSuffix("MinValue_RBV"), Format.HINTED_UNCACHED_SIGNAL + ] + max_value: A[ + SignalR[float], PvSuffix("MaxValue_RBV"), Format.HINTED_UNCACHED_SIGNAL + ] + mean_value: A[ + SignalR[float], PvSuffix("MeanValue_RBV"), Format.HINTED_UNCACHED_SIGNAL + ] + total: A[SignalR[float], PvSuffix("Total_RBV"), Format.HINTED_UNCACHED_SIGNAL] class NDROIStatIO(NDPluginBaseIO): diff --git a/src/ophyd_async/epics/advimba.py b/src/ophyd_async/epics/advimba.py index 1f74af96e4..9567ee5c30 100644 --- a/src/ophyd_async/epics/advimba.py +++ b/src/ophyd_async/epics/advimba.py @@ -100,9 +100,6 @@ class VimbaTriggerLogic(DetectorTriggerLogic): process_plugin: NDProcessIO | None override_deadtime: float | None = None - def config_sigs(self) -> set[SignalR]: - return {self.driver.model} - def get_deadtime(self, config_values: SignalDict) -> float: return get_camera_deadtime( model=config_values[self.driver.model], diff --git a/src/ophyd_async/fastcs/eiger/_detector.py b/src/ophyd_async/fastcs/eiger/_detector.py index 7257bde6a7..f993114c40 100644 --- a/src/ophyd_async/fastcs/eiger/_detector.py +++ b/src/ophyd_async/fastcs/eiger/_detector.py @@ -2,6 +2,7 @@ PathProvider, SignalR, StandardDetector, + StandardReadableFormat, TriggerableCommand, soft_signal_rw, ) @@ -44,6 +45,6 @@ def __init__( ), ) - self.add_config_signals(self.pixel_mask) + self.set_readable_format(self.pixel_mask, StandardReadableFormat.CONFIG_SIGNAL) super().__init__(name=name, connector=connector) diff --git a/src/ophyd_async/sim/_blob_detector.py b/src/ophyd_async/sim/_blob_detector.py index ed21968784..6760e088a1 100644 --- a/src/ophyd_async/sim/_blob_detector.py +++ b/src/ophyd_async/sim/_blob_detector.py @@ -1,6 +1,7 @@ from collections.abc import Sequence from ophyd_async.core import PathProvider, SignalR, StandardDetector +from ophyd_async.core import StandardReadableFormat as Format from ._blob_acquire_logic import BlobAcquireLogic from ._blob_data_logic import BlobDataLogic @@ -26,5 +27,6 @@ def __init__( path_provider=path_provider, pattern_generator=self.pattern_generator ), ) - self.add_config_signals(*config_sigs) + for signal in config_sigs: + self.set_readable_format(signal, Format.CONFIG_SIGNAL) super().__init__(name=name) diff --git a/tests/container_tests/test_adsim_system.py b/tests/container_tests/test_adsim_system.py index f1791639f0..b9a3c3f426 100644 --- a/tests/container_tests/test_adsim_system.py +++ b/tests/container_tests/test_adsim_system.py @@ -198,10 +198,24 @@ def test_software_triggering( "data": { "adsim-driver-acquire_period": 0.005, "adsim-driver-acquire_time": 0.1, + "adsim-driver-num_images": 1, + "adsim-driver-image_mode": "Multiple", + "adsim-driver-manufacturer": "Simulated detector", + "adsim-driver-model": "Basic simulator", + "adsim-driver-serial_number": "No serial number", + "adsim-driver-sdk_version": ANY, + "adsim-driver-firmware_version": "No firmware", }, "timestamps": { "adsim-driver-acquire_period": ANY, "adsim-driver-acquire_time": ANY, + "adsim-driver-num_images": ANY, + "adsim-driver-image_mode": ANY, + "adsim-driver-manufacturer": ANY, + "adsim-driver-model": ANY, + "adsim-driver-serial_number": ANY, + "adsim-driver-sdk_version": ANY, + "adsim-driver-firmware_version": ANY, }, "data_keys": { "adsim-driver-acquire_period": { @@ -220,6 +234,50 @@ def test_software_triggering( "units": "", "precision": 3, }, + "adsim-driver-num_images": { + "dtype": "integer", + "shape": [], + "dtype_numpy": " set[SignalR]: - """Return the deadtime signal as a config signal.""" - return {self.deadtime_signal} - def get_deadtime(self, config_values: SignalDict) -> float: - """Return the deadtime from the signal value.""" + """Return the deadtime from the signal value. + + The signal reaches us because the detector registers it as + CONFIG_SIGNAL, not because this logic nominates it. + """ return config_values[self.deadtime_signal] async def prepare_internal(self, num: int, livetime: float, deadtime: float): @@ -129,26 +129,14 @@ async def ensure_stopped(self): self.disarm_count += 1 -class ReadableOnlyDataLogic(DetectorDataLogic): - """Produces only readable (non-streaming) data.""" - - def __init__(self): - self.signal = soft_signal_rw(int, initial_value=42, name="foo-value") - - async def prepare_single(self, datakey_name: str) -> ReadableDataProvider: - return SignalDataProvider(self.signal) - - def get_hinted_fields(self, datakey_name: str) -> Sequence[str]: - return ["foo-value"] - - class StreamableOnlyDataLogic(DetectorDataLogic): """Produces only streamable (file-based) data.""" - def __init__(self, tmp_path): + def __init__(self, tmp_path, datakey_suffix: str = ""): self.collections_written = soft_signal_rw(int) self.stop_count = 0 self.tmp_path = tmp_path + self.datakey_suffix = datakey_suffix async def prepare_unbounded(self, datakey_name: str) -> StreamableDataProvider: resource = StreamResourceInfo( @@ -231,6 +219,10 @@ async def test_get_trigger_deadtime( det = StandardDetector() if trigger_logic: det.add_detector_logics(trigger_logic) + if isinstance(trigger_logic, DeadtimeTriggerLogic): + # A logic that needs a signal declares it as configuration; the logic + # no longer nominates signals separately + det.set_readable_format(trigger_logic.deadtime_signal, Format.CONFIG_SIGNAL) triggers, deadtime = await det.get_trigger_deadtime() assert triggers == expected_triggers assert deadtime == expected_deadtime @@ -246,6 +238,7 @@ async def test_get_trigger_deadtime_with_settings(): det.sig = deadtime_signal tl = DeadtimeTriggerLogic(deadtime_signal) det.add_detector_logics(tl) + det.set_readable_format(deadtime_signal, Format.CONFIG_SIGNAL) # Verify initial deadtime from signal triggers, deadtime = await det.get_trigger_deadtime() @@ -432,10 +425,10 @@ async def test_arm_logic_called_on_stage(): assert al.armed is False -async def test_describe_before_prepare_raises(): +async def test_describe_before_prepare_raises(tmp_path): """Test that describe() fails before prepare().""" det = StandardDetector() - det.add_detector_logics(ReadableOnlyDataLogic()) + det.add_detector_logics(StreamableOnlyDataLogic(tmp_path)) with pytest.raises(RuntimeError, match="Prepare not run"): await det.describe() @@ -624,23 +617,21 @@ async def test_stage_resets_state(): assert await det.events_to_kickoff.get_value() == 0 -async def test_hints_from_single_data_logic(): +async def test_hints_from_single_data_logic(tmp_path): """Test that hints come from data logic.""" - det = StandardDetector() - det.add_detector_logics(ReadableOnlyDataLogic()) + det = StandardDetector(name="bar") + det.add_detector_logics(StreamableOnlyDataLogic(tmp_path)) await det.prepare(TriggerInfo()) - assert det.hints == { - "fields": ["foo-value"] - } # ReadableOnlyDataLogic uses foo-value + assert det.hints == {"fields": ["bar"]} async def test_hints_from_multiple_data_logics(tmp_path): """Test that hints are aggregated from multiple data logics.""" det = StandardDetector(name="bar") - dl1 = ReadableOnlyDataLogic() - dl2 = StreamableOnlyDataLogic(tmp_path) + dl1 = StreamableOnlyDataLogic(tmp_path) + dl2 = StreamableOnlyDataLogic(tmp_path, datakey_suffix="-extra") det.add_detector_logics(dl1, dl2) await det.prepare(TriggerInfo()) @@ -648,7 +639,7 @@ async def test_hints_from_multiple_data_logics(tmp_path): # Should include hints from both logics hints = det.hints assert "fields" in hints - assert hints["fields"] == ["foo-value", "bar"] + assert hints["fields"] == ["bar", "bar-extra"] @pytest.mark.parametrize( @@ -667,7 +658,7 @@ async def test_config_signals_in_describe_configuration( signal = soft_signal_rw( signal_type, initial_value=initial_value, name="test-config" ) - det.add_config_signals(signal) + det.set_readable_format(signal, Format.CONFIG_SIGNAL) await det.stage() @@ -694,13 +685,13 @@ async def test_config_signals_in_describe_configuration( async def test_kickoff_without_streamable_data_raises(): """Test that kickoff() without streamable data fails.""" det = StandardDetector(name="foo") - det.add_detector_logics(JustInternalTriggerLogic(), ReadableOnlyDataLogic()) + det.add_detector_logics(JustInternalTriggerLogic()) - # Single event prepare for readable-only logic + # A detector with no data logic can still be triggered for a step scan await det.prepare(TriggerInfo()) await det.trigger() # This works - # Readable-only logic doesn't support kickoff + # ...but there is nothing to collect, so it cannot be flown await det.prepare(TriggerInfo(number_of_events=5)) with pytest.raises( ValueError, match="Detector foo is not streamable, so cannot kickoff" @@ -787,13 +778,13 @@ async def test_streamable_supports_both_step_and_fly(tmp_path): async def test_read_returns_correct_values(): - """Test that read() returns values from readable providers.""" + """Test that read() returns the values of registered signals.""" det = StandardDetector(name="det") - dl = ReadableOnlyDataLogic() - det.add_detector_logics(dl) + det.counts, _ = soft_signal_r_and_setter(int, 42, name="det-counts") + det.set_readable_format(det.counts, Format.HINTED_SIGNAL) await det.trigger() - await assert_reading(det, {"foo-value": {"value": 42}}) + await assert_reading(det, {"det-counts": {"value": 42}}) async def test_detector_with_no_logics(): @@ -902,19 +893,6 @@ async def ensure_stopped(self): assert al.shutter_closed is True # unstage() must close the shutter -async def test_multiple_collections_with_single_only_logic_warns(caplog): - """Test that requesting multiple collections warns, leads to empty prepare ctx.""" - det = StandardDetector() - det.add_detector_logics(ReadableOnlyDataLogic()) - - await det.prepare(TriggerInfo(number_of_events=5)) - - assert "only supports a single collection" in caplog.text - assert det._prepare_ctx is not None - assert det._prepare_ctx.readable_data_providers == [] - assert det._prepare_ctx.streamable_data_providers == [] - - async def test_data_logic_with_no_prepare_methods_raises(): """Test error when DataLogic doesn't override any prepare methods.""" @@ -979,16 +957,18 @@ async def test_different_collections_written_raises(tmp_path): await det.kickoff() -async def test_multiple_data_logics(tmp_path): - """Test detector with multiple data logics.""" +async def test_data_logic_and_registered_signal(tmp_path): + """A registered signal and a data logic both describe(), only one collects.""" det = StandardDetector(name="det") - dl1 = ReadableOnlyDataLogic() - dl2 = StreamableOnlyDataLogic(tmp_path) - det.add_detector_logics(JustInternalTriggerLogic(), dl1, dl2) + det.counts, _ = soft_signal_r_and_setter(int, 42, name="det-counts") + det.set_readable_format(det.counts, Format.HINTED_SIGNAL) + det.add_detector_logics( + JustInternalTriggerLogic(), StreamableOnlyDataLogic(tmp_path) + ) await det.prepare(TriggerInfo()) - # Should have data from both logics + # Should have data from the data logic and the registered signal description = await det.describe() assert description == { "det": { @@ -998,11 +978,11 @@ async def test_multiple_data_logics(tmp_path): "shape": [1, 10, 15], "source": f"file://localhost/{tmp_path.as_posix().lstrip('/')}/test.h5", }, - "foo-value": { + "det-counts": { "dtype": "integer", "dtype_numpy": "