Skip to content

feat(uds): the collected UDS findings -- busy repeat, functional addressing, and four corrections - #150

Merged
dborgards merged 55 commits into
mainfrom
fix/uds-collected-findings
Sep 22, 2026
Merged

dborgards merged 55 commits into
mainfrom
fix/uds-collected-findings

Conversation

@dborgards

@dborgards dborgards commented Sep 21, 2026

Copy link
Copy Markdown
Owner

What does this change?

The six UDS findings collected in #57:

  • DiagnosticSessionControlAsync(byte) no longer masks with 0x7F. 0x00 (ISOSAEReserved) and any value with bit 7 set — suppressPosRspMsgIndication, not a session type — are rejected with ArgumentOutOfRangeException before anything is sent.
  • SendRawAsync with the suppress bit set does not wait. On a service that carries a sub-function, a request with bit 7 set is sent the way a suppressed TesterPresent is and returns empty, instead of waiting P2 for a response that never comes; the next request for the same service waits out the suppressed send's P2 so a late negative response to it is not taken — one window per service, extended by P2* on NRC 0x78; the functional client keeps a listener per service alive for the whole window, on one subscription made before the send — the new IsoTpFunctionalClient.Listen() / IsoTpFunctionalListener — so nothing in it goes unobserved.
  • NRC 0x21 (busyRepeatRequest) gets its repeat. New options MaxBusyRepeatRequests (default 3) and BusyRepeatRequestDelay (default zero); the negative response surfaces once the repeats are used up.
  • UdsTimeoutException.Elapsed is the budget on every path, never a measurement of how late the client noticed.
  • Functional addressing: new UdsFunctionalClient over IsoTpFunctionalClient — one request on the functional identifier, every ECU's Single-Frame answer collected within a window and read as UDS (UdsFunctionalResponse: source identifier, bytes, IsNegative, NRC); TesterPresentAsync() is the keep-alive to everyone (3E 80). Only answers correlated to the request (its positive SID echoing the request's leading bytes — sub-function, DID, routine identifier, block counter, mode of operation, memory address and size — or a negative response naming it) are attributed to a call; calls run one at a time, and a call queued behind another does not send after Dispose.
  • Dispose leaves the request lock to a holder that outlasts the wait, so the holder's eventual release does not throw ObjectDisposedException into an operation that was merely slow. The wait is an internal, test-settable timeout (InternalsVisibleTo added for the tests).

Closes #57.

Type of change

  • feat — new behaviour (minor release): the busy-repeat options, UdsFunctionalClient, IsoTpFunctionalResponse.HostArrivalTimestamp, IsoTpFunctionalClient.Listen() with IsoTpFunctionalListener, IsoTpFunctionalClient.SendWithTransmitStampAsync() / SendAndCollectWithTransmitStampAsync() with IsoTpFunctionalCollection, IIsoTpChannel.SettleAsync() and IIsoTpChannel.DiscardPendingPdus(long)
  • fix / perf — bug or performance fix (patch release): the four corrections, in the same commit
  • docs / test / refactor / chore / ci — no release
  • Breaking change (! in the title, plus a BREAKING CHANGE: footer explaining the migration) — in c3c025b and a8b54a1, not the title: IIsoTpChannel gains SettleAsync and DiscardPendingPdus(long), source-breaking for external implementers only; a minor bump under ADR-0001 before v1.3.0, as SendWithTransmitStampAsync was in fix(uds): end P2 with the response's first frame, and treat an all-zero seed as unlocked #143

Checklist

  • dotnet build CanKit.Pro.sln -c Release succeeds (with -p:CI=true, 0 warnings)
  • dotnet test CanKit.Pro.sln -c Release passes (net10.0, all 937)
  • Public API changes are documented with XML comments — UdsFunctionalClient, UdsFunctionalResponse, the two options, the two IUdsClient docs
  • New behaviour is covered by a test — DiagnosticSessionControl_Rejects_A_Session_Type_Outside_01_To_7F, SendRaw_With_The_Suppress_Bit_Does_Not_Wait_For_A_Response, BusyRepeatRequest_Is_Repeated_Until_The_Server_Answers / …_Is_Surfaced_Once_The_Repeats_Are_Used_Up, M_A_Timeout_Reports_The_Budget_Not_How_Late_The_Client_Noticed, N_Dispose_Leaves_The_Lock_To_A_Holder_That_Outlasts_The_Wait, the twenty-nine UdsFunctionalClientTests, A_Late_Negative_Response_To_A_Suppressed_Send_Is_Not_The_Next_Requests, Suppressed_Send_Windows_Are_Kept_Per_Service, A_Pending_Answer_To_A_Suppressed_Send_Extends_Its_Window_By_P2Star, A_Queued_Pending_Answer_Still_Extends_A_Window_That_Has_Run_Out, A_Queued_Pending_Answer_From_After_The_Windows_End_Does_Not_Revive_It, A_Pending_Answer_Still_On_Its_Way_Through_The_Channel_Extends_The_Window, Settle_Takes_A_Buffered_Single_Frame_Through_To_The_Inbox, A_Discard_Given_A_Stamp_Keeps_What_Arrived_After_It, Functional_Send_Refuses_A_Window_Beyond_A_Timers_Reach_Before_Transmitting, A_Cancelled_Wait_Keeps_The_Rest_Of_The_Window, O_A_Suppressed_Send_Cancelled_Before_Confirmation_Still_Opens_Its_Window, P_A_Stale_Transport_Fault_Queued_Behind_A_Suppressed_Send_Does_Not_Fail_The_Next_Request, R_A_Suppressed_Send_Cancelled_After_Its_Provisional_Window_Still_Opens_One, A_Pending_Answer_For_Another_Service_Heard_During_A_Wait_Extends_That_Services_Window, A_Pending_Answer_Consumed_As_Another_Requests_Stray_Still_Extends_Its_Window, A_Pending_Answer_From_Before_The_Handoff_Is_Not_This_Requests, A_Stale_Pending_Answer_Queued_Before_A_Suppressed_Send_Does_Not_Extend_Its_Window, Q_A_Stray_Pending_From_Before_The_Handoff_Still_Moves_Its_Services_Window, A_Pending_Answer_From_After_A_Windows_End_Does_Not_Revive_It, A_Pending_Answer_From_After_A_Windows_End_Heard_In_A_Wait_Out_Does_Not_Revive_It, Functional_Listener_Keeps_What_Arrives_Between_Collections, Functional_Listener_Disposal_Ends_A_Collection_In_Progress, Functional_Listener_Reports_A_Service_Disposed_Underneath_On_The_Next_Collection, Options_With_Carries_The_Busy_Repeat_Settings and A_Negative_Busy_Repeat_Count_Is_Rejected_At_Construction; each fails with its behaviour reverted (mutation-checked)
  • The requirement or ADR this relates to is referenced — FR-UDS-001, FR-UDS-007, FR-UDS-008, FR-UDS-010

🤖 Generated with Claude Code

…essing, and four corrections

The six UDS findings collected in #57.

DiagnosticSessionControlAsync(byte) masked the session type with 0x7F, so a
caller passing 0x83 sent 0x03 without being told; 0x00 and any value with
bit 7 set are rejected now, the bit being suppressPosRspMsgIndication.

SendRawAsync with that bit set waited P2 for a positive response that was
never coming and timed out; it now sends the way a suppressed TesterPresent
does and returns empty.

NRC 0x21 (busyRepeatRequest) surfaced as a failure; it asks for a repeat,
and the client repeats, up to MaxBusyRepeatRequests (new option, default 3)
after BusyRepeatRequestDelay (new, default zero), each with a fresh P2.

UdsTimeoutException.Elapsed reported the budget on one timeout path and a
measurement of how late the client noticed on another; it is the budget on
every path now.

There was no functional-addressing path: UdsFunctionalClient (new) wraps an
IsoTpFunctionalClient -- one request on the functional identifier, every
ECU's Single-Frame answer collected within a window and read as UDS in a
UdsFunctionalResponse, with TesterPresentAsync as the keep-alive to everyone.

Dispose disposed the request lock after a five-second wait while a holder
still owned it, turning a slow operation into an ObjectDisposedException on
its thread; a holder that outlasts the wait now keeps the semaphore.

Each is mutation-checked: the mask, the wait, the repeat, the measurement,
the disposal and the functional parse each fail their test when reverted.

Closes #57.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Sep 21, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Large changes to UDS P2/P2* timing, suppressed-send state, and ISO-TP inbox ordering; mis-handling could mis-attribute responses or break deadline behavior under concurrency.

Overview
ISO-TP gains SettleAsync and DiscardPendingPdus(long) so callers can flush in-flight frames before judging timeouts or drops without losing frames that arrived after a pre-read stamp. The functional ISO-TP client adds transmit stamps, host arrival timestamps on responses, Listen() / IsoTpFunctionalListener for one subscription across collections, and deadline-aware window filtering (including pre-send validation of timer limits).

