docs: state in every package README what is validated and what is not - #245
Conversation
ADR 0001 (release checklist, item 5) requires each package README to say what has been validated and what has not, software doubles versus hardware. The nine READMEs carried the status block about the withdrawn 1.x releases but nothing on validation; the only statement was in the roadmap of the root README. Each README now has the same two-part section. What is validated is what the tests in this repository cover, named per package. What is not validated is stated per package too, and every one of them says that nothing has run against real CAN hardware, a conformance tester or a third-party implementation: the test project references CanKit.Adapter.Virtual and no hardware adapter. Only claims that the test project supports are made. TX echo of real adapters is said to be modelled by a test double, and the UDS client is said to be tested against a simulation written from the same reading of the standard. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWRpkQzKkNDYz3WiNgWvWU
PR SummaryLow Risk Overview Each section has Validated (what Reviewed by Cursor Bugbot for commit 6e34911. 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! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f02878de2
ℹ️ 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".
Two statements in the new validation sections claimed more than the tests do. The IsoTp section said STmin spacing is measured on the loopback with CI-tolerant bounds. IsoTpStminTimingTests runs on a clock the test drives and brackets each interval exactly; nothing in it measures elapsed time, and its class documentation says so. The section now says that, and moves real-time STmin spacing to the not-validated half. The Reliability section said bus states are set in software on the virtual bus. BusStateMonitorTests.OpenBus returns a ControllableBus and the tests set that double's BusState directly; the virtual adapter only supplies its configuration. The section now names the double. Checking the other seven sections against the tests found three more that were looser than the suites: the Actor's time-dependent tests are only partly on a virtual clock, and the J1939 and J1939Tp suites run through ControllableBus as well as directly on the virtual adapter. Each is reworded to what the suites do. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWRpkQzKkNDYz3WiNgWvWU
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a09ebb903
ℹ️ 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".
…y are Two more statements in the validation sections did not match the test project. The RawCan section named `EchoWorlds` as the test double that models the TX echo of SocketCAN, Kvaser and Vector. EchoWorlds.cs defines the EchoWorld enum and the EchoWorldFixture; the double is ControllableBus.EchoCapable, whose documentation is where those three adapters are named. The section now names it. The CANopen section offered PeerSdoLaboratory as an example of a peer written for the tests. It is a factory for test EDS/DCF descriptions and a helper that binds them to a node; it sends and receives nothing. The tests run between instances of the node itself and against raw frames they transmit, and the section says that instead. Checked again, name by name and claim by claim, against the test project: the UDS section said the ECU simulation was written from the same reading of the standard as the client, which nothing shows, and now says it is a scripted simulation whose responses each test defines. It also now says that some services are covered thinly. A doubled "in" in the Addressing section is removed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWRpkQzKkNDYz3WiNgWvWU
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df9c1d752a
ℹ️ 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 Actor section said mailbox ordering and the single-writer discipline are validated in all three execution modes. The tests do not do that: the ordering test runs in the default dedicated-thread mode, serialization under concurrent callers is tested in the dedicated-thread and thread-pool modes, and the synchronization-context tests cover marshaling, failure surfacing, timers and dispose, not ordering or concurrent serialization. The section now says which mode each property is tested in. The earlier count of references to the three modes in the test file had counted mentions, not what each test asserts. The other enumerations in the sections were compared against test method names this time rather than against hit counts in test bodies: the J1939-TP timers T1 to T4, ISO-TP's N_As, N_Bs and N_Cr, CANopen SYNC and EMCY, and the UDS services each have a test of that name or subject. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWRpkQzKkNDYz3WiNgWvWU
What does this change?
Release checklist item 5 of ADR 0001 requires each package README to state what has been validated and what has not, software doubles versus hardware. The nine READMEs carried the status block about the withdrawn 1.x releases but nothing on validation; the only statement was in the roadmap of the root
README.md.Each README now has the same two-part section,
## What is validated, and what is not, directly after the status block:CanKit.Adapter.Virtualand no hardware adapter, which is what that sentence rests on.Only claims the test project supports are made, and each name in the sections was checked against it. Two examples of what that looks like: the TX echo of real adapters is said to be modelled by a test double (
ControllableBus.EchoCapable), not observed on those adapters, and the UDS client is said to be tested againstSimulatedUdsEcu, a scripted ECU simulation in this repository whose responses each test defines.Documentation only; no code, API, or test changes.
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 (run with-p:CI=true, 0 warnings)dotnet test CanKit.Pro.sln -c Releasepasses — not run; only Markdown changedAlso run locally:
dotnet format --verify-no-changes(clean) anddotnet packwitheng/verify-packages.py(the READMEs are packed into the packages, which the script checks).What to check
The wording of the Not validated half is the part only a maintainer can vouch for. In particular: that no HIL run, conformance test or third-party interop exists anywhere outside this repository. This PR states what the repository shows; if such a run exists, the sentence is wrong for that package and should say so.
The older Timing accuracy — STmin pacing paragraph in the IsoTp README describes a wall-clock measurement the STmin test no longer makes. It predates this PR and is not changed here; it is tracked in #246.
The dated ADR checklist is left as written; the README statement it asks for is what this PR adds.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UWRpkQzKkNDYz3WiNgWvWU