diff --git a/code-reviews/Driver.Modbus.Addressing/findings.md b/code-reviews/Driver.Modbus.Addressing/findings.md index 3a34003..e6ffadf 100644 --- a/code-reviews/Driver.Modbus.Addressing/findings.md +++ b/code-reviews/Driver.Modbus.Addressing/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-22 | | Commit reviewed | `76d35d1` | | Status | Reviewed | -| Open findings | 9 | +| Open findings | 8 | ## Checklist coverage @@ -36,7 +36,7 @@ a category produced nothing rather than leaving it blank. | Severity | High | | Category | Correctness & logic bugs | | Location | `ModbusAddressParser.cs:230-235`, `DirectLogicAddress.cs:66-73` | -| Status | Open | +| Status | Resolved | **Description:** The DL205 family-native branch routes every V-prefixed address through `DirectLogicAddress.UserVMemoryToPdu`, which is a plain octal-to-decimal conversion. DL205/DL260 @@ -54,7 +54,10 @@ route it through `SystemVMemoryToPdu`, or explicitly reject system V-memory in t with a diagnostic pointing at the structured tag form. Either way, fix the V40400 test to assert the corrected mapping. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — added `DirectLogicAddress.VMemoryToPdu`, which detects the +system bank (octal >= V40400) and relocates it through `SystemVMemoryToPdu` to PDU 0x2100; the +DL205 V branch in `ModbusAddressParser` now calls it, and the `ModbusFamilyParserTests` V40400 +assertion was corrected from 16640 to 0x2100 with system-bank regression cases added. ### Driver.Modbus.Addressing-002 diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/DirectLogicAddress.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/DirectLogicAddress.cs index 8cc60ce..94b453c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/DirectLogicAddress.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/DirectLogicAddress.cs @@ -21,9 +21,22 @@ public static class DirectLogicAddress /// depends on CPU model — DL205 D2-260 user memory is V1400-V7377 + V10000-V17777 /// octal, DL260 extends to V77777 octal. /// + /// + /// This handles ONLY user V-memory (octal address below the system base, V40400). For a + /// V-address that may fall in either bank, call , which routes + /// system-bank addresses through instead. + /// /// Input is null / empty / contains non-octal digits (8,9). /// Parsed value exceeds ushort.MaxValue (0xFFFF). public static ushort UserVMemoryToPdu(string vAddress) + => (ushort)DecodeOctalVAddress(vAddress); // DecodeOctalVAddress guards the 0xFFFF ceiling + + /// + /// Decode the octal digits of a V-address (with optional V prefix) to a uint. + /// Used both as the user-memory PDU value and as the octal magnitude that decides which + /// bank a V-address belongs to. + /// + private static uint DecodeOctalVAddress(string vAddress) { if (string.IsNullOrWhiteSpace(vAddress)) throw new ArgumentException("V-memory address must not be empty", nameof(vAddress)); @@ -46,7 +59,7 @@ public static class DirectLogicAddress throw new OverflowException( $"V-memory address '{vAddress}' exceeds the 16-bit Modbus PDU address range"); } - return (ushort)result; + return result; } /// @@ -58,6 +71,14 @@ public static class DirectLogicAddress /// public const ushort SystemVMemoryBasePdu = 0x2100; + /// + /// Octal magnitude of the first system V-memory register (V40400). A V-address whose + /// octal-decoded value is at or above this belongs to the system bank and must NOT be + /// mapped with the plain octal-to-decimal user formula — see . + /// + /// Octal 40400 == decimal 16640 (0x4100). + public const ushort SystemVMemoryOctalBase = 0x4100; // octal 40400 decoded + /// /// 0-based register offset within the system bank. Pass 0 for V40400 itself; pass 1 for /// V40401 (octal), and so on. NOT an octal-decoded value — the system bank lives at @@ -72,6 +93,35 @@ public static class DirectLogicAddress return (ushort)pdu; } + /// + /// Convert any DirectLOGIC V-memory address (octal, optional V prefix) to a 0-based + /// Modbus PDU address, routing user-bank and system-bank addresses through the correct + /// formula. User V-memory (octal < V40400) is a plain octal-to-decimal decode; system + /// V-memory (octal >= V40400) is relocated to the fixed system base at PDU 0x2100. + /// This is the address the parser must use — the system bank is NOT a simple octal decode. + /// + /// + /// The system-bank consecutive offset is the octal-decoded distance from V40400, not an + /// octal-decoded value itself: V40400 → PDU 0x2100, V40401 → 0x2101, and so on. + /// See docs/v2/dl205.md §V-Memory Addressing. + /// + /// Input is null / empty / contains non-octal digits. + /// The result exceeds the 16-bit Modbus PDU range. + public static ushort VMemoryToPdu(string vAddress) + { + var octalValue = DecodeOctalVAddress(vAddress); + if (octalValue < SystemVMemoryOctalBase) + return (ushort)octalValue; + + // System bank: the registers are contiguous from V40400, so the offset within the bank + // is the plain decimal distance from the octal base, not another octal decode. + var offsetWithinBank = octalValue - SystemVMemoryOctalBase; + if (offsetWithinBank > ushort.MaxValue) + throw new OverflowException( + $"V-memory address '{vAddress}' is outside the addressable system bank"); + return SystemVMemoryToPdu((ushort)offsetWithinBank); + } + // Bit-memory bases per DL260 user manual §I/O-configuration. // Numbers after X / Y / C / SP are OCTAL in DirectLOGIC notation. The Modbus base is // added to the octal-decoded offset; e.g. Y017 = Modbus coil 2048 + octal(17) = 2048 + 15 = 2063. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusAddressParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusAddressParser.cs index bb98f56..c571735 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusAddressParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusAddressParser.cs @@ -229,7 +229,9 @@ public static class ModbusAddressParser // SP → DiscreteInputs (special relays). if (text.StartsWith("V", StringComparison.OrdinalIgnoreCase)) { - offset = DirectLogicAddress.UserVMemoryToPdu(text); + // VMemoryToPdu routes user vs system V-memory: the system bank (octal + // >= V40400) is relocated to PDU 0x2100, NOT a plain octal decode. + offset = DirectLogicAddress.VMemoryToPdu(text); region = ModbusRegion.HoldingRegisters; return true; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusFamilyParserTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusFamilyParserTests.cs index 3971ae8..2f4afbe 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusFamilyParserTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusFamilyParserTests.cs @@ -17,8 +17,8 @@ public sealed class ModbusFamilyParserTests [Theory] [InlineData("V0", 0)] [InlineData("V2000", 1024)] // octal 2000 = decimal 1024 - [InlineData("V40400", 16640)] // octal 40400 = decimal 16640 (system bank in user mapping per the helper) - public void DL205_VMemory_To_HoldingRegisters(string addr, int expectedOffset) + [InlineData("V7777", 4095)] // octal 7777 = decimal 4095, top of the user bank example + public void DL205_UserVMemory_To_HoldingRegisters(string addr, int expectedOffset) { var p = ModbusAddressParser.Parse(addr, ModbusFamily.DL205); p.Region.ShouldBe(ModbusRegion.HoldingRegisters); @@ -26,6 +26,32 @@ public sealed class ModbusFamilyParserTests p.DataType.ShouldBe(ModbusDataType.Int16); } + // Regression: Driver.Modbus.Addressing-001. DL205/DL260 system V-memory (V40400+) is NOT a + // plain octal decode — the CPU relocates the system bank to Modbus PDU 0x2100. Octal-decoding + // V40400 yields 16640 (0x4100), the WRONG register. Per docs/v2/dl205.md §V-Memory Addressing, + // V40400 must map to PDU 0x2100 (decimal 8448) and the bank is contiguous from there. + [Theory] + [InlineData("V40400", 0x2100)] // system base + [InlineData("V40401", 0x2101)] // next register — contiguous, +1 decimal + [InlineData("V40410", 0x2108)] // octal 40410 = base + octal(10) = base + 8 decimal + public void DL205_SystemVMemory_To_HoldingRegisters_SystemBank(string addr, int expectedOffset) + { + var p = ModbusAddressParser.Parse(addr, ModbusFamily.DL205); + p.Region.ShouldBe(ModbusRegion.HoldingRegisters); + p.Offset.ShouldBe((ushort)expectedOffset); + p.DataType.ShouldBe(ModbusDataType.Int16); + } + + [Fact] + public void DL205_SystemVMemory_Helper_Routes_Through_SystemBank() + { + // Direct helper coverage: VMemoryToPdu routes user vs system; the legacy + // UserVMemoryToPdu is the plain octal decoder used only for the user bank. + DirectLogicAddress.VMemoryToPdu("V40400").ShouldBe((ushort)DirectLogicAddress.SystemVMemoryBasePdu); + DirectLogicAddress.VMemoryToPdu("V2000").ShouldBe((ushort)1024); + DirectLogicAddress.UserVMemoryToPdu("V40400").ShouldBe((ushort)16640); // plain octal decode — user-bank only + } + [Fact] public void DL205_Y_Output_Maps_To_Coils_Bank() {