feat(modbus-rtu): validate response unit + cover partial-read/truncation in ModbusRtuFraming

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTransportMode.cs

@@ -0,0 +1,17 @@
+namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus;
+
+/// <summary>
+///     Wire transport for a Modbus driver instance. <see cref="Tcp"/> = Modbus/TCP (MBAP + TxId);
+///     <see cref="RtuOverTcp"/> = RTU framing (address + CRC-16, no MBAP) tunnelled over a socket to
+///     a serial→Ethernet gateway. Default <see cref="Tcp"/> — existing configs omit the field.
+///     A direct-serial <c>Rtu</c> member is intentionally NOT reserved here (descoped 2026-07-15);
+///     do not number-squat it.
+/// </summary>
+public enum ModbusTransportMode
+{
+    /// <summary>Modbus/TCP — 7-byte MBAP header + transaction id (the historical default).</summary>
+    Tcp,
+
+    /// <summary>RTU framing (<c>[addr][PDU][CRC-16]</c>) tunnelled over a socket to a serial→Ethernet gateway.</summary>
+    RtuOverTcp,
+}

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusCrc.cs

@@ -0,0 +1,48 @@
+namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus;
+
+/// <summary>
+///     CRC-16/MODBUS helper: reflected polynomial <c>0xA001</c>, initial value <c>0xFFFF</c>.
+///     On the wire the CRC is appended low byte first, high byte second — <see cref="Append"/>
+///     does that; <see cref="Compute"/> just returns the 16-bit value. Byte-stream-agnostic:
+///     no socket or framing knowledge lives here.
+/// </summary>
+public static class ModbusCrc
+{
+    /// <summary>Computes the CRC-16/MODBUS checksum over <paramref name="data"/>.</summary>
+    /// <param name="data">The bytes to checksum (e.g. the RTU frame minus the trailing CRC).</param>
+    /// <returns>The 16-bit CRC value.</returns>
+    public static ushort Compute(ReadOnlySpan<byte> data)
+    {
+        ushort crc = 0xFFFF;
+        foreach (var b in data)
+        {
+            crc ^= b;
+            for (var i = 0; i < 8; i++)
+            {
+                var carry = (crc & 0x0001) != 0;
+                crc >>= 1;
+                if (carry)
+                {
+                    crc ^= 0xA001;
+                }
+            }
+        }
+
+        return crc;
+    }
+
+    /// <summary>
+    ///     Returns <paramref name="body"/> with its CRC-16/MODBUS appended, low byte first.
+    /// </summary>
+    /// <param name="body">The frame body to checksum.</param>
+    /// <returns>A new array: <paramref name="body"/> followed by <c>[crc &amp; 0xFF, crc &gt;&gt; 8]</c>.</returns>
+    public static byte[] Append(ReadOnlySpan<byte> body)
+    {
+        var crc = Compute(body);
+        var result = new byte[body.Length + 2];
+        body.CopyTo(result);
+        result[^2] = (byte)(crc & 0xFF);
+        result[^1] = (byte)(crc >> 8);
+        return result;
+    }
+}

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusRtuFraming.cs

