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<NodeId>();
         var discovered = 0;
+        var pendingVariables = new List<PendingVariable>();
 
         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(); }
     }
 
+    /// <summary>
+    ///     A variable collected during the browse pass, waiting for attribute enrichment
+    ///     before being registered on the address-space builder.
+    /// </summary>
+    private readonly record struct PendingVariable(
+        IAddressSpaceBuilder ParentFolder,
+        string BrowseName,
+        string DisplayName,
+        NodeId NodeId);
+
     private async Task BrowseRecursiveAsync(
         ISession session, NodeId node, IAddressSpaceBuilder folder, HashSet<NodeId> visited,
-        int depth, Func<int> discovered, Action increment, CancellationToken ct)
+        int depth, Func<int> discovered, Action increment,
+        List<PendingVariable> pendingVariables, CancellationToken ct)
     {
         if (depth >= _options.MaxBrowseDepth) return;
         if (discovered() >= _options.MaxDiscoveredNodes) return;
@@ -650,25 +676,153 @@ 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
+                pendingVariables.Add(new PendingVariable(folder, browseName, displayName, childId));
-                // round-trippable string. Deferring the DataType + AccessLevel fetch to a
+                increment();
-                // 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,
+
+    /// <summary>
+    ///     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 <see cref="DriverAttributeInfo"/>.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         Attributes read: <c>DataType</c> (NodeId of the value type),
+    ///         <c>ValueRank</c> (-1 = scalar, 1 = array), <c>UserAccessLevel</c> (the
+    ///         effective access mask for our session — more accurate than AccessLevel which
+    ///         is the server-side configured mask before user filtering), and
+    ///         <c>Historizing</c> (server flags whether historian data is available).
+    ///     </para>
+    ///     <para>
+    ///         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.
+    ///     </para>
+    /// </remarks>
+    private async Task EnrichAndRegisterVariablesAsync(
+        ISession session, IReadOnlyList<PendingVariable> 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));
-                increment();
         }
     }
+
+    /// <summary>
+    ///     Map an upstream OPC UA built-in DataType NodeId (via <c>DataTypeIds.*</c>) to a
+    ///     <see cref="DriverDataType"/>. Unknown / custom types fall through to
+    ///     <see cref="DriverDataType.String"/> 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.
+    /// </summary>
+    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;
+    }
+
+    /// <summary>
+    ///     Map an OPC UA AccessLevel/UserAccessLevel attribute value (<c>AccessLevels</c>
+    ///     bitmask) to a <see cref="SecurityClassification"/> the local node-manager's ACL
+    ///     layer can gate writes off. CurrentWrite-capable variables surface as
+    ///     <see cref="SecurityClassification.Operate"/>; read-only as <see cref="SecurityClassification.ViewOnly"/>.
+    /// </summary>
+    internal static SecurityClassification MapAccessLevelToSecurityClass(byte accessLevel)
+    {
+        const byte CurrentWrite = 2; // AccessLevels.CurrentWrite = 0x02
+        return (accessLevel & CurrentWrite) != 0
+            ? SecurityClassification.Operate
+            : SecurityClassification.ViewOnly;
     }
 
     // ---- ISubscribable ----

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