dohertj2/lmxopcua
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(driver-modbus-addressing): resolve High code-review finding (Driver.Modbus.Addressing-001)

Browse Source 
The DL205 family-native branch routed every V-prefixed address through
DirectLogicAddress.UserVMemoryToPdu, a plain octal-to-decimal decode.
DL205/DL260 system V-memory (V40400 and up) is not a simple octal decode:
the CPU relocates the system bank to Modbus PDU 0x2100. Octal-decoding
V40400 produced 16640 (0x4100), the wrong register, so any tag addressing
a system register through the grammar string silently read/wrote the
wrong PLC memory.

- Add DirectLogicAddress.VMemoryToPdu, which decodes the octal V-address,
  detects the system bank (octal >= V40400 == SystemVMemoryOctalBase) and
  relocates it through SystemVMemoryToPdu to PDU 0x2100; user-bank
  addresses keep the plain octal decode.
- ModbusAddressParser's DL205 V branch now calls VMemoryToPdu instead of
  UserVMemoryToPdu. UserVMemoryToPdu is retained for user-bank-only callers.
- Correct the ModbusFamilyParserTests V40400 assertion (16640 -> 0x2100)
  and add system-bank regression cases plus direct helper coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-22 06:53:59 -04:00
parent 532b961cf2
commit 1837b5a828
4 changed files with 88 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
code-reviews/Driver.Modbus.Addressing/findings.md
View File

@@ -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

52
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/DirectLogicAddress.cs
View File

@@ -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.
/// </summary>
/// <remarks>
/// This handles ONLY user V-memory (octal address below the system base, V40400). For a
/// V-address that may fall in either bank, call <see cref="VMemoryToPdu"/>, which routes
/// system-bank addresses through <see cref="SystemVMemoryToPdu"/> instead.
/// </remarks>
/// <exception cref="ArgumentException">Input is null / empty / contains non-octal digits (8,9).</exception>
/// <exception cref="OverflowException">Parsed value exceeds ushort.MaxValue (0xFFFF).</exception>
public static ushort UserVMemoryToPdu(string vAddress)
=> (ushort)DecodeOctalVAddress(vAddress); // DecodeOctalVAddress guards the 0xFFFF ceiling
/// <summary>
/// Decode the octal digits of a V-address (with optional <c>V</c> 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.
/// </summary>
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;
}
/// <summary>
@@ -58,6 +71,14 @@ public static class DirectLogicAddress
/// </summary>
public const ushort SystemVMemoryBasePdu = 0x2100;
/// <summary>
/// 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 <see cref="VMemoryToPdu"/>.
/// </summary>
/// <remarks>Octal 40400 == decimal 16640 (0x4100).</remarks>
public const ushort SystemVMemoryOctalBase = 0x4100; // octal 40400 decoded
/// <param name="offsetWithinSystemBank">
/// 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;
}
/// <summary>
/// Convert any DirectLOGIC V-memory address (octal, optional <c>V</c> prefix) to a 0-based
/// Modbus PDU address, routing user-bank and system-bank addresses through the correct
/// formula. User V-memory (octal &lt; V40400) is a plain octal-to-decimal decode; system
/// V-memory (octal &gt;= 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.
/// </summary>
/// <remarks>
/// 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 <c>docs/v2/dl205.md</c> §V-Memory Addressing.
/// </remarks>
/// <exception cref="ArgumentException">Input is null / empty / contains non-octal digits.</exception>
/// <exception cref="OverflowException">The result exceeds the 16-bit Modbus PDU range.</exception>
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.

4
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing/ModbusAddressParser.cs
View File

@@ -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;
}

30
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Addressing.Tests/ModbusFamilyParserTests.cs
View File

@@ -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()
{