From 3699fc16a8ce6c65324c838be07bc8b9d71fe8c4 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 17 Jun 2026 20:05:01 -0400 Subject: [PATCH] feat(twincat): expand discovered struct/UDT symbols into addressable member leaves --- .../AdsTwinCATClient.cs | 34 +++- .../TwinCATSymbolExpander.cs | 96 +++++++++++ .../TwinCATSymbolExpanderTests.cs | 162 ++++++++++++++++++ 3 files changed, 288 insertions(+), 4 deletions(-) create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSymbolExpander.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSymbolExpanderTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs index 38895924..3a3c600c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs @@ -326,7 +326,12 @@ internal sealed class AdsTwinCATClient : ITwinCATClient var loader = SymbolLoaderFactory.Create(_client, settings); await Task.Yield(); // honors the async surface; pragmatic given the loader itself is sync - foreach (ISymbol symbol in loader.Symbols) + // Expand struct/UDT/FB-instance symbols down to their addressable atomic members via the + // pure TwinCATSymbolExpander (unit-proven; the real ISymbol surface is non-injectable so the + // recursion is tested through the ITwinCATSymbolNode abstraction, then adapted here). The + // expander dedups by InstancePath, so it also neutralizes any Flat-vs-tree double-listing. + foreach (var ds in TwinCATSymbolExpander.ExpandLeaves( + loader.Symbols.Select(s => new AdsSymbolNode(s)))) { // ThrowIfCancellationRequested — not yield break — so a cancelled browse propagates // as OperationCanceledException rather than a silent clean completion. DiscoverAsync @@ -334,12 +339,33 @@ internal sealed class AdsTwinCATClient : ITwinCATClient // distinctly from a genuine browse failure; a yield break would let a partial // symbol set appear as a fully successful discovery (Driver.TwinCAT-010). cancellationToken.ThrowIfCancellationRequested(); - var (mapped, arrayLength) = MapSymbolType(symbol.DataType); - var readOnly = !IsSymbolWritable(symbol); - yield return new TwinCATDiscoveredSymbol(symbol.InstancePath, mapped, readOnly, arrayLength); + yield return ds; } } + /// + /// Adapts Beckhoff's onto so the pure + /// can walk it. Children are materialized eagerly per node + /// (the loader has already downloaded the whole symbol-info blob, so descending sub-symbols is + /// a local operation). Reuses the existing + + /// so atomic-type mapping and access-rights handling stay identical to the pre-expansion path. + /// + private sealed class AdsSymbolNode(ISymbol symbol) : ITwinCATSymbolNode + { + public string InstancePath => symbol.InstancePath; + + public bool IsStruct => symbol.DataType?.Category == DataTypeCategory.Struct; + + public (TwinCATDataType? Type, int? ArrayLength) Mapped => MapSymbolType(symbol.DataType); + + public IReadOnlyList Children => + symbol.SubSymbols is { } subs + ? subs.Select(s => (ITwinCATSymbolNode)new AdsSymbolNode(s)).ToList() + : []; + + public bool ReadOnly => !IsSymbolWritable(symbol); + } + /// /// Resolves a symbol's to the driver's atomic /// plus an optional 1-D array length. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSymbolExpander.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSymbolExpander.cs new file mode 100644 index 00000000..e67d0591 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSymbolExpander.cs @@ -0,0 +1,96 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +/// +/// Minimal view over an ADS symbol that needs to walk a +/// struct/UDT/FB instance down to its addressable atomic leaves. An adapter in +/// AdsTwinCATClient.BrowseSymbolsAsync wraps the real TwinCAT.TypeSystem.ISymbol +/// onto this interface so the recursion is unit-testable without a live ADS target — the real +/// ISymbol / SymbolLoaderFactory surface is non-injectable. +/// +public interface ITwinCATSymbolNode +{ + /// Full dotted instance path of this symbol (e.g. MAIN.Motor1.Speed). + string InstancePath { get; } + + /// + /// true when this symbol is a struct/UDT/FB instance (DataType.Category == Struct) + /// and therefore has addressable members under rather than a value of + /// its own. + /// + bool IsStruct { get; } + + /// + /// The symbol mapped onto the driver's atomic type surface plus an optional 1-D array length. + /// A null Type means an unsupported atomic leaf (pointer / reference / out-of-scope + /// type) which the expander drops. + /// + (TwinCATDataType? Type, int? ArrayLength) Mapped { get; } + + /// Struct members (empty for atomic leaves). + IReadOnlyList Children { get; } + + /// true when the symbol's access rights forbid writes. + bool ReadOnly { get; } +} + +/// +/// Pure, dependency-free walker that flattens a forest of roots +/// into the addressable atomic leaves the driver can surface as OPC UA variable nodes. A struct +/// node recurses into its (depth-guarded); an atomic +/// leaf with a non-null mapped is yielded as a +/// ; unsupported leaves (null mapped type) and +/// depth-exceeded nodes are dropped. This is the §2 fix that makes discovered struct/UDT/FB +/// members individually addressable. +/// +public static class TwinCATSymbolExpander +{ + /// + /// Maximum struct-nesting depth the walker descends before dropping a node. Guards against a + /// pathological / self-referential type graph turning discovery into an unbounded walk. Deep + /// nesting beyond this is exceedingly rare in real PLC code; the operator can still predeclare + /// a member-path tag directly for anything past the floor. + /// + public const int MaxDepth = 8; + + /// + /// Walk to their atomic leaves. A struct node recurses its children + /// (depth-guarded by ); an atomic leaf with a non-null mapped + /// is yielded; unsupported leaves (null type) and depth-exceeded + /// nodes are dropped. De-duplicates by so any + /// Flat-vs-tree double-listing from the underlying loader collapses to a single emission. + /// + /// The top-level symbols (e.g. loader.Symbols) to expand. + /// One per addressable atomic leaf. + public static IEnumerable ExpandLeaves(IEnumerable roots) + { + var seen = new HashSet(StringComparer.Ordinal); + foreach (var root in roots) + foreach (var leaf in Walk(root, depth: 0, seen)) + yield return leaf; + } + + private static IEnumerable Walk( + ITwinCATSymbolNode node, int depth, HashSet seen) + { + // Depth guard: drop a node whose nesting exceeds the floor rather than recursing further. + if (depth >= MaxDepth) yield break; + + if (node.IsStruct) + { + // A struct contributes no value of its own — recurse its members. (Empty children => + // nothing emitted, which is the correct "no addressable atomic" outcome.) + foreach (var child in node.Children) + foreach (var leaf in Walk(child, depth + 1, seen)) + yield return leaf; + yield break; + } + + var (type, arrayLength) = node.Mapped; + if (type is null) yield break; // unsupported atomic leaf (pointer / out-of-scope type) — drop. + + // Dedup by InstancePath — neutralizes any Flat-vs-tree double-listing of the same leaf. + if (!seen.Add(node.InstancePath)) yield break; + + yield return new TwinCATDiscoveredSymbol(node.InstancePath, type, node.ReadOnly, arrayLength); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSymbolExpanderTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSymbolExpanderTests.cs new file mode 100644 index 00000000..47262bd6 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSymbolExpanderTests.cs @@ -0,0 +1,162 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; + +[Trait("Category", "Unit")] +public sealed class TwinCATSymbolExpanderTests +{ + /// + /// A struct with atomic members + a nested struct expands to exactly the atomic leaves, each + /// carrying its full dotted InstancePath and mapped atomic type. The struct nodes themselves + /// contribute no variable. + /// + [Fact] + public void Struct_expands_to_atomic_member_leaves_with_full_paths() + { + // MAIN.Motor1 : struct { Speed: REAL, Running: BOOL, Status: struct { Code: DINT } } + var motor = Struct("MAIN.Motor1", + Atomic("MAIN.Motor1.Speed", TwinCATDataType.Real), + Atomic("MAIN.Motor1.Running", TwinCATDataType.Bool), + Struct("MAIN.Motor1.Status", + Atomic("MAIN.Motor1.Status.Code", TwinCATDataType.DInt))); + + var leaves = TwinCATSymbolExpander.ExpandLeaves([motor]).ToList(); + + leaves.Select(l => l.InstancePath).ShouldBe( + ["MAIN.Motor1.Speed", "MAIN.Motor1.Running", "MAIN.Motor1.Status.Code"]); + leaves.Single(l => l.InstancePath == "MAIN.Motor1.Speed").DataType.ShouldBe(TwinCATDataType.Real); + leaves.Single(l => l.InstancePath == "MAIN.Motor1.Running").DataType.ShouldBe(TwinCATDataType.Bool); + leaves.Single(l => l.InstancePath == "MAIN.Motor1.Status.Code").DataType.ShouldBe(TwinCATDataType.DInt); + // No struct container is ever emitted as a leaf. + leaves.ShouldNotContain(l => l.InstancePath == "MAIN.Motor1"); + leaves.ShouldNotContain(l => l.InstancePath == "MAIN.Motor1.Status"); + } + + /// A top-level atomic symbol yields itself unchanged. + [Fact] + public void Top_level_atomic_yields_itself() + { + var counter = Atomic("MAIN.Counter", TwinCATDataType.DInt); + + var leaves = TwinCATSymbolExpander.ExpandLeaves([counter]).ToList(); + + leaves.Count.ShouldBe(1); + leaves[0].InstancePath.ShouldBe("MAIN.Counter"); + leaves[0].DataType.ShouldBe(TwinCATDataType.DInt); + } + + /// The atomic leaf carries its mapped array length through unchanged (1-D array element). + [Fact] + public void Array_leaf_carries_element_type_and_length() + { + var arr = new FakeNode("GVL.Data", IsStruct: false, + Mapped: (TwinCATDataType.Int, 4), Children: [], ReadOnly: false); + + var leaf = TwinCATSymbolExpander.ExpandLeaves([arr]).Single(); + + leaf.InstancePath.ShouldBe("GVL.Data"); + leaf.DataType.ShouldBe(TwinCATDataType.Int); + leaf.ArrayLength.ShouldBe(4); + } + + /// The read-only flag is propagated onto the emitted discovered symbol. + [Fact] + public void ReadOnly_flag_propagates() + { + var ro = new FakeNode("MAIN.Status", IsStruct: false, + Mapped: (TwinCATDataType.DInt, null), Children: [], ReadOnly: true); + + TwinCATSymbolExpander.ExpandLeaves([ro]).Single().ReadOnly.ShouldBeTrue(); + } + + /// An unsupported atomic leaf (mapped Type == null, e.g. a pointer) is dropped. + [Fact] + public void Unsupported_atomic_leaf_is_dropped() + { + var pointer = new FakeNode("MAIN.pData", IsStruct: false, + Mapped: (null, null), Children: [], ReadOnly: false); + var ok = Atomic("MAIN.Counter", TwinCATDataType.DInt); + + var leaves = TwinCATSymbolExpander.ExpandLeaves([pointer, ok]).ToList(); + + leaves.Select(l => l.InstancePath).ShouldBe(["MAIN.Counter"]); + } + + /// An empty struct (no members) yields nothing — there is no addressable atomic. + [Fact] + public void Empty_struct_yields_nothing() + { + var empty = Struct("MAIN.Empty"); + + TwinCATSymbolExpander.ExpandLeaves([empty]).ShouldBeEmpty(); + } + + /// + /// A member nested beyond is dropped; members at + /// or above the floor still surface. + /// + [Fact] + public void Node_beyond_MaxDepth_is_dropped() + { + // Build a chain of nested structs: root (depth 0) → struct → struct → ... with a single + // atomic leaf at the very bottom. The leaf sits at depth == number-of-struct-levels. + // Place the leaf at depth MaxDepth (== one past the deepest descended level) so it is dropped, + // and a sibling leaf at a shallow depth so we prove the walk itself still works. + ITwinCATSymbolNode deep = Atomic("Root" + new string('.', 0) + "Leaf.Deep", TwinCATDataType.DInt); + // Wrap the atomic in MaxDepth struct levels so the atomic ends up at depth == MaxDepth. + for (var i = 0; i < TwinCATSymbolExpander.MaxDepth; i++) + deep = Struct($"Level{i}", deep); + + // Shallow leaf reachable at depth 1. + var shallow = Struct("Shallow", Atomic("Shallow.Ok", TwinCATDataType.Int)); + + var leaves = TwinCATSymbolExpander.ExpandLeaves([deep, shallow]).ToList(); + + leaves.Select(l => l.InstancePath).ShouldBe(["Shallow.Ok"]); + } + + /// A node at exactly the deepest descended level is still emitted (boundary check). + [Fact] + public void Node_at_max_depth_boundary_is_emitted() + { + // Nest the atomic under (MaxDepth - 1) struct levels so it sits at depth == MaxDepth - 1, + // which is the last level the walker still descends. + ITwinCATSymbolNode node = Atomic("Deep.Boundary", TwinCATDataType.DInt); + for (var i = 0; i < TwinCATSymbolExpander.MaxDepth - 1; i++) + node = Struct($"L{i}", node); + + var leaves = TwinCATSymbolExpander.ExpandLeaves([node]).ToList(); + + leaves.Select(l => l.InstancePath).ShouldBe(["Deep.Boundary"]); + } + + /// A duplicate InstancePath (Flat-vs-tree double-listing) is emitted exactly once. + [Fact] + public void Duplicate_InstancePath_is_emitted_once() + { + var a = Atomic("MAIN.Counter", TwinCATDataType.DInt); + var b = Atomic("MAIN.Counter", TwinCATDataType.DInt); + + var leaves = TwinCATSymbolExpander.ExpandLeaves([a, b]).ToList(); + + leaves.Count.ShouldBe(1); + leaves[0].InstancePath.ShouldBe("MAIN.Counter"); + } + + // ---- helpers ---- + + private static FakeNode Atomic(string path, TwinCATDataType type) => + new(path, IsStruct: false, Mapped: (type, null), Children: [], ReadOnly: false); + + private static FakeNode Struct(string path, params ITwinCATSymbolNode[] children) => + new(path, IsStruct: true, Mapped: (null, null), Children: children, ReadOnly: false); + + private sealed record FakeNode( + string InstancePath, + bool IsStruct, + (TwinCATDataType? Type, int? ArrayLength) Mapped, + IReadOnlyList Children, + bool ReadOnly) : ITwinCATSymbolNode; +}