feat(alarms): bridge driver comms-loss into scripted-alarm input quality (#481)
Layer 4 of the condition-Quality chain (#477 -> #478). #478 gave a scripted condition the worst quality of its input tags, but only for inputs whose driver PUBLISHES a Bad data change. A poll driver whose device goes unreachable publishes nothing — it raises ConnectivityChanged(false) and goes silent — so the engine held its last Good value, WorstInputStatus stayed Good, and worse, AreInputsReady stayed true, so the predicate went on evaluating a stale value after the source had died. `DriverHostActor.FanCommsLossToDependencyMux` pushes a synthetic BadNoCommunication with a null value into the dependency mux for every value ref the disconnected driver owns. The issue asked for a driver -> all-tag-refs map to be built for this. It already exists: `_nodeIdByDriverRef`, keyed (DriverInstanceId, RawPath) and rebuilt from the composition on every apply. Alarm-bearing tags live in the separate `_alarmNodeIdByDriverRef` and are deliberately excluded — a native condition is not a mux value and takes the Part 9 quality path instead. Nothing is pushed on reconnect, deliberately: publishing Good would assert a value quality this actor has not observed. Recovery rides the driver's own republish, and the mechanism was verified rather than assumed — the Connected -> Reconnecting edge calls DetachSubscription, which nulls the subscription handle, so the reconnect's ResubscribeDesired builds a FRESH subscription whose PollGroupEngine initial push runs with forceRaise: true. Without that detach the poll engine would compare against its retained LastValues, see an unchanged value, raise nothing, and strand these refs Bad forever — the value-dedup trap that bit the v3 Batch 4 virtual-tag reassert. Closes #478 Finding 3, which the above depends on. WorstInputStatus skipped every input whose VALUE was null as "no data yet" — and Bad-with-no-value is exactly the shape a comms-lost driver produces, so the skip was hiding the case that matters most; without this half the fan-out above would be silently inert. The upstream source now stamps a never-pushed path BadWaitingForInitialData — the honest Part 4 code for that state — and the engine skips that specific code rather than every null value. Deleting the null-skip outright is the obvious fix and is wrong in the other direction: it makes every not-yet-published input drag its condition Bad at deploy, flooding the quality path with load-time annotations, which is what the skip existed to prevent. Both mistakes are pinned by a test pair that goes red under the opposite error, and both were falsified by reverting the production code: old null-skip restored -> A_pushed_Bad_carrying_no_value RED null-skip deleted outright -> An_input_that_was_never_pushed RED fan-out call removed -> the three DriverHostActor tests RED (1 of 7) The never-pushed test was vacuous on first writing and is worth recording: the quality bucket only emits on CHANGE, so the load-time evaluation is the one that flashes Bad, and subscribing to OnEvent after LoadAsync made the assertion unfalsifiable — it passed against the broken behaviour. It now subscribes first. `FakeUpstream.ReadTag` returned 0x80340000 (BadNotConnected) for an unset path, a different Bad code from production's placeholder. Harmless while any Bad behaved alike; load-bearing now, so it is aligned and pinned by a reflection guard asserting the engine's private constant agrees with what the production source stamps. Behaviour change to weigh, flagged in the issue and confirmed: virtual tags computed from a comms-lost input now go Bad too. That follows from using the shared mux and reads as correct, but it is broader than scripted alarms. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
This commit is contained in:
Joseph Doherty
@@ -38,11 +38,23 @@ public sealed class FakeUpstream : ITagUpstreamSource
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Reads the current value of a tag, or returns a bad-status snapshot if not set.</summary>
|
||||
/// <summary>
|
||||
/// OPC UA <c>BadWaitingForInitialData</c> — the status a never-pushed path reports, matching what
|
||||
/// the production <c>DependencyMuxTagUpstreamSource</c> stamps.
|
||||
/// <para>This used to be <c>0x80340000</c> (<c>BadNotConnected</c>), a different Bad code from the
|
||||
/// one production uses. That drift was invisible while <c>WorstInputStatus</c> skipped every
|
||||
/// null-valued entry — any Bad code behaved the same. #481 made the specific code load-bearing (it
|
||||
/// is how cold start is told apart from a driver reporting Bad), so a fake disagreeing with
|
||||
/// production here would let every alarm with an unset input report Bad in tests and Good in the
|
||||
/// real runtime, or vice versa.</para>
|
||||
/// </summary>
|
||||
public const uint StatusNeverPushed = 0x80320000u;
|
||||
|
||||
/// <summary>Reads the current value of a tag, or returns a never-pushed snapshot if not set.</summary>
|
||||
/// <param name="path">The tag path to read.</param>
|
||||
public DataValueSnapshot ReadTag(string path)
|
||||
=> _values.TryGetValue(path, out var v) ? v
|
||||
: new DataValueSnapshot(null, 0x80340000u, null, DateTime.UtcNow);
|
||||
: new DataValueSnapshot(null, StatusNeverPushed, null, DateTime.UtcNow);
|
||||
|
||||
/// <summary>Subscribes an observer to tag changes for the given path.</summary>
|
||||
/// <param name="path">The tag path to subscribe to.</param>
|
||||
|
||||
@@ -1103,6 +1103,75 @@ public sealed class ScriptedAlarmEngineTests
|
||||
events.ShouldNotContain(e => e.Emission == EmissionKind.QualityChanged);
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// #481 — cold start vs. a driver reporting Bad (closes #478 Finding 3)
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
/// <summary>
|
||||
/// A driver reporting Bad with no value counts as Bad quality.
|
||||
/// <para>This is the hole #478 Finding 3 recorded. <c>WorstInputStatus</c> skipped every entry whose
|
||||
/// value was null as "no data yet", which is indistinguishable from a real fault when the fault
|
||||
/// carries no value — and a null-valued Bad is exactly what a comms-lost driver produces, so the
|
||||
/// case being skipped was the case that mattered most. Before the fix this test sees no
|
||||
/// QualityChanged at all.</para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task A_pushed_Bad_carrying_no_value_counts_as_Bad_quality()
|
||||
{
|
||||
var up = new FakeUpstream();
|
||||
up.Set("Temp", 50); // Good to start, so the bucket has somewhere to move from
|
||||
using var eng = Build(up, out _);
|
||||
await eng.LoadAsync([Alarm("HighTemp", """return (int)ctx.GetTag("Temp").Value > 100;""")],
|
||||
TestContext.Current.CancellationToken);
|
||||
|
||||
var events = new List<ScriptedAlarmEvent>();
|
||||
eng.OnEvent += (_, e) => events.Add(e);
|
||||
|
||||
// The shape DriverHostActor pushes on ConnectivityChanged(false): Bad, no value.
|
||||
up.Push("Temp", null, 0x80310000u); // BadNoCommunication
|
||||
await WaitForAsync(() => events.Any(e => e.Emission == EmissionKind.QualityChanged));
|
||||
|
||||
var q = events.First(e => e.Emission == EmissionKind.QualityChanged);
|
||||
(q.WorstInputStatusCode >> 30).ShouldBeGreaterThanOrEqualTo(2u);
|
||||
// The sub-code survives, so an operator can tell comms loss from any other fault.
|
||||
q.WorstInputStatusCode.ShouldBe(0x80310000u);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// …and the protection the old null-skip existed for still holds: an input nothing has ever pushed
|
||||
/// does NOT drag the condition Bad.
|
||||
/// <para>The falsifying control for the test above. Deleting the null-skip outright — the obvious
|
||||
/// way to make a null-valued Bad count — would also make every not-yet-published input count, which
|
||||
/// is what the skip was there to prevent: every scripted condition flashing Bad at deploy until its
|
||||
/// first push, flooding the quality path with load-time annotations. Both tests pass only if the
|
||||
/// engine keys off the never-pushed status code specifically.</para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task An_input_that_was_never_pushed_does_not_drag_the_condition_Bad()
|
||||
{
|
||||
var up = new FakeUpstream();
|
||||
up.Set("Temp", 50);
|
||||
// "Spare" is never Set or Pushed, so the engine's cache refill resolves it through ReadTag and
|
||||
// gets the never-pushed placeholder — value null, Bad severity, the same shape a genuine fault has.
|
||||
using var eng = Build(up, out _);
|
||||
|
||||
// Subscribed BEFORE LoadAsync, deliberately. The quality bucket is tracked per alarm and only a
|
||||
// CHANGE emits, so the load-time evaluation is the one that would flash Bad — subscribing after it
|
||||
// makes this assertion vacuous, and the first draft of this test was exactly that: it passed
|
||||
// against the broken behaviour because the flash had already happened by the time it was listening.
|
||||
var events = new List<ScriptedAlarmEvent>();
|
||||
eng.OnEvent += (_, e) => events.Add(e);
|
||||
|
||||
await eng.LoadAsync(
|
||||
[Alarm("HighTemp", """return (int)ctx.GetTag("Temp").Value > 100 || (int?)ctx.GetTag("Spare").Value == 1;""")],
|
||||
TestContext.Current.CancellationToken);
|
||||
|
||||
up.Push("Temp", 60, 0u);
|
||||
await Task.Delay(150, TestContext.Current.CancellationToken);
|
||||
|
||||
events.ShouldNotContain(e => e.Emission == EmissionKind.QualityChanged);
|
||||
}
|
||||
|
||||
private static async Task WaitForAsync(Func<bool> cond, int timeoutMs = 2000)
|
||||
{
|
||||
var deadline = DateTime.UtcNow.AddMilliseconds(timeoutMs);
|
||||
|
||||
@@ -151,6 +151,94 @@ public sealed class DriverHostActorLiveValueTests : RuntimeActorTestBase
|
||||
.FullReference.ShouldBe("Plant/Modbus/dev1/nope");
|
||||
}
|
||||
|
||||
// ---- #481 (Layer 4): comms loss reaches the dependency mux ------------------------------------
|
||||
//
|
||||
// #478 gave a scripted condition the worst quality of its inputs, but only for inputs whose driver
|
||||
// PUBLISHES a Bad data change. A poll driver whose device goes unreachable publishes nothing — it
|
||||
// raises ConnectivityChanged(false) and goes silent — so the engine held its last Good value and
|
||||
// went on evaluating it. These pin the synthetic Bad that closes that hole.
|
||||
|
||||
/// <summary>Comms loss pushes a Bad for every value ref the driver owns — not one, and not the
|
||||
/// alarm-bearing ones (those take the Part 9 quality path instead).</summary>
|
||||
[Fact]
|
||||
public void Driver_comms_loss_pushes_BadNoCommunication_into_the_mux_for_every_ref_it_owns()
|
||||
{
|
||||
var db = NewInMemoryDbFactory();
|
||||
var deploymentId = SeedV3Deployment(db, RevA,
|
||||
rawTags: new[]
|
||||
{
|
||||
(Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false),
|
||||
(Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "temp", DataType: "Double", Writable: false),
|
||||
},
|
||||
unsRefs: Array.Empty<(string, string, string, string, string?)>());
|
||||
|
||||
var (actor, _, mux) = SpawnHostAndApply(db, deploymentId);
|
||||
|
||||
actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-1", Connected: false));
|
||||
|
||||
var pushes = new[]
|
||||
{
|
||||
mux.ExpectMsg<DriverInstanceActor.AttributeValuePublished>(TimeSpan.FromSeconds(5)),
|
||||
mux.ExpectMsg<DriverInstanceActor.AttributeValuePublished>(TimeSpan.FromSeconds(5)),
|
||||
};
|
||||
|
||||
pushes.Select(p => p.FullReference).ShouldBe(
|
||||
new[] { "Plant/Modbus/dev1/speed", "Plant/Modbus/dev1/temp" }, ignoreOrder: true);
|
||||
|
||||
foreach (var p in pushes)
|
||||
{
|
||||
// BadNoCommunication, not the bare Bad: #519's reasoning applies here too — the sub-code
|
||||
// names the actual cause and reaches a client through the scripted condition's quality.
|
||||
p.StatusCode.ShouldBe(OpcUaStatus.BadNoCommunication);
|
||||
// Null rather than the last-known value: the honest statement is "no current value". The
|
||||
// engine's AreInputsReady gate holds the predicate on any Bad, so no script sees the null.
|
||||
p.Value.ShouldBeNull();
|
||||
p.DriverInstanceId.ShouldBe("drv-1");
|
||||
}
|
||||
|
||||
mux.ExpectNoMsg(TimeSpan.FromMilliseconds(300));
|
||||
}
|
||||
|
||||
/// <summary>Reconnect pushes NOTHING — and this is the falsifying control for the test above, since
|
||||
/// both drive the same handler over the same populated map and only one may produce traffic.</summary>
|
||||
[Fact]
|
||||
public void Driver_reconnect_pushes_nothing_into_the_mux()
|
||||
{
|
||||
// Publishing Good here would assert a value quality the host has not observed. Recovery instead
|
||||
// rides the driver's own republish: the Connected -> Reconnecting edge calls DetachSubscription,
|
||||
// so the reconnect builds a FRESH subscription whose PollGroupEngine initial push runs with
|
||||
// forceRaise: true and re-publishes every tag with its real quality. Without that detach the poll
|
||||
// engine would compare against retained LastValues, see an unchanged value, raise nothing, and
|
||||
// strand these refs Bad forever — the value-dedup trap from the v3 Batch 4 virtual-tag reassert.
|
||||
var db = NewInMemoryDbFactory();
|
||||
var deploymentId = SeedV3Deployment(db, RevA,
|
||||
rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) },
|
||||
unsRefs: Array.Empty<(string, string, string, string, string?)>());
|
||||
|
||||
var (actor, _, mux) = SpawnHostAndApply(db, deploymentId);
|
||||
|
||||
actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-1", Connected: true));
|
||||
|
||||
mux.ExpectNoMsg(TimeSpan.FromMilliseconds(500));
|
||||
}
|
||||
|
||||
/// <summary>Comms loss for a driver this host knows nothing about pushes nothing — the fan-out is
|
||||
/// scoped by DriverInstanceId, so one dead device cannot Bad-out another driver's tags.</summary>
|
||||
[Fact]
|
||||
public void Comms_loss_for_another_driver_pushes_nothing_into_the_mux()
|
||||
{
|
||||
var db = NewInMemoryDbFactory();
|
||||
var deploymentId = SeedV3Deployment(db, RevA,
|
||||
rawTags: new[] { (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false) },
|
||||
unsRefs: Array.Empty<(string, string, string, string, string?)>());
|
||||
|
||||
var (actor, _, mux) = SpawnHostAndApply(db, deploymentId);
|
||||
|
||||
actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-OTHER", Connected: false));
|
||||
|
||||
mux.ExpectNoMsg(TimeSpan.FromMilliseconds(500));
|
||||
}
|
||||
|
||||
/// <summary>Spawns the host with publish + mux probes, dispatches the deployment, and waits for the
|
||||
/// Applied ACK so the map build in PushDesiredSubscriptions has completed before the test publishes a
|
||||
/// value. A VirtualTag-host probe is injected so the real host isn't spawned.</summary>
|
||||
|
||||
+31
@@ -1,5 +1,6 @@
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
using ZB.MOM.WW.OtOpcUa.Runtime.ScriptedAlarms;
|
||||
|
||||
@@ -40,6 +41,36 @@ public sealed class DependencyMuxTagUpstreamSourceTests
|
||||
snap.StatusCode.ShouldNotBe(0u);
|
||||
// Timestamp comes from the injected clock.
|
||||
snap.ServerTimestampUtc.ShouldBe(fixedNow);
|
||||
|
||||
// #481: the SPECIFIC code is load-bearing, not merely "some Bad". ScriptedAlarmEngine skips
|
||||
// exactly this code when computing worst-of-input quality, which is how it tells a path nothing
|
||||
// has ever pushed to apart from a driver reporting Bad with no value — the shape a comms-lost
|
||||
// driver produces, and previously indistinguishable from cold start. Any other Bad code here
|
||||
// would make every not-yet-published input drag its condition Bad at deploy.
|
||||
snap.StatusCode.ShouldBe(OpcUaStatus.BadWaitingForInitialData);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The engine's private copy of the never-pushed sentinel agrees with the one this source stamps.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <c>Core.ScriptedAlarms</c> deliberately does not reference <c>Commons</c>, so the engine inlines
|
||||
/// the constant rather than sharing <see cref="OpcUaStatus"/>. The value is fixed by the OPC UA spec
|
||||
/// so it cannot legitimately drift, but a typo in either copy would silently un-wire #481: the
|
||||
/// engine would stop recognising the placeholder and every cold-start input would report Bad. Read
|
||||
/// by reflection because the engine's copy is private — deliberately, since nothing outside the
|
||||
/// engine should key off it.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public void The_engine_agrees_with_this_source_on_the_never_pushed_status_code()
|
||||
{
|
||||
var engineConst = typeof(Core.ScriptedAlarms.ScriptedAlarmEngine)
|
||||
.GetField("StatusNeverPushed", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)
|
||||
.ShouldNotBeNull("ScriptedAlarmEngine.StatusNeverPushed is gone or was renamed — #481's cold-start "
|
||||
+ "vs. reported-Bad distinction is wired through it, so this guard must be repointed, not deleted");
|
||||
|
||||
engineConst.GetRawConstantValue().ShouldBe(
|
||||
new DependencyMuxTagUpstreamSource().ReadTag("never/pushed").StatusCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
Reference in New Issue
Block a user