fix(twincat): update BrowseSymbolsAsync doc + cache adapter fields + Flat-mode note (review)

- ITwinCATClient.BrowseSymbolsAsync XML doc updated: states the implementation now
  expands struct/UDT/FB symbols into atomic member leaves via TwinCATSymbolExpander;
  callers receive only atomic/array leaves with full InstancePaths, never struct containers.
- AdsSymbolNode: cache IsStruct, Mapped, Children, ReadOnly as readonly fields computed
  once in the ctor so repeated property access during recursive expansion doesn't
  re-materialize or re-invoke MapSymbolType/IsSymbolWritable.
- BrowseSymbolsAsync: add operator-gated live risk note next to SymbolsLoadMode.Flat
  warning that a real TC3 target may not populate SubSymbols in Flat mode, with
  guidance to switch to VirtualTree if members don't surface — do not change mode now.
- TwinCATSymbolExpanderTests: simplify confusing `new string('.', 0)` no-op to `""`.
This commit is contained in:
Joseph Doherty
2026-06-17 20:22:45 -04:00
parent 0f929ae668
commit fdd6b332fe
3 changed files with 42 additions and 15 deletions
@@ -322,6 +322,14 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
// 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.
//
// LIVE RISK (operator-gated, not yet verified against a real TC3 target): Flat mode
// may not populate ISymbol.SubSymbols on struct/UDT/FB symbols on some TC3 firmware
// versions — in that case TwinCATSymbolExpander sees no children and drops those
// symbols silently. If struct members don't surface on a real target, switch to
// SymbolsLoadMode.VirtualTree which is documented to always populate SubSymbols via
// the hierarchy. Do NOT change the mode here until confirmed against live ADS — Flat is
// required for Phase 4c array discovery to work correctly.
var settings = new SymbolLoaderSettings(SymbolsLoadMode.Flat);
var loader = SymbolLoaderFactory.Create(_client, settings);
await Task.Yield(); // honors the async surface; pragmatic given the loader itself is sync
@@ -350,20 +358,36 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
/// a local operation). Reuses the existing <see cref="MapSymbolType"/> + <see cref="IsSymbolWritable"/>
/// so atomic-type mapping and access-rights handling stay identical to the pre-expansion path.
/// </summary>
private sealed class AdsSymbolNode(ISymbol symbol) : ITwinCATSymbolNode
private sealed class AdsSymbolNode : ITwinCATSymbolNode
{
public string InstancePath => symbol.InstancePath;
// Computed once in the ctor so repeated property access during recursive expansion
// doesn't re-materialize Children or re-invoke MapSymbolType / IsSymbolWritable.
private readonly ISymbol _symbol;
private readonly bool _isStruct;
private readonly (TwinCATDataType? Type, int? ArrayLength) _mapped;
private readonly IReadOnlyList<ITwinCATSymbolNode> _children;
private readonly bool _readOnly;
public bool IsStruct => symbol.DataType?.Category == DataTypeCategory.Struct;
public (TwinCATDataType? Type, int? ArrayLength) Mapped => MapSymbolType(symbol.DataType);
public IReadOnlyList<ITwinCATSymbolNode> Children =>
symbol.SubSymbols is { } subs
public AdsSymbolNode(ISymbol symbol)
{
_symbol = symbol;
_isStruct = symbol.DataType?.Category == DataTypeCategory.Struct;
_mapped = MapSymbolType(symbol.DataType);
_children = symbol.SubSymbols is { } subs
? subs.Select(s => (ITwinCATSymbolNode)new AdsSymbolNode(s)).ToList()
: [];
_readOnly = !IsSymbolWritable(symbol);
}
public bool ReadOnly => !IsSymbolWritable(symbol);
public string InstancePath => _symbol.InstancePath;
public bool IsStruct => _isStruct;
public (TwinCATDataType? Type, int? ArrayLength) Mapped => _mapped;
public IReadOnlyList<ITwinCATSymbolNode> Children => _children;
public bool ReadOnly => _readOnly;
}
/// <summary>