From 18f8eb5d2f000b8739c67ac7fab39c14d70c62ca Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 07:39:05 -0400 Subject: [PATCH] feat(address-space): server-side array writes gated per driver capability (#610) Array nodes were forced read-only for every driver. A new single authority, Core.Abstractions DriverArrayWriteSupport, says which (driver, element type) pairs encode a whole-array write: Modbus (Coils / numeric HoldingRegisters), S7 (Byte/Int16/UInt16/Int32/UInt32/Float32), OpcUaClient (pass-through) and Galaxy (Boolean/Int32/Int64/Float/Double/String/DateTime). Everything else stays read-only, so the server never advertises a write the driver refuses. - Composer derives RawTagPlan.ArrayWritable + UnsReferenceVariable .BackingArrayWritable at compose time (both seams, byte-parity); the applier's one writability rule is Writable && (!IsArray || ArrayWritable) for materialise and surgical paths, raw and UNS alike. - ArrayWritable is on the classifier's surgical raw-tag whitelist and mirrored on the UNS record (UnsReferenceBackingParityTests updated). - Node manager refuses a wrong-length / non-array value on an array node (BadTypeMismatch) and any IndexRange write (BadWriteNotSupported) inline, before dispatch; WriteOperate/ACL gates and the revert are unchanged. - Per-driver parity tests pin the table against the real Modbus write path, S7 EncodeArrayBlock and Galaxy MxValueEncoder (UInt32/UInt64 excluded: array covariance reinterprets their bits). - Over-the-wire proof in OpcUaServer.IntegrationTests (ArrayWriteWireTests). - ValueRank > 1 documented as out of scope by decision (docs/Uns.md). Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/Raw.md | 5 +- docs/Uns.md | 44 +++- docs/drivers/S7.md | 7 +- .../DriverArrayWriteSupport.cs | 120 ++++++++++ .../AddressSpaceApplier.cs | 40 +++- .../AddressSpaceChangeClassifier.cs | 7 +- .../AddressSpaceComposer.cs | 46 +++- .../OtOpcUaNodeManager.cs | 35 ++- .../DriverArrayWriteSupportTests.cs | 87 +++++++ .../GalaxyArrayWriteSupportParityTests.cs | 63 +++++ .../ModbusArrayWriteSupportParityTests.cs | 58 +++++ .../S7ArrayWriteSupportParityTests.cs | 49 ++++ .../ArrayWriteWireTests.cs | 225 ++++++++++++++++++ .../AddressSpaceApplierRawUnsTests.cs | 35 +++ .../AddressSpaceApplierTests.cs | 29 +++ ...SpaceChangeClassifierDualNamespaceTests.cs | 7 +- .../AddressSpaceComposerDualNamespaceTests.cs | 38 +++ .../EquipmentWriteGateTests.cs | 39 +++ .../UnsReferenceBackingParityTests.cs | 2 +- .../DeploymentArtifactRawUnsParityTests.cs | 6 +- 20 files changed, 907 insertions(+), 35 deletions(-) create mode 100644 src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs create mode 100644 tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverArrayWriteSupportTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyArrayWriteSupportParityTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ArrayWriteSupportParityTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ArrayWriteWireTests.cs diff --git a/docs/Raw.md b/docs/Raw.md index f855f8de..f49f90d3 100644 --- a/docs/Raw.md +++ b/docs/Raw.md @@ -80,8 +80,9 @@ removed later. Editing a raw **tag's** attributes is no longer a fleet-wide event. A **non-alarm** raw-tag delta whose only differences are the in-place-updatable attributes — -`Writable`, `IsHistorized`, `HistorianTagname`, `DataType`, `IsArray`, `ArrayLength` — -and/or its set of referencing equipment is applied **in place** on the existing Raw-realm +`Writable`, `IsHistorized`, `HistorianTagname`, `DataType`, `IsArray`, `ArrayLength`, +`ArrayWritable` (the driver's array-write capability, #610 — see +[Uns.md §Array writes](Uns.md#array-writes-610)) — and/or its set of referencing equipment is applied **in place** on the existing Raw-realm node (`ISurgicalAddressSpaceSink.UpdateTagAttributes`), so every client subscription on that tag and on the rest of the server survives. The same is true of **every** UNS tag-reference change: adding, removing or re-pointing a reference used to flip the diff --git a/docs/Uns.md b/docs/Uns.md index c053afab..c48fbaf0 100644 --- a/docs/Uns.md +++ b/docs/Uns.md @@ -312,10 +312,50 @@ Combined with historization (values are arrays — history of the whole array sn | **AB Legacy** | PCCC multi-element file read via libplctag (cap 256 elements) | Unit-proven (sim fixture down) | | **TwinCAT** | Raw-bytes read of the declared `SymbolPath` at the resolved array type's layout + per-element decode; native ADS notifications carry the whole array (same decode). Multi-dim PLC arrays publish flattened; a shorter `arrayLength` reads the leading sub-range | Live-verified 2026-10-07 on two TwinCAT 2.11 PLCs (see [TwinCAT.md §Array tags](drivers/TwinCAT.md#array-tags)) | +### Array writes (#610) + +An authored-`ReadWrite` array tag is **writable only when its driver can write that array** — +the server never advertises a write the driver will refuse. The capability is one table, +`DriverArrayWriteSupport` (`Core.Abstractions`), consulted by the composer at compose time and +carried on the plan records (`RawTagPlan.ArrayWritable`, mirrored onto every UNS reference as +`UnsReferenceVariable.BackingArrayWritable`); the node is writable iff +`Writable && (!IsArray || ArrayWritable)`, on the raw node and every UNS mirror alike. + +| Driver | Array writes | Element types | +|---|---|---| +| **Modbus** | Yes | `region: Coils` (FC15, any element); `region: HoldingRegisters` with `Int16`/`UInt16`/`Bcd16`/`Int32`/`UInt32`/`Bcd32`/`Float32`/`Int64`/`UInt64`/`Float64` (FC16). Not BitInRegister / String / Bool-in-register; input regions are read-only | +| **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 | +| **Galaxy** | Yes | OPC UA `Boolean`/`Int32`/`Int64`/`Float`/`Double`/`String`/`DateTime` (the `MxValueEncoder` array variants) | +| AB CIP, AB Legacy, TwinCAT, FOCAS, MQTT, MTConnect, Sql, Calculation | **No** — scalar-only write encoders; array nodes stay read-only and a client write is `BadNotWritable` | — | + +Each "Yes" row is pinned against the driver's real encoder by a parity test in that driver's +test project (`*ArrayWriteSupportParityTests`), so a driver gaining or losing an array encoder +fails a test until the table follows. + +What a client sees on a writable array node: + +- **Whole-array writes only.** The value must be a 1-D array of exactly `arrayLength` elements + (an unbounded `arrayLength`/`ArrayDimensions = [0]` takes any length); anything else is + `BadTypeMismatch`, refused inline before dispatch. A partial **IndexRange** write is + `BadWriteNotSupported` — every array-capable driver writes the whole block, and the server + does not hold the device array authoritatively enough to read-modify-write it. +- The same realm-qualified `WriteOperate` / ACL gate as a scalar applies to both NodeIds, and a + device-refused array write is reverted on the NodeId it was written through (the scalar + write-outcome self-correction, unchanged). +- Flipping a tag's array-write capability (a driver-type or element-type edit on an existing + array tag) is a **surgical** in-place writability update, not a rebuild: `ArrayWritable` is on + the classifier's raw-tag whitelist, and the mirrors repair through `BackingArrayWritable`. + ### Out of scope (named deferrals) -- **Array writes** (inbound client→device write of an array value) — tagged for a follow-up phase. The S7 driver's array write path exists (`Byte`/`Int16`/`UInt16`/`Int32`/`UInt32`/`Float32`, not `Bool`; see [S7.md](drivers/S7.md)) but the applier still forces array nodes read-only (#610 item 1). -- **Multi-dimensional arrays** (`ValueRank > 1`) — not supported; all arrays are 1-D. +- **Multi-dimensional arrays** (`ValueRank > 1`) — **out of scope by decision** (#610 item 3), + not merely deferred. Every driver's array read decodes a flat 1-D block (TwinCAT already + publishes multi-dim PLC arrays flattened), and the node manager materialises every array + `ValueRank = OneDimension` with a single `ArrayDimensions` entry. Supporting rank > 1 would + need per-rank dimensions in TagConfig, the plan records, the classifier whitelist, the node + manager and every driver's decode/encode, and no consumer has asked for it. Author a + multi-dimensional PLC array as a flat 1-D tag (row-major), or as several 1-D tags. - **Array historization** — a historized array tag materialises with the correct `Historizing` flag, but the historian backend treats the value as an opaque blob; per-element history is out of scope. (Continuous historization is numeric-analog only — array / non-numeric values are not recorded.) See the individual driver docs under `docs/drivers/` for per-driver implementation details. diff --git a/docs/drivers/S7.md b/docs/drivers/S7.md index af025019..f58d2987 100644 --- a/docs/drivers/S7.md +++ b/docs/drivers/S7.md @@ -253,12 +253,13 @@ decode), and the value must be an array of exactly `arrayLength` elements (else health impact). **Writable `Bool` arrays are refused at init** (`NotSupportedException`): the array read is byte-granular, so a matching write would overwrite the 7 neighbouring bits of every byte it touches, and a safe bit-packed write needs read-modify-write. Mark Bool arrays `writable: false`. -Note the *server* still presents every array node read-only to OPC UA clients ([Uns.md §Array -tags](../Uns.md#array-tags-1-d), #610 item 1) — the driver path is reachable today through the Client +The server exposes these as **writable OPC UA array nodes** (#610) for exactly the element types above — +a `Bool` / wide-type S7 array stays read-only at the node (`BadNotWritable`), see [Uns.md §Array +writes](../Uns.md#array-writes-610). The driver path is also reachable directly through the Client CLI / driver-level callers and is proven by the unit suite plus the snap7 fixture test (`S7_1500ArrayWriteTests`); the AdminUI tag editor rejects wide-type and writable-Bool arrays at author time. -**Deferrals** — server-side exposure of array writes, multi-dimensional arrays, per-element historization, +**Deferrals** — multi-dimensional arrays (out of scope by decision, see Uns.md), per-element historization, wide-type array elements (`Int64`, `UInt64`, `Float64`, `String`, `DateTime`), and Timer/Counter array elements. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs new file mode 100644 index 00000000..68a33120 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverArrayWriteSupport.cs @@ -0,0 +1,120 @@ +using System.Text.Json; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +/// +/// Gitea #610 — the single authority on whether a driver can WRITE a 1-D array tag. The address-space +/// composer consults it at compose time (both compose seams run the same composer, so the live-edit and +/// artifact-decode sides cannot disagree) to decide whether an authored-ReadWrite array tag +/// materialises as a writable OPC UA node. Before #610 every array node was forced read-only regardless of +/// driver; that remains the answer for every (driver, element type) pair NOT listed here. +/// +/// The rule is "never advertise a write the driver will refuse". A pair is listed only when the +/// driver's IWritable.WriteAsync encodes a whole-array value end to end; a driver that refuses an +/// array value (a scalar-only encoder, a NotSupportedException guard) is left out so its array +/// nodes stay read-only and a client write is refused at the node (BadNotWritable) rather than +/// optimistically applied and reverted. The table is pure data, keyed off the same TagConfig keys the +/// drivers' own tag-definition factories read, because the composer has no reference to driver +/// assemblies. Each listed driver's test project pins its row against the real encoder +/// (*ArrayWriteSupportParityTests), so a driver gaining or losing an array encoder fails a test +/// until this table follows. +/// +/// +/// Modbus — region: Coils (FC15, any element) and region: HoldingRegisters +/// with a fixed-width numeric dataType (FC16). Bit-in-register, String and Bool-in-register +/// arrays have no array encoder; input regions are 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. +/// OpcUaClient — every element type: the value is forwarded to the upstream server as a +/// Variant unchanged, and the upstream server is the arbiter. +/// Galaxy — the array variants MxValueEncoder encodes, keyed by the OPC UA +/// DataType the node is created with: Boolean / Int32 / Int64 / Float / Double / String / +/// DateTime. +/// Everything else (AbCip, AbLegacy, TwinCAT, FOCAS, MQTT, MTConnect, Sql, Calculation) — scalar +/// write encoders only; array nodes stay read-only. +/// +/// Multi-dimensional arrays (ValueRank > 1) are out of scope altogether — every array node +/// is materialised 1-D (ValueRank = OneDimension), so nothing here considers more than one rank. +/// +public static class DriverArrayWriteSupport +{ + private static readonly HashSet ModbusRegisterArrayTypes = new(StringComparer.OrdinalIgnoreCase) + { + "Int16", "UInt16", "Bcd16", "Int32", "UInt32", "Bcd32", "Float32", "Int64", "UInt64", "Float64", + }; + + private static readonly HashSet S7ArrayTypes = new(StringComparer.OrdinalIgnoreCase) + { + "Byte", "Int16", "UInt16", "Int32", "UInt32", "Float32", + }; + + private static readonly HashSet GalaxyArrayDataTypes = new(StringComparer.Ordinal) + { + "Boolean", "Int32", "Int64", "Float", "Double", "String", "DateTime", + }; + + /// + /// Whether a driver of can write a 1-D array tag of the given shape. + /// + /// The owning driver instance's DriverType (a + /// constant); or unknown ⇒ . + /// The tag's OPC UA DataType name (Tag.DataType). + /// The tag's TagConfig JSON (driver-native address + element type). + /// only when the driver encodes a whole-array write for this tag. + public static bool Supports(string? driverType, string? dataType, string? tagConfig) + { + switch (driverType) + { + case DriverTypeNames.OpcUaClient: + return true; + case DriverTypeNames.Galaxy: + return dataType is not null && GalaxyArrayDataTypes.Contains(dataType); + case DriverTypeNames.Modbus: + { + // Same reads as ModbusTagDefinitionFactory: an absent (or non-string) region defaults to + // HoldingRegisters and an absent dataType to Int16 (TagConfigJson.TryReadEnumStrict). + if (!TryParseObject(tagConfig, out var root)) return false; + var region = ReadEnumName(root, "region") ?? "HoldingRegisters"; + var native = ReadEnumName(root, "dataType") ?? "Int16"; + if (string.Equals(region, "Coils", StringComparison.OrdinalIgnoreCase)) return true; + return string.Equals(region, "HoldingRegisters", StringComparison.OrdinalIgnoreCase) + && ModbusRegisterArrayTypes.Contains(native); + } + case DriverTypeNames.S7: + { + // S7TagDefinitionFactory requires a dataType key (absent ⇒ not an S7 tag at all); a present + // non-string value falls back to Int16, exactly as the factory's strict enum read does. + if (!TryParseObject(tagConfig, out var root) || !root.TryGetProperty("dataType", out _)) return false; + var native = ReadEnumName(root, "dataType") ?? "Int16"; + return S7ArrayTypes.Contains(native); + } + default: + return false; + } + } + + // Parses the TagConfig root object; a blank / malformed / non-object blob is "no tag" ⇒ false. The clone + // detaches the element from the disposed document. + private static bool TryParseObject(string? tagConfig, out JsonElement root) + { + root = default; + if (string.IsNullOrWhiteSpace(tagConfig)) return false; + try + { + using var doc = JsonDocument.Parse(tagConfig); + if (doc.RootElement.ValueKind != JsonValueKind.Object) return false; + root = doc.RootElement.Clone(); + return true; + } + catch (JsonException) + { + return false; + } + } + + // An enum-valued field's NAME, or null when absent or not a JSON string — the drivers' strict reads treat a + // non-string token (a numerically-serialised enum) as absent and apply the field's default. + private static string? ReadEnumName(JsonElement root, string name) => + root.TryGetProperty(name, out var el) && el.ValueKind == JsonValueKind.String ? el.GetString() : null; +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs index 8358ee60..2086e878 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs @@ -229,9 +229,9 @@ public sealed class AddressSpaceApplier } // Compute writable + historian EXACTLY as MaterialiseRawSubtree does so the in-place update - // matches what a rebuild would have produced: arrays are forced read-only (the driver write - // path can't handle them) and a blank historian override falls back to the RawPath (== NodeId). - var writable = d.Current.Writable && !d.Current.IsArray; + // matches what a rebuild would have produced: an array is writable only when its driver can + // write arrays (#610) and a blank historian override falls back to the RawPath (== NodeId). + var writable = RawNodeWritable(d.Current); var historian = d.Current.IsHistorized ? (string.IsNullOrWhiteSpace(d.Current.HistorianTagname) ? d.Current.NodeId : d.Current.HistorianTagname) : null; @@ -275,9 +275,9 @@ public sealed class AddressSpaceApplier if (IsIdentityPreservingUnsRefDelta(d)) { // Resolve writable + historian EXACTLY as MaterialiseUnsReferences does when it creates - // the node: arrays are forced read-only, and a blank override falls back to the backing - // RawPath (which IS the backing raw tag's NodeId). - var writable = d.Current.Writable && !d.Current.BackingIsArray; + // the node: an array is writable only when its backing driver can write arrays (#610), and + // a blank override falls back to the backing RawPath (which IS the backing raw tag's NodeId). + var writable = UnsReferenceNodeWritable(d.Current); var historian = d.Current.BackingIsHistorized ? (string.IsNullOrWhiteSpace(d.Current.BackingHistorianTagname) ? d.Current.BackingRawPath : d.Current.BackingHistorianTagname) : null; @@ -467,6 +467,21 @@ public sealed class AddressSpaceApplier /// /// The UNS reference delta to classify. /// true when the delta is attribute-only and may be applied in place. + /// + /// Gitea #610 — the ONE writability rule for a raw value node, shared by the materialise pass and the + /// surgical in-place update so the two can never disagree: authored ReadWrite, and — for an array — + /// a driver that can write the array (, from + /// ). An array tag on a driver + /// with no array encoder stays read-only, so a client write is refused at the node (BadNotWritable) + /// instead of being optimistically applied and reverted. + /// + internal static bool RawNodeWritable(RawTagPlan t) => t.Writable && (!t.IsArray || t.ArrayWritable); + + /// The UNS-mirror counterpart of , off the inherited Backing* + /// fields — a mirror is writable exactly when its raw node is. + internal static bool UnsReferenceNodeWritable(UnsReferenceVariable v) => + v.Writable && (!v.BackingIsArray || v.BackingArrayWritable); + private static bool IsIdentityPreservingUnsRefDelta(AddressSpacePlan.UnsReferenceDelta d) => string.Equals(d.Previous.NodeId, d.Current.NodeId, StringComparison.Ordinal) && string.Equals(d.Previous.EquipmentId, d.Current.EquipmentId, StringComparison.Ordinal) && @@ -972,11 +987,12 @@ public sealed class AddressSpaceApplier else { // A historized tag materialises Historizing + HistoryRead; the effective historian tagname is - // the override else the RawPath (== NodeId). Array writes are out of scope → force read-only. + // the override else the RawPath (== NodeId). An array is writable only when its driver can + // write arrays (#610) — see RawNodeWritable. string? historianTagname = t.IsHistorized ? (string.IsNullOrWhiteSpace(t.HistorianTagname) ? t.NodeId : t.HistorianTagname) : null; - var writable = t.Writable && !t.IsArray; + var writable = RawNodeWritable(t); if (!SafeEnsureVariable(t.NodeId, t.ParentNodeId, t.Name, t.DataType, writable, AddressSpaceRealm.Raw, historianTagname, t.IsArray, t.ArrayLength)) failed++; } } @@ -1033,7 +1049,7 @@ public sealed class AddressSpaceApplier string? historianTagname = v.BackingIsHistorized ? (string.IsNullOrWhiteSpace(v.BackingHistorianTagname) ? v.BackingRawPath : v.BackingHistorianTagname) : null; - var writable = v.Writable && !isArray; + var writable = UnsReferenceNodeWritable(v); if (!SafeEnsureVariable(v.NodeId, v.EquipmentId, v.EffectiveName, v.DataType, writable, AddressSpaceRealm.Uns, historianTagname, isArray, arrayLength)) { failed++; @@ -1120,9 +1136,9 @@ public sealed class AddressSpaceApplier string? historianTagname = tag.IsHistorized ? (string.IsNullOrWhiteSpace(tag.HistorianTagname) ? tag.FullName : tag.HistorianTagname) : null; - // Array writes are out of scope (Phase 4c read-only surface): force array tags read-only - // even if authored ReadWrite, so a client write cannot reach the driver write path which - // does not handle arrays (e.g. S7 BoxValueForWrite would crash). + // Retired equipment-tag path (structurally empty in v3): array tags stay forced read-only + // here. The live array-write capability (#610) is carried on RawTagPlan / + // UnsReferenceVariable only — see RawNodeWritable. var writable = tag.Writable && !tag.IsArray; if (!SafeEnsureVariable(nodeId, parent, tag.Name, tag.DataType, writable, AddressSpaceRealm.Uns, historianTagname, tag.IsArray, tag.ArrayLength)) failed++; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs index 24ebcb4f..c9022489 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs @@ -166,8 +166,10 @@ public static class AddressSpaceChangeClassifier }).Equals(d.Current); // v3 surgical raw-tag eligibility: a NON-alarm raw tag whose only differences are the in-place-updatable - // attributes (Writable / IsHistorized / HistorianTagname / DataType / IsArray / ArrayLength) and/or its - // ReferencingEquipmentPaths. The referencing set is node-inert for a VALUE tag — it only drives native-alarm + // attributes (Writable / IsHistorized / HistorianTagname / DataType / IsArray / ArrayLength / ArrayWritable) + // and/or its ReferencingEquipmentPaths. ArrayWritable (Gitea #610) only feeds the node's writability — the + // same AccessLevel + OnWriteValue swap Writable already drives in place — so it must be listed here or + // every array-tag edit that flips it would fall through to a full rebuild. The referencing set is node-inert for a VALUE tag — it only drives native-alarm // notifier wiring (the WP4 guard), which is why Alarm != null on EITHER side disqualifies. Identity fields // (NodeId is the diff key; TagId / DriverInstanceId / Name / ParentNodeId / Realm) still force a rebuild via // the with-equality default-closed pattern. @@ -181,6 +183,7 @@ public static class AddressSpaceChangeClassifier DataType = d.Current.DataType, IsArray = d.Current.IsArray, ArrayLength = d.Current.ArrayLength, + ArrayWritable = d.Current.ArrayWritable, ReferencingEquipmentPaths = d.Current.ReferencingEquipmentPaths, }).Equals(d.Current); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs index 90f9e812..4b3c7ece 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs @@ -3,6 +3,7 @@ using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.OpcUaServer; @@ -137,6 +138,11 @@ public sealed record RawContainerNode( /// referencing equipment. is always . /// A null means the historian tagname defaults to the RawPath (resolved /// later, not here). +/// (Gitea #610) is whether the owning driver can write this tag as a whole +/// 1-D array (); always +/// for a scalar. The materialised node is writable iff +/// Writable && (!IsArray || ArrayWritable), so an array tag on a driver with no array +/// encoder stays read-only exactly as before #610. /// ADDING A FIELD IS A FOUR-PLACE CONTRACT — the hand-written equality below does NOT close /// over new members. Because forces a custom /// Equals/GetHashCode, the compiler's positional equality is suppressed, so a field added @@ -163,7 +169,8 @@ public sealed record RawTagPlan( string? HistorianTagname = null, bool IsArray = false, uint? ArrayLength = null, - AddressSpaceRealm Realm = AddressSpaceRealm.Raw) + AddressSpaceRealm Realm = AddressSpaceRealm.Raw, + bool ArrayWritable = false) { /// // Structural equality: the auto-generated record equality compares ReferencingEquipmentPaths (an @@ -185,6 +192,7 @@ public sealed record RawTagPlan( IsArray == other.IsArray && ArrayLength == other.ArrayLength && Realm == other.Realm && + ArrayWritable == other.ArrayWritable && ReferencingEquipmentPaths.SequenceEqual(other.ReferencingEquipmentPaths, StringComparer.Ordinal); /// @@ -194,7 +202,7 @@ public sealed record RawTagPlan( hash.Add(TagId); hash.Add(NodeId); hash.Add(ParentNodeId); hash.Add(DriverInstanceId); hash.Add(Name); hash.Add(DataType); hash.Add(Writable); hash.Add(Alarm); hash.Add(IsHistorized); hash.Add(HistorianTagname); hash.Add(IsArray); hash.Add(ArrayLength); - hash.Add(Realm); + hash.Add(Realm); hash.Add(ArrayWritable); foreach (var p in ReferencingEquipmentPaths) hash.Add(p, StringComparer.Ordinal); return hash.ToHashCode(); } @@ -226,7 +234,9 @@ public sealed record RawTagPlan( /// existing machinery repairs it: the applier removes the previous UNS node and /// recreates it from these values. /// / were already inherited onto the record and are -/// covered by the same mechanism. +/// covered by the same mechanism, as is (Gitea #610 — the backing +/// tag's , so a mirror is writable exactly when its raw node +/// is). /// Equality is the COMPILER-GENERATED positional-record equality: every member is a /// value/string type (no IReadOnlyList member, unlike ), so the four /// new fields are compared by value with no hand-written Equals/GetHashCode needed. If @@ -245,7 +255,8 @@ public sealed record UnsReferenceVariable( string? BackingHistorianTagname = null, bool BackingIsArray = false, uint? BackingArrayLength = null, - AddressSpaceRealm Realm = AddressSpaceRealm.Uns); + AddressSpaceRealm Realm = AddressSpaceRealm.Uns, + bool BackingArrayWritable = false); public sealed record UnsAreaProjection(string UnsAreaId, string DisplayName); public sealed record UnsLineProjection(string UnsLineId, string UnsAreaId, string DisplayName); @@ -657,7 +668,7 @@ public static class AddressSpaceComposer rawFolders, driverInstances, devices, tagGroups, tags, unsTagReferences, unsAreas, unsLines, equipment, topology); var unsReferenceVariables = BuildUnsReferenceVariables( - unsTagReferences, tags, unsAreas, unsLines, equipment, topology); + unsTagReferences, tags, unsAreas, unsLines, equipment, topology, driverInstances); return new AddressSpaceComposition(areas, lines, nodes, plans, alarms) { @@ -756,6 +767,7 @@ public static class AddressSpaceComposer // TagId → the set of referencing equipment UNS folder paths (Area/Line/Equipment), for the // native-alarm multi-notifier. Distinct + ordinal-sorted for determinism. var equipPathByTag = BuildReferencingEquipmentPathsByTag(unsTagReferences, unsAreas, unsLines, equipment); + var driverTypeById = BuildDriverTypeById(driverInstances); var rawTags = new List(); foreach (var t in tags ?? Array.Empty()) @@ -767,6 +779,8 @@ public static class AddressSpaceComposer : topology.DevicePaths.GetValueOrDefault(t.DeviceId); var driverInstanceId = topology.DriverIdByDevice.GetValueOrDefault(t.DeviceId, string.Empty); var intent = TagConfigIntent.Parse(t.TagConfig); + var arrayWritable = intent.IsArray && DriverArrayWriteSupport.Supports( + driverTypeById.GetValueOrDefault(driverInstanceId), t.DataType, t.TagConfig); var alarm = intent.Alarm is { } a ? new EquipmentTagAlarmInfo(a.AlarmType, a.Severity, a.HistorizeToAveva) : null; @@ -785,7 +799,8 @@ public static class AddressSpaceComposer IsHistorized: intent.IsHistorized, HistorianTagname: intent.HistorianTagname, IsArray: intent.IsArray, - ArrayLength: intent.ArrayLength)); + ArrayLength: intent.ArrayLength, + ArrayWritable: arrayWritable)); } containers.Sort((x, y) => string.CompareOrdinal(x.NodeId, y.NodeId)); @@ -793,6 +808,13 @@ public static class AddressSpaceComposer return (containers, rawTags); } + /// DriverInstanceId → DriverType, for the per-driver array-write capability (Gitea #610). + /// First row wins on a duplicate id, matching the composer's other id maps. + private static IReadOnlyDictionary BuildDriverTypeById(IReadOnlyList driverInstances) => + driverInstances + .GroupBy(d => d.DriverInstanceId, StringComparer.Ordinal) + .ToDictionary(g => g.Key, g => g.First().DriverType, StringComparer.Ordinal); + /// /// Emit the UNS reference Variables: one per whose backing raw tag + /// equipment path resolve, keyed (realm=Uns, s=Area/Line/Equipment/EffectiveName) and carrying @@ -806,10 +828,12 @@ public static class AddressSpaceComposer IReadOnlyList unsAreas, IReadOnlyList unsLines, IReadOnlyList equipment, - RawTopology topology) + RawTopology topology, + IReadOnlyList driverInstances) { var refs = unsTagReferences ?? Array.Empty(); if (refs.Count == 0) return Array.Empty(); + var driverTypeById = BuildDriverTypeById(driverInstances); var tagsById = (tags ?? Array.Empty()) .GroupBy(t => t.TagId, StringComparer.Ordinal) @@ -840,6 +864,11 @@ public static class AddressSpaceComposer // doc-comment; 2026-08-15 surgical-deploys review). Parsed from the SAME TagConfigIntent the raw // tag plan uses, so the two sides can never disagree. var intent = TagConfigIntent.Parse(tag.TagConfig); + // Same derivation as BuildRawSubtree's ArrayWritable, off the same row, so a mirror is writable + // exactly when its raw node is. + var backingArrayWritable = intent.IsArray && DriverArrayWriteSupport.Supports( + driverTypeById.GetValueOrDefault(topology.DriverIdByDevice.GetValueOrDefault(tag.DeviceId, string.Empty)), + tag.DataType, tag.TagConfig); vars.Add(new UnsReferenceVariable( UnsTagReferenceId: r.UnsTagReferenceId, @@ -852,7 +881,8 @@ public static class AddressSpaceComposer BackingIsHistorized: intent.IsHistorized, BackingHistorianTagname: intent.HistorianTagname, BackingIsArray: intent.IsArray, - BackingArrayLength: intent.ArrayLength)); + BackingArrayLength: intent.ArrayLength, + BackingArrayWritable: backingArrayWritable)); } vars.Sort((x, y) => string.CompareOrdinal(x.NodeId, y.NodeId)); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index b078231a..70c9fbb9 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1940,7 +1940,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // synchronously so the SDK never applies it (no optimistic-Good-then-revert round-trip + no needless // device dispatch). Runs AFTER the authz gate (so we never leak structure detail to an unauthorised // caller) but BEFORE the optimistic dispatch below. - var structure = EvaluateEquipmentWriteStructure(value, node); + var structure = EvaluateEquipmentWriteRange(indexRange) ?? EvaluateEquipmentWriteStructure(value, node); if (structure is not null) return structure; // Capture the optimistic value + the REAL prior value/status BEFORE the SDK applies the write @@ -2057,6 +2057,25 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 return new ServiceResult(StatusCodes.BadTypeMismatch, "null value write rejected"); } + // Gitea #610 — an array node (writable only when its driver encodes whole-array writes) takes a whole + // 1-D array of its declared length: every array-capable driver refuses a short or long block (Modbus + // FC15/FC16, S7's contiguous block), so a wrong shape is rejected here instead of being optimistically + // applied, dispatched, refused and reverted. ArrayDimensions [0] / absent means unbounded ⇒ any length. + if (node is BaseDataVariableState { ValueRank: ValueRanks.OneDimension } arrayNode) + { + if (value is not Array { Rank: 1 } array) + { + return new ServiceResult(StatusCodes.BadTypeMismatch, "a 1-D array node takes a 1-D array value"); + } + var declared = arrayNode.ArrayDimensions is { Count: > 0 } dims ? dims[0] : 0u; + if (declared > 0 && array.Length != declared) + { + return new ServiceResult( + StatusCodes.BadTypeMismatch, + $"array write of {array.Length} element(s) to a {declared}-element array node"); + } + } + // Cheap, confidence-gated built-in-type compatibility check. Only when the node is a value variable // with a concretely-resolvable expected built-in type AND the payload's built-in type is also cheaply // resolvable do we compare; otherwise proceed and let the SDK's WriteValue coercion be authoritative. @@ -2084,6 +2103,20 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 return null; } + /// + /// Gitea #610 — an equipment-tag write must be a WHOLE-value write. A partial (IndexRange) write to an + /// array node is refused: every array-capable driver writes the whole block, so honouring a range would + /// mean a read-modify-write of the device array the server does not hold authoritatively (the node value + /// can be stale or optimistic), and the SDK would apply the range to the node before the device answered. + /// A range on a scalar node is meaningless and refused the same way. Pure + static. + /// + /// The write's parsed IndexRange ( when absent). + /// null to proceed; otherwise BadWriteNotSupported. + internal static ServiceResult? EvaluateEquipmentWriteRange(NumericRange indexRange) => + indexRange == NumericRange.Empty + ? null + : new ServiceResult(StatusCodes.BadWriteNotSupported, "IndexRange writes are not supported; write the whole array"); + /// /// Conservative built-in-type compatibility test for . /// Returns true (compatible — allow through) unless there is a confident mismatch. Exact matches pass; 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 new file mode 100644 index 00000000..7518fcf7 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverArrayWriteSupportTests.cs @@ -0,0 +1,87 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests; + +/// +/// Gitea #610 — the per-driver array-write capability table. The table's agreement with each listed driver's +/// real encoder is pinned in that driver's own test project (*ArrayWriteSupportParityTests); these +/// tests pin the table's own reading of TagConfig (defaults, numeric enums, malformed blobs). +/// +public sealed class DriverArrayWriteSupportTests +{ + [Theory] + [InlineData("{\"region\":\"Coils\",\"address\":0,\"dataType\":\"Bool\"}", true)] + [InlineData("{\"region\":\"Coils\",\"address\":0}", true)] // FC15 ignores dataType + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"Float32\"}", true)] + [InlineData("{\"region\":\"holdingregisters\",\"address\":0,\"dataType\":\"bcd16\"}", true)] // case-insensitive, like the driver + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0}", true)] // dataType ⇒ Int16 + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"BitInRegister\"}", false)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"String\"}", false)] + [InlineData("{\"region\":\"HoldingRegisters\",\"address\":0,\"dataType\":\"Bool\"}", false)] + [InlineData("{\"region\":\"InputRegisters\",\"address\":0,\"dataType\":\"Int16\"}", false)] // read-only region + [InlineData("{\"region\":\"DiscreteInputs\",\"address\":0}", false)] + [InlineData("{\"region\":0,\"address\":0,\"dataType\":\"Int16\"}", true)] // numeric ⇒ absent ⇒ HoldingRegisters (driver reads it that way) + [InlineData("not json", false)] + [InlineData("", false)] + public void Modbus_rows_follow_region_and_element_type(string tagConfig, bool expected) => + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", tagConfig).ShouldBe(expected); + + [Theory] + [InlineData("Byte", true)] + [InlineData("Int16", true)] + [InlineData("UInt16", true)] + [InlineData("Int32", true)] + [InlineData("UInt32", true)] + [InlineData("Float32", true)] + [InlineData("Bool", false)] + [InlineData("Int64", false)] + [InlineData("UInt64", false)] + [InlineData("Float64", false)] + [InlineData("String", false)] + [InlineData("DateTime", false)] + public void S7_rows_follow_the_element_type(string s7Type, bool expected) => + DriverArrayWriteSupport.Supports(DriverTypeNames.S7, "Int16", $"{{\"address\":\"DB1.DBW0\",\"dataType\":\"{s7Type}\"}}") + .ShouldBe(expected); + + [Fact] + public void S7_without_a_dataType_key_is_not_an_S7_tag() => + DriverArrayWriteSupport.Supports(DriverTypeNames.S7, "Int16", "{\"address\":\"DB1.DBW0\"}").ShouldBeFalse(); + + [Theory] + [InlineData("Boolean", true)] + [InlineData("Int32", true)] + [InlineData("Int64", true)] + [InlineData("Float", true)] + [InlineData("Double", true)] + [InlineData("String", true)] + [InlineData("DateTime", true)] + [InlineData("Int16", false)] + [InlineData("UInt32", false)] + [InlineData("Byte", false)] + public void Galaxy_rows_follow_the_opc_ua_data_type(string dataType, bool expected) => + DriverArrayWriteSupport.Supports(DriverTypeNames.Galaxy, dataType, "{\"FullName\":\"Obj.Attr\"}").ShouldBe(expected); + + [Fact] + public void OpcUaClient_forwards_every_array() => + DriverArrayWriteSupport.Supports(DriverTypeNames.OpcUaClient, "UInt16", "{\"nodeId\":\"ns=2;s=x\"}").ShouldBeTrue(); + + /// Every driver type NOT in the table — and an unknown or absent one — stays read-only. + [Fact] + public void Every_other_driver_type_is_not_array_writable() + { + var listed = new[] { DriverTypeNames.Modbus, DriverTypeNames.S7, DriverTypeNames.OpcUaClient, DriverTypeNames.Galaxy }; + var others = DriverTypeNames.All.Except(listed).ToList(); + others.ShouldNotBeEmpty(); + const string blob = "{\"region\":\"Coils\",\"dataType\":\"Int32\"}"; + // Positive control: the same blob IS array-writable under the listed drivers that read it. + DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int32", blob).ShouldBeTrue(); + DriverArrayWriteSupport.Supports(DriverTypeNames.S7, "Int32", blob).ShouldBeTrue(); + foreach (var driverType in others.Append("NoSuchDriver").Append(null)) + { + // A blob every listed driver would accept, so only the driver type can make the answer false. + DriverArrayWriteSupport.Supports(driverType, "Int32", "{\"region\":\"Coils\",\"dataType\":\"Int32\"}") + .ShouldBeFalse(driverType ?? ""); + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyArrayWriteSupportParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyArrayWriteSupportParityTests.cs new file mode 100644 index 00000000..86985574 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyArrayWriteSupportParityTests.cs @@ -0,0 +1,63 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests.Runtime; + +/// +/// Gitea #610 — pins 's Galaxy row against the REAL write encoder +/// (). The OPC UA server hands the driver the CLR array type of the node's +/// DataType (the SDK refuses a mistyped array before the write handler runs), so for every +/// Tag.DataType the server can materialise, the node is advertised writable exactly when the encoder +/// accepts that array type. +/// +[Trait("Category", "Unit")] +public sealed class GalaxyArrayWriteSupportParityTests +{ + // Tag.DataType name → the CLR array the OPC UA stack delivers for a 1-D write of that type. + private static readonly Dictionary ClrArrays = new() + { + ["Boolean"] = new[] { true, false }, + ["SByte"] = new sbyte[] { 1, 2 }, + ["Byte"] = new byte[] { 1, 2 }, + ["Int16"] = new short[] { 1, 2 }, + ["UInt16"] = new ushort[] { 1, 2 }, + ["Int32"] = new[] { 1, 2 }, + ["UInt32"] = new uint[] { 1, 2 }, + ["Int64"] = new long[] { 1, 2 }, + ["UInt64"] = new ulong[] { 1, 2 }, + ["Float"] = new[] { 1f, 2f }, + ["Double"] = new[] { 1d, 2d }, + ["String"] = new[] { "a", "b" }, + ["DateTime"] = new[] { DateTime.UtcNow, DateTime.UtcNow }, + }; + + public static TheoryData DataTypes() => new(ClrArrays.Keys); + + [Theory] + [MemberData(nameof(DataTypes))] + public void Table_agrees_with_MxValueEncoder(string dataType) + { + bool encoderAccepts; + try + { + MxValueEncoder.Encode(ClrArrays[dataType]); + encoderAccepts = true; + } + catch (ArgumentException) + { + encoderAccepts = false; + } + + // UInt32 / UInt64 are a deliberate exclusion: CLR array covariance lets uint[] / ulong[] match the encoder's + // int[] / long[] arms, so it "accepts" them by REINTERPRETING the bits (uint.MaxValue → -1) — unlike the + // scalar arms, which range-check. A Galaxy attribute is never unsigned (DataTypeMap), so the table refuses + // rather than advertise a write that silently corrupts large values. + var expected = encoderAccepts && dataType is not ("UInt32" or "UInt64"); + if (dataType is "UInt32" or "UInt64") encoderAccepts.ShouldBeTrue("the covariance hazard this exclusion documents"); + + DriverArrayWriteSupport.Supports(DriverTypeNames.Galaxy, dataType, "{\"FullName\":\"Obj.Attr\"}") + .ShouldBe(expected, dataType); + } +} 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 new file mode 100644 index 00000000..49e2078f --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs @@ -0,0 +1,58 @@ +using Shouldly; +using Xunit; +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). +/// +[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); + + public static TheoryData AllShapes() + { + var data = new TheoryData(); + foreach (var (region, dt) in Shapes()) data.Add(region, dt); + return data; + } + + [Theory] + [MemberData(nameof(AllShapes))] + public async Task Table_agrees_with_the_driver_write_path(ModbusRegion region, ModbusDataType dataType) + { + var json = $$"""{"region":"{{region}}","address":10,"dataType":"{{dataType}}","stringLength":4,"isArray":true,"arrayLength":2}"""; + var fake = new ModbusDriverTests.FakeTransport(); + var drv = new ModbusDriver( + new ModbusDriverOptions { Host = "fake", RawTags = [new RawTagEntry("cell/modbus/arr", json, false)] }, + "modbus-array-parity", _ => fake); + 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}"); + } + + /// Positive control: the theory above is not vacuously "everything refused". + [Fact] + public void The_matrix_contains_both_answers() + { + var answers = Shapes() + .Select(s => DriverArrayWriteSupport.Supports( + DriverTypeNames.Modbus, "Int16", + $$"""{"region":"{{s.Region}}","address":10,"dataType":"{{s.DataType}}"}""")) + .ToHashSet(); + answers.ShouldBe(new[] { true, false }, ignoreOrder: true); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ArrayWriteSupportParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ArrayWriteSupportParityTests.cs new file mode 100644 index 00000000..90886827 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ArrayWriteSupportParityTests.cs @@ -0,0 +1,49 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// Gitea #610 — pins 's S7 row against the REAL array encoder +/// (). For every , the server advertises an +/// array node writable exactly when the encoder accepts the element type, so a driver change that adds or +/// drops an array element type fails here until the table follows. +/// +[Trait("Category", "Unit")] +public sealed class S7ArrayWriteSupportParityTests +{ + public static TheoryData AllTypes() => new(Enum.GetValues()); + + // An address whose access size matches the element type, so only the element type can make the encoder refuse. + private static string AddressFor(S7DataType dt) => dt switch + { + S7DataType.Bool => "DB1.DBX0.0", + S7DataType.Byte => "DB1.DBB0", + S7DataType.Int16 or S7DataType.UInt16 => "DB1.DBW0", + _ => "DB1.DBD0", + }; + + [Theory] + [MemberData(nameof(AllTypes))] + public void Table_agrees_with_EncodeArrayBlock(S7DataType dataType) + { + var address = AddressFor(dataType); + var tag = new S7TagDefinition("Arr", address, dataType, Writable: true, ArrayCount: 2); + + bool encoderAccepts; + try + { + S7Driver.EncodeArrayBlock(tag, S7AddressParser.Parse(address), new object[] { 1, 1 }); + encoderAccepts = true; + } + catch (NotSupportedException) + { + encoderAccepts = false; + } + + DriverArrayWriteSupport.Supports( + DriverTypeNames.S7, "Int16", $$"""{"address":"{{address}}","dataType":"{{dataType}}","isArray":true,"arrayLength":2}""") + .ShouldBe(encoderAccepts, dataType.ToString()); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ArrayWriteWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ArrayWriteWireTests.cs new file mode 100644 index 00000000..6bca0e58 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ArrayWriteWireTests.cs @@ -0,0 +1,225 @@ +using Microsoft.Extensions.Logging.Abstractions; +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// Gitea #610 — server-side array writes, over the wire. The address space is built by the PRODUCTION +/// composer + applier (not hand-made EnsureVariable calls), so these tests prove the whole chain: +/// DriverArrayWriteSupport → RawTagPlan.ArrayWritable / UnsReferenceVariable.BackingArrayWritable +/// → the node's AccessLevel → a real client write through the realm-qualified WriteOperate gate → +/// the node write gateway (the stand-in for DriverHostActor's routing). +/// +/// An S7 Int16 array (S7 encodes non-Bool arrays as one block) is writable on BOTH its raw node and +/// its UNS mirror, and a whole-array write round-trips. +/// An AbCip DInt array (AbCip has a scalar-only write encoder) stays read-only even though it is +/// authored ReadWrite: the write is BadNotWritable at the node and never reaches the +/// gateway — the "never advertise a write the driver will refuse" rule. +/// A device-refused array write is reverted on whichever NodeId it was written through. +/// A wrong-length array and an IndexRange (partial) write are refused inline, before dispatch, and +/// leave the node holding its prior array. +/// +/// +[Trait("Category", "Integration")] +public sealed class ArrayWriteWireTests +{ + private const string S7ArrayRaw = "Plant/S7a/plc1/Levels"; + private const string AbArrayRaw = "Plant/Ab1/plc2/Counts"; + private const string S7ArrayUns = "filling/line-1/station-1/Levels"; + + private sealed class RecordingWriteGateway(bool succeed) : IOpcUaNodeWriteGateway + { + public List<(string NodeId, object? Value, AddressSpaceRealm Realm)> Writes { get; } = []; + + public Task WriteAsync(string nodeId, object? value, AddressSpaceRealm realm, CancellationToken ct) + { + lock (Writes) Writes.Add((nodeId, value, realm)); + return Task.FromResult(new NodeWriteOutcome(succeed, succeed ? null : "device refused")); + } + + public int Count { get { lock (Writes) return Writes.Count; } } + } + + /// Composes + materialises the two array tags and the UNS mirror of the S7 one, through the + /// production composer and applier, then publishes an initial value to every node. + private static async Task StartAsync(string name, CancellationToken ct) + { + var rig = await ServerEventRig.StartAsync(name, encrypted: false, ct); + + var folder = new RawFolder { RawFolderId = "raw-plant", ClusterId = "c1", Name = "Plant" }; + var s7 = new DriverInstance + { + DriverInstanceId = "drv-s7", ClusterId = "c1", RawFolderId = "raw-plant", + Name = "S7a", DriverType = "S7", DriverConfig = "{}", + }; + var ab = new DriverInstance + { + DriverInstanceId = "drv-ab", ClusterId = "c1", RawFolderId = "raw-plant", + Name = "Ab1", DriverType = "AbCip", DriverConfig = "{}", + }; + var plc1 = new Device { DeviceId = "dev-1", DriverInstanceId = "drv-s7", Name = "plc1", DeviceConfig = "{}" }; + var plc2 = new Device { DeviceId = "dev-2", DriverInstanceId = "drv-ab", Name = "plc2", DeviceConfig = "{}" }; + var levels = new Tag + { + TagId = "t-levels", DeviceId = "dev-1", Name = "Levels", DataType = "Int16", + AccessLevel = TagAccessLevel.ReadWrite, + TagConfig = "{\"address\":\"DB1.DBW0\",\"dataType\":\"Int16\",\"isArray\":true,\"arrayLength\":4}", + }; + var counts = new Tag + { + TagId = "t-counts", DeviceId = "dev-2", Name = "Counts", DataType = "Int32", + AccessLevel = TagAccessLevel.ReadWrite, + TagConfig = "{\"tagPath\":\"Counts\",\"dataType\":\"DInt\",\"isArray\":true,\"arrayLength\":4}", + }; + var area = new UnsArea { UnsAreaId = "area-1", ClusterId = "c1", Name = "filling" }; + var line = new UnsLine { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line-1" }; + var equip = new Equipment { EquipmentId = "eq-1", UnsLineId = "line-1", Name = "station-1", MachineCode = "ST1" }; + var reference = new UnsTagReference { UnsTagReferenceId = "ref-levels", EquipmentId = "eq-1", TagId = "t-levels" }; + + var composition = AddressSpaceComposer.Compose( + [area], [line], [equip], [s7, ab], Array.Empty(), + unsTagReferences: [reference], tags: [levels, counts], + rawFolders: [folder], devices: [plc1, plc2]); + + var applier = new AddressSpaceApplier(rig.Sink, NullLogger.Instance); + applier.MaterialiseHierarchy(composition).ShouldBe(0); + applier.MaterialiseRawSubtree(composition).ShouldBe(0); + applier.MaterialiseUnsReferences(composition).ShouldBe(0); + + var ts = DateTime.UtcNow; + rig.Sink.WriteValue(S7ArrayRaw, new short[] { 1, 2, 3, 4 }, OpcUaStatus.Good, ts, AddressSpaceRealm.Raw); + rig.Sink.WriteValue(S7ArrayUns, new short[] { 1, 2, 3, 4 }, OpcUaStatus.Good, ts, AddressSpaceRealm.Uns); + rig.Sink.WriteValue(AbArrayRaw, new[] { 5, 6, 7, 8 }, OpcUaStatus.Good, ts, AddressSpaceRealm.Raw); + return rig; + } + + private static NodeId Raw(ISession s, string id) => + new(id, (ushort)s.NamespaceUris.GetIndex(V3NodeIds.RawNamespaceUri)); + + private static async Task WriteAsync(ISession session, NodeId node, object value, CancellationToken ct, string? indexRange = null) + { + var response = await session.WriteAsync(null, new WriteValueCollection + { + new WriteValue { NodeId = node, AttributeId = Attributes.Value, IndexRange = indexRange, Value = new DataValue(new Variant(value)) }, + }, ct); + return response.Results.ShouldHaveSingleItem(); + } + + private static async Task ReadShortsAsync(ISession session, NodeId node, CancellationToken ct) => + (short[])(await session.ReadValueAsync(node, ct)).Value; + + private static async Task ReadAccessLevelAsync(ISession session, NodeId node, CancellationToken ct) + { + var response = await session.ReadAsync(null, 0, TimestampsToReturn.Neither, new ReadValueIdCollection + { + new ReadValueId { NodeId = node, AttributeId = Attributes.UserAccessLevel }, + }, ct); + return (byte)response.Results.ShouldHaveSingleItem().Value; + } + + private static async Task WaitForAsync(Func> condition) + { + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10); + while (DateTime.UtcNow < deadline) + { + if (await condition()) return; + await Task.Delay(50); + } + (await condition()).ShouldBeTrue("condition not met within timeout"); + } + + /// A writable-capable array round-trips a whole-array write through BOTH NodeIds. + [Fact] + public async Task A_writable_array_roundtrips_a_whole_array_write_through_both_nodeids() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await StartAsync("array-write", ct); + var gateway = new RecordingWriteGateway(succeed: true); + rig.Nm.NodeWriteGateway = gateway; + using var session = await rig.ConnectAsync(anonymous: false, ct); + var raw = Raw(session, S7ArrayRaw); + var uns = ServerEventRig.UnsNodeId(session, S7ArrayUns); + + (await ReadAccessLevelAsync(session, raw, ct) & AccessLevels.CurrentWrite).ShouldBe(AccessLevels.CurrentWrite); + (await ReadAccessLevelAsync(session, uns, ct) & AccessLevels.CurrentWrite).ShouldBe(AccessLevels.CurrentWrite); + + (await WriteAsync(session, raw, new short[] { 10, 20, 30, 40 }, ct)).ShouldBe((StatusCode)StatusCodes.Good); + (await ReadShortsAsync(session, raw, ct)).ShouldBe(new short[] { 10, 20, 30, 40 }); + + (await WriteAsync(session, uns, new short[] { -1, -2, -3, -4 }, ct)).ShouldBe((StatusCode)StatusCodes.Good); + (await ReadShortsAsync(session, uns, ct)).ShouldBe(new short[] { -1, -2, -3, -4 }); + + lock (gateway.Writes) + { + gateway.Writes.Count.ShouldBe(2); + gateway.Writes[0].Realm.ShouldBe(AddressSpaceRealm.Raw); + gateway.Writes[0].Value.ShouldBe(new short[] { 10, 20, 30, 40 }); + gateway.Writes[1].Realm.ShouldBe(AddressSpaceRealm.Uns); + gateway.Writes[1].Value.ShouldBe(new short[] { -1, -2, -3, -4 }); + } + } + + /// An array on a driver with no array encoder stays read-only though authored ReadWrite. + [Fact] + public async Task An_array_the_driver_cannot_write_is_read_only_and_a_write_is_BadNotWritable() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await StartAsync("array-ro", ct); + var gateway = new RecordingWriteGateway(succeed: true); + rig.Nm.NodeWriteGateway = gateway; + using var session = await rig.ConnectAsync(anonymous: false, ct); + var ab = Raw(session, AbArrayRaw); + + (await ReadAccessLevelAsync(session, ab, ct) & AccessLevels.CurrentWrite).ShouldBe((byte)0); + (await WriteAsync(session, ab, new[] { 50, 60, 70, 80 }, ct)).ShouldBe((StatusCode)StatusCodes.BadNotWritable); + ((int[])(await session.ReadValueAsync(ab, ct)).Value).ShouldBe(new[] { 5, 6, 7, 8 }); + gateway.Count.ShouldBe(0); + + // Positive control: the same session's write to the S7 array is dispatched, so the refusal is the node's. + (await WriteAsync(session, Raw(session, S7ArrayRaw), new short[] { 9, 9, 9, 9 }, ct)).ShouldBe((StatusCode)StatusCodes.Good); + gateway.Count.ShouldBe(1); + } + + /// A device-refused array write reverts the node it was written through, on either NodeId. + [Fact] + public async Task A_device_refused_array_write_is_reverted_through_either_nodeid() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await StartAsync("array-revert", ct); + rig.Nm.NodeWriteGateway = new RecordingWriteGateway(succeed: false); + using var session = await rig.ConnectAsync(anonymous: false, ct); + + foreach (var node in new[] { Raw(session, S7ArrayRaw), ServerEventRig.UnsNodeId(session, S7ArrayUns) }) + { + (await WriteAsync(session, node, new short[] { 99, 99, 99, 99 }, ct)).ShouldBe((StatusCode)StatusCodes.Good); + await WaitForAsync(async () => (await ReadShortsAsync(session, node, ct)).SequenceEqual(new short[] { 1, 2, 3, 4 })); + } + } + + /// A wrong-length array and a partial (IndexRange) write are refused before dispatch. + [Fact] + public async Task A_wrong_length_or_index_range_array_write_is_refused_inline_and_leaves_the_node_intact() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await StartAsync("array-shape", ct); + var gateway = new RecordingWriteGateway(succeed: true); + rig.Nm.NodeWriteGateway = gateway; + using var session = await rig.ConnectAsync(anonymous: false, ct); + var raw = Raw(session, S7ArrayRaw); + + StatusCode.IsBad(await WriteAsync(session, raw, new short[] { 7, 7, 7 }, ct)).ShouldBeTrue("a 3-element write to a 4-element array"); + (await WriteAsync(session, raw, new short[] { 7, 7, 7, 7 }, ct, indexRange: "0:3")).ShouldBe((StatusCode)StatusCodes.BadWriteNotSupported, "an IndexRange write, even one spanning the whole array"); + (await ReadShortsAsync(session, raw, ct)).ShouldBe(new short[] { 1, 2, 3, 4 }); + gateway.Count.ShouldBe(0); + + // Positive control: a correctly-shaped whole-array write on the same node is dispatched. + (await WriteAsync(session, raw, new short[] { 7, 7, 7, 7 }, ct)).ShouldBe((StatusCode)StatusCodes.Good); + gateway.Count.ShouldBe(1); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs index c8f2244f..ce8b3789 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierRawUnsTests.cs @@ -316,6 +316,41 @@ public sealed class AddressSpaceApplierRawUnsTests v.Writable.ShouldBeFalse(); // Writable && !IsArray ⇒ an array mirror is read-only } + /// Gitea #610: an authored-ReadWrite ARRAY materialises writable on the raw node AND its UNS mirror + /// exactly when the driver can write arrays (ArrayWritable / BackingArrayWritable), and + /// read-only otherwise; an authored-Read array stays read-only either way. + [Theory] + [InlineData(true, true, true)] + [InlineData(true, false, false)] + [InlineData(false, true, false)] + public void Array_nodes_are_writable_only_when_the_driver_can_write_arrays(bool authoredWritable, bool arrayWritable, bool expected) + { + var sink = new RealmRecordingSink(); + var composition = new AddressSpaceComposition( + Array.Empty(), Array.Empty(), Array.Empty()) + { + RawTags = new[] + { + new RawTagPlan("t1", "Plant/A/dev/levels", "Plant/A/dev", "drv", "levels", "Int16", + Writable: authoredWritable, Alarm: null, ReferencingEquipmentPaths: Array.Empty(), + IsArray: true, ArrayLength: 4, ArrayWritable: arrayWritable), + }, + UnsReferenceVariables = new[] + { + new UnsReferenceVariable("r1", "EQ-1", "filling/line1/station1/levels", "levels", + "Plant/A/dev/levels", "Int16", Writable: authoredWritable, + BackingIsArray: true, BackingArrayLength: 4, BackingArrayWritable: arrayWritable), + }, + }; + + var applier = NewApplier(sink); + applier.MaterialiseRawSubtree(composition); + applier.MaterialiseUnsReferences(composition); + + sink.Variables.Count.ShouldBe(2); + sink.Variables.ShouldAllBe(v => v.Writable == expected && v.IsArray); + } + /// The historian-resolution semantics carried over verbatim from the old back-lookup: a BLANK /// BackingHistorianTagname on a historized reference falls back to the BackingRawPath (which /// IS the backing raw tag's NodeId, the raw side's own default), and a non-historized reference registers diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs index db71e68d..c7ae412a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs @@ -1727,6 +1727,35 @@ public sealed class AddressSpaceApplierTests sink.RemoveCalls.ShouldBeEmpty(); } + /// Gitea #610: the array-write capability flipping on an already-array tag (its driver gained an + /// array encoder, or the element type moved into the supported set) is applied IN PLACE on the raw node AND + /// its UNS mirror — both become writable — with no rebuild. Without ArrayWritable on the classifier whitelist + /// this edit would tear down every node; without BackingArrayWritable the mirror would stay read-only. + [Fact] + public void Array_write_capability_flip_updates_raw_and_uns_writability_in_place() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var rawBefore = RawTag("Plant/Modbus/dev1/block") with { Writable = true, IsArray = true, ArrayLength = 8 }; + var rawAfter = rawBefore with { ArrayWritable = true }; + var unsBefore = UnsRef("filling/line-1/station-1/block", "block") with { BackingIsArray = true, BackingArrayLength = 8 }; + var unsAfter = unsBefore with { BackingArrayWritable = true }; + + var plan = AddressSpacePlanner.Compute( + CompositionWith(rawBefore, unsBefore), CompositionWith(rawAfter, unsAfter)); + plan.ChangedRawTags.Count.ShouldBe(1); + plan.ChangedUnsReferenceVariables.Count.ShouldBe(1); + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeFalse(); + sink.RebuildCalls.ShouldBe(0); + sink.RemoveCalls.ShouldBeEmpty(); + sink.SurgicalCalls.Single(c => c.Realm == AddressSpaceRealm.Raw).Writable.ShouldBeTrue(); + sink.SurgicalCalls.Single(c => c.Realm == AddressSpaceRealm.Uns).Writable.ShouldBeTrue(); + } + /// The one-way rebuild ratchet holds for the new raw-tag path: a surgical update that reports the /// node missing (false) falls back to a full rebuild. [Fact] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs index 0f388b92..e66e328a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs @@ -97,7 +97,7 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests } /// The in-place-updatable attribute whitelist (Writable / IsHistorized / HistorianTagname / - /// DataType / IsArray / ArrayLength) on a non-alarm raw tag is surgical, not a rebuild. + /// DataType / IsArray / ArrayLength / ArrayWritable) on a non-alarm raw tag is surgical, not a rebuild. [Theory] [InlineData("writable")] [InlineData("historized")] @@ -105,12 +105,14 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests [InlineData("dataType")] [InlineData("array")] [InlineData("arrayLength")] + [InlineData("arrayWritable")] public void Attribute_raw_tag_delta_is_not_a_rebuild(string field) { var baseline = RawTag("Plant/Modbus1/PLC-A/Speed"); // ArrayLength moving ALONE (a resize of an already-array tag) — the whitelist must carry ArrayLength // in its own right, not only as a passenger of an IsArray flip. Its "previous" is already an array. - var prev = field == "arrayLength" ? baseline with { IsArray = true, ArrayLength = 8 } : baseline; + // ArrayWritable (#610) likewise moves on an already-array tag (a driver-type or element-type edit). + var prev = field is "arrayLength" or "arrayWritable" ? baseline with { IsArray = true, ArrayLength = 8 } : baseline; var cur = field switch { "writable" => prev with { Writable = true }, @@ -119,6 +121,7 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests "dataType" => prev with { DataType = "Double" }, "array" => prev with { IsArray = true, ArrayLength = 8 }, "arrayLength" => prev with { ArrayLength = 16 }, + "arrayWritable" => prev with { ArrayWritable = true }, _ => throw new ArgumentOutOfRangeException(nameof(field)), }; 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 f1e51249..d8f2d4b1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs @@ -269,4 +269,42 @@ public sealed class AddressSpaceComposerDualNamespaceTests a.RawTags.ShouldBe(b.RawTags); a.UnsReferenceVariables.ShouldBe(b.UnsReferenceVariables); } + + /// Gitea #610: ArrayWritable is derived per OWNING DRIVER at compose time, and the UNS mirror + /// inherits it as BackingArrayWritable. An S7 Int16 array is writable-capable; the identical shape on + /// an AbCip driver (scalar-only write encoder) is not; a scalar is never flagged. + [Fact] + public void Array_write_capability_follows_the_owning_driver_and_reaches_the_uns_mirror() + { + var folder = new RawFolder { RawFolderId = "f", ClusterId = "c1", Name = "Plant" }; + var s7 = new DriverInstance { DriverInstanceId = "drv-s7", ClusterId = "c1", RawFolderId = "f", Name = "S7a", DriverType = "S7", DriverConfig = "{}" }; + var ab = new DriverInstance { DriverInstanceId = "drv-ab", ClusterId = "c1", RawFolderId = "f", Name = "Ab1", DriverType = "AbCip", DriverConfig = "{}" }; + var d1 = new Device { DeviceId = "d1", DriverInstanceId = "drv-s7", Name = "plc1", DeviceConfig = "{}" }; + var d2 = new Device { DeviceId = "d2", DriverInstanceId = "drv-ab", Name = "plc2", DeviceConfig = "{}" }; + const string s7Array = "{\"address\":\"DB1.DBW0\",\"dataType\":\"Int16\",\"isArray\":true,\"arrayLength\":4}"; + var tags = new[] + { + new Tag { TagId = "t1", DeviceId = "d1", Name = "Levels", DataType = "Int16", AccessLevel = TagAccessLevel.ReadWrite, TagConfig = s7Array }, + new Tag { TagId = "t2", DeviceId = "d1", Name = "Level", DataType = "Int16", AccessLevel = TagAccessLevel.ReadWrite, TagConfig = "{\"address\":\"DB1.DBW8\",\"dataType\":\"Int16\"}" }, + new Tag { TagId = "t3", DeviceId = "d2", Name = "Counts", DataType = "Int32", AccessLevel = TagAccessLevel.ReadWrite, TagConfig = "{\"tagPath\":\"Counts\",\"dataType\":\"DInt\",\"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 = "r1", EquipmentId = "e", TagId = "t1" }, + new UnsTagReference { UnsTagReferenceId = "r3", EquipmentId = "e", TagId = "t3" }, + }; + + var c = AddressSpaceComposer.Compose( + new[] { area }, new[] { line }, new[] { equip }, new[] { s7, ab }, Array.Empty(), + unsTagReferences: refs, tags: tags, rawFolders: new[] { folder }, devices: new[] { d1, d2 }); + + c.RawTags.Single(t => t.Name == "Levels").ArrayWritable.ShouldBeTrue(); + c.RawTags.Single(t => t.Name == "Level").ArrayWritable.ShouldBeFalse(); // scalar: never flagged + c.RawTags.Single(t => t.Name == "Counts").ArrayWritable.ShouldBeFalse(); // AbCip: no array encoder + c.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "r1").BackingArrayWritable.ShouldBeTrue(); + c.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "r3").BackingArrayWritable.ShouldBeFalse(); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/EquipmentWriteGateTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/EquipmentWriteGateTests.cs index c6afe7ed..84d51ea2 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/EquipmentWriteGateTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/EquipmentWriteGateTests.cs @@ -189,6 +189,45 @@ public sealed class EquipmentWriteGateTests OtOpcUaNodeManager.EvaluateEquipmentWriteStructure(value: "any", node).ShouldBeNull(); } + // ───────────────────────────── #610 — array shape + IndexRange ───────────────────────────── + + /// A whole array of the declared length proceeds; a wrong length or a scalar is refused inline. + [Fact] + public void Structure_array_node_takes_only_a_whole_array_of_its_declared_length() + { + var node = ArrayNode(DataTypeIds.Int16, length: 4); + OtOpcUaNodeManager.EvaluateEquipmentWriteStructure(new short[] { 1, 2, 3, 4 }, node).ShouldBeNull(); + OtOpcUaNodeManager.EvaluateEquipmentWriteStructure(new short[] { 1, 2, 3 }, node)!.StatusCode + .ShouldBe((StatusCode)StatusCodes.BadTypeMismatch); + OtOpcUaNodeManager.EvaluateEquipmentWriteStructure((short)1, node)!.StatusCode + .ShouldBe((StatusCode)StatusCodes.BadTypeMismatch); + } + + /// An unbounded array (ArrayDimensions [0]) accepts any length. + [Fact] + public void Structure_unbounded_array_node_accepts_any_length() + { + var node = ArrayNode(DataTypeIds.Int16, length: 0); + OtOpcUaNodeManager.EvaluateEquipmentWriteStructure(new short[] { 1, 2, 3 }, node).ShouldBeNull(); + } + + /// A partial (IndexRange) write is refused; an absent range proceeds. + [Fact] + public void Range_only_whole_value_writes_proceed() + { + OtOpcUaNodeManager.EvaluateEquipmentWriteRange(NumericRange.Empty).ShouldBeNull(); + OtOpcUaNodeManager.EvaluateEquipmentWriteRange(NumericRange.Parse("1"))!.StatusCode + .ShouldBe((StatusCode)StatusCodes.BadWriteNotSupported); + } + + private static BaseDataVariableState ArrayNode(NodeId dataType, uint length) + { + var node = ValueNode(dataType); + node.ValueRank = ValueRanks.OneDimension; + node.ArrayDimensions = new ReadOnlyList(new UInt32Collection(new[] { length })); + return node; + } + private static BaseDataVariableState ValueNode(NodeId dataType) => new(null) { NodeId = new NodeId("eq/x", 2), diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/UnsReferenceBackingParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/UnsReferenceBackingParityTests.cs index 69855bab..cff79373 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/UnsReferenceBackingParityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/UnsReferenceBackingParityTests.cs @@ -86,7 +86,7 @@ public sealed class UnsReferenceBackingParityTests { nameof(RawTagPlan.DataType), nameof(RawTagPlan.Writable), nameof(RawTagPlan.ReferencingEquipmentPaths), nameof(RawTagPlan.IsHistorized), nameof(RawTagPlan.HistorianTagname), nameof(RawTagPlan.IsArray), - nameof(RawTagPlan.ArrayLength), + nameof(RawTagPlan.ArrayLength), nameof(RawTagPlan.ArrayWritable), }, ignoreOrder: true); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs index c80884ed..df65dcdf 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs @@ -49,7 +49,7 @@ public sealed class DeploymentArtifactRawUnsParityTests var profile = new Tag { TagId = "tag-profile", DeviceId = "dev-1", TagGroupId = "grp-1", Name = "Profile", DataType = "Float", - AccessLevel = TagAccessLevel.Read, TagConfig = "{\"isArray\":true,\"arrayLength\":16}", + AccessLevel = TagAccessLevel.Read, TagConfig = "{\"region\":\"HoldingRegisters\",\"address\":100,\"dataType\":\"Float32\",\"isArray\":true,\"arrayLength\":16}", }; var tempHi = new Tag { @@ -82,7 +82,7 @@ public sealed class DeploymentArtifactRawUnsParityTests { new { TagId = "tag-speed", DeviceId = "dev-1", TagGroupId = "grp-1", Name = "Speed", DataType = "Float", AccessLevel = 1, TagConfig = "{\"isHistorized\":true,\"historianTagname\":\"WW.Speed\"}" }, new { TagId = "tag-run", DeviceId = "dev-1", TagGroupId = (string?)null, Name = "Run", DataType = "Boolean", AccessLevel = 0, TagConfig = "{}" }, - new { TagId = "tag-profile", DeviceId = "dev-1", TagGroupId = "grp-1", Name = "Profile", DataType = "Float", AccessLevel = 0, TagConfig = "{\"isArray\":true,\"arrayLength\":16}" }, + new { TagId = "tag-profile", DeviceId = "dev-1", TagGroupId = "grp-1", Name = "Profile", DataType = "Float", AccessLevel = 0, TagConfig = "{\"region\":\"HoldingRegisters\",\"address\":100,\"dataType\":\"Float32\",\"isArray\":true,\"arrayLength\":16}" }, new { TagId = "tag-temphi", DeviceId = "dev-1", TagGroupId = (string?)null, Name = "TempHi", DataType = "Boolean", AccessLevel = 1, TagConfig = "{\"alarm\":{\"alarmType\":\"OffNormalAlarm\",\"severity\":700}}" }, }, UnsAreas = new[] { new { UnsAreaId = "a1", Name = "filling", ClusterId = "c1" } }, @@ -106,6 +106,7 @@ public sealed class DeploymentArtifactRawUnsParityTests var composedArray = composed.RawTags.Single(t => t.Name == "Profile"); composedArray.IsArray.ShouldBeTrue(); composedArray.ArrayLength.ShouldBe(16u); + composedArray.ArrayWritable.ShouldBeTrue(); // #610: a Modbus holding-register Float32 array is writable var composedAlarm = composed.RawTags.Single(t => t.Name == "TempHi"); composedAlarm.Alarm.ShouldNotBeNull(); composedAlarm.Alarm!.AlarmType.ShouldBe("OffNormalAlarm"); @@ -123,6 +124,7 @@ public sealed class DeploymentArtifactRawUnsParityTests var composedProfileRef = composed.UnsReferenceVariables.Single(v => v.UnsTagReferenceId == "ref-2"); composedProfileRef.BackingIsArray.ShouldBeTrue(); composedProfileRef.BackingArrayLength.ShouldBe(16u); + composedProfileRef.BackingArrayWritable.ShouldBeTrue(); composedProfileRef.BackingIsHistorized.ShouldBeFalse(); // Byte-parity: the three dual-namespace node sets are record-equal between the two seams.