Skip to content

test(canopen): witness SDO server batch completion - #485

Merged
dborgards merged 1 commit into
mainfrom
fix/sdo-server-batch-witnesses-362
Oct 27, 2026
Merged

dborgards merged 1 commit into
mainfrom
fix/sdo-server-batch-witnesses-362

Conversation

@dborgards

Copy link
Copy Markdown
Owner

What does this change?

The SDO block-upload server regressions used 300 ms negative windows to guess when an old batch had stopped, while RawCan echo deadlines still ran on host time. Retain the existing ordered-send Task on its session and expose a narrow internal actor-only snapshot, so the tests can await the original batch even after that session ends. Both fixtures now use a separate VirtualClock confirmation actor with real RawCan echo matching.

Session-end cases wait for the actual batch completion before asserting no further segments. The partial-ACK case captures the distinct retransmission task, positively drains the remaining segments, awaits both batches and verifies the complete sequence 1–20. Abort, index, payload/count and ordering assertions remain. The timeout row uses a 500 ms configured server timeout and steps to 750 ms, before RawCan's unchanged one-second deadline, explicitly checking SdoProtocolTimedOut. No public API or protocol behavior changes; the production additions only retain/snapshot an existing Task.

Refs #362. Final issue inventory remains to be checked after merge; this PR does not close the issue automatically.

Type of change

  • docs / test / refactor / chore / ci — no release

Validation

Six focused theory cases pass. Three actual source mutation configurations were runtime-red and restored: restore runtime RawCan binding (all six fail), ignore ended-session identity, and ignore superseding batch generation. Runner: /private/tmp/cankit-362-server-batch-mutations.py.

Full local gates passed: 3043 net10 tests with coverage, net48 compilation, CI build, formatting, nine verified packages, 50 engineering tests and traceability. All Cobertura classes/nested types were inspected. Both changed executable entries are covered, including both getter branches (100%); no changed missing/partials. Public API baseline is unchanged.

Checklist

@cursor

cursor Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Test-only harness plus internal task retention; no change to CANopen protocol handling or public surface.

Overview
Hardens SDO block-upload server regression tests by replacing flaky wall-clock sleeps with deterministic witnesses of ordered-send batch completion.

Production code only retains the existing SendOrderedControlFrames task on SdoBlockServerSession.UploadBatch and adds an internal SdoBlockUploadBatchForTests snapshot so tests can capture and await the in-flight batch (and a distinct retransmission batch after a partial ACK) even after the session slot is cleared. No wire or public API behavior changes.

Both affected theory fixtures now use VirtualClock + ClockedCanBusService for RawCan echo timing. Session-end cases await firstBatch before asserting no segments 2–20; the partial-ACK path drains echoes and asserts a full 1–20 sequence once. The timeout row uses a 500 ms SdoServerTimeout and asserts SdoProtocolTimedOut on the final abort.

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 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-27T06:55:53.710474Z be41d48 Manual request
ℹ️ 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 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Owner Author

@codex review

Please review the current head be41d48. The PR-opened review summary has remained Running since 06:30 UTC with no submitted review or findings. CI, CodeQL, Codecov and both final CI coverage reports are green; this request is to obtain the required completed head review before the quiet period and merge.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: be41d48b73

ℹ️ 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".

@dborgards
dborgards merged commit 1744acf into main Oct 27, 2026
16 checks passed
@dborgards
dborgards deleted the fix/sdo-server-batch-witnesses-362 branch October 27, 2026 07:08
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