@@ -0,0 +1,186 @@
+namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus;
+
+/// <summary>
+///     Modbus RTU framing: builds a request ADU (<c>[unitId] + PDU + CRC</c>) and deframes a
+///     response back to its bare PDU. Byte-stream-agnostic — it operates on a <see cref="Stream"/>
+///     and knows nothing about sockets or serial ports, so a future serial transport can reuse it.
+/// </summary>
+/// <remarks>
+///     <para>
+///         The single genuinely-new correctness risk of the RTU path: an RTU frame carries <b>no
+///         length field</b> (unlike TCP's MBAP header), so the response size must be derived from
+///         the function code. After reading <c>addr(1)</c> + <c>fc(1)</c>, the remaining byte count
+///         is one of three shapes:
+///     </para>
+///     <list type="table">
+///         <listheader><term>Shape</term><description>Trailing bytes after <c>addr,fc</c></description></listheader>
+///         <item>
+///             <term>Exception (<c>fc &amp; 0x80</c>)</term>
+///             <description><c>excCode(1)</c> + <c>CRC(2)</c></description>
+///         </item>
+///         <item>
+///             <term>Read (FC 01/02/03/04)</term>
+///             <description><c>byteCount(1)</c> then <c>byteCount</c> data bytes + <c>CRC(2)</c></description>
+///         </item>
+///         <item>
+///             <term>Write echo (FC 05/06/15/16)</term>
+///             <description>fixed <c>4</c> bytes + <c>CRC(2)</c></description>
+///         </item>
+///     </list>
+///     <para>
+///         The trailing CRC is validated over <c>[addr, ...pdu]</c> (everything except the two CRC
+///         bytes) <b>before</b> the frame is interpreted — so a corrupt exception frame surfaces as a
+///         desync, never as a bogus <see cref="ModbusException"/>. A CRC mismatch or a short read
+///         (truncation / stream closed mid-frame) throws <see cref="ModbusTransportDesyncException"/>;
+///         a CRC-valid exception PDU throws <see cref="ModbusException"/> carrying the original
+///         function code (<c>fc &amp; 0x7F</c>) and exception code.
+///     </para>
+///     <para>
+///         Once the CRC passes, the response's address byte is validated against the addressed unit.
+///         On an RS-485 multi-drop bus a CRC-valid reply from the <b>wrong</b> slave would otherwise be
+///         silently accepted as the addressed unit's data; such a frame is a unit-mismatch
+///         <see cref="ModbusTransportDesyncException"/>. This ordering is deliberate — a corrupt frame
+///         stays a CRC/desync failure, and only a coherent-but-mis-addressed frame becomes a
+///         unit-mismatch desync.
+///     </para>
+/// </remarks>
+public static class ModbusRtuFraming
+{
+    /// <summary>
+    ///     Builds an RTU request ADU: the unit id, the PDU, and the trailing CRC-16/MODBUS
+    ///     (appended low byte first).
+    /// </summary>
+    /// <param name="unitId">The RTU slave/unit address.</param>
+    /// <param name="pdu">The bare PDU (function code + data).</param>
+    /// <returns>A new array: <c>[unitId, ...pdu, crcLo, crcHi]</c>.</returns>
+    public static byte[] BuildAdu(byte unitId, ReadOnlySpan<byte> pdu)
+    {
+        var frame = new byte[1 + pdu.Length];
+        frame[0] = unitId;
+        pdu.CopyTo(frame.AsSpan(1));
+        return ModbusCrc.Append(frame);
+    }
+
+    /// <summary>
+    ///     Reads a single RTU response frame from <paramref name="stream"/> and returns its bare PDU
+    ///     (<c>[fc, ...data]</c>), with the leading unit-id and trailing CRC stripped.
+    /// </summary>
+    /// <param name="stream">The byte stream to read the response from.</param>
+    /// <param name="expectedUnit">
+    ///     The unit id the request was addressed to. The response's address byte is validated against
+    ///     it after the CRC passes; a mismatch is a unit-mismatch desync.
+    /// </param>
+    /// <param name="ct">A token to cancel the read.</param>
+    /// <returns>The bare response PDU (function code followed by its data).</returns>
+    /// <exception cref="ModbusTransportDesyncException">
+    ///     The frame was truncated (stream closed mid-frame), its trailing CRC did not validate, or its
+    ///     address byte did not match <paramref name="expectedUnit"/>.
+    /// </exception>
+    /// <exception cref="ModbusException">
+    ///     The frame was a CRC-valid Modbus exception PDU (high bit set on the function code).
+    /// </exception>
+    public static async Task<byte[]> ReadResponsePduAsync(
+        Stream stream, byte expectedUnit, CancellationToken ct)
+    {
+        // Header: addr(1) + fc(1). The function code selects how many more bytes to read.
+        var header = new byte[2];
+        await ReadExactlyAsync(stream, header, ct).ConfigureAwait(false);
+        var fc = header[1];
+
+        int trailing; // bytes after addr+fc, up to and including the 2 CRC bytes.
+        if ((fc & 0x80) != 0)
+        {
+            // Exception frame: excCode(1) + CRC(2).
+            trailing = 1 + 2;
+        }
+        else if (fc is 0x01 or 0x02 or 0x03 or 0x04)
+        {
+            // Read response: byteCount(1) then byteCount data bytes + CRC(2).
+            var bc = new byte[1];
+            await ReadExactlyAsync(stream, bc, ct).ConfigureAwait(false);
+            var byteCount = bc[0];
+            var rest = new byte[byteCount + 2];
+            await ReadExactlyAsync(stream, rest, ct).ConfigureAwait(false);
+            return ValidateAndStrip(expectedUnit, header, bc, rest);
+        }
+        else
+        {
+            // Write-echo response (FC 05/06/15/16) or any other length-less fixed shape:
+            // fixed 4 payload bytes + CRC(2).
+            trailing = 4 + 2;
+        }
+
+        var tail = new byte[trailing];
+        await ReadExactlyAsync(stream, tail, ct).ConfigureAwait(false);
+        return ValidateAndStrip(expectedUnit, header, ReadOnlySpan<byte>.Empty, tail);
+    }
+
+    /// <summary>
+    ///     Assembles the full frame from its pieces, validates the trailing CRC over everything
+    ///     except the CRC bytes, then validates the response address against
+    ///     <paramref name="expectedUnit"/> and strips <c>addr</c> + <c>CRC</c> to return the bare PDU.
+    ///     A CRC-valid exception PDU throws <see cref="ModbusException"/>.
+    /// </summary>
+    private static byte[] ValidateAndStrip(
+        byte expectedUnit, ReadOnlySpan<byte> header, ReadOnlySpan<byte> middle, ReadOnlySpan<byte> tail)
+    {
+        // Reassemble the whole frame: header(addr,fc) + middle(optional byteCount) + tail.
+        var frame = new byte[header.Length + middle.Length + tail.Length];
+        header.CopyTo(frame);
+        middle.CopyTo(frame.AsSpan(header.Length));
+        tail.CopyTo(frame.AsSpan(header.Length + middle.Length));
+
+        // CRC covers everything except the trailing 2 CRC bytes.
+        var body = frame.AsSpan(0, frame.Length - 2);
+        var expected = ModbusCrc.Compute(body);
+        var actual = (ushort)(frame[^2] | (frame[^1] << 8));
+        if (actual != expected)
+            throw new ModbusTransportDesyncException(
+                ModbusTransportDesyncException.DesyncReason.TruncatedFrame,
+                $"Modbus RTU CRC mismatch: computed {expected:X4} got {actual:X4}");
+
+        // Unit-address validation runs only AFTER the CRC passes: a corrupt frame is a CRC/desync
+        // failure, and only a coherent-but-mis-addressed frame (a CRC-valid reply from the wrong
+        // RS-485 slave) is a unit-mismatch desync. Never silently accept another unit's data.
+        var addr = frame[0];
+        if (addr != expectedUnit)
+            throw new ModbusTransportDesyncException(
+                ModbusTransportDesyncException.DesyncReason.TruncatedFrame,
+                $"Modbus RTU unit mismatch: expected {expectedUnit} got {addr}");
+
+        // Bare PDU = frame minus leading addr and trailing CRC.
+        var pdu = body[1..].ToArray();
+
+        // Exception PDU: high bit set on the function code. The CRC already validated, so this is a
+        // coherent protocol-level error — surface the ORIGINAL function code (fc & 0x7F).
+        if ((pdu[0] & 0x80) != 0)
+        {
+            var fc = (byte)(pdu[0] & 0x7F);
+            var exCode = pdu[1];
+            throw new ModbusException(fc, exCode, $"Modbus exception fc={fc} code={exCode}");
+        }
+
+        return pdu;
+    }
+
+    /// <summary>
+    ///     Fills <paramref name="buf"/> completely from <paramref name="stream"/>, throwing a
+    ///     truncation desync if the stream ends first. Mirrors
+    ///     <c>ModbusTcpTransport.ReadExactlyAsync</c>, but normalises the end-of-stream to a
+    ///     <see cref="ModbusTransportDesyncException"/> so a length-less RTU frame that arrives
+    ///     short is classified as a desync rather than a bare <see cref="EndOfStreamException"/>.
+    /// </summary>
+    private static async Task ReadExactlyAsync(Stream stream, byte[] buf, CancellationToken ct)
+    {
+        var read = 0;
+        while (read < buf.Length)
+        {
+            var n = await stream.ReadAsync(buf.AsMemory(read), ct).ConfigureAwait(false);
+            if (n == 0)
+                throw new ModbusTransportDesyncException(
+                    ModbusTransportDesyncException.DesyncReason.TruncatedFrame,
+                    $"Modbus RTU frame truncated: expected {buf.Length} bytes, got {read}");
+            read += n;
+        }
+    }
+}

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCrcTests.cs

