Skip to content

test(canopen): clock SDO block confirmation deadlines - #483

Merged
dborgards merged 1 commit into
mainfrom
fix/sdo-block-confirm-clocks-362
Oct 27, 2026
Merged

dborgards merged 1 commit into
mainfrom
fix/sdo-block-confirm-clocks-362

Conversation

@dborgards

Copy link
Copy Markdown
Owner

What does this change?

Two CANopen SDO block-client regression fixtures advanced protocol time while deferred RawCan echo confirmations still used host-time deadlines. Give them a separate VirtualClock confirmation actor and explicitly witness the actual confirmation timer before and after every step. The node uses the same time source, and actor round trips still settle its protocol timers.

The long-subblock case retains its 600 ms segment delays and one-second SDO/RawCan budgets. The late-confirmation case configures a 500 ms SDO timeout and confirms at 750 ms, before RawCan's unchanged one-second deadline: this isolates a protocol timeout followed by a transport-valid late confirmation. Previously its 1.5-second virtual wait passed the physical confirmation deadline without advancing it. All payload, end-request and timeout-abort assertions remain. No production code or public API changes.

Refs #362. Remaining reset and SDO block-server timing fixtures are still open in the inventory.

Type of change

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

Validation

Both focused cases pass. Three real source mutation configurations were runtime-red and restored: restore host-time RawCan binding (both cases fail), omit per-segment SDO rearming, and ignore an already expired block-client timeout. Runner: /private/tmp/cankit-362-sdo-confirm-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 444 Cobertura classes/nested types were parsed; production diff is empty, with no changed executable missing/partial entries.

Checklist

@cursor

cursor Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Test-only harness changes; no production or public API modifications.

Overview
Two SDO block-download regression tests (#362, C12) no longer advance protocol time with ManualTimeSource while deferred echo confirmations still used host-time deadlines.

They now run on a VirtualClock with a dedicated confirmation actor, ClockedCanBusService, and the node's timeSource aligned to that actor. Each segment step waits for the confirmation timer to arm, advances virtual time, then waits for the re-armed SDO segment timer before releasing echoes.

The late-confirmation case uses a 500 ms SdoTimeout (was 1 s) so the protocol timeout fires at 750 ms virtual time before RawCan's unchanged 1 s confirm deadline—isolating timeout-then-late-confirm without reviving the transfer. Assertions on payloads, end requests, and SdoProtocolTimedOut are unchanged.

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

@chatgpt-codex-connector

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 🔄 Running since 2026-10-27T05:28:45.043408Z 332e6fa 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 27, 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 f3edfa3 into main Oct 27, 2026
14 checks passed
@dborgards
dborgards deleted the fix/sdo-block-confirm-clocks-362 branch October 27, 2026 05:45
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