Repository navigation
feat(canopen): add opt-in controller passive EMCYs - #487
Conversation
PR SummaryMedium Risk Overview Recovery is defined only for error-active, error-warning, or no-error; bus-off and unknown readings do not clear an active passive cause. NMT reset clears reported history, invalidates queued baseline work, and re-samples the controller on Pre-Operational entry without replaying a pre-reset lapse. Baseline refresh runs as a separate actor job (with retry on expected read failures) so reset/LSS bookkeeping is not interrupted. Lifecycle hooks wire monitoring at startup, after communication reset, on EMCY COB-ID apply, and on dispose/failed construction. SRS FR-CO-011, README, API approval, and Reviewed by Cursor Bugbot for commit a4cc274. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ac9cf2c. Configure here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86480a5b44
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
What does this change?
Applications can opt into controller error-passive EMCY 8120h with
CanOpenNodeOptions.EmitControllerErrorEmcy(default false). The existing BusStateMonitor observes the underlying bus on the node actor, using its 50 ms poll and error/fault hints. An initially passive controller is reported when entering Pre-Operational; the default adds no monitor or subscriptions.An observed passive lapse joins the existing communication-error aggregate: one EMCY per event, 1001h bits 4/0 while active, application bits retained, and 0000h only after the last communication cause repairs. Error-active, error-warning and no-error observations repair the cause; bus-off/unknown do not establish recovery. An initially bus-off controller does not imply an observed passive error. Producer re-enable reports only a still-active cause once. NMT reset clears report history and takes a fresh physical sample plus a replacement monitor baseline on re-entry, without replaying a silently repaired lapse or inheriting a pre-reset passive cause into bus-off/unknown. The physical baseline read runs as a separate actor job, so even an unexpected exception cannot abort flying-master reset echo bookkeeping or the LSS configure-node-id reply. An expected baseline read failure (CanKitException or ObjectDisposedException) is reported and retried after 50 ms on the actor clock, with the existing monitor retained until replacement succeeds; teardown cancels the retry. Every reset invalidates older queued baseline jobs and retries, including resets that return LSS to ID255. Jobs capture a generation and must match the current generation before reading or changing state, so an obsolete job cannot consume an untransmitted passive cause. Normal disposal and failed construction remove monitor handlers/timers.
Refs #423, not Closes. This implements 8120h only; 8130h landed in #486. Overrun 8110h clear semantics and recovered-from-bus-off 8140h sequencing remain explicit scope/evidence decisions. The new property is additive; existing With signatures remain compatible and copy the option. README and FR-CO-011 describe these boundaries.
Type of change
feat— new opt-in behaviour (minor release)Validation
42 new clocked wire/lifetime cases cover initial controller states, default-off subscriptions, once-per-lapse and three healthy recovery levels, application bits, bus-off/unknown transitions, overlapping heartbeat causes, disabled/re-enabled producer, reset, fault hints without clock advancement, monitor teardown/constructor failure and option copying.
Fourteen actual source mutations were runtime-red and restored (all prior twelve were rerun after adding generation invalidation): default-on, missing startup baseline, treating bus-off as recovery, missing recovery, omitted monitor disposal, omitted producer re-enable report and omitted option copying; restored stale reset logic; fresh bus read with stale monitor cache; omitted baseline retry; generic catch that incorrectly retries an unexpected error; inline read that interrupts protocol completion; omitted reset invalidation; obsolete generation allowed to execute. Runners:
/private/tmp/cankit-423-controller-mutations.pyand/private/tmp/cankit-423-controller-baseline-mutations.py. Ten added wire cases cover silent physical recovery/BusOff/Unknown before both reset commands without a poll/hint and detection of the next passive edge; four cover both expected exception families, retry and disposal before retry; one verifies that an unexpected error is reported without a retry. Four additional regressions cover both self-reset completions and their echo bookkeeping, the unconfigured LSS configure reply, and disposal with an already queued baseline job. Three further regressions cover LSS configure/reset already queued together, coalescing two queued active resets to one baseline read, and a pending retry invalidated by reset back to ID255; reconfiguration must positively emit the real 8120h.All local gates pass: 3104 net10 tests with coverage, net48 compilation, CI build, format, nine verified packages, 50 engineering tests and traceability. All 60 changed executable Cobertura entries across every class/nested type (including the new controller file) are covered with no missing/partials. The API approval was copied from the generated received file after reviewing its single additive property.
Checklist
-p:CI=true, test (net10.0),dotnet format --verify-no-changes, and pack +eng/verify-packages.pyall pass locally