@@ -0,0 +1,27 @@
+using Shouldly;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests;
+
+public sealed class ModbusCrcTests
+{
+    // Known CRC-16/MODBUS vectors (init 0xFFFF, poly 0xA001). CRC returned as ushort;
+    // on the wire it is appended low-byte-first.
+    [Theory]
+    // FC03 read HR: unit 1, addr 0, qty 1  -> CRC 0x0A84  (bytes 84 0A on the wire)
+    [InlineData(new byte[] { 0x01, 0x03, 0x00, 0x00, 0x00, 0x01 }, (ushort)0x0A84)]
+    // "123456789" canonical check value for CRC-16/MODBUS = 0x4B37
+    [InlineData(new byte[] { 0x31,0x32,0x33,0x34,0x35,0x36,0x37,0x38,0x39 }, (ushort)0x4B37)]
+    public void Compute_matches_known_vectors(byte[] frame, ushort expected)
+        => ModbusCrc.Compute(frame).ShouldBe(expected);
+
+    [Fact]
+    public void AppendLowByteFirst_writes_lo_then_hi()
+    {
+        var body = new byte[] { 0x01, 0x03, 0x00, 0x00, 0x00, 0x01 };
+        var withCrc = ModbusCrc.Append(body);
+        withCrc.Length.ShouldBe(body.Length + 2);
+        withCrc[^2].ShouldBe((byte)0x84); // CRC low byte
+        withCrc[^1].ShouldBe((byte)0x0A); // CRC high byte
+    }
+}

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRtuFramingTests.cs

