From a3f2f95344889a5116b0da5731ee896714bdd71d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 18 Apr 2026 21:58:49 -0400 Subject: [PATCH] Phase 3 PR 49 -- Per-device FC03/FC16 register caps with auto-chunking. Adds MaxRegistersPerRead (default 125, spec max) + MaxRegistersPerWrite (default 123, spec max) to ModbusDriverOptions. Reads that exceed the cap automatically split into consecutive FC03 requests: the driver dispatches chunks of [cap] regs at incrementing addresses, copies each response into an assembled byte[] buffer, and hands the full payload to DecodeRegister. From the caller's view a 240-char string read against a cap-100 device is still one Read() call returning one string -- the chunking is invisible, the wire shows N requests of cap-sized quantity plus one tail chunk. Writes are NOT auto-chunked. Splitting an FC16 across two transactions would lose atomicity -- mid-split crash leaves half the value written, which is strictly worse than rejecting upfront. Instead, writes exceeding MaxRegistersPerWrite throw InvalidOperationException with a message naming the tag + cap + the caller's escape hatch (shorten StringLength or split into multiple tags). The driver catches the exception internally and surfaces it to IWritable as BadInternalError so the caller pattern stays symmetric with other failure modes. Per-family cap cheat-sheet (documented in xml-doc on the option): Modbus-TCP spec = 125 read / 123 write, AutomationDirect DL205/DL260 = 128 read / 100 write (128 exceeds spec byte-count capacity so in practice 125 is the working ceiling), Mitsubishi Q/FX3U = 64 / 64, Omron CJ/CS = 125 / 123. Not all PLCs reject over-cap requests cleanly -- some drop the connection silently -- so having the cap enforced client-side prevents the hard-to-diagnose 'driver just stopped' failure mode. Unit tests: Read_within_cap_issues_single_FC03_request (control: no unnecessary chunking), Read_above_cap_splits_into_two_FC03_requests (120 regs / cap 100 -> 100+20, asserts exact per-chunk (Address,Quantity) and end-to-end payload continuity starting with register[100] high byte = 'A'), Read_cap_honors_Mitsubishi_lower_cap_of_64 (100 regs / cap 64 -> 64+36), Write_exceeding_cap_throws_instead_of_splitting (110 regs / cap 100 -> status != 0 AND Fc16Requests.Count == 0 to prove nothing was sent), Write_within_cap_proceeds_normally (control: cap honored on short writes too). Tests use a new RecordingTransport that captures the (Address, Quantity) tuple of every FC03/FC16 request so the chunk layout is directly assertable -- the existing FakeTransport does not expose request history. 103/103 Modbus.Tests pass; 6/6 DL205 integration tests still pass against the live pymodbus dl205 profile with MODBUS_SIM_PROFILE=dl205. --- .../ModbusDriver.cs | 47 ++++- .../ModbusDriverOptions.cs | 20 +++ .../ModbusCapTests.cs | 165 ++++++++++++++++++ 3 files changed, 226 insertions(+), 6 deletions(-) create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCapTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index db07130..a4a87c6 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -171,11 +171,14 @@ public sealed class ModbusDriver(ModbusDriverOptions options, string driverInsta { var quantity = RegisterCount(tag); var fc = tag.Region == ModbusRegion.HoldingRegisters ? (byte)0x03 : (byte)0x04; - var pdu = new byte[] { fc, (byte)(tag.Address >> 8), (byte)(tag.Address & 0xFF), - (byte)(quantity >> 8), (byte)(quantity & 0xFF) }; - var resp = await transport.SendAsync(_options.UnitId, pdu, ct).ConfigureAwait(false); - // resp = [fc][byte-count][data...] - var data = new ReadOnlySpan(resp, 2, resp[1]); + // Auto-chunk when the tag's register span exceeds the caller-configured cap. + // Affects long strings (FC03/04 > 125 regs is spec-forbidden; DL205 caps at 128, + // Mitsubishi Q caps at 64). Non-string tags max out at 4 regs so the cap never + // triggers for numerics. + var cap = _options.MaxRegistersPerRead == 0 ? (ushort)125 : _options.MaxRegistersPerRead; + var data = quantity <= cap + ? await ReadRegisterBlockAsync(transport, fc, tag.Address, quantity, ct).ConfigureAwait(false) + : await ReadRegisterBlockChunkedAsync(transport, fc, tag.Address, quantity, cap, ct).ConfigureAwait(false); return DecodeRegister(data, tag); } default: @@ -183,6 +186,33 @@ public sealed class ModbusDriver(ModbusDriverOptions options, string driverInsta } } + private async Task ReadRegisterBlockAsync( + IModbusTransport transport, byte fc, ushort address, ushort quantity, CancellationToken ct) + { + var pdu = new byte[] { fc, (byte)(address >> 8), (byte)(address & 0xFF), + (byte)(quantity >> 8), (byte)(quantity & 0xFF) }; + var resp = await transport.SendAsync(_options.UnitId, pdu, ct).ConfigureAwait(false); + // resp = [fc][byte-count][data...] + var data = new byte[resp[1]]; + Buffer.BlockCopy(resp, 2, data, 0, resp[1]); + return data; + } + + private async Task ReadRegisterBlockChunkedAsync( + IModbusTransport transport, byte fc, ushort address, ushort totalRegs, ushort cap, CancellationToken ct) + { + var assembled = new byte[totalRegs * 2]; + ushort done = 0; + while (done < totalRegs) + { + var chunk = (ushort)Math.Min(cap, totalRegs - done); + var chunkBytes = await ReadRegisterBlockAsync(transport, fc, (ushort)(address + done), chunk, ct).ConfigureAwait(false); + Buffer.BlockCopy(chunkBytes, 0, assembled, done * 2, chunkBytes.Length); + done += chunk; + } + return assembled; + } + // ---- IWritable ---- public async Task> WriteAsync( @@ -239,8 +269,13 @@ public sealed class ModbusDriver(ModbusDriverOptions options, string driverInsta } else { - // FC 16 (Write Multiple Registers) for 32-bit types + // FC 16 (Write Multiple Registers) for 32-bit types. var qty = (ushort)(bytes.Length / 2); + var writeCap = _options.MaxRegistersPerWrite == 0 ? (ushort)123 : _options.MaxRegistersPerWrite; + if (qty > writeCap) + throw new InvalidOperationException( + $"Write of {qty} registers to {tag.Name} exceeds MaxRegistersPerWrite={writeCap}. " + + $"Split the tag (e.g. shorter StringLength) — partial FC16 chunks would lose atomicity."); var pdu = new byte[6 + 1 + bytes.Length]; pdu[0] = 0x10; pdu[1] = (byte)(tag.Address >> 8); pdu[2] = (byte)(tag.Address & 0xFF); diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs index ee4e91e..4bc1db4 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs @@ -25,6 +25,26 @@ public sealed class ModbusDriverOptions /// . /// public ModbusProbeOptions Probe { get; init; } = new(); + + /// + /// Maximum registers per FC03 (Read Holding Registers) / FC04 (Read Input Registers) + /// transaction. Modbus-TCP spec allows 125; many device families impose lower caps: + /// AutomationDirect DL205/DL260 cap at 128, Mitsubishi Q/FX3U cap at 64, + /// Omron CJ/CS cap at 125. Set to the lowest cap across the devices this driver + /// instance talks to; the driver auto-chunks larger reads into consecutive requests. + /// Default 125 — the spec maximum, safe against any conforming server. Setting + /// to 0 disables the cap (discouraged — the spec upper bound still applies). + /// + public ushort MaxRegistersPerRead { get; init; } = 125; + + /// + /// Maximum registers per FC16 (Write Multiple Registers) transaction. Spec maximum is + /// 123; DL205/DL260 cap at 100. Matching caller-vs-device semantics: + /// exceeding the cap currently throws (writes aren't auto-chunked because a partial + /// write across two FC16 calls is no longer atomic — caller must explicitly opt in + /// by shortening the tag's StringLength or splitting it into multiple tags). + /// + public ushort MaxRegistersPerWrite { get; init; } = 123; } public sealed class ModbusProbeOptions diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCapTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCapTests.cs new file mode 100644 index 0000000..0c802f3 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCapTests.cs @@ -0,0 +1,165 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +[Trait("Category", "Unit")] +public sealed class ModbusCapTests +{ + /// + /// Records every PDU sent so tests can assert request-count and per-request quantity — + /// the only observable behaviour of the auto-chunking path. + /// + private sealed class RecordingTransport : IModbusTransport + { + public readonly ushort[] HoldingRegisters = new ushort[1024]; + public readonly List<(ushort Address, ushort Quantity)> Fc03Requests = new(); + public readonly List<(ushort Address, ushort Quantity)> Fc16Requests = new(); + + public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask; + + public Task SendAsync(byte unitId, byte[] pdu, CancellationToken ct) + { + var fc = pdu[0]; + if (fc == 0x03) + { + var addr = (ushort)((pdu[1] << 8) | pdu[2]); + var qty = (ushort)((pdu[3] << 8) | pdu[4]); + Fc03Requests.Add((addr, qty)); + var byteCount = (byte)(qty * 2); + var resp = new byte[2 + byteCount]; + resp[0] = 0x03; + resp[1] = byteCount; + for (var i = 0; i < qty; i++) + { + resp[2 + i * 2] = (byte)(HoldingRegisters[addr + i] >> 8); + resp[3 + i * 2] = (byte)(HoldingRegisters[addr + i] & 0xFF); + } + return Task.FromResult(resp); + } + if (fc == 0x10) + { + var addr = (ushort)((pdu[1] << 8) | pdu[2]); + var qty = (ushort)((pdu[3] << 8) | pdu[4]); + Fc16Requests.Add((addr, qty)); + for (var i = 0; i < qty; i++) + HoldingRegisters[addr + i] = (ushort)((pdu[6 + i * 2] << 8) | pdu[7 + i * 2]); + return Task.FromResult(new byte[] { 0x10, pdu[1], pdu[2], pdu[3], pdu[4] }); + } + return Task.FromException(new ModbusException(fc, 0x01, $"fc={fc} unsupported")); + } + + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + + [Fact] + public async Task Read_within_cap_issues_single_FC03_request() + { + var tag = new ModbusTagDefinition("S", ModbusRegion.HoldingRegisters, 0, ModbusDataType.String, + StringLength: 40); // 20 regs — fits in default cap (125). + var transport = new RecordingTransport(); + var opts = new ModbusDriverOptions { Host = "fake", Tags = [tag], Probe = new ModbusProbeOptions { Enabled = false } }; + await using var drv = new ModbusDriver(opts, "modbus-1", _ => transport); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + _ = await drv.ReadAsync(["S"], TestContext.Current.CancellationToken); + + transport.Fc03Requests.Count.ShouldBe(1); + transport.Fc03Requests[0].Quantity.ShouldBe((ushort)20); + } + + [Fact] + public async Task Read_above_cap_splits_into_two_FC03_requests() + { + // 240-char string = 120 regs. Cap = 100 (a typical sub-spec device cap). Expect 100 + 20. + var tag = new ModbusTagDefinition("LongString", ModbusRegion.HoldingRegisters, 100, ModbusDataType.String, + StringLength: 240); + var transport = new RecordingTransport(); + // Seed cells so the re-assembled payload is stable — confirms chunks are stitched in order. + for (ushort i = 100; i < 100 + 120; i++) + transport.HoldingRegisters[i] = (ushort)((('A' + (i - 100) % 26) << 8) | ('A' + (i - 100) % 26)); + + var opts = new ModbusDriverOptions + { + Host = "fake", + Tags = [tag], + MaxRegistersPerRead = 100, + Probe = new ModbusProbeOptions { Enabled = false }, + }; + await using var drv = new ModbusDriver(opts, "modbus-1", _ => transport); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + var results = await drv.ReadAsync(["LongString"], TestContext.Current.CancellationToken); + results[0].StatusCode.ShouldBe(0u); + + transport.Fc03Requests.Count.ShouldBe(2, "120 regs / cap 100 → 2 requests"); + transport.Fc03Requests[0].ShouldBe(((ushort)100, (ushort)100)); + transport.Fc03Requests[1].ShouldBe(((ushort)200, (ushort)20)); + + // Payload continuity: re-assembled string starts where register 100 does and keeps going. + var s = (string)results[0].Value!; + s.Length.ShouldBeGreaterThan(0); + s[0].ShouldBe('A'); // register[100] high byte + } + + [Fact] + public async Task Read_cap_honors_Mitsubishi_lower_cap_of_64() + { + // 200-char string = 100 regs. Mitsubishi Q cap = 64. Expect: 64, 36. + var tag = new ModbusTagDefinition("MitString", ModbusRegion.HoldingRegisters, 0, ModbusDataType.String, + StringLength: 200); + var transport = new RecordingTransport(); + var opts = new ModbusDriverOptions { Host = "fake", Tags = [tag], MaxRegistersPerRead = 64, Probe = new ModbusProbeOptions { Enabled = false } }; + await using var drv = new ModbusDriver(opts, "modbus-1", _ => transport); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + _ = await drv.ReadAsync(["MitString"], TestContext.Current.CancellationToken); + + transport.Fc03Requests.Count.ShouldBe(2); + transport.Fc03Requests[0].Quantity.ShouldBe((ushort)64); + transport.Fc03Requests[1].Quantity.ShouldBe((ushort)36); + } + + [Fact] + public async Task Write_exceeding_cap_throws_instead_of_splitting() + { + // Partial FC16 across two transactions is not atomic. Forcing an explicit exception so the + // caller knows their tag definition is incompatible with the device cap rather than silently + // writing half a string and crashing between chunks. + var tag = new ModbusTagDefinition("LongStringWrite", ModbusRegion.HoldingRegisters, 0, ModbusDataType.String, + StringLength: 220); // 110 regs. + var transport = new RecordingTransport(); + var opts = new ModbusDriverOptions { Host = "fake", Tags = [tag], MaxRegistersPerWrite = 100, Probe = new ModbusProbeOptions { Enabled = false } }; + await using var drv = new ModbusDriver(opts, "modbus-1", _ => transport); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + var results = await drv.WriteAsync( + [new WriteRequest("LongStringWrite", new string('A', 220))], + TestContext.Current.CancellationToken); + + // Driver catches the internal exception and surfaces BadInternalError — the Fc16Requests + // list must still be empty because nothing was sent. + results[0].StatusCode.ShouldNotBe(0u); + transport.Fc16Requests.Count.ShouldBe(0); + } + + [Fact] + public async Task Write_within_cap_proceeds_normally() + { + var tag = new ModbusTagDefinition("ShortStringWrite", ModbusRegion.HoldingRegisters, 0, ModbusDataType.String, + StringLength: 40); // 20 regs. + var transport = new RecordingTransport(); + var opts = new ModbusDriverOptions { Host = "fake", Tags = [tag], MaxRegistersPerWrite = 100, Probe = new ModbusProbeOptions { Enabled = false } }; + await using var drv = new ModbusDriver(opts, "modbus-1", _ => transport); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + var results = await drv.WriteAsync( + [new WriteRequest("ShortStringWrite", "HELLO")], + TestContext.Current.CancellationToken); + + results[0].StatusCode.ShouldBe(0u); + transport.Fc16Requests.Count.ShouldBe(1); + transport.Fc16Requests[0].Quantity.ShouldBe((ushort)20); + } +}