DiamondLightSource / DiamondLightSource/fastcs-catio

PDO group selection is per terminal-class, not per slave instance

Open
#58 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
1
Forks
0
PR merge metrics
No merged PRs in 30d

Description

## Problem

`src/catio_terminals/models.py` keys terminal configuration by terminal ID:

```python
class TerminalConfig(BaseModel):
terminal_types: dict[str, TerminalType] # "EL3314" → TerminalType
```

`TerminalType.selected_pdo_group: str | None` therefore applies to *every* instance of that terminal class on the chain. If a user has two EL3314s on the same EtherCAT bus, one configured for "Inputs only" and another for "with ColdJunction Compensation", the YAML can't express that — both slaves get whichever group the YAML names.

## How this surfaced

Hit during #54 hardware verification. The rig (172.23.242.39) runs three EL3314s in "Inputs only" PDO mode. The YAML had `selected_pdo_group: "with ColdJunction Compensation"`, so the bus-side expansion looked for `TC Outputs Channel N.CJCompensation` symbols that don't exist on the bus and logged one warning per channel per slave:

```
No bus node provides offset for 'Term 4 (EL3314).TC Outputs Channel 1.CJCompensation'; ...
Terminal has dynamic PDO groups and the YAML selects 'with ColdJunction Compensation';
the bus may be configured for a different group.
```

Switching the YAML to "Inputs only" fixed it — but that's a global change. With mixed-mode EL3314s on one chain there is no correct YAML.

## Two possible directions

### Option A — Per-slave override (smaller)

Add an optional mapping in the IOC config (not `terminal_types.yaml`) from slave name/address → PDO group name. Something like:

```yaml
# server_config_.yaml
slave_pdo_groups:
Term 4 (EL3314): Inputs only
Term 5 (EL3314): with ColdJunction Compensation
```

`terminal_types.yaml` stays the per-class catalogue; the rig config provides instance overrides. Simple to add. Still leaves the YAML's `selected_pdo_group` as a default.

### Option B — Infer from the bus (#54-shaped)

For a slave whose `TerminalType.has_dynamic_pdos == True`, at startup look at which symbols the bus actually exposes and match against `pdo_groups[].symbol_indices` to pick the active group automatically. The bus is the real source of truth — `selected_pdo_group` becomes a hint/default rather than ground truth.

Concretely in `src/fastcs_catio/symbols.py::expand_symbols_for_slave`:

1. Build the set of bus-discovered symbol names under this slave.
2. For each PDO group in `terminal.pdo_groups`, check whether the symbols listed at `terminal.symbol_nodes[symbol_indices]` (after channel substitution + slave prefix) are present on the bus.
3. Pick the group with the best match; only emit symbols from that group.

This removes the YAML/rig disagreement class entirely — they can't disagree if the rig is the truth. Bigger change, and the inference logic needs to be robust to partial PDO availability (e.g. an output PDO with no notification subscriptions might not appear in the symbol table even when "active").

## Recommendation

**B is the right shape long-term** — same principle as #54 (the bus tells us what's real). **A is a faster patch** that solves the immediate mixed-mode case without committing to inference.

Maybe ship A behind a config flag now, then add B and deprecate A.

## Files in scope

- `src/catio_terminals/models.py` — where `selected_pdo_group` lives; if going with A, this stays per-class.
- `src/fastcs_catio/symbols.py::expand_symbols_for_slave` — bus-side expansion; main change site for B.
- `src/fastcs_catio/terminal_config.py::get_terminal_type_by_identity` — currently returns a `TerminalType` with the class-level `selected_pdo_group`; A would wrap this to apply per-slave overrides.
- `src/catio_terminals/ui_app.py` / `ui_dialogs/` — the GUI editor surfaces `selected_pdo_group` per class today; would gain a per-instance affordance under A.

## Related

- #54 — collapsing bus+YAML pipelines; this issue surfaced during hardware verification of that PR.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.