Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions src/CanKit.Pro.J1939/J1939NodeImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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); }
Comment thread
dborgards marked this conversation as resolved.
}

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,
Expand Down
33 changes: 33 additions & 0 deletions tests/CanKit.Pro.Tests/TestCases/J1939/J1939NodeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Task> failed = () => claim.WithTimeout(ShortTimeout);
var thrown = await failed.Should().ThrowAsync<J1939NodeException>();
thrown.Which.Should().NotBeOfType<J1939CannotClaimException>();
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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -4753,6 +4784,8 @@ public async Task<TxConfirmation> 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");
}
Expand Down
Loading