Skip to content

feat(canopen): add read-only peer safety inspection - #458

Merged
dborgards merged 2 commits into
mainfrom
feat/peer-safety-inspection-403
Oct 26, 2026
Merged

dborgards merged 2 commits into
mainfrom
feat/peer-safety-inspection-403

Conversation

@dborgards

@dborgards dborgards commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

What does this change?

Tools can now acquire a peer's safety communication configuration without supplying an expected DCF. ReadPeerSafetyConfigurationAsync returns an immutable PeerSafetySnapshot with raw values/provenance, peer identity entries, acquisition identity/time, interpreted SRDOs and per-entry findings. The device's 13FEh flag, independently recomputed signature comparison and optional expected-configuration comparison are separate outcomes.

The optional ICanOpenSafetyInspection and extension preserve existing ICanOpenSafety implementations. The operation reserves this client's peer-SDO channel, snapshots its bound description/expectation and performs gated uploads only. The peer count cannot expand the acquisition beyond the bound file/64 SRDOs; advertised mapping slots remain bounded and pass the existing gate. No download, NMT, store/commit, SRDO or GFC command is sent. Normal SDO cancellation aborts remain possible.

Every acquired entry is read again. Different live/explicit SRVT-rule evidence reports ChangeObserved; identical answers still leave atomic consistency Unconfirmed. Missing/malformed required evidence prevents eligibility. File/default fallback stays distinguishable and never becomes live checksum evidence. Existing CRC serialization, expected-value generation, mapped-object validation and CiA 319 SRVT handling are reused. Deleted records' stale checksums are not verified. The known interoperability questions in #289 remain visible; these diagnostics are not safety approval.

IsComplete concerns the required SRDO image/rechecks; optional identity/GFC unavailability remains visible in entry findings. Expected Match requires a complete acquisition without observed changes; a known difference can still report Mismatch in a partial image. Each acquisition gets a new identity, and replacing monitor input requires a new evaluation period. Continuous monitoring remains separate in #401.

Closes #403. Refs #401 and #289.

Type of change

  • feat — new behaviour (minor release)
  • fix / perf — bug or performance fix (patch release)
  • docs / test / refactor / chore / ci — no release
  • Breaking change

Checklist

  • Build with -p:CI=true, test (net10.0), dotnet format --verify-no-changes, and pack + eng/verify-packages.py all pass locally
  • Public API changes are documented with XML comments
  • New behaviour is covered by a test
  • The requirement or ADR this relates to is referenced — FR-CO-060 in both SRS editions

Validation

  • Final Release CI build: zero warnings/errors. Full net10.0 suite: 2971/2971 passed. Format verification passed. Fresh pack verified all 9 packages. Additional net48 CI build passed; runtime execution is confirmed below.
  • 77 new deterministic cases cover complete/partial/changing images, raw widths, mapping pairs/limits/object access, signature and expectation outcomes, provenance, count bounds, explicit SRVT rule, channel reservation, cancellation/local timeout, immutable results and upload-only traffic. Virtual time controls SDO timeout; real-time bounds are hang guards only.
  • Targeted source mutations produced test failures: mislabel fallback as live; ignore changes; remove shared channel reservation; disable mapped-object validation; expose mutable result bytes; inject forbidden NMT traffic; and remove the inactive-record nullable guard. Sources restored. Added regression cases also failed before fixing malformed rechecks, changed live/default evidence and expected Match on partial/changing images, and false communication errors for unused records with COB-IDs 0.
  • Final full-suite Cobertura: 271 changed executable class-line entries covered, no missing or partial changed entry. Inspected every class/state-machine entry rather than deduplicating by filename. The final-head Linux CI artifact confirms this same count without missing/partial entries; Codecov's patch check also passes.
  • Public API approval copied from the actual generated received file after inspecting an additive-only diff; no existing interface member changed. All 50 verification-script tests pass; requirement traceability has no new gaps.

Final CI and reviews

Final head: 05f6241a. Linux, macOS and Windows pass net10.0 using the suite count above; Windows also passes 2958 net48 tests. Format, CodeQL, documentation, configuration/requirements checks and the package/SBOM gate all pass.

Both code reviews completed on this head without unresolved findings. The deleted-record and CodeQL Where findings were fixed, answered in their original threads and confirmed resolved by the reviewing bots; CodeQL alert 749 is fixed. No review threads remain open. Existing norm/interoperability questions in #289 remain outside this PR.

@cursor

cursor Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
New upload-only API on the safety path with strict gating and no peer writes, but misinterpreted snapshots could mislead monitoring tools if callers ignore completeness, consistency, and findings.

Overview
Adds read-only peer safety inspection (FR-CO-060 / #403): node.Safety().ReadPeerSafetyConfigurationAsync uploads a bounded safety image via SDO only—no downloads, NMT, or store—and returns an immutable PeerSafetySnapshot with raw entries, provenance, interpreted SRDOs, and structured findings.

The feature sits on optional ICanOpenSafetyInspection plus an extension so existing ICanOpenSafety implementers stay unchanged. Acquisition requires a bound peer EDS/DCF, holds the peer SDO channel, optionally compares against an expected PeerSafetyConfiguration, repeats every read to detect ChangeObserved, and keeps 13FEh, recomputed CRC signatures, and expected comparison as separate outcomes. File/default fallbacks and partial data cannot masquerade as a complete verified image; EligibleForMonitoring is explicit.

SRS (EN/DE), CANopen README, public API approvals, and broad deterministic tests document and lock the behavior.

Reviewed by Cursor Bugbot for commit 05f6241. Bugbot is set up for automated code reviews on this repo. Configure here.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-25T21:58:18.225793Z 05f6241 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@dborgards dborgards added area: canopen CanKit.Pro.CANopen — SDO, PDO, NMT, object dictionary type: feature New behaviour labels Oct 25, 2026 — with ChatGPT Codex Connector
Comment thread src/CanKit.Pro.CANopen/Safety/PeerSafetyDiagnostics.cs Fixed

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 89d82a1. Configure here.

Comment thread src/CanKit.Pro.CANopen/Safety/PeerSafetyDiagnostics.cs
@codecov

codecov Bot commented Oct 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@dborgards
dborgards merged commit aa41820 into main Oct 26, 2026
16 checks passed
@dborgards
dborgards deleted the feat/peer-safety-inspection-403 branch October 26, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: canopen CanKit.Pro.CANopen — SDO, PDO, NMT, object dictionary type: feature New behaviour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CANopen Safety: add read-only configuration acquisition and structured diagnostics for tools

2 participants