From 501d8f494b1dca2bc4b5dfa3854bb4673f3b300c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Apr 2026 23:34:18 -0400 Subject: [PATCH] =?UTF-8?q?Task=20#136=20=E2=80=94=20Modicon=20address-str?= =?UTF-8?q?ing=20parser=20(5/6-digit)=20+=20shared=20addressing=20assembly?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Foundation for the Modbus addressing-grammar work tracked in #137-#145. Adds ModbusModiconAddress.Parse / TryParse that turns classic Modicon strings (40001 / 400001 / 30001 / 00001 / 10001) into (Region, ushort PduOffset). Also extracts ModbusRegion to a new Driver.Modbus.Addressing assembly so the Admin UI (#145) can reference the addressing surface without taking a dep on the wire driver. The new assembly intentionally extends the same ZB.MOM.WW.OtOpcUa.Driver.Modbus namespace as the driver — callers see the type as if it lived in one place; only the project layout changes. No existing call site needed editing (zero-churn move). Behaviour: - Single leading digit selects region (0=Coils, 1=DiscreteInputs, 3=InputRegisters, 4=HoldingRegisters). - 5-digit form: trailing 4 digits are 1-based register, supports 1..9999. - 6-digit form: trailing 5 digits are 1-based register, supports 1..65536 (full PDU address space). - Strict 5-or-6 length check; whitespace trimmed; clear FormatException diagnostics for every malformed shape (wrong length, non-digit body, illegal leading digit, register zero, register overflow). 29/29 new unit tests pass. Full Driver.Modbus suite (182 tests) and the solution-wide build still green after the ModbusRegion move. --- ZB.MOM.WW.OtOpcUa.slnx | 2 + .../ModbusModiconAddress.cs | 116 ++++++++++++++++++ .../ModbusRegion.cs | 21 ++++ ...WW.OtOpcUa.Driver.Modbus.Addressing.csproj | 19 +++ .../ModbusDriverOptions.cs | 2 - .../ZB.MOM.WW.OtOpcUa.Driver.Modbus.csproj | 1 + .../ModbusModiconAddressTests.cs | 86 +++++++++++++ ...pcUa.Driver.Modbus.Addressing.Tests.csproj | 26 ++++ 8 files changed, 271 insertions(+), 2 deletions(-) create mode 100644 src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusModiconAddress.cs create mode 100644 src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusRegion.cs create mode 100644 src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.csproj create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusModiconAddressTests.cs create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests.csproj diff --git a/ZB.MOM.WW.OtOpcUa.slnx b/ZB.MOM.WW.OtOpcUa.slnx index 02b892c..2cebeed 100644 --- a/ZB.MOM.WW.OtOpcUa.slnx +++ b/ZB.MOM.WW.OtOpcUa.slnx @@ -13,6 +13,7 @@ + @@ -48,6 +49,7 @@ + diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusModiconAddress.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusModiconAddress.cs new file mode 100644 index 0000000..1ec6f37 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusModiconAddress.cs @@ -0,0 +1,116 @@ +using System.Globalization; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +/// +/// Parses classic Modicon address strings — both 5-digit (40001) and 6-digit +/// (400001) forms — into the protocol-level + +/// zero-based PDU offset the driver speaks on the wire. +/// +/// +/// +/// Modicon notation uses a leading region digit (0 coil, 1 discrete input, +/// 3 input register, 4 holding register) followed by a 1-based register +/// number. The two forms differ only in how many trailing digits encode the register +/// number: 5-digit caps at 9999, 6-digit at 65535. Both decode to the same wire +/// representation — the PDU offset is always 0..65535 — so the only meaningful +/// distinction is range coverage. +/// +/// +/// Foundational helper for the addressing grammar work tracked in +/// docs/v2/modbus-addressing.md. The richer suffix grammar (type / bit / +/// byte-order / array) layered on top in a follow-up calls into this parser to extract +/// the region + offset before processing modifiers. +/// +/// +public static class ModbusModiconAddress +{ + /// Parse a Modicon address string. + /// Either 5-digit (40001) or 6-digit (400001) form. + /// Region + zero-based PDU offset the driver uses on the wire. + /// When the input is not a valid Modicon address. + public static (ModbusRegion Region, ushort Offset) Parse(string address) + { + if (TryParse(address, out var region, out var offset, out var error)) + return (region, offset); + throw new FormatException(error); + } + + /// + /// Try-parse variant for hot-path / config-bind scenarios where a parse failure should + /// surface a structured diagnostic rather than throw. is + /// null on success. + /// + public static bool TryParse(string? address, out ModbusRegion region, out ushort offset, out string? error) + { + region = default; + offset = 0; + + if (string.IsNullOrWhiteSpace(address)) + { + error = "Modicon address is null or empty"; + return false; + } + + // Range check up-front — keeps the rest of the parser straight-line. 5-digit Modicon + // is always exactly 5 chars (40001..49999, with the lead digit selecting region), and + // 6-digit is exactly 6 (400001..465536-shaped). Anything else is unambiguously + // malformed so we reject before doing the per-character work. + var s = address.Trim(); + if (s.Length is not (5 or 6)) + { + error = $"Modicon address must be 5 or 6 digits, got {s.Length} ('{address}')"; + return false; + } + + if (!s.All(char.IsDigit)) + { + error = $"Modicon address must contain only digits ('{address}')"; + return false; + } + + var leading = s[0]; + region = leading switch + { + '0' => ModbusRegion.Coils, + '1' => ModbusRegion.DiscreteInputs, + '3' => ModbusRegion.InputRegisters, + '4' => ModbusRegion.HoldingRegisters, + _ => (ModbusRegion)(-1), + }; + if ((int)region == -1) + { + error = $"Modicon address leading digit must be 0/1/3/4, got '{leading}'"; + return false; + } + + // The remaining 4 (5-digit) or 5 (6-digit) digits are the 1-based register number. + // 1-based-to-0-based conversion happens here so callers downstream uniformly see PDU + // offsets — which is what the wire format actually uses. + var registerNumberText = s[1..]; + if (!int.TryParse(registerNumberText, NumberStyles.None, CultureInfo.InvariantCulture, out var registerNumber)) + { + error = $"Modicon register number is not a valid integer ('{registerNumberText}')"; + return false; + } + + if (registerNumber < 1) + { + error = $"Modicon register number must be >= 1 (got {registerNumber})"; + return false; + } + + // 5-digit form caps at 9999 by construction (4 trailing digits); reject if the parsed + // value exceeds the wire-protocol maximum of 65536 (i.e. PDU offset 65535). 6-digit + // form can address the full 65535-offset range. + if (registerNumber > 65536) + { + error = $"Modicon register number {registerNumber} exceeds the wire maximum (65536 / PDU offset 65535)"; + return false; + } + + offset = (ushort)(registerNumber - 1); + error = null; + return true; + } +} diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusRegion.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusRegion.cs new file mode 100644 index 0000000..6edbd05 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusRegion.cs @@ -0,0 +1,21 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +/// +/// The four Modbus register regions a tag can target. Maps directly to function-code +/// selection on the wire: uses FC01/FC05/FC15, +/// uses FC02 (read-only), uses FC04 (read-only), and +/// uses FC03/FC06/FC16. +/// +/// +/// Lives in the shared addressing assembly so Admin UI and the parser library can speak +/// about regions without taking a dependency on the wire driver. The driver-side +/// Driver.Modbus assembly extends the same namespace, so callers see this type as +/// if it lived in one place. +/// +public enum ModbusRegion +{ + Coils, + DiscreteInputs, + InputRegisters, + HoldingRegisters, +} diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.csproj b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.csproj new file mode 100644 index 0000000..b89fe62 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.csproj @@ -0,0 +1,19 @@ + + + + net10.0 + enable + enable + latest + true + true + $(NoWarn);CS1591 + ZB.MOM.WW.OtOpcUa.Driver.Modbus + Pure Modbus address-grammar parsing — shared by Driver.Modbus, Driver.Modbus.Cli, and Admin so the wire driver and the configuration UI agree on a single grammar definition. Namespace is intentionally identical to Driver.Modbus's so callers see addressing types as if they lived in one assembly; this assembly stays dependency-free so Admin can reference it without taking a transport-layer dep. + + + + + + + diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs index c119d4e..9e60b5c 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverOptions.cs @@ -112,8 +112,6 @@ public sealed record ModbusTagDefinition( ModbusStringByteOrder StringByteOrder = ModbusStringByteOrder.HighByteFirst, bool WriteIdempotent = false); -public enum ModbusRegion { Coils, DiscreteInputs, InputRegisters, HoldingRegisters } - public enum ModbusDataType { Bool, diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ZB.MOM.WW.OtOpcUa.Driver.Modbus.csproj b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ZB.MOM.WW.OtOpcUa.Driver.Modbus.csproj index 5a77a80..0d096c6 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ZB.MOM.WW.OtOpcUa.Driver.Modbus.csproj +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ZB.MOM.WW.OtOpcUa.Driver.Modbus.csproj @@ -14,6 +14,7 @@ + diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusModiconAddressTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusModiconAddressTests.cs new file mode 100644 index 0000000..334b7fe --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusModiconAddressTests.cs @@ -0,0 +1,86 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests; + +[Trait("Category", "Unit")] +public sealed class ModbusModiconAddressTests +{ + [Theory] + // 5-digit form, one per region. Trailing 4 digits = 1-based register; PDU offset is one less. + [InlineData("00001", ModbusRegion.Coils, (ushort)0)] + [InlineData("09999", ModbusRegion.Coils, (ushort)9998)] + [InlineData("10001", ModbusRegion.DiscreteInputs, (ushort)0)] + [InlineData("19999", ModbusRegion.DiscreteInputs, (ushort)9998)] + [InlineData("30001", ModbusRegion.InputRegisters, (ushort)0)] + [InlineData("39999", ModbusRegion.InputRegisters, (ushort)9998)] + [InlineData("40001", ModbusRegion.HoldingRegisters, (ushort)0)] + [InlineData("49999", ModbusRegion.HoldingRegisters, (ushort)9998)] + // 6-digit form unlocks the full 16-bit address space — 1..65536 → PDU 0..65535. + [InlineData("400001", ModbusRegion.HoldingRegisters, (ushort)0)] + [InlineData("410000", ModbusRegion.HoldingRegisters, (ushort)9999)] + [InlineData("465536", ModbusRegion.HoldingRegisters, (ushort)65535)] + [InlineData("000001", ModbusRegion.Coils, (ushort)0)] + [InlineData("100001", ModbusRegion.DiscreteInputs, (ushort)0)] + [InlineData("365536", ModbusRegion.InputRegisters, (ushort)65535)] + public void Parse_Valid(string address, ModbusRegion expectedRegion, ushort expectedOffset) + { + var (region, offset) = ModbusModiconAddress.Parse(address); + region.ShouldBe(expectedRegion); + offset.ShouldBe(expectedOffset); + } + + [Theory] + [InlineData("", "null or empty")] + [InlineData(" ", "null or empty")] + [InlineData("4001", "5 or 6 digits")] // 4 chars + [InlineData("4000001", "5 or 6 digits")] // 7 chars + [InlineData("4000A", "only digits")] // letter in body + [InlineData("X0001", "only digits")] // letter leading + [InlineData("20001", "leading digit must be 0/1/3/4")] // region 2 doesn't exist + [InlineData("50001", "leading digit must be 0/1/3/4")] + [InlineData("90001", "leading digit must be 0/1/3/4")] + [InlineData("40000", "must be >= 1")] // 0-based register number + [InlineData("400000", "must be >= 1")] // 6-digit zero + public void Parse_Invalid_Surfaces_Diagnostic(string address, string fragment) + { + Should.Throw(() => ModbusModiconAddress.Parse(address)) + .Message.ShouldContain(fragment, Case.Insensitive); + } + + [Fact] + public void TryParse_Returns_False_With_Diagnostic_On_Invalid() + { + var ok = ModbusModiconAddress.TryParse("not-an-address", out _, out _, out var error); + ok.ShouldBeFalse(); + error.ShouldNotBeNull(); + } + + [Fact] + public void TryParse_Returns_True_With_Null_Error_On_Valid() + { + var ok = ModbusModiconAddress.TryParse("40001", out var region, out var offset, out var error); + ok.ShouldBeTrue(); + region.ShouldBe(ModbusRegion.HoldingRegisters); + offset.ShouldBe((ushort)0); + error.ShouldBeNull(); + } + + [Fact] + public void TryParse_Handles_Null() + { + ModbusModiconAddress.TryParse(null, out _, out _, out var error).ShouldBeFalse(); + error.ShouldNotBeNull(); + } + + [Fact] + public void TryParse_Trims_Whitespace() + { + // Tag spreadsheets often arrive with stray padding; the parser tolerates it rather than + // forcing every importer to trim — but stays strict on the 5/6-digit length once trimmed. + ModbusModiconAddress.TryParse(" 40001 ", out var region, out var offset, out _).ShouldBeTrue(); + region.ShouldBe(ModbusRegion.HoldingRegisters); + offset.ShouldBe((ushort)0); + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests.csproj b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests.csproj new file mode 100644 index 0000000..b865061 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests.csproj @@ -0,0 +1,26 @@ + + + + net10.0 + enable + enable + false + true + ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + + + + +