diff --git a/src/CanKit.Pro.J1939/J1939NodeImpl.cs b/src/CanKit.Pro.J1939/J1939NodeImpl.cs index b4ba1f9f..5bab579c 100644 --- a/src/CanKit.Pro.J1939/J1939NodeImpl.cs +++ b/src/CanKit.Pro.J1939/J1939NodeImpl.cs @@ -9,6 +9,7 @@ using CanKit.Abstractions.API.Can.Definitions; using CanKit.Abstractions.API.Common; using CanKit.Abstractions.API.Common.Definitions; +using CanKit.Core.Exceptions; using CanKit.Pro.Actor; using CanKit.Pro.Addressing; using CanKit.Pro.J1939Tp; @@ -985,13 +986,22 @@ private async Task TransmitAddressClaimConfirmedAsync(uint canId, byte[] payload try { _actor.Post(() => OnClaimAnnounceTxConfirmed(preferred)); } catch (ObjectDisposedException) { /* the node was disposed: no loop to tell */ } } - catch (Exception ex) - { - // SendConfirmedAsync threw rather than returning a timeout: the scripted and driver - // failures that do this have not put the frame on the wire. - try { _actor.Post(() => OnClaimAnnounceTxFailed(preferred, ex, echo, mayStillEcho: false)); } - catch (ObjectDisposedException) { /* the node was disposed: no loop to tell */ } - } + // SendConfirmedAsync threw rather than returning a timeout. The claim has to fail on every + // one of these: the arbitration deadline is armed only from the success callback, so a + // throw nobody reports leaves ClaimAddressAsync pending in Claiming for good. Cancellation + // is the one CanBusService.Dispose produces for a send still waiting for its confirmation; + // CanKitException is what a device adapter raises. The scripted and driver failures that + // throw have not put the frame on the wire. + catch (ObjectDisposedException ex) { PostClaimAnnounceTxThrew(preferred, ex, echo); } + catch (InvalidOperationException ex) { PostClaimAnnounceTxThrew(preferred, ex, echo); } + catch (OperationCanceledException ex) { PostClaimAnnounceTxThrew(preferred, ex, echo); } + catch (CanKitException ex) { PostClaimAnnounceTxThrew(preferred, ex, echo); } + } + + private void PostClaimAnnounceTxThrew(byte preferred, Exception ex, ClaimEcho? echo) + { + try { _actor.Post(() => OnClaimAnnounceTxFailed(preferred, ex, echo, mayStillEcho: false)); } + catch (ObjectDisposedException) { /* the node was disposed: no loop to tell */ } } private void SetClaimState(J1939ClaimState state, byte? address, byte? contendingSa, diff --git a/tests/CanKit.Pro.Tests/TestCases/J1939/J1939NodeTests.cs b/tests/CanKit.Pro.Tests/TestCases/J1939/J1939NodeTests.cs index 179a010b..7a093956 100644 --- a/tests/CanKit.Pro.Tests/TestCases/J1939/J1939NodeTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/J1939/J1939NodeTests.cs @@ -514,6 +514,35 @@ public async Task A_Failed_Claim_Transmit_Drops_Its_Echo_Marker() node.Address.Should().BeNull(); } + // The claim transmit is fire-and-forget and the arbitration deadline is armed only from its + // callback. A send that throws instead of returning a confirmation must still reach + // OnClaimAnnounceTxFailed, or ClaimAddressAsync never completes. CanBusService.Dispose + // cancels a pending send, so cancellation is one of those throws, not just the + // InvalidOperationException a driver raises. + [Theory] + [InlineData(ScriptedClaimBus.ReleaseKind.Throw)] + [InlineData(ScriptedClaimBus.ReleaseKind.Cancel)] + public async Task A_Claim_Transmit_That_Throws_Fails_The_Claim_Instead_Of_Hanging_It( + ScriptedClaimBus.ReleaseKind how) + { + using var bus = ControllableBus.EchoCapable(NewSession()); + using var raw = new CanBusService(bus); + using var scripted = new ScriptedClaimBus(raw, ScriptedClaimBus.Script.HoldFirstClaim); + using var node = J1939Node.Open(scripted, new J1939NodeOptions(Name(1)) + { + ClaimAnnounceTimeout = TimeSpan.FromMilliseconds(80), + }); + + var claim = node.ClaimAddressAsync(0x11); + await scripted.Held.AsTaskWithTimeout(ShortTimeout); + scripted.Release(how); + + Func failed = () => claim.WithTimeout(ShortTimeout); + var thrown = await failed.Should().ThrowAsync(); + thrown.Which.Should().NotBeOfType(); + node.ClaimState.Should().Be(J1939ClaimState.NotClaimed); + } + // SendConfirmedAsync timing out is not the same as a reject: the frame may already be on the // wire. Dropping its marker makes that late echo a remembered equal NAME, and the retry // of the same address loses before anyone is actually contending. The echo has to be @@ -4641,6 +4670,8 @@ public enum ReleaseKind Rejected, Throw, Timeout, + // What CanBusService.Dispose does to a send that is still waiting for its confirmation. + Cancel, } private readonly ICanBusService _inner; @@ -4753,6 +4784,8 @@ public async Task SendConfirmedAsync(CanFrame frame, TimeSpan? t return Rejected(); case ReleaseKind.Timeout: return TimedOut(); + case ReleaseKind.Cancel: + throw new TaskCanceledException("address claim transmit cancelled"); default: throw new InvalidOperationException("address claim transmit failed"); }