Conversation
Replace AGENTS.md + SKILLS.md + CLAUDE_NOTES.md with a much smaller CLAUDE.md (project-specific rules only) and three proper Claude skills under .claude/skills/ (beckhoff-xml, ads-simulator-testing, mermaid-diagrams). Skills surface automatically via frontmatter descriptions; activation prompts no longer needed.
- client.py: render I/O device dict as a list so loguru's formatter
doesn't crash on the outer '{...}' braces
- catio_dynamic_coe.py: demote deliberate compound-type CoE skips from
WARNING to DEBUG and drop the unreachable second guard
- catio_controller.py: suppress the attribute-map warning for
notification paths containing '._unreferenced_.' (e.g. WcState)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors gilesknap/claude-sandbox#27 (Shape B) onto this repo's embedded snapshot so `uv run`/`ruff`/`pytest` resolve on PATH inside the claude sandbox, matching how they resolve in the regular devcontainer shell. Without this, `uv` falls back to creating a parallel `.venv/` in CWD instead of using the /cache-backed venv that the devcontainer's remoteEnv points to. - claude-shadow: existence-guard `$VIRTUAL_ENV/bin` and append it to the sandbox PATH (still after `$HOME/.local/bin` so the `/usr/local/bin/claude` shadow keeps winning resolution). - claude-shadow: pass VIRTUAL_ENV, UV_PROJECT_ENVIRONMENT, UV_CACHE_DIR, UV_PYTHON_CACHE_DIR, PRE_COMMIT_HOME through --clearenv (only when set). This is a temporary local mirror of an unmerged upstream patch (issue #27); on next re-promote from claude-sandbox after that lands, the snapshot will pick up the canonical version. Verified: all 88 cases pass in a patched copy of claude-sandbox's tests/bwrap_argv.sh (77 existing + 11 new for the venv passthrough). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapse the two parallel pipelines that decided what symbols existed: delete the hardcoded AdsSymbolTypePattern LUT in symbols.py and drive bus-side expansion from the terminal YAML. The XML parser now records each entry's bit_offset within its parent struct on the SymbolNode it emits, so the runtime can compute every sub-symbol's ADS offset without a per-struct case-arm. selected: false rows are no longer subscribed. Also drops the _unreferenced_ carve-out (closes #53) since the seam it papered over no longer exists, and updates the beckhoff-xml skill to remove the "two things" guidance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
==========================================
- Coverage 73.92% 73.82% -0.11%
==========================================
Files 19 19
Lines 4130 4095 -35
==========================================
- Hits 3053 3023 -30
+ Misses 1077 1072 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Beckhoff bumps revision numbers on backward-compatible firmware/silicon updates while keeping the PDO layout identical. The rig at 172.23.242.39 hit identity-not-found warnings because its EL3314 is at rev 0x180000 while the cached XML produced a YAML pinned to 0x100000. Match on (vendor_id, product_code) when no exact revision match exists — same physical layout, same bit_offsets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EA.md and index.md are generated by the IOC at runtime as FastCS attribute dumps in the working directory. They snuck in via `git add -A` in the previous commit. Add them to .gitignore so this doesn't recur. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ADS addresses are byte-granular. YAML rows with `bit_offset % 8 != 0` (e.g. EL3104's `.Status__Limit 1` at bit 2, EL3314's `.Limit 1` at bit 2) represent bit fields within a parent status byte and can't be subscribed as standalone notifications — the ADS server rejects them with ADSERR_DEVICE_SYMBOLVERSIONINVALID, crashing IOC startup. The data is still available via the parent row's subscription; reading individual bit fields is a FastCS-attribute-layer concern, not a bus-side one. Match the old LUT's behavior by simply not emitting AdsSymbols for non-byte-aligned rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The bare parent row of a struct terminal (e.g. EL3314's
`TC Inputs Channel {channel}` with children `.Value`, `.Limit 1`) has
type metadata sized for only the bit-collapsed status (typically 1
byte) but the bus reports the full parent struct size (4 bytes for
AI16/TC). Subscribing succeeds but the notification stream parser then
hits `assert symbol.nbytes == sample.size` (no message) in
messages.py, surfacing as "Notification flushing error:" with an empty
detail and breaking the flush loop.
Only emit AdsSymbols for "leaf" YAML rows — rows whose name_template
isn't a strict prefix of any other row's. Sub-fields cover the data
the user actually cares about; the bit-field bits that lived only in
the bare parent are not yet exposed as PVs (and weren't in the old
LUT path either for terminals like EL3314).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The bare 'assert symbol.nbytes == sample.size' in messages.py (both get_notification_dtype and get_combined_notifications_dtype) and 'assert streams_dtype.fields' in client.py fire with empty AssertionError detail, producing log lines like "Notification flushing error:" with nothing useful after the colon. Add messages that name the symbol and the conflicting sizes so the next failure is diagnosable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
_dtype_for_type_name was treating _DTYPE_MAP's element byte size as the element count, so e.g. type INT (2 bytes per int16) emitted AdsSymbol(dtype=int16, size=2) → nbytes=4, while the server reported size=2 for the actual symbol. The notification stream then failed the sample-size assertion on every flush. For scalar primitives, count is always 1; only the ARRAY [a..b] OF X branch uses an element count derived from the array bounds. Fix the scalar branch to return count=1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The IOC was subscribing to every symbol the bus discovered, including master-device housekeeping like _SyncUnits._default_._unreferenced_.* that has no PV consumer. After the _unreferenced_ carve-out was removed in c65d2dd these notifications surfaced as per-cycle warnings. Pass the server controller's full attribute_map keys into the connection right after the map is built, then skip any AdsSymbol whose f"_{name}" doesn't appear in that key set when add_notifications runs. The connection falls back to subscribing everything if the filter is never set (preserves the pre-#54 behaviour for callers that bypass CATioServerController). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A YAML row whose name_template (after channel substitution) doesn't correspond to any bus-discovered symbol or parent struct silently disappeared from _ecsymbols. The PV still got created via the IOC path, but every read/write failed with "No match for controller N and ADS Symbol ...". Log a warning at expansion time so the gap is visible at startup instead of on first PV write. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a YAML row's parent isn't on the bus and the terminal has dynamic PDO groups, the most likely cause is a mismatch between the YAML's selected_pdo_group and the rig's actual PDO configuration. EL3314 is the canonical example: YAML selects "with ColdJunction Compensation" but the rig runs in the default "Inputs only" mode, so TC Outputs Channel N never appears on the bus. Surface that hypothesis in the warning message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
Contributor
Author
|
closed in favour of #59 |
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
1 participant
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.
Summary
AdsSymbolTypePatternLUT insrc/fastcs_catio/symbols.pywith a YAML-driven expander. For each bus-discoveredAdsSymbolNodewe look up the matching terminal by(vendor_id, product_code, revision_number)and emit oneAdsSymbolperselected: truerow, atparent.offset + row.bit_offset // 8.src/catio_terminals/xml/pdo.py) now records each PDO entry's cumulativebit_offsetwithin its parent struct on theSymbolNodeit emits. The newSymbolNode.bit_offsetfield replaces what the LUT hardcoded.Inputs/Outputsframe-state structs (Frm0State, SlaveCount, etc.) keep a tiny built-in layout insymbols.py— those aren't user-extensible, so they don't belong in YAML._unreferenced_carve-out incatio_controller.py(added for Spurious "No reference to _SyncUnits._default_._unreferenced_.WcState.WcState" warnings on real hardware #53) is gone — the seam it papered over no longer exists. Closes Spurious "No reference to _SyncUnits._default_._unreferenced_.WcState.WcState" warnings on real hardware #53..claude/skills/beckhoff-xml/SKILL.mdno longer says "supporting a new terminal struct type needs two things". One thing now: regenerate the YAML.Why this is the right shape
The YAML rows already describe the full per-struct layout (one row per dotted sub-name), so the bus side only needed the missing offset metadata to drop the LUT entirely.
bit_offsetis read from the XML at YAML-generation time and stored alongsidename_template,type_name,selected, etc.selected: falserows are no longer subscribed at all — kills the wasted bus traffic and the per-cycle "No reference to ... in the CATio attribute map" warning spam.Test plan
uv run pytest— 159 passed, 23 skipped (full suite includingtests/test_system.pysimulator round-trip).uv run ruff check,uv run pyright src tests— both clean.fastcs-catio iocagainst the rig at 172.23.242.39 — confirm EL3314 produces Value PVs without any code change beyond the YAML's existingselected: truerows.🤖 Generated with Claude Code