@@ -0,0 +1,131 @@
+using Shouldly;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests;
+
+public sealed class ModbusRtuFramingTests
+{
+    private static MemoryStream Canned(params byte[] frame) => new(frame);
+
+    [Fact]
+    public void BuildAdu_prefixes_unit_and_appends_crc()
+    {
+        var adu = ModbusRtuFraming.BuildAdu(0x01, new byte[] { 0x03, 0x00, 0x00, 0x00, 0x01 });
+        adu.ShouldBe(new byte[] { 0x01, 0x03, 0x00, 0x00, 0x00, 0x01, 0x84, 0x0A });
+    }
+
+    [Fact]
+    public async Task ReadResponse_FC03_returns_bare_pdu()
+    {
+        // addr=01 fc=03 byteCount=02 data=00 0A  + CRC(lo,hi)
+        var body = new byte[] { 0x01, 0x03, 0x02, 0x00, 0x0A };
+        var frame = ModbusCrc.Append(body);
+        await using var s = Canned(frame);
+        var pdu = await ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken);
+        pdu.ShouldBe(new byte[] { 0x03, 0x02, 0x00, 0x0A }); // fc + byteCount + data, no addr/CRC
+    }
+
+    [Fact]
+    public async Task ReadResponse_exception_frame_throws_ModbusException()
+    {
+        // addr=01 fc=0x83 exc=0x02 + CRC
+        var frame = ModbusCrc.Append(new byte[] { 0x01, 0x83, 0x02 });
+        await using var s = Canned(frame);
+        var ex = await Should.ThrowAsync<ModbusException>(
+            ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken));
+        ex.FunctionCode.ShouldBe((byte)0x03);
+        ex.ExceptionCode.ShouldBe((byte)0x02);
+    }
+
+    [Fact]
+    public async Task ReadResponse_bad_crc_throws_desync()
+    {
+        var frame = ModbusCrc.Append(new byte[] { 0x01, 0x03, 0x02, 0x00, 0x0A });
+        frame[^1] ^= 0xFF; // corrupt CRC high byte
+        await using var s = Canned(frame);
+        await Should.ThrowAsync<ModbusTransportDesyncException>(
+            ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken));
+    }
+
+    [Fact]
+    public async Task ReadResponse_FC06_write_echo_returns_four_byte_pdu()
+    {
+        // addr=01 fc=06 addr=00 07 value=00 2A + CRC  -> PDU = 06 00 07 00 2A
+        var frame = ModbusCrc.Append(new byte[] { 0x01, 0x06, 0x00, 0x07, 0x00, 0x2A });
+        await using var s = Canned(frame);
+        var pdu = await ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken);
+        pdu.ShouldBe(new byte[] { 0x06, 0x00, 0x07, 0x00, 0x2A });
+    }
+
+    [Fact]
+    public async Task ReadResponse_wrong_unit_address_throws_desync()
+    {
+        // CRC-valid FC03 frame addressed to unit 0x02, but we asked for 0x01 -> unit-mismatch desync.
+        var frame = ModbusCrc.Append(new byte[] { 0x02, 0x03, 0x02, 0x00, 0x0A });
+        await using var s = Canned(frame);
+        await Should.ThrowAsync<ModbusTransportDesyncException>(
+            ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken));
+    }
+
+    [Fact]
+    public async Task ReadResponse_FC03_deframes_correctly_when_delivered_in_dribbles()
+    {
+        // Same valid FC03 frame, but the stream hands back only 1-2 bytes per ReadAsync — proves
+        // the ReadExactlyAsync accumulation loop reassembles a length-less frame across many reads.
+        var frame = ModbusCrc.Append(new byte[] { 0x01, 0x03, 0x02, 0x00, 0x0A });
+        await using var s = new DribbleStream(frame, chunk: 2);
+        var pdu = await ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken);
+        pdu.ShouldBe(new byte[] { 0x03, 0x02, 0x00, 0x0A });
+    }
+
+    [Fact]
+    public async Task ReadResponse_stream_ends_early_throws_desync()
+    {
+        // Only the first 4 bytes of a 7-byte FC03 frame are available, then EOF (ReadAsync returns 0)
+        // -> the truncation path in ReadExactlyAsync must surface a desync, not hang or mis-parse.
+        var frame = ModbusCrc.Append(new byte[] { 0x01, 0x03, 0x02, 0x00, 0x0A });
+        var truncated = frame[..4];
+        await using var s = new DribbleStream(truncated, chunk: 2);
+        await Should.ThrowAsync<ModbusTransportDesyncException>(
+            ModbusRtuFraming.ReadResponsePduAsync(s, 0x01, TestContext.Current.CancellationToken));
+    }
+
+    /// <summary>
+    ///     A read-only test double that dribbles its backing bytes out at most <c>chunk</c> at a time
+    ///     per <see cref="ReadAsync(Memory{byte}, CancellationToken)"/> call, then returns 0 (EOF)
+    ///     once exhausted. Exercises the partial-read accumulation loop and the truncation → desync
+    ///     path that a single-shot <see cref="MemoryStream"/> never reaches.
+    /// </summary>
+    private sealed class DribbleStream(byte[] data, int chunk) : Stream
+    {
+        private int _pos;
+
+        public override int Read(byte[] buffer, int offset, int count)
+        {
+            var n = Math.Min(Math.Min(chunk, count), data.Length - _pos);
+            if (n <= 0) return 0;
+            Array.Copy(data, _pos, buffer, offset, n);
+            _pos += n;
+            return n;
+        }
+
+        public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default)
+        {
+            var n = Math.Min(Math.Min(chunk, buffer.Length), data.Length - _pos);
+            if (n <= 0) return ValueTask.FromResult(0);
+            data.AsSpan(_pos, n).CopyTo(buffer.Span);
+            _pos += n;
+            return ValueTask.FromResult(n);
+        }
+
+        public override bool CanRead => true;
+        public override bool CanSeek => false;
+        public override bool CanWrite => false;
+        public override long Length => data.Length;
+        public override long Position { get => _pos; set => throw new NotSupportedException(); }
+        public override void Flush() { }
+        public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
+        public override void SetLength(long value) => throw new NotSupportedException();
+        public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException();
+    }
+}

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusTransportModeTests.cs

@@ -0,0 +1,15 @@
+using Shouldly;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests;
+
+public sealed class ModbusTransportModeTests
+{
+    [Fact]
+    public void Tcp_is_the_default_zero_member()
+        => ((ModbusTransportMode)0).ShouldBe(ModbusTransportMode.Tcp);
+
+    [Fact]
+    public void Exactly_two_members_ship_in_v1()
+        => Enum.GetNames<ModbusTransportMode>().ShouldBe(["Tcp", "RtuOverTcp"]);
+}