From 17dc17194038ae15c25b28a67d6f3706663f52db Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:20:13 -0400 Subject: [PATCH] fix(opcua): carry the condition Comment across a rebuild; the restore report never rolls Time back (#641 review) Native conditions support AddComment / commented Acknowledge, so the comment (and its ClientUserId) is condition state the rebuild dropped. And the deferred restore report re-stamped Time with the carried instant even when a client method call had reported a newer event in the restore->complete window; it now reports at the node's current Time. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../OtOpcUaNodeManager.cs | 22 ++++++++++---- ...ManagerNativeAlarmRebuildCarryOverTests.cs | 29 +++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 9132653a..3584a5f3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -74,7 +74,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// event at (deferred until then so every notifier folder the /// apply wires receives it). Each entry pins the instance restored, so a condition replaced in between is /// not reported. Guarded by Lock. - private readonly List<(RealmKey Key, AlarmConditionState Alarm, DateTime Time)> _restoredConditionReports = new(); + private readonly List<(RealmKey Key, AlarmConditionState Alarm)> _restoredConditionReports = new(); /// Phase C: NodeId → resolved historian tagname for every variable materialised /// Historizing. Populated by when a historian tagname is supplied; the /// (later) HistoryRead override resolves a HistoryRead request's NodeId against this map. Cleared on @@ -1459,8 +1459,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// The condition's Time — the instant its current state came about. /// When it last changed Active, if the SDK recorded it. /// The source timestamp of its Quality annotation, if present. + /// The condition's Comment (a client's ack / AddComment text), if any. + /// Who left that comment, if recorded. private sealed record NativeConditionCarryOver( - AlarmConditionDelta State, DateTime Time, DateTime? ActiveTransitionTime, DateTime? QualityTimestamp); + AlarmConditionDelta State, DateTime Time, DateTime? ActiveTransitionTime, DateTime? QualityTimestamp, + LocalizedText? Comment = null, string? ClientUserId = null); /// /// Gitea #641 — read what a native condition node holds right now, so the rebuild can put it back. @@ -1485,7 +1488,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 ReadConditionDelta(condition), condition.Time?.Value ?? DateTime.UtcNow, condition.ActiveState?.TransitionTime?.Value, - condition.Quality?.SourceTimestamp?.Value); + condition.Quality?.SourceTimestamp?.Value, + condition.Comment?.Value, + condition.ClientUserId?.Value); /// /// Gitea #641 — put a carried state back onto the freshly materialised native condition. MUST be called @@ -1532,10 +1537,12 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 alarm.SetSeverity(SystemContext, (EventSeverity)state.MappedSeverity); alarm.Message.Value = new LocalizedText(state.Message); alarm.Retain.Value = true; + if (alarm.Comment is not null && carried.Comment is not null) alarm.Comment.Value = carried.Comment; + if (alarm.ClientUserId is not null && carried.ClientUserId is not null) alarm.ClientUserId.Value = carried.ClientUserId; alarm.Time.Value = carried.Time; alarm.ReceiveTime.Value = carried.Time; - _restoredConditionReports.Add((conditionKey, alarm, carried.Time)); + _restoredConditionReports.Add((conditionKey, alarm)); } /// One materialised native-alarm condition together with the equipment folders wired as extra @@ -3376,10 +3383,13 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 int reported = 0, discarded; lock (Lock) { - foreach (var (key, alarm, time) in _restoredConditionReports) + foreach (var (key, alarm) in _restoredConditionReports) { if (!_alarmConditions.TryGetValue(key, out var live) || !ReferenceEquals(live, alarm)) continue; - ReportConditionEvent(alarm, time); + // The node's CURRENT Time: the carried instant when nothing reported since the restore, or the + // newer instant of a report that did (a client method call on an SDK thread can land between the + // restore and here) — never roll the condition's Time back. + ReportConditionEvent(alarm, alarm.Time.Value); reported++; } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs index 673fbef2..257b89bb 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs @@ -94,6 +94,35 @@ public sealed class NodeManagerNativeAlarmRebuildCarryOverTests : IDisposable await host.DisposeAsync(); } + /// + /// Gitea #641 review — the comment a client left with its acknowledgement is condition state too (native + /// conditions support AddComment), and an event the condition reported AFTER the restore (a client method + /// call landing on an SDK thread before the apply completes) must not have its newer Time rolled + /// back by the deferred restore report. + /// + [Trait("Category", "Unit")] + [Fact] + public async Task The_comment_is_carried_and_a_newer_report_is_not_rolled_back() + { + var (host, nm) = await BootAsync(); + var raisedAt = new DateTime(2026, 10, 8, 9, 0, 0, DateTimeKind.Utc); + Materialise(nm, native: true); + nm.WriteAlarmCondition(Alarm, Fault(acked: false), raisedAt, AddressSpaceRealm.Raw); + Condition(nm).Comment.Value = new LocalizedText("on it"); + + nm.RebuildAddressSpace(); + Materialise(nm, native: true); + var later = raisedAt.AddMinutes(5); + nm.WriteAlarmQuality(Alarm, StatusCodes.BadNoCommunication, later, AddressSpaceRealm.Raw); // reports at `later` + nm.CompleteAddressSpaceApply(); + + var restored = Condition(nm); + restored.Comment.Value.Text.ShouldBe("on it"); + restored.Time.Value.ShouldBe(later, "the restore report must not stamp the condition back to the raise time"); + + await host.DisposeAsync(); + } + /// Falsification control: a condition in the no-event position comes back exactly as a fresh /// materialise builds it, and reports nothing. [Trait("Category", "Unit")]