diff --git a/src/RioJoy.Core/Serial/RioSerialLink.cs b/src/RioJoy.Core/Serial/RioSerialLink.cs index 2d283eb..7e2f5ee 100644 --- a/src/RioJoy.Core/Serial/RioSerialLink.cs +++ b/src/RioJoy.Core/Serial/RioSerialLink.cs @@ -23,6 +23,14 @@ public sealed class RioSerialLink // Time since the last accepted AnalogReply, for the recovery watchdog. private readonly Stopwatch _sinceAnalog = new(); + // NAK retransmit state: the last COMMAND packet sent (control-byte replies + // are excluded — the board never NAKs those) and how many resends it has + // consumed. Guarded by _retransmitGate; see RioSerialLinkOptions.NakRetransmitLimit. + private readonly object _retransmitGate = new(); + private byte[]? _lastCommand; + private int _lastCommandResends; + private long _nakRetransmits; + public RioSerialLink(IRioTransport transport, RioSerialLinkOptions? options = null) { _transport = transport ?? throw new ArgumentNullException(nameof(transport)); @@ -50,6 +58,9 @@ public sealed class RioSerialLink /// The transport's description, surfaced for status/logging. public string Description => _transport.Description; + /// Total NAK-triggered retransmits since the link was created. + public long NakRetransmits => Interlocked.Read(ref _nakRetransmits); + /// /// Run the receive loop and (if enabled) the analog poll loop until /// fires or the transport closes. @@ -80,6 +91,24 @@ public sealed class RioSerialLink /// Send a pre-built packet (see ) to the RIO. public async Task SendAsync(byte[] packet, CancellationToken cancellationToken = default) + { + if (packet is null) throw new ArgumentNullException(nameof(packet)); + + // Only real command packets participate in NAK retransmit; single + // control bytes (our ACK/NAK replies) are never NAK'd by the board. + if (packet.Length > 1 && _options.NakRetransmitLimit > 0) + { + lock (_retransmitGate) + { + _lastCommand = packet; + _lastCommandResends = 0; + } + } + + await WriteAsync(packet, cancellationToken).ConfigureAwait(false); + } + + private async Task WriteAsync(byte[] packet, CancellationToken cancellationToken) { await Compat.TaskCompat.WaitAsync(_writeLock, cancellationToken).ConfigureAwait(false); try @@ -92,6 +121,24 @@ public sealed class RioSerialLink } } + // The board NAK'd its most recent inbound packet: resend the last command + // (idempotent, see RioSerialLinkOptions.NakRetransmitLimit) unless its + // retry budget is spent. + private async Task RetransmitOnNakAsync(CancellationToken ct) + { + byte[]? packet; + lock (_retransmitGate) + { + if (_lastCommand is null || _lastCommandResends >= _options.NakRetransmitLimit) + return; + _lastCommandResends++; + packet = _lastCommand; + } + + Interlocked.Increment(ref _nakRetransmits); + await WriteAsync(packet!, ct).ConfigureAwait(false); + } + /// Request an analog update (). public Task RequestAnalogAsync(CancellationToken cancellationToken = default) => SendAsync(PacketBuilder.AnalogRequest(), cancellationToken); @@ -141,6 +188,8 @@ public sealed class RioSerialLink break; case RioRxEventKind.ControlByte: + if (ev.Byte == (byte)RioControl.Nak) + await RetransmitOnNakAsync(ct).ConfigureAwait(false); ControlReceived?.Invoke(ev.Byte); break; diff --git a/src/RioJoy.Core/Serial/RioSerialLinkOptions.cs b/src/RioJoy.Core/Serial/RioSerialLinkOptions.cs index cd0e8e1..994d733 100644 --- a/src/RioJoy.Core/Serial/RioSerialLinkOptions.cs +++ b/src/RioJoy.Core/Serial/RioSerialLinkOptions.cs @@ -29,4 +29,17 @@ public sealed record RioSerialLinkOptions /// Read buffer size for the receive loop. public int ReadBufferSize { get; init; } = 256; + + /// + /// How many times a command packet is retransmitted when the board NAKs it + /// (0 = fire-and-forget, the pre-2026-07 behavior). The wire has no sequence + /// numbers, but every PC→RIO command is idempotent (lamp state, analog + /// request, reset), so resending the most recent packet on NAK is safe even + /// in the rare race where the NAK belonged to an earlier one. Bounded so a + /// NAK storm (e.g. a rate the board's RX can't sustain) can't multiply + /// traffic without limit. Bench origin: at 62500 baud ~6.5% of 4-byte lamp + /// commands arrive corrupt (RX-ISR overrun) — one retry cuts the visible + /// lamp-state error rate to ~0.4%, two to ~0.03%. + /// + public int NakRetransmitLimit { get; init; } = 2; } diff --git a/tests/RioJoy.Core.Tests/Serial/RioSerialLinkTests.cs b/tests/RioJoy.Core.Tests/Serial/RioSerialLinkTests.cs index 50031f6..1eb33c0 100644 --- a/tests/RioJoy.Core.Tests/Serial/RioSerialLinkTests.cs +++ b/tests/RioJoy.Core.Tests/Serial/RioSerialLinkTests.cs @@ -141,4 +141,160 @@ public class RioSerialLinkTests cts.Cancel(); await run; } + + // --- NAK retransmit (RioSerialLinkOptions.NakRetransmitLimit) ------------ + + [Fact] + public async Task Nak_RetransmitsLastCommand() + { + var fake = new FakeTransport(); + var link = new RioSerialLink(fake, new RioSerialLinkOptions { AutoPollAnalog = false }); + + using var cts = new CancellationTokenSource(); + Task run = link.RunAsync(cts.Token); + + byte[] lamp = PacketBuilder.LampRequest(0x05, 0x02); + await link.SendAsync(lamp); + Assert.Equal(lamp, await fake.NextWriteAsync()); + + fake.Enqueue((byte)RioControl.Nak); // board: "that arrived corrupt" + + Assert.Equal(lamp, await fake.NextWriteAsync()); // resent + Assert.Equal(1, link.NakRetransmits); + + cts.Cancel(); + await run; + } + + [Fact] + public async Task Nak_RetransmitStopsAtLimit() + { + var fake = new FakeTransport(); + var link = new RioSerialLink(fake, new RioSerialLinkOptions + { + AutoPollAnalog = false, + NakRetransmitLimit = 2, + }); + + 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 + + for (int i = 0; i < 4; i++) + fake.Enqueue((byte)RioControl.Nak); + + 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( + () => fake.NextWriteAsync(TimeSpan.FromMilliseconds(200))); + Assert.Equal(2, link.NakRetransmits); + + cts.Cancel(); + await run; + } + + [Fact] + public async Task Ack_DoesNotRetransmit() + { + var fake = new FakeTransport(); + var link = new RioSerialLink(fake, new RioSerialLinkOptions { AutoPollAnalog = false }); + + using var cts = new CancellationTokenSource(); + Task run = link.RunAsync(cts.Token); + + await link.SendAsync(PacketBuilder.LampRequest(0x05, 0x02)); + await fake.NextWriteAsync(); + + fake.Enqueue((byte)RioControl.Ack); + + await Assert.ThrowsAnyAsync( + () => fake.NextWriteAsync(TimeSpan.FromMilliseconds(200))); + Assert.Equal(0, link.NakRetransmits); + + cts.Cancel(); + await run; + } + + [Fact] + public async Task NewCommand_ResetsRetryBudget() + { + var fake = new FakeTransport(); + var link = new RioSerialLink(fake, new RioSerialLinkOptions + { + AutoPollAnalog = false, + NakRetransmitLimit = 1, + }); + + 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 + + cts.Cancel(); + await run; + } + + [Fact] + public async Task OwnControlByteReplies_AreNeverRetransmitted() + { + var fake = new FakeTransport(); + var link = new RioSerialLink(fake, new RioSerialLinkOptions { AutoPollAnalog = false }); + + 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. + fake.Enqueue(PacketBuilder.Build(RioCommand.ButtonPressed, new byte[] { 0x05 })); + Assert.Equal(new byte[] { (byte)RioControl.Ack }, await fake.NextWriteAsync()); + + fake.Enqueue((byte)RioControl.Nak); + + await Assert.ThrowsAnyAsync( + () => fake.NextWriteAsync(TimeSpan.FromMilliseconds(200))); + Assert.Equal(0, link.NakRetransmits); + + cts.Cancel(); + await run; + } + + [Fact] + public async Task Retransmit_Disabled_IsFireAndForget() + { + var fake = new FakeTransport(); + var link = new RioSerialLink(fake, new RioSerialLinkOptions + { + AutoPollAnalog = false, + NakRetransmitLimit = 0, + }); + + using var cts = new CancellationTokenSource(); + Task run = link.RunAsync(cts.Token); + + await link.SendAsync(PacketBuilder.LampRequest(0x05, 0x02)); + await fake.NextWriteAsync(); + fake.Enqueue((byte)RioControl.Nak); + + await Assert.ThrowsAnyAsync( + () => fake.NextWriteAsync(TimeSpan.FromMilliseconds(200))); + Assert.Equal(0, link.NakRetransmits); + + cts.Cancel(); + await run; + } } diff --git a/tools/RioSerialMonitor/MashTest.cs b/tools/RioSerialMonitor/MashTest.cs index 83b1057..2792e79 100644 --- a/tools/RioSerialMonitor/MashTest.cs +++ b/tools/RioSerialMonitor/MashTest.cs @@ -319,7 +319,7 @@ internal static class MashTest Raw($"firmware : {(version is null ? "(no reply)" : version.ToString())}"); Raw($"presses/releases : {presses} / {releases} ({presses / mins:F0} presses/min)"); Raw($"analog replies : {analogReplies} (~{expectedPolls} poll slots; {100.0 * analogReplies / Math.Max(1, expectedPolls):F1}%), sentinels {sentinels}"); - Raw($"framing / NAK : {framing} / {naks}"); + Raw($"framing / NAK : {framing} / {naks} (NAK-triggered resends: {link.NakRetransmits})"); Raw("gap histogram : " + string.Join(" ", bucketNames.Select((n, i) => $"{n}:{buckets[i]}"))); Raw("longest gaps : " + (longestGaps.Count == 0 ? "(none)"