dohertj2/lmxopcua
1
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(focas): disambiguate duplicate sibling axis/spindle names in the fixed tree (#548)

Browse Source 
A CNC can report duplicate sibling names — the wonder Makino returns two
axes both called AA — which made the Axes/ subtree untrustworthy: the
browser offered two identical folders the commit gate had to refuse, and
the poll loop's name-keyed snapshot cache let the second axis silently
overwrite the first. Spindles and servo loads had the same collision in
their own keying, plus a resolve-to-first-match on the spindle read path.

Scheme: every member of a collision set gets a 1-based occurrence-ordinal
suffix in slot order (AA_1, AA_2); unique names stay bare. Chosen over the
alternatives deliberately:

- Names, not slot numbers, stay the primary identity — the segment map is
  re-derived from the live device each session, so a name-keyed path
  follows the physical axis across config changes, where a slot-qualified
  path would silently rebind on any axis-list shift and rename every axis
  on every machine for one pathological CNC.
- The WHOLE collision set is suffixed: bare AA ceases to exist when
  ambiguous, so a tag authored against it reads BadNodeIdUnknown rather
  than one arbitrary axis's data — the #534/#540 silent-wrongness class.
- Occurrence ordinal, not absolute slot: inserting/removing an unrelated
  axis never renames an existing duplicate pair. If the collision-set
  membership itself changes, segments change on the next browse and stale
  authored segments read Bad, never another axis's data.

One pure helper (FixedTreeNames.Disambiguate, collision-proof against
genuine names by suffix escalation) feeds FocasFixedTreeCache.AxisSegments
/ SpindleSegments, and the emitter, axis snapshot keys, servo-load keys
(cnc_rdsvmeter answers in slot order, so the k-th AA load belongs to the
k-th AA axis) and the spindle read resolution all consume those — they can
no longer disagree. The nameless-spindle S{n} fallback applies before
disambiguation so a fallback name colliding with a real one is handled
like any other duplicate.

Tests are red-first on the pre-fix driver (all five driver-level tests
fail on master's code): browse emits AA_1/AA_2 and no bare AA, each
duplicate reads its own position and servo load, duplicate spindles
resolve per slot, bare AA reads Bad. The wire half runs live against
focas-mock (duplicate names survive the managed wire parse and publish as
suffixed segments); the mock's dynamic store is name-keyed so the
distinct-values half stays with the offline tests. FOCAS 327/327,
integration 11/11, FOCAS.Cli 52, AdminUI 1113, ControlPlane 231.

Closes #548.

Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
This commit is contained in:
Joseph Doherty
2026-08-06 21:09:05 -04:00
parent 0acfc79ad9
commit 8b10d32728
6 changed files with 536 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FixedTreeNames.cs
+95
View File
@@ -0,0 +1,95 @@
namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS;
/// <summary>
/// Turns a device-reported sibling-name sequence (axes, spindles) into unique fixed-tree
/// path segments (Gitea #548).
/// </summary>
/// <remarks>
/// <para>
/// A real CNC can report duplicate sibling names — one observed Makino returns two axis
/// folders both called <c>AA</c>. Before this existed, the emitter offered two identical
/// browse folders, commit-time collision detection refused to author both (same RawPath),
/// and the poll loop's name-keyed snapshot cache let the second axis silently overwrite the
/// first — so the <c>Axes/</c> subtree was untrustworthy on such a machine.
/// </para>
/// <para>
/// <b>Scheme: every member of a collision set gets a 1-based occurrence-ordinal suffix in
/// slot order (<c>AA_1</c>, <c>AA_2</c>); unique names stay bare.</b> Three deliberate
/// choices inside that:
/// </para>
/// <para>
/// <b>Names stay the primary identity, not slot numbers.</b> The segment map is re-derived
/// from the live device at every session bootstrap, so a name-keyed path follows the
/// physical axis across configuration changes; a slot-qualified path (<c>3_AA</c>) would
/// silently rebind to a different physical axis whenever the axis list shifts, and would
/// rename every axis path on every machine for the sake of the rare duplicate.
/// </para>
/// <para>
/// <b>The whole collision set is suffixed, not just the second occurrence.</b> When two
/// axes are both called <c>AA</c>, the bare segment <c>AA</c> ceases to exist — a bare name
/// that silently means "one of the two" is the same silent-wrongness class #534 and #540
/// were about. A tag authored against bare <c>AA</c> on such a machine reads Bad, never one
/// arbitrary axis's data.
/// </para>
/// <para>
/// <b>Occurrence ordinal, not absolute slot number.</b> The ordinal is stable under
/// insertion/removal of <em>unrelated</em> axes (a new <c>B</c> axis ahead of the two AAs
/// renames nothing; their slot numbers would both change). If the collision-set membership
/// itself changes — a third <c>AA</c> appears, or one of two is removed — the segments do
/// change; that surfaces on the next browse + commit, and until then the stale authored
/// segment reads Bad rather than another axis's data (the served tree only changes on a
/// config redeploy).
/// </para>
/// <para>
/// Suffix collisions with genuine names are structurally impossible for axes (FOCAS axis
/// displays are at most two characters — name + suffix — so they cannot contain <c>_</c>),
/// but the generator still checks: a generated segment that would collide with anything
/// else in the set escalates its ordinal until unique, so the output is always
/// collision-free whatever the input.
/// </para>
/// </remarks>
internal static class FixedTreeNames
{
/// <summary>
/// Maps each name in <paramref name="names"/> (in slot order) to a unique sibling path
/// segment. Unique names map to themselves; duplicated names map to
/// <c>{name}_{occurrence}</c>. Comparison is case-insensitive, matching the fixed-tree
/// snapshot cache and read-path lookups.
/// </summary>
/// <param name="names">Device-reported sibling names, in device (slot) order.</param>
/// <returns>One segment per input name, unique within the returned list.</returns>
public static IReadOnlyList<string> Disambiguate(IEnumerable<string> names)
{
var list = names as IReadOnlyList<string> ?? [.. names];
var counts = new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase);
foreach (var name in list)
counts[name] = counts.TryGetValue(name, out var c) ? c + 1 : 1;
// Bare (unique) names are claimed first so a generated "{name}_{k}" can never shadow one.
var used = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
foreach (var name in list)
if (counts[name] == 1)
used.Add(name);
var nextOrdinal = new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase);
var result = new string[list.Count];
for (var i = 0; i < list.Count; i++)
{
var name = list[i];
if (counts[name] == 1)
{
result[i] = name;
continue;
}
var k = nextOrdinal.TryGetValue(name, out var n) ? n : 1;
string segment;
do { segment = $"{name}_{k}"; k++; }
while (!used.Add(segment));
nextOrdinal[name] = k;
result[i] = segment;
}
return result;
}
}
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs
+57 -25
View File
@@ -609,15 +609,19 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
EmitIdentityVariable(identity, "AxisCount", FocasDriverDataType.Int32);
var axesFolder = deviceFolder.Folder("Axes", "Axes");
foreach (var axis in cache.Axes)
for (var a = 0; a < cache.Axes.Count; a++)
{
var axisFolder = axesFolder.Folder(axis.Display, axis.Display);
EmitAxisVariable(axisFolder, axis.Display, "AbsolutePosition");
EmitAxisVariable(axisFolder, axis.Display, "MachinePosition");
EmitAxisVariable(axisFolder, axis.Display, "RelativePosition");
EmitAxisVariable(axisFolder, axis.Display, "DistanceToGo");
// The disambiguated segment, not the raw Display — a CNC reporting two
// axes both named AA gets AA_1 / AA_2 folders instead of two identical
// siblings the commit gate must refuse (#548).
var segment = cache.AxisSegments[a];
var axisFolder = axesFolder.Folder(segment, segment);
EmitAxisVariable(axisFolder, segment, "AbsolutePosition");
EmitAxisVariable(axisFolder, segment, "MachinePosition");
EmitAxisVariable(axisFolder, segment, "RelativePosition");
EmitAxisVariable(axisFolder, segment, "DistanceToGo");
if (cache.Capabilities.ServoLoad)
EmitAxisVariable(axisFolder, axis.Display, "ServoLoad");
EmitAxisVariable(axisFolder, segment, "ServoLoad");
}
EmitAxisVariable(axesFolder, "FeedRate", "Actual");
EmitAxisVariable(axesFolder, "SpindleSpeed", "Actual");
@@ -630,8 +634,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
var spindleRoot = deviceFolder.Folder("Spindle", "Spindle");
for (var i = 0; i < cache.Spindles.Count; i++)
{
var s = cache.Spindles[i];
var name = string.IsNullOrEmpty(s.Display) ? $"S{i + 1}" : s.Display;
var name = cache.SpindleSegments[i];
var spindleFolder = spindleRoot.Folder(name, name);
if (cache.Capabilities.SpindleLoad)
EmitFixedVariable(spindleFolder, $"Spindle/{name}", "Load", DriverDataType.Int32);
@@ -977,9 +980,10 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
for (var i = 0; i < cache.Axes.Count; i++)
{
var axisIndex = i + 1; // FOCAS uses 1-based axis indexing
var axis = cache.Axes[i];
var snap = await client.ReadDynamicAsync(axisIndex, ct).ConfigureAwait(false);
PublishAxisSnapshot(state, axis, snap, i); // i = 0-based axis index → PositionFigures lookup
// Keyed by the disambiguated segment, matching what DiscoverAsync emitted —
// two same-named axes must not share (and overwrite) one cache slot (#548).
PublishAxisSnapshot(state, cache.AxisSegments[i], snap, i); // i = 0-based axis index → PositionFigures lookup
if (i == 0) { firstAxisSnap = snap; PublishRateSnapshot(state, snap); }
}
@@ -1082,7 +1086,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
/// <see cref="PollGroupEngine"/> → <see cref="ReadAsync"/> path, which hits
/// <see cref="TryReadFixedTree"/> and returns these cached values.
/// </summary>
private static void PublishAxisSnapshot(DeviceState state, FocasAxisName axis, FocasDynamicSnapshot snap, int axisIndex)
private static void PublishAxisSnapshot(DeviceState state, string axisSegment, FocasDynamicSnapshot snap, int axisIndex)
{
var host = state.Options.HostAddress;
// cnc_rddynamic2 returns positions as scaled integers; divide by 10^figures so they
@@ -1094,10 +1098,10 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
// via ReadPositionFiguresAsync; the config knob is the per-axis fallback only. FeedRate /
// SpindleSpeed (rate snapshot) and ServoLoad are NOT position-scaled (published elsewhere).
var factor = AxisFactor(state, axisIndex);
state.LastFixedSnapshots[FixedTreeReference(host, $"Axes/{axis.Display}/AbsolutePosition")] = snap.AbsolutePosition / factor;
state.LastFixedSnapshots[FixedTreeReference(host, $"Axes/{axis.Display}/MachinePosition")] = snap.MachinePosition / factor;
state.LastFixedSnapshots[FixedTreeReference(host, $"Axes/{axis.Display}/RelativePosition")] = snap.RelativePosition / factor;
state.LastFixedSnapshots[FixedTreeReference(host, $"Axes/{axis.Display}/DistanceToGo")] = snap.DistanceToGo / factor;
state.LastFixedSnapshots[FixedTreeReference(host, $"Axes/{axisSegment}/AbsolutePosition")] = snap.AbsolutePosition / factor;
state.LastFixedSnapshots[FixedTreeReference(host, $"Axes/{axisSegment}/MachinePosition")] = snap.MachinePosition / factor;
state.LastFixedSnapshots[FixedTreeReference(host, $"Axes/{axisSegment}/RelativePosition")] = snap.RelativePosition / factor;
state.LastFixedSnapshots[FixedTreeReference(host, $"Axes/{axisSegment}/DistanceToGo")] = snap.DistanceToGo / factor;
}
/// <summary>
@@ -1125,14 +1129,22 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
}
/// <summary>
/// Cache servo-load percentages keyed by axis name. Stored separately from
/// Cache servo-load percentages keyed by axis path segment. Stored separately from
/// <c>LastFixedSnapshots</c> (which is keyed by full-reference) because the
/// servo-load read path resolves by axis name.
/// servo-load read path resolves by axis segment.
/// </summary>
/// <remarks>
/// The device-reported load names run through the same disambiguation as the axis list
/// (#548): <c>cnc_rdsvmeter</c> answers per-axis in slot order, so the k-th load named
/// <c>AA</c> belongs to the k-th axis named <c>AA</c> and lands under the same
/// <c>AA_k</c> segment the emitter published — without this, two same-named axes
/// collapsed onto one dictionary entry and the second overwrote the first.
/// </remarks>
private static void PublishServoLoads(DeviceState state, IReadOnlyList<FocasServoLoad> loads)
{
foreach (var load in loads)
state.LastServoLoads[load.AxisName] = load.LoadPercent;
var segments = FixedTreeNames.Disambiguate(loads.Select(l => l.AxisName));
for (var i = 0; i < loads.Count; i++)
state.LastServoLoads[segments[i]] = loads[i].LoadPercent;
}
private static object? TimerValue(DeviceState state, FocasTimerKind kind) =>
@@ -1235,11 +1247,11 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
var spindleName = tail[..slash];
var field = tail[(slash + 1)..];
var idx = -1;
for (var i = 0; i < spindleCache.Spindles.Count; i++)
for (var i = 0; i < spindleCache.SpindleSegments.Count; i++)
{
var s = spindleCache.Spindles[i];
var display = string.IsNullOrEmpty(s.Display) ? $"S{i + 1}" : s.Display;
if (string.Equals(display, spindleName, StringComparison.OrdinalIgnoreCase)) { idx = i; break; }
// Match on the disambiguated segment the emitter published (#548) — matching
// raw Display would resolve every duplicate name to the first spindle.
if (string.Equals(spindleCache.SpindleSegments[i], spindleName, StringComparison.OrdinalIgnoreCase)) { idx = i; break; }
}
if (idx >= 0)
{
@@ -1462,7 +1474,27 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
IReadOnlyList<FocasAxisName> Axes,
IReadOnlyList<FocasSpindleName> Spindles,
IReadOnlyList<int> SpindleMaxRpms,
FocasFixedTreeCapabilities Capabilities);
FocasFixedTreeCapabilities Capabilities)
{
/// <summary>
/// Unique <c>Axes/{segment}</c> path segments, one per axis in slot order — bare
/// display names when unique, occurrence-suffixed (<c>AA_1</c>, <c>AA_2</c>) when a
/// CNC reports duplicate sibling axis names (#548). The emitter, the poll loop's
/// snapshot keys and the read path all use THIS, never <c>Display</c> directly, so
/// they cannot disagree.
/// </summary>
public IReadOnlyList<string> AxisSegments { get; } =
FixedTreeNames.Disambiguate(Axes.Select(a => a.Display));
/// <summary>
/// Unique <c>Spindle/{segment}</c> path segments, one per spindle in slot order. The
/// nameless-spindle fallback (<c>S{n}</c>) is applied BEFORE disambiguation so a
/// fallback name colliding with a real one is disambiguated like any other duplicate.
/// </summary>
public IReadOnlyList<string> SpindleSegments { get; } =
FixedTreeNames.Disambiguate(Spindles.Select(
(s, i) => string.IsNullOrEmpty(s.Display) ? $"S{i + 1}" : s.Display));
}
/// <summary>
/// Per-device optional-API capability flags — which of the "this may or may not