Skip to content

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

Closed
coretl wants to merge 25 commits into
mainfrom
roistat-ts
Closed

coretl wants to merge 25 commits into
mainfrom
roistat-ts

Conversation

@coretl

@coretl coretl commented Nov 3, 2026

Copy link
Copy Markdown
Owner

Draft PR to run CI (including the container tests) on the detector 5 stack tip, branch roistat-ts. Not for review or merge: the stack is still WIP and will be split into separate PRs.

New container test to watch: tests/container_tests/test_adsim_system.py::test_roistat_is_served_by_the_hdf_file_or_the_time_series (ROIStat time series vs HDF file against the sim IOC).

AI Disclaimer

The code and this description were generated with AI and reviewed/tweaked by the author.

coretl and others added 25 commits October 28, 2026 12:55
…luesky#1470)

example-services 3c0274b includes services/epics-opis/compose.yml twice. Compose 5.x (Docker 29, ubuntu-26.04) rejects the merged duplicate security_opt, so `docker compose up` exits 1 and no container starts. The ca_gateway fixture read compose's output to EOF and yielded anyway, so the tests failed later with "Connection refused". Bump to 9e0faf7, which removes the duplicate include.

docker_composer now raises with compose's output when it exits non-zero before printing the ready line.

Assisted-by: Claude Sonnet 5.5
Split cut F of detector-rederive (d0872e185) onto origin/main: the
StandardMovable/StandardFlyable logic rework, the fly() generator,
ensure_stopped(), StoppedError/TransitionError and the motor, pmac,
panda and sim motor ports, with ADRs 0021 and 0022. The old detector
keeps running on it unchanged.

Assisted-by: Claude Opus 5.5
Assisted-by: Claude Sonnet 5.5
…in settings

Split cut G1 (bluesky#1400) of detector-rederive (d0872e185) on split/F:
StandardReadable's per-device format registry and per-verb hook
registries, set/reset/get_readable_format(s), and storing the format
changes with store_settings/apply_settings, with ADR 0020. The
shadowing hook used only by the detector is left to the detector PR.

Assisted-by: Claude Opus 5.5
Assisted-by: Claude Sonnet 5.5
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.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ng read/config/describe from children automatically

(cherry picked from commit 3219303)
…onents for config/read

(cherry picked from commit 64f4cd3)
(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.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(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 left 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.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(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 --
  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.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(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.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(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 -- 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 0021.
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
Split cut G3 (bluesky#1408) of detector-rederive (d0872e185): every file at
its tip content (merged with origin/main), including the readable
suppression hook moved here from bluesky#1400, minus the AD stats time series,
which is the next PR. Deletes the unreleased ADWriterFactory.stats and
ADWriterFactory.hdf(enable_callbacks=) shims rather than splitting them.
ADRs 0023 and 0024.

Assisted-by: Claude Opus 5.5
Assisted-by: Claude Sonnet 5.5
Split cut G4 (bluesky#1367) of detector-rederive (d0872e185): the
StatsTimeSeriesDataLogic, the ADDataFactory.stats_time_series factory
and their tests (UC6, UC7, HDF plus time series, two stats plugins, the
adsim container tests). Together with the PRs below it, the tree is the
tip merged with origin/main, less the deleted ADWriterFactory shims.

Round 3 of the stats naming review folded in: the plugin snapshot is taken
once at stage and also records NDArrayPort and ROIStat channel names, the
plugins a trigger logic holds are not recorded, plugins= are registered as
readables, data factories take plugins by reference, NDROIStatIO is a
StandardReadable, and the writers no longer write EnableCallbacks at prepare.

Assisted-by: Claude Opus 5.5
Assisted-by: Claude Sonnet 5.5
ADDataFactory.roistat_time_series(roistat) publishes one array per used
channel per scalar with a read format, named after that scalar, as event
pages, mirroring stats_time_series.

Assisted-by: Claude Opus 5.5
NDROIStatNIO gains net (Net_RBV, no read format, as on NDStatsIO) and
ts_net, so TSNet is published when net has a read format.

EventPageDataProvider takes an optional refresh, awaited before each
page's arrays are read. The stats time series writes TS:TSRead and the
ROIStat one writes READ to TSControl, so the plugin posts its arrays up
to the current point instead of only about once a second.

Both logics read TSNumPoints back after sizing it and raise if the IOC
clamped it below the scan's number of collections.

Assisted-by: Claude Opus 5.5
- Enable the NDPluginTimeSeries that NDPluginStats embeds when arming the
  stats time series; the IOC starts it disabled, so TSCurrentPoint never moved.
- Leave an empty description out of the NDAttributes XML: eight ROIStat
  attributes were 1093 characters, over the plugin's 1024 element waveform
  (CAException 176, invalid element count).
- Cancel and close the sibling array waits when one times out in make_page,
  which leaked a gather_dict task.

Assisted-by: Claude Sonnet 5.5
…t, not a 1 ms sleep

Assisted-by: Claude Sonnet 5.5
@coretl

coretl commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

🤖 Superseded by the stacked drafts #21 to #25.

@coretl coretl closed this Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants