From d301db33c5fa18534b09d1242c1bafe7a73b4df7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 21:48:45 +0000 Subject: [PATCH 1/8] refactor(uds): measure and wait out P2 on an injectable clock UdsClientImpl can now take an internal ProtocolActor, through a new internal UdsClient.Create(channel, clock, ...) overload. The timings move onto it: - Timestamps: the P2 and P2* budget, the suppressed-response windows and the stale-PDU discard now read Now() and Ticks() on the actor's time source, instead of Stopwatch. - Waits: the P2/P2* receive timeout, the suppressed-window wait and the busy-repeat delay are timers on that actor (CancelAfter, DelayAsync). The timer fires on the actor loop, and the cancellation it triggers is handed to the thread pool rather than run there. Stamps alone are not enough. An earlier attempt on this branch put only the stamps on the clock and kept real timeout sources for the waits. The order of a late answer and the next transmit was then decided on the wall clock, and three converted tests stopped catching the defects they exist for (recorded on #171). With the waits on the clock as well, a window ends only when the test moves the clock. The in-progress recheck stays a real timer. It polls the channel's state rather than enforcing a protocol deadline. With no clock injected, which is always the case through the public API, both helpers fall back to real timers and Stopwatch. Refs #171 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s --- src/CanKit.Pro.Uds/UdsClient.cs | 19 ++++ src/CanKit.Pro.Uds/UdsClientImpl.cs | 139 ++++++++++++++++++++++++---- 2 files changed, 138 insertions(+), 20 deletions(-) diff --git a/src/CanKit.Pro.Uds/UdsClient.cs b/src/CanKit.Pro.Uds/UdsClient.cs index 8d8b581..eb0855e 100644 --- a/src/CanKit.Pro.Uds/UdsClient.cs +++ b/src/CanKit.Pro.Uds/UdsClient.cs @@ -1,4 +1,5 @@ using System; +using CanKit.Pro.Actor; using CanKit.Pro.IsoTp; namespace CanKit.Pro.Uds; @@ -34,4 +35,22 @@ public static IUdsClient Create( if (channel is null) throw new ArgumentNullException(nameof(channel)); return new UdsClientImpl(channel, options ?? new UdsClientOptions(), ownsChannel: !leaveOpen); } + + /// + /// As , measuring and waiting + /// out P2, P2* and the suppressed-response windows on ; null is the + /// wall clock, as + /// the public overload uses. must have been opened on that same + /// actor, and its demux must stamp frames with its time source, or a deadline and an + /// arrival are not comparable (#171). + /// + internal static IUdsClient Create( + IIsoTpChannel channel, + ProtocolActor? clock, + UdsClientOptions? options = null, + bool leaveOpen = true) + { + if (channel is null) throw new ArgumentNullException(nameof(channel)); + return new UdsClientImpl(channel, options ?? new UdsClientOptions(), ownsChannel: !leaveOpen, clock); + } } diff --git a/src/CanKit.Pro.Uds/UdsClientImpl.cs b/src/CanKit.Pro.Uds/UdsClientImpl.cs index 81288c8..b613f43 100644 --- a/src/CanKit.Pro.Uds/UdsClientImpl.cs +++ b/src/CanKit.Pro.Uds/UdsClientImpl.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using CanKit.Pro.Actor; using CanKit.Pro.IsoTp; namespace CanKit.Pro.Uds; @@ -53,6 +54,15 @@ internal sealed class UdsClientImpl : IUdsClient private readonly SemaphoreSlim _requestLock = new(1, 1); private readonly CancellationTokenSource _lifetimeCts = new(); + // Null in production: P2/P2*, the suppressed-response windows and the busy-repeat delay are + // measured on Stopwatch.GetTimestamp() and waited out on real timers. A test injects the + // actor whose clock the channel it opened shares; then every timestamp this class compares + // against a deadline *and* every wait for one is on that clock, so a test advances it + // instead of sleeping, and "did the answer come before the window closed" is decided by + // the test's order of events rather than by the host's scheduling (#171). Not disposed here. + private readonly ProtocolActor? _clock; + private readonly ITimeSource _time; + // Test hook: fires whenever a caller finds _requestLock already held and starts waiting on // it -- the observable a queued call is waiting on, standing in for a wall-clock sleep // timed to land while an earlier call holds the lock (#171). No-op in production; a test @@ -89,10 +99,26 @@ private async Task AcquireRequestLockAsync(CancellationToken cancellationToken) private int _disposed; public UdsClientImpl(IIsoTpChannel channel, UdsClientOptions options, bool ownsChannel) + : this(channel, options, ownsChannel, clock: null) + { + } + + /// + /// As the public constructor, measuring and waiting out P2, P2*, the suppressed-response + /// windows and the busy-repeat delay on ; null (the public + /// constructor's choice) is the wall clock. The + /// channel underneath must have been opened on that same actor, and its demux must stamp + /// frames with its time source, or a deadline and an arrival are not comparable (#171). The + /// actor is not disposed with this client. + /// + internal UdsClientImpl(IIsoTpChannel channel, UdsClientOptions options, bool ownsChannel, + ProtocolActor? clock) { _channel = channel; _options = options; _ownsChannel = ownsChannel; + _clock = clock; + _time = clock?.TimeSource ?? MonotonicTimeSource.Instance; // Every duration here runs a timer, and a timer measures about 49 days at most: one // beyond that would throw when it is armed, after the request went out (Codex on #150). @@ -482,7 +508,7 @@ private async Task SendWithoutResponseAsync(byte[] request, CancellationToken ca // confirmation, the frame is on the bus and may still be answered (Codex on #150). // Moved out to the transmit stamp afterwards. bool hadWindow = _suppressedWindows.TryGetDeadline(request[0], out var previousUntil); - _suppressedWindows.Note(request[0], Stopwatch.GetTimestamp(), _options.P2ClientMax); + _suppressedWindows.Note(request[0], Now(), _options.P2ClientMax, _time.Frequency); IsoTpTransmitStamps stamps; try { @@ -499,11 +525,11 @@ private async Task SendWithoutResponseAsync(byte[] request, CancellationToken ca // A send that leaves by exception -- cancelled, or a transport fault -- may // have put the frame on the bus after the provisional window ran out; its P2 // from the transmission is at most P2 from now (Codex on #150). - _suppressedWindows.Note(request[0], Stopwatch.GetTimestamp(), _options.P2ClientMax); + _suppressedWindows.Note(request[0], Now(), _options.P2ClientMax, _time.Frequency); throw; } - var sent = stamps.LastFrameTransmitTimestamp > 0 ? stamps.LastFrameTransmitTimestamp : Stopwatch.GetTimestamp(); - _suppressedWindows.Note(request[0], sent, _options.P2ClientMax); + var sent = stamps.LastFrameTransmitTimestamp > 0 ? stamps.LastFrameTransmitTimestamp : Now(); + _suppressedWindows.Note(request[0], sent, _options.P2ClientMax, _time.Frequency); } finally { @@ -531,7 +557,7 @@ private async Task WaitOutSuppressedResponseWindowAsync(UdsServiceId serviceId, { while (true) { - var remaining = SuppressedResponseWindows.Remaining(until); + var remaining = SuppressedResponseWindows.Remaining(until, Now(), _time.Frequency); if (remaining <= TimeSpan.Zero) { // The window is over as measured now -- but a 0x78 may be queued already, @@ -542,7 +568,7 @@ private async Task WaitOutSuppressedResponseWindowAsync(UdsServiceId serviceId, if (DrainExtends(sid, ref until)) continue; break; } - using var slice = new CancellationTokenSource(remaining); + using var slice = CancelAfter(remaining); using var combined = CancellationTokenSource.CreateLinkedTokenSource(linkedToken, slice.Token); IsoTpReceivedPdu pdu; try @@ -602,7 +628,7 @@ private bool ExtendOnPending(byte sid, in IsoTpReceivedPdu pdu, ref long until) var data = pdu.Pdu; if (data.Length < 3 || data[0] != NegativeResponseSid || data[2] != NrcResponsePending) return false; - var extendedUntil = pdu.FirstFrameArrivalTimestamp + (long)(_options.P2StarClientMax.TotalSeconds * Stopwatch.Frequency); + var extendedUntil = pdu.FirstFrameArrivalTimestamp + Ticks(_options.P2StarClientMax); if (data[1] != sid) { // Another service's: only a window still open when the 0x78 arrived (Codex on #150). @@ -1052,7 +1078,7 @@ private async Task ExecuteCoreAsync(UdsServiceId serviceId, byte[] reque when (ex.Code == NrcBusyRepeatRequest && repeats < _options.MaxBusyRepeatRequests) { if (_options.BusyRepeatRequestDelay > TimeSpan.Zero) - await Task.Delay(_options.BusyRepeatRequestDelay, linkedToken).ConfigureAwait(false); + await DelayAsync(_options.BusyRepeatRequestDelay, linkedToken).ConfigureAwait(false); } } } @@ -1082,7 +1108,7 @@ private async Task ExchangeOnceAsync(UdsServiceId serviceId, byte[] requ // Read before the request is handed to the channel, as the fallback for a channel that // reports no handoff instant: nothing that reached the wire after this reading can be // an earlier request's response. - var requestStarted = Stopwatch.GetTimestamp(); + var requestStarted = Now(); var stamps = await _channel.SendWithTransmitStampAsync(request, linkedToken) .ConfigureAwait(false); var transmitStamp = stamps.LastFrameTransmitTimestamp; @@ -1090,7 +1116,7 @@ private async Task ExchangeOnceAsync(UdsServiceId serviceId, byte[] requ // Zero means the channel reported no transmit instant. Falling back to now is the old // behaviour, which is worse but not broken; treating zero as a timestamp would read as // infinitely long ago and time out every request. - var budgetStart = transmitStamp > 0 ? transmitStamp : Stopwatch.GetTimestamp(); + var budgetStart = transmitStamp > 0 ? transmitStamp : Now(); // A response whose first frame arrived before this is an earlier request's (Codex on // #143). The bound is the channel's handoff of the request's *last* frame, taken just // before the driver call: a peer answers only a complete request, so nothing on the @@ -1234,7 +1260,7 @@ private static bool IsAllZero(byte[] data, int offset, int count) // among it is routed to its service's window rather than dropped unseen (Codex on #150). private async Task DiscardStalePdusAsync() { - long arrivedBefore = Stopwatch.GetTimestamp(); + long arrivedBefore = Now(); await SettleAsync().ConfigureAwait(false); DiscardStalePdus(arrivedBefore); } @@ -1292,7 +1318,7 @@ private void RouteStrayPending(in IsoTpReceivedPdu pdu) // Only a window still open when the 0x78 arrived: one that had run out is not revived // for a full P2* by a late frame (Codex on #150). _suppressedWindows.ExtendIfOpenAt(data[1], pdu.FirstFrameArrivalTimestamp, - pdu.FirstFrameArrivalTimestamp + (long)(_options.P2StarClientMax.TotalSeconds * Stopwatch.Frequency)); + pdu.FirstFrameArrivalTimestamp + Ticks(_options.P2StarClientMax)); } /// @@ -1324,7 +1350,7 @@ private async Task ReceiveWithTimeoutAsync(UdsServiceId servic notBefore, linkedToken).ConfigureAwait(false); } - using var timeoutCts = new CancellationTokenSource(remaining); + using var timeoutCts = CancelAfter(remaining); using var combined = CancellationTokenSource.CreateLinkedTokenSource( linkedToken, timeoutCts.Token); @@ -1347,7 +1373,9 @@ private async Task ReceiveWithTimeoutAsync(UdsServiceId servic // is still there. The channel publishes a First Frame when it is read off the bus and // withdraws it if the actor then refuses the frame -- with nothing put in the inbox -- so // a wait on it must not be unbounded. The re-check costs nothing when the PDU arrives: - // completion or abort puts an item in the inbox and the wait returns at once. + // completion or abort puts an item in the inbox and the wait returns at once. A real timer + // even on an injected clock: it polls the channel's state, it is no protocol deadline, and a + // test would otherwise have to advance its clock for a reception it is merely waiting on. private static readonly TimeSpan InProgressRecheck = TimeSpan.FromMilliseconds(50); /// @@ -1424,17 +1452,88 @@ private bool ResponseBeganInTime(UdsServiceId serviceId, TimeSpan budget, long b return false; } + // Now, on _time: Stopwatch.GetTimestamp() in production, the injected actor's clock in a + // test (#171). + private long Now() => _time.GetTimestamp(); + + // window in ticks of _time.Frequency, so a deadline noted from Now() and one computed here + // are on the same clock (#171). + private long Ticks(TimeSpan window) => SuppressedResponseWindows.Ticks(window, _time.Frequency); + + /// + /// A token source cancelled once has passed on : + /// a real timer in production, the injected actor's + /// timer in a test, so a deadline and the wait bounded by it are on one clock (#171). The + /// actor's timer fires on its loop; the cancellation is handed to the thread pool rather than + /// run there, because cancelling runs the channel's registrations, and whatever they resume + /// must not run on -- and block -- the loop the channel itself needs. + /// + private ClockTimeout CancelAfter(TimeSpan delay) => new(_clock, delay); + + // A wait of delay on _time, as CancelAfter measures it. + private async Task DelayAsync(TimeSpan delay, CancellationToken cancellationToken) + { + if (_clock is null) + { + await Task.Delay(delay, cancellationToken).ConfigureAwait(false); + return; + } + using var timeout = CancelAfter(delay); + using var combined = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeout.Token); + try + { + await Task.Delay(Timeout.Infinite, combined.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) when (timeout.IsCancellationRequested + && !cancellationToken.IsCancellationRequested) + { + // the delay ran out + } + } + + private sealed class ClockTimeout : IDisposable + { + private readonly CancellationTokenSource _cts; + private readonly IDisposable? _timer; + + public ClockTimeout(ProtocolActor? clock, TimeSpan delay) + { + if (clock is null) + { + _cts = new CancellationTokenSource(delay); + return; + } + _cts = new CancellationTokenSource(); + _timer = clock.Schedule(delay, () => ThreadPool.QueueUserWorkItem(static state => + { + try { ((CancellationTokenSource)state!).Cancel(); } + catch (ObjectDisposedException) { /* the wait ended first */ } + }, _cts)); + } + + public CancellationToken Token => _cts.Token; + + public bool IsCancellationRequested => _cts.IsCancellationRequested; + + public void Dispose() + { + _timer?.Dispose(); + _cts.Dispose(); + } + } + /// - /// Elapsed time between two readings, defaulting the - /// second to now. Kept in one place so the pre-check (how much budget is left) and the - /// post-check (was this PDU inside it) can never drift onto different clocks. + /// Elapsed time between two readings (or the injected + /// clock's equivalent), defaulting the second to now. Kept in one place so the pre-check + /// (how much budget is left) and the post-check (was this PDU inside it) can never drift + /// onto different clocks. /// - private static TimeSpan ElapsedSince(long startTimestamp, long? endTimestamp = null) + private TimeSpan ElapsedSince(long startTimestamp, long? endTimestamp = null) { - var end = endTimestamp ?? Stopwatch.GetTimestamp(); + var end = endTimestamp ?? Now(); var ticks = end - startTimestamp; if (ticks <= 0) return TimeSpan.Zero; - return TimeSpan.FromSeconds((double)ticks / Stopwatch.Frequency); + return TimeSpan.FromSeconds((double)ticks / _time.Frequency); } private void ThrowIfDisposed() From a6e699a1f98e20e9f4a7c8d2b56cfce4220bb1f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 21:48:45 +0000 Subject: [PATCH 2/8] test(uds): drive the suppressed-window and P2 tests on a virtual clock Five UDS tests move onto a ClockPair: the client's channel runs on a VirtualClock actor, and the ECU stays on the wall clock. Each test is a lockstep script: 1. The ECU holds each answer on a gate. 2. The test moves the clock to the instant the answer belongs to. 3. The test releases the gate and waits until the client's actor has taken the frame in. 4. The test waits for the exact interval the client arms, which distinguishes the correct client from each mutant. Converted: - A_Late_Negative_Response_To_A_Suppressed_Send_Is_Not_The_Next_Requests - Suppressed_Send_Windows_Are_Kept_Per_Service - A_Cancelled_Wait_Keeps_The_Rest_Of_The_Window - A_Pending_Answer_Consumed_As_Another_Requests_Stray_Still_Extends_Its_Window (red on macOS for #188) - P2_Ends_With_The_First_Frame_Of_A_MultiFrame_Response (red on macOS for #185 and #188). N_Cr now runs on the virtual clock, so real CF gaps can no longer expire it. P2 is made to expire mid-transfer by moving the clock once the First Frame is in. Mutation checks, each turning exactly its own test red: - no suppressed-window wait: all four window tests - a cancelled wait forgets its window: the cancelled-wait test - no routing of a stray 0x78: the pending-answer test - one window shared by all services: the per-service test - P2 measured against the last frame: the P2 test Supporting changes: - SimulatedUdsEcu gains a Delay hook. Its paced answers default to Task.Delay. - FrameConsumptionCountingBusService now also counts frames drained with TryRead, which is how the ISO-TP channel's pump reads. A frame counts as consumed once the reader asks for the next one. Refs #171 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s --- .../TestCases/J1939TpTests.cs | 20 +- .../TestCases/Uds/SimulatedUdsEcu.cs | 17 +- .../TestCases/Uds/UdsClientTests.cs | 322 +++++++++++++----- 3 files changed, 273 insertions(+), 86 deletions(-) diff --git a/tests/CanKit.Pro.Tests/TestCases/J1939TpTests.cs b/tests/CanKit.Pro.Tests/TestCases/J1939TpTests.cs index c9479c4..f9ac91a 100644 --- a/tests/CanKit.Pro.Tests/TestCases/J1939TpTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/J1939TpTests.cs @@ -2862,7 +2862,25 @@ private async IAsyncEnumerable Count( } } - public bool TryRead(out CanFrameEvent frameEvent) => _inner.TryRead(out frameEvent); + // The same meaning for a reader that drains with TryRead, as the ISO-TP channel's pump + // does: a frame is finished with once the reader asks for the next one -- by then it has + // been handed to the actor. Only one reader drains a subscription at a time (the + // channel's pump lock), so the frame in hand needs no lock of its own. + private CanFrameEvent _taken; + private bool _hasTaken; + + public bool TryRead(out CanFrameEvent frameEvent) + { + if (_hasTaken) + { + _hasTaken = false; + _owner.Consumed(_taken); + } + if (!_inner.TryRead(out frameEvent)) return false; + _taken = frameEvent; + _hasTaken = true; + return true; + } public ValueTask WaitToReadAsync(CancellationToken cancellationToken = default) => _inner.WaitToReadAsync(cancellationToken); diff --git a/tests/CanKit.Pro.Tests/TestCases/Uds/SimulatedUdsEcu.cs b/tests/CanKit.Pro.Tests/TestCases/Uds/SimulatedUdsEcu.cs index 4d8076f..8d53db1 100644 --- a/tests/CanKit.Pro.Tests/TestCases/Uds/SimulatedUdsEcu.cs +++ b/tests/CanKit.Pro.Tests/TestCases/Uds/SimulatedUdsEcu.cs @@ -55,6 +55,15 @@ public sealed class SimulatedUdsEcu : IDisposable /// Last request the ECU saw (or null). public byte[]? LastRequest => Volatile.Read(ref _lastRequest); + /// + /// How the ECU waits out the delays its sentinel exceptions ask for -- the pause before and + /// after a pending answer, the gap between 0x78s. Real time by default. A test on a virtual + /// clock replaces it with gates it opens itself, so each answer goes out at the virtual + /// instant the test has moved the clock to rather than whenever the host gets round to it + /// (#171). + /// + public Func Delay { get; set; } = (delay, ct) => Task.Delay(delay, ct); + /// The ECU's own ISO-TP channel, for a handler that puts something on the wire /// other than the response the loop would build for it. public IIsoTpChannel Channel => _channel; @@ -150,7 +159,7 @@ await SendAndCountAsync(new byte[] { 0x7F, sid, nrc.Code }, ct) // Pending NRCs are intermediate; only the final response counts. await SendNrcAsync(sid, 0x78, ct).ConfigureAwait(false); if (pending.DelayBetween > TimeSpan.Zero) - await Task.Delay(pending.DelayBetween, ct).ConfigureAwait(false); + await Delay(pending.DelayBetween, ct).ConfigureAwait(false); } if (ct.IsCancellationRequested) return; @@ -170,14 +179,14 @@ await SendAndCountAsync(new byte[] { 0x7F, sid, nrc.Code }, ct) try { if (pn.DelayBefore > TimeSpan.Zero) - await Task.Delay(pn.DelayBefore, ct).ConfigureAwait(false); + await Delay(pn.DelayBefore, ct).ConfigureAwait(false); for (int i = 0; i < pn.PendingCount && !ct.IsCancellationRequested; i++) { await SendNrcAsync(sid, 0x78, ct).ConfigureAwait(false); Interlocked.Increment(ref _pendingNrcsSent); } if (pn.DelayAfter > TimeSpan.Zero) - await Task.Delay(pn.DelayAfter, ct).ConfigureAwait(false); + await Delay(pn.DelayAfter, ct).ConfigureAwait(false); if (ct.IsCancellationRequested) return; await SendAndCountAsync(new byte[] { 0x7F, sid, pn.Nrc }, ct) .ConfigureAwait(false); @@ -194,7 +203,7 @@ await SendAndCountAsync(new byte[] { 0x7F, sid, pn.Nrc }, ct) { await SendNrcAsync(sid, 0x78, ct).ConfigureAwait(false); if (pendingSilent.DelayBetween > TimeSpan.Zero) - await Task.Delay(pendingSilent.DelayBetween, ct).ConfigureAwait(false); + await Delay(pendingSilent.DelayBetween, ct).ConfigureAwait(false); } // Then silence: the client's restarted P2* must expire (FR-UDS-008). } diff --git a/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs b/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs index efc94b2..27bb2d3 100644 --- a/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs @@ -73,6 +73,120 @@ private static (IUdsClient client, SimulatedUdsEcu ecu, IDisposable dispose) Bui return (client, ecu, dispose); } + /// + /// A client whose P2, P2* and suppressed-response windows are measured and waited out + /// on a , opposite a on the wall + /// clock (#171). Nothing a test asserts about those windows then depends on how the host + /// schedules: the client's waits end only when the test moves the clock, every frame is + /// stamped with the virtual instant it arrived at, and says when a + /// frame from the ECU has been handed to the client's actor. + /// + private sealed class ClockPair : IDisposable + { + private readonly IDisposable _stack; + + public ClockPair(Action configure, UdsClientOptions? options = null, + IsoTpChannelOptions? clientIsoTp = null) + { + var session = NewSession(); + var busClient = OpenClassic(session, 0); + var busEcu = OpenClassic(session, 1); + + // A zero stamp means "unstamped" to the channel, so the clock starts off zero. + Clock.Advance(TimeSpan.FromMilliseconds(1)); + Actor = Clock.NewActor(); + Service = new FrameConsumptionCountingBusService( + new CanBusService(busClient, Actor.TimeSource.GetTimestamp)); + Channel = new IsoTpChannel(Service, IsoTpEndpoint.Normal(txCanId: 0x7E0, rxCanId: 0x7E8), + clientIsoTp ?? FastIsoTp(useCanFd: false), ownsService: true, Actor); + var ecuChannel = IsoTpFactory.Open(busEcu, IsoTpEndpoint.Normal(txCanId: 0x7E8, rxCanId: 0x7E0), + FastIsoTp(useCanFd: false)); + + Ecu = new SimulatedUdsEcu(ecuChannel); + configure(Ecu); + Ecu.Start(); + Client = UdsClient.Create(Channel, Actor, options); + _stack = new CompositeDisposable(Client, Ecu, ecuChannel, Channel, busEcu, busClient); + } + + public VirtualClock Clock { get; } = new(); + + public ProtocolActor Actor { get; } + + public FrameConsumptionCountingBusService Service { get; } + + public IsoTpChannel Channel { get; } + + public SimulatedUdsEcu Ecu { get; } + + public IUdsClient Client { get; } + + /// + /// Returns once the client has armed a timer exactly away: + /// the wait the test is about to end by moving the clock, and the one a client waiting on + /// something else would not have armed. + /// + public Task WaitUntilWaitingAsync(TimeSpan remaining) + => Clock.WaitUntilTimerArmedAsync(Actor, remaining, ShortTimeout); + + /// + /// Runs -- whatever makes the ECU answer -- and returns once + /// the client's actor has taken in the frame it sends, a Single Frame matching + /// . Its stamp is then the instant the clock stood at, and the + /// clock may be moved again. + /// + public async Task DeliverAsync(Action release, Func match) + { + var taken = Service.WaitUntilConsumedAsync(e => + e.Frame.ID == 0x7E8 && match(e.Frame.Data.ToArray())); + release(); + if (await Task.WhenAny(taken, Task.Delay(ShortTimeout)) != taken) + throw new TimeoutException("The ECU's frame did not reach the client."); + await Clock.SettleAsync(); + } + + public void Dispose() + { + _stack.Dispose(); + Clock.Dispose(); + } + } + + // The task's result, or a timeout rather than a hang when it never comes. + private static async Task Within(Task task) + { + if (await Task.WhenAny(task, Task.Delay(ShortTimeout)) != task) + throw new TimeoutException($"No result within {ShortTimeout}."); + return await task; + } + + // A Single Frame carrying a negative response [0x7F, sid, nrc]. + private static Func Negative(byte sid, byte nrc) + => data => data.Length >= 4 && data[1] == 0x7F && data[2] == sid && data[3] == nrc; + + /// + /// Stands in for : each delay the ECU asks for waits until + /// the test releases that delay by its length, after moving the clock to where it ends. + /// + private sealed class EcuSteps + { + private readonly Dictionary> _gates = new(); + + public Task WaitAsync(TimeSpan delay, CancellationToken _) => Gate(delay).Task; + + public void Release(TimeSpan delay) => Gate(delay).TrySetResult(true); + + private TaskCompletionSource Gate(TimeSpan delay) + { + lock (_gates) + { + if (!_gates.TryGetValue(delay, out var gate)) + _gates[delay] = gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + return gate; + } + } + } + private static ICanBus OpenCanFd(string session, int channel) => CanBus.Open( $"virtual://{session}/{channel}", cfg => cfg.SetProtocolMode(CanProtocolMode.CanFd).Fd(VirtualAdapterFixture.Bitrate, VirtualAdapterFixture.DataBitrate)); @@ -358,15 +472,16 @@ await client.SecurityAccessAsync( [Fact] public async Task P2_Ends_With_The_First_Frame_Of_A_MultiFrame_Response() { - // 90-byte response = FF (6 data bytes) + 12 CFs. The client advertises STmin = 127 ms, - // so the ECU cannot deliver the last CF earlier than 12 x 127 ms = 1.5 s after the - // First Frame -- three times the P2 budget below. The First Frame itself is answered - // at once; the quantity the host perturbs is that single round trip, against a 500 ms - // budget the suite's other P2 tests already trust at 80 ms. + // 90-byte response = FF (6 data bytes) + 12 CFs, paced by the client's advertised STmin + // of 127 ms: the ECU needs 1.5 s of real time to deliver it. On a virtual clock (#171) + // the client's P2 is made to expire in the middle of that transfer -- the clock is moved + // past P2 once the First Frame is in -- and its N_Cr, on the same clock, never can. On + // the wall clock the transfer was a race between 12 real CF gaps and a 500 ms N_Cr, + // which macOS CI lost (#185, #188). var record = Enumerable.Range(0, 87).Select(i => (byte)i).ToArray(); var p2 = TimeSpan.FromMilliseconds(500); - var (client, _, dispose) = BuildPair( + using var pair = new ClockPair( e => e.On(0x22, req => { var body = new byte[2 + record.Length]; @@ -382,22 +497,26 @@ public async Task P2_Ends_With_The_First_Frame_Of_A_MultiFrame_Response() UsePadding = true, NAs = TimeSpan.FromMilliseconds(500), NBs = TimeSpan.FromMilliseconds(500), - NCr = TimeSpan.FromMilliseconds(500), + NCr = TimeSpan.FromSeconds(10), LocalStMin = TimeSpan.FromMilliseconds(127), }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - var sw = Stopwatch.StartNew(); - var data = await client.ReadDataByIdentifierAsync(0xF190, cts.Token); - sw.Stop(); + using var cts = new CancellationTokenSource(ShortTimeout); + var read = pair.Client.ReadDataByIdentifierAsync(0xF190, cts.Token); - data.Should().Equal(record); - // The transfer really outlasted P2 -- otherwise the assertion above would hold for - // a client that still measures P2 against the last frame. - sw.Elapsed.Should().BeGreaterThan(p2); + var deadline = DateTime.UtcNow + ShortTimeout; + while (pair.Channel.GetReceptionsInProgress().Count == 0) + { + if (DateTime.UtcNow > deadline) throw new TimeoutException("The First Frame never arrived."); + await Task.Delay(1); } + await pair.Clock.AdvanceAsync(p2 + TimeSpan.FromMilliseconds(100)); // P2 runs out mid-transfer + // The transfer really outlasted P2: otherwise the result below would hold for a client + // that still measures P2 against the last frame. 1.4 s of CF gaps are still to come. + pair.Channel.GetReceptionsInProgress().Should().NotBeEmpty(); + + var data = await Within(read); + data.Should().Equal(record); } // ----------------------------------------------------------------------------------- @@ -512,28 +631,36 @@ public async Task A_Contended_Lock_Needs_No_Subscriber() [Fact] public async Task A_Late_Negative_Response_To_A_Suppressed_Send_Is_Not_The_Next_Requests() { - var (client, ecu, dispose) = BuildPair( + // On a virtual clock (#171): the ECU holds its negative answer until the test lets it go, + // so "late" is an instant on the clock rather than a sleep that must land on the right + // side of the next request's send. + using var answer = new ManualResetEventSlim(); + using var pair = new ClockPair( e => e.On(0x3E, req => { if ((req[1] & 0x80) != 0) { - Thread.Sleep(100); // late ... + answer.Wait(ShortTimeout); // late ... throw new EcuNegativeResponse(0x12); // ... and negative, to the suppressed one } return new byte[] { 0x00 }; }), options: new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(300) }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); - - // Follows at once; the ECU's negative answer to the suppressed send is still coming. - Func act = () => client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); - await act.Should().NotThrowAsync("the negative response belongs to the suppressed send"); - ecu.RequestsHandled.Should().Be(2); - } + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // its window: 300 ms + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(100)); + + var next = pair.Client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); + // The next request waits out what is left of the window -- 200 ms -- rather than going + // out and arming a fresh P2 of 300. + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(200)); + await pair.DeliverAsync(answer.Set, Negative(0x3E, 0x12)); // at 100 ms + + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(200)); // the window closes + Func act = () => next; + await act.Should().NotThrowAsync("the negative response belongs to the suppressed send"); + pair.Ecu.RequestsHandled.Should().Be(2); } // Codex and Bugbot on #150: the windows are per service. A suppressed send for another @@ -541,13 +668,14 @@ public async Task A_Late_Negative_Response_To_A_Suppressed_Send_Is_Not_The_Next_ [Fact] public async Task Suppressed_Send_Windows_Are_Kept_Per_Service() { - var (client, ecu, dispose) = BuildPair( + using var answer = new ManualResetEventSlim(); + using var pair = new ClockPair( e => e .On(0x3E, req => { if ((req[1] & 0x80) != 0) { - Thread.Sleep(100); + answer.Wait(ShortTimeout); throw new EcuNegativeResponse(0x12); } return new byte[] { 0x00 }; @@ -555,16 +683,21 @@ public async Task Suppressed_Send_Windows_Are_Kept_Per_Service() .On(0x11, req => Array.Empty()), options: new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(300) }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // its window opens - await client.SendRawAsync(new byte[] { 0x11, 0x81 }, cts.Token); // another service's - - Func act = () => client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); - await act.Should().NotThrowAsync("the TesterPresent window is still open, whatever came after"); - ecu.RequestsHandled.Should().Be(3); - } + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // its window: to 300 ms + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(50)); + await pair.Client.SendRawAsync(new byte[] { 0x11, 0x81 }, cts.Token); // another service's: to 350 ms + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(50)); + + var next = pair.Client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); + // TesterPresent's own window, 200 ms from here -- not the later one's 250, and not none. + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(200)); + await pair.DeliverAsync(answer.Set, Negative(0x3E, 0x12)); + + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(200)); + Func act = () => next; + await act.Should().NotThrowAsync("the TesterPresent window is still open, whatever came after"); + pair.Ecu.RequestsHandled.Should().Be(3); } // Codex on #150: NRC 0x78 to a suppressed send says the final answer is still coming, up @@ -884,32 +1017,40 @@ public async Task A_Stale_Pending_Answer_Queued_Before_A_Suppressed_Send_Does_No [Fact] public async Task A_Cancelled_Wait_Keeps_The_Rest_Of_The_Window() { - var (client, ecu, dispose) = BuildPair( + using var answer = new ManualResetEventSlim(); + using var pair = new ClockPair( e => e.On(0x3E, req => { if ((req[1] & 0x80) != 0) { - Thread.Sleep(200); + answer.Wait(ShortTimeout); throw new EcuNegativeResponse(0x12); } return new byte[] { 0x00 }; }), options: new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(400) }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); - - using var early = new CancellationTokenSource(TimeSpan.FromMilliseconds(50)); - Func cancelled = () => client.TesterPresentAsync(suppressPositiveResponse: false, early.Token); - await cancelled.Should().ThrowAsync(); + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // its window: 400 ms + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(50)); - // The negative at 200 ms is still coming; the window must still be honoured. - Func act = () => client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); - await act.Should().NotThrowAsync("the remaining window survived the cancelled wait"); - ecu.RequestsHandled.Should().Be(2); - } + using var early = new CancellationTokenSource(); + var cancelled = pair.Client.TesterPresentAsync(suppressPositiveResponse: false, early.Token); + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(350)); // waiting out the window + early.Cancel(); + Func cancel = () => cancelled; + await cancel.Should().ThrowAsync(); + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(50)); + + // The negative is still coming; the rest of the window -- 300 ms -- must still be honoured. + var next = pair.Client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(300)); + await pair.DeliverAsync(answer.Set, Negative(0x3E, 0x12)); + + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(300)); + Func act = () => next; + await act.Should().NotThrowAsync("the remaining window survived the cancelled wait"); + pair.Ecu.RequestsHandled.Should().Be(2); } // Codex on #150: a 0x78 for service B, heard while waiting out service A's window, moves @@ -957,37 +1098,56 @@ public async Task A_Pending_Answer_For_Another_Service_Heard_During_A_Wait_Exten [Fact] public async Task A_Pending_Answer_Consumed_As_Another_Requests_Stray_Still_Extends_Its_Window() { - var (client, _, dispose) = BuildPair( - e => e - .On(0x11, req => - { - if ((req[1] & 0x80) != 0) - throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, - delayBefore: TimeSpan.FromMilliseconds(100), delayAfter: TimeSpan.FromMilliseconds(650)); - Thread.Sleep(400); // A's request, out at 600 ms without the routing, is answered at 1000: the stale negative at 750 is first in line - return new byte[] { 0x01 }; - }) - .On(0x22, req => - { - Thread.Sleep(200); // B's request is on the wire while A's 0x78 arrives - return new byte[] { 0xF1, 0x90, 0xAA }; - }), + // On a virtual clock (#171), each ECU answer released at its instant: the 400 ms and + // 200 ms sleeps that placed them before were a 200 ms margin a loaded runner overran. + var steps = new EcuSteps(); + using var answerB = new ManualResetEventSlim(); + var pendingAt = TimeSpan.FromMilliseconds(100); // A's 0x78, while B's request is out + var negativeAt = TimeSpan.FromMilliseconds(650); // A's final negative, 650 ms after it + using var pair = new ClockPair( + e => + { + e.Delay = steps.WaitAsync; + e.On(0x11, req => + { + if ((req[1] & 0x80) != 0) + throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, + delayBefore: pendingAt, delayAfter: negativeAt); + return new byte[] { 0x01 }; + }) + .On(0x22, req => + { + answerB.Wait(ShortTimeout); // B's request is on the wire while A's 0x78 arrives + return new byte[] { 0xF1, 0x90, 0xAA }; + }); + }, options: new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(600), P2StarClientMax = TimeSpan.FromMilliseconds(2000), }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - await client.SendRawAsync(new byte[] { 0x11, 0x81 }, cts.Token); // A, suppressed: 0x78 at 100 ms, negative at 750 ms - await client.ReadDataByIdentifierAsync(0xF190, cts.Token); // B, another service, consumes A's 0x78 as a stray - - // A's request follows: its window must reach past 750 ms. - var reset = await client.SendRawAsync(new byte[] { 0x11, 0x01 }, cts.Token); - reset.Should().Equal(0x51, 0x01); - } + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x11, 0x81 }, cts.Token); // A, suppressed: its window to 600 ms + var b = pair.Client.ReadDataByIdentifierAsync(0xF190, cts.Token); // B, another service + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(600)); // B's P2 + + await pair.Clock.AdvanceAsync(pendingAt); + await pair.DeliverAsync(() => steps.Release(pendingAt), Negative(0x11, 0x78)); // B consumes it as a stray + answerB.Set(); + await Within(b); + + // A's request follows: its window now reaches P2* past the 0x78 -- 2100 ms, 2000 from here. + // Without the routing it would end at 600 ms, before A's negative at 750 is on the wire. + var a = pair.Client.SendRawAsync(new byte[] { 0x11, 0x01 }, cts.Token); + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(2000)); + + await pair.Clock.AdvanceAsync(negativeAt); + await pair.DeliverAsync(() => steps.Release(negativeAt), Negative(0x11, 0x12)); // at 750 ms + + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(1350)); // the window closes at 2100 + var reset = await Within(a); + reset.Should().Equal(0x51, 0x01); } // Codex on #150: a 0x78 for service A that arrives after A's window has run out answers From b802ff657cc08406745a03fc10e599048651acbd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 22:00:31 +0000 Subject: [PATCH 3/8] refactor(uds): leave the actor's timeout source undisposed On the actor's clock, the source a timer cancels holds nothing that needs disposing. Leaving it undisposed means a cancellation the timer has already handed to the thread pool can no longer find it disposed, so the exception that case needed caught is gone. The busy-repeat delay now waits on the clock with a timer and a completion source, which is the helper UdsFunctionalClient already uses. Refs #171 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s --- src/CanKit.Pro.Uds/UdsClientImpl.cs | 38 ++++++++++++----------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/src/CanKit.Pro.Uds/UdsClientImpl.cs b/src/CanKit.Pro.Uds/UdsClientImpl.cs index b613f43..3449b09 100644 --- a/src/CanKit.Pro.Uds/UdsClientImpl.cs +++ b/src/CanKit.Pro.Uds/UdsClientImpl.cs @@ -1471,24 +1471,16 @@ private bool ResponseBeganInTime(UdsServiceId serviceId, TimeSpan budget, long b private ClockTimeout CancelAfter(TimeSpan delay) => new(_clock, delay); // A wait of delay on _time, as CancelAfter measures it. - private async Task DelayAsync(TimeSpan delay, CancellationToken cancellationToken) + private Task DelayAsync(TimeSpan delay, CancellationToken cancellationToken) + => _clock is null ? Task.Delay(delay, cancellationToken) : WaitOnClockAsync(_clock, delay, cancellationToken); + + private static async Task WaitOnClockAsync(ProtocolActor clock, TimeSpan delay, CancellationToken cancellationToken) { - if (_clock is null) - { - await Task.Delay(delay, cancellationToken).ConfigureAwait(false); - return; - } - using var timeout = CancelAfter(delay); - using var combined = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeout.Token); - try - { - await Task.Delay(Timeout.Infinite, combined.Token).ConfigureAwait(false); - } - catch (OperationCanceledException) when (timeout.IsCancellationRequested - && !cancellationToken.IsCancellationRequested) - { - // the delay ran out - } + var done = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var registration = cancellationToken.Register(static state => + ((TaskCompletionSource)state!).TrySetCanceled(), done); + using var handle = clock.Schedule(delay, () => done.TrySetResult(true)); + await done.Task.ConfigureAwait(false); } private sealed class ClockTimeout : IDisposable @@ -1505,20 +1497,20 @@ public ClockTimeout(ProtocolActor? clock, TimeSpan delay) } _cts = new CancellationTokenSource(); _timer = clock.Schedule(delay, () => ThreadPool.QueueUserWorkItem(static state => - { - try { ((CancellationTokenSource)state!).Cancel(); } - catch (ObjectDisposedException) { /* the wait ended first */ } - }, _cts)); + ((CancellationTokenSource)state!).Cancel(), _cts)); } public CancellationToken Token => _cts.Token; public bool IsCancellationRequested => _cts.IsCancellationRequested; + // On the actor's clock only the timer is released: a source without a timer of its own + // holds nothing that needs it, and one whose cancellation the timer has already handed + // to the thread pool must stay cancellable rather than throw there. public void Dispose() { - _timer?.Dispose(); - _cts.Dispose(); + if (_timer is null) _cts.Dispose(); + else _timer.Dispose(); } } From 0e1e867b796e26a692acb78b49a63fc87ba40856 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 01:01:04 +0000 Subject: [PATCH 4/8] refactor(actor): add an internal ScheduleAt for fixed deadlines Schedule(delay) measures the delay from a clock reading it takes itself. A caller whose deadline is already fixed has to turn it into a delay from its own, earlier reading. If the clock moves in between, the timer lands late by that whole move. On a VirtualClock a test advances in steps, so the timer can end up past the last advance and never fire. The internal ScheduleAt(dueTimestamp, callback) arms the instant as it is. Schedule and ScheduleAt share one insertion path. The public API is unchanged. The new test was mutation-checked: making ScheduleAt land late by the clock's move turns it red. Refs #171 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s --- src/CanKit.Pro.Actor/ProtocolActor.cs | 20 ++++++++++++- .../TestCases/ProtocolActorTimerTests.cs | 30 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/CanKit.Pro.Actor/ProtocolActor.cs b/src/CanKit.Pro.Actor/ProtocolActor.cs index f48a233..5591fe2 100644 --- a/src/CanKit.Pro.Actor/ProtocolActor.cs +++ b/src/CanKit.Pro.Actor/ProtocolActor.cs @@ -345,7 +345,25 @@ public IDisposable Schedule(TimeSpan delay, Action callback) if (callback is null) throw new ArgumentNullException(nameof(callback)); if (delay < TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(delay), "Delay must not be negative."); - var entry = new TimerEntry(DueTimestamp(delay), callback); + return Insert(new TimerEntry(DueTimestamp(delay), callback)); + } + + /// + /// As , due at on + /// rather than a delay from the reading this call takes. For a + /// caller whose deadline is fixed already: a delay computed from its own earlier reading + /// lands late by however far the clock moved in between, and on a clock a test moves in + /// steps that can be a whole step -- a timer that then never fires (#171). A due instant + /// already past fires on the loop's next pass. + /// + internal IDisposable ScheduleAt(long dueTimestamp, Action callback) + { + if (callback is null) throw new ArgumentNullException(nameof(callback)); + return Insert(new TimerEntry(dueTimestamp, callback)); + } + + private IDisposable Insert(TimerEntry entry) + { lock (_disposeGate) { ThrowIfDisposed(); diff --git a/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTimerTests.cs b/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTimerTests.cs index 965f71f..44dbd86 100644 --- a/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTimerTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/ProtocolActorTimerTests.cs @@ -213,4 +213,34 @@ public async Task A_Clean_Dispose_Reports_Nothing() observed.Should().BeNull(); } + + // #171: a caller whose deadline is fixed already arms it as that instant. Armed as a delay + // from a fresh reading instead, a clock that moved after the caller's own reading would put + // the timer late by the whole move -- on a clock a test moves, a timer that never fires. + [Fact] + public async Task ScheduleAt_Fires_At_The_Instant_Not_A_Delay_From_When_It_Was_Armed() + { + using var clock = new VirtualClock(); + var actor = clock.NewActor(); + var time = actor.TimeSource; + long Ms(int ms) => (long)(ms / 1000.0 * time.Frequency); + + var deadline = time.GetTimestamp() + Ms(50); // the caller's reading, and its deadline + await clock.AdvanceAsync(TimeSpan.FromMilliseconds(30)); // the clock moves before the arming + var fired = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var handle = actor.ScheduleAt(deadline, () => fired.TrySetResult(true)); + + await clock.AdvanceAsync(TimeSpan.FromMilliseconds(19)); + fired.Task.IsCompleted.Should().BeFalse("the deadline is still 1 ms away"); + await clock.AdvanceAsync(TimeSpan.FromMilliseconds(1)); + fired.Task.IsCompleted.Should().BeTrue("due 50 ms after the caller's reading, not 50 ms after the arming"); + + var late = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var past = actor.ScheduleAt(deadline, () => late.TrySetResult(true)); + await clock.SettleAsync(); + late.Task.IsCompleted.Should().BeTrue("an instant already past fires on the loop's next pass"); + + Action noCallback = () => actor.ScheduleAt(deadline, null!); + noCallback.Should().Throw(); + } } From 8068815aabd6bce80d19548c50dd1247328d74ea Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 01:01:05 +0000 Subject: [PATCH 5/8] refactor(uds): arm the P2 and window timeouts at their deadlines On the injected clock, the P2/P2* receive timeout and the suppressed-window slices were armed as "remaining from now". The remaining time was computed on a thread-pool continuation, and the actor measured it again from its own reading. When a test moved the clock between the two readings, the timer was armed that much too late and never fired. This happened under load in A_Cancelled_Wait_Keeps_The_Rest_Of_The_Window: 2 hangs in 40 runs alongside a concurrent test process. Both timeouts are now armed at the deadline itself: budgetStart + P2 for the receive, and the window's own end for a slice. They go through ProtocolActor.ScheduleAt. After the change the same stress passed 60/60. The production path without an injected clock is unchanged: a real CancellationTokenSource timer for the remaining time. Refs #171 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s --- src/CanKit.Pro.Uds/UdsClientImpl.cs | 38 ++++++++++++++++------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/CanKit.Pro.Uds/UdsClientImpl.cs b/src/CanKit.Pro.Uds/UdsClientImpl.cs index 3449b09..f464491 100644 --- a/src/CanKit.Pro.Uds/UdsClientImpl.cs +++ b/src/CanKit.Pro.Uds/UdsClientImpl.cs @@ -568,7 +568,7 @@ private async Task WaitOutSuppressedResponseWindowAsync(UdsServiceId serviceId, if (DrainExtends(sid, ref until)) continue; break; } - using var slice = CancelAfter(remaining); + using var slice = CancelAt(until); using var combined = CancellationTokenSource.CreateLinkedTokenSource(linkedToken, slice.Token); IsoTpReceivedPdu pdu; try @@ -1350,7 +1350,7 @@ private async Task ReceiveWithTimeoutAsync(UdsServiceId servic notBefore, linkedToken).ConfigureAwait(false); } - using var timeoutCts = CancelAfter(remaining); + using var timeoutCts = CancelAt(budgetStart + Ticks(budget)); using var combined = CancellationTokenSource.CreateLinkedTokenSource( linkedToken, timeoutCts.Token); @@ -1461,16 +1461,22 @@ private bool ResponseBeganInTime(UdsServiceId serviceId, TimeSpan budget, long b private long Ticks(TimeSpan window) => SuppressedResponseWindows.Ticks(window, _time.Frequency); /// - /// A token source cancelled once has passed on : - /// a real timer in production, the injected actor's - /// timer in a test, so a deadline and the wait bounded by it are on one clock (#171). The - /// actor's timer fires on its loop; the cancellation is handed to the thread pool rather than - /// run there, because cancelling runs the channel's registrations, and whatever they resume - /// must not run on -- and block -- the loop the channel itself needs. + /// A token source cancelled once reaches : a + /// real timer in production, the injected actor's + /// timer in a test, so a deadline and the wait bounded by it are on one clock (#171). On the + /// actor the deadline is armed as the instant it is, not as a delay from a fresh reading -- + /// a test that moves the clock between this client's reading and the arming would otherwise + /// push the timer past the deadline, and it would never fire. The actor's timer fires on its + /// loop; the cancellation is handed to the thread pool rather than run there, because + /// cancelling runs the channel's registrations, and whatever they resume must not run on -- + /// and block -- the loop the channel itself needs. /// - private ClockTimeout CancelAfter(TimeSpan delay) => new(_clock, delay); + private ClockTimeout CancelAt(long deadline) + => _clock is null + ? new ClockTimeout(SuppressedResponseWindows.Remaining(deadline, Now(), _time.Frequency)) + : new ClockTimeout(_clock, deadline); - // A wait of delay on _time, as CancelAfter measures it. + // A wait of delay on _time, as CancelAt measures it. private Task DelayAsync(TimeSpan delay, CancellationToken cancellationToken) => _clock is null ? Task.Delay(delay, cancellationToken) : WaitOnClockAsync(_clock, delay, cancellationToken); @@ -1488,15 +1494,13 @@ private sealed class ClockTimeout : IDisposable private readonly CancellationTokenSource _cts; private readonly IDisposable? _timer; - public ClockTimeout(ProtocolActor? clock, TimeSpan delay) + public ClockTimeout(TimeSpan remaining) + => _cts = new CancellationTokenSource(remaining); + + public ClockTimeout(ProtocolActor clock, long deadline) { - if (clock is null) - { - _cts = new CancellationTokenSource(delay); - return; - } _cts = new CancellationTokenSource(); - _timer = clock.Schedule(delay, () => ThreadPool.QueueUserWorkItem(static state => + _timer = clock.ScheduleAt(deadline, () => ThreadPool.QueueUserWorkItem(static state => ((CancellationTokenSource)state!).Cancel(), _cts)); } From 63057e373f84ec6f2ef15e7c1b6618a37cd00279 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 00:46:34 +0000 Subject: [PATCH 6/8] test(uds): finish moving the P2/P2*/suppressed-window tests onto the virtual clock Converts the remaining fourteen P2/P2*/suppressed-window tests in UdsClientTests.cs from wall-clock pacing (Thread.Sleep/Task.Delay/Stopwatch bounds) onto the ClockPair pattern introduced by a6e699a, or onto a bare VirtualClock actor for the two that use a StarvedReaderBusService double instead of SimulatedUdsEcu. Two small helpers were added: OrderedGates (an EcuSteps sibling that gates SimulatedUdsEcu.Delay calls by order rather than by duration, for scenarios that reuse the same delay length more than once) and a non-generic Within(Task) overload. Converted, with each one's mutation and the observed result (both original and converted red on it, verified before every revert; `git diff src/` was empty before this commit): - P2_Is_Not_Extended_By_A_MultiFrame_Transfer_For_Another_Service Mutation: ResponseBeganInTime (UdsClientImpl.cs) drops the response-SID check, so any in-progress reception -- not just this request's own -- extends the wait. Original: BeLessThan(1500ms) failed at ~2.6s. Converted: initially passed (the dropped real-time bound had made the exception-type-only assertion insufficient, since the mutant also eventually throws UdsTimeoutException once the ECU's STmin-paced, real-time transfer finishes). A later commit on this branch replaces the wall-clock bound this was first fixed with by a frozen transfer the mutant can only escape by cancellation. - A_Pending_Answer_To_A_Suppressed_Send_Extends_Its_Window_By_P2Star - A_Queued_Pending_Answer_Still_Extends_A_Window_That_Has_Run_Out Mutation: ExtendOnPending's own-service branch (UdsClientImpl.cs) never extends (`return false` unconditionally). Both: original failed on the request count / the elapsed-time floor; converted failed with WaitUntilTimerArmedAsync unable to find the extended interval ("the earliest is none"). - A_Queued_Pending_Answer_From_After_The_Windows_End_Does_Not_Revive_It - 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 Mutation: ExtendOnPending's own-service guard (UdsClientImpl.cs) drops the "arrived after the window's own end" check, so a late 0x78 always revives it. Original: BeLessThan(1s) failed at ~1.4-1.8s. Converted: hung past its own clock choreography and was cancelled by the caller's token (OperationCanceledException). - A_Pending_Answer_For_Another_Service_Heard_During_A_Wait_Extends_That_Services_Window Mutation: ExtendOnPending's another-service branch (UdsClientImpl.cs) is a no-op. Original failed with a UdsNegativeResponseException (the stale negative was taken as the reset request's own answer); converted failed on WaitUntilTimerArmedAsync. - A_Pending_Answer_Still_On_Its_Way_Through_The_Channel_Extends_The_Window Mutation: the zero-remaining branch of WaitOutSuppressedResponseWindowAsync (UdsClientImpl.cs) skips SettleAsync() before deciding the inbox is empty. Original: BeGreaterThanOrEqualTo(600ms) failed at ~280ms. Converted: threw UdsTimeoutException too early (the window closed before WaitUntilTimerArmedAsync ever saw the extended interval, so the second wait step failed). - A_Suppressed_Send_The_Channel_Refuses_Leaves_No_Window Mutation: SendWithoutResponseAsync's RefusedBeforeTransmission catch (UdsClientImpl.cs) no longer calls SuppressedResponseWindows.Restore, leaving the provisional window in place. Original: BeLessThan(1s) failed at ~2s. Converted: hung waiting out the leaked window and was cancelled by Within's own timeout. - A_Suppressed_Send_The_Channel_Refuses_Leaves_An_Earlier_Window_As_It_Was Mutation: SuppressedResponseWindows.Restore ignores `had` and always removes the window. Original: BeGreaterThanOrEqualTo(1900ms) failed at ~54ms. Converted: WaitUntilTimerArmedAsync never saw the 2s window (the earlier window was wiped). Each of the two Restore mutations was checked against both tests to confirm it broke only the one it targets. - A_Pending_Answer_Still_On_Its_Way_Is_Routed_By_An_Aborted_Requests_Discard Mutation: RouteStrayPending (UdsClientImpl.cs) never extends any window. Original: BeGreaterThanOrEqualTo(600ms) failed at ~199ms. Converted: WaitUntilTimerArmedAsync never saw the extension. (A first attempt mutated DiscardStalePdusAsync's own SettleAsync call, which the original test caught but the converted one did not -- DiscardPendingPdus's own internal pump made that call redundant on the virtual clock. RouteStrayPending is the mutation that isolates the property the test actually names.) - A_Stale_Pending_Answer_Queued_Before_A_Suppressed_Send_Does_Not_Extend_Its_Window Mutation: SendWithoutResponseAsync's pre-send discard call (UdsClientImpl.cs) is skipped. Original: BeLessThan(1s) failed at ~2s. Converted: WaitUntilTimerArmedAsync never saw the un-extended (100 ms) interval it expects. - Client_Times_Out_With_P2Star_When_Ecu_Sends_Only_ResponsePending - ResponsePending_Restarts_P2Star_And_Returns_Final_Response Mutation: the NRC-0x78 branch of ExchangeOnceAsync (UdsClientImpl.cs) no longer resets budgetStart/notBefore/timeout/timerKind, so the client never leaves its initial P2 budget. Original: Timer assertion / P2 timeout instead of a positive result. Converted: WaitUntilTimerArmedAsync never saw the restarted P2* interval. Stress-testing this change surfaced a hang in A_Cancelled_Wait_Keeps_The_Rest_Of_The_Window and A_Pending_Answer_Consumed_As_Another_Requests_Stray_Still_Extends_Its_Window. Both tests were converted earlier on this branch, so the defect is this branch's own: the injected clock's timeouts were armed as a delay from a fresh reading and could land past the test's last advance. It is fixed on this branch by arming them at their deadlines (ProtocolActor.ScheduleAt). Left unconverted, with reasons: - TimedOut_Request_Does_Not_Poison_Next_Same_Service_Transaction, ResponsePending_Loop_Aborts_When_Exceeding_MaxResponsePendingCount: excluded by the task (not P2/P2*/window timing, or already deliberately real-clock). Refs #171 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s --- .../TestCases/Uds/UdsClientTests.cs | 833 +++++++++++------- 1 file changed, 522 insertions(+), 311 deletions(-) diff --git a/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs b/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs index 27bb2d3..4f3e4aa 100644 --- a/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs @@ -160,6 +160,16 @@ private static async Task Within(Task task) return await task; } + // As Within, for a task with no result -- a wait that hangs (a suppressed-window bug + // left waiting forever on a clock nobody advances further) surfaces as a timeout here + // rather than the test itself hanging. + private static async Task Within(Task task) + { + if (await Task.WhenAny(task, Task.Delay(ShortTimeout)) != task) + throw new TimeoutException($"No result within {ShortTimeout}."); + await task; + } + // A Single Frame carrying a negative response [0x7F, sid, nrc]. private static Func Negative(byte sid, byte nrc) => data => data.Length >= 4 && data[1] == 0x7F && data[2] == sid && data[3] == nrc; @@ -187,6 +197,50 @@ private TaskCompletionSource Gate(TimeSpan delay) } } + /// + /// Stands in for like , but gates + /// by call order rather than by the delay's length: needed whenever a scenario asks for the + /// same duration more than once (two 0x78s spaced by the same gap), where + /// would hand both callers the same, already-resolved gate. + /// + private sealed class OrderedGates + { + private readonly Queue> _waiters = new(); + private readonly object _gate = new(); + private int _pendingReleases; + + // Symmetric with EcuSteps' own race-freedom (see its remarks): a ReleaseNext() that + // arrives before the matching WaitAsync -- the ECU has not yet reached its gate on its + // own thread -- must not be lost. It is banked instead, so the next WaitAsync call + // returns at once. + public Task WaitAsync(TimeSpan _, CancellationToken ct) + { + lock (_gate) + { + if (_pendingReleases > 0) + { + _pendingReleases--; + return Task.CompletedTask; + } + + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _waiters.Enqueue(tcs); + return tcs.Task.WaitAsync(ct); + } + } + + public void ReleaseNext() + { + TaskCompletionSource? tcs = null; + lock (_gate) + { + if (_waiters.Count > 0) tcs = _waiters.Dequeue(); + else _pendingReleases++; + } + tcs?.TrySetResult(true); + } + } + private static ICanBus OpenCanFd(string session, int channel) => CanBus.Open( $"virtual://{session}/{channel}", cfg => cfg.SetProtocolMode(CanProtocolMode.CanFd).Fd(VirtualAdapterFixture.Bitrate, VirtualAdapterFixture.DataBitrate)); @@ -528,14 +582,18 @@ public async Task P2_Ends_With_The_First_Frame_Of_A_MultiFrame_Response() public async Task P2_Is_Not_Extended_By_A_MultiFrame_Transfer_For_Another_Service() { // The ECU answers the RDBI request with silence, but first starts a 146-byte - // ReadDTCInformation response (SID 0x59): FF + 20 CFs at the client's STmin of 127 ms, - // i.e. 2.5 s on the wire. The client must time out at P2 (500 ms), not after the - // transfer; the bound below leaves 1 s for the host between the two. + // ReadDTCInformation response (SID 0x59): FF + 20 CFs at the client's STmin of 127 ms. + // On a virtual clock (#171) the transfer is proven still in progress by + // GetReceptionsInProgress() rather than by outrunning a real N_Cr; P2 is then moved + // past on the clock and must fire regardless -- the sibling of + // P2_Ends_With_The_First_Frame_Of_A_MultiFrame_Response, whose in-progress transfer + // extends the wait because it *is* this request's answer, where this one's must not + // because it is somebody else's. var unrelated = new byte[146]; unrelated[0] = 0x59; var p2 = TimeSpan.FromMilliseconds(500); - var (client, _, dispose) = BuildPair( + using var pair = new ClockPair( e => e.On(0x22, req => { _ = e.Channel.SendAsync(unrelated); @@ -548,21 +606,36 @@ public async Task P2_Is_Not_Extended_By_A_MultiFrame_Transfer_For_Another_Servic UsePadding = true, NAs = TimeSpan.FromMilliseconds(500), NBs = TimeSpan.FromMilliseconds(500), - NCr = TimeSpan.FromMilliseconds(500), + NCr = TimeSpan.FromSeconds(10), LocalStMin = TimeSpan.FromMilliseconds(127), }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - var sw = Stopwatch.StartNew(); - Func act = () => client.ReadDataByIdentifierAsync(0xF190, cts.Token); - await act.Should().ThrowAsync(); - sw.Stop(); + using var cts = new CancellationTokenSource(ShortTimeout); + var sw = Stopwatch.StartNew(); + var read = pair.Client.ReadDataByIdentifierAsync(0xF190, cts.Token); - sw.Elapsed.Should().BeLessThan(TimeSpan.FromMilliseconds(1500), - "an unrelated transfer must not hold the request past its budget"); + var deadline = DateTime.UtcNow + ShortTimeout; + while (pair.Channel.GetReceptionsInProgress().Count == 0) + { + if (DateTime.UtcNow > deadline) throw new TimeoutException("The unrelated First Frame never arrived."); + await Task.Delay(1); } + await pair.Clock.AdvanceAsync(p2 + TimeSpan.FromMilliseconds(100)); // P2 runs out mid-transfer + // The unrelated transfer is really still in progress: otherwise the result below would + // hold for a client that has nothing left to be fooled by. + pair.Channel.GetReceptionsInProgress().Should().NotBeEmpty(); + + Func act = () => read; + await act.Should().ThrowAsync( + "an unrelated transfer must not hold the request past its budget"); + sw.Stop(); + // The unrelated transfer's remaining CFs are paced by the ECU's own (real) STmin -- + // 127 ms x 19 more CFs, well over a second -- so a client that keeps waiting for it (by + // treating it as this request's own answer) is caught here, not just by the exception + // type: both a correct and a fooled client eventually throw UdsTimeoutException, only + // the fooled one does so after the whole transfer has played out in real time. + sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(1), + "an unrelated transfer must not hold the request past its budget"); } // ----------------------------------------------------------------------------------- @@ -705,88 +778,93 @@ public async Task Suppressed_Send_Windows_Are_Kept_Per_Service() [Fact] public async Task A_Pending_Answer_To_A_Suppressed_Send_Extends_Its_Window_By_P2Star() { - var (client, ecu, dispose) = BuildPair( - e => e.On(0x3E, req => + // On a virtual clock (#171): the ECU's 0x78 and its following negative are each held on + // a gate the test releases at the virtual instant they belong to. + var steps = new EcuSteps(); + var pendingAt = TimeSpan.FromMilliseconds(50); + var negativeAt = TimeSpan.FromMilliseconds(400); // after the 0x78, not from the send + using var pair = new ClockPair( + e => { - if ((req[1] & 0x80) != 0) - throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, - delayBefore: TimeSpan.FromMilliseconds(50), delayAfter: TimeSpan.FromMilliseconds(400)); - return new byte[] { 0x00 }; - }), + e.Delay = steps.WaitAsync; + e.On(0x3E, req => + { + if ((req[1] & 0x80) != 0) + throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, + delayBefore: pendingAt, delayAfter: negativeAt); + return new byte[] { 0x00 }; + }); + }, options: new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(300), P2StarClientMax = TimeSpan.FromMilliseconds(1500), }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // its window: 300 ms - // 0x78 at 50 ms, the negative at 450 ms: past P2, inside P2* from the 0x78. - Func act = () => client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); - await act.Should().NotThrowAsync("the negative answer belongs to the suppressed send"); - ecu.RequestsHandled.Should().Be(2); - } + var next = pair.Client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(300)); + + await pair.Clock.AdvanceAsync(pendingAt); // 50 ms: past nothing yet, inside the window + await pair.DeliverAsync(() => steps.Release(pendingAt), Negative(0x3E, 0x78)); + // The window moves out to P2* from the 0x78's arrival: 50 + 1500 = 1550 ms, 1500 from here. + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(1500)); + + await pair.Clock.AdvanceAsync(negativeAt); // 450 ms total: past the original P2 (300 ms) + await pair.DeliverAsync(() => steps.Release(negativeAt), Negative(0x3E, 0x12)); + + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(1100)); // the window closes at 1550 + Func act = () => next; + await act.Should().NotThrowAsync("the negative answer belongs to the suppressed send"); + pair.Ecu.RequestsHandled.Should().Be(2); } // Bugbot on #150: a 0x78 already queued when the window is over still moves it out. [Fact] public async Task A_Queued_Pending_Answer_Still_Extends_A_Window_That_Has_Run_Out() { - // P2 is long enough that a loaded host still delivers the 0x78 inside it. The test - // then waits that P2 out itself, so the client finds the window over and the 0x78 - // already queued. P2* from that arrival reaches past P2; a fixed 300 ms sleep on a - // 200 ms window did not -- the 0x78 was still on its way, the call returned at once, - // and the suppressed send had not been counted yet (macOS CI on #153). + // On a virtual clock (#171): the 0x78 is delivered -- so it is queued, not merely on its + // way -- before the test moves the clock past the window's end, which is the scenario + // this test names (Bugbot on #150): a 0x78 already queued when the window is over still + // moves it out. + var steps = new EcuSteps(); + var pendingAt = TimeSpan.FromMilliseconds(20); var p2 = TimeSpan.FromMilliseconds(1500); var p2Star = TimeSpan.FromMilliseconds(2500); - var (client, ecu, dispose) = BuildPair( - e => e.On(0x3E, req => + using var pair = new ClockPair( + e => { - if ((req[1] & 0x80) != 0) - throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, - delayBefore: TimeSpan.FromMilliseconds(20), delayAfter: TimeSpan.FromSeconds(20)); - return new byte[] { 0x00 }; - }), + e.Delay = steps.WaitAsync; + e.On(0x3E, req => + { + if ((req[1] & 0x80) != 0) + throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, + delayBefore: pendingAt, delayAfter: TimeSpan.FromSeconds(20)); + return new byte[] { 0x00 }; + }); + }, options: new UdsClientOptions { P2ClientMax = p2, P2StarClientMax = p2Star }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); - // After the send returns the window has already started, so waiting P2 from here - // lands past its end. - long sentAt = Stopwatch.GetTimestamp(); + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // its window: 1500 ms - var deadline = DateTime.UtcNow + ShortTimeout; - while (ecu.PendingNrcsSent == 0) - { - if (DateTime.UtcNow > deadline) throw new TimeoutException("the 0x78 was not sent"); - await Task.Delay(5); - } - long pendingAt = Stopwatch.GetTimestamp(); - var sinceSend = TimeSpan.FromSeconds((pendingAt - sentAt) / (double)Stopwatch.Frequency); - sinceSend.Should().BeLessThan(p2, "the 0x78 has to arrive while the window is still open"); - - var windowEnd = sentAt + (long)(p2.TotalSeconds * Stopwatch.Frequency); - while (Stopwatch.GetTimestamp() < windowEnd) - await Task.Delay(10); - - long callAt = Stopwatch.GetTimestamp(); - var sw = Stopwatch.StartNew(); - await client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); - sw.Stop(); - - // P2* from the 0x78, minus how long we already waited past that arrival. A missed - // extension returns in a round trip; this bound sits under the shortest extension - // (a 0x78 that arrived as the window opened) and a loaded host only lengthens it. - var extensionLeft = p2Star - TimeSpan.FromSeconds((callAt - pendingAt) / (double)Stopwatch.Frequency); - sw.Elapsed.Should().BeGreaterThanOrEqualTo(extensionLeft - TimeSpan.FromMilliseconds(200), - "the queued 0x78 moved the window out to P2* from its arrival"); - ecu.LastRequest.Should().BeEquivalentTo(new byte[] { 0x3E, 0x00 }); - } + await pair.Clock.AdvanceAsync(pendingAt); // 20 ms: well inside the window + await pair.DeliverAsync(() => steps.Release(pendingAt), Negative(0x3E, 0x78)); + + await pair.Clock.AdvanceAsync(p2 - pendingAt); // the window is now over, as measured + + var next = pair.Client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); + // The 0x78, already queued, moves the window out to P2* from its arrival: 20 + 2500 = + // 2520 ms; the clock stands at p2 (1500 ms), so 1020 ms remain. + var remaining = p2Star - p2 + pendingAt; + await pair.WaitUntilWaitingAsync(remaining); + await pair.Clock.AdvanceAsync(remaining); + + Func act = () => next; + await act.Should().NotThrowAsync("the queued 0x78 moved the window out to P2* from its arrival"); + pair.Ecu.LastRequest.Should().BeEquivalentTo(new byte[] { 0x3E, 0x00 }); } // Codex on #150: the converse -- a 0x78 queued after the window ran out answers nothing @@ -794,33 +872,36 @@ public async Task A_Queued_Pending_Answer_Still_Extends_A_Window_That_Has_Run_Ou [Fact] public async Task A_Queued_Pending_Answer_From_After_The_Windows_End_Does_Not_Revive_It() { - var (client, _, dispose) = BuildPair( - e => e.On(0x3E, req => - { - if ((req[1] & 0x80) != 0) - throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, - delayBefore: TimeSpan.FromMilliseconds(500), delayAfter: TimeSpan.FromSeconds(3)); - return new byte[] { 0x00 }; - }), - options: new UdsClientOptions + // On a virtual clock (#171): the 0x78 is delivered, and so queued, only once the clock + // already stands past the window's end -- late by construction, not by a race against a + // sleep. + var steps = new EcuSteps(); + var pendingAt = TimeSpan.FromMilliseconds(500); + var p2 = TimeSpan.FromMilliseconds(200); + using var pair = new ClockPair( + e => { - P2ClientMax = TimeSpan.FromMilliseconds(200), - P2StarClientMax = TimeSpan.FromMilliseconds(2000), - }); + e.Delay = steps.WaitAsync; + e.On(0x3E, req => + { + if ((req[1] & 0x80) != 0) + throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, + delayBefore: pendingAt, delayAfter: TimeSpan.FromSeconds(3)); + return new byte[] { 0x00 }; + }); + }, + options: new UdsClientOptions { P2ClientMax = p2, P2StarClientMax = TimeSpan.FromMilliseconds(2000) }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); - await Task.Delay(700); // the window ran out at 200 ms; the 0x78 at 500 ms is queued - - // Revived, the window would reach 2500 ms and this call would wait most of two - // seconds; a loaded host only makes the call slower, so the bound is wide. - var sw = Stopwatch.StartNew(); - await client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); - sw.Stop(); - sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(1), "the queued 0x78 from after the window did not revive it"); - } + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // its window: 200 ms + + await pair.Clock.AdvanceAsync(pendingAt); // 500 ms: past the window's own end (200 ms) + await pair.DeliverAsync(() => steps.Release(pendingAt), Negative(0x3E, 0x78)); + + // Revived, the window would reach 2500 ms; not revived, nothing is left to wait out, and + // this resolves without the clock moving any further -- the only way it *can* resolve, + // since nothing here ever advances it that far. + await Within(pair.Client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token)); } // Codex on #150: a 0x78 stamped inside a suppressed send's window may still be on its way @@ -829,10 +910,18 @@ public async Task A_Queued_Pending_Answer_From_After_The_Windows_End_Does_Not_Re [Fact] public async Task A_Pending_Answer_Still_On_Its_Way_Through_The_Channel_Extends_The_Window() { + // On a virtual clock (#171): the client's channel is opened on a VirtualClock actor, as + // ClockPair does, so the property -- a 0x78 stamped inside the window may still be on + // its way through the channel's actor when the window is measured as over -- is a fact + // about the interval the client arms, not a race against a sleep. + using var clock = new VirtualClock(); + clock.Advance(TimeSpan.FromMilliseconds(1)); // a zero stamp means "unstamped" to the channel + using var actor = clock.NewActor(); using var service = new StarvedReaderBusService(); - using var channel = IsoTpFactory.Open(service, IsoTpEndpoint.Normal(0x7E0, 0x7E8), FastIsoTp(useCanFd: false), leaveOpen: true); + using var channel = new IsoTpChannel(service, IsoTpEndpoint.Normal(0x7E0, 0x7E8), + FastIsoTp(useCanFd: false), ownsService: false, actor); var pendingBudget = TimeSpan.FromMilliseconds(600); - using var client = UdsClient.Create(channel, new UdsClientOptions + using var client = UdsClient.Create(channel, actor, new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(100), P2StarClientMax = pendingBudget, @@ -840,23 +929,29 @@ public async Task A_Pending_Answer_Still_On_Its_Way_Through_The_Channel_Extends_ using var cts = new CancellationTokenSource(ShortTimeout); await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // suppressed: window P2 = 100 ms + // The ECU's 0x78, stamped at the send -- inside the window by construction, not by a // timer -- buffered by the demux; the reader task that would take it to the actor is // starved by construction. - long arrival = Stopwatch.GetTimestamp(); + long arrival = clock.Elapsed.Ticks; byte[] sf = { 0x03, 0x7F, 0x3E, 0x78, 0x00, 0x00, 0x00, 0x00 }; service.Deliver(new CanFrameView(CanFrameType.Can20, 0x7E8, sf, FrameFlags.None), arrival); - await Task.Delay(150); // the window has run out, as measured - - // The next TesterPresent waits the window out: P2* from the 0x78 if the channel was - // settled before the inbox was read empty, else nothing. It is never answered; what - // matters is that it did not fail before P2* from the 0x78 had passed -- a lower bound - // a loaded host only raises. - Func next = () => client.SendRawAsync(new byte[] { 0x3E, 0x00 }, cts.Token); - await next.Should().ThrowAsync(); - var sinceArrival = TimeSpan.FromSeconds((Stopwatch.GetTimestamp() - arrival) / (double)Stopwatch.Frequency); - sinceArrival.Should().BeGreaterThanOrEqualTo(pendingBudget, - "the 0x78 on its way through the channel moved the window out to P2* from its arrival"); + + await clock.AdvanceAsync(TimeSpan.FromMilliseconds(150)); // the window has run out, as measured + + var next = client.SendRawAsync(new byte[] { 0x3E, 0x00 }, cts.Token); + // The window moves out to P2* from the 0x78's arrival, settled in before the deadline is + // read as empty: 600 ms from the arrival stamp, 450 ms from here. + var remaining = TimeSpan.FromTicks(arrival + pendingBudget.Ticks - clock.Elapsed.Ticks); + await clock.WaitUntilTimerArmedAsync(actor, remaining, ShortTimeout); + await clock.AdvanceAsync(remaining); // the window closes; the real request goes out + + // Nothing answers it either: its own P2 must now run out too. + await clock.WaitUntilTimerArmedAsync(actor, TimeSpan.FromMilliseconds(100), ShortTimeout); + await clock.AdvanceAsync(TimeSpan.FromMilliseconds(100)); + + Func nextFn = () => next; + await nextFn.Should().ThrowAsync(); } // Codex on #150: a suppressed request the channel refuses before transmitting -- here, @@ -865,26 +960,21 @@ public async Task A_Pending_Answer_Still_On_Its_Way_Through_The_Channel_Extends_ [Fact] public async Task A_Suppressed_Send_The_Channel_Refuses_Leaves_No_Window() { - var (client, _, dispose) = BuildPair( + // On a virtual clock (#171): a window left behind by mistake would have this call wait + // out P2 = 2 s on a clock nothing here ever advances, so it can only resolve -- inside + // Within's real timeout -- if no window was left at all. + using var pair = new ClockPair( e => e.On(0x3E, req => new byte[] { 0x00 }), options: new UdsClientOptions { P2ClientMax = TimeSpan.FromSeconds(2), P2StarClientMax = TimeSpan.FromSeconds(2) }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - var oversized = new byte[4200]; - oversized[0] = 0x3E; - oversized[1] = 0x80; - Func refused = () => client.SendRawAsync(oversized, cts.Token); - await refused.Should().ThrowAsync(); - - // Left with a window, this call would wait P2 = 2 s before sending; a loaded host - // only makes the call slower, so the bound is wide. - var sw = Stopwatch.StartNew(); - await client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); - sw.Stop(); - sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(1), "nothing was transmitted, so nothing is answered"); - } + using var cts = new CancellationTokenSource(ShortTimeout); + var oversized = new byte[4200]; + oversized[0] = 0x3E; + oversized[1] = 0x80; + Func refused = () => pair.Client.SendRawAsync(oversized, cts.Token); + await refused.Should().ThrowAsync(); + + await Within(pair.Client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token)); } // Bugbot on #150: a refused request puts the window back to what it was -- an earlier @@ -892,28 +982,26 @@ public async Task A_Suppressed_Send_The_Channel_Refuses_Leaves_No_Window() [Fact] public async Task A_Suppressed_Send_The_Channel_Refuses_Leaves_An_Earlier_Window_As_It_Was() { - var (client, _, dispose) = BuildPair( + // On a virtual clock (#171): the earlier window's survival is proven by the exact + // interval the next call arms, not by a real elapsed-time floor. + using var pair = new ClockPair( e => e.On(0x3E, req => new byte[] { 0x00 }), options: new UdsClientOptions { P2ClientMax = TimeSpan.FromSeconds(2), P2StarClientMax = TimeSpan.FromSeconds(2) }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - var sw = Stopwatch.StartNew(); - await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // suppressed: a 2 s window, not waited out by the next suppressed send - var oversized = new byte[4200]; - oversized[0] = 0x3E; - oversized[1] = 0x80; - Func refused = () => client.SendRawAsync(oversized, cts.Token); - await refused.Should().ThrowAsync(); - - // The unsuppressed TesterPresent waits the first send's window out: at least 2 s - // after it, a lower bound a loaded host only raises. - await client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); - sw.Stop(); - sw.Elapsed.Should().BeGreaterThanOrEqualTo(TimeSpan.FromMilliseconds(1900), - "the refused send left the earlier suppressed send's window as it was"); - } + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // suppressed: a 2 s window, not waited out by the next suppressed send + var oversized = new byte[4200]; + oversized[0] = 0x3E; + oversized[1] = 0x80; + Func refused = () => pair.Client.SendRawAsync(oversized, cts.Token); + await refused.Should().ThrowAsync(); + + var next = pair.Client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); + // The unsuppressed TesterPresent waits the first send's window out: 2 s, unshortened by + // the refused send in between. + await pair.WaitUntilWaitingAsync(TimeSpan.FromSeconds(2)); + await pair.Clock.AdvanceAsync(TimeSpan.FromSeconds(2)); + await Within(next); } // Codex on #150: the discard on an aborted request -- here, one the caller cancelled -- @@ -922,10 +1010,20 @@ public async Task A_Suppressed_Send_The_Channel_Refuses_Leaves_An_Earlier_Window [Fact] public async Task A_Pending_Answer_Still_On_Its_Way_Is_Routed_By_An_Aborted_Requests_Discard() { + // On a virtual clock (#171), as A_Pending_Answer_Still_On_Its_Way_Through_The_Channel_ + // Extends_The_Window above: only the frame's stamp and the client's own timers need to + // be virtual. The race the comment below describes is about *ordering* (the frame must + // be delivered before the abort, once the send is visible on the wire) rather than about + // *when* on a clock, so the poll for service.Sent.Count stays a bounded real poll of an + // observable condition. + using var clock = new VirtualClock(); + clock.Advance(TimeSpan.FromMilliseconds(1)); + using var actor = clock.NewActor(); using var service = new StarvedReaderBusService(); - using var channel = IsoTpFactory.Open(service, IsoTpEndpoint.Normal(0x7E0, 0x7E8), FastIsoTp(useCanFd: false), leaveOpen: true); + using var channel = new IsoTpChannel(service, IsoTpEndpoint.Normal(0x7E0, 0x7E8), + FastIsoTp(useCanFd: false), ownsService: false, actor); var pendingBudget = TimeSpan.FromMilliseconds(600); - using var client = UdsClient.Create(channel, new UdsClientOptions + using var client = UdsClient.Create(channel, actor, new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(100), P2StarClientMax = pendingBudget, @@ -938,14 +1036,13 @@ public async Task A_Pending_Answer_Still_On_Its_Way_Is_Routed_By_An_Aborted_Requ // The 0x78 is stamped 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 rather than by a timer (macOS CI on #150) -- and delivered while the - // request waits, before its abort, whose discard is the one under test. A 50 ms timer - // lost that race on macOS: the host resumed after P2 had already elapsed and the wait - // reported a timeout. Cancelling from here, once the send is visible, does not depend - // on a timer firing first. (A host that delays the delivery past the abort lets the - // next call's wait-out route it instead: a pass for the wrong reason, never a failure.) + // request waits, before its abort, whose discard is the one under test. Cancelling from + // here, once the send is visible, does not depend on a timer firing first. (A host that + // delays the delivery past the abort lets the next call's wait-out route it instead: a + // pass for the wrong reason, never a failure.) using var early = new CancellationTokenSource(); var other = client.ReadDataByIdentifierAsync(0xF190, early.Token); - long arrival = Stopwatch.GetTimestamp(); + long arrival = clock.Elapsed.Ticks; var sent = DateTime.UtcNow + ShortTimeout; while (service.Sent.Count < 2) { @@ -958,11 +1055,19 @@ public async Task A_Pending_Answer_Still_On_Its_Way_Is_Routed_By_An_Aborted_Requ Func cancelled = () => other; await cancelled.Should().ThrowAsync(); - // The next TesterPresent waits P2* from the 0x78 if the abort routed it, else nothing. - Func next = () => client.SendRawAsync(new byte[] { 0x3E, 0x00 }, cts.Token); - await next.Should().ThrowAsync(); - var sinceArrival = TimeSpan.FromSeconds((Stopwatch.GetTimestamp() - arrival) / (double)Stopwatch.Frequency); - sinceArrival.Should().BeGreaterThanOrEqualTo(pendingBudget, + var next = client.SendRawAsync(new byte[] { 0x3E, 0x00 }, cts.Token); + // The window moves out to P2* from the 0x78's arrival: 600 ms from the arrival stamp, + // and the clock has not moved since, so 600 ms from here too. + var remaining = TimeSpan.FromTicks(arrival + pendingBudget.Ticks - clock.Elapsed.Ticks); + await clock.WaitUntilTimerArmedAsync(actor, remaining, ShortTimeout); + await clock.AdvanceAsync(remaining); // the window closes; the real request goes out + + // Nothing answers it either: its own P2 must now run out too. + await clock.WaitUntilTimerArmedAsync(actor, TimeSpan.FromMilliseconds(100), ShortTimeout); + await clock.AdvanceAsync(TimeSpan.FromMilliseconds(100)); + + Func nextFn = () => next; + await nextFn.Should().ThrowAsync( "the aborted request's discard routed the 0x78 to the suppressed send's window"); } @@ -972,45 +1077,55 @@ public async Task A_Pending_Answer_Still_On_Its_Way_Is_Routed_By_An_Aborted_Requ [Fact] public async Task A_Stale_Pending_Answer_Queued_Before_A_Suppressed_Send_Does_Not_Extend_Its_Window() { + // On a virtual clock (#171): the earlier request's own P2 wait, its cancellation, and + // the stale 0x78's delivery are all driven by the test rather than raced against sleeps. + var steps = new EcuSteps(); int calls = 0; - var (client, _, dispose) = BuildPair( - e => e.On(0x3E, req => + using var pair = new ClockPair( + e => { - switch (Interlocked.Increment(ref calls)) + e.Delay = steps.WaitAsync; + e.On(0x3E, req => { - case 1: // the request the caller cancels: its 0x78 at 100 ms is stale by then, its negative never comes in time - throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, - delayBefore: TimeSpan.FromMilliseconds(100), delayAfter: TimeSpan.FromSeconds(3)); - case 2: // the suppressed send: no answer - throw new EcuSilent(); - default: - return new byte[] { 0x00 }; - } - }), + switch (Interlocked.Increment(ref calls)) + { + case 1: // the request the caller cancels: its 0x78 is stale by the time the suppressed send discards it + throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, + delayBefore: TimeSpan.FromMilliseconds(100), delayAfter: TimeSpan.FromSeconds(3)); + case 2: // the suppressed send: no answer + throw new EcuSilent(); + default: + return new byte[] { 0x00 }; + } + }); + }, options: new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(100), P2StarClientMax = TimeSpan.FromMilliseconds(2000), }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - using var early = new CancellationTokenSource(TimeSpan.FromMilliseconds(30)); - Func cancelled = () => client.SendRawAsync(new byte[] { 0x3E, 0x00 }, early.Token); - await cancelled.Should().ThrowAsync(); - await Task.Delay(150); // the stale 0x78 (100 ms) is queued - - await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // suppressed: window P2 = 100 ms - - // Read as this send's, the stale 0x78 would move the window out to 2100 ms and this - // call wait most of two seconds; a loaded host only makes it slower. - var sw = Stopwatch.StartNew(); - var answer = await client.SendRawAsync(new byte[] { 0x3E, 0x00 }, cts.Token); - sw.Stop(); - answer.Should().Equal(0x7E, 0x00); - sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(1), "the stale 0x78 was discarded before the suppressed send"); - } + using var cts = new CancellationTokenSource(ShortTimeout); + using var early = new CancellationTokenSource(); + var cancelled = pair.Client.SendRawAsync(new byte[] { 0x3E, 0x00 }, early.Token); + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(100)); // call #1 waiting on its own P2 + early.Cancel(); + Func cancelledFn = () => cancelled; + await cancelledFn.Should().ThrowAsync(); + + // The stale 0x78 (its 100 ms delay) is queued before the suppressed send opens its window. + await pair.DeliverAsync(() => steps.Release(TimeSpan.FromMilliseconds(100)), Negative(0x3E, 0x78)); + + await pair.Client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // suppressed: window P2 = 100 ms + + var next = pair.Client.SendRawAsync(new byte[] { 0x3E, 0x00 }, cts.Token); + // Read as this send's, the stale 0x78 would move the window out to 2100 ms; the exact + // interval armed here is the suppressed send's own 100 ms, unextended. + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(100)); + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(100)); + + var answer = await Within(next); + answer.Should().Equal(0x7E, 0x00); } // Bugbot on #150: a wait cancelled part-way keeps what remains of the window. @@ -1058,39 +1173,58 @@ public async Task A_Cancelled_Wait_Keeps_The_Rest_Of_The_Window() [Fact] public async Task A_Pending_Answer_For_Another_Service_Heard_During_A_Wait_Extends_That_Services_Window() { - var (client, _, dispose) = BuildPair( - e => e - .On(0x3E, req => - { - if ((req[1] & 0x80) != 0) throw new EcuSilent(); - return new byte[] { 0x00 }; - }) - .On(0x11, req => - { - if ((req[1] & 0x80) != 0) - throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, - delayBefore: TimeSpan.FromMilliseconds(50), delayAfter: TimeSpan.FromMilliseconds(500)); - Thread.Sleep(250); // slow enough that the stale negative (at 550 ms) would be first in line - return new byte[] { 0x01 }; - }), + // On a virtual clock (#171): B's 0x78 is delivered while A's TesterPresent is actively + // waiting out A's window, at the exact instant the test moves the clock to, and B's + // window's extension is then proven by the exact interval B's own next request arms -- + // stronger than the original's implicit race against a slow positive reply, and not + // dependent on one. + var steps = new EcuSteps(); + var pendingAt = TimeSpan.FromMilliseconds(50); + using var pair = new ClockPair( + e => + { + e.Delay = steps.WaitAsync; + e.On(0x3E, req => + { + if ((req[1] & 0x80) != 0) throw new EcuSilent(); + return new byte[] { 0x00 }; + }) + .On(0x11, req => + { + if ((req[1] & 0x80) != 0) + throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, + delayBefore: pendingAt, delayAfter: TimeSpan.FromMilliseconds(500)); + return new byte[] { 0x01 }; + }); + }, options: new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(400), P2StarClientMax = TimeSpan.FromMilliseconds(1500), }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // A, silent - await client.SendRawAsync(new byte[] { 0x11, 0x81 }, cts.Token); // B: 0x78 at 50 ms, negative at 450 ms - - // A's request waits A's window out and hears B's 0x78 meanwhile. - await client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); - // B's request follows at once: B's window must now reach past 450 ms. - var reset = await client.SendRawAsync(new byte[] { 0x11, 0x01 }, cts.Token); - reset.Should().Equal(0x51, 0x01); - } + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // A, silent: window(0x3E) to 400 ms + await pair.Client.SendRawAsync(new byte[] { 0x11, 0x81 }, cts.Token); // B, suppressed: window(0x11) to 400 ms too + + // A's request waits A's window out and hears B's 0x78 meanwhile. + var a = pair.Client.TesterPresentAsync(suppressPositiveResponse: false, cts.Token); + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(400)); + + await pair.Clock.AdvanceAsync(pendingAt); // 50 ms + await pair.DeliverAsync(() => steps.Release(pendingAt), Negative(0x11, 0x78)); // heard as B's, not A's + + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(350)); // A's own window, unaffected + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(350)); // A's window closes at 400 ms + await Within(a); + + // B's request follows: B's window now reaches P2* past the 0x78 -- 1550 ms, 1150 from here. + var reset = pair.Client.SendRawAsync(new byte[] { 0x11, 0x01 }, cts.Token); + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(1150)); + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(1150)); + + var result = await Within(reset); + result.Should().Equal(0x51, 0x01); } // Codex on #150: a 0x78 for service A, consumed as a stray while service B's request runs @@ -1156,38 +1290,57 @@ public async Task A_Pending_Answer_Consumed_As_Another_Requests_Stray_Still_Exte [Fact] public async Task A_Pending_Answer_From_After_A_Windows_End_Does_Not_Revive_It() { - var (client, _, dispose) = BuildPair( - e => e - .On(0x11, req => - { - if ((req[1] & 0x80) != 0) - throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, - delayBefore: TimeSpan.FromMilliseconds(500), delayAfter: TimeSpan.FromSeconds(3)); - return new byte[] { 0x01 }; - }) - .On(0x22, req => throw new EcuResponsePending(pendingCount: 1, - finalResponse: new byte[] { 0xF1, 0x90, 0xAA }, delayBetween: TimeSpan.FromMilliseconds(700))), + // On a virtual clock (#171): A's late 0x78 is delivered once B's own receive loop is + // waiting for it, at an instant already past A's window's end -- late by construction -- + // and "not revived" is then proven by A's next request resolving without the clock + // moving any further, the only way it can resolve if nothing was revived. + var steps = new EcuSteps(); + var pendingAt = TimeSpan.FromMilliseconds(500); + var bGap = TimeSpan.FromMilliseconds(700); + using var pair = new ClockPair( + e => + { + e.Delay = steps.WaitAsync; + e.On(0x11, req => + { + if ((req[1] & 0x80) != 0) + throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, + delayBefore: pendingAt, delayAfter: TimeSpan.FromSeconds(3)); + return new byte[] { 0x01 }; + }) + .On(0x22, req => throw new EcuResponsePending(pendingCount: 1, + finalResponse: new byte[] { 0xF1, 0x90, 0xAA }, delayBetween: bGap)); + }, options: new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(200), P2StarClientMax = TimeSpan.FromMilliseconds(2000), }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - await client.SendRawAsync(new byte[] { 0x11, 0x81 }, cts.Token); // A, suppressed: window P2 = 200 ms; its 0x78 comes at 500 ms - await client.ReadDataByIdentifierAsync(0xF190, cts.Token); // B, waiting in P2* until 700 ms, consumes A's late 0x78 as a stray - - // A's next request: the window ran out at 200 ms, and the 0x78 at 500 did not - // reopen it -- revived, it would reach 2500 ms, and this call would wait most of - // two seconds. A loaded host only makes the call slower, so the bound is wide. - var sw = Stopwatch.StartNew(); - var reset = await client.SendRawAsync(new byte[] { 0x11, 0x01 }, cts.Token); - sw.Stop(); - reset.Should().Equal(0x51, 0x01); - sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(1), "the late 0x78 did not revive the window"); - } + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x11, 0x81 }, cts.Token); // A, suppressed: window(0x11) to 200 ms + + // B's own first 0x78 goes out the instant its request is handled, with nothing pacing + // it -- on a fast host that can land before a check for B's initial P2 would even see + // it armed, so what is checked directly is the interval B restarts to, which only a + // client that has already received and processed that 0x78 could have armed. + var b = pair.Client.ReadDataByIdentifierAsync(0xF190, cts.Token); + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(2000)); // B's P2*, restarted + await pair.Clock.AdvanceAsync(pendingAt); // 500 ms: past A's window's end (200 ms) + await pair.DeliverAsync(() => steps.Release(pendingAt), Negative(0x11, 0x78)); // A's late 0x78, heard as B's stray + + // B's own wait is unaffected: 2000 ms from its 0x78 at 1 ms, minus the 500 ms elapsed. + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(1500)); + await pair.Clock.AdvanceAsync(bGap - pendingAt); // 200 ms further: B's own gap (700 ms) ends + await pair.DeliverAsync(() => steps.Release(bGap), data => data.Length >= 2 && data[1] == 0x62); // B's final response + var data = await Within(b); + data.Should().Equal(0xAA); + + // A's next request: the window ran out at 200 ms, and the late 0x78 did not reopen it -- + // revived, it would reach 2500 ms, and this resolves without the clock moving any + // further, which is only possible if nothing was revived. + var reset = await Within(pair.Client.SendRawAsync(new byte[] { 0x11, 0x01 }, cts.Token)); + reset.Should().Equal(0x51, 0x01); } // Codex on #150: the same, heard while another service's wait-out runs rather than by a @@ -1195,43 +1348,61 @@ public async Task A_Pending_Answer_From_After_A_Windows_End_Does_Not_Revive_It() [Fact] public async Task A_Pending_Answer_From_After_A_Windows_End_Heard_In_A_Wait_Out_Does_Not_Revive_It() { - var (client, _, dispose) = BuildPair( - e => e - .On(0x11, req => - { - if ((req[1] & 0x80) != 0) - throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, - delayBefore: TimeSpan.FromMilliseconds(1500), delayAfter: TimeSpan.FromSeconds(3)); - return new byte[] { 0x01 }; - }) - .On(0x3E, req => - { - if ((req[1] & 0x80) != 0) - throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, - delayBefore: TimeSpan.FromMilliseconds(100), delayAfter: TimeSpan.FromSeconds(3)); - return new byte[] { 0x00 }; - }), + // On a virtual clock (#171): B's late 0x78 is delivered while A is actively waiting out + // A's own (self-extended) window -- the "heard in a wait-out" path, as opposed to the + // sibling test's "heard by a request's stray branch" -- again proven by exact intervals + // rather than a race against a sleep. + var steps = new EcuSteps(); + var aPendingAt = TimeSpan.FromMilliseconds(100); + var bPendingAt = TimeSpan.FromMilliseconds(1500); + using var pair = new ClockPair( + e => + { + e.Delay = steps.WaitAsync; + e.On(0x11, req => + { + if ((req[1] & 0x80) != 0) + throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, + delayBefore: bPendingAt, delayAfter: TimeSpan.FromSeconds(3)); + return new byte[] { 0x01 }; + }) + .On(0x3E, req => + { + if ((req[1] & 0x80) != 0) + throw new EcuResponsePendingThenNegative(pendingCount: 1, nrc: 0x12, + delayBefore: aPendingAt, delayAfter: TimeSpan.FromSeconds(3)); + return new byte[] { 0x00 }; + }); + }, options: new UdsClientOptions { P2ClientMax = TimeSpan.FromMilliseconds(200), P2StarClientMax = TimeSpan.FromMilliseconds(2000), }); - using (dispose) - { - using var cts = new CancellationTokenSource(ShortTimeout); - await client.SendRawAsync(new byte[] { 0x11, 0x81 }, cts.Token); // B, suppressed: window to 200 ms; its 0x78 comes at 1500 ms - await client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // A, suppressed: its own 0x78 at 100 ms moves its window to 2100 ms - await client.SendRawAsync(new byte[] { 0x3E, 0x00 }, cts.Token); // A again: waits its window out until 2100 ms, hearing B's late 0x78 at 1500 - - // B's next request: revived at 1500 ms, B's window would reach 3500 ms and this - // call, at ~2100 ms, would wait most of 1.5 s. - var sw = Stopwatch.StartNew(); - var reset = await client.SendRawAsync(new byte[] { 0x11, 0x01 }, cts.Token); - sw.Stop(); - reset.Should().Equal(0x51, 0x01); - sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(1), "the late 0x78 heard in the wait-out did not revive the window"); - } + using var cts = new CancellationTokenSource(ShortTimeout); + await pair.Client.SendRawAsync(new byte[] { 0x11, 0x81 }, cts.Token); // B, suppressed: window(0x11) to 200 ms + await pair.Client.SendRawAsync(new byte[] { 0x3E, 0x80 }, cts.Token); // A, suppressed: window(0x3E) to 200 ms too + + var again = pair.Client.SendRawAsync(new byte[] { 0x3E, 0x00 }, cts.Token); // A again: waits its own window out + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(200)); + + await pair.Clock.AdvanceAsync(aPendingAt); // 100 ms: A's own 0x78 arrives + await pair.DeliverAsync(() => steps.Release(aPendingAt), Negative(0x3E, 0x78)); + // A's own window moves out to P2* from its 0x78: 100 + 2000 = 2100 ms, 2000 from here. + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(2000)); + + await pair.Clock.AdvanceAsync(bPendingAt - aPendingAt); // 1500 ms total: B's late 0x78 arrives + await pair.DeliverAsync(() => steps.Release(bPendingAt), Negative(0x11, 0x78)); // heard in A's own wait-out + // A's wait is unaffected: 2100 ms total, 600 ms from here. + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(600)); + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(600)); // A's window closes at 2100 ms + await Within(again); + + // B's next request: revived, B's window would reach 3500 ms; unrevived, it resolves + // without the clock moving any further. + var reset = await Within(pair.Client.SendRawAsync(new byte[] { 0x11, 0x01 }, cts.Token)); + reset.Should().Equal(0x51, 0x01); } // NRC 0x21 asks for a repeat; the client repeats, up to MaxBusyRepeatRequests. @@ -1622,29 +1793,45 @@ public async Task Client_Times_Out_When_Ecu_Silent_Within_P2() [Fact] public async Task Client_Times_Out_With_P2Star_When_Ecu_Sends_Only_ResponsePending() { - var (client, _, dispose) = BuildPair( - e => e.On(0x22, _ => throw new EcuResponsePendingThenSilent( - pendingCount: 2, delayBetween: TimeSpan.FromMilliseconds(40))), + // On a virtual clock (#171): each 0x78 is delivered at the exact virtual instant it + // belongs to, via OrderedGates because the two delays share the same 40 ms length -- + // EcuSteps would hand both callers the same, already-resolved gate. The first 0x78 goes + // out the instant the request is handled, with nothing pacing it -- on a fast host that + // can land, and be processed, before a check for the initial P2 would even see it armed + // -- so what is checked directly is the interval the client restarts to, which only a + // client that has already received and processed that 0x78 could have armed. + var gates = new OrderedGates(); + var gap = TimeSpan.FromMilliseconds(40); + var p2 = TimeSpan.FromMilliseconds(100); + var p2Star = TimeSpan.FromMilliseconds(250); + using var pair = new ClockPair( + e => + { + e.Delay = gates.WaitAsync; + e.On(0x22, _ => throw new EcuResponsePendingThenSilent(pendingCount: 2, delayBetween: gap)); + }, options: new UdsClientOptions { - P2ClientMax = TimeSpan.FromMilliseconds(100), - P2StarClientMax = TimeSpan.FromMilliseconds(250), + P2ClientMax = p2, + P2StarClientMax = p2Star, MaxResponsePendingCount = 10, }); - using (dispose) - { - var sw = Stopwatch.StartNew(); - Func act = () => client.ReadDataByIdentifierAsync(0xF190, - new CancellationTokenSource(ShortTimeout).Token); - var ex = (await act.Should().ThrowAsync()).Which; - sw.Stop(); - ex.Timer.Should().Be(UdsTimeoutTimer.P2Star, - "the ECU answered with NRC 0x78 (response pending), so the running timer is P2* — not P2"); - ex.RequestedService.Should().Be(UdsServiceId.ReadDataByIdentifier); - sw.Elapsed.Should().BeGreaterThanOrEqualTo(TimeSpan.FromMilliseconds(200), - "P2* restarts on each 0x78, so the timeout must fire well past the initial P2 budget (100 ms)"); - } + using var cts = new CancellationTokenSource(ShortTimeout); + var read = pair.Client.ReadDataByIdentifierAsync(0xF190, cts.Token); + + await pair.WaitUntilWaitingAsync(p2Star); // restarted on the first 0x78's arrival + await pair.Clock.AdvanceAsync(gap); + await pair.DeliverAsync(gates.ReleaseNext, Negative(0x22, 0x78)); // the second 0x78, at 40 ms + + await pair.WaitUntilWaitingAsync(p2Star); // restarted again, from the second 0x78 + await pair.Clock.AdvanceAsync(p2Star); // then silence: P2* must expire + + Func act = () => read; + var ex = (await act.Should().ThrowAsync()).Which; + ex.Timer.Should().Be(UdsTimeoutTimer.P2Star, + "the ECU answered with NRC 0x78 (response pending), so the running timer is P2* — not P2"); + ex.RequestedService.Should().Be(UdsServiceId.ReadDataByIdentifier); } // ----------------------------------------------------------------------------------- @@ -1654,26 +1841,50 @@ public async Task Client_Times_Out_With_P2Star_When_Ecu_Sends_Only_ResponsePendi [Fact] public async Task ResponsePending_Restarts_P2Star_And_Returns_Final_Response() { + // On a virtual clock (#171): the three 0x78s (60 ms apart) and the final response are + // each delivered at the exact instant they belong to, via OrderedGates for the same + // reason as the P2* timeout test above. The first 0x78 goes out unpaced and can land + // before a check for the initial P2 would see it armed, so what is checked directly is + // the interval the client restarts to. The exact interval armed after every 0x78 proves + // the restart directly, rather than by the client merely finishing with the right + // payload despite a short initial P2. + var gates = new OrderedGates(); + var gap = TimeSpan.FromMilliseconds(60); + var p2 = TimeSpan.FromMilliseconds(120); + var p2Star = TimeSpan.FromSeconds(1); byte[] finalBody = { 0xF1, 0x90, 0xAA, 0xBB, 0xCC, 0xDD }; - var (client, _, dispose) = BuildPair( - e => e.On(0x22, _ => throw new EcuResponsePending( - pendingCount: 3, finalResponse: finalBody, - delayBetween: TimeSpan.FromMilliseconds(60))), + using var pair = new ClockPair( + e => + { + e.Delay = gates.WaitAsync; + e.On(0x22, _ => throw new EcuResponsePending( + pendingCount: 3, finalResponse: finalBody, delayBetween: gap)); + }, options: new UdsClientOptions { - // P2 short: proves the client actually restarts on 0x78 rather than living - // inside the (accidentally) generous initial budget. 120 ms is several times - // the first-hop cliff measured for #118 (see the P2* timeout test above). - P2ClientMax = TimeSpan.FromMilliseconds(120), - P2StarClientMax = TimeSpan.FromSeconds(1), + P2ClientMax = p2, + P2StarClientMax = p2Star, MaxResponsePendingCount = 10, }); - using (dispose) - { - var data = await client.ReadDataByIdentifierAsync(0xF190, - new CancellationTokenSource(ShortTimeout).Token); - data.Should().Equal(0xAA, 0xBB, 0xCC, 0xDD); - } + + using var cts = new CancellationTokenSource(ShortTimeout); + var read = pair.Client.ReadDataByIdentifierAsync(0xF190, cts.Token); + + await pair.WaitUntilWaitingAsync(p2Star); // restarted on the 1st (unpaced) 0x78's arrival + await pair.Clock.AdvanceAsync(gap); + await pair.DeliverAsync(gates.ReleaseNext, Negative(0x22, 0x78)); // 2nd 0x78 @ 60 ms + + await pair.WaitUntilWaitingAsync(p2Star); + await pair.Clock.AdvanceAsync(gap); + await pair.DeliverAsync(gates.ReleaseNext, Negative(0x22, 0x78)); // 3rd 0x78 @ 120 ms + + await pair.WaitUntilWaitingAsync(p2Star); + await pair.Clock.AdvanceAsync(gap); + // Releasing the 3rd gate lets the final positive response go out right away. + await pair.DeliverAsync(gates.ReleaseNext, data => data.Length >= 2 && data[1] == 0x62); + + var data = await Within(read); + data.Should().Equal(0xAA, 0xBB, 0xCC, 0xDD); } // ----------------------------------------------------------------------------------- From 15c7395df9e66ee52efc5afe7c90fea485cae05d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 04:40:11 +0000 Subject: [PATCH 7/8] test(uds): bound the virtual-clock tests and cover the busy delay - P2_Is_Not_Extended_By_A_MultiFrame_Transfer_For_Another_Service no longer bounds the client with a real Stopwatch. - The ECU sends only the First Frame of the unrelated transfer, so the reception stays in progress. The client's N_Cr is on the virtual clock and cannot end it. - P2 then runs out on the clock, and a correct client times out. - A client fooled into waiting for the transfer never returns. The test's own token then ends it with a cancellation rather than a timeout. - Mutation checked: with ResponseBeganInTime's response-SID check dropped, the test is red on exactly that cancellation. - EcuSteps now honours the ECU's cancellation token. Under a mutation that stops a 0x78 from restarting P2*, a failed test disposed its ECU while the ECU loop was parked on a gate nobody opens. The disposal hung, and so did the whole test run. The same mutation now turns its three tests red in 32 s. - New A_Busy_Repeat_Waits_Its_Delay_On_The_Clients_Clock and A_Busy_Repeat_Delay_On_The_Clients_Clock_Ends_On_Cancellation cover the busy-repeat delay on the injected clock. - A delay that ignores the clock turns both red. - A delay deaf to the token turns the cancellation test red. Refs #171 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s --- .../TestCases/Uds/UdsClientTests.cs | 95 ++++++++++++++----- 1 file changed, 69 insertions(+), 26 deletions(-) diff --git a/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs b/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs index 4f3e4aa..b86a81e 100644 --- a/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs @@ -102,6 +102,7 @@ public ClockPair(Action configure, UdsClientOptions? options = var ecuChannel = IsoTpFactory.Open(busEcu, IsoTpEndpoint.Normal(txCanId: 0x7E8, rxCanId: 0x7E0), FastIsoTp(useCanFd: false)); + EcuBus = busEcu; Ecu = new SimulatedUdsEcu(ecuChannel); configure(Ecu); Ecu.Start(); @@ -119,6 +120,9 @@ public ClockPair(Action configure, UdsClientOptions? options = public SimulatedUdsEcu Ecu { get; } + /// The ECU's raw bus, for a frame its ISO-TP channel would not send. + public ICanBus EcuBus { get; private set; } = null!; + public IUdsClient Client { get; } /// @@ -182,7 +186,15 @@ private sealed class EcuSteps { private readonly Dictionary> _gates = new(); - public Task WaitAsync(TimeSpan delay, CancellationToken _) => Gate(delay).Task; + // Honours the ECU's token: a test that fails before releasing a gate still disposes the + // ECU, and an ECU loop parked on a gate nobody opens would hang that disposal -- and the + // test run with it -- instead of letting the failure be reported. + public async Task WaitAsync(TimeSpan delay, CancellationToken cancellationToken) + { + var cancelled = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using (cancellationToken.Register(() => cancelled.TrySetCanceled())) + await await Task.WhenAny(Gate(delay).Task, cancelled.Task).ConfigureAwait(false); + } public void Release(TimeSpan delay) => Gate(delay).TrySetResult(true); @@ -518,6 +530,46 @@ await client.SecurityAccessAsync( } } + // #57 on a virtual clock (#171): NRC 0x21 repeats the request after BusyRepeatRequestDelay, + // measured on the client's clock like its P2 -- the repeat goes out only once the test has + // moved the clock past the delay. + [Fact] + public async Task A_Busy_Repeat_Waits_Its_Delay_On_The_Clients_Clock() + { + var calls = 0; + using var pair = new ClockPair( + e => e.On(0x22, req => Interlocked.Increment(ref calls) == 1 + ? throw new EcuNegativeResponse(0x21) + : new byte[] { 0xF1, 0x90, 0xAA }), + options: new UdsClientOptions { BusyRepeatRequestDelay = TimeSpan.FromMilliseconds(100) }); + + using var cts = new CancellationTokenSource(ShortTimeout); + var read = pair.Client.ReadDataByIdentifierAsync(0xF190, cts.Token); + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(100)); // the busy delay, not P2 + Volatile.Read(ref calls).Should().Be(1, "the repeat waits for the delay"); + + await pair.Clock.AdvanceAsync(TimeSpan.FromMilliseconds(100)); + (await Within(read)).Should().Equal(0xAA); + Volatile.Read(ref calls).Should().Be(2); + } + + // A caller cancelling during the busy delay ends the wait at once, as Task.Delay's token did. + [Fact] + public async Task A_Busy_Repeat_Delay_On_The_Clients_Clock_Ends_On_Cancellation() + { + using var pair = new ClockPair( + e => e.On(0x22, req => throw new EcuNegativeResponse(0x21)), + options: new UdsClientOptions { BusyRepeatRequestDelay = TimeSpan.FromMilliseconds(100) }); + + using var cts = new CancellationTokenSource(ShortTimeout); + var read = pair.Client.ReadDataByIdentifierAsync(0xF190, cts.Token); + await pair.WaitUntilWaitingAsync(TimeSpan.FromMilliseconds(100)); + cts.Cancel(); + + Func act = () => Within(read); // a delay deaf to the token would never end + await act.Should().ThrowAsync(); + } + // ----------------------------------------------------------------------------------- // #28 — ISO 14229-2: P2 ends with the *first* frame of the response. A multi-frame // response whose transfer outlasts P2 (here: paced by the client's own STmin) is not a @@ -582,21 +634,23 @@ public async Task P2_Ends_With_The_First_Frame_Of_A_MultiFrame_Response() public async Task P2_Is_Not_Extended_By_A_MultiFrame_Transfer_For_Another_Service() { // The ECU answers the RDBI request with silence, but first starts a 146-byte - // ReadDTCInformation response (SID 0x59): FF + 20 CFs at the client's STmin of 127 ms. - // On a virtual clock (#171) the transfer is proven still in progress by - // GetReceptionsInProgress() rather than by outrunning a real N_Cr; P2 is then moved - // past on the clock and must fire regardless -- the sibling of - // P2_Ends_With_The_First_Frame_Of_A_MultiFrame_Response, whose in-progress transfer - // extends the wait because it *is* this request's answer, where this one's must not - // because it is somebody else's. - var unrelated = new byte[146]; - unrelated[0] = 0x59; + // ReadDTCInformation response (SID 0x59) and never finishes it: only its First Frame goes + // on the bus. On a virtual clock (#171) the client's N_Cr cannot end that reception, so + // it stays in progress for as long as the test runs. P2 is then moved past on the clock: + // the client must time out, because that transfer is somebody else's answer -- the + // sibling of P2_Ends_With_The_First_Frame_Of_A_MultiFrame_Response, where the transfer + // in progress is this request's own and does extend the wait. A client fooled into + // waiting for it never returns, and the test's own token ends it with a cancellation, + // not a timeout: no bound on the wall clock is needed to tell the two apart. var p2 = TimeSpan.FromMilliseconds(500); + ClockPair? stack = null; - using var pair = new ClockPair( + using var pair = stack = new ClockPair( e => e.On(0x22, req => { - _ = e.Channel.SendAsync(unrelated); + // FF of a 146-byte (0x92) response for SID 0x59; the Consecutive Frames never follow. + stack!.EcuBus.Transmit(CanFrame.Classic(0x7E8, + new byte[] { 0x10, 0x92, 0x59, 0x00, 0x00, 0x00, 0x00, 0x00 }, isExtendedFrame: false)); throw new EcuSilent(); }), options: new UdsClientOptions { P2ClientMax = p2, P2StarClientMax = p2 }, @@ -606,12 +660,10 @@ public async Task P2_Is_Not_Extended_By_A_MultiFrame_Transfer_For_Another_Servic UsePadding = true, NAs = TimeSpan.FromMilliseconds(500), NBs = TimeSpan.FromMilliseconds(500), - NCr = TimeSpan.FromSeconds(10), - LocalStMin = TimeSpan.FromMilliseconds(127), + NCr = TimeSpan.FromSeconds(10), // longer than the test: the reception outlives P2 }); using var cts = new CancellationTokenSource(ShortTimeout); - var sw = Stopwatch.StartNew(); var read = pair.Client.ReadDataByIdentifierAsync(0xF190, cts.Token); var deadline = DateTime.UtcNow + ShortTimeout; @@ -621,21 +673,12 @@ public async Task P2_Is_Not_Extended_By_A_MultiFrame_Transfer_For_Another_Servic await Task.Delay(1); } await pair.Clock.AdvanceAsync(p2 + TimeSpan.FromMilliseconds(100)); // P2 runs out mid-transfer - // The unrelated transfer is really still in progress: otherwise the result below would - // hold for a client that has nothing left to be fooled by. - pair.Channel.GetReceptionsInProgress().Should().NotBeEmpty(); + pair.Channel.GetReceptionsInProgress().Should().NotBeEmpty( + "the unrelated transfer is still in progress when P2 runs out"); Func act = () => read; await act.Should().ThrowAsync( "an unrelated transfer must not hold the request past its budget"); - sw.Stop(); - // The unrelated transfer's remaining CFs are paced by the ECU's own (real) STmin -- - // 127 ms x 19 more CFs, well over a second -- so a client that keeps waiting for it (by - // treating it as this request's own answer) is caught here, not just by the exception - // type: both a correct and a fooled client eventually throw UdsTimeoutException, only - // the fooled one does so after the whole transfer has played out in real time. - sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(1), - "an unrelated transfer must not hold the request past its budget"); } // ----------------------------------------------------------------------------------- From ae072faddcbf31ddfeda5d5becd4e1fae7098020 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 04:48:10 +0000 Subject: [PATCH 8/8] test(uds): cover the clock overload and the wall-clock busy delay Codecov on #189 reported three partial lines: - The internal UdsClient.Create(channel, clock, ...) overload was never called with a null channel or with omitted options. - DelayAsync's wall-clock branch was never reached, because no existing test uses a non-zero BusyRepeatRequestDelay without an injected clock. Three small tests cover those paths. The wall-clock one asserts only that the repeated request succeeds. How long the delay lasts is what A_Busy_Repeat_Waits_Its_Delay_On_The_Clients_Clock proves. Refs #171 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s --- .../TestCases/Uds/UdsClientTests.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs b/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs index b86a81e..49888aa 100644 --- a/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/Uds/UdsClientTests.cs @@ -530,6 +530,48 @@ await client.SecurityAccessAsync( } } + // The same repeat without an injected clock, where the delay is a real Task.Delay: the + // request goes out again and succeeds. No timing is asserted -- how long the delay lasts is + // what the clock test below proves; this only runs the wall-clock path end to end. + [Fact] + public async Task A_Busy_Repeat_With_A_Delay_Repeats_On_The_Wall_Clock() + { + var calls = 0; + var (client, _, dispose) = BuildPair( + e => e.On(0x22, req => Interlocked.Increment(ref calls) == 1 + ? throw new EcuNegativeResponse(0x21) + : new byte[] { 0xF1, 0x90, 0xAA }), + options: new UdsClientOptions { BusyRepeatRequestDelay = TimeSpan.FromMilliseconds(10) }); + using (dispose) + { + using var cts = new CancellationTokenSource(ShortTimeout); + (await client.ReadDataByIdentifierAsync(0xF190, cts.Token)).Should().Equal(0xAA); + Volatile.Read(ref calls).Should().Be(2); + } + } + + // The clock-injecting overload (#171) guards its channel as the public one does. + [Fact] + public void Create_On_A_Clock_Rejects_A_Null_Channel() + { + using var clock = new VirtualClock(); + Action act = () => UdsClient.Create(null!, clock.NewActor()); + act.Should().Throw().WithParameterName("channel"); + } + + // Omitted options default as the public overload's do. + [Fact] + public void Create_On_A_Clock_Defaults_Omitted_Options() + { + using var clock = new VirtualClock(); + var actor = clock.NewActor(); + using var service = new StarvedReaderBusService(); + using var channel = new IsoTpChannel(service, IsoTpEndpoint.Normal(0x7E0, 0x7E8), + FastIsoTp(useCanFd: false), ownsService: false, actor); + using var client = UdsClient.Create(channel, actor); + client.Options.P2ClientMax.Should().Be(new UdsClientOptions().P2ClientMax); + } + // #57 on a virtual clock (#171): NRC 0x21 repeats the request after BusyRepeatRequestDelay, // measured on the client's clock like its P2 -- the repeat goes out only once the test has // moved the clock past the delay.