Auto: twincat-4.1 — nested UDT browse via online type walker

Closes #315

src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs

@@ -485,21 +485,39 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
     }
 
     public async IAsyncEnumerable<TwinCATDiscoveredSymbol> BrowseSymbolsAsync(
+        int maxArrayExpansion,
         [EnumeratorCancellation] CancellationToken cancellationToken)
     {
         // SymbolLoaderFactory downloads the symbol-info blob once then iterates locally — the
         // async surface on this interface is for our callers, not for the underlying call which
         // is effectively sync on top of the already-open AdsClient.
-        var settings = new SymbolLoaderSettings(SymbolsLoadMode.Flat);
+        //
+        // PR 4.1 / #315 — switched from VirtualView-flat (top-level only) to VirtualTree so the
+        // loader hands us each top-level symbol with its full IDataType graph wired up. We then
+        // run TwinCATTypeWalker against each root to flatten structs / arrays into per-leaf
+        // entries. Atomic top-level symbols still surface as a single leaf — the walker treats
+        // a primitive root as a 1-leaf walk.
+        var settings = new SymbolLoaderSettings(SymbolsLoadMode.VirtualTree);
         var loader = SymbolLoaderFactory.Create(_client, settings);
         await Task.Yield(); // honors the async surface; pragmatic given the loader itself is sync
 
+        // The walker only needs MaxArrayExpansion off TwinCATDriverOptions — synthesise a
+        // throwaway options instance with just that field set so the walker doesn't gain a
+        // direct dependency on TwinCATDriverOptions.
+        var walkerOptions = new TwinCATDriverOptions { MaxArrayExpansion = Math.Max(1, maxArrayExpansion) };
+
         foreach (ISymbol symbol in loader.Symbols)
         {
             if (cancellationToken.IsCancellationRequested) yield break;
-            var mapped = ResolveSymbolDataType(symbol.DataType);
             var readOnly = !IsSymbolWritable(symbol);
-            yield return new TwinCATDiscoveredSymbol(symbol.InstancePath, mapped, readOnly);
+            foreach (var leaf in TwinCATTypeWalker.Walk(
+                symbol.DataType, symbol.InstancePath, offsetRoot: 0, readOnly, walkerOptions))
+            {
+                if (cancellationToken.IsCancellationRequested) yield break;
+                yield return new TwinCATDiscoveredSymbol(
+                    leaf.InstancePath, leaf.AtomicType, leaf.ReadOnly,
+                    leaf.IsArrayRoot, leaf.ArrayLength);
+            }
         }
     }
 

src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs

@@ -115,13 +115,22 @@ public interface ITwinCATClient : IDisposable
         CancellationToken cancellationToken);
 
     /// <summary>
-    ///     Walk the target's symbol table via the TwinCAT <c>SymbolLoaderFactory</c> (flat mode).
-    ///     Yields each top-level symbol the PLC exposes — global variables, program-scope locals,
-    ///     function-block instance fields. Filters for our atomic type surface; structured /
-    ///     UDT / function-block typed symbols surface with <c>DataType = null</c> so callers can
-    ///     decide whether to drill in via their own walker.
+    ///     Walk the target's symbol table via the TwinCAT <c>SymbolLoaderFactory</c> (flat mode)
+    ///     and yield one <see cref="TwinCATDiscoveredSymbol"/> per atomic leaf. PR 4.1 / #315
+    ///     extended this to recurse into struct / UDT members and array elements via
+    ///     <see cref="TwinCATTypeWalker"/> so callers see <c>MyStruct.Inner.Field</c> /
+    ///     <c>aTags[3]</c> rows instead of the parent symbol with <c>DataType = null</c>.
     /// </summary>
-    IAsyncEnumerable<TwinCATDiscoveredSymbol> BrowseSymbolsAsync(CancellationToken cancellationToken);
+    /// <param name="maxArrayExpansion">
+    ///     Upper bound on per-element expansion for array-typed members. Arrays whose element
+    ///     count exceeds this value surface as a single whole-array root with
+    ///     <see cref="TwinCATDiscoveredSymbol.IsArrayRoot"/> set instead of N individual
+    ///     <c>Path[i]</c> rows.
+    /// </param>
+    /// <param name="cancellationToken">Cancels the enumeration; in-flight loader downloads still complete.</param>
+    IAsyncEnumerable<TwinCATDiscoveredSymbol> BrowseSymbolsAsync(
+        int maxArrayExpansion,
+        CancellationToken cancellationToken);
 
     /// <summary>
     ///     PR 2.2 — wipe process-scoped optional caches (today: the ADS variable-handle
