RioSerialLink: stop-and-wait command delivery (supersedes NAK-race resend)
Bench falsified the v1 retransmit (testlogs/riomash-patched-62500-retx): resends tracked NAKs 1:1 (321/321) yet lamps still stuck/missed — under mash bursts the NAK arrives after a newer command is already "latest", so the wrong packet was resent; and total shreds never NAK at all. Now commands are stop-and-wait: ONE in flight (_commandGate), resolved by ACK, NAK, or AckTimeout (50ms); NAK/timeout retransmits THE SAME packet up to CommandRetransmitLimit (2), then drops (idempotent - the next state update supersedes). Attribution is exact by construction and timeouts catch silent shreds. Control-byte replies bypass the gate so board traffic is never delayed; a request's own reply (analog/version/ check) also resolves the wait, so request/reply exchanges never burn the timeout even if the board sends no explicit ACK. 7 tests (same-packet resend, timeout retry+drop, ACK completion, serialization, reply-resolves-request, stray-NAK no-op, disable); 282 green. Mash summary now reports NAK/timeout resends. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -142,124 +142,151 @@ public class RioSerialLinkTests
|
||||
await run;
|
||||
}
|
||||
|
||||
// --- NAK retransmit (RioSerialLinkOptions.NakRetransmitLimit) ------------
|
||||
// --- stop-and-wait command retransmit (CommandRetransmitLimit) -----------
|
||||
|
||||
private static RioSerialLinkOptions StopAndWait(int limit = 2, int timeoutMs = 150) => new()
|
||||
{
|
||||
AutoPollAnalog = false,
|
||||
CommandRetransmitLimit = limit,
|
||||
AckTimeout = TimeSpan.FromMilliseconds(timeoutMs),
|
||||
};
|
||||
|
||||
[Fact]
|
||||
public async Task Nak_RetransmitsLastCommand()
|
||||
public async Task Nak_RetransmitsTheSamePacket()
|
||||
{
|
||||
var fake = new FakeTransport();
|
||||
var link = new RioSerialLink(fake, new RioSerialLinkOptions { AutoPollAnalog = false });
|
||||
var link = new RioSerialLink(fake, StopAndWait());
|
||||
|
||||
using var cts = new CancellationTokenSource();
|
||||
Task run = link.RunAsync(cts.Token);
|
||||
|
||||
byte[] lamp = PacketBuilder.LampRequest(0x05, 0x02);
|
||||
await link.SendAsync(lamp);
|
||||
Task send = link.SendAsync(lamp);
|
||||
|
||||
Assert.Equal(lamp, await fake.NextWriteAsync());
|
||||
fake.Enqueue((byte)RioControl.Nak); // board: "that arrived corrupt"
|
||||
Assert.Equal(lamp, await fake.NextWriteAsync()); // exact same packet again
|
||||
fake.Enqueue((byte)RioControl.Ack); // second copy lands
|
||||
|
||||
fake.Enqueue((byte)RioControl.Nak); // board: "that arrived corrupt"
|
||||
|
||||
Assert.Equal(lamp, await fake.NextWriteAsync()); // resent
|
||||
Assert.Equal(1, link.NakRetransmits);
|
||||
await send.WaitAsync(Timeout);
|
||||
Assert.Equal(1, link.Retransmits);
|
||||
|
||||
cts.Cancel();
|
||||
await run;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Nak_RetransmitStopsAtLimit()
|
||||
public async Task Timeout_RetransmitsWhenBoardNeverResponds()
|
||||
{
|
||||
var fake = new FakeTransport();
|
||||
var link = new RioSerialLink(fake, new RioSerialLinkOptions
|
||||
{
|
||||
AutoPollAnalog = false,
|
||||
NakRetransmitLimit = 2,
|
||||
});
|
||||
var link = new RioSerialLink(fake, StopAndWait(limit: 2, timeoutMs: 80));
|
||||
|
||||
using var cts = new CancellationTokenSource();
|
||||
Task run = link.RunAsync(cts.Token);
|
||||
|
||||
byte[] lamp = PacketBuilder.LampRequest(0x05, 0x02);
|
||||
await link.SendAsync(lamp);
|
||||
await fake.NextWriteAsync(); // original
|
||||
Task send = link.SendAsync(lamp);
|
||||
|
||||
for (int i = 0; i < 4; i++)
|
||||
fake.Enqueue((byte)RioControl.Nak);
|
||||
// Original + 2 timeout-driven retries, then the command is dropped.
|
||||
Assert.Equal(lamp, await fake.NextWriteAsync());
|
||||
Assert.Equal(lamp, await fake.NextWriteAsync());
|
||||
Assert.Equal(lamp, await fake.NextWriteAsync());
|
||||
await send.WaitAsync(Timeout); // completes (dropped), doesn't hang
|
||||
Assert.Equal(2, link.Retransmits);
|
||||
|
||||
Assert.Equal(lamp, await fake.NextWriteAsync()); // retry 1
|
||||
Assert.Equal(lamp, await fake.NextWriteAsync()); // retry 2
|
||||
|
||||
// Budget spent: NAKs 3 and 4 must produce no further writes.
|
||||
await Assert.ThrowsAnyAsync<OperationCanceledException>(
|
||||
() => fake.NextWriteAsync(TimeSpan.FromMilliseconds(200)));
|
||||
Assert.Equal(2, link.NakRetransmits);
|
||||
|
||||
cts.Cancel();
|
||||
await run;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Ack_DoesNotRetransmit()
|
||||
public async Task Ack_CompletesWithoutRetransmit()
|
||||
{
|
||||
var fake = new FakeTransport();
|
||||
var link = new RioSerialLink(fake, new RioSerialLinkOptions { AutoPollAnalog = false });
|
||||
var link = new RioSerialLink(fake, StopAndWait());
|
||||
|
||||
using var cts = new CancellationTokenSource();
|
||||
Task run = link.RunAsync(cts.Token);
|
||||
|
||||
await link.SendAsync(PacketBuilder.LampRequest(0x05, 0x02));
|
||||
Task send = link.SendAsync(PacketBuilder.LampRequest(0x05, 0x02));
|
||||
await fake.NextWriteAsync();
|
||||
|
||||
fake.Enqueue((byte)RioControl.Ack);
|
||||
|
||||
await send.WaitAsync(Timeout);
|
||||
Assert.Equal(0, link.Retransmits);
|
||||
|
||||
await Assert.ThrowsAnyAsync<OperationCanceledException>(
|
||||
() => fake.NextWriteAsync(TimeSpan.FromMilliseconds(200)));
|
||||
Assert.Equal(0, link.NakRetransmits);
|
||||
|
||||
cts.Cancel();
|
||||
await run;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task NewCommand_ResetsRetryBudget()
|
||||
public async Task Commands_AreSerialized_SecondWaitsForFirst()
|
||||
{
|
||||
var fake = new FakeTransport();
|
||||
var link = new RioSerialLink(fake, new RioSerialLinkOptions
|
||||
{
|
||||
AutoPollAnalog = false,
|
||||
NakRetransmitLimit = 1,
|
||||
});
|
||||
var link = new RioSerialLink(fake, StopAndWait());
|
||||
|
||||
using var cts = new CancellationTokenSource();
|
||||
Task run = link.RunAsync(cts.Token);
|
||||
|
||||
byte[] first = PacketBuilder.LampRequest(0x05, 0x02);
|
||||
await link.SendAsync(first);
|
||||
await fake.NextWriteAsync();
|
||||
fake.Enqueue((byte)RioControl.Nak);
|
||||
Assert.Equal(first, await fake.NextWriteAsync()); // budget of 'first' spent
|
||||
|
||||
byte[] second = PacketBuilder.LampRequest(0x06, 0x01);
|
||||
await link.SendAsync(second);
|
||||
Assert.Equal(second, await fake.NextWriteAsync());
|
||||
fake.Enqueue((byte)RioControl.Nak);
|
||||
Assert.Equal(second, await fake.NextWriteAsync()); // fresh budget applies
|
||||
Task sendA = link.SendAsync(first);
|
||||
Task sendB = link.SendAsync(second);
|
||||
|
||||
// Only the first is on the wire until it resolves.
|
||||
Assert.Equal(first, await fake.NextWriteAsync());
|
||||
await Assert.ThrowsAnyAsync<OperationCanceledException>(
|
||||
() => fake.NextWriteAsync(TimeSpan.FromMilliseconds(100)));
|
||||
|
||||
fake.Enqueue((byte)RioControl.Ack);
|
||||
Assert.Equal(second, await fake.NextWriteAsync()); // now B goes out
|
||||
fake.Enqueue((byte)RioControl.Ack);
|
||||
|
||||
await sendA.WaitAsync(Timeout);
|
||||
await sendB.WaitAsync(Timeout);
|
||||
|
||||
cts.Cancel();
|
||||
await run;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task OwnControlByteReplies_AreNeverRetransmitted()
|
||||
public async Task RequestReply_ResolvesTheInFlightCommand_WithoutExplicitAck()
|
||||
{
|
||||
var fake = new FakeTransport();
|
||||
var link = new RioSerialLink(fake, new RioSerialLinkOptions { AutoPollAnalog = false });
|
||||
var link = new RioSerialLink(fake, StopAndWait());
|
||||
|
||||
using var cts = new CancellationTokenSource();
|
||||
Task run = link.RunAsync(cts.Token);
|
||||
|
||||
// Inbound button packet -> the link replies with a 1-byte ACK. That ACK
|
||||
// must not become "the last command" for retransmit purposes.
|
||||
// An analog request answered by the reply alone (no ACK byte) must not
|
||||
// burn the ACK timeout — the reply proves the request landed.
|
||||
Task send = link.RequestAnalogAsync();
|
||||
await fake.NextWriteAsync();
|
||||
fake.Enqueue(PacketBuilder.Build(RioCommand.AnalogReply, new byte[10]));
|
||||
|
||||
await send.WaitAsync(TimeSpan.FromMilliseconds(120)); // well under AckTimeout
|
||||
Assert.Equal(0, link.Retransmits);
|
||||
|
||||
cts.Cancel();
|
||||
await run;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task UnsolicitedNak_WithNothingPending_IsIgnored()
|
||||
{
|
||||
var fake = new FakeTransport();
|
||||
var link = new RioSerialLink(fake, StopAndWait());
|
||||
|
||||
using var cts = new CancellationTokenSource();
|
||||
Task run = link.RunAsync(cts.Token);
|
||||
|
||||
// Inbound button packet -> our 1-byte ACK reply (bypasses the command
|
||||
// gate). A stray NAK afterwards must not resend anything.
|
||||
fake.Enqueue(PacketBuilder.Build(RioCommand.ButtonPressed, new byte[] { 0x05 }));
|
||||
Assert.Equal(new byte[] { (byte)RioControl.Ack }, await fake.NextWriteAsync());
|
||||
|
||||
@@ -267,7 +294,7 @@ public class RioSerialLinkTests
|
||||
|
||||
await Assert.ThrowsAnyAsync<OperationCanceledException>(
|
||||
() => fake.NextWriteAsync(TimeSpan.FromMilliseconds(200)));
|
||||
Assert.Equal(0, link.NakRetransmits);
|
||||
Assert.Equal(0, link.Retransmits);
|
||||
|
||||
cts.Cancel();
|
||||
await run;
|
||||
@@ -277,22 +304,19 @@ public class RioSerialLinkTests
|
||||
public async Task Retransmit_Disabled_IsFireAndForget()
|
||||
{
|
||||
var fake = new FakeTransport();
|
||||
var link = new RioSerialLink(fake, new RioSerialLinkOptions
|
||||
{
|
||||
AutoPollAnalog = false,
|
||||
NakRetransmitLimit = 0,
|
||||
});
|
||||
var link = new RioSerialLink(fake, StopAndWait(limit: 0));
|
||||
|
||||
using var cts = new CancellationTokenSource();
|
||||
Task run = link.RunAsync(cts.Token);
|
||||
|
||||
await link.SendAsync(PacketBuilder.LampRequest(0x05, 0x02));
|
||||
// Completes immediately (no ACK wait), and a NAK triggers nothing.
|
||||
await link.SendAsync(PacketBuilder.LampRequest(0x05, 0x02)).WaitAsync(Timeout);
|
||||
await fake.NextWriteAsync();
|
||||
fake.Enqueue((byte)RioControl.Nak);
|
||||
|
||||
await Assert.ThrowsAnyAsync<OperationCanceledException>(
|
||||
() => fake.NextWriteAsync(TimeSpan.FromMilliseconds(200)));
|
||||
Assert.Equal(0, link.NakRetransmits);
|
||||
Assert.Equal(0, link.Retransmits);
|
||||
|
||||
cts.Cancel();
|
||||
await run;
|
||||
|
||||
@@ -18,4 +18,15 @@ internal static class TaskTestExtensions
|
||||
cts.Cancel(); // stop the delay timer
|
||||
return await task.ConfigureAwait(false);
|
||||
}
|
||||
|
||||
/// <summary>Non-generic counterpart of <see cref="WaitAsync{T}"/>.</summary>
|
||||
public static async Task WaitAsync(this Task task, TimeSpan timeout)
|
||||
{
|
||||
using var cts = new CancellationTokenSource();
|
||||
Task completed = await Task.WhenAny(task, Task.Delay(timeout, cts.Token)).ConfigureAwait(false);
|
||||
if (completed != task)
|
||||
throw new TimeoutException($"Task did not complete within {timeout}.");
|
||||
cts.Cancel();
|
||||
await task.ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user