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

revert(drivers): remove the periodic 5-minute device re-browse (#523)

Browse Source 
Removes the tag-set drift detector shipped in e77c8a35. The design was sound
-- the tautology drivers were correctly gated out, and the structurally
undetectable "vanished" direction was declared rather than faked -- but none of
that is the objection. The feature browsed real plant equipment on a recurring
timer, at a frequency no operator could configure (the interval was a
constructor parameter with no appsettings key), and that outbound traffic was
never once observed against a real PLC, CNC or MTConnect Agent.

Removed: TagSetDrift/TagSetDriftDetector; DriverInstanceActor's drift timer,
tick message, Props/ctor parameter, gate, handler and leaf-collector;
ITagDiscovery.AuthoredDiscoveryRefs + .DiscoveryStreamIncludesAuthoredTags and
their four implementations; 14 tests.

Kept: ITagDiscovery.SupportsOnlineDiscovery (it predates this and drives the
universal browse picker), and the whole IRediscoverable consumption from
09a401b8 -- the driver tells us, we do not poll.

CONSEQUENCE, deliberately accepted: AbCip and FOCAS browse a live backend but
implement no IRediscoverable, so they now have NO tag-change signal at all -- a
PLC re-download is invisible until someone re-browses by hand. Recorded in
CLAUDE.md so the next reader does not have to rediscover it, along with the
shape to reach for if that coverage is wanted back (event-driven, or triggered
by a reconnect/deploy, not a wall-clock timer).

Runtime.Tests 476 pass / 13 skipped (skip set unchanged); FOCAS 275, TwinCAT
192, MTConnect 491, AbCip 342, Core.Abstractions 266 all green.

Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
This commit is contained in:
Joseph Doherty
2026-07-28 13:59:06 -04:00
parent 16752032e5
commit 5fff597324
11 changed files with 59 additions and 743 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDriftCheckTests.cs
-268
View File
@@ -1,268 +0,0 @@
using Akka.Actor;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness;
namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers;
/// <summary>
/// The tag-set drift check: periodically re-browse a genuinely-browsable driver and compare what the
/// remote offers against what an operator authored.
/// <para><b>Why this is gated rather than run for every driver.</b> Modbus, S7, AbLegacy, Sql and Mqtt
/// all stream their AUTHORED tags back out of <c>DiscoverAsync</c> rather than enumerating the device, so
/// diffing the two sets for them is a tautology that reports "no drift" forever. A check that cannot fail
/// is worse than no check, because it reads as coverage — so the actor runs this only for drivers that
/// declare <c>SupportsOnlineDiscovery</c> AND report <c>AuthoredDiscoveryRefs</c>.</para>
/// <para>The signal is advisory and shares the Stage-2 surfacing: it raises the same <c>/hosts</c>
/// re-browse prompt an <see cref="IRediscoverable"/> raise does. The served address space is never
/// rebuilt — raw tags are authored through the <c>/raw</c> browse-commit flow.</para>
/// </summary>
[Trait("Category", "Unit")]
public sealed class DriverInstanceActorDriftCheckTests : RuntimeActorTestBase
{
private static readonly TimeSpan Tick = TimeSpan.FromMilliseconds(60);
private static readonly TimeSpan Budget = TimeSpan.FromSeconds(3);
/// <summary>A device offering a tag nothing binds raises the operator prompt, naming what appeared.</summary>
[Fact]
public void A_new_ref_on_the_device_raises_the_re_browse_prompt()
{
var driver = new BrowsableStubDriver(discovered: ["a", "b", "c"], authored: ["a", "b"]);
var publisher = new RecordingHealthPublisher();
var actor = SpawnConnected(driver, publisher);
AwaitAssert(
() =>
{
var signalled = publisher.Published.LastOrDefault(p => p.RediscoveryNeededUtc is not null);
signalled.ShouldNotBeNull();
signalled!.RediscoveryReason.ShouldNotBeNull();
signalled.RediscoveryReason!.ShouldContain("1 new on device");
signalled.RediscoveryReason!.ShouldContain("c");
},
Budget);
Sys.Stop(actor);
}
/// <summary>
/// <b>The gate that makes this feature honest.</b> A driver whose <c>DiscoverAsync</c> replays
/// authored config rather than browsing the device declares <c>SupportsOnlineDiscovery == false</c>,
/// and must never be drift-checked — for it the comparison could only ever say "no drift".
/// <para>Positive control: the driver is handed a set that WOULD read as drift, and the assertion is
/// that discovery is never even invoked. Asserting only "no prompt raised" would pass for the wrong
/// reason (e.g. if the timer never fired at all).</para>
/// </summary>
[Fact]
public void A_driver_that_replays_authored_config_is_never_drift_checked()
{
var driver = new BrowsableStubDriver(
discovered: ["a", "b", "c"], authored: ["a"], supportsOnlineDiscovery: false);
var publisher = new RecordingHealthPublisher();
var actor = SpawnConnected(driver, publisher);
// Long enough for several ticks had the timer been scheduled.
ExpectNoMsg(TimeSpan.FromMilliseconds(400));
driver.DiscoverCount.ShouldBe(
0,
"a driver that replays authored config was browsed for drift — the comparison is a tautology "
+ "for it and would report 'no drift' forever, which reads as coverage");
publisher.Published.ShouldAllBe(p => p.RediscoveryNeededUtc == null);
Sys.Stop(actor);
}
/// <summary>A driver that browses but does NOT report its authored refs opts out — the default. An empty
/// collection would mean "nothing authored, everything is new", so null must not be read that way.</summary>
[Fact]
public void A_driver_that_reports_no_authored_refs_is_never_drift_checked()
{
var driver = new BrowsableStubDriver(discovered: ["a", "b"], authored: null);
var publisher = new RecordingHealthPublisher();
var actor = SpawnConnected(driver, publisher);
ExpectNoMsg(TimeSpan.FromMilliseconds(400));
driver.DiscoverCount.ShouldBe(0);
publisher.Published.ShouldAllBe(p => p.RediscoveryNeededUtc == null);
Sys.Stop(actor);
}
/// <summary>
/// An UNCHANGED drift is reported once, not re-stamped on every check — a prompt whose timestamp
/// keeps moving reads as "it just happened again" rather than "it is still true".
/// </summary>
[Fact]
public void An_unchanged_drift_is_reported_once()
{
var driver = new BrowsableStubDriver(discovered: ["a", "b"], authored: ["a"]);
var publisher = new RecordingHealthPublisher();
var actor = SpawnConnected(driver, publisher);
AwaitAssert(
() => publisher.Published.Count(p => p.RediscoveryNeededUtc is not null).ShouldBe(1),
Budget);
// Several more ticks pass with the same drift.
AwaitAssert(() => driver.DiscoverCount.ShouldBeGreaterThan(3), Budget);
publisher.Published.Count(p => p.RediscoveryNeededUtc is not null).ShouldBe(
1,
"an unchanged drift was re-announced; the operator prompt would keep re-stamping its timestamp");
Sys.Stop(actor);
}
/// <summary>A failed browse is not drift. A device that is merely unreachable already shows on the health
/// surface; reporting drift would turn every comms blip into "all your tags vanished".</summary>
[Fact]
public void A_failed_browse_is_not_reported_as_drift()
{
var driver = new BrowsableStubDriver(discovered: ["a"], authored: ["a", "b"]) { DiscoverThrows = true };
var publisher = new RecordingHealthPublisher();
var actor = SpawnConnected(driver, publisher);
AwaitAssert(() => driver.DiscoverCount.ShouldBeGreaterThan(1), Budget);
publisher.Published.ShouldAllBe(
p => p.RediscoveryNeededUtc == null,
"a browse that threw was treated as drift — an unreachable device would report every authored "
+ "tag as vanished");
Sys.Stop(actor);
}
/// <summary>Drift that resolves — the operator re-browsed and committed — clears the prompt, so the next
/// genuine drift re-raises with a fresh timestamp instead of being deduped against the stale one.</summary>
[Fact]
public void Drift_that_resolves_clears_the_prompt()
{
var driver = new BrowsableStubDriver(discovered: ["a", "b"], authored: ["a"]);
var publisher = new RecordingHealthPublisher();
var actor = SpawnConnected(driver, publisher);
AwaitAssert(
() => publisher.Published.Any(p => p.RediscoveryNeededUtc is not null),
Budget);
// The operator commits the new tag: authored now matches the device.
driver.SetAuthored(["a", "b"]);
AwaitAssert(
() => publisher.Published[^1].RediscoveryNeededUtc.ShouldBeNull(),
Budget);
Sys.Stop(actor);
}
private IActorRef SpawnConnected(IDriver driver, IDriverHealthPublisher publisher)
{
var parent = CreateTestProbe();
parent.IgnoreMessages(_ => true);
var actor = parent.ChildActorOf(DriverInstanceActor.Props(
driver, healthPublisher: publisher, driftCheckInterval: Tick));
actor.Tell(new DriverInstanceActor.InitializeRequested("{}"));
return actor;
}
/// <summary>Captured health publishes, so a test can read the rediscovery fields.</summary>
private sealed record HealthPublish(DateTime? RediscoveryNeededUtc, string? RediscoveryReason);
private sealed class RecordingHealthPublisher : IDriverHealthPublisher
{
private readonly List<HealthPublish> _published = [];
public IReadOnlyList<HealthPublish> Published
{
get { lock (_published) return _published.ToArray(); }
}
/// <inheritdoc />
public void Publish(
string clusterId, string driverInstanceId, DriverHealth health, int errorCount5Min,
DateTime? rediscoveryNeededUtc = null, string? rediscoveryReason = null)
{
lock (_published) _published.Add(new HealthPublish(rediscoveryNeededUtc, rediscoveryReason));
}
}
/// <summary>
/// A driver that browses a fake "device". Health is STABLE so the publish dedup genuinely engages —
/// the shared <c>StubDriver</c> returns <c>DateTime.UtcNow</c>, which would make every publish look
/// changed and let a dedup-related bug pass unnoticed.
/// </summary>
private sealed class BrowsableStubDriver(
string[] discovered,
string[]? authored,
bool supportsOnlineDiscovery = true) : IDriver, ITagDiscovery
{
private static readonly DateTime FixedLastRead = new(2026, 7, 28, 9, 0, 0, DateTimeKind.Utc);
private volatile string[]? _authored = authored;
private int _discoverCount;
/// <summary>Number of discovery passes the actor has driven — the direct read of whether the drift
/// check ran at all.</summary>
public int DiscoverCount => Volatile.Read(ref _discoverCount);
/// <summary>When true, every browse throws — models an unreachable device.</summary>
public bool DiscoverThrows { get; init; }
/// <inheritdoc />
public string DriverInstanceId => "browsable-stub-1";
/// <inheritdoc />
public string DriverType => "Stub";
/// <inheritdoc />
public bool SupportsOnlineDiscovery => supportsOnlineDiscovery;
/// <inheritdoc />
public IReadOnlyCollection<string>? AuthoredDiscoveryRefs => _authored;
/// <summary>Models the operator committing (or removing) tags between checks.</summary>
public void SetAuthored(string[] refs) => _authored = refs;
/// <inheritdoc />
public Task DiscoverAsync(IAddressSpaceBuilder builder, CancellationToken cancellationToken)
{
Interlocked.Increment(ref _discoverCount);
if (DiscoverThrows) throw new IOException("device unreachable");
var folder = builder.Folder("Stub", "Stub");
foreach (var r in discovered)
{
folder.Variable(r, r, new DriverAttributeInfo(
FullName: r,
DriverDataType: DriverDataType.Float64,
IsArray: false,
ArrayDim: null,
SecurityClass: SecurityClassification.ViewOnly,
IsHistorized: false));
}
return Task.CompletedTask;
}
/// <inheritdoc />
public Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask;
/// <inheritdoc />
public Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask;
/// <inheritdoc />
public Task ShutdownAsync(CancellationToken cancellationToken) => Task.CompletedTask;
/// <inheritdoc />
public DriverHealth GetHealth() => new(DriverState.Healthy, FixedLastRead, null);
/// <inheritdoc />
public long GetMemoryFootprint() => 0;
/// <inheritdoc />
public Task FlushOptionalCachesAsync(CancellationToken cancellationToken) => Task.CompletedTask;
}
}
tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/TagSetDriftDetectorTests.cs
-113
View File
@@ -1,113 +0,0 @@
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers;
/// <summary>
/// The pure half of tag-set drift detection: what the remote offers vs. what an operator authored.
/// Kept out of the actor so the comparison rules are testable without an actor system.
/// </summary>
[Trait("Category", "Unit")]
public sealed class TagSetDriftDetectorTests
{
/// <summary>Matching sets are not drift, and must not raise an operator prompt.</summary>
[Fact]
public void Identical_sets_are_not_drift()
{
var drift = TagSetDriftDetector.Compare(["a", "b"], ["b", "a"]);
drift.HasDrift.ShouldBeFalse();
drift.Describe().ShouldBe("no drift");
}
/// <summary>A tag the device now offers that nothing binds — the common case after a PLC re-download.</summary>
[Fact]
public void A_ref_on_the_device_that_is_not_authored_is_reported_as_appeared()
{
var drift = TagSetDriftDetector.Compare(["a", "b", "c"], ["a", "b"]);
drift.Appeared.ShouldBe(["c"]);
drift.Vanished.ShouldBeEmpty();
drift.HasDrift.ShouldBeTrue();
}
/// <summary>An authored tag the device no longer offers — detectable only for a driver whose discovery
/// stream is purely device-derived.</summary>
[Fact]
public void An_authored_ref_missing_from_the_device_is_reported_as_vanished()
{
var drift = TagSetDriftDetector.Compare(["a"], ["a", "b"]);
drift.Vanished.ShouldBe(["b"]);
drift.Appeared.ShouldBeEmpty();
}
/// <summary>
/// <b>The honesty guard.</b> When a driver re-emits its authored tags into the discovery stream —
/// FOCAS, TwinCAT and AbCip all do — the discovered set always contains the authored set, so a
/// vanished tag is structurally invisible. The detector must not compute a misleadingly-empty
/// Vanished list; it must flag that the direction is undetectable, and say so in the description.
/// </summary>
[Fact]
public void When_vanished_is_undetectable_it_is_declared_rather_than_reported_as_empty()
{
// "b" is authored and absent from the device — but this driver would have re-emitted it, so its
// absence here cannot occur in practice and must not be presented as a clean result either way.
var drift = TagSetDriftDetector.Compare(["a", "c"], ["a", "b"], detectVanished: false);
drift.VanishedDetectable.ShouldBeFalse();
drift.Vanished.ShouldBeEmpty();
drift.Appeared.ShouldBe(["c"]);
drift.Describe().ShouldContain(
"missing-tag detection unavailable",
customMessage:
"a driver that cannot see vanished tags must SAY so — otherwise the absence of a 'missing' "
+ "clause reads as 'nothing is missing', which is the reassurance this whole exercise removes");
}
/// <summary>Comparison is ordinal: a device legitimately exposing both <c>Speed</c> and <c>speed</c>
/// must not have them collapsed, which would hide a genuinely new tag.</summary>
[Fact]
public void Comparison_is_case_sensitive()
{
var drift = TagSetDriftDetector.Compare(["Speed", "speed"], ["Speed"]);
drift.Appeared.ShouldBe(["speed"]);
}
/// <summary>The signature identifies a drift so an UNCHANGED one is not re-announced every check.
/// Enumeration order must not make an identical drift look new.</summary>
[Fact]
public void Signature_is_stable_across_enumeration_order()
{
var a = TagSetDriftDetector.Compare(["x", "y", "z"], ["x"]);
var b = TagSetDriftDetector.Compare(["z", "y", "x"], ["x"]);
a.Signature.ShouldBe(b.Signature);
}
/// <summary>A different drift must get a different signature, or the second one is never reported.</summary>
[Fact]
public void Signature_changes_when_the_drift_changes()
{
var a = TagSetDriftDetector.Compare(["x", "y"], ["x"]);
var b = TagSetDriftDetector.Compare(["x", "y", "z"], ["x"]);
a.Signature.ShouldNotBe(b.Signature);
}
/// <summary>The description names a couple of refs and then counts, so swapping a whole PLC program
/// yields a readable message rather than a wall of tag names.</summary>
[Fact]
public void Describe_samples_rather_than_listing_everything()
{
var discovered = Enumerable.Range(0, 50).Select(i => $"tag{i:D2}").ToArray();
var drift = TagSetDriftDetector.Compare(discovered, []);
var text = drift.Describe();
text.ShouldContain("50 new on device");
text.ShouldContain("+48 more");
text.Length.ShouldBeLessThan(120);
}
}