@@ -139,16 +148,28 @@ public interface ITwinCATNotificationHandle : IDisposable { }
 
 /// <summary>
 ///     One symbol yielded by <see cref="ITwinCATClient.BrowseSymbolsAsync"/> — full instance
-///     path + detected <see cref="TwinCATDataType"/> + read-only flag.
+///     path + detected <see cref="TwinCATDataType"/> + read-only flag. PR 4.1 / #315 added
+///     <see cref="IsArrayRoot"/> + <see cref="ArrayLength"/> so the discovery layer can
+///     surface arrays that exceeded <see cref="TwinCATDriverOptions.MaxArrayExpansion"/>
+///     as whole-array tags instead of per-element entries.
 /// </summary>
-/// <param name="InstancePath">Full dotted symbol path (e.g. <c>MAIN.bStart</c>, <c>GVL.Counter</c>).</param>
-/// <param name="DataType">Mapped <see cref="TwinCATDataType"/>; <c>null</c> when the symbol's type
-///     doesn't map onto our supported atomic surface (UDTs, pointers, function blocks).</param>
+/// <param name="InstancePath">Full dotted symbol path (e.g. <c>MAIN.bStart</c>, <c>GVL.Counter</c>,
+///     <c>MyStruct.Inner.Field</c>, <c>aTags[3]</c>).</param>
+/// <param name="DataType">Mapped <see cref="TwinCATDataType"/>; <c>null</c> when the symbol's
+///     type doesn't map onto our supported atomic surface (struct-typed array root over the
+///     expansion cap, pointer / reference / function-block instance).</param>
 /// <param name="ReadOnly"><c>true</c> when the symbol's AccessRights flag forbids writes.</param>
+/// <param name="IsArrayRoot"><c>true</c> when the symbol represents an array whose element count
+///     exceeded <see cref="TwinCATDriverOptions.MaxArrayExpansion"/>; the caller should surface
+///     it as a whole-array tag rather than per-element.</param>
+/// <param name="ArrayLength">Element count when <see cref="IsArrayRoot"/> is <c>true</c>;
+///     <c>null</c> otherwise.</param>
 public sealed record TwinCATDiscoveredSymbol(
     string InstancePath,
     TwinCATDataType? DataType,
-    bool ReadOnly);
+    bool ReadOnly,
+    bool IsArrayRoot = false,
+    int? ArrayLength = null);
 
 /// <summary>Factory for <see cref="ITwinCATClient"/>s. One client per device.</summary>
 public interface ITwinCATClientFactory

src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs

@@ -408,23 +408,42 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
 
             // Controller-side symbol browse — opt-in. Falls back to pre-declared-only on any
             // client-side error so a flaky symbol-table download doesn't block discovery.
