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