From 5533de62814f04d261dba1ccaddf17c64da585c0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:10:00 -0400 Subject: [PATCH 1/3] feat(arrays): Modbus String[]/BitInRegister[] array writes reachable; table follows the resolver The Modbus lane (#741 U3) gave the driver String[] and BitInRegister[] holding-register array encoders, and moved address resolution to ModbusTagAddressResolution (keys first, a bit region coerced to Bool, strict key reads), but DriverArrayWriteSupport still listed both array shapes read-only (follow-up F1), so a client write was BadNotWritable before it reached the driver. Modbus row now mirrors the driver end to end: - Keys shape only (region + address both present). A string-only addressString tag is resolved by the driver with its own family through a parser Core.Abstractions cannot reference, so it stays read-only (pinned: the conservative direction). - Strict reads as the resolver: address an integer 0-65535; a present region / dataType / byteOrder / stringByteOrder string must name a member. - Coils: any dataType (coerced to Bool), <= 1968 coils (FC15). - HoldingRegisters (FC16, never Bool): numeric, String ((stringLength+1)/2 registers, stringLength >= 1), BitInRegister (1 register, bitIndex 0-15, span ending <= 65535), all within maxRegistersPerWrite. FC23 is opt-in and never changes the answer. - Elements convert (ScadaBridge int[] -> Int16 stays writable); only a String / DateTime node over a numeric or bit element is left out, since such an element never converts. Tests: the parity test's PendingF1 exemption and its guard are gone; the parity matrix is now region x element type x node type against the real WriteAsync with a spec-conformant device (FC15 > 1968 refused), plus shape edges at every cap boundary (numeric, String, BitInRegister, coil), strict key reads, FC23 on/off, string-only, the ScadaBridge int[] shape and U3 wire bytes. Core.Abstractions row tests and a composer test (String[] and BitInRegister[] writable; over-cap, no-stringLength and string-only read-only). Docs: table XML doc, Uns.md, modbus-addressing.md, drivers/Modbus.md, data-plane rules. Refs #741 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .claude/rules/data-plane-auth.md | 5 +- docs/Uns.md | 9 +- docs/drivers/Modbus.md | 12 +- docs/v2/modbus-addressing.md | 35 ++ .../DriverArrayWriteSupport.cs | 138 +++++- .../DriverArrayWriteSupportTests.cs | 54 ++- .../ModbusArrayWriteSupportParityTests.cs | 414 ++++++++++++++---- .../AddressSpaceComposerDualNamespaceTests.cs | 60 +++ 8 files changed, 597 insertions(+), 130 deletions(-) diff --git a/.claude/rules/data-plane-auth.md b/.claude/rules/data-plane-auth.md index ca9f7886..0287d536 100644 --- a/.claude/rules/data-plane-auth.md +++ b/.claude/rules/data-plane-auth.md @@ -148,8 +148,9 @@ Two things to know before relying on any of this: `refuseArrayOnScalar` — pass-through drivers keep their browse-committed scalar-shaped array tags writable; a new sink wrapper must forward that argument, and the reflection guard checks only that the call arrives); `IndexRange` writes ⇒ `BadWriteNotSupported`, `ValueRank > 1` out of scope. An - array node is writable only when the driver can write the array (`DriverArrayWriteSupport`: Modbus bounded - by `maxRegistersPerWrite`/coil cap, S7 non-Bool/non-wide types, OpcUaClient, Galaxy signed types **with a + array node is writable only when the driver can write the array (`DriverArrayWriteSupport`: Modbus keys-addressed tags bounded + by `maxRegistersPerWrite`/coil cap incl. String/BitInRegister arrays (#741; a string-only `addressString` tag + stays read-only — family unknown at compose time), S7 non-Bool/non-wide types, OpcUaClient, Galaxy signed types **with a declared `arrayLength`** — an undeclared length stays read-only because a short whole-array write would resize the Galaxy attribute; AbCip atomic non-BOOL types, AbLegacy Int/Long/Float on the matching data file (no `/bit`/`.sub`/indirect/`S:`/`I:`/`O:`, not native PLC-5, ≤ 256), TwinCAT every codec type but a BOOL diff --git a/docs/Uns.md b/docs/Uns.md index 8de7c783..e6c07878 100644 --- a/docs/Uns.md +++ b/docs/Uns.md @@ -323,7 +323,7 @@ carried on the plan records (`RawTagPlan.ArrayWritable`, mirrored onto every UNS | Driver | Array writes | Element types | |---|---|---| -| **Modbus** | Yes | `region: Coils` (FC15, any element, ≤ 1968 coils); `region: HoldingRegisters` with `Int16`/`UInt16`/`Bcd16`/`Int32`/`UInt32`/`Bcd32`/`Float32`/`Int64`/`UInt64`/`Float64` (FC16) whose `arrayLength × registers-per-element` fits `maxRegistersPerWrite` (default 123; the smallest value set on the driver or the tag's device wins) — the driver refuses rather than splits an over-cap FC16, so e.g. a 62-element `Int32` array (124 registers) stays read-only. Not BitInRegister / String / Bool-in-register; input regions are read-only | +| **Modbus** | Yes (String / BitInRegister arrays since #741) | Tags addressed by `region` + `address` keys (strict reads: an integer address 0–65535, enum strings that name a member). `region: Coils` (FC15, ≤ 1968 coils) with **any** `dataType` — a bit region is coerced to Bool, never refused. `region: HoldingRegisters` (FC16) with a fixed-width numeric type (`Int16`/`UInt16`/`Bcd16`/`Int32`/`UInt32`/`Bcd32`/`Float32`/`Int64`/`UInt64`/`Float64`), `String` (`(stringLength+1)/2` registers per element, `stringLength >= 1`) or `BitInRegister` (one register per element, read-modify-written in one FC03 + one FC16, `bitIndex` 0–15, span ending at or before register 65535), whose `arrayLength × registers-per-element` fits `maxRegistersPerWrite` (default 123; the smallest value set on the driver or the tag's device wins) — the driver refuses rather than splits an over-cap FC16, so e.g. a 62-element `Int32` array (124 registers) stays read-only. FC23 (opt-in) never changes the answer. Not Bool-in-register; input regions are read-only. A **string-only** tag (`addressString`, no keys) stays read-only: the driver resolves it with its own family, which the composer cannot know — use the editor's Resolve to pin the keys | | **S7** | Yes | `Byte`/`Int16`/`UInt16`/`Int32`/`UInt32`/`Float32` (one contiguous block). Not `Bool` (would clobber neighbouring bits) nor the wide / String / DateTime types | | **OpcUaClient** | Yes | All — forwarded upstream unchanged; the upstream server decides (shape included) | | **Galaxy** | Yes | OPC UA `Boolean`/`Int32`/`Int64`/`Float`/`Double`/`String`/`DateTime` (the `MxValueEncoder` array variants) — **only with a declared `arrayLength`**: an undeclared length materialises an unbounded node, and a short whole-array write would *resize* the Galaxy attribute (mxaccessgw contract), so such a node stays read-only | @@ -340,9 +340,10 @@ stays read-only. **Elements convert; they are not exact-matched.** ScadaBridge writes CLR `int[]` / `List` to Int16 nodes: the node manager converts a numeric array to the node's element type, and the AB CIP, AB Legacy and TwinCAT encoders convert each element again to the controller's type (an element that does not fit -is `BadOutOfRange`). Those rows therefore do not key on an exact node `DataType`; they leave out only -node types whose elements never convert — a `DateTime` or `String` node over a numeric element, and a -`Boolean` node on AB Legacy. An array tag deployed read-only before its driver gained an encoder turns +is `BadOutOfRange`), as does Modbus to the register / coil type. Those rows therefore do not key on an +exact node `DataType`; they leave out only node types whose elements never convert — a `DateTime` or +`String` node over a numeric element (on Modbus, over a numeric or bit element; a Modbus `String` +element takes any node type), and a `Boolean` node on AB Legacy. An array tag deployed read-only before its driver gained an encoder turns writable on the next deploy (a surgical `ArrayWritable` update, no rebuild). Each "Yes" row is pinned against the driver's real encoder by a parity test in that driver's diff --git a/docs/drivers/Modbus.md b/docs/drivers/Modbus.md index 024aaa78..0276404c 100644 --- a/docs/drivers/Modbus.md +++ b/docs/drivers/Modbus.md @@ -138,11 +138,15 @@ numeric, String, and BitInRegister array reads. **Live-verify** — Mac-verifiable against the simulator at `10.100.0.35:5020` (no fixture bring-up required; the sim is always-on on the shared Docker host). -**Array writes** (#610) — a whole-array write goes out as one FC15 (coils) or one FC16 (numeric -holding registers); the driver refuses rather than splits an FC16 longer than `maxRegistersPerWrite` -(default 123). The server mirrors that: an array tag is client-writable only when +**Array writes** (#610, #741) — a whole-array write goes out as one FC15 (coils) or one FC16 +(holding registers: numeric, `String` at `(stringLength+1)/2` registers per element, and +`BitInRegister` as one FC03 + one FC16 read-modify-write of the span); the driver refuses rather than +splits an FC16 longer than `maxRegistersPerWrite` (default 123). The server mirrors that: an array tag +is client-writable only when it is addressed by `region` + `address` keys and `arrayLength × registers-per-element` fits the cap (and a coil array ≤ 1968), else its node stays -read-only — see [Uns.md §Array writes](../Uns.md#array-writes-610), including the upgrade note. +read-only. A string-only (`addressString`) array stays read-only, because the composer cannot resolve +it under the driver's family — see [Uns.md §Array writes](../Uns.md#array-writes-610) and +[modbus-addressing.md §Array writes](../v2/modbus-addressing.md#array-writes-610-741). **Deferrals** — multi-dimensional arrays (`arrayLength` is always 1-D); per-element historization. diff --git a/docs/v2/modbus-addressing.md b/docs/v2/modbus-addressing.md index 74fbfe2c..2eb2e73e 100644 --- a/docs/v2/modbus-addressing.md +++ b/docs/v2/modbus-addressing.md @@ -89,6 +89,13 @@ For 8-byte values (Int64 / Float64) the same labels apply pairwise. `40001:F:5` = `Float32[5]` (consumes HR[0..9]). Array + bit suffix is rejected. Strings are not arrays. +An array written through the grammar string alone (a **string-only** tag — +`addressString` with no `region`/`address` keys) is served and written by the +driver, but its OPC UA node stays **read-only**: whether a write fits is +decided at compose time, where the driver's family is not known. Pin the keys +(the editor's "Resolve with driver family") to make the array writable — see +"Array writes" below. + ### Composition The 3-field shorthand `40001:F:5` is parsed as `(type=F, count=5)` because @@ -279,3 +286,31 @@ than asking users to rewrite spreadsheets. `BitInRegister`). - Exception 01/02/03 are device refusals: per-tag Bad, one Warning per tag, no driver Degraded; reads and writes share one comms-fault health slot. + +## Array writes (#610, #741) + +Whether an array node is client-writable is decided by +`DriverArrayWriteSupport` (Core.Abstractions) at compose time, and it says yes +only for what `ModbusDriver.WriteAsync` encodes end to end +(`ModbusArrayWriteSupportParityTests` pins the two against each other): + +- **Keys only.** The tag must carry `region` + `address` (read as strictly as + the driver reads them). A string-only tag stays read-only — the composer + cannot resolve the grammar under the driver's family. +- **Coils:** one FC15 of at most 1968 coils (the spec ceiling; the driver does + not chunk). Any `dataType` — a bit region is coerced to Bool. +- **HoldingRegisters:** one FC16 whose register count fits + `maxRegistersPerWrite` (default 123; the smallest value on the driver or the + tag's device wins). Registers per element: 1 / 2 / 4 for the numeric types, + `(stringLength+1)/2` for `String` (each element nul-padded to that width), 1 + for `BitInRegister` (the span is read with one FC03, the addressed bit of each + register is set or cleared, and the span is written back with one FC16 — + the other 15 bits are preserved; `bitIndex` 0–15, the span must end at or + before register 65535). FC23 (`useFC23ForMultiRegisterWrites`) is opt-in, + capped at 121 registers and falls back to FC16 above that, so it never + changes which arrays are writable. +- **Never writable:** DiscreteInputs / InputRegisters, Bool in a register + region, a `String` with no `stringLength`, a `bitIndex` above 15, a missing + or zero `arrayLength`, and a `String` / `DateTime` node over a numeric or bit + element (such an element never converts). Elements otherwise convert, so a + ScadaBridge `int[]` to an `Int16` node is writable. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs index 08b87af5..7b723ed2 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs @@ -20,11 +20,20 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// until this table follows. /// /// -/// Modbus — region: Coils (FC15, any element, at most 1968 coils) and -/// region: HoldingRegisters with a fixed-width numeric dataType (FC16) whose -/// arrayLength × registers-per-element fits the driver's maxRegistersPerWrite (default -/// 123 — the driver refuses rather than chunks a longer FC16). Bit-in-register, String and -/// Bool-in-register arrays have no array encoder; input regions are read-only. +/// Modbus — tags addressed by the region + address KEYS (the +/// ModbusTagAddressResolution rule: keys win whenever both are present), read as strictly as the +/// driver reads them (address an integer 0–65535; a present region / dataType / +/// byteOrder / stringByteOrder string must name a member). region: Coils: FC15, at most +/// 1968 coils, with ANY dataType — the resolver coerces a bit region to Bool and never refuses it. +/// region: HoldingRegisters (FC16), never Bool: a fixed-width numeric dataType, +/// String ((stringLength+1)/2 registers per element, stringLength >= 1) or +/// BitInRegister (one register per element, read-modify-written; bitIndex 0–15, the span +/// ending at or before register 65535), whose arrayLength × registers-per-element fits the +/// driver's maxRegistersPerWrite (default 123 — the driver refuses rather than chunks a longer +/// FC16). FC23 is opt-in, capped lower and falls back to FC16, so it never changes the answer. Input +/// regions are read-only. A string-only tag (an addressString with no keys) is resolved by the +/// driver with ITS family through a parser this assembly cannot reference, so it cannot be proven +/// writable here and stays read-only. /// S7 — dataType Byte / Int16 / UInt16 / Int32 / UInt32 / Float32 (one contiguous /// block, EncodeArrayBlock). Bool arrays would clobber neighbouring bits; the wide / String / /// DateTime element types have no array encoder. @@ -61,9 +70,11 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// Elements convert; they are not exact-matched. ScadaBridge writes CLR arrays such as /// int[] / List<int> to Int16 nodes; the node manager converts a numeric array to the /// node's element type, and the AbCip, AbLegacy and TwinCAT encoders then Convert each element to the -/// controller's type (an element that does not fit is BadOutOfRange). So those rows do not key on an -/// exact node DataType; they leave out only the node types whose elements NEVER convert: a -/// DateTime or String node over a numeric element (AbCip, AbLegacy, TwinCAT — TwinCAT keeps +/// controller's type (an element that does not fit is BadOutOfRange); Modbus Converts each element to +/// the register / coil type the same way. So those rows do not key on an exact node DataType; they leave +/// out only the node types whose elements NEVER convert: a DateTime or String node over a numeric +/// element (Modbus — over a numeric or bit element; a Modbus String element takes any node type — AbCip, +/// AbLegacy, TwinCAT — TwinCAT keeps /// DATE / DT and WSTRING under a DateTime node, and STRING / WSTRING under a String node; a DateTime's text /// form is culture-dependent, so a STRING under a DateTime node is left out), and a Boolean node on /// AbLegacy, whose encoder refuses bool elements. An unknown DataType name (served as BaseDataType) hands the @@ -89,6 +100,30 @@ public static class DriverArrayWriteSupport ["Int64"] = 4, ["UInt64"] = 4, ["Float64"] = 4, }; + // The Modbus enums' member names (ModbusRegion / ModbusDataType / ModbusByteOrder / ModbusStringByteOrder in + // Driver.Modbus.Addressing, which this assembly cannot reference). The parity tests enumerate the real enums, so a + // member added there without being added here fails a test. + private static readonly HashSet ModbusRegions = new(StringComparer.OrdinalIgnoreCase) + { + "Coils", "DiscreteInputs", "InputRegisters", "HoldingRegisters", + }; + + private static readonly HashSet ModbusDataTypes = new(StringComparer.OrdinalIgnoreCase) + { + "Bool", "Int16", "UInt16", "Int32", "UInt32", "Int64", "UInt64", "Float32", "Float64", + "BitInRegister", "String", "Bcd16", "Bcd32", + }; + + private static readonly HashSet ModbusByteOrders = new(StringComparer.OrdinalIgnoreCase) + { + "BigEndian", "WordSwap", "ByteSwap", "FullReverse", + }; + + private static readonly HashSet ModbusStringByteOrders = new(StringComparer.OrdinalIgnoreCase) + { + "HighByteFirst", "LowByteFirst", + }; + private static readonly HashSet S7ArrayTypes = new(StringComparer.OrdinalIgnoreCase) { "Byte", "Int16", "UInt16", "Int32", "UInt32", "Float32", @@ -148,22 +183,7 @@ public static class DriverArrayWriteSupport return dataType is not null && GalaxyArrayDataTypes.Contains(dataType) && TryParseObject(tagConfig, out var galaxyRoot) && ReadArrayLength(galaxyRoot) is not null; case DriverTypeNames.Modbus: - { - // Same reads as ModbusTagDefinitionFactory: a blob with no `region` or no numeric `address` is not - // a Modbus tag at all (the factory refuses it). A present non-string region is read as absent ⇒ - // HoldingRegisters, and an absent dataType ⇒ Int16 (TagConfigJson.TryReadEnumStrict). - if (!TryParseObject(tagConfig, out var root) - || !root.TryGetProperty("region", out _) - || !root.TryGetProperty("address", out var address) || address.ValueKind != JsonValueKind.Number - || ReadArrayLength(root) is not { } count) - return false; - var region = ReadEnumName(root, "region") ?? "HoldingRegisters"; - var native = ReadEnumName(root, "dataType") ?? "Int16"; - if (string.Equals(region, "Coils", StringComparison.OrdinalIgnoreCase)) return count <= ModbusMaxCoilsPerWrite; - return string.Equals(region, "HoldingRegisters", StringComparison.OrdinalIgnoreCase) - && ModbusRegisterArrayTypes.TryGetValue(native, out var registersPerElement) - && (long)count * registersPerElement <= ModbusMaxRegistersPerWrite(driverConfigs); - } + return ModbusSupports(dataType, tagConfig, driverConfigs); case DriverTypeNames.S7: { // S7TagDefinitionFactory requires a dataType key (absent ⇒ not an S7 tag at all); a present @@ -185,6 +205,76 @@ public static class DriverArrayWriteSupport } } + // ModbusTagAddressResolution + ModbusTagDefinitionFactory + ModbusDriver.WriteAsync, KEYS shape only: + // - `region` AND `address` both present (Classify == Keys). Anything else is a string-only (`addressString`) blob the + // driver resolves with ITS family through ModbusAddressParser, which this assembly cannot reference — so it is + // never proven writable here and stays read-only (conservative: a hidden write, never an advertised refusal). + // - the strict key reads: `address` an integer 0–65535; a present string `region` / `dataType` / `byteOrder` / + // `stringByteOrder` must be a member name (a non-string token is read as absent ⇒ the default). + // - Coils: the resolver COERCES any type to Bool (never refuses); one FC15 of at most 1968 coils. + // - HoldingRegisters: never Bool; fixed-width numeric, String ((stringLength+1)/2 registers, stringLength >= 1) or + // BitInRegister (one register each, bitIndex 0–15, span ending at or before 65535); arrayLength × registers per + // element within maxRegistersPerWrite (the driver refuses rather than chunks; FC23 is opt-in and capped lower, + // falling back to FC16, so it never changes the answer). + // - Elements CONVERT, so the node type is not exact-matched; a String / DateTime node over a numeric or bit element + // never converts and is left out. A String element Convert.ToString()s any node type. + private static bool ModbusSupports(string? dataType, string? tagConfig, string?[]? driverConfigs) + { + if (!TryParseObject(tagConfig, out var root) + || !root.TryGetProperty("region", out _) + || !root.TryGetProperty("address", out var addressEl) || addressEl.ValueKind != JsonValueKind.Number + || !addressEl.TryGetInt32(out var address) || address is < 0 or > ushort.MaxValue + || ReadArrayLength(root) is not { } count + || !TryReadModbusEnum(root, "region", ModbusRegions, out var region) + || !TryReadModbusEnum(root, "dataType", ModbusDataTypes, out var native) + || !TryReadModbusEnum(root, "byteOrder", ModbusByteOrders, out _) + || !TryReadModbusEnum(root, "stringByteOrder", ModbusStringByteOrders, out _)) + return false; + region ??= "HoldingRegisters"; + + if (region.Equals("Coils", StringComparison.OrdinalIgnoreCase)) + return count <= ModbusMaxCoilsPerWrite && dataType is not ("String" or "DateTime"); + if (!region.Equals("HoldingRegisters", StringComparison.OrdinalIgnoreCase)) return false; // input regions are read-only + + native ??= "Int16"; + int registersPerElement; + if (native.Equals("String", StringComparison.OrdinalIgnoreCase)) + { + // The factory narrows to ushort ((ushort)ReadInt) and refuses a zero length. + var stringLength = (ushort)ReadInt(root, "stringLength"); + if (stringLength < 1) return false; + registersPerElement = (stringLength + 1) / 2; + } + else + { + if (dataType is "String" or "DateTime") return false; + if (native.Equals("BitInRegister", StringComparison.OrdinalIgnoreCase)) + { + // The factory narrows to byte ((byte)ReadInt) and refuses a bit above 15; the array RMW refuses a span + // past the last register. + if ((byte)ReadInt(root, "bitIndex") > 15 || (long)address + count - 1 > ushort.MaxValue) return false; + registersPerElement = 1; + } + else if (!ModbusRegisterArrayTypes.TryGetValue(native, out registersPerElement)) + { + return false; // Bool in a register region: the factory refuses the pair + } + } + return (long)count * registersPerElement <= ModbusMaxRegistersPerWrite(driverConfigs); + } + + // TagConfigJson.TryReadEnumStrict over the Modbus enum's member names: absent or a non-string token ⇒ null (the + // caller applies the default); a string must name a member (trimmed, case-insensitive), else the tag is refused. + // Numeric spellings are not member names, so they are refused exactly as ClassifyEnumText refuses them. + private static bool TryReadModbusEnum(JsonElement root, string name, HashSet members, out string? value) + { + value = ReadEnumName(root, name)?.Trim(); + return value is null || members.Contains(value); + } + + private static int ReadInt(JsonElement root, string name) => + root.TryGetProperty(name, out var el) && el.ValueKind == JsonValueKind.Number && el.TryGetInt32(out var v) ? v : 0; + // AbCipTagDefinitionFactory + AbCipDriver.WriteAsync: a non-blank string tagPath, an atomic element type the codec // has a width for (BOOL arrays are bit-packed DWORDs, STRING / structure elements are structures), a declared // length, and not a safety tag (refused BadNotWritable). Each element is CONVERTED, so any numeric / Boolean node diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverArrayWriteSupportTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverArrayWriteSupportTests.cs index 18137306..f410c8bb 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverArrayWriteSupportTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverArrayWriteSupportTests.cs @@ -15,12 +15,27 @@ public sealed class DriverArrayWriteSupportTests [Theory] [InlineData("{\"region\":\"Coils\",\"address\":0,\"dataType\":\"Bool\"" + Arr2 + "}", true)] - [InlineData("{\"region\":\"Coils\",\"address\":0" + Arr2 + "}", true)] // FC15 ignores dataType + [InlineData("{\"region\":\"Coils\",\"address\":0" + Arr2 + "}", true)] // absent dataType ⇒ Bool + [InlineData("{\"region\":\"Coils\",\"address\":0,\"dataType\":\"Int16\"" + Arr2 + "}", true)] // a bit region COERCES any type to Bool + [InlineData("{\"region\":\"Coils\",\"address\":0,\"dataType\":\"Int17\"" + Arr2 + "}", false)] // ...but a typo refuses the tag [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"Float32\"" + Arr2 + "}", true)] [InlineData("{\"region\":\"holdingregisters\",\"address\":0,\"dataType\":\"bcd16\"" + Arr2 + "}", true)] // case-insensitive, like the driver [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0" + Arr2 + "}", true)] // dataType ⇒ Int16 - [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"BitInRegister\"" + Arr2 + "}", false)] - [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"String\"" + Arr2 + "}", false)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"BitInRegister\"" + Arr2 + "}", true)] // #741 U3 + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"BitInRegister\",\"bitIndex\":15" + Arr2 + "}", true)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"BitInRegister\",\"bitIndex\":16" + Arr2 + "}", false)] // factory refuses + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"String\",\"stringLength\":4" + Arr2 + "}", true)] // #741 U3 + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"String\"" + Arr2 + "}", false)] // no stringLength ⇒ refused + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"String\",\"stringLength\":0" + Arr2 + "}", false)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"Int16\",\"byteOrder\":\"Sideways\"" + Arr2 + "}", false)] // strict enum + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"String\",\"stringLength\":4,\"stringByteOrder\":\"Middle\"" + Arr2 + "}", false)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":65536,\"dataType\":\"Int16\"" + Arr2 + "}", false)] // not a register address + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":1.5,\"dataType\":\"Int16\"" + Arr2 + "}", false)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":65535,\"dataType\":\"BitInRegister\",\"isArray\":true,\"arrayLength\":1}", true)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":65535,\"dataType\":\"BitInRegister\"" + Arr2 + "}", false)] // span runs past 65535 + [InlineData("{\"addressString\":\"40001:S::2\"" + Arr2 + "}", false)] // string-only: family unknown at compose time + [InlineData("{\"region\":\"HoldingRegisters\",\"addressString\":\"40001:S::2\"" + Arr2 + "}", false)] // region alone is not the keys + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"addressString\":\"30001\",\"dataType\":\"Int16\"" + Arr2 + "}", true)] // keys win [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"Bool\"" + Arr2 + "}", false)] [InlineData("{\"region\":\"InputRegisters\",\"address\":0,\"dataType\":\"Int16\"" + Arr2 + "}", false)] // read-only region [InlineData("{\"region\":\"DiscreteInputs\",\"address\":0" + Arr2 + "}", false)] @@ -75,6 +90,39 @@ public sealed class DriverArrayWriteSupportTests "{\"region\":\"Coils\",\"address\":0,\"isArray\":true,\"arrayLength\":1969}").ShouldBeFalse(); } + /// #741 U3 — String[] and BitInRegister[] arrays ride the same FC16 cap: (stringLength+1)/2 registers + /// per String element, one per BitInRegister element. + [Theory] + [InlineData("\"dataType\":\"String\",\"stringLength\":4", 61, true)] // 122 registers + [InlineData("\"dataType\":\"String\",\"stringLength\":4", 62, false)] // 124 + [InlineData("\"dataType\":\"String\",\"stringLength\":5", 41, true)] // 3 registers each: 123 + [InlineData("\"dataType\":\"String\",\"stringLength\":5", 42, false)] // 126 + [InlineData("\"dataType\":\"BitInRegister\"", 123, true)] + [InlineData("\"dataType\":\"BitInRegister\"", 124, false)] + public void Modbus_string_and_bit_arrays_respect_the_write_cap(string typeKeys, int length, bool expected) => + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", + $"{{\"region\":\"HoldingRegisters\",\"address\":0,{typeKeys},\"isArray\":true,\"arrayLength\":{length}}}").ShouldBe(expected); + + /// Elements CONVERT (ScadaBridge's int[] to an Int16 node), so the Modbus row keys on no exact node + /// DataType; it leaves out only a String or DateTime node over a numeric / bit element, which never converts. A + /// String element takes any node type (each element is Convert.ToString'd). + [Theory] + [InlineData("Int16", "Int16", true)] + [InlineData("Int16", "Int32", true)] + [InlineData("Int16", "Double", true)] + [InlineData("Int16", "Boolean", true)] + [InlineData("Int16", "String", false)] + [InlineData("Int16", "DateTime", false)] + [InlineData("BitInRegister", "Boolean", true)] + [InlineData("BitInRegister", "String", false)] + [InlineData("String", "String", true)] + [InlineData("String", "Int16", true)] + [InlineData("String", "DateTime", true)] + [InlineData("String", null, true)] + public void Modbus_rows_leave_out_only_node_types_that_never_convert(string element, string? nodeType, bool expected) => + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, nodeType, + $"{{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"{element}\",\"stringLength\":4" + Arr2 + "}").ShouldBe(expected); + private static string RegisterArray(string dataType, int length) => $"{{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"{dataType}\",\"isArray\":true,\"arrayLength\":{length}}}"; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs index 41a20e50..df481e42 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs @@ -5,131 +5,359 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; /// -/// Gitea #610 — pins 's Modbus row against the REAL write path. For every -/// region × element type, the server advertises an array node writable exactly when the driver accepts a -/// whole-array write for it. A driver change that gains or loses an array encoder fails here until the table -/// follows — so the server can never advertise a write this driver refuses (nor hide one it accepts). +/// Gitea #610 / #741 — pins 's Modbus row against the REAL write path. For +/// every region × element type × node DataType, and for every shape edge (the FC16 / FC15 caps, the declared +/// length, the type-dependent guards, the address keys), the server advertises an array node writable exactly when +/// encodes the whole array onto the wire. A driver change that gains or loses +/// an array encoder fails here until the table follows — so the server can never advertise a write this driver +/// refuses (nor hide one it accepts). /// +/// +/// "Accepted" means Good status AND a write PDU (FC15 / FC16 / FC23) reached the transport. The transport is a +/// spec-conformant device: it holds the whole 0–65535 address space and answers an FC15 of more than 1968 coils +/// with exception 03, as every conformant device does — the driver itself sends one FC15 without chunking, so the +/// table's 1968 ceiling is the device's refusal, not the driver's. The FC16 cap is the driver's own +/// (MaxRegistersPerWrite). FC23 is opt-in and never changes the answer (pinned below). +/// [Trait("Category", "Unit")] public sealed class ModbusArrayWriteSupportParityTests { - private static IEnumerable<(ModbusRegion Region, ModbusDataType DataType)> Shapes() => - from region in Enum.GetValues() - from dt in Enum.GetValues() - select (region, dt); + private const string Ref = "cell/modbus/arr"; - public static TheoryData AllShapes() + /// The CLR array the node manager hands the driver for a node of this DataType (it converts a numeric + /// client array to the node's element type first). + private static Array ClrArray(string nodeType, int n) => nodeType switch { - var data = new TheoryData(); - foreach (var (region, dt) in Shapes()) data.Add(region, dt); - return data; - } + "Boolean" => Enumerable.Repeat(true, n).ToArray(), + "Int16" => Enumerable.Repeat((short)1, n).ToArray(), + "UInt16" => Enumerable.Repeat((ushort)1, n).ToArray(), + "Int32" => Enumerable.Repeat(1, n).ToArray(), + "Int64" => Enumerable.Repeat(1L, n).ToArray(), + "Float" => Enumerable.Repeat(1f, n).ToArray(), + "Double" => Enumerable.Repeat(1d, n).ToArray(), + // Non-numeric text: a String node over a numeric / bit element converts only when the text happens to parse, + // which is not a write the server can promise — the table keeps it read-only. + "String" => Enumerable.Repeat("a", n).ToArray(), + "DateTime" => Enumerable.Repeat(DateTime.UtcNow, n).ToArray(), + _ => throw new ArgumentOutOfRangeException(nameof(nodeType)), + }; - /// - /// Shapes where the driver has moved ahead of the server-side table (review 07 P6 / U3, Modbus lane) and - /// — owned outside the Modbus lane — has not followed yet - /// (follow-up F1): the tag factory now REFUSES a Coils tag with a non-Bool type (so the driver serves no - /// such tag), and the driver now encodes String[] and BitInRegister[] holding-register writes. - /// fails the moment the table follows, so this list - /// is removed with it rather than left to rot. - /// - private static bool PendingF1(ModbusRegion region, ModbusDataType dataType) => - (region == ModbusRegion.Coils && dataType != ModbusDataType.Bool) - || (region == ModbusRegion.HoldingRegisters && dataType is ModbusDataType.String or ModbusDataType.BitInRegister); + private static readonly string[] NodeTypes = ["Boolean", "Int16", "UInt16", "Int32", "Int64", "Float", "Double", "String", "DateTime"]; - - public static TheoryData PendingShapes() + /// Drives the real driver: true iff the write is Good AND a write PDU reached the device. + private static async Task<(bool Accepted, uint Status)> DriverEncodesAsync( + string tagConfig, object value, ushort maxRegistersPerWrite = 123, bool fc23 = false) { - var data = new TheoryData(); - foreach (var (region, dt) in Shapes().Where(s => PendingF1(s.Region, s.DataType))) data.Add(region, dt); - return data; - } - - [Theory] - [MemberData(nameof(PendingShapes))] - public void Pending_F1_divergences_still_exist(ModbusRegion region, ModbusDataType dataType) - { - var json = $$"""{"region":"{{region}}","address":10,"dataType":"{{dataType}}","stringLength":4,"isArray":true,"arrayLength":2}"""; - var driverAccepts = region == ModbusRegion.HoldingRegisters; // String[] / BitInRegister[] encode; mistyped Coils are refused - DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", json).ShouldNotBe(driverAccepts, - "F1 landed — DriverArrayWriteSupport now agrees with the driver for this shape; delete it from PendingF1"); - } - - [Theory] - [MemberData(nameof(AllShapes))] - public async Task Table_agrees_with_the_driver_write_path(ModbusRegion region, ModbusDataType dataType) - { - if (PendingF1(region, dataType)) return; // covered by Pending_F1_divergences_still_exist - var json = $$"""{"region":"{{region}}","address":10,"dataType":"{{dataType}}","stringLength":4,"isArray":true,"arrayLength":2}"""; - var fake = new ModbusDriverTests.FakeTransport(); + var device = new ConformantDevice(); var drv = new ModbusDriver( - new ModbusDriverOptions { Host = "fake", RawTags = [new RawTagEntry("cell/modbus/arr", json, false)] }, - "modbus-array-parity", _ => fake); + new ModbusDriverOptions + { + Host = "fake", RawTags = [new RawTagEntry(Ref, tagConfig, false)], + MaxRegistersPerWrite = maxRegistersPerWrite, UseFC23ForMultiRegisterWrites = fc23, + }, + "modbus-array-parity", _ => device); await drv.InitializeAsync("{}", CancellationToken.None); - - // A two-element value every element type can convert (Convert.ToBoolean / ToInt16 / ToSingle … of 1). - var result = await drv.WriteAsync([new WriteRequest("cell/modbus/arr", new object[] { 1, 1 })], CancellationToken.None); - var driverAccepts = result.Single().StatusCode == 0u; - - DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", json).ShouldBe( - driverAccepts, $"region={region} dataType={dataType}: driver status 0x{result.Single().StatusCode:X8}"); + var result = await drv.WriteAsync([new WriteRequest(Ref, value)], CancellationToken.None); + var status = result.Single().StatusCode; + return (status == 0u && device.WritePdus > 0, status); } - /// Positive control: the theory above is not vacuously "everything refused". + public static TheoryData TypeMatrix() + { + var data = new TheoryData(); + foreach (var region in Enum.GetValues()) + foreach (var dt in Enum.GetValues()) + foreach (var node in NodeTypes) + data.Add(region, dt, node); + return data; + } + + /// Every region × element type × node DataType: the table answers exactly what the write path does — + /// including String[] and BitInRegister[] holding-register arrays (#741 U3) and Coils authored with a non-Bool type, + /// which the resolver COERCES to Bool (never refuses). + [Theory] + [MemberData(nameof(TypeMatrix))] + public async Task Table_agrees_with_the_driver_write_path(ModbusRegion region, ModbusDataType dataType, string nodeType) + { + var json = $$"""{"region":"{{region}}","address":10,"dataType":"{{dataType}}","stringLength":4,"bitIndex":3,"isArray":true,"arrayLength":2}"""; + + var (accepted, status) = await DriverEncodesAsync(json, ClrArray(nodeType, 2)); + + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, nodeType, json).ShouldBe( + accepted, $"region={region} dataType={dataType} node={nodeType}: driver status 0x{status:X8}"); + } + + /// Positive control: the matrix above is not vacuously one answer, and it contains the shapes #741 added. [Fact] public void The_matrix_contains_both_answers() { - var answers = Shapes() - .Select(s => DriverArrayWriteSupport.Supports( + var answers = Enum.GetValues() + .SelectMany(r => Enum.GetValues().Select(d => DriverArrayWriteSupport.Supports( DriverTypeNames.Modbus, "Int16", - $$"""{"region":"{{s.Region}}","address":10,"dataType":"{{s.DataType}}","isArray":true,"arrayLength":2}""")) + $$"""{"region":"{{r}}","address":10,"dataType":"{{d}}","stringLength":4,"isArray":true,"arrayLength":2}"""))) .ToHashSet(); answers.ShouldBe(new[] { true, false }, ignoreOrder: true); } /// - /// #610 review MINOR-1/-2 — the table must also agree with the driver on the SHAPE edges: the FC16 write cap - /// (the driver refuses rather than chunks an over-cap array; default 123 registers, or the configured - /// MaxRegistersPerWrite) and a missing / zero arrayLength (the factory maps it to a scalar tag, - /// whose scalar encoder refuses an array value). + /// The shape edges, both directions, against the real write path: the FC16 cap for numeric, String and + /// BitInRegister arrays (default 123, or a configured MaxRegistersPerWrite), the FC15 ceiling, a + /// missing / zero arrayLength (a scalar to the factory, whose scalar encoder refuses an array value), + /// the String length and bit-index guards, the BitInRegister span end, and the strict key reads. /// [Theory] - [InlineData("Int32", "\"arrayLength\":61", 0, 61)] // 122 registers — fits the default cap - [InlineData("Int32", "\"arrayLength\":62", 0, 62)] // 124 registers — over it - [InlineData("Float64", "\"arrayLength\":31", 0, 31)] // 124 registers - [InlineData("Int16", "\"arrayLength\":10", 10, 10)] // exactly a configured cap - [InlineData("Int16", "\"arrayLength\":11", 10, 11)] // one over it - [InlineData("Int16", "\"arrayLength\":0", 0, 2)] // declared length 0 ⇒ scalar to the driver - [InlineData("Int16", "\"isArrayLengthMissing\":true", 0, 2)] - public async Task Table_agrees_with_the_driver_on_write_cap_and_declared_length( - string dataType, string lengthProp, ushort maxRegistersPerWrite, int elements) + // Numeric FC16 cap. + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int32","isArray":true,"arrayLength":61}""", 61, 0)] // 122 regs + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int32","isArray":true,"arrayLength":62}""", 62, 0)] // 124 regs + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":123}""", 123, 0)] // exactly 123 + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":124}""", 124, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Float64","isArray":true,"arrayLength":30}""", 30, 0)] // 120 regs + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Float64","isArray":true,"arrayLength":31}""", 31, 0)] // 124 regs + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":10}""", 10, 10)] // exactly a configured cap + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":11}""", 11, 10)] // one over it + // String FC16 cap: (stringLength+1)/2 registers per element. + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":4,"isArray":true,"arrayLength":61}""", 61, 0)] // 122 regs + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":4,"isArray":true,"arrayLength":62}""", 62, 0)] // 124 regs + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":5,"isArray":true,"arrayLength":41}""", 41, 0)] // 3 regs each: 123 + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":5,"isArray":true,"arrayLength":42}""", 42, 0)] // 126 + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":1,"isArray":true,"arrayLength":10}""", 10, 10)] // 1 reg each, at a configured cap + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":1,"isArray":true,"arrayLength":11}""", 11, 10)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":0,"isArray":true,"arrayLength":2}""", 2, 0)] // factory refuses + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","isArray":true,"arrayLength":2}""", 2, 0)] // no length ⇒ refused + // BitInRegister FC16 cap: one register per element, plus the bit-index guard and the span end. + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","bitIndex":15,"isArray":true,"arrayLength":123}""", 123, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","bitIndex":15,"isArray":true,"arrayLength":124}""", 124, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","isArray":true,"arrayLength":10}""", 10, 10)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","isArray":true,"arrayLength":11}""", 11, 10)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","bitIndex":16,"isArray":true,"arrayLength":2}""", 2, 0)] // factory refuses + [InlineData("""{"region":"HoldingRegisters","address":65530,"dataType":"BitInRegister","isArray":true,"arrayLength":6}""", 6, 0)] // ends at 65535 + [InlineData("""{"region":"HoldingRegisters","address":65530,"dataType":"BitInRegister","isArray":true,"arrayLength":7}""", 7, 0)] // past the end + // FC15 ceiling (one request, no chunking; a conformant device refuses more than 1968 coils). + [InlineData("""{"region":"Coils","address":0,"isArray":true,"arrayLength":1968}""", 1968, 0)] + [InlineData("""{"region":"Coils","address":0,"isArray":true,"arrayLength":1969}""", 1969, 0)] + [InlineData("""{"region":"Coils","address":0,"dataType":"Float64","isArray":true,"arrayLength":1968}""", 1968, 0)] // coerced to Bool + [InlineData("""{"region":"Coils","address":0,"dataType":"Float64","isArray":true,"arrayLength":1969}""", 1969, 0)] + // Declared length. + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":0}""", 2, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true}""", 2, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":1}""", 1, 0)] + // Strict key reads: a present-but-invalid enum or an out-of-range address refuses the tag. + [InlineData("""{"region":"Coils","address":0,"dataType":"Int17","isArray":true,"arrayLength":2}""", 2, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","byteOrder":"Sideways","isArray":true,"arrayLength":2}""", 2, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":4,"stringByteOrder":"Middle","isArray":true,"arrayLength":2}""", 2, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","byteOrder":"WordSwap","isArray":true,"arrayLength":2}""", 2, 0)] // valid order: positive control + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":4,"stringByteOrder":"LowByteFirst","isArray":true,"arrayLength":2}""", 2, 0)] + [InlineData("""{"region":"HoldingRegisters","address":65536,"dataType":"Int16","isArray":true,"arrayLength":2}""", 2, 0)] + [InlineData("""{"region":"HoldingRegisters","address":-1,"dataType":"Int16","isArray":true,"arrayLength":2}""", 2, 0)] + [InlineData("""{"region":"HoldingRegisters","address":1.5,"dataType":"Int16","isArray":true,"arrayLength":2}""", 2, 0)] + [InlineData("""{"region":"HoldingRegisters","address":"10","dataType":"Int16","isArray":true,"arrayLength":2}""", 2, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Bool","isArray":true,"arrayLength":2}""", 2, 0)] // register region never carries Bool + [InlineData("""{"region":0,"address":10,"dataType":"Int16","isArray":true,"arrayLength":2}""", 2, 0)] // non-string region ⇒ absent ⇒ HoldingRegisters + // Keys win over an addressString beside them (the driver never reads the string then). + [InlineData("""{"region":"HoldingRegisters","address":10,"addressString":"30011","dataType":"Int16","isArray":true,"arrayLength":2}""", 2, 0)] + public async Task Table_agrees_with_the_driver_on_each_shape_edge(string tagConfig, int elements, ushort maxRegistersPerWrite) { - var json = $$"""{"region":"HoldingRegisters","address":10,"dataType":"{{dataType}}","isArray":true,{{lengthProp}}}"""; var driverConfig = maxRegistersPerWrite == 0 ? "{}" : $$"""{"maxRegistersPerWrite":{{maxRegistersPerWrite}}}"""; - var fake = new ModbusDriverTests.FakeTransport(); - var options = new ModbusDriverOptions - { - Host = "fake", RawTags = [new RawTagEntry("cell/modbus/arr", json, false)], - MaxRegistersPerWrite = maxRegistersPerWrite == 0 ? (ushort)123 : maxRegistersPerWrite, - }; - var drv = new ModbusDriver(options, "modbus-array-cap-parity", _ => fake); - await drv.InitializeAsync("{}", CancellationToken.None); + var cap = maxRegistersPerWrite == 0 ? (ushort)123 : maxRegistersPerWrite; - var value = Enumerable.Repeat(1, elements).ToArray(); - var result = await drv.WriteAsync([new WriteRequest("cell/modbus/arr", value)], CancellationToken.None); - var driverAccepts = result.Single().StatusCode == 0u; + var (accepted, status) = await DriverEncodesAsync(tagConfig, ClrArray("Int16", elements), cap); - DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", json, driverConfig).ShouldBe( - driverAccepts, $"{dataType} {lengthProp} cap={maxRegistersPerWrite}: driver status 0x{result.Single().StatusCode:X8}"); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", tagConfig, driverConfig).ShouldBe( + accepted, $"{tagConfig} cap={cap}: driver status 0x{status:X8}"); } - /// Positive control for the theory above: both answers occur, so it is not vacuously all-refused. + /// The table restates the Modbus enums' member names (Core.Abstractions cannot reference them); every + /// byte order and string byte order the driver accepts must be one it accepts too, so a new member fails here. + [Fact] + public async Task Every_byte_order_member_agrees_with_the_driver() + { + foreach (var order in Enum.GetValues()) + foreach (var stringOrder in Enum.GetValues()) + { + var json = $$"""{"region":"HoldingRegisters","address":10,"dataType":"Int32","byteOrder":"{{order}}","stringByteOrder":"{{stringOrder}}","isArray":true,"arrayLength":2}"""; + var (accepted, _) = await DriverEncodesAsync(json, ClrArray("Int16", 2)); + accepted.ShouldBeTrue(json); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int32", json).ShouldBeTrue(json); + } + } + + /// Positive control for the edges: each cap boundary has a writable side and a refused side in the table. [Fact] public void The_shape_edges_contain_both_answers() { - DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int32", - """{"region":"HoldingRegisters","address":10,"dataType":"Int32","isArray":true,"arrayLength":61}""").ShouldBeTrue(); - DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int32", - """{"region":"HoldingRegisters","address":10,"dataType":"Int32","isArray":true,"arrayLength":62}""").ShouldBeFalse(); + static bool S(string json) => DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", json); + S("""{"region":"HoldingRegisters","address":10,"dataType":"Int32","isArray":true,"arrayLength":61}""").ShouldBeTrue(); + S("""{"region":"HoldingRegisters","address":10,"dataType":"Int32","isArray":true,"arrayLength":62}""").ShouldBeFalse(); + S("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":5,"isArray":true,"arrayLength":41}""").ShouldBeTrue(); + S("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":5,"isArray":true,"arrayLength":42}""").ShouldBeFalse(); + S("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","isArray":true,"arrayLength":123}""").ShouldBeTrue(); + S("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","isArray":true,"arrayLength":124}""").ShouldBeFalse(); + S("""{"region":"Coils","address":0,"isArray":true,"arrayLength":1968}""").ShouldBeTrue(); + S("""{"region":"Coils","address":0,"isArray":true,"arrayLength":1969}""").ShouldBeFalse(); + } + + /// + /// FC23 is opt-in and never changes writability: with UseFC23ForMultiRegisterWrites on, the driver + /// writes a multi-register array via FC23 up to its 121-register ceiling and falls back to FC16 above it, so + /// the accept boundary is still the FC16 cap the table uses. + /// + [Theory] + [InlineData("Int16", 121)] + [InlineData("Int16", 123)] + [InlineData("Int16", 124)] + [InlineData("String", 61)] + [InlineData("String", 62)] + public async Task FC23_does_not_change_the_answer(string dataType, int elements) + { + var json = $$"""{"region":"HoldingRegisters","address":10,"dataType":"{{dataType}}","stringLength":4,"isArray":true,"arrayLength":{{elements}}}"""; + + var (fc16, _) = await DriverEncodesAsync(json, ClrArray("Int16", elements), fc23: false); + var (fc23, _) = await DriverEncodesAsync(json, ClrArray("Int16", elements), fc23: true); + + fc23.ShouldBe(fc16); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", json).ShouldBe(fc23); + } + + /// + /// A string-only tag (addressString, no region + address keys) is resolved by the DRIVER + /// with its own family. Core.Abstractions cannot parse that grammar (it references no driver assembly), so the + /// table cannot prove such an array writable and keeps it read-only — even though the Generic-family driver does + /// write this one. That is the conservative direction (a hidden write, never an advertised refusal); a tag that + /// needs to be array-writable is authored with the keys (the editor's "Resolve" pins them). + /// + [Theory] + [InlineData("""{"addressString":"40011:S::2","isArray":true,"arrayLength":2}""")] + [InlineData("""{"addressString":"40011:F:2"}""")] + [InlineData("""{"region":"HoldingRegisters","addressString":"40011:S::2","isArray":true,"arrayLength":2}""")] // region alone is not the keys + public async Task A_string_only_array_tag_stays_read_only(string tagConfig) + { + var (accepted, status) = await DriverEncodesAsync(tagConfig, ClrArray("Int16", 2)); + + accepted.ShouldBeTrue($"the Generic driver resolves and writes it (status 0x{status:X8}) — this pins the conservative gap, not a refusal"); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", tagConfig).ShouldBeFalse(); + } + + /// + /// The ScadaBridge wire shape: a CLR int[] / List<int> to an Int16 holding-register array on + /// an Int16 node is advertised writable and CONVERTS element by element onto the wire (it is not exact-matched). + /// + [Fact] + public async Task A_ScadaBridge_int_list_to_an_Int16_array_encodes() + { + const string json = """{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":3}"""; + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", json).ShouldBeTrue(); + + foreach (object value in new object[] { new[] { 1, -2, 300 }, new List { 1, -2, 300 } }) + { + var device = new ConformantDevice(); + var drv = new ModbusDriver( + new ModbusDriverOptions { Host = "fake", RawTags = [new RawTagEntry(Ref, json, false)] }, "modbus-sb", _ => device); + await drv.InitializeAsync("{}", CancellationToken.None); + + (await drv.WriteAsync([new WriteRequest(Ref, value)], CancellationToken.None)).Single().StatusCode.ShouldBe(0u); + device.Holding[10..13].ShouldBe(new ushort[] { 1, 0xFFFE, 300 }); + } + } + + /// The two #741 U3 shapes encode end to end with their own node types: String[] on a String node and + /// BitInRegister[] on a Boolean node, each preserving what the wire must carry. + [Fact] + public async Task String_and_BitInRegister_arrays_encode_on_their_own_node_types() + { + const string strings = """{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":3,"isArray":true,"arrayLength":2}"""; + const string bits = """{"region":"HoldingRegisters","address":20,"dataType":"BitInRegister","bitIndex":2,"isArray":true,"arrayLength":2}"""; + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "String", strings).ShouldBeTrue(); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Boolean", bits).ShouldBeTrue(); + + var device = new ConformantDevice(); + device.Holding[20] = 0x0001; + device.Holding[21] = 0x0005; + var drv = new ModbusDriver( + new ModbusDriverOptions { Host = "fake", RawTags = [new RawTagEntry("s", strings, false), new RawTagEntry("b", bits, false)] }, + "modbus-u3", _ => device); + await drv.InitializeAsync("{}", CancellationToken.None); + + var r = await drv.WriteAsync( + [new WriteRequest("s", new[] { "ABC", "de" }), new WriteRequest("b", new[] { true, false })], CancellationToken.None); + + r.Select(x => x.StatusCode).ShouldBe(new[] { 0u, 0u }); + device.Holding[10..14].ShouldBe(new ushort[] { 0x4142, 0x4300, 0x6465, 0x0000 }); // 2 regs per 3-char element, nul-padded + device.Holding[20..22].ShouldBe(new ushort[] { 0x0005, 0x0001 }); // bit 2 set / cleared, other bits kept + } + + /// + /// A spec-conformant Modbus device over the whole 0–65535 address space. It refuses an FC15 of more than + /// 1968 coils with exception 03 (Illegal Data Value), as the spec requires; everything else it answers. + /// + private sealed class ConformantDevice : IModbusTransport + { + public readonly ushort[] Holding = new ushort[65536]; + public readonly bool[] Coils = new bool[65536]; + public int WritePdus; + + public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask; + + public Task SendAsync(byte unitId, byte[] pdu, CancellationToken ct) + { + var addr = (pdu[1] << 8) | pdu[2]; + var qty = (pdu[3] << 8) | pdu[4]; + switch (pdu[0]) + { + case 0x01: + case 0x02: + { + var resp = new byte[2 + (qty + 7) / 8]; + resp[0] = pdu[0]; resp[1] = (byte)((qty + 7) / 8); + for (var i = 0; i < qty; i++) if (Coils[addr + i]) resp[2 + i / 8] |= (byte)(1 << (i % 8)); + return Task.FromResult(resp); + } + case 0x03: + case 0x04: + return Task.FromResult(ReadRegs(pdu[0], addr, qty)); + case 0x05: + WritePdus++; + Coils[addr] = pdu[3] == 0xFF; + return Task.FromResult(pdu); + case 0x06: + WritePdus++; + Holding[addr] = (ushort)((pdu[3] << 8) | pdu[4]); + return Task.FromResult(pdu); + case 0x0F: + if (qty is < 1 or > 1968) + return Task.FromException(new ModbusException(0x0F, 0x03, "FC15 quantity outside 1..1968")); + WritePdus++; + for (var i = 0; i < qty; i++) Coils[addr + i] = ((pdu[6 + i / 8] >> (i % 8)) & 1) == 1; + return Task.FromResult(new byte[] { 0x0F, pdu[1], pdu[2], pdu[3], pdu[4] }); + case 0x10: + WritePdus++; + for (var i = 0; i < qty; i++) Holding[addr + i] = (ushort)((pdu[6 + i * 2] << 8) | pdu[7 + i * 2]); + return Task.FromResult(new byte[] { 0x10, pdu[1], pdu[2], pdu[3], pdu[4] }); + case 0x17: + { + WritePdus++; + var writeAddr = (pdu[5] << 8) | pdu[6]; + var writeQty = (pdu[7] << 8) | pdu[8]; + for (var i = 0; i < writeQty; i++) Holding[writeAddr + i] = (ushort)((pdu[10 + i * 2] << 8) | pdu[11 + i * 2]); + var resp = ReadRegs(0x17, addr, qty); + return Task.FromResult(resp); + } + default: + return Task.FromException(new ModbusException(pdu[0], 0x01, "unsupported")); + } + } + + private byte[] ReadRegs(byte fc, int addr, int qty) + { + var resp = new byte[2 + qty * 2]; + resp[0] = fc; resp[1] = (byte)(qty * 2); + for (var i = 0; i < qty; i++) + { + resp[2 + i * 2] = (byte)(Holding[addr + i] >> 8); + resp[3 + i * 2] = (byte)(Holding[addr + i] & 0xFF); + } + return resp; + } + + public ValueTask DisposeAsync() => ValueTask.CompletedTask; } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs index fbbd41ac..a7baea9a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs @@ -414,4 +414,64 @@ public sealed class AddressSpaceComposerDualNamespaceTests AddressSpaceApplier.UnsReferenceNodeWritable(c.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == id)).ShouldBeTrue(id); AddressSpaceApplier.UnsReferenceNodeWritable(c.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "r-bits")).ShouldBeFalse(); } + /// + /// #741 (Modbus lane U3, follow-up F1): the Modbus driver encodes String[] and BitInRegister[] holding-register + /// arrays, so an authored-ReadWrite array of each materialises WRITABLE — raw node and UNS mirror alike. A + /// Coils array authored with a non-Bool type (the resolver coerces a bit region to Bool) is writable too. Shapes + /// the driver refuses stay read-only on the SAME driver: a BitInRegister array over the FC16 cap, a String array + /// with no stringLength, and a string-only (addressString) array whose family the composer cannot + /// know — the negative controls. + /// + [Fact] + public void Modbus_string_and_bit_in_register_arrays_materialise_writable_and_refused_shapes_stay_read_only() + { + var folder = new RawFolder { RawFolderId = "f", ClusterId = "c1", Name = "Plant" }; + var mb = new DriverInstance { DriverInstanceId = "drv-mb", ClusterId = "c1", RawFolderId = "f", Name = "Mb", DriverType = "Modbus", DriverConfig = "{}" }; + var dev = new Device { DeviceId = "d1", DriverInstanceId = "drv-mb", Name = "plc", DeviceConfig = "{}" }; + static Tag Arr(string id, string dataType, string tagConfig) => new() + { + TagId = id, DeviceId = "d1", Name = id, DataType = dataType, AccessLevel = TagAccessLevel.ReadWrite, + TagConfig = tagConfig, + }; + var tags = new[] + { + Arr("names", "String", "{\"region\":\"HoldingRegisters\",\"address\":100,\"dataType\":\"String\",\"stringLength\":8,\"isArray\":true,\"arrayLength\":4}"), + Arr("flags", "Boolean", "{\"region\":\"HoldingRegisters\",\"address\":200,\"dataType\":\"BitInRegister\",\"bitIndex\":3,\"isArray\":true,\"arrayLength\":8}"), + Arr("legacyCoils", "Int16", "{\"region\":\"Coils\",\"address\":0,\"dataType\":\"Int16\",\"isArray\":true,\"arrayLength\":8}"), + Arr("overCap", "Boolean", "{\"region\":\"HoldingRegisters\",\"address\":300,\"dataType\":\"BitInRegister\",\"isArray\":true,\"arrayLength\":124}"), + Arr("noLength", "String", "{\"region\":\"HoldingRegisters\",\"address\":400,\"dataType\":\"String\",\"isArray\":true,\"arrayLength\":4}"), + Arr("stringOnly", "Int16", "{\"addressString\":\"40501:S::4\",\"isArray\":true,\"arrayLength\":4}"), + }; + var area = new UnsArea { UnsAreaId = "a", ClusterId = "c1", Name = "filling" }; + var line = new UnsLine { UnsLineId = "l", UnsAreaId = "a", Name = "line-1" }; + var equip = new Equipment { EquipmentId = "e", UnsLineId = "l", Name = "station-1", MachineCode = "S1" }; + var refs = new[] + { + new UnsTagReference { UnsTagReferenceId = "r-names", EquipmentId = "e", TagId = "names" }, + new UnsTagReference { UnsTagReferenceId = "r-flags", EquipmentId = "e", TagId = "flags" }, + new UnsTagReference { UnsTagReferenceId = "r-over", EquipmentId = "e", TagId = "overCap" }, + }; + + var c = AddressSpaceComposer.Compose( + new[] { area }, new[] { line }, new[] { equip }, new[] { mb }, Array.Empty(), + unsTagReferences: refs, tags: tags, rawFolders: new[] { folder }, devices: new[] { dev }); + + foreach (var writable in new[] { "names", "flags", "legacyCoils" }) + { + var plan = c.RawTags.Single(t => t.Name == writable); + plan.ArrayWritable.ShouldBeTrue(writable); + AddressSpaceApplier.RawNodeWritable(plan).ShouldBeTrue(writable); + } + foreach (var refused in new[] { "overCap", "noLength", "stringOnly" }) + { + var plan = c.RawTags.Single(t => t.Name == refused); + plan.IsArray.ShouldBeTrue(refused); // still an array node — just not a writable one + plan.ArrayWritable.ShouldBeFalse(refused); + AddressSpaceApplier.RawNodeWritable(plan).ShouldBeFalse(refused); + } + + foreach (var id in new[] { "r-names", "r-flags" }) + AddressSpaceApplier.UnsReferenceNodeWritable(c.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == id)).ShouldBeTrue(id); + AddressSpaceApplier.UnsReferenceNodeWritable(c.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "r-over")).ShouldBeFalse(); + } } From cf62007678e60a7cf84d2989bece6c64ee0e97a8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:32:57 -0400 Subject: [PATCH 2/3] fix(arrays): Modbus row honours the FC16/FC03 protocol ceilings, the RMW read cap and the address-space end Review r0 of Integration-W3: - I1: the effective FC16 cap is min(configured maxRegistersPerWrite, 123). The driver trusts a larger configured cap and sends an over-ceiling FC16 (past 127 registers its byte-count byte wraps), which every conformant device refuses, so such an array is never advertised writable. - I2: a BitInRegister[] write reads its span first in one unchunked FC03, so it is also bounded by min(configured maxRegistersPerRead, 125) (absent/0 = 125). - m2: every span (registers and coils) must end at or before address 65535. - m1: the String-node wording says it converts only when the text parses, which the server cannot promise (DateTime never converts). - m4: the parity device now enforces FC01/02 2000, FC03/04 125 (or a smaller device read limit), FC15 1968, FC16 123, FC23 121/125, the FC15/FC16 byte-count field (exception 03) and the address-space end (exception 02). Tests: parity rows for each ceiling/cap/span boundary against the real write path, and Core.Abstractions row tests. The configured-cap test that asserted a cap of 200 lifts the ceiling now asserts the opposite. Refs #741 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .claude/rules/data-plane-auth.md | 3 +- docs/Uns.md | 8 +- docs/drivers/Modbus.md | 4 +- docs/v2/modbus-addressing.md | 14 ++- .../DriverArrayWriteSupport.cs | 84 ++++++++++---- .../DriverArrayWriteSupportTests.cs | 46 +++++++- .../ModbusArrayWriteSupportParityTests.cs | 107 +++++++++++++++--- 7 files changed, 212 insertions(+), 54 deletions(-) diff --git a/.claude/rules/data-plane-auth.md b/.claude/rules/data-plane-auth.md index 0287d536..1125ca20 100644 --- a/.claude/rules/data-plane-auth.md +++ b/.claude/rules/data-plane-auth.md @@ -149,7 +149,8 @@ Two things to know before relying on any of this: a new sink wrapper must forward that argument, and the reflection guard checks only that the call arrives); `IndexRange` writes ⇒ `BadWriteNotSupported`, `ValueRank > 1` out of scope. An array node is writable only when the driver can write the array (`DriverArrayWriteSupport`: Modbus keys-addressed tags bounded - by `maxRegistersPerWrite`/coil cap incl. String/BitInRegister arrays (#741; a string-only `addressString` tag + by `maxRegistersPerWrite` (never above FC16's 123)/coil cap/address 65535 incl. String/BitInRegister arrays + (BitInRegister also by `maxRegistersPerRead`: its RMW is one unchunked FC03) (#741; a string-only `addressString` tag stays read-only — family unknown at compose time), S7 non-Bool/non-wide types, OpcUaClient, Galaxy signed types **with a declared `arrayLength`** — an undeclared length stays read-only because a short whole-array write would resize the Galaxy attribute; AbCip atomic non-BOOL types, AbLegacy Int/Long/Float on the matching data file diff --git a/docs/Uns.md b/docs/Uns.md index e6c07878..ce3dd85e 100644 --- a/docs/Uns.md +++ b/docs/Uns.md @@ -323,7 +323,7 @@ carried on the plan records (`RawTagPlan.ArrayWritable`, mirrored onto every UNS | Driver | Array writes | Element types | |---|---|---| -| **Modbus** | Yes (String / BitInRegister arrays since #741) | Tags addressed by `region` + `address` keys (strict reads: an integer address 0–65535, enum strings that name a member). `region: Coils` (FC15, ≤ 1968 coils) with **any** `dataType` — a bit region is coerced to Bool, never refused. `region: HoldingRegisters` (FC16) with a fixed-width numeric type (`Int16`/`UInt16`/`Bcd16`/`Int32`/`UInt32`/`Bcd32`/`Float32`/`Int64`/`UInt64`/`Float64`), `String` (`(stringLength+1)/2` registers per element, `stringLength >= 1`) or `BitInRegister` (one register per element, read-modify-written in one FC03 + one FC16, `bitIndex` 0–15, span ending at or before register 65535), whose `arrayLength × registers-per-element` fits `maxRegistersPerWrite` (default 123; the smallest value set on the driver or the tag's device wins) — the driver refuses rather than splits an over-cap FC16, so e.g. a 62-element `Int32` array (124 registers) stays read-only. FC23 (opt-in) never changes the answer. Not Bool-in-register; input regions are read-only. A **string-only** tag (`addressString`, no keys) stays read-only: the driver resolves it with its own family, which the composer cannot know — use the editor's Resolve to pin the keys | +| **Modbus** | Yes (String / BitInRegister arrays since #741) | Tags addressed by `region` + `address` keys (strict reads: an integer address 0–65535, enum strings that name a member). `region: Coils` (FC15, ≤ 1968 coils) with **any** `dataType` — a bit region is coerced to Bool, never refused. `region: HoldingRegisters` (FC16) with a fixed-width numeric type (`Int16`/`UInt16`/`Bcd16`/`Int32`/`UInt32`/`Bcd32`/`Float32`/`Int64`/`UInt64`/`Float64`), `String` (`(stringLength+1)/2` registers per element, `stringLength >= 1`) or `BitInRegister` (one register per element, read-modify-written in one FC03 + one FC16, `bitIndex` 0–15, and — because of that unchunked FC03 — also within `maxRegistersPerRead`, default and protocol ceiling 125), whose `arrayLength × registers-per-element` fits `maxRegistersPerWrite` (default 123; the smallest value set on the driver or the tag's device wins; a value above FC16's protocol ceiling of 123 never lifts it, since a conformant device refuses a longer FC16) — the driver refuses rather than splits an over-cap FC16, so e.g. a 62-element `Int32` array (124 registers) stays read-only. Every span (registers or coils) must end at or before address 65535. FC23 (opt-in) never changes the answer. Not Bool-in-register; input regions are read-only. A **string-only** tag (`addressString`, no keys) stays read-only: the driver resolves it with its own family, which the composer cannot know — use the editor's Resolve to pin the keys | | **S7** | Yes | `Byte`/`Int16`/`UInt16`/`Int32`/`UInt32`/`Float32` (one contiguous block). Not `Bool` (would clobber neighbouring bits) nor the wide / String / DateTime types | | **OpcUaClient** | Yes | All — forwarded upstream unchanged; the upstream server decides (shape included) | | **Galaxy** | Yes | OPC UA `Boolean`/`Int32`/`Int64`/`Float`/`Double`/`String`/`DateTime` (the `MxValueEncoder` array variants) — **only with a declared `arrayLength`**: an undeclared length materialises an unbounded node, and a short whole-array write would *resize* the Galaxy attribute (mxaccessgw contract), so such a node stays read-only | @@ -341,9 +341,9 @@ stays read-only. nodes: the node manager converts a numeric array to the node's element type, and the AB CIP, AB Legacy and TwinCAT encoders convert each element again to the controller's type (an element that does not fit is `BadOutOfRange`), as does Modbus to the register / coil type. Those rows therefore do not key on an -exact node `DataType`; they leave out only node types whose elements never convert — a `DateTime` or -`String` node over a numeric element (on Modbus, over a numeric or bit element; a Modbus `String` -element takes any node type), and a `Boolean` node on AB Legacy. An array tag deployed read-only before its driver gained an encoder turns +exact node `DataType`; they leave out only node types whose elements the server cannot promise to convert — a `DateTime` (never converts) or +`String` node over a numeric element (converts only when the text happens to parse, `"1"` / `"true"`; +on Modbus, over a numeric or bit element; a Modbus `String` element takes any node type), and a `Boolean` node on AB Legacy. An array tag deployed read-only before its driver gained an encoder turns writable on the next deploy (a surgical `ArrayWritable` update, no rebuild). Each "Yes" row is pinned against the driver's real encoder by a parity test in that driver's diff --git a/docs/drivers/Modbus.md b/docs/drivers/Modbus.md index 0276404c..03396c85 100644 --- a/docs/drivers/Modbus.md +++ b/docs/drivers/Modbus.md @@ -141,7 +141,9 @@ bring-up required; the sim is always-on on the shared Docker host). **Array writes** (#610, #741) — a whole-array write goes out as one FC15 (coils) or one FC16 (holding registers: numeric, `String` at `(stringLength+1)/2` registers per element, and `BitInRegister` as one FC03 + one FC16 read-modify-write of the span); the driver refuses rather than -splits an FC16 longer than `maxRegistersPerWrite` (default 123). The server mirrors that: an array tag +splits an FC16 longer than `maxRegistersPerWrite` (default 123; a larger value never lifts FC16's +protocol ceiling of 123, and a BitInRegister array's unchunked FC03 also bounds it by +`maxRegistersPerRead`). The server mirrors that: an array tag is client-writable only when it is addressed by `region` + `address` keys and `arrayLength × registers-per-element` fits the cap (and a coil array ≤ 1968), else its node stays read-only. A string-only (`addressString`) array stays read-only, because the composer cannot resolve diff --git a/docs/v2/modbus-addressing.md b/docs/v2/modbus-addressing.md index 2eb2e73e..abd367cf 100644 --- a/docs/v2/modbus-addressing.md +++ b/docs/v2/modbus-addressing.md @@ -301,16 +301,22 @@ only for what `ModbusDriver.WriteAsync` encodes end to end not chunk). Any `dataType` — a bit region is coerced to Bool. - **HoldingRegisters:** one FC16 whose register count fits `maxRegistersPerWrite` (default 123; the smallest value on the driver or the - tag's device wins). Registers per element: 1 / 2 / 4 for the numeric types, + tag's device wins), and never more than FC16's protocol ceiling of 123 — a + larger configured cap does not lift it, because a conformant device refuses + the longer FC16. Registers per element: 1 / 2 / 4 for the numeric types, `(stringLength+1)/2` for `String` (each element nul-padded to that width), 1 for `BitInRegister` (the span is read with one FC03, the addressed bit of each register is set or cleared, and the span is written back with one FC16 — - the other 15 bits are preserved; `bitIndex` 0–15, the span must end at or - before register 65535). FC23 (`useFC23ForMultiRegisterWrites`) is opt-in, + the other 15 bits are preserved; `bitIndex` 0–15). Because that FC03 is one + unchunked read of the whole span, a BitInRegister array must also fit + `maxRegistersPerRead` (default and protocol ceiling 125). +- **Every span** — registers or coils — must end at or before address 65535 + (a conformant device answers exception 02 past it). FC23 (`useFC23ForMultiRegisterWrites`) is opt-in, capped at 121 registers and falls back to FC16 above that, so it never changes which arrays are writable. - **Never writable:** DiscreteInputs / InputRegisters, Bool in a register region, a `String` with no `stringLength`, a `bitIndex` above 15, a missing or zero `arrayLength`, and a `String` / `DateTime` node over a numeric or bit - element (such an element never converts). Elements otherwise convert, so a + element (a DateTime never converts; a String converts only when its text + happens to parse, which the server cannot promise). Elements otherwise convert, so a ScadaBridge `int[]` to an `Int16` node is writable. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs index 7b723ed2..eca09158 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs @@ -27,10 +27,14 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// 1968 coils, with ANY dataType — the resolver coerces a bit region to Bool and never refuses it. /// region: HoldingRegisters (FC16), never Bool: a fixed-width numeric dataType, /// String ((stringLength+1)/2 registers per element, stringLength >= 1) or -/// BitInRegister (one register per element, read-modify-written; bitIndex 0–15, the span -/// ending at or before register 65535), whose arrayLength × registers-per-element fits the -/// driver's maxRegistersPerWrite (default 123 — the driver refuses rather than chunks a longer -/// FC16). FC23 is opt-in, capped lower and falls back to FC16, so it never changes the answer. Input +/// BitInRegister (one register per element; bitIndex 0–15), whose +/// arrayLength × registers-per-element fits the driver's maxRegistersPerWrite (default 123 — +/// the driver refuses rather than chunks a longer FC16) and never more than FC16's protocol ceiling of 123, +/// however high the cap is configured (a conformant device refuses a longer FC16, so that write always +/// fails). A BitInRegister array is read-modify-written — one unchunked FC03 of the span, then one FC16 — +/// so it is also bounded by maxRegistersPerRead (default and protocol ceiling 125). Every span +/// (registers or coils) ends at or before address 65535. FC23 is opt-in, capped lower and falls back to +/// FC16, so it never changes the answer. Input /// regions are read-only. A string-only tag (an addressString with no keys) is resolved by the /// driver with ITS family through a parser this assembly cannot reference, so it cannot be proven /// writable here and stays read-only. @@ -72,9 +76,10 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// node's element type, and the AbCip, AbLegacy and TwinCAT encoders then Convert each element to the /// controller's type (an element that does not fit is BadOutOfRange); Modbus Converts each element to /// the register / coil type the same way. So those rows do not key on an exact node DataType; they leave -/// out only the node types whose elements NEVER convert: a DateTime or String node over a numeric -/// element (Modbus — over a numeric or bit element; a Modbus String element takes any node type — AbCip, -/// AbLegacy, TwinCAT — TwinCAT keeps +/// out only the node types whose elements the server cannot promise to convert: a DateTime node over a +/// numeric element (it never converts) or a String node over one (it converts only when the text happens +/// to parse, "1" / "true") — on Modbus over a numeric or bit element, and a Modbus String element +/// takes any node type; likewise on AbCip, AbLegacy and TwinCAT — TwinCAT keeps /// DATE / DT and WSTRING under a DateTime node, and STRING / WSTRING under a String node; a DateTime's text /// form is culture-dependent, so a STRING under a DateTime node is left out), and a Boolean node on /// AbLegacy, whose encoder refuses bool elements. An unknown DataType name (served as BaseDataType) hands the @@ -85,13 +90,26 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; public static class DriverArrayWriteSupport { /// The Modbus driver's FC16 write cap when maxRegistersPerWrite is absent or 0 - /// (ModbusDriver resolves it the same way). + /// (ModbusDriver resolves it the same way). A configured value only ever LOWERS it — see + /// . public const int ModbusDefaultMaxRegistersPerWrite = 123; /// Modbus FC15's protocol ceiling on coils per request (0x07B0). The driver sends one FC15 for the /// whole array without chunking, so a longer coil array is refused by every conformant device. public const int ModbusMaxCoilsPerWrite = 1968; + /// Modbus FC16's protocol ceiling on registers per request (0x7B). A configured + /// maxRegistersPerWrite above it never lifts it: a conformant device refuses a longer FC16. + public const int ModbusProtocolMaxRegistersPerWrite = 123; + + /// Modbus FC03's protocol ceiling on registers per request (0x7D) — the bound on a BitInRegister[] + /// write's read-modify-write read, together with a configured maxRegistersPerRead. + public const int ModbusProtocolMaxRegistersPerRead = 125; + + /// The last Modbus address (registers and coils alike): a span ending past it is refused by every + /// conformant device (exception 02). + private const long ModbusLastAddress = ushort.MaxValue; + // Element type → registers per element (ModbusDriver.RegisterCount). private static readonly Dictionary ModbusRegisterArrayTypes = new(StringComparer.OrdinalIgnoreCase) { @@ -213,11 +231,14 @@ public static class DriverArrayWriteSupport // `stringByteOrder` must be a member name (a non-string token is read as absent ⇒ the default). // - Coils: the resolver COERCES any type to Bool (never refuses); one FC15 of at most 1968 coils. // - HoldingRegisters: never Bool; fixed-width numeric, String ((stringLength+1)/2 registers, stringLength >= 1) or - // BitInRegister (one register each, bitIndex 0–15, span ending at or before 65535); arrayLength × registers per - // element within maxRegistersPerWrite (the driver refuses rather than chunks; FC23 is opt-in and capped lower, - // falling back to FC16, so it never changes the answer). - // - Elements CONVERT, so the node type is not exact-matched; a String / DateTime node over a numeric or bit element - // never converts and is left out. A String element Convert.ToString()s any node type. + // BitInRegister (one register each, bitIndex 0–15); arrayLength × registers per element within + // maxRegistersPerWrite clamped to FC16's 123 (the driver refuses rather than chunks; FC23 is opt-in and capped + // lower, falling back to FC16, so it never changes the answer). BitInRegister[] also READS its span first in one + // unchunked FC03, so it is bounded by maxRegistersPerRead (clamped to FC03's 125) too. + // - Every span ends at or before address 65535 (a conformant device refuses one past it with exception 02). + // - Elements CONVERT, so the node type is not exact-matched. A DateTime node over a numeric or bit element never + // converts; a String node over one converts only when the text happens to parse ("1", "true"), which the server + // cannot promise — both are left out. A String element Convert.ToString()s any node type. private static bool ModbusSupports(string? dataType, string? tagConfig, string?[]? driverConfigs) { if (!TryParseObject(tagConfig, out var root) @@ -233,7 +254,8 @@ public static class DriverArrayWriteSupport region ??= "HoldingRegisters"; if (region.Equals("Coils", StringComparison.OrdinalIgnoreCase)) - return count <= ModbusMaxCoilsPerWrite && dataType is not ("String" or "DateTime"); + return count <= ModbusMaxCoilsPerWrite && address + (long)count - 1 <= ModbusLastAddress + && dataType is not ("String" or "DateTime"); if (!region.Equals("HoldingRegisters", StringComparison.OrdinalIgnoreCase)) return false; // input regions are read-only native ??= "Int16"; @@ -250,9 +272,9 @@ public static class DriverArrayWriteSupport if (dataType is "String" or "DateTime") return false; if (native.Equals("BitInRegister", StringComparison.OrdinalIgnoreCase)) { - // The factory narrows to byte ((byte)ReadInt) and refuses a bit above 15; the array RMW refuses a span - // past the last register. - if ((byte)ReadInt(root, "bitIndex") > 15 || (long)address + count - 1 > ushort.MaxValue) return false; + // The factory narrows to byte ((byte)ReadInt) and refuses a bit above 15. The RMW reads the whole span + // in ONE unchunked FC03 before writing it, so the read cap bounds it too. + if ((byte)ReadInt(root, "bitIndex") > 15 || count > ModbusMaxRegistersPerRead(driverConfigs)) return false; registersPerElement = 1; } else if (!ModbusRegisterArrayTypes.TryGetValue(native, out registersPerElement)) @@ -260,7 +282,8 @@ public static class DriverArrayWriteSupport return false; // Bool in a register region: the factory refuses the pair } } - return (long)count * registersPerElement <= ModbusMaxRegistersPerWrite(driverConfigs); + var registers = (long)count * registersPerElement; + return registers <= ModbusMaxRegistersPerWrite(driverConfigs) && address + registers - 1 <= ModbusLastAddress; } // TagConfigJson.TryReadEnumStrict over the Modbus enum's member names: absent or a non-string token ⇒ null (the @@ -428,9 +451,24 @@ public static class DriverArrayWriteSupport public static bool RefusesArrayValueOnScalarTag(string? driverType) => driverType is DriverTypeNames.Modbus or DriverTypeNames.S7; - // The smallest Modbus write cap configured in any config; none configured (absent / non-positive) ⇒ the driver - // default (123). The key is matched case-insensitively, like the driver's DTO binding. - private static int ModbusMaxRegistersPerWrite(string?[]? driverConfigs) + // The effective Modbus FC16 write cap: the smallest maxRegistersPerWrite configured in any config (absent / + // non-positive ⇒ the driver default, 123), CLAMPED to FC16's protocol ceiling of 123. The driver trusts a larger + // configured cap and sends the over-ceiling FC16 (past 127 registers its one-byte byte-count field even wraps), which + // every conformant device refuses — so such a write always fails and is never advertised. + private static int ModbusMaxRegistersPerWrite(string?[]? driverConfigs) => + Math.Min(SmallestPositive(driverConfigs, "maxRegistersPerWrite") ?? ModbusDefaultMaxRegistersPerWrite, + ModbusProtocolMaxRegistersPerWrite); + + // The effective Modbus FC03 read cap: the smallest maxRegistersPerRead configured (absent / non-positive ⇒ the driver + // default, 125), clamped to FC03's protocol ceiling of 125. Only the BitInRegister[] write reads (its RMW issues one + // unchunked FC03 of the whole span), so only that shape is bounded by it. + private static int ModbusMaxRegistersPerRead(string?[]? driverConfigs) => + Math.Min(SmallestPositive(driverConfigs, "maxRegistersPerRead") ?? ModbusProtocolMaxRegistersPerRead, + ModbusProtocolMaxRegistersPerRead); + + // The smallest positive Int32 value of `key` (matched case-insensitively, like the driver's DTO binding) at the root + // of any config; null when none carries one. + private static int? SmallestPositive(string?[]? driverConfigs, string key) { int? cap = null; foreach (var config in driverConfigs ?? []) @@ -438,7 +476,7 @@ public static class DriverArrayWriteSupport if (!TryParseObject(config, out var root)) continue; foreach (var prop in root.EnumerateObject()) { - if (string.Equals(prop.Name, "maxRegistersPerWrite", StringComparison.OrdinalIgnoreCase) + if (string.Equals(prop.Name, key, StringComparison.OrdinalIgnoreCase) && prop.Value.ValueKind == JsonValueKind.Number && prop.Value.TryGetInt32(out var configured) && configured > 0) { @@ -446,7 +484,7 @@ public static class DriverArrayWriteSupport } } } - return cap ?? ModbusDefaultMaxRegistersPerWrite; + return cap; } // The declared element count: isArray is the JSON literal true AND arrayLength is an Int32 >= 1 — the rule the diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverArrayWriteSupportTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverArrayWriteSupportTests.cs index f410c8bb..8e6c355e 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverArrayWriteSupportTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverArrayWriteSupportTests.cs @@ -74,11 +74,50 @@ public sealed class DriverArrayWriteSupportTests DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", tag, "{\"maxRegistersPerWrite\":100}", "{\"maxRegistersPerWrite\":9}") .ShouldBeFalse(); DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", tag, "{\"maxRegistersPerWrite\":0}", null, "not json").ShouldBeTrue(); - // A larger configured cap lifts the default: 124 registers fit under 200. - DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", RegisterArray("Int16", 124), "{\"maxRegistersPerWrite\":200}") + // A configured cap above the FC16 protocol ceiling (123) never lifts it: a conformant device refuses 124 + // registers (and past 127 the driver's one-byte byte-count field wraps), so the write always fails. + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", RegisterArray("Int16", 123), "{\"maxRegistersPerWrite\":200}") .ShouldBeTrue(); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", RegisterArray("Int16", 124), "{\"maxRegistersPerWrite\":200}") + .ShouldBeFalse(); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", RegisterArray("BitInRegister", 124), "{\"maxRegistersPerWrite\":200}") + .ShouldBeFalse(); } + /// A BitInRegister[] write is a read-modify-write: one unchunked FC03 of the whole span first, so its + /// length is also bounded by maxRegistersPerRead (smallest positive value across the configs; absent / 0 => + /// 125, and never above FC03's protocol ceiling of 125). The other register shapes never read on write. + [Fact] + public void Modbus_bit_in_register_arrays_respect_the_read_cap() + { + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Boolean", RegisterArray("BitInRegister", 64), "{\"maxRegistersPerRead\":64}").ShouldBeTrue(); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Boolean", RegisterArray("BitInRegister", 65), "{\"maxRegistersPerRead\":64}").ShouldBeFalse(); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Boolean", RegisterArray("BitInRegister", 65), "{\"maxRegistersPerRead\":100}", "{\"MaxRegistersPerRead\":64}") + .ShouldBeFalse(); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Boolean", RegisterArray("BitInRegister", 123), "{\"maxRegistersPerRead\":0}").ShouldBeTrue(); + // A numeric array of the same length is not read on write, so the read cap does not apply to it. + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", RegisterArray("Int16", 65), "{\"maxRegistersPerRead\":64}").ShouldBeTrue(); + // With the write cap lifted, the read cap is the bound. + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Boolean", RegisterArray("BitInRegister", 100), + "{\"maxRegistersPerRead\":100,\"maxRegistersPerWrite\":200}").ShouldBeTrue(); + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Boolean", RegisterArray("BitInRegister", 101), + "{\"maxRegistersPerRead\":100,\"maxRegistersPerWrite\":200}").ShouldBeFalse(); + } + + /// Every shape's span must end at or before address 65535 - a conformant device refuses a write past it + /// (exception 02). + [Theory] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":65530,\"dataType\":\"Int16\",\"isArray\":true,\"arrayLength\":6}", true)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":65530,\"dataType\":\"Int16\",\"isArray\":true,\"arrayLength\":7}", false)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":65532,\"dataType\":\"Int32\",\"isArray\":true,\"arrayLength\":2}", true)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":65533,\"dataType\":\"Int32\",\"isArray\":true,\"arrayLength\":2}", false)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":65534,\"dataType\":\"String\",\"stringLength\":4,\"isArray\":true,\"arrayLength\":1}", true)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":65535,\"dataType\":\"String\",\"stringLength\":4,\"isArray\":true,\"arrayLength\":1}", false)] + [InlineData("{\"region\":\"Coils\",\"address\":65000,\"isArray\":true,\"arrayLength\":536}", true)] + [InlineData("{\"region\":\"Coils\",\"address\":65000,\"isArray\":true,\"arrayLength\":537}", false)] + public void Modbus_array_spans_end_within_the_address_space(string tagConfig, bool expected) => + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", tagConfig).ShouldBe(expected); + /// FC15 carries at most 1968 coils and the driver sends one request, so a longer coil array is not /// writable. [Fact] @@ -104,7 +143,8 @@ public sealed class DriverArrayWriteSupportTests $"{{\"region\":\"HoldingRegisters\",\"address\":0,{typeKeys},\"isArray\":true,\"arrayLength\":{length}}}").ShouldBe(expected); /// Elements CONVERT (ScadaBridge's int[] to an Int16 node), so the Modbus row keys on no exact node - /// DataType; it leaves out only a String or DateTime node over a numeric / bit element, which never converts. A + /// DataType; it leaves out only a String or DateTime node over a numeric / bit element (a DateTime never converts; a String converts only when its text happens to + /// parse, which the server cannot promise). A /// String element takes any node type (each element is Convert.ToString'd). [Theory] [InlineData("Int16", "Int16", true)] diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs index df481e42..cca64cf5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs @@ -13,11 +13,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; /// refuses (nor hide one it accepts). /// /// -/// "Accepted" means Good status AND a write PDU (FC15 / FC16 / FC23) reached the transport. The transport is a -/// spec-conformant device: it holds the whole 0–65535 address space and answers an FC15 of more than 1968 coils -/// with exception 03, as every conformant device does — the driver itself sends one FC15 without chunking, so the -/// table's 1968 ceiling is the device's refusal, not the driver's. The FC16 cap is the driver's own -/// (MaxRegistersPerWrite). FC23 is opt-in and never changes the answer (pinned below). +/// "Accepted" means Good status AND a write PDU (FC15 / FC16 / FC23) the device took. The transport is a +/// spec-conformant device: it holds the whole 0–65535 address space and refuses what every conformant device +/// refuses — a quantity over a protocol ceiling (FC15 1968 coils, FC16 123 and FC03 125 registers) or a span past +/// address 65535. The driver sends one FC15 / FC16 (and, for BitInRegister[], one FC03) without chunking, so those +/// ceilings are the device's refusal; a configured MaxRegistersPerWrite below 123 is the driver's own. FC23 +/// is opt-in and never changes the answer (pinned below). /// [Trait("Category", "Unit")] public sealed class ModbusArrayWriteSupportParityTests @@ -46,14 +47,17 @@ public sealed class ModbusArrayWriteSupportParityTests /// Drives the real driver: true iff the write is Good AND a write PDU reached the device. private static async Task<(bool Accepted, uint Status)> DriverEncodesAsync( - string tagConfig, object value, ushort maxRegistersPerWrite = 123, bool fc23 = false) + string tagConfig, object value, ushort maxRegistersPerWrite = 123, bool fc23 = false, ushort maxRegistersPerRead = 125) { - var device = new ConformantDevice(); + // The operator configures maxRegistersPerRead because the DEVICE reads no more than that per FC03, so the + // modelled device refuses a longer read (never above FC03's protocol ceiling of 125). + var device = new ConformantDevice { MaxReadRegisters = Math.Min(maxRegistersPerRead == 0 ? 125 : (int)maxRegistersPerRead, 125) }; var drv = new ModbusDriver( new ModbusDriverOptions { Host = "fake", RawTags = [new RawTagEntry(Ref, tagConfig, false)], MaxRegistersPerWrite = maxRegistersPerWrite, UseFC23ForMultiRegisterWrites = fc23, + MaxRegistersPerRead = maxRegistersPerRead, }, "modbus-array-parity", _ => device); await drv.InitializeAsync("{}", CancellationToken.None); @@ -181,6 +185,55 @@ public sealed class ModbusArrayWriteSupportParityTests } } + /// + /// The protocol ceilings and the configured caps together, against the real write path: + /// + /// a maxRegistersPerWrite above FC16's 123 never lifts it - the driver sends the over-ceiling + /// FC16 and a conformant device refuses it (past 127 the byte-count field even wraps); + /// a BitInRegister[] write reads its span first in ONE unchunked FC03, so it is also bounded by + /// maxRegistersPerRead (and FC03's 125); numeric arrays never read on write; + /// every span ends at or before address 65535 (exception 02 past it). + /// + /// + [Theory] + // Write cap above the FC16 ceiling. + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":123}""", 123, 200, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":124}""", 124, 200, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":150}""", 150, 200, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"String","stringLength":4,"isArray":true,"arrayLength":62}""", 62, 200, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","isArray":true,"arrayLength":123}""", 123, 200, 0)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","isArray":true,"arrayLength":124}""", 124, 200, 200)] + // BitInRegister[] RMW read bounded by maxRegistersPerRead. + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","isArray":true,"arrayLength":64}""", 64, 0, 64)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","isArray":true,"arrayLength":65}""", 65, 0, 64)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"BitInRegister","isArray":true,"arrayLength":100}""", 100, 0, 64)] + [InlineData("""{"region":"HoldingRegisters","address":10,"dataType":"Int16","isArray":true,"arrayLength":100}""", 100, 0, 64)] // no read on write + // Span end at 65535. + [InlineData("""{"region":"HoldingRegisters","address":65530,"dataType":"Int16","isArray":true,"arrayLength":6}""", 6, 0, 0)] + [InlineData("""{"region":"HoldingRegisters","address":65530,"dataType":"Int16","isArray":true,"arrayLength":7}""", 7, 0, 0)] + [InlineData("""{"region":"HoldingRegisters","address":65532,"dataType":"Int32","isArray":true,"arrayLength":2}""", 2, 0, 0)] + [InlineData("""{"region":"HoldingRegisters","address":65533,"dataType":"Int32","isArray":true,"arrayLength":2}""", 2, 0, 0)] + [InlineData("""{"region":"HoldingRegisters","address":65534,"dataType":"String","stringLength":4,"isArray":true,"arrayLength":1}""", 1, 0, 0)] + [InlineData("""{"region":"HoldingRegisters","address":65535,"dataType":"String","stringLength":4,"isArray":true,"arrayLength":1}""", 1, 0, 0)] + [InlineData("""{"region":"HoldingRegisters","address":65530,"dataType":"BitInRegister","isArray":true,"arrayLength":6}""", 6, 0, 0)] + [InlineData("""{"region":"HoldingRegisters","address":65530,"dataType":"BitInRegister","isArray":true,"arrayLength":7}""", 7, 0, 0)] + [InlineData("""{"region":"Coils","address":65000,"isArray":true,"arrayLength":536}""", 536, 0, 0)] + [InlineData("""{"region":"Coils","address":65000,"isArray":true,"arrayLength":537}""", 537, 0, 0)] + public async Task Table_agrees_with_the_driver_on_protocol_ceilings_and_spans( + string tagConfig, int elements, ushort maxRegistersPerWrite, ushort maxRegistersPerRead) + { + var configs = new List(); + if (maxRegistersPerWrite != 0) configs.Add($$"""{"maxRegistersPerWrite":{{maxRegistersPerWrite}}}"""); + if (maxRegistersPerRead != 0) configs.Add($$"""{"maxRegistersPerRead":{{maxRegistersPerRead}}}"""); + + var (accepted, status) = await DriverEncodesAsync(tagConfig, ClrArray("Int16", elements), + maxRegistersPerWrite == 0 ? (ushort)123 : maxRegistersPerWrite, + maxRegistersPerRead: maxRegistersPerRead == 0 ? (ushort)125 : maxRegistersPerRead); + + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", tagConfig, configs.ToArray()).ShouldBe( + accepted, $"{tagConfig} write={maxRegistersPerWrite} read={maxRegistersPerRead}: driver status 0x{status:X8}"); + } + /// Positive control for the edges: each cap boundary has a writable side and a refused side in the table. [Fact] public void The_shape_edges_contain_both_answers() @@ -286,8 +339,10 @@ public sealed class ModbusArrayWriteSupportParityTests } /// - /// A spec-conformant Modbus device over the whole 0–65535 address space. It refuses an FC15 of more than - /// 1968 coils with exception 03 (Illegal Data Value), as the spec requires; everything else it answers. + /// A spec-conformant Modbus device over the whole 0-65535 address space. It enforces the protocol quantity + /// ceilings (FC01/02 <= 2000, FC03/04 <= 125, FC15 <= 1968, FC16 <= 123, FC23 write <= 121 / read <= 125) and the + /// FC15 / FC16 byte-count field with exception 03 (Illegal Data Value), and a span past address 65535 with + /// exception 02 (Illegal Data Address) - what a real device answers. Everything else it answers. /// private sealed class ConformantDevice : IModbusTransport { @@ -295,25 +350,38 @@ public sealed class ModbusArrayWriteSupportParityTests public readonly bool[] Coils = new bool[65536]; public int WritePdus; + /// The device's FC03/FC04 quantity limit: the protocol's 125, or a smaller device-specific one. + public int MaxReadRegisters { get; init; } = 125; + public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask; + private static Task Refuse(byte fc, byte code) => + Task.FromException(new ModbusException(fc, code, $"fc={fc:X2} exception {code:X2}")); + + // Exception 03 for a quantity outside 1..max, 02 for a span past the last address, else null (accepted). + private static byte? Check(int addr, int qty, int max) => + qty < 1 || qty > max ? (byte)0x03 : addr + qty > 65536 ? (byte)0x02 : null; + public Task SendAsync(byte unitId, byte[] pdu, CancellationToken ct) { + var fc = pdu[0]; var addr = (pdu[1] << 8) | pdu[2]; var qty = (pdu[3] << 8) | pdu[4]; - switch (pdu[0]) + switch (fc) { case 0x01: case 0x02: { + if (Check(addr, qty, 2000) is { } e) return Refuse(fc, e); var resp = new byte[2 + (qty + 7) / 8]; - resp[0] = pdu[0]; resp[1] = (byte)((qty + 7) / 8); + resp[0] = fc; resp[1] = (byte)((qty + 7) / 8); for (var i = 0; i < qty; i++) if (Coils[addr + i]) resp[2 + i / 8] |= (byte)(1 << (i % 8)); return Task.FromResult(resp); } case 0x03: case 0x04: - return Task.FromResult(ReadRegs(pdu[0], addr, qty)); + if (Check(addr, qty, MaxReadRegisters) is { } re) return Refuse(fc, re); + return Task.FromResult(ReadRegs(fc, addr, qty)); case 0x05: WritePdus++; Coils[addr] = pdu[3] == 0xFF; @@ -323,26 +391,29 @@ public sealed class ModbusArrayWriteSupportParityTests Holding[addr] = (ushort)((pdu[3] << 8) | pdu[4]); return Task.FromResult(pdu); case 0x0F: - if (qty is < 1 or > 1968) - return Task.FromException(new ModbusException(0x0F, 0x03, "FC15 quantity outside 1..1968")); + if (Check(addr, qty, 1968) is { } ce) return Refuse(fc, ce); + if (pdu[5] != (qty + 7) / 8 || pdu.Length < 6 + pdu[5]) return Refuse(fc, 0x03); WritePdus++; for (var i = 0; i < qty; i++) Coils[addr + i] = ((pdu[6 + i / 8] >> (i % 8)) & 1) == 1; return Task.FromResult(new byte[] { 0x0F, pdu[1], pdu[2], pdu[3], pdu[4] }); case 0x10: + if (Check(addr, qty, 123) is { } we) return Refuse(fc, we); + if (pdu[5] != qty * 2 || pdu.Length < 6 + pdu[5]) return Refuse(fc, 0x03); WritePdus++; for (var i = 0; i < qty; i++) Holding[addr + i] = (ushort)((pdu[6 + i * 2] << 8) | pdu[7 + i * 2]); return Task.FromResult(new byte[] { 0x10, pdu[1], pdu[2], pdu[3], pdu[4] }); case 0x17: { - WritePdus++; var writeAddr = (pdu[5] << 8) | pdu[6]; var writeQty = (pdu[7] << 8) | pdu[8]; + if (Check(addr, qty, 125) is { } r23) return Refuse(fc, r23); + if (Check(writeAddr, writeQty, 121) is { } w23) return Refuse(fc, w23); + WritePdus++; for (var i = 0; i < writeQty; i++) Holding[writeAddr + i] = (ushort)((pdu[10 + i * 2] << 8) | pdu[11 + i * 2]); - var resp = ReadRegs(0x17, addr, qty); - return Task.FromResult(resp); + return Task.FromResult(ReadRegs(0x17, addr, qty)); } default: - return Task.FromException(new ModbusException(pdu[0], 0x01, "unsupported")); + return Refuse(fc, 0x01); } } From 0f305f18273d788cea8be8f4e0518e280a3cae9c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:35:40 -0400 Subject: [PATCH 3/3] fix(modbus): multi-write PDUs carry no trailing byte past their byte count The FC15, FC16, BitInRegister[] RMW FC16 and FC23 builders allocated one byte more than they filled (6 + 1 + n / 10 + 1 + n, data copied at offset 6 / 10), so every multi-write request ended in a stray zero byte beyond the length its byte-count field declares. A strict device or gateway that checks the frame length against the byte count refuses it. Each PDU is now exactly 6 + n (FC15/FC16) or 10 + n (FC23). Tests: ModbusWritePduFramingTests asserts the exact length and byte-count field for FC15 (1/8/9 coils), FC16 (array, Float64 array, scalar multi-register), the BitInRegister RMW FC16 and FC23 (all red before the fix); the parity suite's conformant device now refuses a frame-length mismatch on FC15/FC16/FC23. Live: Driver.Modbus.IntegrationTests against 10.100.0.35:5020 (standard profile) passes, including the FC16 and FC23 multi-register round trips. Refs #741 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../ModbusDriver.cs | 12 +- .../ModbusArrayWriteSupportParityTests.cs | 8 +- .../ModbusWritePduFramingTests.cs | 107 ++++++++++++++++++ 3 files changed, 120 insertions(+), 7 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusWritePduFramingTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index 83a28405..36cea8d1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -1603,7 +1603,8 @@ public sealed class ModbusDriver for (var i = 0; i < values.Length; i++) if (values[i]) bitmap[i / 8] |= (byte)(1 << (i % 8)); var qty = (ushort)values.Length; - var pdu15 = new byte[6 + 1 + byteCount]; + // [fc][addr:2][qty:2][byteCount] + data: exactly 6 + byteCount (no trailing byte past the declared data). + var pdu15 = new byte[6 + byteCount]; pdu15[0] = 0x0F; pdu15[1] = (byte)(tag.Address >> 8); pdu15[2] = (byte)(tag.Address & 0xFF); pdu15[3] = (byte)(qty >> 8); pdu15[4] = (byte)(qty & 0xFF); @@ -1638,7 +1639,8 @@ public sealed class ModbusDriver $"Write of {qty} registers to {tag.Name} exceeds MaxRegistersPerWrite={writeCap}. " + $"Split the tag (e.g. shorter StringLength or smaller ArrayCount) — partial FC16 chunks would lose atomicity."); if (await TryWriteViaFc23Async(transport, tag, bytes, qty, ct).ConfigureAwait(false)) return; - var pdu = new byte[6 + 1 + bytes.Length]; + // [fc][addr:2][qty:2][byteCount] + data: exactly 6 + byteCount. + var pdu = new byte[6 + bytes.Length]; pdu[0] = 0x10; pdu[1] = (byte)(tag.Address >> 8); pdu[2] = (byte)(tag.Address & 0xFF); pdu[3] = (byte)(qty >> 8); pdu[4] = (byte)(qty & 0xFF); @@ -1685,7 +1687,8 @@ public sealed class ModbusDriver if (!_options.UseFC23ForMultiRegisterWrites || _options.DisableFC23 || qty < 2 || qty > Fc23MaxWriteRegisters || unsupported.ContainsKey(unit)) return false; - var pdu = new byte[10 + 1 + bytes.Length]; + // [fc][readAddr:2][readQty:2][writeAddr:2][writeQty:2][writeByteCount] + data: exactly 10 + byteCount. + var pdu = new byte[10 + bytes.Length]; pdu[0] = 0x17; pdu[1] = (byte)(tag.Address >> 8); pdu[2] = (byte)(tag.Address & 0xFF); // read start pdu[3] = (byte)(qty >> 8); pdu[4] = (byte)(qty & 0xFF); // read quantity @@ -1824,7 +1827,8 @@ public sealed class ModbusDriver payload[i * 2] = (byte)(reg >> 8); payload[i * 2 + 1] = (byte)(reg & 0xFF); } - var pdu = new byte[6 + 1 + payload.Length]; + // [fc][addr:2][qty:2][byteCount] + data: exactly 6 + byteCount. + var pdu = new byte[6 + payload.Length]; pdu[0] = 0x10; pdu[1] = (byte)(tag.Address >> 8); pdu[2] = (byte)(tag.Address & 0xFF); pdu[3] = (byte)(count >> 8); pdu[4] = (byte)(count & 0xFF); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs index cca64cf5..56af4ad4 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs @@ -341,7 +341,8 @@ public sealed class ModbusArrayWriteSupportParityTests /// /// A spec-conformant Modbus device over the whole 0-65535 address space. It enforces the protocol quantity /// ceilings (FC01/02 <= 2000, FC03/04 <= 125, FC15 <= 1968, FC16 <= 123, FC23 write <= 121 / read <= 125) and the - /// FC15 / FC16 byte-count field with exception 03 (Illegal Data Value), and a span past address 65535 with + /// FC15 / FC16 / FC23 byte-count field AND the exact frame length it implies (a strict device refuses a trailing + /// byte) with exception 03 (Illegal Data Value), and a span past address 65535 with /// exception 02 (Illegal Data Address) - what a real device answers. Everything else it answers. /// private sealed class ConformantDevice : IModbusTransport @@ -392,13 +393,13 @@ public sealed class ModbusArrayWriteSupportParityTests return Task.FromResult(pdu); case 0x0F: if (Check(addr, qty, 1968) is { } ce) return Refuse(fc, ce); - if (pdu[5] != (qty + 7) / 8 || pdu.Length < 6 + pdu[5]) return Refuse(fc, 0x03); + if (pdu[5] != (qty + 7) / 8 || pdu.Length != 6 + pdu[5]) return Refuse(fc, 0x03); // strict frame length WritePdus++; for (var i = 0; i < qty; i++) Coils[addr + i] = ((pdu[6 + i / 8] >> (i % 8)) & 1) == 1; return Task.FromResult(new byte[] { 0x0F, pdu[1], pdu[2], pdu[3], pdu[4] }); case 0x10: if (Check(addr, qty, 123) is { } we) return Refuse(fc, we); - if (pdu[5] != qty * 2 || pdu.Length < 6 + pdu[5]) return Refuse(fc, 0x03); + if (pdu[5] != qty * 2 || pdu.Length != 6 + pdu[5]) return Refuse(fc, 0x03); // strict frame length WritePdus++; for (var i = 0; i < qty; i++) Holding[addr + i] = (ushort)((pdu[6 + i * 2] << 8) | pdu[7 + i * 2]); return Task.FromResult(new byte[] { 0x10, pdu[1], pdu[2], pdu[3], pdu[4] }); @@ -408,6 +409,7 @@ public sealed class ModbusArrayWriteSupportParityTests var writeQty = (pdu[7] << 8) | pdu[8]; if (Check(addr, qty, 125) is { } r23) return Refuse(fc, r23); if (Check(writeAddr, writeQty, 121) is { } w23) return Refuse(fc, w23); + if (pdu[9] != writeQty * 2 || pdu.Length != 10 + pdu[9]) return Refuse(fc, 0x03); // strict frame length WritePdus++; for (var i = 0; i < writeQty; i++) Holding[writeAddr + i] = (ushort)((pdu[10 + i * 2] << 8) | pdu[11 + i * 2]); return Task.FromResult(ReadRegs(0x17, addr, qty)); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusWritePduFramingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusWritePduFramingTests.cs new file mode 100644 index 00000000..bc125c3f --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusWritePduFramingTests.cs @@ -0,0 +1,107 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +/// +/// #741 (Integration-W3 fix round): every multi-write PDU the driver builds is EXACTLY its header plus the bytes its +/// byte-count field declares — FC15 and FC16 are 6 + byteCount, FC23 is 10 + writeByteCount. The +/// builders used to allocate one byte more than they filled, so every FC15 / FC16 / FC23 (and the BitInRegister[] +/// read-modify-write's FC16) carried a trailing zero byte past the declared data — which a strict device or gateway +/// that checks the frame length against the byte count refuses. +/// +[Trait("Category", "Unit")] +public sealed class ModbusWritePduFramingTests +{ + private const string Ref = "arr"; + + /// Records every PDU; answers reads with zeros and multi-writes with the standard echo. + private sealed class CapturingTransport : IModbusTransport + { + public readonly List Sent = []; + + public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask; + + public Task SendAsync(byte unitId, byte[] pdu, CancellationToken ct) + { + Sent.Add(pdu); + var qty = (pdu[3] << 8) | pdu[4]; + return Task.FromResult(pdu[0] switch + { + 0x01 or 0x02 => [pdu[0], (byte)((qty + 7) / 8), .. new byte[(qty + 7) / 8]], + 0x03 or 0x04 or 0x17 => [pdu[0], (byte)(qty * 2), .. new byte[qty * 2]], + 0x0F or 0x10 => new[] { pdu[0], pdu[1], pdu[2], pdu[3], pdu[4] }, + _ => pdu, + }); + } + + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + + private static async Task WriteAndCaptureAsync(string tagConfig, object value, byte functionCode, bool fc23 = false) + { + var transport = new CapturingTransport(); + var drv = new ModbusDriver( + new ModbusDriverOptions + { + Host = "fake", RawTags = [new RawTagEntry(Ref, tagConfig, false)], UseFC23ForMultiRegisterWrites = fc23, + }, + "modbus-framing", _ => transport); + await drv.InitializeAsync("{}", CancellationToken.None); + + (await drv.WriteAsync([new WriteRequest(Ref, value)], CancellationToken.None)).Single().StatusCode.ShouldBe(0u); + + return transport.Sent.Single(p => p[0] == functionCode); + } + + [Theory] + [InlineData(1)] + [InlineData(8)] + [InlineData(9)] + public async Task FC15_pdu_is_exactly_header_plus_byte_count(int coils) + { + var pdu = await WriteAndCaptureAsync( + $$"""{"region":"Coils","address":0,"isArray":true,"arrayLength":{{coils}}}""", + Enumerable.Repeat(true, coils).ToArray(), 0x0F); + + pdu[5].ShouldBe((byte)((coils + 7) / 8)); + pdu.Length.ShouldBe(6 + pdu[5]); + } + + [Theory] + [InlineData("""{"region":"HoldingRegisters","address":0,"dataType":"Int16","isArray":true,"arrayLength":3}""", 6)] + [InlineData("""{"region":"HoldingRegisters","address":0,"dataType":"Float64","isArray":true,"arrayLength":2}""", 16)] + [InlineData("""{"region":"HoldingRegisters","address":0,"dataType":"Int32"}""", 4)] // scalar multi-register + public async Task FC16_pdu_is_exactly_header_plus_byte_count(string tagConfig, int byteCount) + { + object value = tagConfig.Contains("isArray") ? new[] { 1, 2, 3 }.Take(tagConfig.Contains("Float64") ? 2 : 3).ToArray() : 7; + + var pdu = await WriteAndCaptureAsync(tagConfig, value, 0x10); + + pdu[5].ShouldBe((byte)byteCount); + pdu.Length.ShouldBe(6 + pdu[5]); + } + + [Fact] + public async Task BitInRegister_array_RMW_FC16_pdu_is_exactly_header_plus_byte_count() + { + var pdu = await WriteAndCaptureAsync( + """{"region":"HoldingRegisters","address":0,"dataType":"BitInRegister","bitIndex":2,"isArray":true,"arrayLength":4}""", + new[] { true, false, true, false }, 0x10); + + pdu[5].ShouldBe((byte)8); + pdu.Length.ShouldBe(6 + pdu[5]); + } + + [Fact] + public async Task FC23_pdu_is_exactly_header_plus_write_byte_count() + { + var pdu = await WriteAndCaptureAsync( + """{"region":"HoldingRegisters","address":0,"dataType":"Int16","isArray":true,"arrayLength":3}""", + new[] { 1, 2, 3 }, 0x17, fc23: true); + + pdu[9].ShouldBe((byte)6); + pdu.Length.ShouldBe(10 + pdu[9]); + } +}