From 846cc5d96ddf72bb3f6dca15633c9610259a44cf Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 17 Jul 2026 14:02:27 -0400 Subject: [PATCH] fix(dcl): route native OPC UA alarms by binding identity, not event name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A native OPC UA alarm source's SourceReference has to be two things at once: it is parsed as a NodeId to open the monitored item, and matched as a plain-name prefix against the event's SourceName to route the transition to an instance. No string is both, so a NodeId-form binding subscribed correctly and then silently dropped every transition — "pymodbus/plc/HR200".StartsWith("nsu=...;s=pymodbus/plc/HR200") is false. Only the empty (Server-object) binding worked, because StartsWith("") matches everything, which is why the sole live smoke test never caught it. Each OPC UA alarm feed is opened for exactly one binding, so every transition on it belongs to that binding. The adapter now tags each transition's routing identity (SourceObjectReference) with the binding string verbatim via the pure OpcUaAlarmMapper.BuildIdentity, making DataConnectionActor's routing key an exact match regardless of whether the binding is stored as ns= or the durable nsu= form. The Server-object aggregate feed keeps an empty routing identity, so it reaches only "mirror everything" subscribers and never leaks into a specific-node binding. The per-condition SourceReference key stays the readable SourceName.ConditionName, so persistence and display are unchanged, and MxGateway is untouched — its bindings are names and its mapper already emits matching names. Unblocked by lmxopcua#473 (OtOpcUa now populates SourceNode/SourceName/EventType on conditions); SourceName is the RawPath, so the per-condition key is unique. Live end-to-end verification against native alarms still needs a v3 rig. Fixes: Gitea #17 --- .../Component-DataConnectionLayer.md | 4 +- .../Adapters/OpcUaAlarmMapper.cs | 39 ++++++++++ .../Adapters/RealOpcUaClient.cs | 18 +++-- .../DataConnectionActorAlarmTests.cs | 38 ++++++++++ .../OpcUaAlarmMapperTests.cs | 71 +++++++++++++++++++ 5 files changed, 163 insertions(+), 7 deletions(-) diff --git a/docs/requirements/Component-DataConnectionLayer.md b/docs/requirements/Component-DataConnectionLayer.md index c5abdc13..edbfdbed 100644 --- a/docs/requirements/Component-DataConnectionLayer.md +++ b/docs/requirements/Component-DataConnectionLayer.md @@ -254,7 +254,9 @@ Other/custom protocols do not implement the capability; a subscribe request agai ### Connection Actor Behavior -The `DataConnectionActor` opens **one alarm feed per connection** (not per subscriber) and routes incoming transitions to instance subscribers by **source-object reference** — a prefix match of the transition's `SourceObjectReference` (falling back to `SourceReference`) against each subscriber's registered `SourceReference`. Subscribers (the Site Runtime's `NativeAlarmActor` instances) are **ref-counted per source**, so the underlying feed is opened once and torn down only when the last subscriber for that source unsubscribes. +The `DataConnectionActor` opens **one alarm feed per source** (not per subscriber) and routes incoming transitions to instance subscribers by **source-object reference** — a prefix match of the transition's `SourceObjectReference` (falling back to `SourceReference`) against each subscriber's registered `SourceReference`. Subscribers (the Site Runtime's `NativeAlarmActor` instances) are **ref-counted per source**, so the underlying feed is opened once and torn down only when the last subscriber for that source unsubscribes. + +The registered `SourceReference` is a **NodeId** for OPC UA (the picker, CSV and config all store NodeIds), so the transition's routing identity must live in the same space or nothing matches. **Gitea #17**: the OPC UA adapter previously set `SourceObjectReference` from the event's `SourceName` (a plain name — the RawPath for OtOpcUa), which never prefix-matches a NodeId binding, so every native OPC UA alarm transition was silently dropped. Because each OPC UA feed is opened for exactly one binding, the adapter now tags every transition on that feed with the **binding string verbatim** (via the pure `OpcUaAlarmMapper.BuildIdentity`), making the routing key an exact match independent of whether the binding is stored as `ns=` or the durable `nsu=` form. The Server-object aggregate feed (empty binding) keeps an empty routing identity, so it reaches only "mirror everything" subscribers and never leaks into a specific-node binding. The per-condition `SourceReference` key stays the human-readable `SourceName.ConditionName`, so persistence and display are unchanged. MxGateway is unaffected — its bindings are object names and its mapper already emits matching names. - **State gating**: `SubscribeAlarmsRequest` is handled only in the **Connected** state; requests arriving while **Connecting**/**Reconnecting** are stashed (standard Become/Stash) and processed on entering Connected. - **Capability check**: if `_adapter is not IAlarmSubscribableConnection`, the actor replies `SubscribeAlarmsResponse(Success = false, ...)`. diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaAlarmMapper.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaAlarmMapper.cs index 308307cc..5b06a419 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaAlarmMapper.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaAlarmMapper.cs @@ -12,6 +12,45 @@ namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters; /// public static class OpcUaAlarmMapper { + /// + /// Derives the two identity strings a native OPC UA alarm transition carries: + /// the routing identity that DataConnectionActor matches against the stored + /// binding, and the per-condition key used for state, persistence and display. + /// + /// + /// + /// Why the routing identity is the binding string, not the event's name (Gitea #17). + /// Routing does transition.SourceObjectReference.StartsWith(bindingRef), where the + /// binding is a NodeId reference (the picker, CSV and config all store NodeIds). Deriving + /// the routing identity from the event's SourceName — a plain name like the RawPath + /// — never prefix-matches a NodeId binding, so every transition was silently dropped. + /// Each OPC UA alarm feed is opened for exactly one binding, so every transition on it + /// belongs to that binding: tagging the transition with + /// verbatim makes the routing key an exact match, independent of whether the binding is + /// stored as ns=<index> or the durable nsu=<uri> form. An empty binding + /// (the Server-object aggregate feed) stays empty, so it reaches only the "mirror everything" + /// subscribers and never leaks into a specific-node binding. + /// + /// + /// The per-condition key keeps the human-readable (the RawPath + /// for OtOpcUa) plus so two conditions on one source stay + /// distinct; it falls back to the binding reference only when the server leaves SourceName + /// unset. + /// + /// + /// The binding string this alarm feed was subscribed under (empty for the Server-object aggregate feed). + /// The event's SourceName field (the RawPath for OtOpcUa post-#473), or null/empty when unset. + /// The event's ConditionName field, or null/empty when unset. + /// The per-condition SourceReference key and the SourceObjectReference routing identity. + public static (string SourceReference, string SourceObjectReference) BuildIdentity( + string? subscriptionSourceReference, string? sourceName, string? conditionName) + { + var routingRef = subscriptionSourceReference ?? ""; + var keyBase = !string.IsNullOrEmpty(sourceName) ? sourceName : routingRef; + var sourceRef = string.IsNullOrEmpty(conditionName) ? keyBase : $"{keyBase}.{conditionName}"; + return (sourceRef, routingRef); + } + /// Clamps an OPC UA severity (1–1000, sometimes out of range) to the unified 0–1000 scale. /// The raw OPC UA severity value to clamp. /// The severity clamped to the range [0, 1000]. diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs index 344cb774..55d8c1fe 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs @@ -494,7 +494,9 @@ public class RealOpcUaClient : IOpcUaClient item.Notification += (_, e) => { if (e.NotificationValue is EventFieldList efl) - HandleAlarmEvent(handle, efl, onTransition); + // sourceNodeId is the binding this feed was subscribed under; every + // transition on the feed is routed under it verbatim (see #17). + HandleAlarmEvent(handle, sourceNodeId, efl, onTransition); }; _subscription.AddItem(item); @@ -724,7 +726,9 @@ public class RealOpcUaClient : IOpcUaClient } } - private void HandleAlarmEvent(string handle, EventFieldList efl, Action onTransition) + private void HandleAlarmEvent( + string handle, string? subscriptionSourceReference, EventFieldList efl, + Action onTransition) { var fields = efl.EventFields; if (fields == null || fields.Count < AlarmStateFields.Length) @@ -746,14 +750,16 @@ public class RealOpcUaClient : IOpcUaClient } // Field layout (AlarmStateFields): [1]=SourceNode (NodeId), [2]=SourceName (string). - // Prefer the human-readable SourceName; fall back to the SourceNode NodeId string - // only when SourceName is absent/empty, so the condition still has a stable key. + // The routing identity is the binding this feed was subscribed under (not the + // event's name), so DataConnectionActor's NodeId-keyed routing matches — see #17 + // and OpcUaAlarmMapper.BuildIdentity. SourceName seeds the readable per-condition + // key; fall back to the SourceNode NodeId string only when it is absent. var sourceName = fields[2].Value as string; if (string.IsNullOrEmpty(sourceName)) sourceName = (fields[1].Value as NodeId)?.ToString() ?? ""; var conditionName = fields.Count > 11 ? fields[11].Value as string : null; - var sourceObjectRef = sourceName; - var sourceRef = string.IsNullOrEmpty(conditionName) ? sourceName : $"{sourceName}.{conditionName}"; + var (sourceRef, sourceObjectRef) = + OpcUaAlarmMapper.BuildIdentity(subscriptionSourceReference, sourceName, conditionName); if (string.IsNullOrEmpty(sourceRef)) return; // not a condition event we can key diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorAlarmTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorAlarmTests.cs index 80647534..031ba22d 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorAlarmTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorAlarmTests.cs @@ -67,6 +67,44 @@ public class DataConnectionActorAlarmTests : TestKit ExpectMsg(u => u.Transition.SourceObjectReference == "Tank01"); } + [Fact] + public void SubscribeAlarms_NodeIdBinding_RoutesTransition() + { + // Gitea #17 end-to-end: an instance binds a native alarm source by NodeId (the form + // the picker / CSV / config store). The adapter builds the transition's identity via + // OpcUaAlarmMapper.BuildIdentity(binding, SourceName, ConditionName). With the old + // "SourceObjectReference = SourceName" logic the event's name ("pymodbus/plc/HR200") + // never prefix-matches the NodeId binding, so this transition was dropped. Now the + // routing identity IS the binding, so it delivers. + const string binding = "nsu=https://zb.com/otopcua/raw;s=pymodbus/plc/HR200"; + + AlarmTransitionCallback? cb = null; + var adapter = Substitute.For(); + adapter.ConnectAsync(Arg.Any>(), Arg.Any()) + .Returns(Task.CompletedTask); + ((IAlarmSubscribableConnection)adapter) + .SubscribeAlarmsAsync(Arg.Any(), Arg.Any(), + Arg.Do(c => cb = c), Arg.Any()) + .Returns(Task.FromResult("alarm-sub-1")); + + var actor = Sys.ActorOf(Props.Create(() => new DataConnectionActor( + "conn", adapter, _options, _health, _factory, "OpcUa"))); + + actor.Tell(new SubscribeAlarmsRequest("c", "inst", "conn", binding, null, DateTimeOffset.UtcNow)); + ExpectMsg(m => m.Success); + + Assert.NotNull(cb); + var (sourceRef, sourceObjectRef) = DataConnectionLayer.Adapters.OpcUaAlarmMapper.BuildIdentity( + subscriptionSourceReference: binding, + sourceName: "pymodbus/plc/HR200", // the event's SourceName (RawPath) — NOT the binding + conditionName: "HR200"); + cb!(Raise(sourceRef, sourceObjectRef)); + + var update = ExpectMsg(); + Assert.Equal(binding, update.Transition.SourceObjectReference); + Assert.Equal("pymodbus/plc/HR200.HR200", update.Transition.SourceReference); + } + [Fact] public void SubscribeAlarms_OnNonAlarmCapableAdapter_RepliesFailure() { diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/OpcUaAlarmMapperTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/OpcUaAlarmMapperTests.cs index 6944399b..2fb4c422 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/OpcUaAlarmMapperTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/OpcUaAlarmMapperTests.cs @@ -6,6 +6,77 @@ namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests; /// Task-11: pure OPC UA A&C field → AlarmConditionState/transition mapping. public class OpcUaAlarmMapperTests { + // ── Gitea #17: routing identity must be the binding, not the event name ── + + [Fact] + public void BuildIdentity_NodeIdBinding_RoutesOnBindingNotName() + { + // OtOpcUa v3 post-#473: SourceName is the RawPath, ConditionName the leaf. The + // binding is a durable node reference. The bug was SourceObjectReference = SourceName, + // which never prefix-matches a NodeId binding, so every transition was dropped. + var (sourceRef, sourceObjectRef) = OpcUaAlarmMapper.BuildIdentity( + subscriptionSourceReference: "nsu=https://zb.com/otopcua/raw;s=pymodbus/plc/HR200", + sourceName: "pymodbus/plc/HR200", + conditionName: "HR200"); + + // Routing identity == the binding, verbatim → DataConnectionActor exact-matches. + Assert.Equal("nsu=https://zb.com/otopcua/raw;s=pymodbus/plc/HR200", sourceObjectRef); + // Per-condition key stays the readable RawPath + condition, kept distinct. + Assert.Equal("pymodbus/plc/HR200.HR200", sourceRef); + } + + [Fact] + public void BuildIdentity_LegacyIndexBinding_RoutesVerbatim() + { + // A binding stored in the old ns= form routes just as well — the point of using + // the binding verbatim is that it is form-agnostic. + var (_, sourceObjectRef) = OpcUaAlarmMapper.BuildIdentity( + subscriptionSourceReference: "ns=2;s=pymodbus/plc/HR200", + sourceName: "pymodbus/plc/HR200", + conditionName: "HR200"); + + Assert.Equal("ns=2;s=pymodbus/plc/HR200", sourceObjectRef); + } + + [Fact] + public void BuildIdentity_AggregateFeed_StaysEmpty() + { + // Empty binding = the Server-object aggregate feed: it must reach only "mirror + // everything" subscribers (StartsWith("") matches all) and never a specific-node + // binding, so the routing identity stays empty while the per-condition key is real. + var (sourceRef, sourceObjectRef) = OpcUaAlarmMapper.BuildIdentity( + subscriptionSourceReference: "", + sourceName: "pymodbus/plc/HR200", + conditionName: "HR200"); + + Assert.Equal("", sourceObjectRef); + Assert.Equal("pymodbus/plc/HR200.HR200", sourceRef); + } + + [Fact] + public void BuildIdentity_NoConditionName_UsesSourceNameAlone() + { + var (sourceRef, _) = OpcUaAlarmMapper.BuildIdentity( + subscriptionSourceReference: "nsu=urn:x;s=Tank01", + sourceName: "Tank01", + conditionName: null); + + Assert.Equal("Tank01", sourceRef); + } + + [Fact] + public void BuildIdentity_NoSourceName_FallsBackToBindingForKey() + { + // A server that leaves SourceName unset still yields a stable, non-empty key. + var (sourceRef, sourceObjectRef) = OpcUaAlarmMapper.BuildIdentity( + subscriptionSourceReference: "ns=2;s=X", + sourceName: null, + conditionName: "Hi"); + + Assert.Equal("ns=2;s=X.Hi", sourceRef); + Assert.Equal("ns=2;s=X", sourceObjectRef); + } + [Fact] public void NormalizeSeverity_ClampsTo0_1000() {