+            //
+            // PR 4.1 / #315 — the symbol stream now contains one entry per atomic leaf rather
+            // than per top-level symbol. Dotted paths (`MyStruct.Inner.Field`) split into
+            // nested folders so the OPC UA browse tree mirrors the PLC's UDT hierarchy. Array
+            // elements (`aTags[3]`) stay as variable leaves under the parent folder — they
+            // don't get an extra folder level because that would explode the tree for every
+            // array element while adding no useful navigation.
             if (_options.EnableControllerBrowse && _devices.TryGetValue(device.HostAddress, out var state))
             {
                 IAddressSpaceBuilder? discoveredFolder = null;
+                // Cached per-path folder builders so a struct with N members shares the parent
+                // folder rather than creating it N times. Keyed on the full dotted prefix.
+                var folderCache = new Dictionary<string, IAddressSpaceBuilder>(StringComparer.Ordinal);
                 try
                 {
                     var client = await EnsureConnectedAsync(state, cancellationToken).ConfigureAwait(false);
-                    await foreach (var sym in client.BrowseSymbolsAsync(cancellationToken).ConfigureAwait(false))
+                    await foreach (var sym in client.BrowseSymbolsAsync(_options.MaxArrayExpansion, cancellationToken).ConfigureAwait(false))
                     {
                         if (TwinCATSystemSymbolFilter.IsSystemSymbol(sym.InstancePath)) continue;
-                        if (sym.DataType is not TwinCATDataType dt) continue; // unsupported type
+
+                        // Array root over the expansion cap surfaces with IsArrayRoot=true; the
+                        // element type may or may not be atomic. Skip unless the element is
+                        // atomic — there's no useful read shape for a struct-of-N>cap today.
+                        if (sym.IsArrayRoot && sym.DataType is null) continue;
+                        if (sym.DataType is not TwinCATDataType dt) continue; // unsupported leaf type
 
                         discoveredFolder ??= deviceFolder.Folder("Discovered", "Discovered");
-                        discoveredFolder.Variable(sym.InstancePath, sym.InstancePath, new DriverAttributeInfo(
+                        var (parentBuilder, leafName) = ResolveLeafFolder(
+                            discoveredFolder, sym.InstancePath, folderCache);
+
+                        parentBuilder.Variable(leafName, leafName, new DriverAttributeInfo(
                             FullName: sym.InstancePath,
                             DriverDataType: dt.ToDriverDataType(),
-                            IsArray: false,
-                            ArrayDim: null,
+                            IsArray: sym.IsArrayRoot,
+                            ArrayDim: sym.IsArrayRoot && sym.ArrayLength is int n && n > 0
+                                ? (uint)n : null,
                             SecurityClass: sym.ReadOnly
                                 ? SecurityClassification.ViewOnly
                                 : SecurityClassification.Operate,
@@ -443,6 +462,48 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
         }
     }
 
+    /// <summary>
+    ///     PR 4.1 / #315 — split a dotted instance path into (parent folder, leaf segment).
+    ///     Each interior segment maps to a folder; the final segment is the variable name.
+    ///     Folders are cached so two members of the same struct share the parent. Array
+    ///     subscripts on interior segments (e.g. <c>aMotors[0].Status.Running</c>) keep the
+    ///     bracketed segment as a single folder name — the OPC UA browse name preserves the
+    ///     array-element identifier.
+    /// </summary>
+    internal static (IAddressSpaceBuilder Parent, string LeafName) ResolveLeafFolder(
+        IAddressSpaceBuilder root, string instancePath,
+        Dictionary<string, IAddressSpaceBuilder> folderCache)
+    {
+        var lastDot = instancePath.LastIndexOf('.');
+        if (lastDot < 0)
+            return (root, instancePath); // top-level — no parent folder
+
+        var prefix = instancePath.Substring(0, lastDot);
+        var leaf = instancePath.Substring(lastDot + 1);
+
+        if (folderCache.TryGetValue(prefix, out var cached))
+            return (cached, leaf);
+
+        // Walk segment-by-segment, materialising each folder once. Reuse cached prefixes so
+        // sibling members share the same parent.
+        var segments = prefix.Split('.');
+        var current = root;
+        var sb = new System.Text.StringBuilder();
+        foreach (var seg in segments)
+        {
+            if (sb.Length > 0) sb.Append('.');
+            sb.Append(seg);
+            var key = sb.ToString();
+            if (!folderCache.TryGetValue(key, out var folder))
+            {
+                folder = current.Folder(seg, seg);
+                folderCache[key] = folder;
+            }
+            current = folder;
+        }
+        return (current, leaf);
+    }
+
     // ---- ISubscribable (native ADS notifications with poll fallback) ----
 
     private readonly ConcurrentDictionary<long, NativeSubscription> _nativeSubs = new();

src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverOptions.cs

@@ -32,6 +32,18 @@ public sealed class TwinCATDriverOptions
     ///     the strict-config path for deployments where only declared tags should appear.
     /// </summary>
     public bool EnableControllerBrowse { get; init; }
+
+    /// <summary>
+    ///     PR 4.1 / #315 — upper bound on per-element array expansion during nested-UDT browse.
+    ///     Arrays whose total <c>ElementCount</c> exceeds this value surface as a single
+    ///     whole-array root leaf instead of N individual <c>Path[i]</c> entries; that keeps
+    ///     pathological multi-thousand-element arrays from inflating the discovered address
+    ///     space. The default of <c>1024</c> covers typical recipe / lookup tables; raise it
+    ///     when the PLC routinely ships larger arrays whose individual elements are operationally
+    ///     interesting, lower it to keep discovery cheap. Has no effect on whole-array tags
+    ///     declared in <see cref="Tags"/> (those bypass the walker entirely).
+    /// </summary>
+    public int MaxArrayExpansion { get; init; } = 1024;
 }
 
 /// <summary>

src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATTypeWalker.cs

@@ -0,0 +1,233 @@
+using TwinCAT.TypeSystem;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT;
+
+/// <summary>
+///     PR 4.1 / #315 — pure helper that walks a TwinCAT <see cref="IDataType"/> tree starting at
+///     a top-level instance and yields one <see cref="DiscoveredLeaf"/> per atomic leaf along
+///     the dotted instance path. Lets <see cref="AdsTwinCATClient.BrowseSymbolsAsync"/> expose
+///     UDT members as individually-readable OPC UA variable nodes instead of dropping the whole
+///     symbol the way PR 1.5 did.
+/// </summary>
+/// <remarks>
+///     <para><b>Scope</b> — IEC primitives + <c>STRING</c>/<c>WSTRING</c> + IEC time/date types
+///     resolve to a leaf; aliases recurse into their base type; structs recurse into each
+///     member; arrays expand element-by-element up to <see cref="TwinCATDriverOptions.MaxArrayExpansion"/>;
+///     pointers / references / unions / function-block instances / interfaces are skipped.</para>
+///
+///     <para><b>Recursion guard</b> — the walker tracks visited <see cref="IDataType"/> instances
+///     plus a hard depth cap (<see cref="MaxDepth"/>) so a self-pointer or cyclic typedef
+///     terminates rather than blowing the stack.</para>
+///
+///     <para><b>Array policy</b> — when an array's <c>ElementCount &gt; MaxArrayExpansion</c>
+///     a single leaf is emitted with <see cref="DiscoveredLeaf.IsArrayRoot"/> set so the
+///     caller can surface the array root as a whole-array tag (the existing 1.4 read-path
+///     handles that). Arrays-of-structs follow the same cap — over the limit, only the
+///     root surfaces.</para>
+///
+///     <para><b>Offset</b> — every leaf carries the cumulative byte offset from the symbol
+///     root. Not used by the current discovery path (which still resolves writes/reads by
+///     symbolic name) but PR 4.x will consume it for offset-based bulk reads.</para>
+/// </remarks>
+internal static class TwinCATTypeWalker
+{
+    /// <summary>Hard recursion cap — kicks in before <see cref="System.StackOverflowException"/>.</summary>
+    internal const int MaxDepth = 8;
+
+    /// <summary>
+    ///     One atomic leaf yielded by <see cref="Walk"/>. Either an atomic-typed scalar / array
+    ///     element (<see cref="IsArrayRoot"/> = false, <see cref="AtomicType"/> non-null) or a
+    ///     "too-large array" placeholder where the element count exceeded
+    ///     <see cref="TwinCATDriverOptions.MaxArrayExpansion"/> (<see cref="IsArrayRoot"/> = true
+    ///     + <see cref="ArrayLength"/> set).
+    /// </summary>
+    /// <param name="InstancePath">Dotted symbol path (e.g. <c>MAIN.Recipe.Step</c>, <c>GVL.Tags[3]</c>).</param>
+    /// <param name="AtomicType">Mapped atomic <see cref="TwinCATDataType"/>; <c>null</c> only when
+    ///     <see cref="IsArrayRoot"/> is <c>true</c> and the element type itself is a struct.</param>
+    /// <param name="Offset">Byte offset of this leaf relative to the walked root.</param>
+    /// <param name="ReadOnly"><c>true</c> when the parent symbol forbids writes.</param>
+    /// <param name="IsArrayRoot"><c>true</c> when this leaf represents an array whose element
+    ///     count exceeded the configured expansion limit; the caller should expose it as a
+    ///     whole-array tag rather than per-element.</param>
+    /// <param name="ArrayLength">Element count when <see cref="IsArrayRoot"/> is <c>true</c>.</param>
+    public sealed record DiscoveredLeaf(
+        string InstancePath,
+        TwinCATDataType? AtomicType,
+        int Offset,
+        bool ReadOnly,
+        bool IsArrayRoot = false,
+        int? ArrayLength = null);
+
+    /// <summary>
+    ///     Walk <paramref name="root"/> and yield every atomic leaf reachable from it. Path
+    ///     prefixes start at <paramref name="instancePathRoot"/>; offsets accumulate from
+    ///     <paramref name="offsetRoot"/>.
+    /// </summary>
+    public static IEnumerable<DiscoveredLeaf> Walk(
+        IDataType? root,
+        string instancePathRoot,
+        int offsetRoot,
+        bool readOnly,
+        TwinCATDriverOptions options)
+    {
+        ArgumentNullException.ThrowIfNull(options);
+        if (root is null) yield break;
+
+        var visited = new HashSet<IDataType>(ReferenceEqualityComparer.Instance);
+        foreach (var leaf in WalkInner(root, instancePathRoot, offsetRoot, readOnly, options, visited, depth: 0))
+            yield return leaf;
+    }
+
+    private static IEnumerable<DiscoveredLeaf> WalkInner(
+        IDataType type,
+        string instancePath,
+        int offset,
+        bool readOnly,
+        TwinCATDriverOptions options,
+        HashSet<IDataType> visited,
+        int depth)
+    {
+        if (depth >= MaxDepth) yield break; // depth cap — self-pointer / cyclic alias defence
+        if (!visited.Add(type)) yield break; // already on the recursion stack — break the cycle
+
+        try
+        {
+            // Pointers / references — out of scope. Surfacing them would require dereferencing
+            // through the AMS routing layer which has its own access patterns. The Beckhoff
+            // 7.x stack moved the IsPointer / IsReference checks off IDataType onto the
+            // DataTypeExtension static helpers; using DataTypeCategory directly avoids the
+            // obsolete-warning and is the same wire-level signal.
+            if (type.Category is DataTypeCategory.Pointer or DataTypeCategory.Reference) yield break;
+
+            // Aliases (incl. enums, since IEnumType : IAliasType) — recurse into BaseType
+            // without changing the path / offset. Atomic terminus is handled when the base
+            // hits a primitive.
+            if (type.Category is DataTypeCategory.Alias or DataTypeCategory.Enum
+                && type is IAliasType alias && alias.BaseType is not null)
+            {
+                foreach (var leaf in WalkInner(alias.BaseType, instancePath, offset, readOnly, options, visited, depth + 1))
+                    yield return leaf;
+                yield break;
+            }
+
+            // Atomic primitives + strings — terminate. AdsTwinCATClient.MapSymbolTypeName is the
+            // single source of truth for IEC name → enum mapping; reuse it so alias chains that
+            // bottom out at primitives match the same surface as flat-mode browse.
+            if (type.Category is DataTypeCategory.Primitive or DataTypeCategory.String)
+            {
+                var atomic = AdsTwinCATClient.ResolveSymbolDataType(type);
+                if (atomic is TwinCATDataType dt)
+                    yield return new DiscoveredLeaf(instancePath, dt, offset, readOnly);
+                // Unknown primitive name — drop. Caller still gets the rest of the tree.
+                yield break;
+            }
+
+            // Structs — recurse into each member, appending ".Name" to the path and adding the
+            // member's byte offset. IStructType.AllMembers walks the inheritance chain so derived
+            // FB / struct types still surface members from the base — Members alone would miss them.
+            if (type is IStructType structType)
+            {
+                var members = structType.AllMembers ?? structType.Members;
+                if (members is null) yield break;
+                foreach (var member in members)
+                {
+                    if (member?.DataType is null) continue;
+                    var childPath = string.IsNullOrEmpty(instancePath)
+                        ? member.InstanceName
+                        : instancePath + "." + member.InstanceName;
+                    var childOffset = offset + member.ByteOffset;
+                    foreach (var leaf in WalkInner(member.DataType, childPath, childOffset, readOnly, options, visited, depth + 1))
+                        yield return leaf;
+                }
+                yield break;
+            }
+
+            // Arrays — element-by-element expansion up to MaxArrayExpansion. Arrays of atoms
+            // produce N indexed leaves; arrays of structs produce N × (struct-member-count)
+            // leaves, also bounded by the cap. Multi-dim arrays surface as ElementCount in
+            // row-major order; a follow-up PR can break that out into per-dim indices if
+            // operators ever need them.
+            if (type is IArrayType arrayType)
+            {
+                var elementType = arrayType.ElementType;
+                var dims = arrayType.Dimensions;
+                if (elementType is null || dims is null)
+                    yield break;
+
+                var elementCount = dims.ElementCount;
+                if (elementCount <= 0)
+                    yield break;
+
+                if (elementCount > options.MaxArrayExpansion)
+                {
+                    // Over the cap — surface only the array root. Element type may be atomic or
+                    // structured; for atomic element types we set AtomicType so the caller can
+                    // configure a whole-array read against it. For struct element types we
+                    // leave AtomicType null + flag IsArrayRoot — the address-space layer
+                    // typically renders this as a folder placeholder.
+                    var rootAtomic = AdsTwinCATClient.ResolveSymbolDataType(elementType);
+                    yield return new DiscoveredLeaf(
+                        instancePath, rootAtomic, offset, readOnly,
+                        IsArrayRoot: true, ArrayLength: elementCount);
+                    yield break;
+                }
+
+                var elementSize = Math.Max(0, elementType.ByteSize);
+                var lowerBounds = dims.LowerBounds;
+                var dimLengths = dims.GetDimensionLengths();
+                // Multi-dim — emit per-element with row-major index linearisation. Single-dim
+                // is just the trivial case of that loop.
+                var indices = new int[dimLengths.Length];
+                for (var flat = 0; flat < elementCount; flat++)
+                {
+                    LinearToIndices(flat, dimLengths, indices);
+                    var indexedPath = AppendIndices(instancePath, indices, lowerBounds);
+                    var elementOffset = offset + flat * elementSize;
+                    foreach (var leaf in WalkInner(elementType, indexedPath, elementOffset, readOnly, options, visited, depth + 1))
+                        yield return leaf;
+                }
+                yield break;
+            }
+
+            // Anything else (Union / FB instance / Interface / Unknown) — out of scope.
+            yield break;
+        }
+        finally
+        {
+            visited.Remove(type);
+        }
+    }
+
+    /// <summary>
+    ///     Convert a flat row-major index into per-dimension indices (in-place into
+    ///     <paramref name="indices"/>). Single-dim arrays just write <c>flat</c> to slot 0.
+    /// </summary>
+    private static void LinearToIndices(int flat, int[] dimLengths, int[] indices)
+    {
+        for (var d = dimLengths.Length - 1; d >= 0; d--)
+        {
+            var len = dimLengths[d] <= 0 ? 1 : dimLengths[d];
+            indices[d] = flat % len;
+            flat /= len;
+        }
+    }
+
+    /// <summary>
+    ///     Render <c>Path[i]</c> / <c>Path[i,j]</c> form for an array element. Lower bounds add
+    ///     so an <c>ARRAY[1..N]</c> element 0 surfaces as <c>Path[1]</c> rather than <c>Path[0]</c>.
+    /// </summary>
+    private static string AppendIndices(string path, int[] indices, int[]? lowerBounds)
+    {
+        var sb = new System.Text.StringBuilder(path.Length + 4 + indices.Length * 4);
+        sb.Append(path);
+        sb.Append('[');
+        for (var i = 0; i < indices.Length; i++)
+        {
+            if (i > 0) sb.Append(',');
+            var lb = lowerBounds is { Length: > 0 } && i < lowerBounds.Length ? lowerBounds[i] : 0;
+            sb.Append(indices[i] + lb);
+        }
+        sb.Append(']');
+        return sb.ToString();
+    }
+}