From d4c1873998f331449063f0014bc307d78a8ca1ef Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 18 Apr 2026 23:04:52 -0400 Subject: [PATCH] Phase 3 PR 59 -- MelsecAddress helper for MELSEC X/Y hex-vs-octal family trap + D/M bank bases. Adds MelsecAddress static class with XInputToDiscrete, YOutputToCoil, MRelayToCoil, DRegisterToHolding helpers and a MelsecFamily enum {Q_L_iQR, F_iQF} that drives whether X/Y addresses are parsed as hex (Q-series convention) or octal (FX-series convention). This is the #1 MELSEC driver bug source per docs/v2/mitsubishi.md: the string 'X20' on a MELSEC-Q means DI 32 (hex 0x20) while the same string on an FX3U means DI 16 (octal 0o20). The helper forces the caller to name the family explicitly; no 'sensible default' because wrong defaults just move the bug. Key design decisions: (1) Family is an enum argument, not a helper-level static-selector, because real deployments have BOTH Q-series and FX-series PLCs on the same gateway -- one driver instance per device means family must be per-tag, not per-driver. (2) Bank base is a ushort argument defaulting to 0. Real QJ71MT91/LJ71MT91 assignment blocks commonly place X at DI 8192+, Y at coil 8192+, etc. to leave the low-address range for D-registers; the helper takes the site's configured base as runtime config rather than a compile-time constant. Matches the 'driver opt-in per tag' pattern DirectLogicAddress established for DL260. (3) M-relay and D-register are DECIMAL on every MELSEC family -- docs explicitly; the MELSEC confusion is only about X/Y, not about data registers or internal relays. Helpers reject non-numeric M/D addresses and honor bank bases the same way. (4) Parser walks digits manually for both hex and octal (instead of int.Parse with NumberStyles) so non-hex / non-octal characters give a clear ArgumentException with the offending char + family name. Prevents a subtle class of bugs where int.Parse('X20', Hex) silently returns 32 even for F_iQF callers. Unit tests (MelsecAddressTests, 34 facts): XInputToDiscrete_QLiQR_parses_hex theory (X0, X9, XA, XF, X10, X20, X1FF + lowercase); XInputToDiscrete_FiQF_parses_octal theory (X0, X7, X10, X20, X777); YOutputToCoil equivalents; Same_address_string_decodes_differently_between_families (the headline trap, X20 => 32 on Q vs 16 on FX); reject-non-octal / reject-non-hex / reject-empty / overflow facts; honors-bank-base for X and M and D. 176/176 Modbus.Tests pass (143 prior + 34 new Melsec). No driver core changes -- this is purely a new helper class in the Driver.Modbus project. PR 60 wires it into integration tests against the mitsubishi pymodbus profile. --- .../MelsecAddress.cs | 161 ++++++++++++++++++ .../MelsecAddressTests.cs | 116 +++++++++++++ 2 files changed, 277 insertions(+) create mode 100644 src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/MelsecAddress.cs create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/MelsecAddressTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/MelsecAddress.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/MelsecAddress.cs new file mode 100644 index 0000000..7b7afd5 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/MelsecAddress.cs @@ -0,0 +1,161 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +/// +/// Mitsubishi MELSEC PLC family selector for address-translation helpers. The Q/L/iQ-R +/// families write bit-device addresses (X, Y) in hexadecimal in GX Works and the +/// CPU manuals; the FX and iQ-F families write them in octal (same convention as +/// AutomationDirect DirectLOGIC). Mixing the two up is the #1 MELSEC driver bug source — +/// an operator typing X20 into a Q-series tag config means decimal 32, but the +/// same string on an FX3U means decimal 16, so the helper must know the family to route +/// correctly. +/// +public enum MelsecFamily +{ + /// + /// MELSEC-Q / MELSEC-L / MELSEC iQ-R. X and Y device numbers are interpreted as + /// hexadecimal; X20 means decimal 32. + /// + Q_L_iQR, + + /// + /// MELSEC-F (FX3U / FX3GE / FX3G) and MELSEC iQ-F (FX5U). X and Y device numbers + /// are interpreted as octal (same as DirectLOGIC); X20 means decimal 16. + /// iQ-F has a GX Works3 project toggle that can flip to decimal — if a site uses + /// that, configure the tag's Address directly as a decimal PDU address and do not + /// route through this helper. + /// + F_iQF, +} + +/// +/// Mitsubishi MELSEC address-translation helpers for the QJ71MT91 / LJ71MT91 / RJ71EN71 / +/// iQ-R built-in / iQ-F / FX3U-ENET-P502 Modbus modules. MELSEC does NOT hard-wire +/// Modbus-to-device mappings like DL260 does — every site configures its own "Modbus +/// Device Assignment Parameter" block of up to 16 entries. The helpers here cover only +/// the address-notation portion of the translation (hex X20 vs octal X20 + adding +/// the bank base); the caller is still responsible for knowing the assignment-block +/// offset for their site. +/// +/// +/// See docs/v2/mitsubishi.md §device-assignment + §X-Y-hex-trap for the full +/// matrix and primary-source citations. +/// +public static class MelsecAddress +{ + /// + /// Translate a MELSEC X-input address (e.g. "X0", "X10") to a 0-based + /// Modbus discrete-input address, given the PLC family's address notation (hex or + /// octal) and the Modbus Device Assignment block's X-range base. + /// + /// MELSEC X address. X prefix optional, case-insensitive. + /// The PLC family — determines whether the trailing digits are hex or octal. + /// + /// 0-based Modbus DI address the assignment-block has configured X0 to land at. + /// Typical default on QJ71MT91 sample projects: 0. Pass the site-specific value. + /// + public static ushort XInputToDiscrete(string xAddress, MelsecFamily family, ushort xBankBase = 0) => + AddFamilyOffset(xBankBase, StripPrefix(xAddress, 'X'), family); + + /// + /// Translate a MELSEC Y-output address to a 0-based Modbus coil address. Same rules + /// as for hex/octal parsing. + /// + public static ushort YOutputToCoil(string yAddress, MelsecFamily family, ushort yBankBase = 0) => + AddFamilyOffset(yBankBase, StripPrefix(yAddress, 'Y'), family); + + /// + /// Translate a MELSEC M-relay address (internal relay) to a 0-based Modbus coil + /// address. M-addresses are decimal on every MELSEC family — unlike X/Y which + /// are hex on Q/L/iQ-R. Includes the bank base that the assignment-block configured. + /// + public static ushort MRelayToCoil(string mAddress, ushort mBankBase = 0) + { + var digits = StripPrefix(mAddress, 'M'); + if (!ushort.TryParse(digits, out var offset)) + throw new ArgumentException( + $"M-relay address '{mAddress}' is not a valid decimal integer", nameof(mAddress)); + var result = mBankBase + offset; + if (result > ushort.MaxValue) + throw new OverflowException($"M-relay {mAddress} + base {mBankBase} exceeds 0xFFFF"); + return (ushort)result; + } + + /// + /// Translate a MELSEC D-register address (data register) to a 0-based Modbus holding + /// register address. D-addresses are decimal. Default assignment convention is + /// D0 → HR 0 (pass = 0); sites with shifted layouts pass + /// their configured base. + /// + public static ushort DRegisterToHolding(string dAddress, ushort dBankBase = 0) + { + var digits = StripPrefix(dAddress, 'D'); + if (!ushort.TryParse(digits, out var offset)) + throw new ArgumentException( + $"D-register address '{dAddress}' is not a valid decimal integer", nameof(dAddress)); + var result = dBankBase + offset; + if (result > ushort.MaxValue) + throw new OverflowException($"D-register {dAddress} + base {dBankBase} exceeds 0xFFFF"); + return (ushort)result; + } + + private static string StripPrefix(string address, char expectedPrefix) + { + if (string.IsNullOrWhiteSpace(address)) + throw new ArgumentException("Address must not be empty", nameof(address)); + var s = address.Trim(); + if (s.Length > 0 && char.ToUpperInvariant(s[0]) == char.ToUpperInvariant(expectedPrefix)) + s = s.Substring(1); + if (s.Length == 0) + throw new ArgumentException($"Address '{address}' has no digits after prefix", nameof(address)); + return s; + } + + private static ushort AddFamilyOffset(ushort baseAddr, string digits, MelsecFamily family) + { + uint offset = family switch + { + MelsecFamily.Q_L_iQR => ParseHex(digits), + MelsecFamily.F_iQF => ParseOctal(digits), + _ => throw new ArgumentOutOfRangeException(nameof(family), family, "Unknown MELSEC family"), + }; + var result = baseAddr + offset; + if (result > ushort.MaxValue) + throw new OverflowException($"Address {baseAddr}+{offset} exceeds 0xFFFF"); + return (ushort)result; + } + + private static uint ParseHex(string digits) + { + uint result = 0; + foreach (var ch in digits) + { + uint nibble; + if (ch >= '0' && ch <= '9') nibble = (uint)(ch - '0'); + else if (ch >= 'A' && ch <= 'F') nibble = (uint)(ch - 'A' + 10); + else if (ch >= 'a' && ch <= 'f') nibble = (uint)(ch - 'a' + 10); + else throw new ArgumentException( + $"Address contains non-hex digit '{ch}' — Q/L/iQ-R X/Y addresses are hexadecimal", + nameof(digits)); + result = result * 16 + nibble; + if (result > ushort.MaxValue) + throw new OverflowException($"Hex address exceeds 0xFFFF"); + } + return result; + } + + private static uint ParseOctal(string digits) + { + uint result = 0; + foreach (var ch in digits) + { + if (ch < '0' || ch > '7') + throw new ArgumentException( + $"Address contains non-octal digit '{ch}' — FX/iQ-F X/Y addresses are octal (0-7)", + nameof(digits)); + result = result * 8 + (uint)(ch - '0'); + if (result > ushort.MaxValue) + throw new OverflowException($"Octal address exceeds 0xFFFF"); + } + return result; + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/MelsecAddressTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/MelsecAddressTests.cs new file mode 100644 index 0000000..41a4bdd --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/MelsecAddressTests.cs @@ -0,0 +1,116 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +[Trait("Category", "Unit")] +public sealed class MelsecAddressTests +{ + // --- X / Y hex vs octal family trap --- + + [Theory] + [InlineData("X0", (ushort)0)] + [InlineData("X9", (ushort)9)] + [InlineData("XA", (ushort)10)] // hex + [InlineData("XF", (ushort)15)] + [InlineData("X10", (ushort)16)] // hex 0x10 = decimal 16 + [InlineData("X20", (ushort)32)] // hex 0x20 = decimal 32 — the classic MELSEC-Q trap + [InlineData("X1FF", (ushort)511)] + [InlineData("x10", (ushort)16)] // lowercase prefix + public void XInputToDiscrete_QLiQR_parses_hex(string x, ushort expected) + => MelsecAddress.XInputToDiscrete(x, MelsecFamily.Q_L_iQR).ShouldBe(expected); + + [Theory] + [InlineData("X0", (ushort)0)] + [InlineData("X7", (ushort)7)] + [InlineData("X10", (ushort)8)] // octal 10 = decimal 8 + [InlineData("X20", (ushort)16)] // octal 20 = decimal 16 — SAME string, DIFFERENT value on FX + [InlineData("X777", (ushort)511)] + public void XInputToDiscrete_FiQF_parses_octal(string x, ushort expected) + => MelsecAddress.XInputToDiscrete(x, MelsecFamily.F_iQF).ShouldBe(expected); + + [Theory] + [InlineData("Y0", (ushort)0)] + [InlineData("Y1F", (ushort)31)] + public void YOutputToCoil_QLiQR_parses_hex(string y, ushort expected) + => MelsecAddress.YOutputToCoil(y, MelsecFamily.Q_L_iQR).ShouldBe(expected); + + [Theory] + [InlineData("Y0", (ushort)0)] + [InlineData("Y17", (ushort)15)] + public void YOutputToCoil_FiQF_parses_octal(string y, ushort expected) + => MelsecAddress.YOutputToCoil(y, MelsecFamily.F_iQF).ShouldBe(expected); + + [Fact] + public void Same_address_string_decodes_differently_between_families() + { + // This is the headline quirk: "X20" in GX Works means one thing on Q-series and + // another on FX-series. The driver's family selector is the only defence. + MelsecAddress.XInputToDiscrete("X20", MelsecFamily.Q_L_iQR).ShouldBe((ushort)32); + MelsecAddress.XInputToDiscrete("X20", MelsecFamily.F_iQF).ShouldBe((ushort)16); + } + + [Theory] + [InlineData("X8")] // 8 is non-octal + [InlineData("X12G")] // G is non-hex + public void XInputToDiscrete_FiQF_rejects_non_octal(string bad) + => Should.Throw(() => MelsecAddress.XInputToDiscrete(bad, MelsecFamily.F_iQF)); + + [Theory] + [InlineData("X12G")] + public void XInputToDiscrete_QLiQR_rejects_non_hex(string bad) + => Should.Throw(() => MelsecAddress.XInputToDiscrete(bad, MelsecFamily.Q_L_iQR)); + + [Fact] + public void XInputToDiscrete_honors_bank_base_from_assignment_block() + { + // Real-world QJ71MT91 assignment blocks commonly place X at DI 8192+ when other + // ranges take the low Modbus addresses. Helper must add the base cleanly. + MelsecAddress.XInputToDiscrete("X10", MelsecFamily.Q_L_iQR, xBankBase: 8192).ShouldBe((ushort)(8192 + 16)); + } + + // --- M-relay (decimal, both families) --- + + [Theory] + [InlineData("M0", (ushort)0)] + [InlineData("M10", (ushort)10)] // M addresses are DECIMAL, not hex or octal + [InlineData("M511", (ushort)511)] + [InlineData("m99", (ushort)99)] // lowercase + public void MRelayToCoil_parses_decimal(string m, ushort expected) + => MelsecAddress.MRelayToCoil(m).ShouldBe(expected); + + [Fact] + public void MRelayToCoil_honors_bank_base() + => MelsecAddress.MRelayToCoil("M0", mBankBase: 512).ShouldBe((ushort)512); + + [Fact] + public void MRelayToCoil_rejects_non_numeric() + => Should.Throw(() => MelsecAddress.MRelayToCoil("M1F")); + + // --- D-register (decimal, both families) --- + + [Theory] + [InlineData("D0", (ushort)0)] + [InlineData("D100", (ushort)100)] + [InlineData("d1023", (ushort)1023)] + public void DRegisterToHolding_parses_decimal(string d, ushort expected) + => MelsecAddress.DRegisterToHolding(d).ShouldBe(expected); + + [Fact] + public void DRegisterToHolding_honors_bank_base() + => MelsecAddress.DRegisterToHolding("D10", dBankBase: 4096).ShouldBe((ushort)4106); + + [Fact] + public void DRegisterToHolding_rejects_empty() + => Should.Throw(() => MelsecAddress.DRegisterToHolding("D")); + + // --- overflow --- + + [Fact] + public void XInputToDiscrete_overflow_throws() + { + // 0xFFFF + base 1 = 0x10000 — past ushort. + Should.Throw(() => + MelsecAddress.XInputToDiscrete("XFFFF", MelsecFamily.Q_L_iQR, xBankBase: 1)); + } +} -- 2.49.1