From ddd76782690015774da880a07b6a204a773ae37b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:02:12 -0400 Subject: [PATCH 1/9] fix(opcua): carry native alarm condition state across a full rebuild (#641) A full RebuildAddressSpace recreated every native Part 9 condition inactive + acknowledged, and when the alarm-tag set was unchanged (e.g. one UnsTagReference added to an alarm-bearing raw tag) nothing re-told the new node its state: the host does not re-subscribe for an unchanged set, and Galaxy / OpcUaClient have no snapshot to replay anyway. Active alarms read normal until their next device transition. OtOpcUaNodeManager now captures each NATIVE condition's live state (from the node, so a client-applied ack is kept) before the rebuild clears, restores it when the same id is re-materialised native in the same apply, and reports ONE condition event per restored off-normal condition at the new IOpcUaAddressSpaceSink.CompleteAddressSpaceApply (after notifier wiring). Unclaimed captures are discarded there, so a later re-add is fresh. Quality is carried silently for normal conditions too (it is only re-annotated on a connectivity edge). Scripted conditions are untouched (#487 re-asserts them). OpcUaPublishActor calls CompleteAddressSpaceApply at the end of every apply. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../OpcUa/DeferredAddressSpaceSink.cs | 3 + .../OpcUa/IOpcUaAddressSpaceSink.cs | 13 + .../OtOpcUaNodeManager.cs | 145 ++++++++++ .../SdkAddressSpaceSink.cs | 3 + .../OpcUa/OpcUaPublishActor.cs | 21 ++ .../DeferredSinkForwardingReflectionTests.cs | 8 +- .../NativeAlarmRebuildCarryOverWireTests.cs | 125 +++++++++ ...ManagerNativeAlarmRebuildCarryOverTests.cs | 255 ++++++++++++++++++ .../OpcUa/OpcUaPublishActorRebuildTests.cs | 41 +++ 9 files changed, 611 insertions(+), 3 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/NativeAlarmRebuildCarryOverWireTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs index b53aa8b0..0c592bb1 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs @@ -69,6 +69,9 @@ public sealed class DeferredAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgical /// public void RebuildAddressSpace() => _inner.RebuildAddressSpace(); + /// + public void CompleteAddressSpaceApply() => _inner.CompleteAddressSpaceApply(); + /// public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm) => _inner.RaiseNodesAddedModelChange(affectedNodeId, realm); diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs index 1573e05a..498cd59e 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs @@ -185,6 +185,19 @@ public interface IOpcUaAddressSpaceSink /// void RebuildAddressSpace(); + /// + /// Gitea #641 — mark the END of one address-space apply pass (after , if + /// one ran, and every materialise / notifier-wiring call that followed it). An SDK-backed sink carries each + /// native condition's live Part 9 state across a rebuild and restores it when the same condition is + /// re-materialised; this is where it reports the ONE condition event per restored condition (now that + /// every notifier folder is wired) and DISCARDS any captured state no materialise claimed, so a later + /// deploy that re-adds the same id gets a fresh node rather than a resurrection. OpcUaPublishActor + /// calls it once at the end of every apply, rebuild or not. Idempotent; the default is a no-op. + /// + void CompleteAddressSpaceApply() + { + } + /// /// Announce that nodes were added at runtime (discovered-node injection) under /// so subscribed clients refresh their browse diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 925bf37d..9132653a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -65,6 +65,16 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// a native re-materialise adds, and clears it alongside /// so a re-materialise as the other kind is correct. private readonly HashSet _nativeAlarmNodeIds = new(); + /// Gitea #641: every NATIVE condition's live state, captured by + /// just before it clears , waiting for + /// to re-create the same id as native in the same apply. Whatever is unclaimed when + /// runs is discarded. Guarded by Lock. + private readonly Dictionary _rebuildCarryOver = new(); + /// Gitea #641: conditions whose non-normal state a re-materialise restored, owed ONE condition + /// 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(); /// 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 @@ -1432,9 +1442,102 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // H6a: record native (driver-fed) conditions so a later task can route their inbound // Acknowledge to the driver rather than the scripted engine. if (isNative) _nativeAlarmNodeIds.Add(conditionKey); + + // Gitea #641: a native condition the rebuild in this apply discarded gets its live state back. Removed + // whatever the kind, so a native→scripted swap on the same id drops the capture instead of leaving it + // for a later re-add to resurrect. + if (_rebuildCarryOver.Remove(conditionKey, out var carried) && isNative) + RestoreCarryOver(alarm, conditionKey, carried); } } + /// + /// Gitea #641 — one native condition's live Part 9 state, carried across a full + /// . + /// + /// The gate-relevant fields, read off the node as does. + /// 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. + private sealed record NativeConditionCarryOver( + AlarmConditionDelta State, DateTime Time, DateTime? ActiveTransitionTime, DateTime? QualityTimestamp); + + /// + /// Gitea #641 — read what a native condition node holds right now, so the rebuild can put it back. + /// + /// + /// + /// Why the NODE, not a host-side cache. The node is the only place that knows the true state: + /// a client's Acknowledge (and its comment) is applied to the node by the SDK and never passes through + /// the driver or NativeAlarmProjector. Replaying from any host-side record would un-acknowledge it. + /// + /// + /// Why it is needed at all. A rebuild recreates every condition inactive + acknowledged. When the + /// alarm-tag set is unchanged — the common case: adding a UnsTagReference to an alarm-bearing raw + /// tag is enough to force one — the host does not re-subscribe the driver's alarms, and even a + /// re-subscribe would not help a transition-only source (Galaxy, OpcUaClient) with nothing to replay. + /// Without this, every active alarm read normal until its next device transition. + /// + /// + /// The live native condition, about to be discarded. + /// Its carried state. + private static NativeConditionCarryOver CaptureCarryOver(AlarmConditionState condition) => new( + ReadConditionDelta(condition), + condition.Time?.Value ?? DateTime.UtcNow, + condition.ActiveState?.TransitionTime?.Value, + condition.Quality?.SourceTimestamp?.Value); + + /// + /// Gitea #641 — put a carried state back onto the freshly materialised native condition. MUST be called + /// under Lock, after the condition is registered. + /// + /// + /// + /// Quality always; the rest only off-normal. Source quality is annotated only on a driver's + /// connectivity edge, so a rebuild otherwise left every native condition — normal ones included — + /// reading BadWaitingForInitialData until the driver next flapped; it is restored silently. A + /// condition in the no-event position (inactive + acknowledged) otherwise keeps exactly what the + /// materialise built, message and authored severity included, and reports nothing — the same rule the + /// scripted re-assert follows (#487). + /// + /// + /// Reported later, once. The state is applied now, so a Read or ConditionRefresh in the window + /// already sees it, but the one condition event waits for : the + /// equipment folders the condition reports through are wired AFTER this call + /// (), and reporting now would miss them. Native conditions refuse + /// Confirm, shelving and Enable/Disable (#587), so Enabled and Shelving are not carried. + /// + /// + /// The new condition instance. + /// Its key. + /// What the discarded instance held. + private void RestoreCarryOver(AlarmConditionState alarm, RealmKey conditionKey, NativeConditionCarryOver carried) + { + var state = carried.State; + if (alarm.Quality is not null) + { + alarm.Quality.Value = state.Quality; + if (alarm.Quality.SourceTimestamp is not null && carried.QualityTimestamp is { } qualityAt) + alarm.Quality.SourceTimestamp.Value = qualityAt; + } + + var retained = state.Active || !state.Acknowledged; + if (!retained) return; + + alarm.SetActiveState(SystemContext, state.Active); + if (carried.ActiveTransitionTime is { } activeAt && alarm.ActiveState?.TransitionTime is { } transitionTime) + transitionTime.Value = activeAt; + alarm.SetAcknowledgedState(SystemContext, state.Acknowledged); + if (alarm.ConfirmedState is not null) alarm.SetConfirmedState(SystemContext, state.Confirmed); + alarm.SetSeverity(SystemContext, (EventSeverity)state.MappedSeverity); + alarm.Message.Value = new LocalizedText(state.Message); + alarm.Retain.Value = true; + alarm.Time.Value = carried.Time; + alarm.ReceiveTime.Value = carried.Time; + + _restoredConditionReports.Add((conditionKey, alarm, carried.Time)); + } + /// One materialised native-alarm condition together with the equipment folders wired as extra /// event-notifier roots for it. The list is mutated in place under Lock as /// notifiers are wired / torn down; is the concrete @@ -3207,6 +3310,16 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 _historizedTagnames.Clear(); _arrayOnScalarRefused.Clear(); + // Gitea #641: capture every NATIVE condition's live state before the conditions are discarded, so the + // re-materialise that follows in this same apply can put it back (see CaptureCarryOver). Scripted + // conditions are not captured — their engine re-asserts them after the reload (#487). + _rebuildCarryOver.Clear(); + _restoredConditionReports.Clear(); + foreach (var (key, alarm) in _alarmConditions) + { + if (_nativeAlarmNodeIds.Contains(key)) _rebuildCarryOver[key] = CaptureCarryOver(alarm); + } + foreach (var alarm in _alarmConditions.Values) { alarm.Parent?.RemoveChild(alarm); @@ -3251,6 +3364,38 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } } + /// + /// + /// Gitea #641: reports ONE condition event for each native condition whose off-normal state the rebuild + /// carried over (stamped with the instant that state came about, not the deploy time — as the scripted + /// re-assert does), then discards whatever captured state no materialise claimed. A condition the apply + /// replaced again after restoring it is skipped: the instance reported must be the one restored. + /// + public void CompleteAddressSpaceApply() + { + int reported = 0, discarded; + lock (Lock) + { + foreach (var (key, alarm, time) in _restoredConditionReports) + { + if (!_alarmConditions.TryGetValue(key, out var live) || !ReferenceEquals(live, alarm)) continue; + ReportConditionEvent(alarm, time); + reported++; + } + + discarded = _rebuildCarryOver.Count; + _restoredConditionReports.Clear(); + _rebuildCarryOver.Clear(); + } + + if (reported > 0 || discarded > 0) + { + _logger.LogInformation( + "OtOpcUaNodeManager: rebuild carried {Reported} active/unacknowledged native condition(s) over; discarded {Discarded} captured condition(s) the apply did not re-create", + reported, discarded); + } + } + /// public bool RemoveVariableNode(string variableNodeId, AddressSpaceRealm realm) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs index 3d4dda2c..32d37245 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs @@ -84,6 +84,9 @@ public sealed class SdkAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgicalAddre /// public void RebuildAddressSpace() => _nodeManager.RebuildAddressSpace(); + /// + public void CompleteAddressSpaceApply() => _nodeManager.CompleteAddressSpaceApply(); + /// public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm) => _nodeManager.RaiseNodesAddedModelChange(affectedNodeId, realm); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs index 27a321f7..883f8c6f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -437,6 +437,27 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers } private void HandleRebuild(RebuildAddressSpace msg) + { + try + { + HandleRebuildCore(msg); + } + finally + { + // Gitea #641 — close the apply on EVERY exit, early returns and a throwing pass included: the node + // manager reports here the native conditions a rebuild carried over (after the notifier wiring the + // passes did) and discards any captured state no materialise claimed. Skipping it on a throw would + // leave that state for a later surgical re-add of the same id to resurrect. No-op when nothing is + // pending; never throws out of the handler. + try { _sink.CompleteAddressSpaceApply(); } + catch (Exception ex) + { + _log.Error(ex, "OpcUaPublish: sink.CompleteAddressSpaceApply threw (correlation={Correlation})", msg.Correlation); + } + } + } + + private void HandleRebuildCore(RebuildAddressSpace msg) { using var span = OtOpcUaTelemetry.StartAddressSpaceRebuildSpan(); span?.SetTag("otopcua.correlation_id", msg.Correlation.ToString()); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs index 7a455cec..6a53880e 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs @@ -80,11 +80,13 @@ public class DeferredSinkForwardingReflectionTests // v3 B4-WP2 dual-namespace invariant: a node id alone is no longer globally unique across the two // namespaces (Raw vs UNS), so every sink method that NAMES a node must carry an AddressSpaceRealm // discriminator — the sink resolves the namespace from the realm rather than parsing the id string. - // RebuildAddressSpace is the sole node-naming-free method (it clears BOTH realms). This guard locks in - // the realm surface so a future method can't be added that names a node without a realm. + // RebuildAddressSpace (it clears BOTH realms) and CompleteAddressSpaceApply (it closes an apply pass, + // Gitea #641) are the only node-naming-free methods. This guard locks in the realm surface so a future + // method can't be added that names a node without a realm. foreach (var method in ForwardingInterfaces.SelectMany(i => i.GetMethods())) { - if (method.Name == nameof(IOpcUaAddressSpaceSink.RebuildAddressSpace)) continue; + if (method.Name is nameof(IOpcUaAddressSpaceSink.RebuildAddressSpace) + or nameof(IOpcUaAddressSpaceSink.CompleteAddressSpaceApply)) continue; method.GetParameters().Any(p => CarriesRealm(p.ParameterType)).ShouldBeTrue( $"{method.DeclaringType!.Name}.{method.Name} names a node but has no AddressSpaceRealm parameter — " + diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/NativeAlarmRebuildCarryOverWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/NativeAlarmRebuildCarryOverWireTests.cs new file mode 100644 index 00000000..4a09b100 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/NativeAlarmRebuildCarryOverWireTests.cs @@ -0,0 +1,125 @@ +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using static ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests.ServerEventRig; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// Gitea #641 over the wire — a full address-space rebuild that leaves the native alarm set unchanged must not +/// leave an active condition reading inactive. A Server-object (i=2253) item, which outlives the rebuild +/// (#586), receives exactly ONE condition event carrying the restored state — and an acknowledgement a client +/// applied before the rebuild survives it. +/// +/// +/// The rebuild is driven exactly as OpcUaPublishActor drives one: RebuildAddressSpace, the +/// re-materialise + notifier wiring, then CompleteAddressSpaceApply. The count is read after a later +/// sentinel transition arrives on the same item, so a duplicate would already be queued ahead of it. +/// The native ack router echoes nothing back, like MTConnect and FOCAS: the acknowledgement lives only on the node. +/// +public sealed class NativeAlarmRebuildCarryOverWireTests +{ + private const NodePermissions All = + NodePermissions.Browse | NodePermissions.Read | NodePermissions.Subscribe | NodePermissions.AlarmRead + | NodePermissions.AlarmAcknowledge | NodePermissions.MethodCall; + + private static AlarmConditionSnapshot Fault(string message) => new( + Active: true, Acknowledged: false, Confirmed: true, Enabled: true, + Shelving: AlarmShelvingKind.Unshelved, Severity: 800, Message: message); + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_rebuild_with_an_unchanged_alarm_set_reports_the_restored_state_once(bool clientAcked) + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync($"Rebuild641{clientAcked}", encrypted: false, ct); + try + { + rig.Enforce(false, [Key()], ClusterGrant(KeyPrincipal, "acl-key", All)); + rig.Nm.NativeAlarmAckRouter = _ => { }; // a driver whose ack is a no-op: nothing echoes back + rig.MaterialiseNative(); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var serverItem = RecordedEvents.AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + rig.Project(native: true, Fault("fault")); + await RecordedEvents.WaitUntilAsync(() => events.Has(serverItem, "fault")); + var expectedBefore = 1; + + if (clientAcked) + { + await session.CallAsync( + ConditionNodeId(session, native: true), MethodIds.AcknowledgeableConditionType_Acknowledge, ct, + events.LatestEventId(serverItem, "fault"), new LocalizedText("ack")); + await RecordedEvents.WaitUntilAsync(() => events.ConditionEvents(serverItem, "fault").Count == 2); + expectedBefore = 2; + } + + // The deploy: a rebuild whose composition carries the same alarm (e.g. one UnsTagReference added). + rig.Sink.RebuildAddressSpace(); + rig.MaterialiseNative(); + rig.Sink.CompleteAddressSpaceApply(); + + rig.Project(native: true, Fault("sentinel") with { Acknowledged = clientAcked }); + await RecordedEvents.WaitUntilAsync(() => events.Has(serverItem, "sentinel")); + + var faultEvents = events.ConditionEvents(serverItem, "fault"); + faultEvents.Count.ShouldBe(expectedBefore + 1, "the pre-rebuild event(s) plus exactly one restored event"); + var restored = faultEvents[^1]; + restored[RecordedEvents.ActiveField].Value.ShouldBe(true); + restored[RecordedEvents.AckedField].Value.ShouldBe(clientAcked); + restored[RecordedEvents.RetainField].Value.ShouldBe(true); + restored[RecordedEvents.SeverityField].Value.ShouldBe((ushort)800); + + // And a ConditionRefresh-style read of the live node agrees. + var active = await session.ReadValueAsync( + new NodeId($"{RawAlarmPath}/ActiveState/Id", ConditionNodeId(session, native: true).NamespaceIndex), ct); + active.Value.ShouldBe(true); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// Falsification control: a NORMAL native condition produces no event across the rebuild. + [Fact] + public async Task A_rebuild_reports_nothing_for_a_normal_condition() + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync("Rebuild641Normal", encrypted: false, ct); + try + { + rig.Enforce(false, [Key()], ClusterGrant(KeyPrincipal, "acl-key", All)); + rig.MaterialiseNative(); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var serverItem = RecordedEvents.AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + rig.Project(native: true, Fault("cleared") with { Active = false, Acknowledged = true }); + await RecordedEvents.WaitUntilAsync(() => events.Has(serverItem, "cleared")); + + rig.Sink.RebuildAddressSpace(); + rig.MaterialiseNative(); + rig.Sink.CompleteAddressSpaceApply(); + + rig.Project(native: true, Fault("sentinel")); + await RecordedEvents.WaitUntilAsync(() => events.Has(serverItem, "sentinel")); + + events.ConditionEvents(serverItem, "cleared").Count.ShouldBe(1); + events.ConditionEvents(serverItem, "temp_hi").ShouldBeEmpty("the fresh node is not reported"); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs new file mode 100644 index 00000000..673fbef2 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs @@ -0,0 +1,255 @@ +using Opc.Ua; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// Gitea #641 — a full recreates every native condition +/// inactive, and when the alarm-tag set is unchanged nothing tells the new node its state again (the driver +/// does not re-subscribe, and Galaxy / OpcUaClient have nothing to replay anyway). The node manager therefore +/// carries each NATIVE condition's live state across the rebuild: captured before the clear, restored when +/// the same id is re-materialised native in the same apply, and reported as ONE condition event when the +/// apply completes (). +/// +/// +/// "One event" is observed here through the condition's EventId, which every report stamps fresh. +/// The exact count over the wire is pinned by NativeAlarmRebuildCarryOverWireTests. +/// +public sealed class NodeManagerNativeAlarmRebuildCarryOverTests : IDisposable +{ + private const string Device = "Plant/MTConnect/dev1"; + private const string Alarm = "Plant/MTConnect/dev1/Xtravel"; + + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private readonly string _pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-641-{Guid.NewGuid():N}"); + + private static AlarmConditionSnapshot Fault(bool acked) => new( + Active: true, Acknowledged: acked, Confirmed: true, Enabled: true, + Shelving: AlarmShelvingKind.Unshelved, Severity: 800, Message: "Xtravel FAULT 1001", + StatusCode: OpcUaStatus.Good); + + [Trait("Category", "Unit")] + [Fact] + public async Task An_active_unacked_native_condition_survives_a_rebuild_and_is_reported_once() + { + 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); + + nm.RebuildAddressSpace(); + Materialise(nm, native: true); + var restored = Condition(nm); + var idBeforeComplete = (byte[]?)restored.EventId.Value?.Clone(); + + // State is back as soon as the node exists — a Read or a ConditionRefresh in between already sees it. + restored.ActiveState.Id.Value.ShouldBeTrue(); + restored.AckedState.Id.Value.ShouldBeFalse(); + restored.Retain.Value.ShouldBeTrue(); + restored.Severity.Value.ShouldBe((ushort)800); + restored.Message.Value.Text.ShouldBe("Xtravel FAULT 1001"); + restored.Quality.Value.Code.ShouldBe(StatusCodes.Good); + + nm.CompleteAddressSpaceApply(); + + // The report happens at the END of the apply (after the notifier wiring), stamped with the instant the + // state came about, not the deploy time. + restored.EventId.Value.ShouldNotBe(idBeforeComplete); + restored.Time.Value.ShouldBe(raisedAt); + + // ...and only once: a second completion with nothing pending reports nothing. + var idAfterComplete = (byte[]?)restored.EventId.Value?.Clone(); + nm.CompleteAddressSpaceApply(); + restored.EventId.Value.ShouldBe(idAfterComplete); + + await host.DisposeAsync(); + } + + /// + /// The acknowledgement a CLIENT applied lives only on the node (the SDK applied it; no driver or projector + /// saw it) — the reason the carry-over reads the node rather than any host-side cache. + /// + [Trait("Category", "Unit")] + [Fact] + public async Task A_client_acknowledgement_survives_the_rebuild() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: true); + nm.WriteAlarmCondition(Alarm, Fault(acked: false), DateTime.UtcNow, AddressSpaceRealm.Raw); + // What the SDK's Acknowledge handler does to the node; nothing else hears of it. + Condition(nm).SetAcknowledgedState(nm.SystemContext, true); + + nm.RebuildAddressSpace(); + Materialise(nm, native: true); + nm.CompleteAddressSpaceApply(); + + var restored = Condition(nm); + restored.ActiveState.Id.Value.ShouldBeTrue(); + restored.AckedState.Id.Value.ShouldBeTrue(); + restored.Retain.Value.ShouldBeTrue(); + + 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")] + [Fact] + public async Task A_normal_native_condition_reports_nothing_and_keeps_the_fresh_state() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: true); + nm.WriteAlarmCondition(Alarm, Fault(acked: false) with { Active = false, Acknowledged = true, Message = "cleared" }, + DateTime.UtcNow, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + Materialise(nm, native: true); + var fresh = Condition(nm); + var idBefore = (byte[]?)fresh.EventId.Value?.Clone(); + nm.CompleteAddressSpaceApply(); + + fresh.EventId.Value.ShouldBe(idBefore); + fresh.ActiveState.Id.Value.ShouldBeFalse(); + fresh.AckedState.Id.Value.ShouldBeTrue(); + fresh.Retain.Value.ShouldBeFalse(); + fresh.Message.Value.Text.ShouldBe("Xtravel", "a normal condition keeps the materialise's message"); + fresh.Severity.Value.ShouldBe((ushort)700, "and its authored severity"); + + await host.DisposeAsync(); + } + + /// + /// The source-data quality is carried for a NORMAL condition too, silently: quality is annotated only on a + /// driver's connectivity edge, so without the carry a rebuild left every normal native condition reading + /// BadWaitingForInitialData until the driver next flapped. + /// + [Trait("Category", "Unit")] + [Fact] + public async Task A_normal_condition_keeps_its_quality_without_an_event() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: true); + nm.WriteAlarmQuality(Alarm, OpcUaStatus.Good, DateTime.UtcNow, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + Materialise(nm, native: true); + var fresh = Condition(nm); + var idBefore = (byte[]?)fresh.EventId.Value?.Clone(); + nm.CompleteAddressSpaceApply(); + + fresh.Quality.Value.Code.ShouldBe(StatusCodes.Good); + fresh.EventId.Value.ShouldBe(idBefore); + + await host.DisposeAsync(); + } + + /// A condition the rebuild did NOT re-create has its captured state discarded when the apply + /// completes, so a later deploy that adds the same RawPath back gets a fresh node, not a resurrection. + [Trait("Category", "Unit")] + [Fact] + public async Task A_condition_removed_by_the_rebuild_is_not_resurrected_by_a_later_add() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: true); + nm.WriteAlarmCondition(Alarm, Fault(acked: false), DateTime.UtcNow, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + nm.CompleteAddressSpaceApply(); // the alarm was removed: nothing re-materialised it + + Materialise(nm, native: true); // a later, surgical re-add of the same RawPath + nm.CompleteAddressSpaceApply(); + + var readded = Condition(nm); + readded.ActiveState.Id.Value.ShouldBeFalse(); + readded.AckedState.Id.Value.ShouldBeTrue(); + + await host.DisposeAsync(); + } + + /// A native condition re-materialised as SCRIPTED under the same id is a different alarm: nothing + /// is restored onto it (and the scripted engine's own #487 re-assert owns scripted state). + [Trait("Category", "Unit")] + [Fact] + public async Task A_kind_swap_on_the_same_id_is_not_restored() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: true); + nm.WriteAlarmCondition(Alarm, Fault(acked: false), DateTime.UtcNow, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + Materialise(nm, native: false); + nm.CompleteAddressSpaceApply(); + + Condition(nm).ActiveState.Id.Value.ShouldBeFalse(); + + await host.DisposeAsync(); + } + + /// Scripted conditions are not captured — ScriptedAlarmHostActor.ReassertConditionNodes + /// (#487) already re-projects them after a reload, and a second source would double-report. + [Trait("Category", "Unit")] + [Fact] + public async Task A_scripted_condition_is_not_carried() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: false); + nm.WriteAlarmCondition(Alarm, Fault(acked: false), DateTime.UtcNow, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + Materialise(nm, native: false); + nm.CompleteAddressSpaceApply(); + + Condition(nm).ActiveState.Id.Value.ShouldBeFalse(); + + await host.DisposeAsync(); + } + + private static void Materialise(OtOpcUaNodeManager nm, bool native) + { + nm.EnsureFolder(Device, parentNodeId: null, displayName: "dev1", realm: AddressSpaceRealm.Raw); + nm.MaterialiseAlarmCondition(Alarm, Device, "Xtravel", "OffNormalAlarm", 700, AddressSpaceRealm.Raw, isNative: native); + } + + private static Opc.Ua.AlarmConditionState Condition(OtOpcUaNodeManager nm) => + nm.TryGetAlarmCondition(Alarm, AddressSpaceRealm.Raw).ShouldNotBeNull(); + + private async Task<(OpcUaApplicationHost Host, OtOpcUaNodeManager Nm)> BootAsync() + { + var host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.Gitea641Test", + ApplicationUri = $"urn:OtOpcUa.Gitea641Test:{Guid.NewGuid():N}", + OpcUaPort = AllocateFreePort(), + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + + var server = new OtOpcUaSdkServer(); + await host.StartAsync(server, Ct); + return (host, server.NodeManager!); + } + + private static int AllocateFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); + listener.Start(); + var port = ((System.Net.IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Cleans up the PKI root directory. + public void Dispose() + { + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch { /* best-effort cleanup */ } + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs index 94453d9a..93880e0c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs @@ -61,6 +61,41 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase sink.RebuildCalls.ShouldBe(0); } + /// + /// Gitea #641 — every apply ends with CompleteAddressSpaceApply, AFTER the last materialise / announce + /// call: that is where the node manager reports the conditions a rebuild carried over (their notifier + /// folders are wired by then) and drops any captured state no materialise claimed. + /// + [Fact] + public void An_apply_completes_after_every_materialise_pass() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + SeedDeployment(db, equipmentIds: new[] { "eq-1", "eq-2" }, driverIds: new[] { "drv-1" }); + + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); + + AwaitAssert(() => sink.CompletedAfterCallCount.Count.ShouldBe(1), duration: PresenceBudget); + sink.CompletedAfterCallCount.Single().ShouldBe(sink.Calls.Count, "nothing may follow the completion"); + sink.Calls.ShouldContain("NA:line-1"); + } + + /// Gitea #641 — the raw-sink fallback rebuild (no applier) completes too, so the state it captured + /// is discarded rather than left for a later add to resurrect. + [Fact] + public void The_raw_sink_fallback_rebuild_completes_the_apply() + { + var sink = new RecordingSink(); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink)); + + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); + + AwaitAssert(() => sink.CompletedAfterCallCount.Count.ShouldBe(1), duration: PresenceBudget); + sink.RebuildCalls.ShouldBe(1); + } + /// Tests that rebuild with no artifact is idempotent no-op. [Fact] public void Rebuild_with_no_artifact_is_idempotent_no_op() @@ -1053,6 +1088,12 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase } /// Records a rebuild address space call. public void RebuildAddressSpace() => Interlocked.Increment(ref RebuildCalls); + /// Gets, per call, how many had been + /// recorded at that moment — so a test can prove the completion came after every pass (Gitea #641). + /// Kept out of so existing call-count assertions are unaffected. + public ConcurrentQueue CompletedAfterCallCount { get; } = new(); + /// Records the end of an apply pass (Gitea #641). + public void CompleteAddressSpaceApply() => CompletedAfterCallCount.Enqueue(Calls.Count); /// Records a NodeAdded model-change announcement. /// The node under which discovered nodes were added. public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns) => Calls.Enqueue($"NA:{affectedNodeId}"); From 11f13f632613ac8af1398f30301aebf5f297d16c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:05:42 -0400 Subject: [PATCH 2/9] fix(runtime): prune the native-alarm projector on deploy instead of clearing it (#641) DriverHostActor cleared NativeAlarmProjector's per-condition prior state on EVERY apply, so after any deploy a driver Acknowledge of a still-active alarm projected Active=false (writing it inactive) and a Clear of an unacknowledged alarm projected Acked=true (silently acknowledging it). The projector is now pruned to the conditions the new composition still carries (RetainOnly). Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../Drivers/DriverHostActor.cs | 17 +++-- .../Drivers/NativeAlarmProjector.cs | 18 ++++- .../DriverHostActorNativeAlarmTests.cs | 69 ++++++++++++++++++- .../Drivers/NativeAlarmProjectorTests.cs | 15 ++++ 4 files changed, 113 insertions(+), 6 deletions(-) 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 33eafca8..e62c8d2d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -1793,9 +1793,10 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS // #626 — a QUALITY-ONLY annotation (an MTConnect condition gone Unavailable, or back) is not an alarm // transition. It rides the SAME quality-only node path the #477 connectivity annotation uses // (AlarmQualityUpdate → WriteAlarmQuality, which touches Quality and nothing else), never a projected - // snapshot: the projector's prior state is cleared on every apply, so a projected "keep the state" - // after a deploy would default to inactive+acked and silently acknowledge a retained, unacked - // condition — and would rewrite its message and severity. Ungated by role (a Secondary keeps quality + // snapshot: the projector never sees a client's acknowledgement (that is applied to the node by the + // SDK) and forgets a condition a deploy removed, so a projected "keep the state" can disagree with + // the node — silently acknowledging a retained, unacked condition or rewriting its message and + // severity. Ungated by role (a Secondary keeps quality // warm, as for connectivity) and no alerts row. if (msg.Args is { IsQualityOnly: true, QualityStatusCode: { } quality }) { @@ -3078,7 +3079,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS _alarmNodeIdByDriverRef.Clear(); _driverRefByAlarmNodeId.Clear(); _alarmMetaByNodeId.Clear(); - _nativeAlarmProjector.Clear(); + // NOT _nativeAlarmProjector.Clear() (Gitea #641): it is pruned to the still-deployed conditions after the + // maps are rebuilt below, so a redeploy that keeps an alarm keeps its projected prior state. // The per-driver secondary indexes are rebuilt WHOLE alongside their primaries below — same pass, // same clear — so no incremental maintenance exists to get out of step with them. _valueRefsByDriver.Clear(); @@ -3132,6 +3134,13 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS } } + // Gitea #641 — forget the projected prior state of conditions this composition no longer carries, and keep + // the rest. Clearing it whole on every apply made the next driver Acknowledge of a still-active alarm + // project Active=false (the default prior), and the next Clear of an unacknowledged one project + // Acked=true — on any deploy, rebuild or not. The node manager carries the NODE's state across a rebuild, + // so the two stay in step. Keyed by the bare condition NodeId, exactly as ForwardNativeAlarm projects. + _nativeAlarmProjector.RetainOnly(_driverRefByAlarmNodeId.Keys.ToHashSet(StringComparer.Ordinal)); + // Cache the per-driver payload as THE authority, in the same clear-and-repopulate pass as the routing // maps above so a rename/removal can never leave a stale entry behind. Keyed over the UNION of the two // ref maps AND the running children: a child with no authored tags must get an explicit EMPTY set (which diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs index 4e52cf90..fb0f8075 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs @@ -40,9 +40,25 @@ public sealed class NativeAlarmProjector StatusCode: e.QualityStatusCode ?? OpcUaStatus.Good); } - /// Clears all tracked per-node state (call on address-space rebuild). + /// Clears all tracked per-node state. public void Clear() => _prior.Clear(); + /// + /// Gitea #641 — drop the tracked state of every condition NOT in , keeping + /// the rest. Called on every apply instead of : a condition that is still deployed keeps + /// its state across the deploy (the node manager carries the node's state across a rebuild too), so the + /// next Acknowledge does not project Active = false onto an active alarm and the next Clear does not + /// project Acked = true onto an unacknowledged one. A removed condition is forgotten, so a later + /// re-add starts from the default. + /// + /// The condition node ids the new composition still materialises. + public void RetainOnly(IReadOnlySet deployedNodeIds) + { + ArgumentNullException.ThrowIfNull(deployedNodeIds); + foreach (var nodeId in _prior.Keys.Where(k => !deployedNodeIds.Contains(k)).ToList()) + _prior.Remove(nodeId); + } + private static ushort MapSeverity(AlarmSeverity s) => s switch { AlarmSeverity.Low => 200, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs index 848ee47d..2a2fadaa 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs @@ -409,6 +409,66 @@ public sealed class DriverHostActorNativeAlarmTests : RuntimeActorTestBase publish.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); } + /// + /// Gitea #641 — a redeploy that keeps the alarm must not reset the projector's prior state for it. It used + /// to be cleared on EVERY apply, so a driver Acknowledge arriving after any deploy projected + /// (prev.Active = false, Acked) and wrote an ACTIVE alarm inactive. + /// + [Fact] + public void An_acknowledge_after_a_redeploy_keeps_the_alarm_active() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedV3AlarmDeployment(db, RevA, Driver: "drv-1", Tag: "temp_hi"); + var (actor, publish, coordinator) = SpawnHostAndApplyWithCoordinator(db, deploymentId); + + actor.Tell(Transition(AlarmTransitionKind.Raise)); + publish.ExpectMsg(TimeSpan.FromSeconds(5)).State.Active.ShouldBeTrue(); + + Redeploy(db, actor, publish, coordinator); + + actor.Tell(Transition(AlarmTransitionKind.Acknowledge)); + var acked = publish.FishForMessage(_ => true, TimeSpan.FromSeconds(5)); + acked.State.Active.ShouldBeTrue("the alarm is still active at the device"); + acked.State.Acknowledged.ShouldBeTrue(); + } + + /// Gitea #641 — likewise a Clear after a redeploy must not silently acknowledge an unacknowledged + /// alarm (the cleared prior state defaulted Acked to true). + [Fact] + public void A_clear_after_a_redeploy_keeps_the_alarm_unacknowledged() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedV3AlarmDeployment(db, RevA, Driver: "drv-1", Tag: "temp_hi"); + var (actor, publish, coordinator) = SpawnHostAndApplyWithCoordinator(db, deploymentId); + + actor.Tell(Transition(AlarmTransitionKind.Raise)); + publish.ExpectMsg(TimeSpan.FromSeconds(5)).State.Acknowledged.ShouldBeFalse(); + + Redeploy(db, actor, publish, coordinator); + + actor.Tell(Transition(AlarmTransitionKind.Clear)); + var cleared = publish.FishForMessage(_ => true, TimeSpan.FromSeconds(5)); + cleared.State.Active.ShouldBeFalse(); + cleared.State.Acknowledged.ShouldBeFalse("nobody acknowledged it"); + } + + private static DriverInstanceActor.AttributeAlarmPublished Transition(AlarmTransitionKind kind) => + new("drv-1", new AlarmEventArgs( + new StubAlarmHandle(), "Temp", AlarmRawPath, "OffNormalAlarm", kind.ToString(), AlarmSeverity.High, Ts, + Kind: kind)); + + /// A second deployment carrying the SAME alarm (a new revision of an identical artifact). + private void Redeploy( + IDbContextFactory db, IActorRef actor, TestProbe publish, TestProbe coordinator) + { + var revB = RevisionHash.Parse(new string('b', 64)); + var second = SeedV3AlarmDeployment(db, revB, Driver: "drv-1", Tag: "temp_hi"); + actor.Tell(new DispatchDeployment(second, revB, CorrelationId.NewId())); + coordinator.FishForMessage(a => a.DeploymentId == second, TimeSpan.FromSeconds(5)) + .Outcome.ShouldBe(ApplyAckOutcome.Applied); + publish.FishForMessage(_ => true, TimeSpan.FromSeconds(5)); + } + /// Subscribe to the alerts DPS topic and wait for the ack. private void SubscribeToAlerts(TestProbe probe) { @@ -439,6 +499,13 @@ public sealed class DriverHostActorNativeAlarmTests : RuntimeActorTestBase /// publishes an alarm. A VirtualTag-host probe is injected so the real host isn't spawned. private (IActorRef Actor, TestProbe Publish) SpawnHostAndApply( IDbContextFactory db, DeploymentId deploymentId) + { + var (actor, publish, _) = SpawnHostAndApplyWithCoordinator(db, deploymentId); + return (actor, publish); + } + + private (IActorRef Actor, TestProbe Publish, TestProbe Coordinator) SpawnHostAndApplyWithCoordinator( + IDbContextFactory db, DeploymentId deploymentId) { var coordinator = CreateTestProbe(); var publish = CreateTestProbe(); @@ -460,7 +527,7 @@ public sealed class DriverHostActorNativeAlarmTests : RuntimeActorTestBase // ExpectMsg assertions see only alarm updates. publish.ExpectMsg(TimeSpan.FromSeconds(5)); - return (actor, publish); + return (actor, publish, coordinator); } /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs index eaee1d77..e3a2c876 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs @@ -24,6 +24,21 @@ public class NativeAlarmProjectorTests snap.Message.ShouldBe("boom"); } + /// Gitea #641 — RetainOnly keeps a still-deployed condition's prior state and forgets a removed + /// one's (which then starts from the default inactive + acked). + [Fact] + public void RetainOnly_keeps_deployed_conditions_and_forgets_removed_ones() + { + var sut = new NativeAlarmProjector(); + sut.Project("kept", Evt(AlarmTransitionKind.Raise)); + sut.Project("removed", Evt(AlarmTransitionKind.Raise)); + + sut.RetainOnly(new HashSet(StringComparer.Ordinal) { "kept" }); + + sut.Project("kept", Evt(AlarmTransitionKind.Acknowledge)).Active.ShouldBeTrue(); + sut.Project("removed", Evt(AlarmTransitionKind.Acknowledge)).Active.ShouldBeFalse(); + } + [Fact] public void Retrigger_projects_active_unacked() { From 3fe6cbb9056dd86428ad3518fe0a97b9cb872a2e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:06:57 -0400 Subject: [PATCH 3/9] fix(mtconnect): on a changed alarm set forget only the conditions that left (#641) The driver cleared its whole "last told" record whenever the subscribed alarm set changed, assuming the rebuild that accompanies an alarm-bearing delta had reset every condition node. The server now carries each surviving native condition's state (acknowledgement included) across the rebuild, so re-raising a survivor would un-acknowledge it. Only entries that left the set are dropped; a condition that comes back later is told again onto its fresh node. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/drivers/MTConnect.md | 11 ++++---- .../MTConnectDriver.Alarms.cs | 28 ++++++++----------- .../MTConnectAlarmSourceTests.cs | 22 +++++++++------ 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/docs/drivers/MTConnect.md b/docs/drivers/MTConnect.md index 1a8921d5..3aa5b5a8 100644 --- a/docs/drivers/MTConnect.md +++ b/docs/drivers/MTConnect.md @@ -462,11 +462,12 @@ and records a state only when an alarm handler was attached to receive it. Conse condition the driver owns `BadNoCommunication` on the connectivity edge (#477) and keeps its Active state; the reconnect edge re-annotates Good, after which the driver re-asserts any still-`Unavailable` condition's Bad quality on its re-subscribe. -- A re-subscribe with a **changed** alarm set clears the whole told record: only an alarm-bearing raw-tag - delta changes the set, the server applies that as a full rebuild, and a rebuild recreates every - condition node inactive — so every active condition is re-asserted onto its new node. The **same** set - (a reconnect) keeps the record. (A rebuild that leaves this driver's alarm set unchanged triggers no - re-subscribe; that is a host-wide gap shared with every native source.) +- A re-subscribe with a **changed** alarm set drops the told record of only the conditions that **left** + the set. The server carries every native condition's state (acknowledgement included) across a full + rebuild itself (Gitea #641 — see `docs/AlarmTracking.md`), so re-raising a surviving condition would + only un-acknowledge it; a condition that left and later comes back was materialised fresh and is told + again. The **same** set (a reconnect) keeps the whole record. A rebuild that leaves the alarm set + unchanged triggers no re-subscribe, and needs none — the server-side carry-over covers it. - An alarm-only driver (no value tags) still runs the `/sample` pump. **EVENT controlled vocabularies as OPC UA enumerations — won't fix.** EVENT values (`Execution`, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.Alarms.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.Alarms.cs index 6c82d568..5c22eb43 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.Alarms.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.Alarms.cs @@ -50,18 +50,13 @@ public sealed partial class MTConnectDriver : IAlarmSource /// /// ConditionId (RawPath) → what the server was last told. Guarded by . Survives a /// re-subscribe with the SAME reference set (a reconnect) and lifecycle restarts — the server's condition - /// nodes outlive both, and re-raising there would un-acknowledge. Cleared WHOLE when the subscribed set - /// changes: only an alarm-bearing raw-tag delta changes it, that delta is a full address-space rebuild, - /// and a rebuild recreates every condition node inactive — so everything must be told again. + /// nodes outlive both, and re-raising there would un-acknowledge. When the subscribed set changes (an + /// alarm-bearing raw-tag delta, i.e. a full rebuild) only the entries that LEFT the set are dropped: the + /// server carries every surviving condition's state across the rebuild (Gitea #641), so a survivor needs + /// no re-tell, while a condition that comes back later was materialised fresh and is told again. /// private readonly Dictionary _deliveredConditions = new(StringComparer.Ordinal); - /// - /// The reference set of the last , kept across an unsubscribe so a - /// release-then-resubscribe of an identical set is still recognised. Guarded by . - /// - private HashSet? _lastAlarmReferences; - /// The CONDITION tracker for the current session; replaced alongside the observation index. private MTConnectConditionTracker _conditions = new(); @@ -108,16 +103,15 @@ public sealed partial class MTConnectDriver : IAlarmSource lock (_alarmLock) { - // A CHANGED set means an alarm-bearing delta, which the server applies as a full rebuild: every - // condition node — including ones still in the set — was recreated inactive and has been told - // nothing. Re-tell all of them. The SAME set is a reconnect: keep the record, or every still-active - // alarm would be re-raised and lose its acknowledgement. - if (_lastAlarmReferences is null || !_lastAlarmReferences.SetEquals(references)) + // Forget only the conditions that LEFT the set (Gitea #641). A changed set is an alarm-bearing delta, + // which the server applies as a full rebuild — but the server carries every surviving condition's + // state (acknowledgement included) across that rebuild, so re-raising a survivor would only + // un-acknowledge it. A condition that left and later comes back was materialised fresh and has no + // record, so it is told again. The SAME set (a reconnect) prunes nothing. + foreach (var conditionId in _deliveredConditions.Keys.Where(k => !references.Contains(k)).ToList()) { - _deliveredConditions.Clear(); + _deliveredConditions.Remove(conditionId); } - - _lastAlarmReferences = references; } EmitConditionTransitions(touchedDataItemIds: null, resync: true); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAlarmSourceTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAlarmSourceTests.cs index ccabf8d7..fe8097a7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAlarmSourceTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAlarmSourceTests.cs @@ -151,27 +151,31 @@ public sealed class MTConnectAlarmSourceTests } [Fact] - public async Task A_changed_alarm_set_re_tells_every_condition_and_the_same_set_does_not() + public async Task A_changed_alarm_set_re_tells_only_conditions_that_left_and_came_back() { - // Review MAJOR 2: a changed set is an alarm-bearing delta ⇒ full rebuild ⇒ every condition node was - // recreated inactive, so an active condition still in the set must be re-asserted onto it. + // Gitea #641: a changed set is an alarm-bearing delta ⇒ a full rebuild, but the server now carries every + // surviving condition's state (acknowledgement included) across it. Re-raising a survivor would + // UN-acknowledge it, so only the record of a condition that LEFT the set is dropped: one that comes back + // was materialised fresh and must be told again. var (driver, agent, events) = await AlarmDriverAsync(); await driver.SubscribeAlarmsAsync([ConditionRawPath], Ct); await agent.PumpAsync(Chunk(108, 109, Cond("Fault", "E17"))).WaitAsync(Watchdog, Ct); events.ShouldHaveSingleItem().Kind.ShouldBe(AlarmTransitionKind.Raise); - await driver.SubscribeAlarmsAsync([ConditionRawPath, OtherConditionRawPath], Ct); - events.Count.ShouldBe(2, "A is re-raised onto its rebuilt node"); + await driver.SubscribeAlarmsAsync([ConditionRawPath, OtherConditionRawPath], Ct); // A survives: silent + await driver.SubscribeAlarmsAsync([OtherConditionRawPath, ConditionRawPath], Ct); // same set: silent + events.Count.ShouldBe(1, "a surviving condition is not re-raised"); + + await driver.SubscribeAlarmsAsync([OtherConditionRawPath], Ct); // A removed + await driver.SubscribeAlarmsAsync([OtherConditionRawPath, ConditionRawPath], Ct); // A re-added: fresh node + events.Count.ShouldBe(2, "a condition that left the set and came back is told again"); events.Last().ConditionId.ShouldBe(ConditionRawPath); events.Last().Kind.ShouldBe(AlarmTransitionKind.Raise); - await driver.SubscribeAlarmsAsync([OtherConditionRawPath, ConditionRawPath], Ct); - // Positive control: the next real transition is the very next event. await agent.PumpAsync(Chunk(109, 110, Cond("Normal"))).WaitAsync(Watchdog, Ct); events.Select(e => e.Kind).ShouldBe( - [AlarmTransitionKind.Raise, AlarmTransitionKind.Raise, AlarmTransitionKind.Clear], - "the same set (a reconnect) must re-raise nothing"); + [AlarmTransitionKind.Raise, AlarmTransitionKind.Raise, AlarmTransitionKind.Clear]); await driver.ShutdownAsync(Ct); } From 64d2514e2abad9b7312f060479a036f1d1ff2215 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:08:47 -0400 Subject: [PATCH 4/9] fix(abcip): report ALMD transitions with ConditionId = RawPath and a Kind (#641) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found while verifying item 4 of #641. The host routes a native alarm transition on an exact ConditionId == RawPath match (DriverHostActor.ForwardNativeAlarm), but AbCipAlarmProjection emitted "{ref}#active" and no Kind, so every ALMD raise/clear was dropped as "no alarm condition" — and even if routed, an Unspecified kind projects no state change. It now reports the subscribed reference (the RawPath) and Raise/Clear. A host test pins that a decorated id is dropped. FOCAS has the same id shape plus a device filter keyed by host address (a RawPath subscription polls nothing); it needs a mapping decision and is documented in docs/drivers/FOCAS.md rather than patched here. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/drivers/AbCip.md | 4 ++++ docs/drivers/FOCAS.md | 7 +++++++ .../AbCipAlarmProjection.cs | 14 +++++++++---- .../AbCipAlarmProjectionTests.cs | 15 +++++++++++++ .../DriverHostActorNativeAlarmTests.cs | 21 +++++++++++++++++++ 5 files changed, 57 insertions(+), 4 deletions(-) diff --git a/docs/drivers/AbCip.md b/docs/drivers/AbCip.md index c71dcfe7..276a1735 100644 --- a/docs/drivers/AbCip.md +++ b/docs/drivers/AbCip.md @@ -264,6 +264,10 @@ analog alarms are a follow-up) and **disabled by default** — shops running FT Alarm & Events should keep it off and take alarms through the native route, since the projection semantics don't exactly mirror Rockwell FT A&E. +Each event carries `ConditionId` = the subscribed reference (the alarm tag's RawPath — the key the host +routes native transitions on) and `Kind` = `Raise` / `Clear`. Before Gitea #641 the projection sent +`"{ref}#active"` with no kind, so the host dropped every ALMD transition as unrouted. + ## Testing - **Unit tests** — `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/` cover the driver, host-address parser, UDT planner, and alarm projection via fake tag runtimes. diff --git a/docs/drivers/FOCAS.md b/docs/drivers/FOCAS.md index f2442d1b..305d6444 100644 --- a/docs/drivers/FOCAS.md +++ b/docs/drivers/FOCAS.md @@ -294,6 +294,13 @@ Every alarm transition fires `OnAlarmEvent` with: Cleared alarms fire a second event with `" (cleared)"` appended to the message so downstream consumers can ignore the clear if they only care about raises. +> ⚠️ **Not routable to a v3 raw condition (found under Gitea #641).** The server routes a native transition +> only when `ConditionId` equals an alarm-bearing raw tag's RawPath, and the host subscribes those RawPaths — +> but this projection filters devices by **host address** (so a RawPath list matches no device and nothing is +> polled) and emits `"{host}#{Type}:{AlarmNumber}"` with no `Kind`. FOCAS native alarms therefore reach no +> Part 9 condition today. Fixing it needs a mapping decision (the CNC exposes one flat alarm list per device, +> not one condition per tag), so it is tracked separately rather than patched. + ## Handle recycling FANUC CNCs have a finite FWLIB handle pool (~5–10 concurrent connections) and diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs index 83629420..ac429441 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs @@ -193,23 +193,29 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable var wasFaulted = sub.LastInFaulted.GetValueOrDefault(nodeId, false); sub.LastInFaulted[nodeId] = nowFaulted; + // ConditionId is the subscribed reference itself — the alarm tag's RawPath, the key the host routes + // native transitions on — and Kind tells the host's projector what changed (Gitea #641). This used + // to send "{ref}#active" with no Kind, so the host dropped every ALMD transition as unrouted (and an + // Unspecified kind would have projected no state change anyway). if (!wasFaulted && nowFaulted) { _driver.InvokeAlarmEvent(new AlarmEventArgs( - sub.Handle, nodeId, ConditionId: $"{nodeId}#active", + sub.Handle, nodeId, ConditionId: nodeId, AlarmType: "ALMD", Message: $"ALMD {nodeId} raised", Severity: MapSeverity(severity), - SourceTimestampUtc: DateTime.UtcNow)); + SourceTimestampUtc: DateTime.UtcNow, + Kind: AlarmTransitionKind.Raise)); } else if (wasFaulted && !nowFaulted) { _driver.InvokeAlarmEvent(new AlarmEventArgs( - sub.Handle, nodeId, ConditionId: $"{nodeId}#active", + sub.Handle, nodeId, ConditionId: nodeId, AlarmType: "ALMD", Message: $"ALMD {nodeId} cleared", Severity: MapSeverity(severity), - SourceTimestampUtc: DateTime.UtcNow)); + SourceTimestampUtc: DateTime.UtcNow, + Kind: AlarmTransitionKind.Clear)); } } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs index 808b3043..558554f5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs @@ -189,6 +189,21 @@ public sealed class AbCipAlarmProjectionTests await drv.UnsubscribeAlarmsAsync(handle, CancellationToken.None); await drv.ShutdownAsync(CancellationToken.None); + + // Gitea #641 (item 4): the host routes a native transition on ConditionId == the subscribed RawPath and + // projects it from Kind. The projection used to emit "{ref}#active" with an Unspecified kind, so every + // ALMD transition was dropped as "no alarm condition" — and would have projected no state change even if + // it had routed. + AlarmEventArgs raised, cleared; + lock (events) + { + raised = events.First(e => e.Message.Contains("raised")); + cleared = events.First(e => e.Message.Contains("cleared")); + } + raised.ConditionId.ShouldBe("HighTemp"); + raised.Kind.ShouldBe(AlarmTransitionKind.Raise); + cleared.ConditionId.ShouldBe("HighTemp"); + cleared.Kind.ShouldBe(AlarmTransitionKind.Clear); } /// Verifies that unsubscribing stops the alarm poll loop. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs index 2a2fadaa..1ac1b060 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs @@ -104,6 +104,27 @@ public sealed class DriverHostActorNativeAlarmTests : RuntimeActorTestBase publish.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); } + /// Gitea #641 (item 4) — routing is an EXACT match on the RawPath: a driver that decorates its + /// ConditionId (AbCip used to send "{RawPath}#active", FOCAS sends "{host}#{type}:{number}") has + /// every transition dropped, so a native source MUST report ConditionId = RawPath. + [Fact] + public void A_decorated_condition_id_is_dropped() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedV3AlarmDeployment(db, RevA, Driver: "drv-1", Tag: "temp_hi"); + var (actor, publish) = SpawnHostAndApply(db, deploymentId); + + actor.Tell(new DriverInstanceActor.AttributeAlarmPublished("drv-1", new AlarmEventArgs( + new StubAlarmHandle(), AlarmRawPath, $"{AlarmRawPath}#active", "ALMD", "decorated", AlarmSeverity.High, Ts, + Kind: AlarmTransitionKind.Raise))); + // Positive control: the undecorated id routes, and is the first update. + actor.Tell(new DriverInstanceActor.AttributeAlarmPublished("drv-1", new AlarmEventArgs( + new StubAlarmHandle(), AlarmRawPath, AlarmRawPath, "ALMD", "plain", AlarmSeverity.High, Ts, + Kind: AlarmTransitionKind.Raise))); + + publish.ExpectMsg(TimeSpan.FromSeconds(5)).State.Message.ShouldBe("plain"); + } + /// Primary (default/unset role) fan-out: a native alarm RAISE on a known ref publishes exactly one /// to the cluster alerts topic with AlarmId = the raw /// condition NodeId (RawPath), alongside the (ungated) OPC UA condition update. From bd23f557f31c74a8a2334a4dd749726be45518dc Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:09:24 -0400 Subject: [PATCH 5/9] docs(alarms): native condition state across a rebuild, projector prune, exact ConditionId routing (#641) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 9 +++++++-- docs/AlarmTracking.md | 45 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1c68f757..026af80a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1136,10 +1136,15 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa active nativeCode (ties: ordinal nativeCode, then type); ack is server-side only. ⚠️ UNAVAILABLE never clears: an active condition stays active+acked with a quality-only `BadNoCommunication` (same as #481 transport loss). The driver dedups against what the server was last TOLD (level+nativeCode+nativeSeverity+qualifier — not text), records - only while a handler is attached, and clears that record when the subscribed alarm SET changes (a rebuild recreates - the nodes inactive) but not on a reconnect of the same set — a replayed `/current` must never re-raise (un-ack). + only while a handler is attached, and drops the record of only the conditions that LEFT the set when the subscribed + alarm SET changes (#641) — never on a reconnect of the same set; a replayed `/current` must never re-raise (un-ack). `AlarmEventArgs` gained optional `OpcUaSeverity` (exact Part 9 severity) and `QualityStatusCode`; a quality-only event goes out as `AlarmQualityUpdate` per node and never through the snapshot projection. +- **Native condition state survives a full rebuild (#641):** `RebuildAddressSpace` captures each NATIVE condition's + live node state (client acks included), the same-apply re-materialise restores it, and `CompleteAddressSpaceApply` + (called by `OpcUaPublishActor` at the end of every apply) reports ONE event per restored off-normal condition and + discards unclaimed captures. `NativeAlarmProjector` is pruned per apply, not cleared. Native routing is an exact + `ConditionId == RawPath` match — AbCip's `#active` suffix was fixed; FOCAS is still unroutable (`docs/drivers/FOCAS.md`). - **Driver removal (#609):** removing a driver releases its capability-invoker pipelines and resilience tracker, *then* evicts its telemetry snapshots. ⚠️ Both are **tombstoned**, not just cleared: the removed child stops asynchronously and its in-flight subscribe/connect still completes through the invoker (and diff --git a/docs/AlarmTracking.md b/docs/AlarmTracking.md index 5a021ac3..ec7344c4 100644 --- a/docs/AlarmTracking.md +++ b/docs/AlarmTracking.md @@ -54,6 +54,51 @@ same condition is wired as an **event notifier of every referencing equipment fo - The `alerts`-topic `AlarmTransitionEvent` carries the (possibly empty) **list of referencing equipment paths**, and the AdminUI `/alerts` page shows **one row per condition** (primary identity RawPath + condition NodeId) with the equipment list as display metadata. +- **Routing is an exact `ConditionId == RawPath` match** (`DriverHostActor.ForwardNativeAlarm`). A driver that + decorates the id has every transition dropped as "no alarm condition" — AbCip did (`"{ref}#active"`, fixed + under #641); FOCAS still does (see `drivers/FOCAS.md`). A native source must also set `Kind`: an + `Unspecified` kind projects no state change. + +### Native condition state survives a full rebuild (Gitea #641) + +A full `RebuildAddressSpace` recreates every condition inactive + acknowledged. For **scripted** alarms the +engine re-asserts every non-normal condition after the reload (#487). **Native** conditions had nothing +equivalent: the host re-subscribes a driver's alarms only when its alarm-ref *set* changes, and a +transition-only source (Galaxy, OpcUaClient) has nothing to replay anyway — so a rebuild that left the alarm +set unchanged (adding one `UnsTagReference` to an alarm-bearing raw tag is enough to force one, as are +`ChangedEquipment`, a scripted-alarm edit, a raw container rename, an alarm severity/type edit, or any +surgical fallback to `SafeRebuild()`) left every active native alarm reading **normal** until its next device +transition. + +The node manager now carries native state across the rebuild itself: + +1. `RebuildAddressSpace` captures each **native** condition's live state off the **node** — Active, Acked, + Confirmed, Severity, Message, Quality, Time, `ActiveState.TransitionTime` — before clearing. Reading the + node, not a host-side cache, is what keeps a **client's** acknowledgement: the SDK applies it to the node + and neither the driver nor `NativeAlarmProjector` ever sees it. +2. `MaterialiseAlarmCondition(isNative: true)` for the same id in the same apply restores it. **Quality is + always restored** (it is otherwise re-annotated only on a connectivity edge, so even a normal condition + would sit at `BadWaitingForInitialData`); the rest only when the condition was off-normal (active or + unacknowledged). A normal condition keeps what the materialise built — message and authored severity + included — exactly the #487 rule. Shelving/Enabled are not carried (native conditions refuse them, #587). +3. `IOpcUaAddressSpaceSink.CompleteAddressSpaceApply`, which `OpcUaPublishActor` calls at the end of + **every** apply (in a `finally`), reports **one** condition event per restored condition — after the + notifier wiring, so the raw folder, every referencing equipment folder and `i=2253` items all get it, + stamped with the original `Time`, not the deploy time — and **discards** every capture no materialise + claimed, so a later deploy re-adding the same RawPath gets a fresh node (no resurrection). A kind swap + (native ↔ scripted on one id) is never restored. Node-only: no `/alerts` row, no historian record. + +Two companions keep the host and driver in step with the carried node: + +- `DriverHostActor` **prunes** `NativeAlarmProjector` to the still-deployed conditions on each apply + (`RetainOnly`) instead of clearing it. Clearing made the next driver Acknowledge of an active alarm + project `Active=false`, and the next Clear of an unacknowledged one project `Acked=true` — after *any* + deploy, rebuild or not. +- MTConnect's "last told" record is pruned to the conditions that **left** the subscribed set rather than + cleared on any set change; re-raising a surviving (carried) condition would un-acknowledge it. + +Accepted residual: a condition that clears at the device while the rebuild runs is restored active until +the driver's Clear arrives — which it does, because the driver's own record still holds it active. ## Condition event identity fields (what a client reads on the wire) From 5c88de078c2ea35d726012baca4061b0bd3b109f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:17:55 -0400 Subject: [PATCH 6/9] fix(abcip): keep the ALMD last-told state across a re-subscribe so a still-faulted alarm is not re-raised (#641 review) Now that ALMD transitions route, a re-subscribe (every reconnect, every alarm-set change) started from an empty per-subscription record and re-RAISED every still-faulted alarm, which the host projects as Active+unacked - un-acknowledging a condition the server had just carried across the rebuild with its client ack. The record is now driver-owned and pruned to the new set, mirroring MTConnect's _deliveredConditions. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../AbCipAlarmProjection.cs | 48 +++++++++++++-- .../AbCipDriver.cs | 4 ++ .../AbCipAlarmProjectionTests.cs | 59 +++++++++++++++++++ 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs index ac429441..73f6261b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs @@ -70,6 +70,12 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable // prior subscriptions before starting the new one is semantically faithful. Snapshotting + // clearing under the same lock DisposeAsync uses guarantees the stale subs can never be // double-owned (and thus never double-disposed) by a racing dispose. + // Gitea #641 review — forget only the refs that LEFT the set. A survivor keeps its last-seen InFaulted, so + // a re-subscribe (every reconnect, every alarm-set change) does not re-RAISE a still-faulted alarm: the + // server carries the condition across a rebuild, and a Raise would un-acknowledge it. A ref that left and + // comes back was materialised fresh, so it starts from "not faulted" and is raised again. + _driver.AlarmFaultRecord.RetainOnly(sub.SourceNodeIds); + List stale; lock (_subsLock) { @@ -190,8 +196,7 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable // is intentional and matches the documented behaviour. var severity = ToInt(severityDv.Value); - var wasFaulted = sub.LastInFaulted.GetValueOrDefault(nodeId, false); - sub.LastInFaulted[nodeId] = nowFaulted; + var wasFaulted = _driver.AlarmFaultRecord.Exchange(nodeId, nowFaulted); // ConditionId is the subscribed reference itself — the alarm tag's RawPath, the key the host routes // native transitions on — and Kind tells the host's projector what changed (Gitea #641). This used @@ -303,9 +308,44 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable /// Gets or sets the polling loop task. public Task Loop { get; set; } = Task.CompletedTask; + } +} - /// Gets the dictionary tracking the last known InFaulted state for each node. - public Dictionary LastInFaulted { get; } = new(StringComparer.Ordinal); +/// +/// Gitea #641 review — the last InFaulted the ALMD projection told the server, per alarm RawPath. Owned by +/// the rather than a subscription or a projection, so it survives a re-subscribe and a +/// re-initialise (both of which happen on every reconnect): the server's condition nodes outlive them, and a +/// re-raise there would un-acknowledge. Thread-safe — a retiring poll loop may overlap the new one for a tick. +/// +internal sealed class AbCipAlarmFaultRecord +{ + private readonly Dictionary _lastInFaulted = new(StringComparer.Ordinal); + private readonly Lock _lock = new(); + + /// Records for , returning the prior value + /// (false when none). + /// The alarm RawPath. + /// The state just read. + /// The previously recorded state. + public bool Exchange(string nodeId, bool nowFaulted) + { + lock (_lock) + { + var was = _lastInFaulted.GetValueOrDefault(nodeId, false); + _lastInFaulted[nodeId] = nowFaulted; + return was; + } + } + + /// Forgets every ref not in . + /// The newly subscribed refs. + public void RetainOnly(IReadOnlyList nodeIds) + { + var keep = new HashSet(nodeIds, StringComparer.Ordinal); + lock (_lock) + { + foreach (var k in _lastInFaulted.Keys.Where(k => !keep.Contains(k)).ToList()) _lastInFaulted.Remove(k); + } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index a80a86e0..2fade80c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -118,6 +118,10 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, private readonly ILogger _logger; private AbCipAlarmProjection _alarmProjection; + + /// Gitea #641 review — the ALMD projection's last-told InFaulted per alarm ref; outlives every + /// projection and subscription (see ). + internal AbCipAlarmFaultRecord AlarmFaultRecord { get; } = new(); private DriverHealth _health = new(DriverState.Unknown, null, null); /// Occurs when a subscribed tag's value changes. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs index 558554f5..ba312794 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs @@ -206,6 +206,65 @@ public sealed class AbCipAlarmProjectionTests cleared.Kind.ShouldBe(AlarmTransitionKind.Clear); } + /// + /// Gitea #641 review — the server carries every surviving native condition's state (a client's + /// acknowledgement included) across an address-space rebuild, and the host re-subscribes alarms on every + /// reconnect and every alarm-set change. A re-subscribe used to start from an empty "last seen" record, so + /// a still-faulted ALMD was re-RAISED, which the host projects as Active+UNacked — un-acknowledging an alarm + /// an operator had already acknowledged. The record is the driver's, kept for refs that stay in the set. + /// + [Fact] + public async Task A_resubscribe_does_not_re_raise_a_still_faulted_alarm() + { + var factory = new FakeAbCipTagFactory(); + var drv = new AbCipDriver(Options(enabled: true), "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var raises = new List(); + drv.OnAlarmEvent += (_, e) => { if (e.Kind == AlarmTransitionKind.Raise) lock (raises) raises.Add(e); }; + + var sub1 = await drv.SubscribeAlarmsAsync(["HighTemp"], CancellationToken.None); + await WaitForTagCreation(factory, "HighTemp"); + factory.Tags["HighTemp"].ValuesByOffset[8] = 500; + factory.Tags["HighTemp"].ValuesByOffset[0] = 1; + await WaitForEventAsync(raises, _ => true); + + // A reconnect / alarm-set change: the same ref, still faulted. + var sub2 = await drv.SubscribeAlarmsAsync(["HighTemp"], CancellationToken.None); + await WaitForMoreReadsAsync(factory, "HighTemp", 6); + + lock (raises) raises.Count.ShouldBe(1, "a re-subscribe must not re-raise (and so un-acknowledge) a still-faulted alarm"); + + await drv.UnsubscribeAlarmsAsync(sub1, CancellationToken.None); + await drv.UnsubscribeAlarmsAsync(sub2, CancellationToken.None); + await drv.ShutdownAsync(CancellationToken.None); + } + + /// Gitea #641 review — positive control: a ref that LEFT the subscribed set was materialised fresh + /// (inactive) when it came back, so it is raised again. + [Fact] + public async Task A_ref_that_left_the_set_and_came_back_is_raised_again() + { + var factory = new FakeAbCipTagFactory(); + var drv = new AbCipDriver(Options(enabled: true), "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var raises = new List(); + drv.OnAlarmEvent += (_, e) => { if (e.Kind == AlarmTransitionKind.Raise) lock (raises) raises.Add(e); }; + + await drv.SubscribeAlarmsAsync(["HighTemp"], CancellationToken.None); + await WaitForTagCreation(factory, "HighTemp"); + factory.Tags["HighTemp"].ValuesByOffset[8] = 500; + factory.Tags["HighTemp"].ValuesByOffset[0] = 1; + await WaitForEventAsync(raises, _ => true); + + await drv.SubscribeAlarmsAsync([], CancellationToken.None); + await drv.SubscribeAlarmsAsync(["HighTemp"], CancellationToken.None); + await WaitForEventAsync(raises, _ => raises.Count >= 2); + + await drv.ShutdownAsync(CancellationToken.None); + } + /// Verifies that unsubscribing stops the alarm poll loop. [Fact] public async Task Unsubscribe_Stops_The_Poll_Loop() From 25f02e41028ac771875dfeaa2d09e9968d2891e3 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:19:24 -0400 Subject: [PATCH 7/9] fix(runtime): the native-alarm projector learns a routed client acknowledgement (#641 review) #641 keeps the projector's prior state across a deploy instead of resetting it to the acked default. The projector never saw a client's ack (the SDK applies it to the node and the host only routes it), so a Clear after ack+deploy now projected Acked=false and un-acknowledged the operator's ack - previously masked by the per-deploy reset. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../Drivers/DriverHostActor.cs | 6 +++++ .../Drivers/NativeAlarmProjector.cs | 11 ++++++++ .../DriverHostActorNativeAlarmTests.cs | 27 +++++++++++++++++++ 3 files changed, 44 insertions(+) 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 e62c8d2d..b287ca3c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -2018,6 +2018,12 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS return; } + // Gitea #641 review — the node is already acknowledged (the SDK applies the client's ack); tell the + // projector too, or the next driver Clear projects (Active=false, Acked=prev=false) and un-acknowledges it. + // Most sources (MTConnect, FOCAS) send no Acknowledge transition back, and the projector now survives a + // deploy rather than resetting to its acked default. + _nativeAlarmProjector.MarkAcknowledged(msg.ConditionNodeId); + if (!_children.TryGetValue(target.DriverInstanceId, out var entry)) { _log.Debug("DriverHost {Node}: driver {Driver} not running for alarm ack of {Cond} — dropped", diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs index fb0f8075..9e10f29b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs @@ -40,6 +40,17 @@ public sealed class NativeAlarmProjector StatusCode: e.QualityStatusCode ?? OpcUaStatus.Good); } + /// + /// Gitea #641 review — record a client acknowledgement the node manager applied to + /// and routed toward the driver. The projector otherwise never sees it, and a later Clear would project the + /// stale unacknowledged prior. An untracked condition already defaults to acknowledged. + /// + /// The acknowledged condition node id. + public void MarkAcknowledged(string nodeId) + { + if (_prior.TryGetValue(nodeId, out var p)) _prior[nodeId] = (p.Active, true); + } + /// Clears all tracked per-node state. public void Clear() => _prior.Clear(); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs index 1ac1b060..36701991 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs @@ -473,6 +473,33 @@ public sealed class DriverHostActorNativeAlarmTests : RuntimeActorTestBase cleared.State.Acknowledged.ShouldBeFalse("nobody acknowledged it"); } + /// + /// Gitea #641 review — a CLIENT acknowledgement is applied to the node by the SDK and only ROUTED here; the + /// projector must learn of it. Before #641 the projector was cleared on every deploy, so a Clear arriving + /// after one defaulted Acked to true and happened to be right for a client-acked alarm; now that the + /// projector survives the deploy, a projector that never heard the ack projects the Clear as UNacked and + /// un-acknowledges the alarm the operator acknowledged. + /// + [Fact] + public void A_clear_after_a_client_acknowledge_and_a_redeploy_stays_acknowledged() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedV3AlarmDeployment(db, RevA, Driver: "drv-1", Tag: "temp_hi"); + var (actor, publish, coordinator) = SpawnHostAndApplyWithCoordinator(db, deploymentId); + + actor.Tell(Transition(AlarmTransitionKind.Raise)); + publish.ExpectMsg(TimeSpan.FromSeconds(5)).State.Acknowledged.ShouldBeFalse(); + + // The client's Acknowledge on the node, routed toward the driver (no transition comes back for it). + actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, "seen", "op")); + Redeploy(db, actor, publish, coordinator); + + actor.Tell(Transition(AlarmTransitionKind.Clear)); + var cleared = publish.FishForMessage(_ => true, TimeSpan.FromSeconds(5)); + cleared.State.Active.ShouldBeFalse(); + cleared.State.Acknowledged.ShouldBeTrue("the operator acknowledged it before it cleared"); + } + private static DriverInstanceActor.AttributeAlarmPublished Transition(AlarmTransitionKind kind) => new("drv-1", new AlarmEventArgs( new StubAlarmHandle(), "Temp", AlarmRawPath, "OffNormalAlarm", kind.ToString(), AlarmSeverity.High, Ts, From 17dc17194038ae15c25b28a67d6f3706663f52db Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:20:13 -0400 Subject: [PATCH 8/9] 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")] From fcae87a5bcf79adeac7a20085e9b3d0040efe5be Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:20:38 -0400 Subject: [PATCH 9/9] docs(alarms): #641 review follow-ups (comment carry, AbCip record, projector ack, residuals) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/AlarmTracking.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/AlarmTracking.md b/docs/AlarmTracking.md index ec7344c4..a475ce3a 100644 --- a/docs/AlarmTracking.md +++ b/docs/AlarmTracking.md @@ -73,7 +73,7 @@ transition. The node manager now carries native state across the rebuild itself: 1. `RebuildAddressSpace` captures each **native** condition's live state off the **node** — Active, Acked, - Confirmed, Severity, Message, Quality, Time, `ActiveState.TransitionTime` — before clearing. Reading the + Confirmed, Severity, Message, Comment + ClientUserId, Quality, Time, `ActiveState.TransitionTime` — before clearing. Reading the node, not a host-side cache, is what keeps a **client's** acknowledgement: the SDK applies it to the node and neither the driver nor `NativeAlarmProjector` ever sees it. 2. `MaterialiseAlarmCondition(isNative: true)` for the same id in the same apply restores it. **Quality is @@ -84,7 +84,8 @@ The node manager now carries native state across the rebuild itself: 3. `IOpcUaAddressSpaceSink.CompleteAddressSpaceApply`, which `OpcUaPublishActor` calls at the end of **every** apply (in a `finally`), reports **one** condition event per restored condition — after the notifier wiring, so the raw folder, every referencing equipment folder and `i=2253` items all get it, - stamped with the original `Time`, not the deploy time — and **discards** every capture no materialise + stamped with the original `Time`, not the deploy time (or the node's newer `Time`, if a client method call + reported in the restore→complete window — that window can yield a second, state-correct event) — and **discards** every capture no materialise claimed, so a later deploy re-adding the same RawPath gets a fresh node (no resurrection). A kind swap (native ↔ scripted on one id) is never restored. Node-only: no `/alerts` row, no historian record. @@ -95,10 +96,16 @@ Two companions keep the host and driver in step with the carried node: project `Active=false`, and the next Clear of an unacknowledged one project `Acked=true` — after *any* deploy, rebuild or not. - MTConnect's "last told" record is pruned to the conditions that **left** the subscribed set rather than - cleared on any set change; re-raising a surviving (carried) condition would un-acknowledge it. + cleared on any set change; re-raising a surviving (carried) condition would un-acknowledge it. AbCip's ALMD + last-seen `InFaulted` record is likewise driver-owned and pruned to the set, so a re-subscribe (every + reconnect / set change) does not re-raise a still-faulted alarm. +- A routed **client** acknowledgement marks the projector's prior state acknowledged + (`NativeAlarmProjector.MarkAcknowledged`), so a later driver Clear does not project it unacknowledged. Accepted residual: a condition that clears at the device while the rebuild runs is restored active until -the driver's Clear arrives — which it does, because the driver's own record still holds it active. +the driver's Clear arrives — which it does, because the driver's own record still holds it active. The capture +is keyed by RawPath only, so a tag deleted and re-created under the same RawPath in one deploy inherits the old +condition's state until its next transition. ## Condition event identity fields (what a client reads on the wire)