fix(canopen): report a short classic upload, refuse a long one, stop reporting reserved heartbeat states as boot-up - #265
Conversation
…reporting reserved heartbeat states as boot-up #253: the classic upload client now aborts with 0607 0012h when the server sends more than it announced (as the server's receive path and the block-upload client do), and reports through BackgroundExceptionOccurred when it sends fewer; the shorter data is still returned. #255: a heartbeat or guarding state byte that CiA 301 reserves is no longer raised as HeartbeatReceived / NodeGuardingReceived with State = Initializing, which read as a boot-up. It still counts as a sign of life and is still recorded for the NMT master. #254: SendSyncAsync is documented as the raw send it is (not gated by NMT state or bit 30 of 1005h); the periodic producer is the gated one. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PR SummaryMedium Risk Overview Classic segmented SDO upload (#253) — The client now remembers the server’s indicated size from the upload-init response. Receiving more bytes than announced aborts with Heartbeat and node guarding (#255) — Shared SYNC API clarity (#254) — Documentation only: Tests cover upload length enforcement, shortfall reporting, and reserved heartbeat/guarding bytes. Reviewed by Cursor Bugbot for commit 48c6cbe. 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. |
![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: 8af66cc298
ℹ️ 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".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 124130a79b
ℹ️ 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".
…asure the reserved-state tests on all events Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43851c7ad2
ℹ️ 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".
…the transfer cap Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ff03eb5c6
ℹ️ 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".
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
1 participant
What does this change?
Three CANopen findings from the deep review that were filed as decision issues; decided here on spec and best practice:
0607 0012hwhen the server sends more than it announced (as the server's receive path and the block-upload client already do). When it sends fewer, the shorter data is still returned (devices announcing a VISIBLE_STRING's maximum length exist), but the shortfall is reported throughBackgroundExceptionOccurredinstead of passing silently.HeartbeatReceived/NodeGuardingReceivedwithState = Initializing, which is the value of a boot-up. It still counts as a sign of life and is still recorded for the NMT master (an existing test pins that).SendSyncAsyncis documented as a raw send, not gated by the NMT state or bit 30 of1005h; the README's Table 37 row says so. Gating it would change the behaviour of existing callers and two integration tests, for a method whose name is not the producer's.Closes #253, closes #254, closes #255.
Type of change
feat— new behaviour (minor release)fix/perf— bug or performance fix (patch release)docs/test/refactor/chore/ci— no release!in the title, plus aBREAKING CHANGE:footer explaining the migration)Checklist
dotnet build CanKit.Pro.sln -c Releasesucceeds (with-p:CI=true)dotnet test CanKit.Pro.sln -c Releasepasses (net10.0; net48 only runs in CI)Mutation checks: removing the over-length abort, removing the shortfall report, decoding reserved bytes as
Initializing, and reporting them from the guarding path each fail the matching new test. #254 is documentation only and has no test.🤖 Generated with Claude Code