Physical UDS (UdsClientImpl) implements the #57 corrections: DiagnosticSessionControlAsync(byte) rejects 0x00 and suppress-bit session types; SendRawAsync with suppress bit returns empty without waiting P2; NRC 0x21 repeats via MaxBusyRepeatRequests / BusyRepeatRequestDelay; UdsTimeoutException.Elapsed always reports the configured budget; Dispose avoids disposing the request lock if the holder outlasts a timeout. A new SuppressedResponseWindows tracker waits out late negatives/0x78s from suppressed sends (per service, P2* extensions) using SettleAsync and stamped discards that route stray 0x78 before dropping.

UdsFunctionalClient is new: functional broadcast requests with answer correlation, one-at-a-time calls, per-service background listeners through the collection window (and P2* on 0x78), plus TesterPresentAsync / DiagnosticSessionControlAsync helpers. API approval snapshots and extensive tests cover settle, stamped discard, listeners, and suppressed-window races.

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 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-09-22T05:39:41.856439Z 9c458bd New commits
ℹ️ 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 Sep 21, 2026

Copy link
Copy Markdown

Comment thread tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs Fixed
Comment thread tests/CanKit.Pro.Tests/TestCases/Uds/UdsExpiredDeadlineTests.cs Fixed
…150

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e460d7fa6f

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

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs
Comment thread src/CanKit.Pro.Uds/UdsClientOptions.cs

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread src/CanKit.Pro.Uds/UdsClientOptions.cs
Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs
…ow for an unsuppressed TesterPresent, carry the busy-repeat options in With

Codex on #150, three times. The functional client wrapped every Single Frame
on the response range as an answer; it now keeps only a positive response to
the request's service or a negative response naming it, as the physical
client correlates. An unsuppressed TesterPresent with no window collected for
zero time; the window is required there. And UdsClientOptions.With dropped
the two new options on a clone; it carries and accepts them. Each
mutation-checked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7cbebdc6a0

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

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
Comment thread src/CanKit.Pro.Uds/UdsClientOptions.cs
…epeat count

Codex on #150, twice more. Two overlapping functional calls with the same
SID each subscribed to the response range and collected the other's answers;
a request lock now spans the send and its collection window, as the physical
client's does. A negative MaxBusyRepeatRequests read as "disabled"; it and a
negative BusyRepeatRequestDelay are rejected at construction, beside the
other option checks. Both mutation-checked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 77368d2994

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

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs
… dispose, and a positive response must echo the sub-function

Codex and Bugbot on #150. A call queued on the functional client's request
lock had passed its disposal check before the wait; Dispose now cancels a
lifetime token linked into the wait, and the lock holder rechecks disposal
before sending. And for a service with a sub-function, a positive response
is this request's only if it echoes the sub-function -- a late answer to an
Extended-session request no longer passes as the answer to the next one for
Default. Both mutation-checked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2769a4b9be

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

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs
Comment thread tests/CanKit.Pro.Tests/TestCases/Uds/UdsFunctionalClientTests.cs Fixed
…wait out a suppressed send's window before a same-service request

Codex on #150, twice more. A positive response echoes the request's leading
bytes -- the sub-function, a DID, a routine identifier, a block counter --
and the functional client now requires them to match, so a late answer for
DID F190 is not attributed to the request for F191. And a suppressed raw
send may still draw a negative response up to P2 after it; the physical
client's next request for the same service now waits that window out rather
than taking the negative response as its own. Both mutation-checked. The
CodeQL note on the dispose test is taken: the client is a using.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e514791330

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

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs Outdated
Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs Outdated
…s, and refuse a functional multi-DID read

Codex and Bugbot on #150. The physical client tracked one suppressed send;
a suppressed send for another service in between shortened the first's
window. Both clients now share a per-service table: a suppressed send notes
its service and the instant its P2 ends, and the next call for that service
waits the window out -- the functional client too, which released its lock
at once before, so a late negative answer to 3E 80 landed in the next
TesterPresent's window. And a functional ReadDataByIdentifier for more than
one DID is refused: a Single Frame cannot carry the answer, and only one DID
would be correlated. Each mutation-checked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 15882ec1b7

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

Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs
Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread tests/CanKit.Pro.Tests/ApiApprovals/CanKit.Pro.Uds.received [conflicted].txt Outdated
…ry functional request's window open for the rest of its P2

Codex on #150, twice more, and Bugbot once. NRC 0x78 to a suppressed send
says the peer's final answer is still coming, up to P2* later; the window
now moves out with it, on both clients, which listen while they wait. And a
functional request whose collection window ended before the peer's P2 did
could still be answered into the next same-service call's window; every
functional send now notes its window, not only a suppressed one. Both
mutation-checked. The conflict copy of the approval file a sync tool made
and `git add -A` swept in is removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Fixed
…e predicate

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29b5babd93

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

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs Outdated
Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs
Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a09fceb91e

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

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
…wait's remainder, drain before ending a wait, and correlate 0x2A on its identifier

