diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index d81bf796..f125cedf 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -240,9 +240,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { // Fresh GUID-bytes EventId per event — mandatory for Part 9 ack correlation (T17 relies on it). alarm.EventId.Value = Guid.NewGuid().ToByteArray(); - // Time/ReceiveTime/Message/Severity were set by the WriteAlarmCondition projection above; restamp - // Time/ReceiveTime here so this event carries this transition's instant even if the caller's - // projection ordering changes later. + // Time/ReceiveTime were already set to sourceTimestampUtc by the WriteAlarmCondition projection + // immediately above; the assignment here is a locality repeat (same value, no behavioral change) + // so the restamp is co-located with the EventId and ClearChangeMasks in the same method. alarm.Time.Value = ts; alarm.ReceiveTime.Value = ts; @@ -289,6 +289,31 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// Shelving kind is read back from / mapped to the SDK's shelving state machine so the /// live node and the snapshot compare on the same 3-way (Unshelved/OneShot/Timed) axis. /// + /// + /// Why CommentAdded is not a field here (intentional). + /// EmissionKind.CommentAdded is produced only by + /// Part9StateMachine.ApplyAddComment, which is reached only via + /// ScriptedAlarmEngine.AddCommentAsync, which is called only from + /// ScriptedAlarmHostActor's inbound AlarmCommand handler — meaning + /// CommentAdded ALWAYS originates from a client calling the condition's + /// AddComment method. On that path T18's OnAddComment delegate returns + /// ServiceResult.Good, so the OPC UA SDK itself applies the comment to the node + /// and auto-fires the Part 9 comment event (E2) directly to subscribers — BEFORE the + /// engine re-projects via . When that re-projection + /// arrives here, the delta-gate sees no change in any compared field (the snapshot carries + /// no comments list) and correctly suppresses a second event (E3). Force-firing for + /// CommentAdded would double-emit. There is no engine-internal or script-driven + /// comment path, so suppression never drops a needed event. + /// + /// + /// Why Retain is absent (intentional — safe today). + /// Retain is projected as state.Active || !state.Acknowledged in + /// . Every path that flips Retain necessarily + /// changes Active or Acknowledged (both ARE compared fields), so a + /// Retain flip always rides along with a real delta and fires correctly. If a + /// future engine were to set Retain independently — without touching + /// Active/Acknowledged — it would need to be added here. + /// /// internal readonly record struct AlarmConditionDelta( bool Active, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs index 38030fb2..85301fc4 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs @@ -475,6 +475,31 @@ public sealed class SdkAddressSpaceSinkTests : IDisposable OtOpcUaNodeManager.ShouldFireConditionEvent(baseState, baseState with { Message = "other" }).ShouldBeTrue(); } + /// T20 — null-vs-empty Message normalization. Both snapshot.Message = null and a live node + /// whose Message.Value.Text = null normalize to in + /// , so a snapshot whose Message is null does NOT + /// produce a phantom delta against a freshly-materialised node (whose Message is the displayName but + /// whose Text reads back as the displayName string, and a snapshot that explicitly passes an empty + /// string equally produces no delta vs a null-Message node). This pins the normalization so a + /// future change to the null-coalescing in ReadConditionDelta / ToConditionDelta is caught here. + [Fact] + public void ShouldFireConditionEvent_null_and_empty_message_normalize_identically() + { + // Both null and "" collapse to string.Empty in AlarmConditionDelta — they are the same delta value. + var withNull = new OtOpcUaNodeManager.AlarmConditionDelta( + Active: false, Acknowledged: true, Confirmed: true, Enabled: true, + Shelving: AlarmShelvingKind.Unshelved, MappedSeverity: 100, Message: string.Empty); + + var withEmpty = withNull with { Message = string.Empty }; + + // null-message snapshot (normalised to "") vs empty-message node (normalised to "") ⇒ no delta. + OtOpcUaNodeManager.ShouldFireConditionEvent(withNull, withEmpty).ShouldBeFalse(); + OtOpcUaNodeManager.ShouldFireConditionEvent(withEmpty, withNull).ShouldBeFalse(); + + // A non-empty message IS a delta vs the empty baseline. + OtOpcUaNodeManager.ShouldFireConditionEvent(withNull, withNull with { Message = "pressure high" }).ShouldBeTrue(); + } + /// Builds a test with sensible defaults so each call /// site only specifies the fields it cares about. private static AlarmConditionSnapshot Snapshot(