From e56926dc473a3ee9c423b7a73a59ff96b4564c20 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 16:54:44 -0400 Subject: [PATCH] fix(opcuaclient): native alarms translate RawPath <-> upstream source/condition ids (#619) The host subscribes alarms with RawPaths and routes transitions on (driver, RawPath), but the driver filtered upstream SourceNode ids against the RawPaths and reported the upstream condition NodeId as ConditionId, so no opc-plc (or any upstream) alarm could ever reach a condition node. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../OpcUaClientDriver.cs | 85 +++++++++++++++---- .../OpcUaClientAlarmTests.cs | 61 +++++++++++++ 2 files changed, 131 insertions(+), 15 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index 03c7e758..ee833757 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -92,6 +92,12 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // ---- IAlarmSource state ---- private readonly System.Collections.Concurrent.ConcurrentDictionary _alarmSubscriptions = new(); + + /// #619: RawPath → the upstream condition NodeId last seen for it. The host acknowledges by + /// RawPath (its only identity for the condition) but Part 9 Acknowledge must be called on the CONDITION + /// node, which differs from the authored source node. + private readonly System.Collections.Concurrent.ConcurrentDictionary _alarmConditionByRawPath = + new(StringComparer.Ordinal); private long _nextAlarmSubscriptionId; /// Occurs when an alarm event is received from the remote OPC UA server. @@ -1635,7 +1641,14 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // Pre-resolve the source-node filter set so the per-event notification handler can // match in O(1) without re-parsing on every event. - var sourceFilter = new HashSet(sourceNodeIds, StringComparer.Ordinal); + // #619: under v3 the host passes RawPaths, never upstream node ids. Translate each to the upstream + // source id its tag authored (TagConfig.nodeId) so the per-event match compares like with like. + var sessionForResolve = Session; + var sourceFilter = BuildAlarmSourceIndex(sourceNodeIds, raw => + { + if (sessionForResolve is null || !TryResolveTagNode(sessionForResolve, raw, out var live)) return null; + return live.ToString(); + }); var subscription = new Subscription(telemetry: null!, new SubscriptionOptions { @@ -1767,7 +1780,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit { // ConditionId is an upstream condition-node id captured from the alarm event (not a // tag RawPath), so it re-binds directly against the session via the static resolve. - if (!NamespaceMap.TryResolve(session, ack.ConditionId, out var conditionId)) continue; + // #619: the host acks by RawPath; Acknowledge must hit the upstream CONDITION node last seen for it. + var upstreamCondition = _alarmConditionByRawPath.TryGetValue(ack.ConditionId, out var seen) + ? seen : ack.ConditionId; + if (!NamespaceMap.TryResolve(session, upstreamCondition, out var conditionId)) continue; callRequests.Add(new CallMethodRequest { ObjectId = conditionId, @@ -1831,30 +1847,69 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit private void OnEventNotification( OpcUaAlarmSubscriptionHandle handle, - HashSet sourceFilter, + IReadOnlyDictionary sourceFilter, MonitoredItem item, MonitoredItemNotificationEventArgs args) { if (args.NotificationValue is not EventFieldList efl) return; - if (efl.EventFields.Count <= AlarmFieldConditionId) return; + var e = ProjectEvent(efl.EventFields, sourceFilter, handle, + (raw, cond) => _alarmConditionByRawPath[raw] = cond); + if (e is not null) OnAlarmEvent?.Invoke(this, e); + } - var sourceNode = efl.EventFields[AlarmFieldSourceNode].Value?.ToString() ?? string.Empty; - if (sourceFilter.Count > 0 && !sourceFilter.Contains(sourceNode)) return; + /// + /// #619 — index an alarm subscription's source refs by UPSTREAM source id. The host (v3) subscribes with + /// the alarm tags' RawPaths; an upstream event carries its own source NodeId, so the match key is + /// the upstream id the tag authored and the value is the RawPath to report back. A ref + /// cannot resolve is kept verbatim (direct upstream-id use). + /// + /// The refs handed to SubscribeAlarmsAsync. + /// RawPath → serialized live upstream NodeId, or null when unknown. + /// Upstream source id → ref to report. + internal static Dictionary BuildAlarmSourceIndex( + IReadOnlyList refs, Func resolveUpstream) + { + var index = new Dictionary(StringComparer.Ordinal); + foreach (var r in refs) + index[resolveUpstream(r) ?? r] = r; + return index; + } - var eventType = efl.EventFields[AlarmFieldEventType].Value?.ToString() ?? "BaseEventType"; - var message = (efl.EventFields[AlarmFieldMessage].Value as LocalizedText)?.Text ?? string.Empty; - var severity = efl.EventFields[AlarmFieldSeverity].Value is ushort sev ? sev : (ushort)0; - var time = efl.EventFields[AlarmFieldTime].Value is DateTime t ? t : DateTime.UtcNow; - var conditionId = efl.EventFields[AlarmFieldConditionId].Value?.ToString() ?? string.Empty; + /// + /// #619 — project one upstream event into an . ConditionId and + /// SourceNodeId are the authored RawPath, because DriverHostActor routes native alarms on + /// (driver, RawPath); the upstream condition node (needed to acknowledge) is handed to + /// . Null when the event's source is not an authored one. + /// + internal static AlarmEventArgs? ProjectEvent( + IReadOnlyList fields, + IReadOnlyDictionary sourceIndex, + IAlarmSubscriptionHandle handle, + Action rememberCondition) + { + if (fields.Count <= AlarmFieldConditionId) return null; - OnAlarmEvent?.Invoke(this, new AlarmEventArgs( + var sourceNode = fields[AlarmFieldSourceNode].Value?.ToString() ?? string.Empty; + string reported; + if (sourceIndex.Count == 0) reported = sourceNode; + else if (!sourceIndex.TryGetValue(sourceNode, out reported!)) return null; + + var eventType = fields[AlarmFieldEventType].Value?.ToString() ?? "BaseEventType"; + var message = (fields[AlarmFieldMessage].Value as LocalizedText)?.Text ?? string.Empty; + var severity = fields[AlarmFieldSeverity].Value is ushort sev ? sev : (ushort)0; + var time = fields[AlarmFieldTime].Value is DateTime t ? t : DateTime.UtcNow; + var upstreamCondition = fields[AlarmFieldConditionId].Value?.ToString() ?? string.Empty; + + if (sourceIndex.Count != 0 && upstreamCondition.Length > 0) rememberCondition(reported, upstreamCondition); + + return new AlarmEventArgs( SubscriptionHandle: handle, - SourceNodeId: sourceNode, - ConditionId: conditionId, + SourceNodeId: reported, + ConditionId: sourceIndex.Count == 0 ? upstreamCondition : reported, AlarmType: eventType, Message: message, Severity: MapSeverity(severity), - SourceTimestampUtc: time)); + SourceTimestampUtc: time); } /// diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAlarmTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAlarmTests.cs index 08008733..186eee3d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAlarmTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAlarmTests.cs @@ -1,3 +1,4 @@ +using Opc.Ua; using Shouldly; using Xunit; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; @@ -71,6 +72,66 @@ public sealed class OpcUaClientAlarmTests await drv.AcknowledgeAsync([], TestContext.Current.CancellationToken); } + // ---- #619: v3 RawPath <-> upstream-source translation on the native-alarm path ---- + + private static VariantCollection EventFields(string sourceNode, string conditionNode) => + [ + new Variant(new byte[] { 1 }), // EventId + new Variant(new NodeId("TripAlarmType", 3)), // EventType + new Variant(NodeId.Parse(sourceNode)), // SourceNode + new Variant(new LocalizedText("tripped")), // Message + new Variant((ushort)700), // Severity + new Variant(DateTime.UtcNow), // Time + new Variant(NodeId.Parse(conditionNode)), // ConditionId + ]; + + /// The host hands SubscribeAlarmsAsync RawPaths; the index must be keyed by the UPSTREAM source id. + [Fact] + public void BuildAlarmSourceIndex_keys_the_upstream_source_id_to_the_RawPath() + { + var index = OpcUaClientDriver.BuildAlarmSourceIndex( + ["Plant/opc/plc/Trip1"], + raw => raw == "Plant/opc/plc/Trip1" ? "ns=3;s=Alarms.Trip1" : null); + + index["ns=3;s=Alarms.Trip1"].ShouldBe("Plant/opc/plc/Trip1"); + } + + /// A ref the authored table cannot resolve is kept verbatim (direct upstream-id use stays valid). + [Fact] + public void BuildAlarmSourceIndex_keeps_an_unresolvable_ref_as_its_own_key() + { + var index = OpcUaClientDriver.BuildAlarmSourceIndex(["ns=3;s=Direct"], _ => null); + index["ns=3;s=Direct"].ShouldBe("ns=3;s=Direct"); + } + + /// The emitted ConditionId must be the RawPath - DriverHostActor routes on (driver, RawPath). + [Fact] + public void ProjectEvent_reports_the_RawPath_as_ConditionId_and_remembers_the_condition_node() + { + var index = new Dictionary { ["ns=3;s=Alarms.Trip1"] = "Plant/opc/plc/Trip1" }; + string? rememberedRaw = null, rememberedCond = null; + + var e = OpcUaClientDriver.ProjectEvent( + EventFields("ns=3;s=Alarms.Trip1", "ns=3;s=Alarms.Trip1.Cond"), index, new FakeAlarmHandle(), + (raw, cond) => { rememberedRaw = raw; rememberedCond = cond; }); + + e.ShouldNotBeNull(); + e.ConditionId.ShouldBe("Plant/opc/plc/Trip1"); + e.SourceNodeId.ShouldBe("Plant/opc/plc/Trip1"); + rememberedRaw.ShouldBe("Plant/opc/plc/Trip1"); + rememberedCond.ShouldBe("ns=3;s=Alarms.Trip1.Cond"); + } + + /// An event from a source nobody authored is filtered out. + [Fact] + public void ProjectEvent_drops_an_unauthored_source() + { + var index = new Dictionary { ["ns=3;s=Alarms.Trip1"] = "Plant/opc/plc/Trip1" }; + OpcUaClientDriver.ProjectEvent( + EventFields("ns=3;s=Other", "ns=3;s=Other.Cond"), index, new FakeAlarmHandle(), (_, _) => { }) + .ShouldBeNull(); + } + private sealed class FakeAlarmHandle : IAlarmSubscriptionHandle { /// Gets the diagnostic identifier for this alarm handle.