From e7d7b6cb1d0dbfb91791e8e0ad8df6505038d3fb Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 22 May 2026 09:53:12 -0400 Subject: [PATCH] fix(driver-modbus-addressing): resolve Medium code-review finding (Driver.Modbus.Addressing-008) Add ModbusAddressEdgeCaseTests.cs covering the overflow/boundary gaps: empty trailing parser field (finding -002 regression), multi-dot input, UserVMemoryToPdu and AddOctalOffset overflow, SystemVMemoryToPdu base+overflow, MelsecAddress ParseHex overflow, and DRegisterToHolding/MRelayToCoil bank-base overflow. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ModbusAddressEdgeCaseTests.cs | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusAddressEdgeCaseTests.cs diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusAddressEdgeCaseTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusAddressEdgeCaseTests.cs new file mode 100644 index 0000000..25c28bc --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusAddressEdgeCaseTests.cs @@ -0,0 +1,149 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests; + +/// +/// Driver.Modbus.Addressing-008: boundary and overflow edge cases for the address-arithmetic +/// helpers and the parser input-validation paths. These cover the risk surface cited in the +/// code review: overflow in DL205 / MELSEC helpers, empty trailing parser fields (finding +/// -002), and coverage of (finding -001 +/// regression guard), which was previously unreachable from the parser. +/// +[Trait("Category", "Unit")] +public sealed class ModbusAddressEdgeCaseTests +{ + // ── Parser: empty trailing-field rejection (Driver.Modbus.Addressing-002) ────────────── + + [Fact] + public void Parser_3field_empty_third_field_rejected() + { + // "40001:F:" — trailing colon with nothing after it must produce a diagnostic, not + // silently parse as a scalar (Enumerable.All returns true for an empty sequence). + var ok = ModbusAddressParser.TryParse("40001:F:", out _, out var error); + ok.ShouldBeFalse(); + error.ShouldNotBeNullOrEmpty(); + error!.ShouldContain("empty"); + } + + [Fact] + public void Parser_4field_empty_third_field_accepted_as_default_order() + { + // "40001:F::5" — empty order field in 4-field form IS valid (means default byte order). + // This is a different case from the 3-field "40001:F:" empty trailing colon. + var ok = ModbusAddressParser.TryParse("40001:F::5", out var result, out _); + ok.ShouldBeTrue(); + result!.ByteOrder.ShouldBe(ModbusByteOrder.BigEndian); + result.ArrayCount.ShouldBe(5); + } + + // ── Parser: misplaced type code gives better diagnostic (Driver.Modbus.Addressing-003) ─ + + [Fact] + public void Parser_3field_misplaced_type_in_third_field_gives_helpful_error() + { + // "40001:S:BOOL" — BOOL is a 4-letter type code typed in the byte-order field. + // The parser should mention that field 3 is a byte order, not a type. + var ok = ModbusAddressParser.TryParse("40001:S:BOOL", out _, out var error); + ok.ShouldBeFalse(); + error.ShouldNotBeNullOrEmpty(); + // The error should guide the user toward the correct field (field 2 for type). + error!.ShouldContain("field 2", Case.Insensitive); + } + + // ── Parser: multi-dot input (Driver.Modbus.Addressing-004) ────────────────────────────── + + [Fact] + public void Parser_multi_dot_input_rejected_with_clear_error() + { + // "40001.5.3" — multiple dots should not silently parse bit as "5.3". + var ok = ModbusAddressParser.TryParse("40001.5.3", out _, out var error); + ok.ShouldBeFalse(); + error.ShouldNotBeNullOrEmpty(); + } + + [Fact] + public void Parser_decimal_point_typo_400_01_gives_precise_error() + { + // "400.01" — looks like a Modicon decimal typo. The bit suffix "01" is valid (bit index + // 1), but "400" fails Modicon validation with a length error — NOT a bit-index error. + // Verify the parser fails (any diagnostic is acceptable; we just check it fails). + var ok = ModbusAddressParser.TryParse("400.01", out _, out var error); + ok.ShouldBeFalse(); + error.ShouldNotBeNullOrEmpty(); + } + + // ── DirectLogicAddress overflow and boundary (Driver.Modbus.Addressing-008) ──────────── + + [Fact] + public void UserVMemoryToPdu_overflow_throws_OverflowException() + { + // V200000 in octal = 65536 decimal — just beyond ushort.MaxValue. + Should.Throw(() => DirectLogicAddress.UserVMemoryToPdu("V200000")); + } + + [Fact] + public void AddOctalOffset_overflow_via_Y_throws_OverflowException() + { + // Y prefix with an octal value that pushes YOutputBaseCoil (2048) past 0xFFFF. + // YOutputBaseCoil = 2048; we need octal digits that decode to > 65535 - 2048 = 63487. + // 63488 in octal = 174000; "Y174000" should overflow. + Should.Throw(() => DirectLogicAddress.YOutputToCoil("Y174000")); + } + + [Fact] + public void AddOctalOffset_overflow_via_C_throws_OverflowException() + { + // CRelayBaseCoil = 3072; we need offset > 65535 - 3072 = 62463. + // 62464 in octal = 172000; "C172000" should overflow. + Should.Throw(() => DirectLogicAddress.CRelayToCoil("C172000")); + } + + [Fact] + public void SystemVMemoryToPdu_is_exercised_and_returns_correct_base() + { + // Direct coverage of SystemVMemoryToPdu — previously unreachable from the parser + // before Driver.Modbus.Addressing-001 was fixed (VMemoryToPdu now calls it). + DirectLogicAddress.SystemVMemoryToPdu(0).ShouldBe(DirectLogicAddress.SystemVMemoryBasePdu); + DirectLogicAddress.SystemVMemoryToPdu(1).ShouldBe((ushort)(DirectLogicAddress.SystemVMemoryBasePdu + 1)); + } + + [Fact] + public void SystemVMemoryToPdu_overflow_throws_OverflowException() + { + // An offset that pushes SystemVMemoryBasePdu (0x2100 = 8448) past 0xFFFF. + // 0xFFFF - 0x2100 + 1 = 57088 (0xDF00) should overflow. + Should.Throw(() => DirectLogicAddress.SystemVMemoryToPdu(0xDF00)); + } + + // ── MelsecAddress overflow / boundary (Driver.Modbus.Addressing-008) ───────────────── + + [Fact] + public void MelsecAddress_ParseHex_overflow_throws_OverflowException() + { + // X address in Q-family (hex): "X10000" = 0x10000 = 65536, overflows ushort. + Should.Throw(() => MelsecAddress.XInputToDiscrete("X10000", MelsecFamily.Q_L_iQR)); + } + + [Fact] + public void MelsecAddress_DRegisterToHolding_overflow_throws_OverflowException() + { + // D65536 + base 0 = 65536, overflows ushort.MaxValue. + Should.Throw(() => MelsecAddress.DRegisterToHolding("D65536")); + } + + [Fact] + public void MelsecAddress_MRelayToCoil_overflow_throws_OverflowException() + { + // M65535 with base 1 = 65536, overflows. + Should.Throw(() => MelsecAddress.MRelayToCoil("M65535", mBankBase: 1)); + } + + [Fact] + public void MelsecAddress_DRegisterToHolding_bank_base_overflow_throws_OverflowException() + { + // D0 with a bank base that itself overflows: base 65535 + D1 = 65536. + Should.Throw(() => MelsecAddress.DRegisterToHolding("D1", dBankBase: 65535)); + } +}