Manage controllers' PV names from yaml config and modify GUI screen generation#63
Open
ggayDiamond wants to merge 22 commits into
Open
Manage controllers' PV names from yaml config and modify GUI screen generation#63ggayDiamond wants to merge 22 commits into
ggayDiamond wants to merge 22 commits into
Conversation
Now 'controllers' is correctly defined as an array rather than a keyed object. Used commands: cd /workspaces/CATio/src/fastcs_catio uv run fastcs-catio schema > schema.json
- Add `IONodeType.Box` enum value and handle it like a coupler in
`get_type_name()` (assigns RIO{node} PV name)
- Detect Box terminals via regex `_BOX_TYPE_RE` in `client.py` and
assign `IONodeType.Box` category during chain location calculation
- Include Box nodes in the controller dispatch match-case alongside
Coupler and Slave; always resolve terminal controllers via
`get_terminal_controller_class()`
- Rename `SUPPORTED_CONTROLLERS` → `SUPPORTED_DEVICE_CONTROLLERS` and
remove commented-out legacy terminal entries (only `ETHERCAT` device
controller remains)
- Rename `get_supported_hardware` → `get_supported_devices` to reflect
device-only scope
- Update unit tests for new Box node type and renamed test method
Couplers/boxes whose resolved path is a single segment (e.g. BL04I-EA-E1RIO-01) are now registered as direct sub-controllers of the server controller rather than of the device controller. Their group_layout is set to GroupLayout.INLINE so PVI renders them as Grid panels on the top-level server screen. PVI forbids nesting a Group(Grid()) inside another Group, so couplers must be at the server level (not inside the device SubScreen) to appear inline. Their FastCS PV paths are unchanged — path resolution is independent of the FastCS parent-child hierarchy. Multi-segment-path couplers (e.g. BL04I-EA-CATIO-01:ETH01:E1RIO01) are unaffected and remain as SubScreen children of the device.
- Pin fastcs[epics]==0.15.0b4 (was >=0.15.0b3) and pvi==0.14.0b1 (was unpinned at 0.12.0); update uv.lock accordingly. - Remove 'title: CATio Demo' from fastcs.yaml; update schema.json so EpicsGUIOptions.title defaults to null rather than a required string, matching the upstream fastcs change. - Fix docstring and unit-test examples: BL04I-EA-ERIO-01 → BL04I-EA-E1RIO-01 (correct real-world device name). - Update fastcs-epics-ioc.md to reference CATioNameMappings templates instead of the removed ecat_name / get_type_name() implementation details.
- Remove verbose per-channel AI Settings / Internal data / Vendor data CoE object groups from EP4374 in terminal_types.yaml to reduce GUI screen clutter. - Independently refresh fastcs-epics-ioc.md: correct class names (CATioDeviceController → EtherCATMasterController), add coupler-hoisting explanation, expand device/terminal attribute tables, and document the three IO handler classes and CATioServerControllerOptions sub-dataclasses.
There was a problem hiding this comment.
Pull request overview
This PR adopts a YAML-driven naming scheme for EtherCAT controllers and reworks PVI screen layout so that single-segment coupler/box controllers are hoisted to the server root and rendered inline (working around a PVI Group-nesting restriction). It pins new beta versions of fastcs and pvi, refreshes the EPICS IOC explanation doc, allows a null EpicsGUIOptions.title, and trims verbose EP4374 CoE objects to reduce screen clutter.
Changes:
- Introduce
CATioNameMappingsdataclass withdevice_prefix/node_prefix/module_prefixtemplates, validated at construction; replace hardcodedget_type_name()methods with template-driven_resolve_controller_name_and_path. - Hoist single-segment coupler/box controllers to the server with
GroupLayout.INLINE; addBoxIONodeType and recognizeE[PQR]P?\d{4}boxes in chain traversal. - Pin
fastcs[epics]==0.15.0b4,pvi==0.14.0b1; expand and correct thefastcs-epics-ioc.mdexplanation document.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml / uv.lock | Pin fastcs/pvi to specific beta versions. |
| src/fastcs_catio/catio_controller.py | New name-template machinery, hoisting logic, path injection into subcontrollers. |
| src/fastcs_catio/catio_hardware.py | Rename SUPPORTED_CONTROLLERS → SUPPORTED_DEVICE_CONTROLLERS, drop terminal entries from the device map. |
| src/fastcs_catio/client.py | Recognize EtherCAT box terminals and assign them node positions. |
| src/fastcs_catio/devices.py | Add Box node type, remove get_type_name() methods, convert ChainLocation to NamedTuple. |
| src/fastcs_catio/utils.py | Add make_node_prefix helper (currently unused) and beamline regex. |
| src/fastcs_catio/main.py | Pass path to server controller constructor. |
| src/fastcs_catio/fastcs.yaml | Switch to absolute id, fully-qualified controller type, add name_mappings. |
| src/fastcs_catio/schema.json | Schema for new CATioNameMappings, nullable title, transports tightening. |
| src/catio_terminals/terminals/terminal_types.yaml | Drop bulky EP4374 per-channel AI/AO CoE objects. |
| tests/test_catio_units.py | New TestControllerNameMappings suite; update hyphen-trim fixtures and remove tests for deleted methods. |
| docs/explanations/fastcs-epics-ioc.md | Documentation refresh aligned with new naming + hoisting behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+320
to
+350
| def make_node_prefix(parent_path: list[str], substitution: str) -> list[str]: | ||
| """ | ||
| Create a node prefix for the CATio controller based on the parent path. | ||
| If the server prefix matches the beamline pattern, the substitution string is used \ | ||
| to create a new prefix based on that pattern (e.g. "BL04I-EA-E1RIO-01"). | ||
| Otherwise, the substitution is simply appended to the parent path \ | ||
| (e.g. "BL04I-EA-CATIO-01:ETH1:E1RIO1"). | ||
|
|
||
| :param parent_path: the parent path provided by the user | ||
| :param substitution: the substitution string to use if the server prefix matches \ | ||
| the beamline pattern | ||
|
|
||
| :returns: a list of strings representing the node path for the controller | ||
| """ | ||
| server_prefix = parent_path[0] | ||
| if _BEAMLINE_NAME_RE.match(server_prefix): | ||
| formatted_sub = "-".join( | ||
| p.zfill(2) if p.isdigit() else p | ||
| for p in [ | ||
| "".join(x) for _, x in itertools.groupby(substitution, key=str.isdigit) | ||
| ] | ||
| ) | ||
| return [ | ||
| re.sub( | ||
| _BEAMLINE_NAME_RE, | ||
| r"\1-\2-" + formatted_sub, | ||
| server_prefix, | ||
| ) | ||
| ] | ||
|
|
||
| return parent_path + [substitution] |
Comment on lines
+1180
to
+1184
| Log the list of I/O ETherCAT devices currently supported by the CATio driver. | ||
| """ | ||
| logger.info( | ||
| "List of I/O hardware currently supported by the CATio driver:\n " | ||
| + f"{list(SUPPORTED_CONTROLLERS.keys())}" | ||
| "List of I/O ETherCAT devices currently supported by the CATio driver:\n " | ||
| + f"{list(SUPPORTED_DEVICE_CONTROLLERS.keys())}" |
Comment on lines
1178
to
1185
| def get_supported_devices(self) -> None: | ||
| """ | ||
| Log the list of I/O hardware currently supported by the CATio driver. | ||
| Log the list of I/O ETherCAT devices currently supported by the CATio driver. | ||
| """ | ||
| logger.info( | ||
| "List of I/O hardware currently supported by the CATio driver:\n " | ||
| + f"{list(SUPPORTED_CONTROLLERS.keys())}" | ||
| "List of I/O ETherCAT devices currently supported by the CATio driver:\n " | ||
| + f"{list(SUPPORTED_DEVICE_CONTROLLERS.keys())}" | ||
| ) |
|
|
||
| logger.verbose( | ||
| f"{len(subcontrollers)} subcontrollers were found for {node.data.name}." | ||
| f"{len(subcontrollers)} subcontrollers were found for " |
| self, | ||
| options: CATioServerControllerOptions, | ||
| *, | ||
| path=None, |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #63 +/- ##
==========================================
+ Coverage 75.26% 75.91% +0.64%
==========================================
Files 19 19
Lines 4096 4197 +101
==========================================
+ Hits 3083 3186 +103
+ Misses 1013 1011 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The previous defaults (ETH{}, E1RIO{}, MOD{}) rendered to bare single-segment
names, causing devices, couplers and modules to appear at the top level of the
PV tree instead of being nested under their parents.
New defaults:
device_prefix "{id}:ETH{:02d}" → [<root>, ETH01]
node_prefix "{device_prefix}:E1RIO{:02d}" → [<root>, ETH01, E1RIO01]
module_prefix "{node_prefix}:MOD{:02d}" → [<root>, ETH01, E1RIO01, MOD03]
Tests updated to assert the correct nested paths and confirm that the old
single-segment behaviour no longer applies by default.
Expose device_prefix, node_prefix and module_prefix as --device-prefix, --node-prefix and --module-prefix options on the 'ioc' command, mirroring the name_mappings block already available in the YAML-driven 'run' command. Also adds docs/how-to/run-ioc.md documenting both launch modes and the PV name template system.
Underscores in EPICS PV name components produce invalid PV names.
Two validation points added:
- CATioNameMappings.__post_init__: rejects underscores in template
literal text at construction time (before any hardware is touched).
- CATioServerController._render: rejects underscores in the fully
rendered result, catching cases where a substituted value such as
{id} contributes an underscore at runtime.
Tests added for both cases. docs/how-to/run-ioc.md updated with a
warning explaining the restriction and pointing to hyphens as the
correct separator.
- Add '# pragma: no cover' to two unreachable defensive guards in CATioServerController (_resolve_controller_name_and_path else-branch and the empty-path fallback) and to CLI-only runtime branches in __main__.py (terminal_defs block and screens_dir fallback). - Add TestGetSupportedDevices: verifies get_supported_devices() is callable with no arguments (fixes the stray 'self' parameter removed in the same session). - Add TestEtherCATChainCategorisation: unit tests for _get_ethercat_chains covering the Box-type branch (EP/EQ/ER terminals), the Coupler branch (EK1100), and plain slaves — covering the previously uncovered client.py lines 1446-1449. - Remove dead make_node_prefix() from utils.py together with its exclusive dependencies (_BEAMLINE_NAME_RE, import itertools). - Fix CATioServerController.__init__ path parameter: remove type annotation from the keyword-only 'path=' argument so FastCS's _launch() introspection correctly identifies CATioServerControllerOptions as the options type instead of raising a LaunchError on startup.
In some CI environments (e.g. GitHub Actions with FORCE_COLOR set), Rich/Typer emits colour codes that split option names such as '--device-prefix' into separate escape sequences, causing plain-string membership tests to fail.
- Remove GroupLayout.INLINE for hoisted root-level controllers; pvi now renders them as top-level screens rather than inline Grid groups - Add GUI title in fastcs.yaml
- cache.py: use client.stream() context manager so the SSL socket is always closed even when raise_for_status() throws, eliminating the ResourceWarning that caused test_get_el1004_controller to fail - test_beckhoff_client.py: skip (not fail) the download integration test when the Beckhoff server is unreachable - conftest.py: skip dependent tests via the beckhoff_xml_cache fixture when fetch_and_parse_xml() returns nothing, replacing the opaque AssertionError with a clean pytest.skip
…er level Two related screen-layout changes to produce a combined server+device screen: 1. GroupLayout.INLINE on device controllers EtherCAT device sub-controllers are now given GroupLayout.INLINE when registered, so their attributes are rendered as an inline labeled Grid box on the server screen file rather than on a separate SubScreen file requiring a navigation button. 2. Hoist device-direct slave terminals to the server Slave terminal (module) sub-controllers that are direct children of a device node (i.e. attached directly on the EBus, not beneath a coupler/box) are now hoisted to the server alongside the existing coupler/box hoisting logic. This causes PVI to render them as SubScreen navigation buttons at the server-screen level, outside the device's inline Grid box. Slaves reached through an intermediate coupler/box are unaffected: they are registered with the coupler controller before the device-level hoisting check runs, so they continue to appear as navigation buttons within the coupler's own screen.
Each TerminalType now carries a short group_alias derived from its description (optional NNV supply-voltage prefix) and group_type (hard- coded short-name dict, e.g. DigOut->DO, all Fieldbus Box variants->BOX). Computed via a model_validator so it stays in sync on both XML parse and YAML load. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the leading-char exclusion in the voltage regex so bipolar/unipolar analog range forms like "+/-10V" and "0-10V" are now extracted alongside plain "24V DC" supplies. Word boundaries still keep "125V" out. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The module_prefix template now accepts a {group_alias} placeholder; when
present, CATioServerController numbers each module slave 1..N per
(coupler-node, group_alias) pair instead of using the chain-wide
position. Slaves whose terminal type has no alias fall back to "MOD" +
chain position, preserving the old behaviour. The deployment fastcs.yaml
switches to "{node_prefix}:{group_alias}{:02d}".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CoE-subindex fastcs_name generator uses a static max_length=40 and the YAML loader can't know the runtime PV prefix, so names like disable_wire_break_detection_idx8000 sit under the abbreviation threshold but produce PVs longer than EPICS's 60-char limit when the controller path eats most of the budget. fastcs then drops the PV silently with a warning. Shorten attribute names at attribute-add time, where the controller's path (and therefore the available budget) is known: a new max_attribute_name_length helper computes the snake-case budget from pv_prefix_from_path + the EPICS limit (minus 4 for _RBV on AttrRW), and a new shorten_fastcs_name helper applies the existing abbreviation pass (preserving any trailing _idx<hex> suffix) only when the name exceeds the budget. CoE and symbol attribute paths both call it; the existing CoE collision detection runs on the post-shortening name. Add abbreviations for the words that show up in the failing EL3162 / EL3314 / ELM3704 CoE subindices (calibration, detection, offset, compensation, resistance, ...) so shortening produces readable names rather than truncating at word boundaries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Inline GUI layout for root-path couplers/boxes
Couplers and EtherCAT boxes whose resolved PV path is a single segment (e.g. BL04I-EA-E1RIO-01) are now registered as direct sub-controllers of the server controller rather than nested inside the device controller. Their group_layout is set to GroupLayout.INLINE, so PVI renders them as Grid panels directly on the top-level server screen instead of behind a SubScreen click.
This works around a PVI constraint that forbids nesting a Group(Grid()) inside another Group. Multi-segment-path couplers (e.g. BL04I-EA-CATIO-01:ETH01:E1RIO01) are unaffected and remain as SubScreen children of the device controller. FastCS PV paths are unchanged — path resolution is independent of the FastCS parent–child hierarchy.
Configurable PV name templates (name_mappings)
A new CATioNameMappings dataclass lets operators control how EtherCAT device, node (coupler/box), and module PV path segments are generated from fastcs.yaml, without touching any Python code. Templates use Python str.format placeholders — {} / {n} for numeric index, {id} for the IOC root prefix, {device_prefix} and {node_prefix} for parent path segments. Using {id} or {node_prefix} causes the rendered result to be treated as an absolute PV path (split on :), enabling standalone coupler roots like BL04I-EA-E1RIO-01 alongside relative names like ETH01. Unknown placeholder keys are validated at construction time, before any hardware connection is attempted.
Also initially included:
Pin fastcs[epics]==0.15.0b4 and pvi==0.14.0b1; update
uv.lock.Allow EpicsGUIOptions.title to be null (removes the requirement for a title field in
fastcs.yaml).Remove verbose per-channel AI Settings / Internal / Vendor data CoE object groups from EP4374 in
terminal_types.yamlto reduce GUI screen clutter.Refresh f
astcs-epics-ioc.md: correct class names (CATioDeviceController → EtherCATMasterController), add coupler-hoisting explanation, expand device/terminal attribute tables, and document the three IO handler classes and CATioServerControllerOptions sub-dataclasses.Follow-up additional features:
Bump pvi to 0.14.0b3 and fastcs to 0.15.0b5 — removes the now-redundant GroupLayout.INLINE usage for hoisted root-level controllers; pvi 0.14 renders them as top-level screens automatically rather than inline Grid groups. Also adds a GUI title in fastcs.yaml.
Fix CI failures when Beckhoff XML download returns 403 — Beckhoff's download server intermittently rejects CI runners. Network-dependent tests now skip cleanly instead of failing: the download integration test in test_beckhoff_client.py calls pytest.skip() on failure, the beckhoff_xml_cache session fixture does the same when no data is available, and cache.py is fixed to always close the SSL connection (even on HTTP errors) to prevent a ResourceWarning that was causing an unrelated test to fail.