From 97f4ea0c9e7f0548852d3a1044e707aeb2129ca5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:07:12 -0400 Subject: [PATCH] fix(runtime): comms loss marks raw + UNS value nodes BadNoCommunication, value retained (archreview 03/S15) --- .../plans/R3-02-stale-good-family-plan.md | 12 ++ ...R3-02-stale-good-family-plan.md.tasks.json | 2 +- .../Drivers/DriverHostActor.cs | 78 ++++++++++- ...DriverHostActorCommsLossValueNodesTests.cs | 126 ++++++++++++++++++ 4 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorCommsLossValueNodesTests.cs diff --git a/archreview/plans/R3-02-stale-good-family-plan.md b/archreview/plans/R3-02-stale-good-family-plan.md index df7da694..851e9931 100644 --- a/archreview/plans/R3-02-stale-good-family-plan.md +++ b/archreview/plans/R3-02-stale-good-family-plan.md @@ -678,3 +678,15 @@ test could only pass once the production change landed. - Metered `OpcUaSinkWrite kind:"status"` so a status annotation is separable from a value write on the same counter; the recording fake asserts **zero** value writes, since a handler falling through to the value path would null every node. + +### T8 — `DriverHostActor.FanCommsLossToValueNodes` + +- RED: `--filter "…DriverHostActorCommsLossValueNodesTests"` ⇒ **2 failed**, both + `Timeout 00:00:15 while waiting for a message of type …AttributeStatusBatchUpdate` (the disconnect + produced no value-node write at all — the finding, reproduced). +- GREEN: 10 passed across the new suite + `DriverHostActorCommsLossTests` (mux half) + + `DriverHostActorBatchFanOutTests`, all unchanged. +- **Deviation (sequencing, not design):** the plan's snippet calls `TrackCommsLatchSet(...)` from inside + this method, but that helper is T9's — calling it here would not compile. The call is added in T9, + where the latch and its audit timer land together. R3-03 boundary respected: this is a new method plus + one branch in `OnDriverConnectivityChanged`; `HandleRestartDriver` and the watchdog are untouched. diff --git a/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json b/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json index 035ba90a..4ec788d7 100644 --- a/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json +++ b/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json @@ -55,7 +55,7 @@ { "id": "T8", "subject": "DriverHostActor.FanCommsLossToValueNodes: one BadNoCommunication status batch over raw + UNS nodes on ConnectivityChanged(false); nothing on reconnect (new method — R3-03 boundary)", - "status": "pending", + "status": "done", "blockedBy": [ "T7" ] diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index e95ca092..56d918d0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -1244,13 +1244,28 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// the source had died. closes that by pushing a /// synthetic Bad for every value ref the driver owns. /// + /// + /// Archreview 03/S15 — and the EXTERNAL data plane, which the two layers above still left + /// out. Conditions and internal consumers learned of the outage; the driver's raw value + /// variables and their UNS mirrors — what an OPC UA client actually subscribes to — did not, so + /// they served the last value at Good for the whole outage. + /// closes that with one status-only batch that keeps each + /// node's value and changes only its status. All three fan-outs run on the disconnect edge only; + /// the reconnect edge writes nothing to values (recovery is the driver's own re-publish) and + /// re-annotates conditions Good, which is a statement about the LINK, not about any tag's value. + /// /// private void OnDriverConnectivityChanged(DriverInstanceActor.ConnectivityChanged msg) { // #481: the mux fan-out is independent of the OPC UA sink — a node with scripted alarms but no // publish actor (the mux-only test/dev path) must still see comms loss. Done before the early // return for exactly that reason. - if (!msg.Connected) FanCommsLossToDependencyMux(msg.DriverInstanceId); + if (!msg.Connected) + { + FanCommsLossToDependencyMux(msg.DriverInstanceId); + // 03/S15: and the EXTERNAL data plane, which #481 stopped one seam short of. + FanCommsLossToValueNodes(msg.DriverInstanceId); + } if (_opcUaPublishActor is null) return; @@ -1282,6 +1297,67 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS _localNode, msg.DriverInstanceId, msg.Connected ? "connected" : "disconnected", annotated, statusCode); } + /// + /// Archreview 03/S15 — the value-node half of the comms-loss fan-out: stamp + /// on every raw NodeId and every referencing UNS + /// NodeId the just-disconnected driver owns, as ONE status batch through the same publish actor the + /// value fan-out uses. + /// + /// + /// + /// Why it was missing. #481 built the per-driver ref index and wired comms loss to the + /// internal consumers (the mux) and to Part 9 conditions, and stopped one seam short of the + /// external OPC UA data plane. A client subscribed to a value node therefore saw the last value + /// at Good quality for the whole outage, for every driver whose failure shape is "the + /// reader stops producing" rather than "publish a Bad snapshot per tag". (Modbus does the + /// latter, quickly, which is why the gap looked smaller than it was — a coalesced-read throw, a + /// dropped session on a push driver, or simply a driver whose last act was a Good publish all + /// held Good indefinitely.) + /// + /// + /// Status-only, so the VALUE is retained. This actor holds no value cache — v3's + /// single-source design deliberately removed the independent buffer — so only the node can keep + /// the last known value, which is what + /// exists for. A Bad node still + /// carrying its last reading is OPC UA Part 8's convention and is what a SCADA consumer expects. + /// + /// + /// Nothing is written on reconnect. Publishing Good would assert a quality this actor has + /// not observed; recovery is the driver's own re-publish (see + /// 's remark on the detach that makes the poll engine + /// force-raise, and MQTT's initial push). + /// + /// + /// Ungated by redundancy role, like the condition annotation: a Secondary keeps its address + /// space honest for failover. Re-stamping an already-Bad node is harmless — the SDK's + /// StatusValue trigger dedups an unchanged status at the monitored item. + /// + /// + /// The driver child that just reported comms loss. + private void FanCommsLossToValueNodes(string driverInstanceId) + { + if (_opcUaPublishActor is null) return; + if (!_valueRefsByDriver.TryGetValue(driverInstanceId, out var entries) || entries.Count == 0) return; + + var ts = DateTime.UtcNow; + var writes = new List(entries.Count); + foreach (var (_, nodes) in entries) + { + foreach (var n in nodes) + { + writes.Add(new StatusWrite(n.NodeId, OpcUaStatus.BadNoCommunication, ts, n.Realm)); + } + } + + if (writes.Count == 0) return; + + _opcUaPublishActor.Tell(new OpcUa.OpcUaPublishActor.AttributeStatusBatchUpdate(writes)); + + _log.Info( + "DriverHost {Node}: driver {Driver} disconnected — {Count} value node(s) marked BadNoCommunication (value retained)", + _localNode, driverInstanceId, writes.Count); + } + /// /// #481 (Layer 4) — push a synthetic into the dependency /// mux for every value ref the just-disconnected driver owns, so scripted alarms and virtual tags see diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorCommsLossValueNodesTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorCommsLossValueNodesTests.cs new file mode 100644 index 00000000..a6e2b528 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorCommsLossValueNodesTests.cs @@ -0,0 +1,126 @@ +using Akka.Actor; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Commons.Types; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.OpcUa; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; +using static ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness.V3ArtifactSeeder; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// Archreview 03/S15 — the value-node half of the comms-loss fan-out. Until this, a driver +/// declared comms-lost annotated its Part 9 conditions and pushed a synthetic Bad into the dependency +/// mux, but its raw and UNS value variables — the product's external surface — got nothing: an +/// OPC UA client subscribed to a value node saw the last value at Good quality for the whole +/// outage, for every driver whose failure shape is "reader stops producing" rather than "per-tag Bad +/// snapshot". +/// +/// Status-only, value retained. The write goes through +/// so the node keeps its last known value +/// under BadNoCommunication (OPC UA Part 8's convention). The host holds no value cache — v3's +/// single-source design — so only the node itself can retain it. +/// +/// Nothing is published on reconnect, deliberately: publishing Good would assert a +/// quality this actor has not observed. Recovery is the driver's own re-publish (the poll engine's +/// forceRaise on a fresh subscription, or the push-model initial push). That contract is +/// end-to-end tested in . +/// +public sealed class DriverHostActorCommsLossValueNodesTests : RuntimeActorTestBase +{ + private static readonly NodeId TestNode = NodeId.Parse("driver-commsloss-values"); + private static readonly RevisionHash RevA = RevisionHash.Parse(new string('c', 64)); + + /// + /// A disconnect produces ONE status batch naming exactly the disconnected driver's raw nodes plus + /// every referencing UNS mirror, all BadNoCommunication. The second driver's ref is the + /// control: a fan-out that walked every value ref on the node instead of the per-driver index would + /// mark a perfectly healthy driver's nodes Bad, and no assertion about the first driver's nodes + /// would notice. + /// + [Fact] + public void Disconnect_marks_the_drivers_raw_and_uns_value_nodes_BadNoCommunication() + { + 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), + (Driver: "drv-2", DriverName: "S7", Device: "dev2", Tag: "other", DataType: "Double", Writable: false), + }, + unsRefs: new[] { (Area: "filling", Line: "line1", Equip: "station1", BackingTag: "speed", Effective: (string?)null) }); + + var (actor, publish, _) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); + + actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-1", Connected: false)); + + var batch = publish.ExpectMsg(PresenceBudget); + batch.Items.Select(i => i.NodeId).ShouldBe(new[] + { + "Plant/Modbus/dev1/speed", + "filling/line1/station1/speed", + "Plant/Modbus/dev1/temp", + }, ignoreOrder: true); + + batch.Items.ShouldAllBe(i => i.StatusCode == OpcUaStatus.BadNoCommunication); + batch.Items.Single(i => i.NodeId == "Plant/Modbus/dev1/speed").Realm.ShouldBe(AddressSpaceRealm.Raw); + batch.Items.Single(i => i.NodeId == "filling/line1/station1/speed").Realm.ShouldBe(AddressSpaceRealm.Uns); + + // The control: the OTHER driver's node is untouched by drv-1's outage. + batch.Items.ShouldNotContain(i => i.NodeId == "Plant/S7/dev2/other"); + + // ONE batch, not one message per node. + publish.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + } + + /// + /// A reconnect publishes nothing to the value nodes. Writing Good here would claim a quality + /// the host has not observed — the device may be answering the transport and still returning + /// garbage — and it would race the driver's own first real value. + /// + [Fact] + public void Reconnect_publishes_no_value_or_status_message() + { + 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, publish, _) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); + + actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-1", Connected: false)); + publish.ExpectMsg(PresenceBudget); + + actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-1", Connected: true)); + + publish.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); + } + + /// + /// A driver that owns no value refs produces no message at all — an empty batch would cost a hop + /// and a sink call for nothing on every flap of an alarm-only or freshly-added driver. + /// + [Fact] + public void A_driver_with_no_value_refs_publishes_nothing() + { + 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, publish, _) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA); + + actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-unknown", Connected: false)); + + publish.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); + } +}