Skip to content

Add a hardware-free naming API for static PV-name prediction - #65

Open
gilesknap wants to merge 3 commits into
gui/inline-root-controllersfrom
catio-naming-api
Open

Add a hardware-free naming API for static PV-name prediction#65
gilesknap wants to merge 3 commits into
gui/inline-root-controllersfrom
catio-naming-api

Conversation

@gilesknap

Copy link
Copy Markdown
Contributor

Adds fastcs_catio.namingpredict_chain() / predict_names() — which derive
the PV names an EtherCAT chain will get without a bus, a controller, or an
event loop
.

No open issue covers this; it is driven by a downstream consumer,
epics-containers/builder2ibek#128.

Why

At runtime the controller names are a side effect of ADS discovery:
FastCSClient._get_ethercat_chains stamps each slave's ChainLocation, and
CATioServerController._resolve_controller_name_and_path renders it through
CATioNameMappings. Both need live hardware.

A site migrating an existing installation onto fastcs-catio needs those same
names before any hardware exists — other IOCs hold PV references that must be
rewritten ahead of the switchover. DLS is doing exactly this: replacing the
legacy ethercat scanner beamline by beamline, where the new names differ from
the old ones and every consumer IOC's database has to be migrated in step.

What it does, and what it deliberately does not

Purely additive. catio_controller.py and client.py are untouched; the
runtime path does not change. The diff is two new files.

Keyed on terminal type names ("EL3104"), not on
(vendor_id, product_code, revision_number). A static description of a chain is
all a migration tool has — DLS builder XML carries type_rev="EL3104 rev 0x00120000" and nothing else — whereas the runtime keys on the identity
reported over ADS. Looking up by type name is revision-independent, which
mirrors the vendor+product fallback get_terminal_type_by_identity already
applies when a rig runs newer firmware than the cached description.

strict=True raises UnknownTerminalTypeError for a terminal missing from
terminal_types.yaml. The runtime is deliberately forgiving here — an
unrecognised identity renders as MOD<position> — but that is the wrong default
for a migration tool, which would then rewrite live PV references to names the
IOC never creates. Callers generating names for real hardware should ask for the
error; strict=False reproduces the runtime's behaviour.

The part worth reviewing most

tests/test_naming.py is the anti-drift mechanism. 18 of its 27 tests drive the
real runtime code path_get_ethercat_chains, _generate_system_tree,
_compute_module_alias_indices, _resolve_controller_name_and_path — over 6
chain shapes × 3 template shapes, and assert predict_names() agrees. A
reimplementation that silently drifts from the runtime is the obvious failure
mode for a module like this, so the tests are written to break in this repo,
where the change that caused the drift lives, rather than downstream.

One deliberate oddity: the helper uses a private event loop and restores the
previous one, rather than asyncio.run(). asyncio.run clears the
process-wide loop on exit, which strands the loop tests/test_system.py
installs for its ADS simulator — that loop and its sockets are then collected
unclosed and pytest raises PytestUnraisableExceptionWarning against an
unrelated test. It is invisible when the file runs alone and only shows up in a
full-suite run, so please don't simplify it back.

Base branch — please read before merging

This targets gui/inline-root-controllers (#63), not main, because
naming.py needs CATioNameMappings and get_terminal_type(...).group_alias,
and a tree-wide grep of main's src/ returns zero hits for either. Against
main it does not merely misbehave, it fails at the import line. Both arrive
with #63.

If #63 merges first, retarget this to main — it should need no other change.

Instructions to reviewer on how to test:

  1. uv run pytest — expect 267 passed, 23 skipped (240 on the base branch,
    plus 27 new tests and 1 new doctest).
  2. uv run pyright src tests — expect 0 errors.
  3. git diff gui/inline-root-controllers --stat — confirm only two files are
    added and nothing existing is modified.
  4. Read the predict_names doctest for the shape of the output:
    a (node, position) -> PV prefix mapping.

Checks for reviewer

  • Would the PR title make sense to a user on a set of release notes

At runtime the controller names are a side effect of ADS discovery:
`_get_ethercat_chains` stamps each slave's ChainLocation and
`_resolve_controller_name_and_path` renders it through CATioNameMappings.
Both need a live bus.

A tool migrating an existing installation onto fastcs-catio needs the same
names *before* any hardware exists, so it can rewrite the PV references other
IOCs hold. `predict_chain`/`predict_names` derive them from the chain order
alone, keyed on terminal type names because that is what a static description
of a chain carries, where the runtime keys on (vendor_id, product_code,
revision_number) reported over ADS.

Purely additive: catio_controller.py is untouched. tests/test_naming.py locks
the output against the real runtime path over 6 chain shapes x 3 template
shapes, so the two implementations cannot drift apart silently.

Written for builder2ibek's `catio` command
(epics-containers/builder2ibek#128), which imports predict_names.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.07143% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.30%. Comparing base (a37e7d6) to head (fdfdd2b).

Files with missing lines Patch % Lines
src/fastcs_catio/naming.py 91.07% 10 Missing ⚠️
Additional details and impacted files
@@                       Coverage Diff                       @@
##           gui/inline-root-controllers      #65      +/-   ##
===============================================================
+ Coverage                        75.91%   76.30%   +0.39%     
===============================================================
  Files                               19       20       +1     
  Lines                             4197     4309     +112     
===============================================================
+ Hits                              3186     3288     +102     
- Misses                            1011     1021      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gilesknap and others added 2 commits August 10, 2026 10:01
The helper added with these tests read the current event loop so it could put
it back afterwards. `asyncio.get_event_loop()` is deprecated from Python 3.12,
and `filterwarnings = "error"` turns that warning into an exception raised
before the coroutine is ever awaited, so the file dies with "coroutine
_get_ethercat_chains was never awaited".

It only fires when no loop has been installed yet, which is why CI did not
catch it: in a full run an earlier test has already installed one and the
lookup returns it without warning. Running just this file -- what a developer
does -- fails on 3.12 and 3.13.

`new_event_loop()` does not install a loop and `run_until_complete` drives the
loop object directly, so nothing needs saving or restoring. That keeps the
original property the helper existed for (not clearing the loop test_system.py
installs for its ADS simulator, whose sockets would then be collected unclosed
and reported against an unrelated test) without reading global asyncio state at
all.

Verified on 3.11, 3.12 and 3.13, both for this file alone and for the full
suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`dox_current_output_current` was the only unselected symbol anyone actually
uses. It is the terminal's whole point -- an EL2595 is an LED constant current
driver and the output current is the value you set -- while the two symbols
that were selected, `dox_status` and `dox_control`, are packed bitfields.

DLS's legacy `ethercat` module exposes it as `$(DEVICE):DOXCURRENT:OUTPUTCURRENT`
and BL21I writes to it on four terminals (`BL21I-DI-LED-01/02/03` and
`BL21I-OP-LED-01`). Without this the migration onto fastcs-catio has nothing to
point those references at, because no PV is created for the attribute at all.

Hand-edited rather than regenerated, by the maintainer's explicit decision: this
is a single boolean selection, exactly what the `catio-terminals edit` GUI
toggles, not an import of new Beckhoff XML. Verified as the only selection
change in the file.

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

1 participant