Bugbot on #150, three times, and Codex once. The functional client noted a
request's window only after a successful collection, so a cancelled one
left none; it is noted before the send. Both wait-outs forgot the window in
a finally, so a cancelled wait dropped what remained; it keeps it, 0x78
extensions included. The physical wait-out ended without reading a 0x78
already queued when the window had run out; it drains the inbox first and
only an empty one ends it. And ReadDataByPeriodicIdentifier's transmission
mode is a parameter, not a sub-function: it is out of HasSubFunction on both
clients, and the functional client correlates its answers on the requested
periodic identifier. Each mutation-checked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…by exception

Codex on #150, the physical client's twin of the functional fix: a
suppressed send cancelled after a transmission that outlasted the
provisional window left only that window, already over, and the next
same-service request took the send's punctual answer. On an exceptional
exit the window is noted from that instant, which is at most P2 from the
transmission. Mutation-checked: without it the new stub test's next
request goes out at once.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e1fdea29b2

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

Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs
Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs
dborgards and others added 3 commits September 21, 2026 21:25
… is what the caller has read

Codex on #150. A caller that reads the inbox and then discards -- the
UDS client, routing a 0x78 to its service's window before dropping the
rest -- loses a frame that arrives between the read and the discard's own
stamp. IIsoTpChannel.DiscardPendingPdus(long arrivedBefore) drops what
arrived before the caller's stamp, taken before the read, and keeps what
arrived since for the caller's next read. The discard stamp never moves
back. Mutation-checked: with the parameter ignored the new test's frame
from after the stamp is dropped.

BREAKING CHANGE: IIsoTpChannel gains DiscardPendingPdus(long). Source-
breaking for external implementers of the interface; consumers are
unaffected. Per docs/decisions/0001-versioning-and-api-stability.md this
is a minor bump before v1.3.0 and no shim is introduced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nbox holds

Codex on #150: a 0x78 for a service with a suppressed send open, arriving
between the routing loop finding the inbox empty and the bulk discard,
was dropped unrouted. The stamp is taken before the settle and the read,
and the discard drops only what arrived before it; a frame from after it
stays for the request's receive loop, which routes a stray itself. Closed
by construction: the gap is a few microseconds wide and no test opens it
reliably; the channel's own test covers the stamp's semantics.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ything is transmitted

Codex on #150: a window past CancellationTokenSource.CancelAfter's bound
passed the positivity check, the functional frame went out, and the
collector's timer threw after it -- a session change on every ECU behind
an argument error. The ISO-TP functional client refuses it before the
send, and the UDS functional client before its listener starts.
Mutation-checked: without both checks the two new tests see the frame
transmitted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6728759ae4

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

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
…ection window is

Codex on #150: a response or pending window beyond a timer's reach was
accepted, the first request went out, and the listener faulted when it
collected for it -- its window unwaited by the next call. Both are
refused at creation. Mutation-checked: without the bound the new theory's
two oversized cases construct a client.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc86326b4c

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

Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs Outdated
Codex on #150, for BusyRepeatRequestDelay: a delay beyond what Task.Delay
accepts was taken at construction and threw at the first NRC 0x21, after
the request went out. P2ClientMax, P2StarClientMax and TesterPresentPeriod
run timers too, and all four are bounded at construction now.
Mutation-checked: without the bounds the new theory's four cases construct
a client.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5cb50c0a91

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

Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs
… transmitting

Codex on #150: a suppressed request the channel rejects for its size --
or, on the functional client, any request too long for a Single Frame --
had its window noted before the send and noted again on the exception,
and the next same-service request waited P2 for an answer to nothing.
The channels refuse before transmitting with an argument error, an
InvalidOperationException or ObjectDisposedException; on those the window
goes back to what it was, and the functional listener started for the
send is retired so the next call does not wait on it. A cancellation or
transport fault, which comes after the frame may be out, still notes the
window. Mutation-checked: with every exception treated as after the
send, the three new cases wait 2 s.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cfec2b8cbe

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

Comment thread src/CanKit.Pro.IsoTp/IsoTpFunctionalListener.cs
dborgards and others added 2 commits September 21, 2026 22:22
… back

