diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs index 6e29f09e..b9d6e513 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs @@ -43,7 +43,12 @@ public sealed class VirtualTagActor : ReceiveActor /// when the script failed/timed out (02/S13) — in which case /// is the last-known value (or null if none). Defaulting to Good keeps /// every existing construction site source-compatible. - public sealed record EvaluationResult(string VirtualTagId, object? Value, DateTime TimestampUtc, CorrelationId Correlation, OpcUaQuality Quality = OpcUaQuality.Good); + /// true when this result is a deploy-time re-assertion of the last + /// value/quality (see ) rather than a fresh evaluation. The host still + /// publishes it (to repair the reset OPC UA node) but must NOT re-record it to the historian — it is a + /// stale value stamped with a fresh deploy-time timestamp, never a real evaluation sample. Defaults to + /// false so every existing construction site stays source-compatible. + public sealed record EvaluationResult(string VirtualTagId, object? Value, DateTime TimestampUtc, CorrelationId Correlation, OpcUaQuality Quality = OpcUaQuality.Good, bool IsReassert = false); private readonly string _virtualTagId; private readonly string _scriptId; @@ -134,15 +139,26 @@ public sealed class VirtualTagActor : ReceiveActor /// deploy that re-materialised our published UNS node to BadWaitingForInitialData recovers even when /// the recomputed value is unchanged (permanent Bad otherwise, with a static dependency). No-op until the /// child has emitted at least once — the first dependency arrival publishes the initial value normally. - /// Ordering is safe: DriverHostActor enqueues the address-space RebuildAddressSpace (which - /// re-materialises the node) to the single-threaded publish actor BEFORE the ApplyVirtualTags that - /// triggers this re-assert, so the re-publish lands on the freshly-materialised node. + /// + /// The result is flagged so the host publishes it (repairing + /// the reset node) but does NOT re-record it to the historian — it is a stale value stamped with a + /// fresh deploy-time timestamp, not a real evaluation sample (M1). + /// + /// + /// Ordering (why the re-publish lands on the freshly-materialised node, not the pre-reset one): the + /// reset and this re-publish converge on the SAME single-threaded OpcUaPublishActor. On a deploy + /// DriverHostActor enqueues RebuildAddressSpace (the reset) to that actor FIRST, then sends + /// ApplyVirtualTags to the VirtualTag HOST; the host tells this child ReassertValue, and + /// only then does this method Tell the host an EvaluationResult which the host bridges onward to + /// the publish actor. That multi-hop chain guarantees the re-publish arrives AFTER RebuildAddressSpace + /// in the publish actor's FIFO mailbox, so the materialise runs before the write. + /// private void OnReassertValue() { if (!_hasLastValue && !_lastPublishedBad) return; var quality = _lastPublishedBad ? OpcUaQuality.Bad : OpcUaQuality.Good; Context.Parent.Tell(new EvaluationResult( - _virtualTagId, _lastValue, DateTime.UtcNow, CorrelationId.NewId(), quality)); + _virtualTagId, _lastValue, DateTime.UtcNow, CorrelationId.NewId(), quality, IsReassert: true)); } private void OnDependencyChanged(DependencyValueChanged msg) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs index 7326b74d..50592064 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs @@ -182,13 +182,17 @@ public sealed class VirtualTagHostActor : ReceiveActor } // Re-assert last values on surviving (not just-spawned) children. Every apply coincides with a - // deploy, and DriverHostActor enqueues the address-space RebuildAddressSpace — which re-materialises - // each VirtualTag's UNS node to BadWaitingForInitialData — to the single-threaded publish actor - // BEFORE this ApplyVirtualTags. A surviving child keeps its value-dedup state, so an unchanged - // recompute is suppressed and its freshly-reset node would stay Bad forever (permanent with a static - // dependency). Telling each survivor to ReassertValue re-publishes its last state through the same - // publish actor AFTER the materialise, so the node recovers. Fresh children are skipped (nothing to - // re-assert; their first dependency arrival publishes normally). + // deploy, and on a deploy DriverHostActor enqueues RebuildAddressSpace — which re-materialises each + // VirtualTag's UNS node to BadWaitingForInitialData — to the single-threaded OpcUaPublishActor FIRST, + // then sends ApplyVirtualTags to THIS host. A surviving child keeps its value-dedup state, so an + // unchanged recompute is suppressed and its freshly-reset node would stay Bad forever (permanent with + // a static dependency). Telling each survivor to ReassertValue makes it re-publish its last state; that + // re-publish reaches the publish actor via a multi-hop chain (host → child ReassertValue → child → + // parent EvaluationResult → OnResult → publish actor), so its AttributeValueUpdate lands AFTER + // RebuildAddressSpace in the publish actor's FIFO mailbox — the materialise runs before the write, and + // the node recovers. (The ordering does NOT come from ApplyVirtualTags itself reaching the publish + // actor — it doesn't; it goes to this host.) Fresh children are skipped (nothing to re-assert; their + // first dependency arrival publishes normally). foreach (var (vtagId, child) in _children) { if (newlySpawned.Contains(vtagId)) continue; @@ -215,12 +219,16 @@ public sealed class VirtualTagHostActor : ReceiveActor _publishActor.Tell(new OpcUaPublishActor.AttributeValueUpdate( nodeId, result.Value, result.Quality, result.TimestampUtc, AddressSpaceRealm.Uns)); - // Historize iff the plan opted in. Reuses _planByVtag (kept in lock-step with _children), so - // no parallel map. The historian path key is the SAME folder-scoped NodeId we just published - // to. For a computed value source == server, so both timestamps are the evaluation time. Bad - // results record BadInternalError (0x80020000u) — the dormant VirtualTagEngine's Bad-snapshot - // status — so the historian trend can distinguish a degraded sample from a Good one. - if (_planByVtag.TryGetValue(result.VirtualTagId, out var plan) && plan.Historize) + // Historize iff the plan opted in — but NEVER for a re-assert (M1). A re-assert re-publishes a + // stale last value stamped with a fresh deploy-time timestamp purely to repair the reset OPC UA + // node; recording it would append an artificial historian sample that never corresponded to a real + // evaluation (and, if the last state was Bad, a fabricated BadInternalError point). Reuses + // _planByVtag (kept in lock-step with _children), so no parallel map. The historian path key is the + // SAME folder-scoped NodeId we just published to. For a computed value source == server, so both + // timestamps are the evaluation time. Bad results record BadInternalError (0x80020000u) — the + // dormant VirtualTagEngine's Bad-snapshot status — so the historian trend can distinguish a degraded + // sample from a Good one. + if (!result.IsReassert && _planByVtag.TryGetValue(result.VirtualTagId, out var plan) && plan.Historize) { var status = result.Quality == OpcUaQuality.Good ? 0u : 0x80020000u /* BadInternalError — dormant-engine parity */; _history.Record(nodeId, new DataValueSnapshot( diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs index cf86cf86..d74f7276 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs @@ -519,6 +519,41 @@ public sealed class VirtualTagHostActorTests : RuntimeActorTestBase mux.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); } + /// + /// M1 (reassert review): a re-assert must PUBLISH (to repair the reset OPC UA node) but must NOT + /// re-record to the historian. Otherwise every deploy appends an artificial historian sample carrying a + /// stale value with a fresh deploy-time timestamp — a data-quality bug the moment a VT history sink is + /// bound. Historize=true survivor: first real evaluation records exactly once; the redeploy's re-assert + /// publishes a second time but the historian count stays 1. + /// + [Fact] + public void Reassert_of_a_historized_vtag_publishes_but_does_not_re_record_to_the_historian() + { + var publish = CreateTestProbe(); + var mux = CreateTestProbe(); + var writer = new CapturingHistoryWriter(); + var host = Sys.ActorOf(VirtualTagHostActor.Props(publish.Ref, mux.Ref, new ConstOkEvaluator(7.0), writer)); + + var plan = new[] { PlanH("vt-1", "eq-1", "speed-rpm", historize: true) }; + host.Tell(new VirtualTagHostActor.ApplyVirtualTags(plan)); + + mux.ExpectMsg(); + var child = mux.LastSender; + + // First real evaluation → Good publish + exactly one historian record. + child.Tell(new VirtualTagActor.DependencyValueChanged("a", 1, DateTime.UtcNow)); + publish.ExpectMsg(); + AwaitAssert(() => writer.Calls.Count.ShouldBe(1)); + + // Redeploy (unchanged plan) → the survivor re-asserts: a SECOND publish repairs the reset node. + host.Tell(new VirtualTagHostActor.ApplyVirtualTags(plan)); + publish.ExpectMsg(); + + // The publish landing means the reassert's OnResult turn is complete (Record runs on the same turn, + // right after the Tell) — so a second Record would already be visible. It must NOT be: still 1. + writer.Calls.Count.ShouldBe(1); + } + /// Evaluator that always returns Ok(value) — a static-output VirtualTag whose recompute /// is dedup-suppressed after the first publish, so the re-assert path is the only way its reset node /// recovers.