Skip to content

fix(canopen): preserve SRDO errors while EMCY is disabled - #461

Merged
dborgards merged 1 commit into
mainfrom
fix/srdo-disabled-emcy-434
Oct 26, 2026
Merged

dborgards merged 1 commit into
mainfrom
fix/srdo-disabled-emcy-434

Conversation

@dborgards

Copy link
Copy Markdown
Owner

What does this change?

Addresses the disabled-EMCY asymmetry in #434. When 1014h disables the EMCY producer during a run of short SRDO pairs, the consumer now leaves that error available to report on the next short pair after enabling, matching the existing documented RPDO rule. An already reported run stays latched across off/on toggles, and only a complete pair or reconfiguration ends it.

The engine reads the host's actor-owned producer state before consuming its one-report flag. No public API changes. The 1003h error-history item in #434 remains open: it is separately documented as unsupported and is not introduced by this correction.

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 (! in the title, plus a BREAKING CHANGE: footer explaining the migration)

Checklist

Validation includes net48 compilation, engineering checks and requirements traceability. Controlled-clock tests cover disabled/unreported runs, enabling, repeated toggles, valid-pair recovery and a subsequent error. A wire-level RPDO/SRDO theory uses application EMCYs as ordering witnesses to assert the exact error/reset sequence without negative sleeps. Removing the engine gate and forcing the node's host state enabled each produced a failing assertion; sources were restored before the passing suite. Coverage is inspected across all Cobertura type entries for the changed executable lines, including partial branches.

@cursor

cursor Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Targeted EMCY gating in the SRDO consumer with no public API change; behavior matches documented RPDO error-control rules.

Overview
Aligns SRDO consumer short-pair handling with the existing RPDO / disabled-EMCY rule (#434): when bit 31 of 1014h turns the EMCY producer off, a run of undersized SRDO pairs no longer latches ShortFrameReported or emits 8210h until EMCY is enabled again.

ISrdoEngineHost gains EmcyEnabled; CanOpenNode maps it to _emcyValid. SrdoEngine.Consumer only sets the one-report flag and calls EmitEmcy when EMCY is enabled, so a persistent short run can still produce a single 8210h on the next short pair after re-enable. Toggling EMCY off/on after that report does not start a new error event; recovery still needs a valid-length pair or SRDO rebuild.

README documents the shared RPDO/SRDO behavior. Tests add unit coverage on the consumer host stub and a wire-level theory for both PDO and SRDO paths.

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 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-26T07:02:53.248919Z 4204086 PR opened
ℹ️ 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.

@codecov

codecov Bot commented Oct 26, 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 4f87688 into main Oct 26, 2026
16 checks passed
@dborgards
dborgards deleted the fix/srdo-disabled-emcy-434 branch October 26, 2026 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant