support multiple firmware revisions per terminal in YAML

.claude/skills/beckhoff-xml/SKILL.md

@@ -285,6 +285,40 @@ Beckhoff bumps the `revision_number` on backward-compatible firmware
 `(vendor, product)` if the rig's revision differs from the cached
 XML's. Don't pin matching to all three components without a fallback.
 
+### Multi-revision YAML entries (issue #60)
+
+Some terminals do rename PDOs across revisions (e.g. EP2338-0002
+renames the RxPDOs `Channel 1..8` → `Channel 9..16` at revision
+`0x00120002`). A single YAML entry per product would silently
+misname ADS symbols on rigs running the newer firmware. The fix is
+to ship multiple entries for the same `(vendor, product)`, each
+pinned to a different `revision_number`.
+
+Keying convention:
+- The lowest-revision entry keeps the bare product ID (e.g.
+  `EP2338-0002`) — existing single-rig deployments don't need
+  renaming.
+- Additional entries use the suffix `__rev<8 hex digits>` (lowercase
+  `rev`, no `@` — that character would flow into GUI DOM IDs in
+  `ui_components/tree_data_builder.py`). Example:
+  `EP2338-0002__rev00120002`.
+
+Lookup precedence in `get_terminal_type_by_identity`:
+1. Exact `(vendor, product, revision)` match.
+2. Among `(vendor, product)` candidates with
+   `revision_number ≤ rig revision`, pick the highest (the most
+   recent layout the rig is compatible with).
+3. Otherwise pick the lowest-revision candidate as a degraded
+   fallback (the rig is older than every cached entry) and log a
+   warning.
+4. Otherwise return `None`.
+
+When `clean-yaml` re-merges from XML, `service_file.py` strips the
+`__rev...` suffix to find the ESI XML device, and passes the YAML
+entry's pinned `revision_number` as `target_revision` so the parser
+picks the right `<Device>` element. The PDO renames flow into
+`channel_indices` on the affected `symbol_nodes`.
+
 ## Subscription gating
 
 `CATioConnection.set_wanted_attribute_keys(...)` (called from the

src/catio_terminals/models.py

@@ -1,5 +1,6 @@
 """Data models for terminal description YAML files."""
 
+import re
 from pathlib import Path
 
 from pydantic import BaseModel, Field, computed_field, model_validator
@@ -36,6 +37,28 @@ def _coe_primitive_bit_size(type_name: str | None) -> int | None:
     return _COE_PRIMITIVE_BIT_SIZES.get(type_name.upper())
 
 
+# Issue #60: multi-revision YAML entries are keyed
+# "<bare-id>__rev<8 hex digits>", lowercase ``rev``, no ``@`` (that
+# character would flow into the GUI's DOM IDs in
+# ui_components/tree_data_builder.py). The bare-id entry (no suffix)
+# is the lowest cached revision for the product.
+_REVISION_SUFFIX_RE = re.compile(r"__rev[0-9a-fA-F]{8}$")
+
+
+def strip_revision_suffix(terminal_id: str) -> str:
+    """Strip the ``__revXXXXXXXX`` suffix from a multi-revision YAML key.
+
+    Returns the bare terminal ID used to look up the ESI XML device.
+    Keys without the suffix pass through unchanged.
+
+    >>> strip_revision_suffix("EP2338-0002")
+    'EP2338-0002'
+    >>> strip_revision_suffix("EP2338-0002__rev00120002")
+    'EP2338-0002'
+    """
+    return _REVISION_SUFFIX_RE.sub("", terminal_id)
+
+
 def _coe_default_data_zero(bit_size: int | None) -> str | None:
     """Zero-valued ``default_data`` string for a given bit size, or None."""
     if not bit_size:

src/catio_terminals/service_file.py

@@ -6,7 +6,7 @@
 from typing import TYPE_CHECKING
 
 from catio_terminals.beckhoff import BeckhoffClient
-from catio_terminals.models import TerminalConfig
+from catio_terminals.models import TerminalConfig, strip_revision_suffix
 
 if TYPE_CHECKING:
     from catio_terminals.models import TerminalType
@@ -99,8 +99,13 @@ async def merge_xml_for_terminal(
         """
         logger.debug(f"Loading XML for terminal: {terminal_id}")
 
+        # Issue #60: multi-revision entries are keyed
+        # "<bare-id>__rev<8 hex>" but the ESI XML only knows the bare ID.
+        # Strip the suffix before fetching/parsing.
+        xml_terminal_id = strip_revision_suffix(terminal_id)
+
         # Fetch XML for this terminal
-        xml_content = await beckhoff_client.fetch_terminal_xml(terminal_id)
+        xml_content = await beckhoff_client.fetch_terminal_xml(xml_terminal_id)
         if not xml_content:
             logger.warning(f"No XML found for {terminal_id}")
             return False
@@ -110,7 +115,7 @@ async def merge_xml_for_terminal(
             # Pass target_revision to match the terminal's existing revision
             target_revision = terminal.identity.revision_number
             xml_terminal, composite_types = beckhoff_client.parse_terminal_xml(
-                xml_content, terminal_id, terminal.group_type, target_revision
+                xml_content, xml_terminal_id, terminal.group_type, target_revision
             )
 
             # Merge composite types into the config

src/catio_terminals/terminals/terminal_types.yaml

@@ -7474,6 +7474,40 @@ terminal_types:
         bit_offset: 0
     coe_objects: []
     group_type: DigOut
+  EP2338-0002__rev00120002:
+    description: 8 Ch. Dig. Input/Output 24V, 0,5A, M12
+    identity:
+      vendor_id: 2
+      product_code: 153239634
+      revision_number: 1179650
+    symbol_nodes:
+      - name_template: Channel {channel}
+        index_group: 61489
+        type_name: InputBits
+        channels: 8
+        access: Read-only
+        fastcs_name: channel_{channel}
+        selected: true
+        bit_offset: 0
+      - name_template: Channel {channel}
+        index_group: 61473
+        type_name: InputBits
+        channels: 8
+        channel_indices:
+          - 9
+          - 10
+          - 11
+          - 12
+          - 13
+          - 14
+          - 15
+          - 16
+        access: Read/Write
+        fastcs_name: channel_{channel}
+        selected: true
+        bit_offset: 0
+    coe_objects: []
+    group_type: DigOut
   EP2624-0002:
     description: 4Ch. Relay Output, NO (125V AC / 30V DC)
     identity:

src/fastcs_catio/terminal_config.py

@@ -199,20 +199,53 @@ def get_terminal_type_by_identity(
 ) -> TerminalType | None:
     """Look up a terminal definition by its EtherCAT identity.
 
-    Used by the bus-side symbol expansion. Beckhoff bumps the revision
-    number on backward-compatible firmware/silicon updates, so the
-    physical layout (the only thing #54 needs) stays the same. We
-    therefore prefer an exact match but fall back to vendor+product
-    when the rig is at a newer revision than the cached XML.
+    Used by the bus-side symbol expansion. Beckhoff usually bumps the
+    revision number on backward-compatible firmware/silicon updates, so
+    one cached YAML entry per product is enough. Occasionally though
+    (e.g. EP2338-0002 at rev 0x00120002) Beckhoff renames PDOs across
+    a revision, which fabricates wrong ADS symbol names if the rig is
+    matched against the wrong YAML entry. Issue #60 lets a product ship
+    multiple YAML entries pinned to different revisions; this lookup
+    picks the right one.
+
+    Resolution order:
+    1. Exact ``(vendor, product, revision)`` match.
+    2. Among ``(vendor, product)`` candidates with ``revision <= rig``,
+       the one with the highest revision (most recent layout the rig is
+       compatible with).
+    3. Any ``(vendor, product)`` candidate (degraded fallback for rigs
+       older than every cached entry) — logged as a warning.
+    4. ``None``.
     """
     config = load_terminal_config()
-    fallback: TerminalType | None = None
-    for terminal in config.terminal_types.values():
-        ident = terminal.identity
-        if ident.vendor_id != vendor_id or ident.product_code != product_code:
-            continue
-        if ident.revision_number == revision_number:
+    candidates: list[TerminalType] = [
+        t
+        for t in config.terminal_types.values()
+        if t.identity.vendor_id == vendor_id and t.identity.product_code == product_code
+    ]
+    if not candidates:
+        return None
+
+    # 1. Exact match wins.
+    for terminal in candidates:
+        if terminal.identity.revision_number == revision_number:
             return terminal
-        if fallback is None:
-            fallback = terminal
+
+    # 2. Highest revision <= rig (most recent compatible layout).
+    le_candidates = [
+        t for t in candidates if t.identity.revision_number <= revision_number
+    ]
+    if le_candidates:
+        return max(le_candidates, key=lambda t: t.identity.revision_number)
+
+    # 3. Degraded fallback: rig is older than every cached entry. Pick
+    # the lowest-revision candidate (closest to the rig) and warn.
+    fallback = min(candidates, key=lambda t: t.identity.revision_number)
+    logger.warning(
+        f"No cached YAML entry has revision <= rig revision 0x{revision_number:08x} "
+        f"for vendor 0x{vendor_id:x} product 0x{product_code:x}; "
+        f"falling back to YAML entry pinned at "
+        f"0x{fallback.identity.revision_number:08x}. "
+        "Symbol layout may not match the rig."
+    )
     return fallback

tests/test_symbol_expansion.py

@@ -513,6 +513,9 @@ def test_exact_match(self, install_terminal_config):
         assert get_terminal_type_by_identity(2, 10, 5) is t
 
     def test_vendor_product_fallback_on_revision_drift(self, install_terminal_config):
+        # Single-entry products still loose-match: rig at a newer revision
+        # than the cached YAML falls through to the only candidate (since
+        # its revision is <= the rig's). This preserves pre-#60 behaviour.
         t = TerminalType(
             description="EL",
             identity=Identity(vendor_id=2, product_code=10, revision_number=5),
@@ -528,3 +531,64 @@ def test_no_match_returns_none(self, install_terminal_config):
         install_terminal_config({"EL": t})
         assert get_terminal_type_by_identity(2, 11, 5) is None
         assert get_terminal_type_by_identity(3, 10, 5) is None
+
+    def test_multi_revision_exact_match_wins(self, install_terminal_config):
+        a = TerminalType(
+            description="EL rev A",
+            identity=Identity(vendor_id=2, product_code=10, revision_number=5),
+        )
+        b = TerminalType(
+            description="EL rev B",
+            identity=Identity(vendor_id=2, product_code=10, revision_number=20),
+        )
+        install_terminal_config({"EL": a, "EL__rev00000014": b})
+        # Exact match on A
+        assert get_terminal_type_by_identity(2, 10, 5) is a
+        # Exact match on B
+        assert get_terminal_type_by_identity(2, 10, 20) is b
+
+    def test_multi_revision_picks_highest_below_rig(self, install_terminal_config):
+        a = TerminalType(
+            description="EL rev A",
+            identity=Identity(vendor_id=2, product_code=10, revision_number=5),
+        )
+        b = TerminalType(
+            description="EL rev B",
+            identity=Identity(vendor_id=2, product_code=10, revision_number=20),
+        )
+        install_terminal_config({"EL": a, "EL__rev00000014": b})
+        # Rig at rev 15 (> A, < B) picks A (the highest <= 15).
+        assert get_terminal_type_by_identity(2, 10, 15) is a
+        # Rig at rev 99 (> B) picks B (the highest cached compatible rev).
+        assert get_terminal_type_by_identity(2, 10, 99) is b
+
+    def test_multi_revision_rig_below_all_falls_back_with_warning(
+        self, install_terminal_config
+    ):
+        from fastcs.logging import logger as _fastcs_logger
+
+        a = TerminalType(
+            description="EL rev A",
+            identity=Identity(vendor_id=2, product_code=10, revision_number=5),
+        )
+        b = TerminalType(
+            description="EL rev B",
+            identity=Identity(vendor_id=2, product_code=10, revision_number=20),
+        )
+        install_terminal_config({"EL": a, "EL__rev00000014": b})
+
+        # terminal_config uses fastcs's loguru logger, which doesn't feed
+        # pytest's caplog. Add a sink we can drain.
+        messages: list[str] = []
+        sink_id = _fastcs_logger.add(
+            lambda msg: messages.append(str(msg)), level="WARNING"
+        )
+        try:
+            # Rig at rev 1 is older than every cached entry. Degraded
+            # fallback: pick the lowest-revision YAML (closest) and warn.
+            result = get_terminal_type_by_identity(2, 10, 1)
+        finally:
+            _fastcs_logger.remove(sink_id)
+        assert result is a
+        joined = " ".join(messages).lower()
+        assert "falling back" in joined