Bugbot on #150: retiring forgets the window, so the restore that preceded
it was undone. On the functional client the earlier window is always
waited out before a send, so nothing observable changed there; on the
physical client, where two suppressed sends follow without a wait-out,
the restore's effect is now covered: an earlier send's window survives a
refused one. Mutation-checked: with the restore forgetting instead, the
next request goes out at once.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…king what was carried over

Codex on #150: the timer threw after the previous collection's carry-over
had been taken, and it was lost. The check precedes the take now. By
construction: the carry-over is filled only in the instant between a
timer and its drain, which no test opens deterministically; the test
covers the refusal and that the subscription's buffer survives it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 92b2ae4ab8

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

Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs Outdated
dborgards and others added 2 commits September 21, 2026 22:34
… two cancelled-send tests

macOS CI on #150, on 92b2ae4: test R's cancellation at 150 ms raced the
stub's 300 ms transmission, and the runner's timer lost by more than
that. The transmission is held until cancelled in R and in O, so nothing
races the cancellation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…st's discard

Codex on #150: the discard on an aborted request pumped and dropped a
0x78 for a suppressed send that was still on its way, unrouted; it now
takes its stamp, settles, reads and routes first, as the pre-send discard
does. Mutation-checked: with the plain discard the new test's next
request goes out at P2 instead of P2*.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 367315c0d2

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

Comment thread src/CanKit.Pro.Uds/UdsClientImpl.cs
…n answered one

Codex on #150: a late 0x78 to an earlier request for the service, queued
since that request was cancelled, was read by the next call's wait-out as
inside the suppressed send's window -- it predates the send, which the
window's end alone does not say -- and moved the window out by P2* for
nothing. The suppressed path runs the pre-send discard now, which routes
a 0x78 for a service with a window still open and drops the rest.
Mutation-checked: without it the new test's next request waits 1.9 s.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5672520701

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

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
dborgards and others added 2 commits September 22, 2026 07:19
… a response from before its handoff

Codex on #150. IsoTpFunctionalClient.SendWithTransmitStampAsync and
SendAndCollectWithTransmitStampAsync return IsoTpTransmitStamps -- the
handoff to the driver and the transmission, as the physical channel
reports them -- for a caller that keeps a deadline from the transmission;
SendAsync and SendAndCollectAsync delegate to them. A response that
arrived before the handoff answers something else: the subscription is
made before the send, and another sender may hold the service's transmit
lock in between. It is left out of the collection. Mutation-checked
through the UDS test that opens that gap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d take no 0x78 from before the handoff

Codex on #150. A 0x78 for the service arriving after the request's
subscription was made but before the frame was handed to the driver --
another sender holding the shared service's transmit lock -- was taken as
this request's and moved its window out by P2*. The functional client
anchors the window at the transmission the driver reports, as the physical
client does, and applies a 0x78 heard while the send was in flight only if
it arrived at or after the handoff. Two tests that encoded the earlier
estimate -- the window anchored at the confirmation, the collection's
length back -- now model a late acceptance, the case the estimate stood in
for; the two readings are told apart in each. Mutation-checked: without
the cutoff the new test's next request waits P2*; without the filter it
collects the 0x78; with the old estimate, or the pre-send note alone, the
anchoring tests fail.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Fixed

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6006623. Configure here.

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6006623333

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

Comment thread src/CanKit.Pro.Uds/UdsFunctionalClient.cs Outdated
dborgards and others added 2 commits September 22, 2026 07:31
…hat reads a 0x78 after the anchoring

Bugbot and Codex on #150, on 6006623: the cutoff was applied only to the
0x78s recorded while the send was in flight. A listener whose collection
returns after the anchoring -- its window longer than the request's
collection -- applied a 0x78 from before the handoff with no cutoff, and
so did the anchoring after the response loop, which passed none and
erased the one just set. The cutoff is kept per service with the window
now, consulted by the listener, carried through both anchorings and
forgotten with the window. The handoff test runs with a 1000 ms window
as well, where the listener is the one that reads the 0x78; without the
listener's check that case waits P2*. CodeQL's Where taken.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rt, not after a timer

macOS CI on #150, on 6006623: the 0x78 was stamped after a 20 ms timer,
which the runner fired after the 100 ms window had run out, so the frame
was -- correctly -- not the window's. The stamp is taken right after the
request's synchronous start: after its pre-send discard's stamp, which
would otherwise drop the frame as older, and inside the window by
construction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dborgards
dborgards merged commit 83be517 into main Sep 22, 2026
14 checks passed
@dborgards
dborgards deleted the fix/uds-collected-findings branch September 22, 2026 06:03
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.

UDS: collected minor findings (silent masking, suppress-response wait, NRC 0x21, dispose)

2 participants