From cd19022d191d799973bb5be529a1c73d2d826604 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 18 Apr 2026 21:43:32 -0400 Subject: [PATCH] Phase 3 PR 45 -- DL205 string byte-order quirk (low-byte-first ASCII packing). Adds ModbusStringByteOrder enum {HighByteFirst, LowByteFirst} + StringByteOrder field on ModbusTagDefinition (default HighByteFirst, the standard Modbus convention). DecodeRegister + EncodeRegister String branches now respect per-tag byte order. Under LowByteFirst each register packs the first char in the low byte instead of the high byte -- the AutomationDirect DirectLOGIC DL205/DL260/DL350 family's headline string quirk. Without the flag the driver decodes 'eHllo' garbage from HR[1040..1042] even though wire bytes are identical. Unit tests: String_LowByteFirst_decodes_DL205_packed_Hello (5 chars across 3 regs with nul pad), String_LowByteFirst_decode_truncates_at_first_nul, String_LowByteFirst_encode_round_trips_with_decode (asserts exact DL205-documented byte sequence {0x65,0x48,0x6C,0x6C,0x00,0x6F} + symmetric encode->decode), String_HighByteFirst_and_LowByteFirst_differ_on_same_wire (control: same wire, different flag => different decode). 56/56 Modbus.Tests pass. Integration test: DL205StringQuirkTests.DL205_string_low_byte_first_decodes_Hello_from_HR1040 against the dl205.json pymodbus profile; reads HR[1040..1042] with both flags on the same tag map and asserts LowByteFirst='Hello' + HighByteFirst!='Hello'. Gated on MODBUS_SIM_PROFILE=dl205 since the standard profile doesn't seed HR[1040..1042]. Verified 2/2 integration tests pass against running pymodbus dl205 simulator. Baseline for PR 46 (BCD decoder), PR 47 (V-memory octal helper), PR 48 (CDAB float order), PR 49 (FC03/FC16 per-device caps) -- each lands its own DL205_ test class in tests/.../DL205/. --- .../ModbusDriver.cs | 25 ++++-- .../ModbusDriverOptions.cs | 23 +++++- .../DL205/DL205StringQuirkTests.cs | 81 +++++++++++++++++++ .../ModbusDataTypeTests.cs | 47 +++++++++++ 4 files changed, 170 insertions(+), 6 deletions(-) create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/DL205/DL205StringQuirkTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index 33a1889..13c9cf0 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -472,13 +472,21 @@ public sealed class ModbusDriver(ModbusDriverOptions options, string driverInsta } case ModbusDataType.String: { - // ASCII, 2 chars per register, packed high byte = first char. - // Respect the caller's StringLength (truncate nul-padded regions). + // ASCII, 2 chars per register. HighByteFirst (standard) packs the first char in + // the high byte of each register; LowByteFirst (DL205/DL260) packs the first char + // in the low byte. Respect StringLength (truncate nul-padded regions). var chars = new char[tag.StringLength]; for (var i = 0; i < tag.StringLength; i++) { - var b = data[i]; - if (b == 0) { return new string(chars, 0, i); } + var regIdx = i / 2; + var highByte = data[regIdx * 2]; + var lowByte = data[regIdx * 2 + 1]; + byte b; + if (tag.StringByteOrder == ModbusStringByteOrder.HighByteFirst) + b = (i % 2 == 0) ? highByte : lowByte; + else + b = (i % 2 == 0) ? lowByte : highByte; + if (b == 0) return new string(chars, 0, i); chars[i] = (char)b; } return new string(chars); @@ -543,7 +551,14 @@ public sealed class ModbusDriver(ModbusDriverOptions options, string driverInsta var s = Convert.ToString(value) ?? string.Empty; var regs = (tag.StringLength + 1) / 2; var b = new byte[regs * 2]; - for (var i = 0; i < tag.StringLength && i < s.Length; i++) b[i] = (byte)s[i]; + for (var i = 0; i < tag.StringLength && i < s.Length; i++) + { + var regIdx = i / 2; + var destIdx = tag.StringByteOrder == ModbusStringByteOrder.HighByteFirst + ? (i % 2 == 0 ? regIdx * 2 : regIdx * 2 + 1) + : (i % 2 == 0 ? regIdx * 2 + 1 : regIdx * 2); + b[destIdx] = (byte)s[i]; + } // remaining bytes stay 0 — nul-padded per PLC convention return b; } diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs index 79c5c41..aeae634 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs @@ -55,6 +55,12 @@ public sealed class ModbusProbeOptions /// Word ordering for multi-register types. Ignored for Bool / Int16 / UInt16 / BitInRegister / String. /// For DataType = BitInRegister: which bit of the holding register (0-15, LSB-first). /// For DataType = String: number of ASCII characters (2 per register, rounded up). +/// +/// Per-register byte order for DataType = String. Standard Modbus packs the first +/// character in the high byte (). +/// AutomationDirect DirectLOGIC (DL205/DL260) and a few legacy families pack the first +/// character in the low byte instead — see docs/v2/dl205.md §strings. +/// public sealed record ModbusTagDefinition( string Name, ModbusRegion Region, @@ -63,7 +69,8 @@ public sealed record ModbusTagDefinition( bool Writable = true, ModbusByteOrder ByteOrder = ModbusByteOrder.BigEndian, byte BitIndex = 0, - ushort StringLength = 0); + ushort StringLength = 0, + ModbusStringByteOrder StringByteOrder = ModbusStringByteOrder.HighByteFirst); public enum ModbusRegion { Coils, DiscreteInputs, InputRegisters, HoldingRegisters } @@ -95,3 +102,17 @@ public enum ModbusByteOrder BigEndian, WordSwap, } + +/// +/// Per-register byte order for ASCII strings packed 2 chars per register. Standard Modbus +/// convention is — the first character of each pair occupies +/// the high byte of the register. AutomationDirect DirectLOGIC (DL205, DL260, DL350) and a +/// handful of legacy controllers pack , which inverts that within +/// each register. Word ordering across multiple registers is always ascending address for +/// strings — only the byte order inside each register flips. +/// +public enum ModbusStringByteOrder +{ + HighByteFirst, + LowByteFirst, +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/DL205/DL205StringQuirkTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/DL205/DL205StringQuirkTests.cs new file mode 100644 index 0000000..fbc35b3 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/DL205/DL205StringQuirkTests.cs @@ -0,0 +1,81 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests.DL205; + +/// +/// Verifies the DL205/DL260 low-byte-first ASCII string packing quirk against the +/// dl205.json pymodbus profile. Standard Modbus packs the first char of each pair +/// in the high byte of the register; DirectLOGIC packs it in the low byte instead. Without +/// the driver decodes "eHllo" garbage +/// even though the bytes on the wire are identical. +/// +/// +/// +/// Requires the dl205 profile (Pymodbus\serve.ps1 -Profile dl205). The standard +/// profile does not seed HR[1040..1042] with string bytes, so running this against the +/// standard profile returns "\0\0\0\0\0" and the test fails. Skip when the env +/// var MODBUS_SIM_PROFILE is not set to dl205. +/// +/// +[Collection(ModbusSimulatorCollection.Name)] +[Trait("Category", "Integration")] +[Trait("Device", "DL205")] +public sealed class DL205StringQuirkTests(ModbusSimulatorFixture sim) +{ + [Fact] + public async Task DL205_string_low_byte_first_decodes_Hello_from_HR1040() + { + if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + if (!string.Equals(Environment.GetEnvironmentVariable("MODBUS_SIM_PROFILE"), "dl205", + StringComparison.OrdinalIgnoreCase)) + { + Assert.Skip("MODBUS_SIM_PROFILE != dl205 — skipping (standard profile does not seed HR[1040..1042])."); + } + + var options = new ModbusDriverOptions + { + Host = sim.Host, + Port = sim.Port, + UnitId = 1, + Timeout = TimeSpan.FromSeconds(2), + Tags = + [ + new ModbusTagDefinition( + Name: "DL205_Hello_Low", + Region: ModbusRegion.HoldingRegisters, + Address: 1040, + DataType: ModbusDataType.String, + Writable: false, + StringLength: 5, + StringByteOrder: ModbusStringByteOrder.LowByteFirst), + // Control: same address, HighByteFirst, to prove the driver would have decoded + // garbage without the quirk flag. + new ModbusTagDefinition( + Name: "DL205_Hello_High", + Region: ModbusRegion.HoldingRegisters, + Address: 1040, + DataType: ModbusDataType.String, + Writable: false, + StringLength: 5, + StringByteOrder: ModbusStringByteOrder.HighByteFirst), + ], + Probe = new ModbusProbeOptions { Enabled = false }, + }; + await using var driver = new ModbusDriver(options, driverInstanceId: "dl205-string"); + await driver.InitializeAsync(driverConfigJson: "{}", TestContext.Current.CancellationToken); + + var results = await driver.ReadAsync(["DL205_Hello_Low", "DL205_Hello_High"], + TestContext.Current.CancellationToken); + + results.Count.ShouldBe(2); + results[0].StatusCode.ShouldBe(0u); + results[0].Value.ShouldBe("Hello", "DL205 low-byte-first ordering must produce 'Hello' from HR[1040..1042]"); + + // The high-byte-first read of the same wire bytes should differ — not asserting the + // exact garbage string (that would couple the test to the ASCII byte math) but the two + // decodes MUST disagree, otherwise the quirk flag is a no-op. + results[1].StatusCode.ShouldBe(0u); + results[1].Value.ShouldNotBe("Hello"); + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDataTypeTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDataTypeTests.cs index 1c0442b..6b07f3a 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDataTypeTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusDataTypeTests.cs @@ -172,4 +172,51 @@ public sealed class ModbusDataTypeTests wire[1].ShouldBe((byte)'i'); for (var i = 2; i < 8; i++) wire[i].ShouldBe((byte)0); } + + // --- DL205 low-byte-first strings (AutomationDirect DirectLOGIC quirk) --- + + [Fact] + public void String_LowByteFirst_decodes_DL205_packed_Hello() + { + // HR[1040] = 0x6548 (wire BE bytes [0x65, 0x48]) decodes first char from low byte = 'H', + // second from high byte = 'e'. HR[1041] = 0x6C6C → 'l','l'. HR[1042] = 0x006F → 'o', nul. + var tag = new ModbusTagDefinition("T", ModbusRegion.HoldingRegisters, 0, ModbusDataType.String, + StringLength: 5, StringByteOrder: ModbusStringByteOrder.LowByteFirst); + var wire = new byte[] { 0x65, 0x48, 0x6C, 0x6C, 0x00, 0x6F }; + ModbusDriver.DecodeRegister(wire, tag).ShouldBe("Hello"); + } + + [Fact] + public void String_LowByteFirst_decode_truncates_at_first_nul() + { + // Low-byte-first with only 2 real chars in register 0 (lo='H', hi='i') and the rest nul. + var tag = new ModbusTagDefinition("T", ModbusRegion.HoldingRegisters, 0, ModbusDataType.String, + StringLength: 6, StringByteOrder: ModbusStringByteOrder.LowByteFirst); + var wire = new byte[] { 0x69, 0x48, 0x00, 0x00, 0x00, 0x00 }; + ModbusDriver.DecodeRegister(wire, tag).ShouldBe("Hi"); + } + + [Fact] + public void String_LowByteFirst_encode_round_trips_with_decode() + { + var tag = new ModbusTagDefinition("T", ModbusRegion.HoldingRegisters, 0, ModbusDataType.String, + StringLength: 5, StringByteOrder: ModbusStringByteOrder.LowByteFirst); + var wire = ModbusDriver.EncodeRegister("Hello", tag); + // Expect exactly the DL205-documented byte sequence. + wire.ShouldBe(new byte[] { 0x65, 0x48, 0x6C, 0x6C, 0x00, 0x6F }); + ModbusDriver.DecodeRegister(wire, tag).ShouldBe("Hello"); + } + + [Fact] + public void String_HighByteFirst_and_LowByteFirst_differ_on_same_wire() + { + // Same wire buffer, different byte order → first char switches 'H' vs 'e'. + var wire = new byte[] { 0x48, 0x65 }; + var hi = new ModbusTagDefinition("T", ModbusRegion.HoldingRegisters, 0, ModbusDataType.String, + StringLength: 2, StringByteOrder: ModbusStringByteOrder.HighByteFirst); + var lo = new ModbusTagDefinition("T", ModbusRegion.HoldingRegisters, 0, ModbusDataType.String, + StringLength: 2, StringByteOrder: ModbusStringByteOrder.LowByteFirst); + ModbusDriver.DecodeRegister(wire, hi).ShouldBe("He"); + ModbusDriver.DecodeRegister(wire, lo).ShouldBe("eH"); + } } -- 2.49.1