From 28328def5d38331179b56a43f4166e72c53b7d30 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 19 Apr 2026 02:00:31 -0400 Subject: [PATCH] Phase 3 PR 73 -- OPC UA Client browse enrichment (DataType + AccessLevel + ValueRank + Historizing). Before this PR discovered variables always registered with DriverDataType.Int32 + SecurityClassification.ViewOnly + IsArray=false as conservative placeholders -- correct wire-format NodeId but useless downstream metadata. PR 73 adds a two-pass browse. Pass 1 unchanged shape but now collects (ParentFolder, BrowseName, DisplayName, NodeId) tuples into a pendingVariables list instead of registering each variable inline; folders still register inline. Pass 2 calls Session.ReadAsync once with (variableCount * 4) ReadValueId entries reading DataType + ValueRank + UserAccessLevel + Historizing for every variable. Server-side chunking via the SDK keeps the request shape within the server's per-request limits automatically. Attribute mapping: MapUpstreamDataType maps every standard DataTypeIds.* to a DriverDataType -- Boolean, SByte+Byte widened to Int16 (DriverDataType has no 8-bit, flagged in comment for future Core.Abstractions widening), Int16/32/64, UInt16/32/64, Float->Float32, Double->Float64, String, DateTime+UtcTime->DateTime. Unknown/vendor-custom NodeIds fall back to String -- safest passthrough for Variant-wrapped structs/enums/extension objects since the cascading-quality path preserves upstream StatusCode+timestamps regardless. MapAccessLevelToSecurityClass reads AccessLevels.CurrentWrite bit (0x02) -- when set, the variable is writable-for-this-user so it surfaces as Operate; otherwise ViewOnly. Uses UserAccessLevel not AccessLevel because UserAccessLevel is post-ACL-filter -- reflects what THIS session can actually do, not the server's default. IsArray derived from ValueRank (-1 = scalar, 0 = 1-D array, 1+ = multi-dim). IsHistorized reflects the server's Historizing flag directly so PR 76's IHistoryProvider routing can gate on it. Graceful degradation: (a) individual attribute failures (Bad StatusCode on DataType read) fall through to the type defaults, variable still registers; (b) wholesale enrichment-read failure (e.g. session dropped mid-browse) catches the exception, registers every pending variable with fallback defaults via RegisterFallback, browse completes. Either way the downstream address space is never empty when browse succeeded the first pass -- partial metadata is strictly better than missing variables. Unit tests (OpcUaClientAttributeMappingTests, 20 facts): MapUpstreamDataType theory covers 11 standard types including Boolean/Int16/UInt16/Int32/UInt32/Int64/UInt64/Float/Double/String/DateTime; separate facts for SByte+Byte (widened to Int16), UtcTime (DateTime), custom NodeId (String fallback); MapAccessLevelToSecurityClass theory covers 6 access-level bitmasks including CurrentRead-only (ViewOnly), CurrentWrite-only (Operate), read+write (Operate), HistoryRead-only (ViewOnly -- no Write bit). 51/51 OpcUaClient.Tests pass (31 prior + 20 new). dotnet build clean. Pending variables structured as a private readonly record struct so the ref-type allocation is stack-local for typical browse sizes. Paves the way for PR 74 SessionReconnectHandler (same enrichment path is re-runnable on reconnect) + PR 76 IHistoryProvider (gates on IsHistorized). --- .../OpcUaClientDriver.cs | 184 ++++++++++++++++-- .../OpcUaClientAttributeMappingTests.cs | 62 ++++++ 2 files changed, 231 insertions(+), 15 deletions(-) create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAttributeMappingTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index e4815d8..1167303 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -581,20 +581,46 @@ public sealed class OpcUaClientDriver(OpcUaClientDriverOptions options, string d var rootFolder = builder.Folder("Remote", "Remote"); var visited = new HashSet(); var discovered = 0; + var pendingVariables = new List(); await _gate.WaitAsync(cancellationToken).ConfigureAwait(false); try { + // Pass 1: browse hierarchy + create folders inline, collect variables into a + // pending list. Defers variable registration until attributes are resolved — the + // address-space builder's Variable call is the one-way commit, so doing it only + // once per variable (with correct DataType/SecurityClass/IsArray) avoids the + // alternative (register with placeholders + mutate later) which the + // IAddressSpaceBuilder contract doesn't expose. await BrowseRecursiveAsync(session, root, rootFolder, visited, - depth: 0, discovered: () => discovered, increment: () => discovered++, + depth: 0, + discovered: () => discovered, increment: () => discovered++, + pendingVariables: pendingVariables, ct: cancellationToken).ConfigureAwait(false); + + // Pass 2: batch-read DataType + AccessLevel + ValueRank + Historizing per + // variable. One wire request for up to ~N variables; for 10k-node servers this is + // still a couple of hundred ms total since the SDK chunks ReadAsync automatically. + await EnrichAndRegisterVariablesAsync(session, pendingVariables, cancellationToken) + .ConfigureAwait(false); } finally { _gate.Release(); } } + /// + /// A variable collected during the browse pass, waiting for attribute enrichment + /// before being registered on the address-space builder. + /// + private readonly record struct PendingVariable( + IAddressSpaceBuilder ParentFolder, + string BrowseName, + string DisplayName, + NodeId NodeId); + private async Task BrowseRecursiveAsync( ISession session, NodeId node, IAddressSpaceBuilder folder, HashSet visited, - int depth, Func discovered, Action increment, CancellationToken ct) + int depth, Func discovered, Action increment, + List pendingVariables, CancellationToken ct) { if (depth >= _options.MaxBrowseDepth) return; if (discovered() >= _options.MaxDiscoveredNodes) return; @@ -650,27 +676,155 @@ public sealed class OpcUaClientDriver(OpcUaClientDriverOptions options, string d var subFolder = folder.Folder(browseName, displayName); increment(); await BrowseRecursiveAsync(session, childId, subFolder, visited, - depth + 1, discovered, increment, ct).ConfigureAwait(false); + depth + 1, discovered, increment, pendingVariables, ct).ConfigureAwait(false); } else if (rf.NodeClass == NodeClass.Variable) { - // Serialize the NodeId so the IReadable/IWritable surface receives a - // round-trippable string. Deferring the DataType + AccessLevel fetch to a - // follow-up PR — initial browse uses a conservative ViewOnly + Int32 default. - var nodeIdString = childId.ToString() ?? string.Empty; - folder.Variable(browseName, displayName, new DriverAttributeInfo( - FullName: nodeIdString, - DriverDataType: DriverDataType.Int32, - IsArray: false, - ArrayDim: null, - SecurityClass: SecurityClassification.ViewOnly, - IsHistorized: false, - IsAlarm: false)); + pendingVariables.Add(new PendingVariable(folder, browseName, displayName, childId)); increment(); } } } + /// + /// Pass 2 of discovery: batch-read DataType + ValueRank + AccessLevel + Historizing + /// for every collected variable in one Session.ReadAsync (the SDK chunks internally + /// to respect the server's per-request limits). Then register each variable on its + /// parent folder with the real . + /// + /// + /// + /// Attributes read: DataType (NodeId of the value type), + /// ValueRank (-1 = scalar, 1 = array), UserAccessLevel (the + /// effective access mask for our session — more accurate than AccessLevel which + /// is the server-side configured mask before user filtering), and + /// Historizing (server flags whether historian data is available). + /// + /// + /// When the upstream server returns Bad on any attribute, the variable falls back + /// to safe defaults (Int32 / ViewOnly / not-array / not-historized) and is still + /// registered — a partial enrichment failure shouldn't drop entire variables from + /// the address space. Operators reading the Admin dashboard see the variable + /// with conservative metadata which is obviously wrong and easy to triage. + /// + /// + private async Task EnrichAndRegisterVariablesAsync( + ISession session, IReadOnlyList pending, CancellationToken ct) + { + if (pending.Count == 0) return; + + // 4 attributes per variable: DataType, ValueRank, UserAccessLevel, Historizing. + var nodesToRead = new ReadValueIdCollection(pending.Count * 4); + foreach (var pv in pending) + { + nodesToRead.Add(new ReadValueId { NodeId = pv.NodeId, AttributeId = Attributes.DataType }); + nodesToRead.Add(new ReadValueId { NodeId = pv.NodeId, AttributeId = Attributes.ValueRank }); + nodesToRead.Add(new ReadValueId { NodeId = pv.NodeId, AttributeId = Attributes.UserAccessLevel }); + nodesToRead.Add(new ReadValueId { NodeId = pv.NodeId, AttributeId = Attributes.Historizing }); + } + + DataValueCollection values; + try + { + var resp = await session.ReadAsync( + requestHeader: null, + maxAge: 0, + timestampsToReturn: TimestampsToReturn.Neither, + nodesToRead: nodesToRead, + ct: ct).ConfigureAwait(false); + values = resp.Results; + } + catch + { + // Enrichment-read failed wholesale (server unreachable mid-browse). Register the + // pending variables with conservative defaults rather than dropping them — the + // downstream catalog is still useful for reading via IReadable. + foreach (var pv in pending) + RegisterFallback(pv); + return; + } + + for (var i = 0; i < pending.Count; i++) + { + var pv = pending[i]; + var baseIdx = i * 4; + var dataTypeDv = values[baseIdx]; + var valueRankDv = values[baseIdx + 1]; + var accessDv = values[baseIdx + 2]; + var histDv = values[baseIdx + 3]; + + var dataType = StatusCode.IsGood(dataTypeDv.StatusCode) && dataTypeDv.Value is NodeId dtId + ? MapUpstreamDataType(dtId) + : DriverDataType.Int32; + var valueRank = StatusCode.IsGood(valueRankDv.StatusCode) && valueRankDv.Value is int vr ? vr : -1; + var isArray = valueRank >= 0; // -1 = scalar; 1+ = array dimensions; 0 = one-dimensional array + var access = StatusCode.IsGood(accessDv.StatusCode) && accessDv.Value is byte ab ? ab : (byte)0; + var securityClass = MapAccessLevelToSecurityClass(access); + var historizing = StatusCode.IsGood(histDv.StatusCode) && histDv.Value is bool b && b; + + pv.ParentFolder.Variable(pv.BrowseName, pv.DisplayName, new DriverAttributeInfo( + FullName: pv.NodeId.ToString() ?? string.Empty, + DriverDataType: dataType, + IsArray: isArray, + ArrayDim: null, + SecurityClass: securityClass, + IsHistorized: historizing, + IsAlarm: false)); + } + + void RegisterFallback(PendingVariable pv) + { + pv.ParentFolder.Variable(pv.BrowseName, pv.DisplayName, new DriverAttributeInfo( + FullName: pv.NodeId.ToString() ?? string.Empty, + DriverDataType: DriverDataType.Int32, + IsArray: false, + ArrayDim: null, + SecurityClass: SecurityClassification.ViewOnly, + IsHistorized: false, + IsAlarm: false)); + } + } + + /// + /// Map an upstream OPC UA built-in DataType NodeId (via DataTypeIds.*) to a + /// . Unknown / custom types fall through to + /// which is the safest passthrough for + /// Variant-wrapped structs + enums + extension objects; downstream clients see a + /// string rendering but the cascading-quality path still preserves upstream + /// StatusCode + timestamps. + /// + internal static DriverDataType MapUpstreamDataType(NodeId dataType) + { + if (dataType == DataTypeIds.Boolean) return DriverDataType.Boolean; + if (dataType == DataTypeIds.SByte || dataType == DataTypeIds.Byte || + dataType == DataTypeIds.Int16) return DriverDataType.Int16; + if (dataType == DataTypeIds.UInt16) return DriverDataType.UInt16; + if (dataType == DataTypeIds.Int32) return DriverDataType.Int32; + if (dataType == DataTypeIds.UInt32) return DriverDataType.UInt32; + if (dataType == DataTypeIds.Int64) return DriverDataType.Int64; + if (dataType == DataTypeIds.UInt64) return DriverDataType.UInt64; + if (dataType == DataTypeIds.Float) return DriverDataType.Float32; + if (dataType == DataTypeIds.Double) return DriverDataType.Float64; + if (dataType == DataTypeIds.String) return DriverDataType.String; + if (dataType == DataTypeIds.DateTime || dataType == DataTypeIds.UtcTime) + return DriverDataType.DateTime; + return DriverDataType.String; + } + + /// + /// Map an OPC UA AccessLevel/UserAccessLevel attribute value (AccessLevels + /// bitmask) to a the local node-manager's ACL + /// layer can gate writes off. CurrentWrite-capable variables surface as + /// ; read-only as . + /// + internal static SecurityClassification MapAccessLevelToSecurityClass(byte accessLevel) + { + const byte CurrentWrite = 2; // AccessLevels.CurrentWrite = 0x02 + return (accessLevel & CurrentWrite) != 0 + ? SecurityClassification.Operate + : SecurityClassification.ViewOnly; + } + // ---- ISubscribable ---- public async Task SubscribeAsync( diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAttributeMappingTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAttributeMappingTests.cs new file mode 100644 index 0000000..700ef4a --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAttributeMappingTests.cs @@ -0,0 +1,62 @@ +using Opc.Ua; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; + +[Trait("Category", "Unit")] +public sealed class OpcUaClientAttributeMappingTests +{ + [Theory] + [InlineData((uint)DataTypes.Boolean, DriverDataType.Boolean)] + [InlineData((uint)DataTypes.Int16, DriverDataType.Int16)] + [InlineData((uint)DataTypes.UInt16, DriverDataType.UInt16)] + [InlineData((uint)DataTypes.Int32, DriverDataType.Int32)] + [InlineData((uint)DataTypes.UInt32, DriverDataType.UInt32)] + [InlineData((uint)DataTypes.Int64, DriverDataType.Int64)] + [InlineData((uint)DataTypes.UInt64, DriverDataType.UInt64)] + [InlineData((uint)DataTypes.Float, DriverDataType.Float32)] + [InlineData((uint)DataTypes.Double, DriverDataType.Float64)] + [InlineData((uint)DataTypes.String, DriverDataType.String)] + [InlineData((uint)DataTypes.DateTime, DriverDataType.DateTime)] + public void MapUpstreamDataType_recognizes_standard_builtin_types(uint typeId, DriverDataType expected) + { + var nodeId = new NodeId(typeId); + OpcUaClientDriver.MapUpstreamDataType(nodeId).ShouldBe(expected); + } + + [Fact] + public void MapUpstreamDataType_maps_SByte_and_Byte_to_Int16_since_DriverDataType_lacks_8bit() + { + // DriverDataType has no 8-bit type; conservative widen to Int16. Documented so a + // future Core.Abstractions PR that adds Int8/Byte can find this call site. + OpcUaClientDriver.MapUpstreamDataType(new NodeId((uint)DataTypes.SByte)).ShouldBe(DriverDataType.Int16); + OpcUaClientDriver.MapUpstreamDataType(new NodeId((uint)DataTypes.Byte)).ShouldBe(DriverDataType.Int16); + } + + [Fact] + public void MapUpstreamDataType_falls_back_to_String_for_unknown_custom_types() + { + // Custom vendor extension object — NodeId in namespace 2 that isn't a standard type. + OpcUaClientDriver.MapUpstreamDataType(new NodeId("CustomStruct", 2)).ShouldBe(DriverDataType.String); + } + + [Fact] + public void MapUpstreamDataType_handles_UtcTime_as_DateTime() + { + OpcUaClientDriver.MapUpstreamDataType(new NodeId((uint)DataTypes.UtcTime)).ShouldBe(DriverDataType.DateTime); + } + + [Theory] + [InlineData((byte)0, SecurityClassification.ViewOnly)] // no access flags set + [InlineData((byte)1, SecurityClassification.ViewOnly)] // CurrentRead only + [InlineData((byte)2, SecurityClassification.Operate)] // CurrentWrite only + [InlineData((byte)3, SecurityClassification.Operate)] // CurrentRead + CurrentWrite + [InlineData((byte)0x0F, SecurityClassification.Operate)] // read+write+historyRead+historyWrite + [InlineData((byte)0x04, SecurityClassification.ViewOnly)] // HistoryRead only — no Write bit + public void MapAccessLevelToSecurityClass_respects_CurrentWrite_bit(byte accessLevel, SecurityClassification expected) + { + OpcUaClientDriver.MapAccessLevelToSecurityClass(accessLevel).ShouldBe(expected); + } +}