dohertj2/lmxopcua
1
0
Fork 0
Code Issues 43 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(modbus): multi-write PDUs carry no trailing byte past their byte count

Browse Source 
The FC15, FC16, BitInRegister[] RMW FC16 and FC23 builders allocated one byte
more than they filled (6 + 1 + n / 10 + 1 + n, data copied at offset 6 / 10), so
every multi-write request ended in a stray zero byte beyond the length its
byte-count field declares. A strict device or gateway that checks the frame
length against the byte count refuses it. Each PDU is now exactly 6 + n
(FC15/FC16) or 10 + n (FC23).

Tests: ModbusWritePduFramingTests asserts the exact length and byte-count field
for FC15 (1/8/9 coils), FC16 (array, Float64 array, scalar multi-register), the
BitInRegister RMW FC16 and FC23 (all red before the fix); the parity suite's
conformant device now refuses a frame-length mismatch on FC15/FC16/FC23.
Live: Driver.Modbus.IntegrationTests against 10.100.0.35:5020 (standard
profile) passes, including the FC16 and FC23 multi-register round trips.

Refs #741

Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 17:35:40 -04:00
parent cf62007678
commit 0f305f1827
3 changed files with 120 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs
+5 -3
View File
@@ -341,7 +341,8 @@ public sealed class ModbusArrayWriteSupportParityTests
/// <summary>
/// A spec-conformant Modbus device over the whole 0-65535 address space. It enforces the protocol quantity
/// ceilings (FC01/02 <= 2000, FC03/04 <= 125, FC15 <= 1968, FC16 <= 123, FC23 write <= 121 / read <= 125) and the
/// FC15 / FC16 byte-count field with exception 03 (Illegal Data Value), and a span past address 65535 with
/// FC15 / FC16 / FC23 byte-count field AND the exact frame length it implies (a strict device refuses a trailing
/// byte) with exception 03 (Illegal Data Value), and a span past address 65535 with
/// exception 02 (Illegal Data Address) - what a real device answers. Everything else it answers.
/// </summary>
private sealed class ConformantDevice : IModbusTransport
@@ -392,13 +393,13 @@ public sealed class ModbusArrayWriteSupportParityTests
return Task.FromResult(pdu);
case 0x0F:
if (Check(addr, qty, 1968) is { } ce) return Refuse(fc, ce);
if (pdu[5] != (qty + 7) / 8 || pdu.Length < 6 + pdu[5]) return Refuse(fc, 0x03);
if (pdu[5] != (qty + 7) / 8 || pdu.Length != 6 + pdu[5]) return Refuse(fc, 0x03); // strict frame length
WritePdus++;
for (var i = 0; i < qty; i++) Coils[addr + i] = ((pdu[6 + i / 8] >> (i % 8)) & 1) == 1;
return Task.FromResult(new byte[] { 0x0F, pdu[1], pdu[2], pdu[3], pdu[4] });
case 0x10:
if (Check(addr, qty, 123) is { } we) return Refuse(fc, we);
if (pdu[5] != qty * 2 || pdu.Length < 6 + pdu[5]) return Refuse(fc, 0x03);
if (pdu[5] != qty * 2 || pdu.Length != 6 + pdu[5]) return Refuse(fc, 0x03); // strict frame length
WritePdus++;
for (var i = 0; i < qty; i++) Holding[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] });
@@ -408,6 +409,7 @@ public sealed class ModbusArrayWriteSupportParityTests
var writeQty = (pdu[7] << 8) | pdu[8];
if (Check(addr, qty, 125) is { } r23) return Refuse(fc, r23);
if (Check(writeAddr, writeQty, 121) is { } w23) return Refuse(fc, w23);
if (pdu[9] != writeQty * 2 || pdu.Length != 10 + pdu[9]) return Refuse(fc, 0x03); // strict frame length
WritePdus++;
for (var i = 0; i < writeQty; i++) Holding[writeAddr + i] = (ushort)((pdu[10 + i * 2] << 8) | pdu[11 + i * 2]);
return Task.FromResult(ReadRegs(0x17, addr, qty));
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusWritePduFramingTests.cs
+107
View File
@@ -0,0 +1,107 @@
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests;
/// <summary>
/// #741 (Integration-W3 fix round): every multi-write PDU the driver builds is EXACTLY its header plus the bytes its
/// byte-count field declares — FC15 and FC16 are <c>6 + byteCount</c>, FC23 is <c>10 + writeByteCount</c>. The
/// builders used to allocate one byte more than they filled, so every FC15 / FC16 / FC23 (and the BitInRegister[]
/// read-modify-write's FC16) carried a trailing zero byte past the declared data — which a strict device or gateway
/// that checks the frame length against the byte count refuses.
/// </summary>
[Trait("Category", "Unit")]
public sealed class ModbusWritePduFramingTests
{
private const string Ref = "arr";
/// <summary>Records every PDU; answers reads with zeros and multi-writes with the standard echo.</summary>
private sealed class CapturingTransport : IModbusTransport
{
public readonly List<byte[]> Sent = [];
public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask;
public Task<byte[]> SendAsync(byte unitId, byte[] pdu, CancellationToken ct)
{
Sent.Add(pdu);
var qty = (pdu[3] << 8) | pdu[4];
return Task.FromResult(pdu[0] switch
{
0x01 or 0x02 => [pdu[0], (byte)((qty + 7) / 8), .. new byte[(qty + 7) / 8]],
0x03 or 0x04 or 0x17 => [pdu[0], (byte)(qty * 2), .. new byte[qty * 2]],
0x0F or 0x10 => new[] { pdu[0], pdu[1], pdu[2], pdu[3], pdu[4] },
_ => pdu,
});
}
public ValueTask DisposeAsync() => ValueTask.CompletedTask;
}
private static async Task<byte[]> WriteAndCaptureAsync(string tagConfig, object value, byte functionCode, bool fc23 = false)
{
var transport = new CapturingTransport();
var drv = new ModbusDriver(
new ModbusDriverOptions
{
Host = "fake", RawTags = [new RawTagEntry(Ref, tagConfig, false)], UseFC23ForMultiRegisterWrites = fc23,
},
"modbus-framing", _ => transport);
await drv.InitializeAsync("{}", CancellationToken.None);
(await drv.WriteAsync([new WriteRequest(Ref, value)], CancellationToken.None)).Single().StatusCode.ShouldBe(0u);
return transport.Sent.Single(p => p[0] == functionCode);
}
[Theory]
[InlineData(1)]
[InlineData(8)]
[InlineData(9)]
public async Task FC15_pdu_is_exactly_header_plus_byte_count(int coils)
{
var pdu = await WriteAndCaptureAsync(
$$"""{"region":"Coils","address":0,"isArray":true,"arrayLength":{{coils}}}""",
Enumerable.Repeat(true, coils).ToArray(), 0x0F);
pdu[5].ShouldBe((byte)((coils + 7) / 8));
pdu.Length.ShouldBe(6 + pdu[5]);
}
[Theory]
[InlineData("""{"region":"HoldingRegisters","address":0,"dataType":"Int16","isArray":true,"arrayLength":3}""", 6)]
[InlineData("""{"region":"HoldingRegisters","address":0,"dataType":"Float64","isArray":true,"arrayLength":2}""", 16)]
[InlineData("""{"region":"HoldingRegisters","address":0,"dataType":"Int32"}""", 4)] // scalar multi-register
public async Task FC16_pdu_is_exactly_header_plus_byte_count(string tagConfig, int byteCount)
{
object value = tagConfig.Contains("isArray") ? new[] { 1, 2, 3 }.Take(tagConfig.Contains("Float64") ? 2 : 3).ToArray() : 7;
var pdu = await WriteAndCaptureAsync(tagConfig, value, 0x10);
pdu[5].ShouldBe((byte)byteCount);
pdu.Length.ShouldBe(6 + pdu[5]);
}
[Fact]
public async Task BitInRegister_array_RMW_FC16_pdu_is_exactly_header_plus_byte_count()
{
var pdu = await WriteAndCaptureAsync(
"""{"region":"HoldingRegisters","address":0,"dataType":"BitInRegister","bitIndex":2,"isArray":true,"arrayLength":4}""",
new[] { true, false, true, false }, 0x10);
pdu[5].ShouldBe((byte)8);
pdu.Length.ShouldBe(6 + pdu[5]);
}
[Fact]
public async Task FC23_pdu_is_exactly_header_plus_write_byte_count()
{
var pdu = await WriteAndCaptureAsync(
"""{"region":"HoldingRegisters","address":0,"dataType":"Int16","isArray":true,"arrayLength":3}""",
new[] { 1, 2, 3 }, 0x17, fc23: true);
pdu[9].ShouldBe((byte)6);
pdu.Length.ShouldBe(10 + pdu[9]);
}
}