RioSerialLink: NAK-driven retransmit of the last command packet

The board ACK/NAKs every inbound packet; we were fire-and-forget, so
any corrupt arrival became a permanent state error (stuck-bright /
missed lamps at 62500, where ~6.5% of 4-byte lamp commands lose bytes
to RX-ISR overrun). Now a NAK control byte triggers a resend of the
most recent command packet, bounded by NakRetransmitLimit (default 2;
0 restores fire-and-forget).

Design notes: the wire has no sequence numbers, but every PC->RIO
command is idempotent (lamp state, analog request, reset), so resending
the latest command is safe even in the rare race where the NAK belonged
to an earlier packet. Single control-byte replies (our ACKs) never
participate. NakRetransmits counter surfaced in the mash summary.

6 new tests (retransmit, limit, budget reset, ACK no-op, control-byte
exclusion, disable switch); 281 green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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
     /// <summary>The transport's description, surfaced for status/logging.</summary>
     public string Description => _transport.Description;
 
+    /// <summary>Total NAK-triggered retransmits since the link was created.</summary>
+    public long NakRetransmits => Interlocked.Read(ref _nakRetransmits);
+
     /// <summary>
     /// Run the receive loop and (if enabled) the analog poll loop until
     /// <paramref name="cancellationToken"/> fires or the transport closes.
@@ -80,6 +91,24 @@ public sealed class RioSerialLink
 
     /// <summary>Send a pre-built packet (see <see cref="PacketBuilder"/>) to the RIO.</summary>
     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);
+    }
+
     /// <summary>Request an analog update (<see cref="RioCommand.AnalogRequest"/>).</summary>
     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;
 

src/RioJoy.Core/Serial/RioSerialLinkOptions.cs

@@ -29,4 +29,17 @@ public sealed record RioSerialLinkOptions
 
     /// <summary>Read buffer size for the receive loop.</summary>
     public int ReadBufferSize { get; init; } = 256;
+
+    /// <summary>
+    /// 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%.
+    /// </summary>
+    public int NakRetransmitLimit { get; init; } = 2;
 }

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<OperationCanceledException>(
+            () => 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<OperationCanceledException>(
+            () => 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<OperationCanceledException>(
+            () => 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<OperationCanceledException>(
+            () => fake.NextWriteAsync(TimeSpan.FromMilliseconds(200)));
+        Assert.Equal(0, link.NakRetransmits);
+
+        cts.Cancel();
+        await run;
+    }
 }

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)"