diff --git a/src/CanKit.Pro.J1939/IJ1939Node.cs b/src/CanKit.Pro.J1939/IJ1939Node.cs index b239d3e..837444b 100644 --- a/src/CanKit.Pro.J1939/IJ1939Node.cs +++ b/src/CanKit.Pro.J1939/IJ1939Node.cs @@ -79,9 +79,17 @@ public interface IJ1939Node : IDisposable, IAsyncDisposable /// per SAE J1939-81 §4.4.3.4 and throw /// (SRS FR-J1939-004). /// + /// The Cannot Claim, and an arbitrary-address node's next claim after losing, go out after + /// the pseudo-random 0..153 ms backoff of SAE J1939-81 §4.4.4.3 -- the low byte of the + /// NAME's bytes summed, times 0.6 ms -- so two nodes colliding on an address do not answer in + /// lockstep (#58). The task faults after that Cannot Claim has gone out, in the order step + /// 3 gives, so disposing the node on the exception cannot suppress the frame; a further + /// claim started before it drops the Cannot Claim and faults this one at once. /// /// The preferred address was lost to a /// higher-priority NAME and no fallback was available. + /// A claim is still in arbitration: it is + /// not silently replaced -- await it, or cancel it, before claiming again (#58). Task ClaimAddressAsync(byte preferredAddress, CancellationToken cancellationToken = default); /// diff --git a/src/CanKit.Pro.J1939/J1939NodeImpl.cs b/src/CanKit.Pro.J1939/J1939NodeImpl.cs index c4c363e..c6ae575 100644 --- a/src/CanKit.Pro.J1939/J1939NodeImpl.cs +++ b/src/CanKit.Pro.J1939/J1939NodeImpl.cs @@ -307,14 +307,130 @@ private void BeginClaim(byte preferredAddress, TaskCompletionSource tcs return; } - // Cancel any previous in-flight claim. - _pendingClaim?.Deadline?.Dispose(); - _pendingClaim?.Tcs.TrySetCanceled(); - _pendingClaim?.CtRegistration.Dispose(); + // A Cannot Claim still waiting its backoff is overtaken by this claim (Codex on #153). + // Dropping it settles the loss that scheduled it just as finally as sending it would, + // so the caller waiting on that loss is answered here. + _cannotClaimBackoff?.Dispose(); + _cannotClaimBackoff = null; + CompleteLostClaim(); + + // A claim still in arbitration is not silently replaced: the caller who started it + // is waiting on it, and a second one is a programming error (#58). A re-claim on a + // node that holds an address, or holds none, is what this method is for. A pending + // claim whose task has already completed -- cancelled by its caller, whose cancel is + // still on its way to this loop -- is over, and is swept here so that awaiting the + // cancellation and claiming again is race-free. + if (_pendingClaim is { } inFlight) + { + if (!inFlight.Tcs.Task.IsCompleted) + { + ctr.Dispose(); + tcs.TrySetException(new InvalidOperationException( + $"An address claim for SA 0x{inFlight.PreferredAddress:X2} is in arbitration; await it, or cancel it, before claiming again.")); + return; + } + _pendingClaim = null; + inFlight.Deadline?.Dispose(); + inFlight.CtRegistration.Dispose(); + } BeginClaimRound(preferredAddress, tcs, ctr); } + // J1939-81 §4.4.4.3: a node that lost arbitration delays its Cannot Claim, and an + // arbitrary-address node its next claim, by a pseudo-random 0..153 ms derived from its + // NAME -- the low byte of the eight bytes' sum, times 0.6 ms, so 255 slots reach the + // 153 ms endpoint (Codex on #153: modulo 255 mapped a sum of 255 to zero) -- so two nodes + // colliding on an address do not answer in lockstep for ever (#58). Fixed by the NAME, so + // a test can choose it; scheduled on the actor, so the state it acts on is the state it + // read. + private TimeSpan ClaimBackoff + { + get + { + int sum = 0; + foreach (var b in _name.ToBytes()) sum += b; + return TimeSpan.FromMilliseconds((sum & 0xFF) * 0.6); + } + } + + // The delayed Cannot Claim of a lost arbitration, tied to the loss that scheduled it: a + // new claim cancels it -- its announcement is the newer word on the bus, and a Cannot + // Claim after it would retract it -- and a second loss reschedules a full backoff rather + // than inheriting the remainder of the first (Codex on #153, twice). The state check is + // the second line. + private IDeadline? _cannotClaimBackoff; + + private void ScheduleCannotClaim() + { + _cannotClaimBackoff?.Dispose(); + _cannotClaimBackoff = AfterClaimBackoff(SendCannotClaimIfStillDue); + } + + // The answer a node without an address owes a Request for Address Claimed waits the same + // backoff, and for a reason the loss path only shares: a Cannot Claim carries the null + // address, so two nodes answering one global request at the same instant put identical + // CAN IDs with different NAMEs on the bus, which arbitration cannot separate (SAE + // J1939-81 §4.4.4.3; Codex on #153). An answer already waiting is the answer -- sending + // now would both bypass that delay and make the armed one a second copy. + private void AnswerRequestWithCannotClaim() + { + if (_cannotClaimBackoff is not null) return; + _cannotClaimBackoff = AfterClaimBackoff(SendCannotClaimIfStillDue); + } + + private void SendCannotClaimIfStillDue() + { + _cannotClaimBackoff = null; + // Claimed: the claim is the newer word and a Cannot Claim would retract it. Claiming: + // the round in hand announces, and a claim overtook this one (Codex on #153). Neither + // is reachable without the arming path having disposed the handle; this is the second + // line. NotClaimed passes, because that is a node that never claimed answering a scan. + var state = (J1939ClaimState)Volatile.Read(ref _claimStateStore); + if (state is not (J1939ClaimState.Claimed or J1939ClaimState.Claiming)) + SendAddressClaimFrame(sourceAddress: J1939Pgn.NullAddress); + CompleteLostClaim(); + } + + // The claim of a loss that owes the bus a Cannot Claim: it faults once that frame has gone + // out, not when the loss became known. A caller that disposes the node on the exception -- + // a `using` scope ending on it -- would otherwise tear the actor down inside the backoff, + // and the frame the loss owes would never be sent at all (Codex on #153). + private PendingClaim? _lostClaim; + + private void CompleteLostClaim() + { + var lost = _lostClaim; + if (lost is null) return; + _lostClaim = null; + lost.CtRegistration.Dispose(); + lost.Tcs.TrySetException(new J1939CannotClaimException(lost.PreferredAddress)); + } + + private IDeadline? AfterClaimBackoff(Action onLoop) + { + var delay = ClaimBackoff; + if (delay <= TimeSpan.Zero) { onLoop(); return null; } + return _deadlines.Arm(delay, () => { if (_disposed == 0) onLoop(); }); + } + + // Registers `retry` as the claim in hand and starts its round after the backoff; a Request + // for Address Claimed or a contest for the candidate starts it at once (Codex on #153). + private void BeginClaimRoundAfterBackoff(PendingClaim retry, byte candidate, byte? scanStart) + { + _pendingClaim = retry; + void Start() + { + if (!ReferenceEquals(_pendingClaim, retry) || retry.Tcs.Task.IsCompleted) return; + retry.StartRound = null; + retry.Deadline?.Dispose(); + retry.Deadline = null; + BeginClaimRound(candidate, retry.Tcs, retry.CtRegistration, scanStart); + } + retry.StartRound = Start; + retry.Deadline = AfterClaimBackoff(Start); + } + // Starts (or restarts, for the arbitrary-address fallback) a single arbitration round for // `preferredAddress` on the actor loop. Unlike BeginClaim it does not cancel the caller's // pending claim — the same TCS (and the scan state) is carried across retries. @@ -523,34 +639,48 @@ private void HandleIncomingAddressClaim(byte peerSa, byte[] payload) // Arbitrary-address fallback (FR-J1939-004 / SAE J1939-81 §4.5): retry with the // next candidate from the arbitrary address field before giving up with // Cannot-Claim. - _pendingClaim = null; pending.Deadline?.Dispose(); + pending.Deadline = null; var scanStart = pending.ArbitraryScanStart; if (ArbitraryClaimingEnabled && TryGetNextArbitraryCandidate(pending.PreferredAddress, ref scanStart, out var nextCandidate)) { // The caller's TCS and its cancellation registration stay alive across - // retries; only the arbitration round is restarted. - BeginClaimRound(nextCandidate, pending.Tcs, pending.CtRegistration, scanStart); + // retries; only the arbitration round is restarted. The pending claim stays + // registered through the backoff, so a second ClaimAddressAsync meanwhile + // meets the in-flight guard, and the delayed round runs only if it is still + // the claim in hand -- not one cancelled or replaced meanwhile (Codex on + // #153). The lost address is no longer announced as ours meanwhile. + WriteAddress(null); + SetClaimState(J1939ClaimState.Claiming, nextCandidate, contendingSa: null, contendingName: null); + var retry = new PendingClaim(nextCandidate, pending.Tcs, deadline: null, pending.CtRegistration) + { + ArbitraryScanStart = scanStart, + }; + BeginClaimRoundAfterBackoff(retry, nextCandidate, scanStart); return; } - pending.CtRegistration.Dispose(); + _pendingClaim = null; WriteAddress(null); // TP channel goes back to placeholder 0xFE — no directed TP traffic reaches // us while unclaimed. RebindTransportOnLoop(J1939Pgn.NullAddress); SetClaimState(J1939ClaimState.CannotClaim, address: null, contendingSa: peerSa, contendingName: peerName); - SendAddressClaimFrame(sourceAddress: J1939Pgn.NullAddress); - pending.Tcs.TrySetException(new J1939CannotClaimException(pending.PreferredAddress)); + // The caller is answered by the backoff, once the Cannot Claim is on the bus. + _lostClaim = pending; + ScheduleCannotClaim(); return; } // Peer's NAME is >= ours: they lose. Re-announce our own claim so they hear it, - // then keep waiting on our deadline. - SendAddressClaimFrame(sourceAddress: pending.PreferredAddress); + // then keep waiting on our deadline. A round still waiting its backoff has nothing + // to re-announce yet: it starts now, and its announcement is the answer (Codex on + // #153). + if (pending.BackingOff) pending.StartRound!(); + else SendAddressClaimFrame(sourceAddress: pending.PreferredAddress); return; } @@ -572,7 +702,16 @@ private void HandleIncomingAddressClaim(byte peerSa, byte[] payload) var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _ = tcs.Task.ContinueWith(t => RaiseBackgroundException(t.Exception!.GetBaseException()), CancellationToken.None, TaskContinuationOptions.OnlyOnFaulted, TaskScheduler.Default); - BeginClaimRound(nextCandidate, tcs, default, scanStart); + // The lost address is the peer's from this instant: invalidated now, so no + // application traffic goes out under it during the backoff; only the next + // round waits. Registered as the claim in hand meanwhile, so a + // ClaimAddressAsync during the backoff meets the in-flight guard, and the + // delayed round runs only if it still is (Codex on #153). + WriteAddress(null); + RebindTransportOnLoop(J1939Pgn.NullAddress); + SetClaimState(J1939ClaimState.Claiming, nextCandidate, contendingSa: null, contendingName: null); + var unseated = new PendingClaim(nextCandidate, tcs, deadline: null, default) { ArbitraryScanStart = scanStart }; + BeginClaimRoundAfterBackoff(unseated, nextCandidate, scanStart); return; } // Broadcast Cannot-Claim and transition. @@ -580,7 +719,7 @@ private void HandleIncomingAddressClaim(byte peerSa, byte[] payload) RebindTransportOnLoop(J1939Pgn.NullAddress); SetClaimState(J1939ClaimState.CannotClaim, address: null, contendingSa: peerSa, contendingName: peerName); - SendAddressClaimFrame(sourceAddress: J1939Pgn.NullAddress); + ScheduleCannotClaim(); } else { @@ -599,11 +738,14 @@ private void AnswerRequestForAddressClaimed() break; case J1939ClaimState.Claiming when _pendingClaim is { } pending: // Arbitrating: the claim for the preferred address is the answer, and a peer - // that hears it contests it now rather than after the window (§4.4.3). - SendAddressClaimFrame(sourceAddress: pending.PreferredAddress); + // that hears it contests it now rather than after the window (§4.4.3). A round + // still waiting its backoff has announced nothing; the request is the moment to + // -- it starts the round, which announces (Codex on #153). + if (pending.BackingOff) pending.StartRound!(); + else SendAddressClaimFrame(sourceAddress: pending.PreferredAddress); break; default: - SendAddressClaimFrame(sourceAddress: J1939Pgn.NullAddress); + AnswerRequestWithCannotClaim(); break; } } @@ -631,31 +773,36 @@ private void TransmitAddressClaimConfirmed(byte sourceAddress) var payload = BuildAddressClaimPayload(); uint canId = J1939Id.ComposePgn(_options.ClaimPriority, J1939Pgn.AddressClaimed, sourceAddress, destinationAddress: J1939Pgn.GlobalAddress); - byte preferred = sourceAddress; - _ = Task.Run(async () => - { - try - { - using var frame = CanFrame.Classic(unchecked((int)canId), payload, isExtendedFrame: true); - var confirmation = await _service.SendConfirmed(frame).ConfigureAwait(false); - if (!confirmation.Confirmed) - { - var ex = new J1939NodeException( - $"J1939 address claim TX failed (id=0x{canId:X8}): {confirmation.FailureReason}."); - try { _actor.Post(() => OnClaimAnnounceTxFailed(preferred, ex)); } - catch (ObjectDisposedException) { } - return; - } + // Called directly rather than through Task.Run: SendConfirmed's synchronous part is + // the driver hand-off, and its continuation already runs off this loop; a pool hop + // in front of it bought nothing and cost one per claim round -- a full arbitrary- + // address scan is 240 of them (#58). + _ = TransmitAddressClaimConfirmedAsync(canId, payload, sourceAddress); + } - try { _actor.Post(() => OnClaimAnnounceTxConfirmed(preferred)); } - catch (ObjectDisposedException) { } - } - catch (Exception ex) + private async Task TransmitAddressClaimConfirmedAsync(uint canId, byte[] payload, byte preferred) + { + try + { + using var frame = CanFrame.Classic(unchecked((int)canId), payload, isExtendedFrame: true); + var confirmation = await _service.SendConfirmed(frame).ConfigureAwait(false); + if (!confirmation.Confirmed) { + var ex = new J1939NodeException( + $"J1939 address claim TX failed (id=0x{canId:X8}): {confirmation.FailureReason}."); try { _actor.Post(() => OnClaimAnnounceTxFailed(preferred, ex)); } - catch (ObjectDisposedException) { } + catch (ObjectDisposedException) { /* the node was disposed: no loop to tell */ } + return; } - }); + + try { _actor.Post(() => OnClaimAnnounceTxConfirmed(preferred)); } + catch (ObjectDisposedException) { /* the node was disposed: no loop to tell */ } + } + catch (Exception ex) + { + try { _actor.Post(() => OnClaimAnnounceTxFailed(preferred, ex)); } + catch (ObjectDisposedException) { /* the node was disposed: no loop to tell */ } + } } private void SetClaimState(J1939ClaimState state, byte? address, byte? contendingSa, @@ -887,22 +1034,24 @@ private void TransmitFrame(uint canId, byte[] payload) { // Fire-and-forget: address-claim traffic doesn't need a task, but we still want a // background exception if the driver rejects it. SendConfirmed is used consistently - // with the rest of the CanKit.Pro stack. - _ = Task.Run(async () => + // with the rest of the CanKit.Pro stack. No Task.Run hop in front of it (#58). + _ = TransmitFrameAsync(canId, payload); + } + + private async Task TransmitFrameAsync(uint canId, byte[] payload) + { + try { - try - { - using var frame = CanFrame.Classic(unchecked((int)canId), payload, isExtendedFrame: true); - var confirmation = await _service.SendConfirmed(frame).ConfigureAwait(false); - if (!confirmation.Confirmed) - RaiseBackgroundException(new J1939NodeException( - $"J1939 frame TX failed (id=0x{canId:X8}): {confirmation.FailureReason}.")); - } - catch (Exception ex) - { - RaiseBackgroundException(ex); - } - }); + using var frame = CanFrame.Classic(unchecked((int)canId), payload, isExtendedFrame: true); + var confirmation = await _service.SendConfirmed(frame).ConfigureAwait(false); + if (!confirmation.Confirmed) + RaiseBackgroundException(new J1939NodeException( + $"J1939 frame TX failed (id=0x{canId:X8}): {confirmation.FailureReason}.")); + } + catch (Exception ex) + { + RaiseBackgroundException(ex); + } } // ========================================================================================= @@ -1165,6 +1314,9 @@ public void Dispose() { _actor.Post(() => { + // A loss whose Cannot Claim never got its backoff still answers its caller: + // the claim failed, and no dispose makes that less true (Codex on #153). + CompleteLostClaim(); var pending = _pendingClaim; if (pending is not null) { @@ -1256,6 +1408,13 @@ public PendingClaim(byte preferredAddress, TaskCompletionSource tcs, ID /// null while no fallback round has run yet. Carried across retries via /// BeginClaimRound. public byte? ArbitraryScanStart { get; set; } + /// + /// The round waits the §4.4.4.3 backoff and has announced nothing yet; + /// is the backoff, and starts the round -- now, when a Request for + /// Address Claimed or a contest for the candidate makes waiting pointless (Codex on #153). + /// + public Action? StartRound { get; set; } + public bool BackingOff => StartRound is not null; } /// diff --git a/src/CanKit.Pro.J1939/J1939NodeOptions.cs b/src/CanKit.Pro.J1939/J1939NodeOptions.cs index 5db6a45..efb3085 100644 --- a/src/CanKit.Pro.J1939/J1939NodeOptions.cs +++ b/src/CanKit.Pro.J1939/J1939NodeOptions.cs @@ -58,9 +58,9 @@ public J1939NodeOptions(J1939Name name) public int ReceiveBufferCapacity { get; init; } = 128; /// - /// Options forwarded to the shared for multi-frame payloads - /// (SRS FR-J1939-006). Defaults to a fresh . Ignored when the - /// caller supplies their own pre-built channel to the factory. + /// Options for the the node opens for multi-frame payloads + /// (SRS FR-J1939-006). Defaults to a fresh . The node always + /// opens that channel itself; no factory overload takes a pre-built one (#58). /// public J1939TpOptions TransportOptions { get; init; } = new J1939TpOptions(); diff --git a/src/CanKit.Pro.J1939/README.md b/src/CanKit.Pro.J1939/README.md index 5928620..7e6aa9e 100644 --- a/src/CanKit.Pro.J1939/README.md +++ b/src/CanKit.Pro.J1939/README.md @@ -23,12 +23,22 @@ FR-J1939-001..006 (Must) and FR-J1939-007 (Should). 0xEE00 from SA = 0xFE) when the field is exhausted. Governed by `J1939NodeOptions.EnableArbitraryAddressClaiming` (default: derived from the NAME's Arbitrary Address Capable bit). A move after a successful claim - is announced through `AddressClaimChanged`; nobody awaits it. + is announced through `AddressClaimChanged`; nobody awaits it. The Cannot + Claim, and the next claim after losing, go out after the pseudo-random + 0..153 ms backoff of SAE J1939-81 §4.4.4.3 (the low byte of the NAME's bytes + summed, times 0.6 ms), so two nodes colliding on an address do not answer in + lockstep; and a second `ClaimAddressAsync` while one is in arbitration faults + with `InvalidOperationException` rather than silently cancelling the first (#58). - **Request for Address Claimed** (SAE J1939-81 §4.2.2): a Request for PGN 0xEE00 is answered by the node itself — with its Address Claimed while it holds or arbitrates an address, with Cannot Claim while it holds none — so - a network-management tool scanning the bus sees it. The request still - reaches `MessageReceived`. + a network-management tool scanning the bus sees it. A Cannot Claim answer + waits the §4.4.4.3 backoff below and shares it with the one a lost claim + owes, since every Cannot Claim carries the same null source address; a + claim still waiting its backoff answers by starting its round. The request + still reaches `MessageReceived`. `ClaimAddressAsync` faults only once the + Cannot Claim it owes has gone out, so a caller that disposes the node on the + exception cannot suppress it. - **Request-PGN** (PGN 0xEA00) send and receive (**FR-J1939-005**). - **Auto-routing** to J1939-TP for payloads > 8 bytes; direct 29-bit frames for payloads ≤ 8 bytes (**FR-J1939-006**). diff --git a/tests/CanKit.Pro.Tests/TestCases/J1939/J1939NodeTests.cs b/tests/CanKit.Pro.Tests/TestCases/J1939/J1939NodeTests.cs index c8d03bd..15e9b95 100644 --- a/tests/CanKit.Pro.Tests/TestCases/J1939/J1939NodeTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/J1939/J1939NodeTests.cs @@ -306,6 +306,382 @@ public async Task CannotClaim_BroadcastsWithNullSourceAddress() J1939Pgn.IsAddressClaim(decomposed.Pgn).Should().BeTrue(); } + // #58 (SAE J1939-81 §4.4.4.3): a node that lost arbitration sends its Cannot Claim after a + // pseudo-random 0..153 ms derived from its NAME -- the low byte of the bytes' sum, times + // 0.6 ms -- so two nodes colliding on an address do not answer in lockstep. The loser's + // NAME here sums to exactly 255, the 153 ms endpoint that a modulo 255 would have mapped + // to zero (Codex on #153); the gap between the winner's re-announcement, which is what the + // loser answers, and the Cannot Claim is at least that, a lower bound a loaded host only + // raises. + [Fact] + public async Task CannotClaim_Is_Sent_After_The_Names_Pseudo_Random_Backoff() + { + var session = NewSession(); + using var busA = Open(session, 0); + using var busB = Open(session, 1); + using var busC = Open(session, 2); // spectator + + long reannouncedAt = 0, cannotClaimedAt = 0; + var winnerClaimed = new[] { false }; + var cannotClaimSeen = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + busC.FrameObserved += (_, e) => + { + if (!e.CanFrame.IsExtendedFrame) return; + var fields = J1939Id.Decompose((uint)e.CanFrame.ID); + if (!J1939Pgn.IsAddressClaim(fields.Pgn)) return; + if (fields.SourceAddress == 0x60 && Interlocked.Read(ref reannouncedAt) == 0 && Volatile.Read(ref winnerClaimed[0])) + Interlocked.Exchange(ref reannouncedAt, Stopwatch.GetTimestamp()); + if (fields.SourceAddress == J1939Pgn.NullAddress) + { + Interlocked.Exchange(ref cannotClaimedAt, Stopwatch.GetTimestamp()); + cannotClaimSeen.TrySetResult(true); + } + }; + + var loserName = Name(0x00015D); // byte sum 255: the 153 ms endpoint + loserName.ToBytes().Sum(b => (int)b).Should().Be(255, "the NAME was chosen for it"); + var loserBackoff = TimeSpan.FromMilliseconds(153); + + using var winner = J1939Node.Open(busA, new J1939NodeOptions(Name(0x000010)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(200) }); + using var loser = J1939Node.Open(busB, new J1939NodeOptions(loserName) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(200) }); + + await winner.ClaimAddressAsync(0x60).WithTimeout(ShortTimeout); + Volatile.Write(ref winnerClaimed[0], true); + Func act = () => loser.ClaimAddressAsync(0x60).WithTimeout(ShortTimeout); + await act.Should().ThrowAsync(); + await cannotClaimSeen.Task.AsTaskWithTimeout(ShortTimeout); + + Interlocked.Read(ref reannouncedAt).Should().NotBe(0, "the winner re-announced its claim, which is what the loser lost to"); + var gap = TimeSpan.FromSeconds((Interlocked.Read(ref cannotClaimedAt) - Interlocked.Read(ref reannouncedAt)) / (double)Stopwatch.Frequency); + gap.Should().BeGreaterThanOrEqualTo(loserBackoff - TimeSpan.FromMilliseconds(5), + "the Cannot Claim waited the NAME's backoff after the claim it lost to"); + } + + // Codex and Bugbot on #153: an arbitrary-address node unseated from a claimed address loses + // the address the instant the winning claim is heard; only its next claim waits the + // backoff. Measured from the winner's claim frame to the node's Claiming transition: at + // once, against a NAME whose backoff is 150 ms -- the reading a node that invalidated only + // when the delayed round ran would give -- with 75 ms to either. + [Fact] + public async Task An_Unseated_Node_Loses_The_Address_At_Once_And_Waits_Only_To_Reclaim() + { + var session = NewSession(); + using var busA = Open(session, 0); + using var busB = Open(session, 1); + const byte contended = 0x40; + + var ownerName = Name(0x000158); // backoff 150 ms + ((ownerName.ToBytes().Sum(b => (int)b) & 0xFF) * 0.6).Should().Be(150); + using var owner = J1939Node.Open(busA, new J1939NodeOptions(ownerName) + { + ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80), + EnableArbitraryAddressClaiming = true, + }); + using var winner = J1939Node.Open(busB, new J1939NodeOptions(Name(0x000010)) + { + ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80), + }); + await owner.ClaimAddressAsync(contended).WithTimeout(ShortTimeout); + + long winnerClaimAt = 0; + // What the handler reads is read in the actor's step that made the transition; read + // after the await, the address could be the re-claimed one already on a host that + // schedules the continuation late (macOS CI on #153). + var claiming = new TaskCompletionSource<(long At, byte? Address)>(TaskCreationOptions.RunContinuationsAsynchronously); + busA.FrameObserved += (_, e) => + { + if (!e.CanFrame.IsExtendedFrame) return; + var fields = J1939Id.Decompose((uint)e.CanFrame.ID); + if (J1939Pgn.IsAddressClaim(fields.Pgn) && fields.SourceAddress == contended && Interlocked.Read(ref winnerClaimAt) == 0 && !e.IsEcho) + Interlocked.Exchange(ref winnerClaimAt, Stopwatch.GetTimestamp()); + }; + owner.AddressClaimChanged += (_, e) => { if (e.State == J1939ClaimState.Claiming) claiming.TrySetResult((Stopwatch.GetTimestamp(), owner.Address)); }; + + await winner.ClaimAddressAsync(contended).WithTimeout(ShortTimeout); + var (claimingAt, addressThen) = await claiming.Task.AsTaskWithTimeout(ShortTimeout); + addressThen.Should().BeNull("the address is the winner's from the instant its claim was heard"); + Interlocked.Read(ref winnerClaimAt).Should().NotBe(0); + var reaction = TimeSpan.FromSeconds((claimingAt - Interlocked.Read(ref winnerClaimAt)) / (double)Stopwatch.Frequency); + reaction.Should().BeLessThan(TimeSpan.FromMilliseconds(75), "the invalidation does not wait the backoff"); + } + + // Codex and Bugbot on #153: a ClaimAddressAsync during the backoff before a re-claim meets + // the in-flight guard -- the claim in hand stays registered through the backoff -- and the + // re-claim completes for its caller. + [Fact] + public async Task A_Claim_During_The_Backoff_Before_A_Reclaim_Faults_And_The_Reclaim_Completes() + { + var session = NewSession(); + using var busA = Open(session, 0); + using var busB = Open(session, 1); + const byte contended = 0x81; + + using var winner = J1939Node.Open(busB, new J1939NodeOptions(Name(0x000010)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80) }); + await winner.ClaimAddressAsync(contended).WithTimeout(ShortTimeout); + using var node = J1939Node.Open(busA, new J1939NodeOptions(Name(0x000158)) // backoff 150 ms + { + ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80), + EnableArbitraryAddressClaiming = true, + }); + + var backingOff = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + node.AddressClaimChanged += (_, e) => { if (e.State == J1939ClaimState.Claiming && e.Address == contended + 1) backingOff.TrySetResult(true); }; + + var first = node.ClaimAddressAsync(contended); // lost to the winner; the scan moves on after the backoff + await backingOff.Task.AsTaskWithTimeout(ShortTimeout); + Func second = () => node.ClaimAddressAsync(0x90).WithTimeout(ShortTimeout); + await second.Should().ThrowAsync(); + + await first.WithTimeout(ShortTimeout); + node.Address.Should().Be((byte)(contended + 1)); + } + + // Codex and Bugbot on #153: a loser that claims again before its delayed Cannot Claim went + // out does not have it go out -- the new claim's announcement is the newer word on the bus. + [Fact] + public async Task A_Delayed_Cannot_Claim_Is_Dropped_Once_A_New_Claim_Has_Started() + { + var session = NewSession(); + using var busA = Open(session, 0); + using var busB = Open(session, 1); + using var busC = Open(session, 2); // spectator + + int cannotClaims = 0; + busC.FrameObserved += (_, e) => + { + if (!e.CanFrame.IsExtendedFrame) return; + var fields = J1939Id.Decompose((uint)e.CanFrame.ID); + if (J1939Pgn.IsAddressClaim(fields.Pgn) && fields.SourceAddress == J1939Pgn.NullAddress) Interlocked.Increment(ref cannotClaims); + }; + + using var winner = J1939Node.Open(busA, new J1939NodeOptions(Name(0x000010)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80) }); + using var loser = J1939Node.Open(busB, new J1939NodeOptions(Name(0x000158)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80) }); // backoff 150 ms + await winner.ClaimAddressAsync(0x60).WithTimeout(ShortTimeout); + + var lossSeen = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + loser.AddressClaimChanged += (_, e) => { if (e.State == J1939ClaimState.CannotClaim) lossSeen.TrySetResult(true); }; + + // The claim faults only once its Cannot Claim is on the bus, so the loss is awaited + // through the state change here -- the exception would come too late to claim inside + // the backoff it is waiting for. + var lost = loser.ClaimAddressAsync(0x60); + await lossSeen.Task.AsTaskWithTimeout(ShortTimeout); + await loser.ClaimAddressAsync(0x61).WithTimeout(ShortTimeout); // within the backoff, and through the new arbitration + + Func awaitLost = () => lost.WithTimeout(ShortTimeout); + await awaitLost.Should().ThrowAsync("dropping the Cannot Claim settles the loss that owed it"); + await Task.Delay(300); // past the backoff, with room + Volatile.Read(ref cannotClaims).Should().Be(0, "the Cannot Claim was overtaken by the new claim"); + loser.Address.Should().Be(0x61); + } + + // Codex on #153: the delayed Cannot Claim belongs to the loss that scheduled it. A second + // claim cancels the first's; when the second loses too, its Cannot Claim waits a full + // backoff from its own loss rather than the remainder of the first's -- measured from the + // second transition to CannotClaim, which is the instant the second loss is known, to the + // one SA 0xFE frame, a lower bound a loaded host only raises. + [Fact] + public async Task A_Second_Loss_Waits_Its_Own_Full_Backoff_Before_Cannot_Claim() + { + var session = NewSession(); + using var busA = Open(session, 0); + using var busB = Open(session, 1); + using var busC = Open(session, 2); // spectator + + long cannotClaimAt = 0; + var cannotClaimSeen = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + busC.FrameObserved += (_, e) => + { + if (!e.CanFrame.IsExtendedFrame) return; + var fields = J1939Id.Decompose((uint)e.CanFrame.ID); + if (J1939Pgn.IsAddressClaim(fields.Pgn) && fields.SourceAddress == J1939Pgn.NullAddress + && Interlocked.CompareExchange(ref cannotClaimAt, Stopwatch.GetTimestamp(), 0) == 0) + cannotClaimSeen.TrySetResult(true); + }; + + using var winner = J1939Node.Open(busA, new J1939NodeOptions(Name(0x000010)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80) }); + using var loser = J1939Node.Open(busB, new J1939NodeOptions(Name(0x000158)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80) }); // backoff 150 ms + await winner.ClaimAddressAsync(0x60).WithTimeout(ShortTimeout); + await Task.Delay(100); + await winner.ClaimAddressAsync(0x61).WithTimeout(ShortTimeout); // the winner holds 0x61 now; both losses are to it + + int losses = 0; + long secondLossAt = 0; + var lossSeen = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + loser.AddressClaimChanged += (_, e) => + { + if (e.State != J1939ClaimState.CannotClaim) return; + if (Interlocked.Increment(ref losses) == 2) Interlocked.Exchange(ref secondLossAt, Stopwatch.GetTimestamp()); + lossSeen.TrySetResult(true); + }; + + var first = loser.ClaimAddressAsync(0x61); // faults when its Cannot Claim goes out, or is dropped + await lossSeen.Task.AsTaskWithTimeout(ShortTimeout); + await Task.Delay(100); // most of the first backoff + Func second = () => loser.ClaimAddressAsync(0x61).WithTimeout(ShortTimeout); + await second.Should().ThrowAsync(); + Func awaitFirst = () => first.WithTimeout(ShortTimeout); + await awaitFirst.Should().ThrowAsync(); + Interlocked.Read(ref secondLossAt).Should().NotBe(0, "the second claim lost too"); + + await cannotClaimSeen.Task.AsTaskWithTimeout(ShortTimeout); + var gap = TimeSpan.FromSeconds((Interlocked.Read(ref cannotClaimAt) - Interlocked.Read(ref secondLossAt)) / (double)Stopwatch.Frequency); + gap.Should().BeGreaterThanOrEqualTo(TimeSpan.FromMilliseconds(145), + "the first loss's Cannot Claim was cancelled by the second claim, and the second loss waits its own backoff"); + } + + // Codex on #153: a round waiting its backoff has announced nothing, and answers a Request + // for Address Claimed by starting -- one announcement, now -- rather than by an + // announcement of its own with the round's to follow. + [Fact] + public async Task A_Request_During_The_Backoff_Starts_The_Round_With_A_Single_Announcement() + { + var session = NewSession(); + using var busA = Open(session, 0); + using var busB = Open(session, 1); + const byte contended = 0x81; + + using var winner = J1939Node.Open(busB, new J1939NodeOptions(Name(0x000010)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80) }); + await winner.ClaimAddressAsync(contended).WithTimeout(ShortTimeout); + using var node = J1939Node.Open(busA, new J1939NodeOptions(Name(0x000158)) // backoff 150 ms + { + ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80), + EnableArbitraryAddressClaiming = true, + }); + + int candidateClaims = 0; + busB.FrameObserved += (_, e) => + { + if (!e.CanFrame.IsExtendedFrame) return; + var fields = J1939Id.Decompose((uint)e.CanFrame.ID); + if (J1939Pgn.IsAddressClaim(fields.Pgn) && fields.SourceAddress == contended + 1) Interlocked.Increment(ref candidateClaims); + }; + var backingOff = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + node.AddressClaimChanged += (_, e) => { if (e.State == J1939ClaimState.Claiming && e.Address == contended + 1) backingOff.TrySetResult(true); }; + + var claim = node.ClaimAddressAsync(contended); + await backingOff.Task.AsTaskWithTimeout(ShortTimeout); + busB.Transmit(CanFrame.Classic( + (int)J1939Id.ComposePgn(6, J1939Pgn.Request, sourceAddress: 0x20, destinationAddress: J1939Pgn.GlobalAddress), + new byte[] { 0x00, 0xEE, 0x00 }, isExtendedFrame: true)); + + await claim.WithTimeout(ShortTimeout); + node.Address.Should().Be((byte)(contended + 1)); + await Task.Delay(300); // past the backoff: a round that still fired would announce again + Volatile.Read(ref candidateClaims).Should().Be(1, "the request started the round, whose announcement is the answer, and nothing announced twice"); + } + + // Codex on #153: the Cannot Claim of a loss is delayed, the exception was not, and a caller + // that ends a `using` scope on that exception disposed the node inside the backoff -- the + // frame the loss owed the bus was then never sent at all. ClaimAddressAsync now faults + // once the frame has gone out, so the loser here disposes as soon as it can and the + // spectator still sees it. + [Fact] + public async Task A_Lost_Claim_Faults_Only_Once_Its_Cannot_Claim_Is_On_The_Bus() + { + var session = NewSession(); + using var busA = Open(session, 0); + using var busB = Open(session, 1); + using var busC = Open(session, 2); // spectator: it outlives the loser + + int cannotClaims = 0; + busC.FrameObserved += (_, e) => + { + if (!e.CanFrame.IsExtendedFrame) return; + var fields = J1939Id.Decompose((uint)e.CanFrame.ID); + if (J1939Pgn.IsAddressClaim(fields.Pgn) && fields.SourceAddress == J1939Pgn.NullAddress) Interlocked.Increment(ref cannotClaims); + }; + + using var winner = J1939Node.Open(busA, new J1939NodeOptions(Name(0x000010)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80) }); + await winner.ClaimAddressAsync(0x63).WithTimeout(ShortTimeout); + var loser = J1939Node.Open(busB, new J1939NodeOptions(Name(0x00015D)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80) }); // backoff 153 ms + try + { + Func act = () => loser.ClaimAddressAsync(0x63).WithTimeout(ShortTimeout); + await act.Should().ThrowAsync(); + } + finally + { + loser.Dispose(); // the scope a caller ends on the exception + } + + await Task.Delay(300); // a backoff that survived the dispose would have fired by now + Volatile.Read(ref cannotClaims).Should().Be(1, + "the claim faulted only after its Cannot Claim went out, so disposing on the exception cannot suppress it"); + } + + // Codex on #153: a Cannot Claim carries the null address, so two nodes answering the same + // global Request for Address Claimed at the same instant put identical CAN IDs with + // different NAMEs on the bus. The answer a losing node owes therefore waits the same + // §4.4.4.3 backoff as the Cannot Claim of the loss itself -- and a request arriving inside + // that backoff is answered by that one frame, not by an immediate second copy. + [Fact] + public async Task A_Request_During_The_Cannot_Claim_Backoff_Is_Answered_By_That_One_Frame() + { + var session = NewSession(); + using var busA = Open(session, 0); + using var busB = Open(session, 1); + const byte contended = 0x62; + + int cannotClaims = 0; + long firstCannotClaimAt = 0; + busB.FrameObserved += (_, e) => + { + if (!e.CanFrame.IsExtendedFrame) return; + var fields = J1939Id.Decompose((uint)e.CanFrame.ID); + if (!J1939Pgn.IsAddressClaim(fields.Pgn) || fields.SourceAddress != J1939Pgn.NullAddress) return; + Interlocked.CompareExchange(ref firstCannotClaimAt, Stopwatch.GetTimestamp(), 0); + Interlocked.Increment(ref cannotClaims); + }; + + using var winner = J1939Node.Open(busB, new J1939NodeOptions(Name(0x000010)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80) }); + await winner.ClaimAddressAsync(contended).WithTimeout(ShortTimeout); + using var loser = J1939Node.Open(busA, new J1939NodeOptions(Name(0x00015D)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80) }); // backoff 153 ms + + long lostAt = 0; + var lossSeen = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + loser.AddressClaimChanged += (_, e) => + { + if (e.State != J1939ClaimState.CannotClaim) return; + Interlocked.CompareExchange(ref lostAt, Stopwatch.GetTimestamp(), 0); + lossSeen.TrySetResult(true); + }; + + // The claim's exception arrives with the Cannot Claim itself, so the request has to be + // sent from the loss, which is the instant the backoff starts. + var lost = loser.ClaimAddressAsync(contended); + await lossSeen.Task.AsTaskWithTimeout(ShortTimeout); + busB.Transmit(CanFrame.Classic( + (int)J1939Id.ComposePgn(6, J1939Pgn.Request, sourceAddress: 0x20, destinationAddress: J1939Pgn.GlobalAddress), + new byte[] { 0x00, 0xEE, 0x00 }, isExtendedFrame: true)); + + Func awaitLost = () => lost.WithTimeout(ShortTimeout); + await awaitLost.Should().ThrowAsync(); + await Task.Delay(500); // past the backoff, with room for a second copy to show up + Volatile.Read(ref cannotClaims).Should().Be(1, "the answer already waiting is the answer to the request"); + var waited = TimeSpan.FromSeconds((Interlocked.Read(ref firstCannotClaimAt) - Interlocked.Read(ref lostAt)) / (double)Stopwatch.Frequency); + waited.Should().BeGreaterThanOrEqualTo(TimeSpan.FromMilliseconds(100), + "the request did not shortcut the backoff -- a lower bound on the 153 ms a loaded host only lengthens"); + } + + // #58: a second ClaimAddressAsync while one is in arbitration faults instead of silently + // cancelling the first, whose caller is waiting on it. + [Fact] + public async Task A_Second_Claim_During_Arbitration_Faults_And_Leaves_The_First_Alone() + { + var session = NewSession(); + using var bus = Open(session, 0); + using var node = J1939Node.Open(bus, new J1939NodeOptions(Name(0x000030)) { ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(200) }); + + var first = node.ClaimAddressAsync(0x70); + Func second = () => node.ClaimAddressAsync(0x71).WithTimeout(ShortTimeout); + await second.Should().ThrowAsync(); + + await first.WithTimeout(ShortTimeout); + node.Address.Should().Be(0x70); + node.ClaimState.Should().Be(J1939ClaimState.Claimed); + } + // --------------------------------------------------------------------------------------- // #34 (SAE J1939-81 §4.2.2): a Request for PGN 0xEE00 is answered by the node itself — // with its Address Claimed while it holds an address, with Cannot-Claim (SA 0xFE) while it @@ -449,7 +825,10 @@ public async Task AddressClaim_ArbitraryFallback_ExhaustsField_ThenCannotClaim() using var busB = Open(session, 1); using var busC = Open(session, 2); // spectator for the final Cannot-Claim broadcast - var nodeName = Name(0x0000BB); + // A NAME whose §4.4.4.3 backoff is zero (the bytes sum to 256, low byte 0): the scan + // pays the backoff before every one of its 240 rounds, and this test's subject is the + // exhaustion, not the delay -- with a 56 ms backoff it took 13 s more (#153). + var nodeName = Name(0x00005F); var peerName = Name(0x000001); var opts = new J1939NodeOptions(nodeName) { @@ -1772,13 +2151,14 @@ public async Task A_Broadcast_Echo_Arriving_While_The_Node_Holds_No_Address_Is_S await claimB.WithTimeout(ShortTimeout); } - // #121 across claim rounds: a re-claim replaced while in flight, then completed for a third + // #121 across claim rounds: a re-claim cancelled while in flight, then completed for a third // address, and the echo of the frame sent under the first address still arrives after all of // it. The ledger is not touched by claim rounds at all, so nothing here can forget the frame; - // the test pins that the replacement does not either (#119 had a marker that a second round - // wiped). + // the test pins that the cancellation does not either (#119 had a marker that a second round + // wiped). Since #58 a second claim no longer replaces one in flight -- it faults -- so the + // first is cancelled by its caller here. [Fact] - public async Task An_Echo_Sent_Under_The_Vacated_Address_Is_Not_Raised_After_A_Replaced_Reclaim() + public async Task An_Echo_Sent_Under_The_Vacated_Address_Is_Not_Raised_After_A_Cancelled_Reclaim() { using var bus = ControllableBus.DeferredEchoCapable(NewSession()); using var node = J1939Node.Open(bus, new J1939NodeOptions(Name(1)) @@ -1801,12 +2181,14 @@ public async Task An_Echo_Sent_Under_The_Vacated_Address_Is_Not_Raised_After_A_R destinationAddress: J1939Pgn.GlobalAddress)); await bus.DeferredEchoes.WaitForEnqueuedAsync(2, ShortTimeout); - var superseded = node.ClaimAddressAsync(0x22); + using var cancel = new CancellationTokenSource(); + var superseded = node.ClaimAddressAsync(0x22, cancel.Token); await bus.DeferredEchoes.WaitForEnqueuedAsync(3, ShortTimeout); - var reclaim = node.ClaimAddressAsync(0x23); - await bus.DeferredEchoes.WaitForEnqueuedAsync(4, ShortTimeout); + cancel.Cancel(); Func awaitSuperseded = () => superseded.WithTimeout(ShortTimeout); await awaitSuperseded.Should().ThrowAsync(); + var reclaim = node.ClaimAddressAsync(0x23); + await bus.DeferredEchoes.WaitForEnqueuedAsync(4, ShortTimeout); bus.DeferredEchoes.DiscardNext().Should().BeTrue(); // the broadcast's echo, held back bus.DeferredEchoes.ReleaseAll(); // both claims' announcements