Repository navigation
Conversation
This was referenced Nov 10, 2026
Rework StandardMovable and StandardFlyable around a single logic, add fly() and ensure_stopped()
#21
Draft
coretl
force-pushed
the
split/G1
branch
2 times, most recently
from
November 10, 2026 15:01
21b8895 to
8642842
Compare
So that set_readable_format() works on a detector at all. read(), describe()
and hints merge the registered children on top of whatever the data logics
produce, and add_config_signals becomes a deprecated wrapper. The detector
overrides _extra_funcs_for()/_extra_hint_sources() rather than reimplementing
the verbs, so a detector with no hinted fields now reports hints as {} rather
than {"fields": []}, matching StandardReadable.
Split out of the runtime-readable-format work so that PR stays core-only.
Assisted-by: Claude Opus 5.5
…ng read/config/describe from children automatically (cherry picked from commit 3219303)
…onents for config/read (cherry picked from commit 64f4cd3)
…er logics (cherry picked from commit 404930b)
(cherry picked from commit dd36b7a)
bluesky#1395 registered every StandardReadable plugin as a CHILD readable in AreaDetector.__init__. That is dropped in the rebase onto bluesky#1400, for the reason @jwlodek gave on the PR himself: "what if we didn't add plugins as readables automatically when passed in at __init__, and instead explicitly required an add_readables call to include them?" and the reason @coretl gave for asking: a detector typically carries far more plugins (16 ROIs, 16 stats) than are wired into its chain, so reporting all of them gives no way to tell which values are meaningful. So the three tests that expected stats readings now say so, with set_readable_format(stat, Format.CHILD). NDStatsIO declares its own config and hinted signals, so opting the plugin in as a CHILD picks up exactly what these tests already assert. The wait_for_plugins assertions go with the AreaDetector override that set it, which both reviewers agreed was too hidden and belongs in an explicit data logic instead. Assisted-by: Claude Opus 5.5 (cherry picked from commit f56c523)
…etector _update_prepare_context override, conditional on number_of_events == 1. Both reviewers called that too hidden, and the condition was unnecessary: WaitForPlugins gates the driver's AcquireBusy going to 0 when acquisition *completes*, so it never throttles frames. A flyscan does exactly one wait_for_idle, at the end, which is precisely when the plugins should have drained. It belongs on the acquire logic because that is what owns "poke this, then check acquire is done": wait_for_idle already blocks on the acquire put callback, which is the AcquireBusy that WaitForPlugins gates, so the existing wait needs no change at all. Defaults to on, since racing is silent and the wait costs nothing when a file writer is the slowest thing in the chain anyway. Off is a constructor argument rather than a subclass, matching how driver_armed_signal already parametrises this class; a detector needing genuinely custom arm/idle logic still subclasses instead. Set from ensure_ready, not start_acquiring: the PV is latched and has no _RBV, so it is written once per scan while the driver is known idle rather than on the latency path before every arm. Stop first, then always write it, never leaving it at whatever the IOC booted with, so a scan cannot depend on who touched the PV last. ADContAcqAcquireLogic deliberately gets no equivalent: the driver is always acquiring there, so there is no AcquireBusy transition to gate. Said so on the class so it does not look like an oversight. Assisted-by: Claude Opus 5.5 (cherry picked from commit fe787ce)
DetectorTriggerLogic no longer nominates the signals its deadtime depends on. get_trigger_deadtime passes the value of every signal the detector reports as configuration instead, so there is no second list to keep in step with the first: the values that determine deadtime are exactly the ones that ought to be recorded for the scan anyway. StandardReadable gains get_config_signals(), which walks the registry and recurses through CHILD-registered children. That recursion is what makes it work for areaDetector: AreaDetector registers the driver whole, and ADBaseIO declares model as CONFIG_SIGNAL, so the signal AravisTriggerLogic and VimbaTriggerLogic need arrives without either class saying anything. Both deadtime_sigs overrides are deleted. Unlike walk_config_signals it reads the registry rather than calling read_configuration(), so it does no I/O and includes read-only SignalRs. driver.model is a SignalR, so the existing helper would have dropped precisely the signal that matters. Two consequences, both real: - A logic needing a signal must ensure it is declared as configuration. If it is not, get_deadtime gets a KeyError rather than a silently fetched value. - get_trigger_deadtime now reads every configuration signal, where a logic could previously declare none and pay nothing. Reads are gathered so it stays one round trip, but it does mean the call now needs a connected device even when deadtime is a constant, so test_adpilatus::test_deadtime had to start connecting its detector. Also moves every add_readables call in adcore/_io.py into the signal annotations, which removes three __init__ methods that existed only to declare formats. The remaining registration calls elsewhere cannot follow: they take caller-supplied config_sigs or a driver passed in as a constructor argument, neither of which is declarable. Assisted-by: Claude Opus 5.5 (cherry picked from commit c239654)
get_config_signals was added as a public method on StandardReadable in the previous commit. bluesky#1367 restructures detector logic, and whether this should be a method, a function, or something else falls out of that, so it stays a module private _config_signals() in _readable.py until then. Nothing is added to core.__all__. Taking a Device rather than a StandardReadable also simplifies the recursion: a CHILD that is not a StandardReadable contributes nothing without the caller having to check. Assisted-by: Claude Opus 5.5 (cherry picked from commit ca5fcb7)
A StandardDetector is a StandardReadable now, so a source that produces one value per event is a registered signal, not a data logic: set_readable_format puts it in read() and describe() for any number of collections, where a prepare_single logic dropped itself with a warning as soon as a scan asked for more than one. That leaves the tier with nothing of its own, as its only implementation, PluginSignalDataLogic, has already gone the same way. So prepare_single, ReadableDataProvider and SignalDataProvider go rather than staying as a second way to get a scalar into an event. Also restores the two doc sections held back from the previous branch, since this is the change that makes them true, and points ADR 0012 at 0023. Assisted-by: Claude Opus 5.5
The replayed bluesky#1403 commits target the old StandardReadable hooks (_Verb, _extra_funcs_for, _extra_hint_sources, _HintedFields), which bluesky#1400 replaced with per-verb registries. StandardDetector now adds __on_read/__on_describe/__on_hints to _read_funcs/_describe_funcs/ _hints_funcs, appended after the registered children as on the tip. _config_signals walks the typed registry entries. Tests and the eiger detector move off the deprecated add_readables/add_config_signals, the stats hints test registers stats.total as a hinted signal now that PluginSignalDataLogic is gone, and two doc links are fixed. Drafts the CHANGELOG entries for this PR. Assisted-by: Claude Opus 5.5
… the trigger logic Assisted-by: Claude Opus 5.5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fourth of six stacked draft PRs, based on #22. It makes
StandardDetectoraStandardReadable, so a detector's configuration and read signals are registered withset_readable_format()like any other readable, and the areaDetector IO classes declare their own formats. Next: #24Supersedes bluesky#1403
Four of its commits are authored by Jakub Wlodek, from the upstream PR; the rest are on top of them.
What changes
StandardDetectoris aStandardReadable(ADR 0024). What the data logics produce is added toread(),describe()andhintsalongside the registered signals.add_config_signals()is deprecated in favour ofset_readable_format(signal, StandardReadableFormat.CONFIG_SIGNAL). A detector with no hinted fields reportshintsas{}rather than{"fields": []}.stage()does, replacing the single_funcs_for(verb)switch and the_extra_*hooks.DetectorTriggerLogic.config_sigs()is removed.get_deadtime()is given the value of every signal the detector reports as configuration, so a signal that determines deadtime must be registered asCONFIG_SIGNAL.prepare_singledata logic tier is removed, withReadableDataProvider,SignalDataProviderandadcore.PluginSignalDataLogic. Register a plugin scalar withset_readable_format(signal, StandardReadableFormat.HINTED_SIGNAL)instead.ADBaseIO,NDROIIO,NDStatsIO,NDROIStatNIO) areStandardReadables that declare their own configuration and read signals, so registering a plugin asCHILDof anAreaDetectorputs them inread()andread_configuration().ADAcquireLogicsetsWaitForPluginsonstage()from its newwait_for_pluginsargument (default on), rather thanAreaDetectorhiding it, so a step scan reads plugin scalars for the frame it has just taken.Breaking changes
Out-of-tree trigger logics that implement
config_sigs()must register those signals asCONFIG_SIGNALinstead;prepare_singledata logics must become registered readables;add_config_signals()andadd_readables()warn. Listed in the CHANGELOG diff.How to review
core/_detector.pyfor the readable integration, thenepics/adcore/_io.pyfor the format annotations on the IO classes, then the AD detector and acquire logic. The commits are small, so reading them in order works well.Position in the stack
Previous: #22. Next: #24
AI Disclaimer
The code and this description were generated with AI and reviewed/tweaked by the author.