8b10d32728
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
204 lines
9.9 KiB
C#
204 lines
9.9 KiB
C#
using Shouldly;
|
||
using Xunit;
|
||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||
using ZB.MOM.WW.OtOpcUa.Driver.FOCAS;
|
||
|
||
namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests;
|
||
|
||
/// <summary>
|
||
/// Gitea #548 — a CNC reporting duplicate sibling axis names (the wonder Makino returns two
|
||
/// axis folders both called <c>AA</c>) made the <c>Axes/</c> subtree untrustworthy: the
|
||
/// browser offered two identical folders commit-time collision detection had to refuse, and
|
||
/// the poll loop's name-keyed snapshot cache let the second axis silently overwrite the
|
||
/// first. Duplicated sibling names are now occurrence-suffixed (<c>AA_1</c>, <c>AA_2</c>)
|
||
/// everywhere — emitter, snapshot keys, servo loads, spindles — so both axes are
|
||
/// authorable and each reads its own axis's data.
|
||
/// </summary>
|
||
[Trait("Category", "Unit")]
|
||
public sealed class FocasDuplicateSiblingNameTests
|
||
{
|
||
private const string Host = "focas://10.0.0.5:8193";
|
||
|
||
/// <summary>A fake modelled on the Makino: axes AA, Y, AA (slots 1–3) with distinct data.</summary>
|
||
private static FakeFocasClientFactory MakinoFactory() => new()
|
||
{
|
||
Customise = () =>
|
||
{
|
||
var c = new FakeFocasClient();
|
||
c.AxisNames.Clear();
|
||
c.AxisNames.AddRange([new("A", "A"), new("Y", ""), new("A", "A")]);
|
||
c.DynamicByAxis[1] = new FocasDynamicSnapshot(1, 0, 0, 0, 0, 0, 0, 111, 0, 0, 0);
|
||
c.DynamicByAxis[2] = new FocasDynamicSnapshot(2, 0, 0, 0, 0, 0, 0, 222, 0, 0, 0);
|
||
c.DynamicByAxis[3] = new FocasDynamicSnapshot(3, 0, 0, 0, 0, 0, 0, 333, 0, 0, 0);
|
||
c.ServoLoads.AddRange([new("AA", 10), new("Y", 20), new("AA", 30)]);
|
||
// Two spindles whose displays also collide, with distinct data per slot.
|
||
c.SpindleNames.Clear();
|
||
c.SpindleNames.AddRange([new("S", "1", "", ""), new("S", "1", "", "")]);
|
||
c.SpindleLoads.AddRange([5, 7]);
|
||
c.SpindleMaxRpms.AddRange([8_000, 12_000]);
|
||
return c;
|
||
},
|
||
};
|
||
|
||
private static FocasDriver NewDriver(params FocasTagDefinition[] tags) =>
|
||
new(new FocasDriverOptions
|
||
{
|
||
Devices = [new FocasDeviceOptions(Host)],
|
||
Probe = new FocasProbeOptions { Enabled = false },
|
||
FixedTree = new FocasFixedTreeOptions { Enabled = true },
|
||
RawTags = FocasRawTags.Entries(tags),
|
||
}, "drv-1", MakinoFactory());
|
||
|
||
private static async Task<FocasDriver> StartedDriverAsync(params FocasTagDefinition[] tags)
|
||
{
|
||
var drv = NewDriver(tags);
|
||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||
var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(5);
|
||
while (DateTime.UtcNow < deadline && drv.GetDeviceState(Host)?.LastFixedPollUtc is null)
|
||
await Task.Delay(25, TestContext.Current.CancellationToken);
|
||
drv.GetDeviceState(Host)!.LastFixedPollUtc.ShouldNotBeNull("the fixed-tree loop never completed a poll");
|
||
return drv;
|
||
}
|
||
|
||
[Fact]
|
||
public async Task Duplicate_axes_browse_as_distinct_suffixed_folders()
|
||
{
|
||
var drv = await StartedDriverAsync();
|
||
var builder = new RecordingBuilder();
|
||
await drv.DiscoverAsync(builder, CancellationToken.None);
|
||
|
||
var folders = builder.Folders.Select(f => f.BrowseName).ToList();
|
||
folders.ShouldContain("AA_1");
|
||
folders.ShouldContain("AA_2");
|
||
folders.ShouldContain("Y");
|
||
folders.ShouldNotContain("AA", customMessage:
|
||
"a bare AA folder would silently mean one of the two duplicate axes");
|
||
// Unique names stay bare — the disambiguation must not rename the rest of the machine.
|
||
folders.ShouldNotContain("Y_1");
|
||
|
||
// The emitted leaves are distinct, parseable addresses (the #540 invariant, held here
|
||
// for the duplicate set specifically).
|
||
var axisLeaves = builder.Variables
|
||
.Select(v => v.Info.FullName)
|
||
.Where(n => n.StartsWith("FIXED:Axes/AA", StringComparison.Ordinal))
|
||
.ToList();
|
||
axisLeaves.ShouldNotBeEmpty();
|
||
axisLeaves.ShouldAllBe(n => FocasAddress.TryParse(n) != null);
|
||
axisLeaves.Distinct(StringComparer.OrdinalIgnoreCase).Count().ShouldBe(axisLeaves.Count);
|
||
axisLeaves.ShouldContain("FIXED:Axes/AA_1/AbsolutePosition");
|
||
axisLeaves.ShouldContain("FIXED:Axes/AA_2/AbsolutePosition");
|
||
|
||
await drv.ShutdownAsync(CancellationToken.None);
|
||
}
|
||
|
||
[Fact]
|
||
public async Task Each_duplicate_axis_reads_its_own_position()
|
||
{
|
||
// The runtime half of #548: before disambiguation the second AA overwrote the first's
|
||
// snapshot slot, so even a hand-authored tag could only ever see one axis.
|
||
var drv = await StartedDriverAsync(
|
||
new FocasTagDefinition("raw/aa1", Host, "FIXED:Axes/AA_1/AbsolutePosition", FocasDataType.Float64, Writable: false),
|
||
new FocasTagDefinition("raw/aa2", Host, "FIXED:Axes/AA_2/AbsolutePosition", FocasDataType.Float64, Writable: false),
|
||
new FocasTagDefinition("raw/y", Host, "FIXED:Axes/Y/AbsolutePosition", FocasDataType.Float64, Writable: false));
|
||
|
||
var read = await drv.ReadAsync(["raw/aa1", "raw/aa2", "raw/y"], CancellationToken.None);
|
||
|
||
read[0].StatusCode.ShouldBe(FocasStatusMapper.Good);
|
||
read[0].Value.ShouldBe(111.0); // slot 1's AA
|
||
read[1].StatusCode.ShouldBe(FocasStatusMapper.Good);
|
||
read[1].Value.ShouldBe(333.0); // slot 3's AA — NOT slot 1's overwritten value
|
||
read[2].StatusCode.ShouldBe(FocasStatusMapper.Good);
|
||
read[2].Value.ShouldBe(222.0); // the unique axis, untouched by disambiguation
|
||
|
||
await drv.ShutdownAsync(CancellationToken.None);
|
||
}
|
||
|
||
[Fact]
|
||
public async Task The_bare_name_of_a_duplicate_set_reads_Bad_rather_than_one_arbitrary_axis()
|
||
{
|
||
// Whole-set suffixing: when two axes are both AA, bare AA ceases to exist. Resolving it
|
||
// to either axis would be the same silent-wrongness class #534/#540 were about.
|
||
var drv = await StartedDriverAsync(
|
||
new FocasTagDefinition("raw/bare", Host, "FIXED:Axes/AA/AbsolutePosition", FocasDataType.Float64, Writable: false));
|
||
|
||
var read = await drv.ReadAsync(["raw/bare"], CancellationToken.None);
|
||
|
||
read[0].StatusCode.ShouldBe(FocasStatusMapper.BadNodeIdUnknown);
|
||
await drv.ShutdownAsync(CancellationToken.None);
|
||
}
|
||
|
||
[Fact]
|
||
public async Task Servo_loads_resolve_per_duplicate_axis()
|
||
{
|
||
// cnc_rdsvmeter answers per-axis in slot order, so the k-th AA load belongs to the k-th
|
||
// AA axis. Before #548 both landed on one name-keyed dictionary entry, last write wins.
|
||
var drv = await StartedDriverAsync(
|
||
new FocasTagDefinition("raw/load1", Host, "FIXED:Axes/AA_1/ServoLoad", FocasDataType.Float64, Writable: false),
|
||
new FocasTagDefinition("raw/load2", Host, "FIXED:Axes/AA_2/ServoLoad", FocasDataType.Float64, Writable: false));
|
||
|
||
// The servo-load pass runs after the axis pass inside the same tick; poll until both
|
||
// distinct keys are populated rather than racing it.
|
||
var state = drv.GetDeviceState(Host)!;
|
||
var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(5);
|
||
while (DateTime.UtcNow < deadline && state.LastServoLoads.Count < 3)
|
||
await Task.Delay(25, TestContext.Current.CancellationToken);
|
||
|
||
var read = await drv.ReadAsync(["raw/load1", "raw/load2"], CancellationToken.None);
|
||
|
||
read[0].StatusCode.ShouldBe(FocasStatusMapper.Good);
|
||
read[0].Value.ShouldBe(10.0);
|
||
read[1].StatusCode.ShouldBe(FocasStatusMapper.Good);
|
||
read[1].Value.ShouldBe(30.0);
|
||
|
||
await drv.ShutdownAsync(CancellationToken.None);
|
||
}
|
||
|
||
[Fact]
|
||
public async Task Duplicate_spindles_resolve_per_slot()
|
||
{
|
||
// The spindle read path matched by display name and took the FIRST index that matched,
|
||
// so a duplicate spindle name made the second spindle's data unreachable.
|
||
var drv = await StartedDriverAsync(
|
||
new FocasTagDefinition("raw/s1load", Host, "FIXED:Spindle/S1_1/Load", FocasDataType.Int32, Writable: false),
|
||
new FocasTagDefinition("raw/s2load", Host, "FIXED:Spindle/S1_2/Load", FocasDataType.Int32, Writable: false),
|
||
new FocasTagDefinition("raw/s1max", Host, "FIXED:Spindle/S1_1/MaxRpm", FocasDataType.Int32, Writable: false),
|
||
new FocasTagDefinition("raw/s2max", Host, "FIXED:Spindle/S1_2/MaxRpm", FocasDataType.Int32, Writable: false));
|
||
|
||
var read = await drv.ReadAsync(["raw/s1load", "raw/s2load", "raw/s1max", "raw/s2max"], CancellationToken.None);
|
||
|
||
read[0].Value.ShouldBe(5);
|
||
read[1].Value.ShouldBe(7);
|
||
read[2].Value.ShouldBe(8_000);
|
||
read[3].Value.ShouldBe(12_000);
|
||
read.ShouldAllBe(r => r.StatusCode == FocasStatusMapper.Good);
|
||
|
||
await drv.ShutdownAsync(CancellationToken.None);
|
||
}
|
||
|
||
/// <summary>Records what <c>DiscoverAsync</c> emits, flattening the folder tree.</summary>
|
||
private sealed class RecordingBuilder : IAddressSpaceBuilder
|
||
{
|
||
public List<(string BrowseName, string DisplayName)> Folders { get; } = [];
|
||
public List<(string BrowseName, DriverAttributeInfo Info)> Variables { get; } = [];
|
||
|
||
public IAddressSpaceBuilder Folder(string browseName, string displayName)
|
||
{ Folders.Add((browseName, displayName)); return this; }
|
||
|
||
public IVariableHandle Variable(string browseName, string displayName, DriverAttributeInfo info)
|
||
{ Variables.Add((browseName, info)); return new Handle(info.FullName); }
|
||
|
||
public void AddProperty(string name, DriverDataType type, object? value) { }
|
||
|
||
private sealed class Handle(string fullRef) : IVariableHandle
|
||
{
|
||
public string FullReference => fullRef;
|
||
public IAlarmConditionSink MarkAsAlarmCondition(AlarmConditionInfo info) => new NullSink();
|
||
}
|
||
|
||
private sealed class NullSink : IAlarmConditionSink
|
||
{
|
||
public void OnTransition(AlarmEventArgs args) { }
|
||
}
|
||
}
|
||
}
|