fix(alarms): populate ConditionClassId/ConditionClassName on conditions (#475)

MaterialiseAlarmCondition never assigned the mandatory Part 9 ConditionType classification fields, so every condition event — native and scripted — shipped ConditionClassId = NodeId.Null (i=0) and ConditionClassName = empty text. Same mechanism as #473: Create() builds the mandatory children from the type's embedded definition but leaves them unset, and nothing downstream synthesises them (ReportEvent / InstanceStateSnapshot copy children verbatim). An HMI bucketing alarms by condition class dropped every OtOpcUa alarm as unclassified. Report BaseConditionClassType — Part 9's "no condition class modelled" value. This is the honest report: we hold no classification at the materialise seam. Deliberately NOT ProcessConditionClassType (the SDK sample's pick), which would assert a classification we cannot back and would be actively wrong for a Galaxy alarm whose upstream category is Safety/Diagnostics — trading a detectable null for an undetectable lie. Real per-alarm classification needs the driver's AlarmCategory carried to this deploy-time seam (it lives only on the runtime AlarmEventArgs transition today) and is a separate feature. Guards, both observed RED against the pre-fix server: - NativeAlarmEventIdentityFieldDeliveryTests: wire-level, its own select clause (the #473 test's clause mirrors ScadaBridge's exactly and its indices are load-bearing, so it is left untouched). The class fields are declared on ConditionType, not BaseEventType, so they are selected against that type. - NodeManagerAlarmSourceFieldsTests: node-level, native (Raw) + scripted (Uns). Stacked on #473 (PR #474) — merge after it.
2026-07-17 00:49:43 -04:00
parent 7339a4af07
commit e08b6b0e69
4 changed files with 166 additions and 12 deletions
@@ -30,6 +30,7 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests;
 public sealed class NativeAlarmEventIdentityFieldDeliveryTests
 {
    private const string ServerUri = "urn:OtOpcUa.AlarmEventIdentityFields";
+    private const string ConditionClassServerUri = "urn:OtOpcUa.AlarmEventConditionClassFields";

    private const string RawDeviceFolder = "pymodbus/plc";
    private const string RawAlarmPath = "pymodbus/plc/HR200";
@@ -45,6 +46,13 @@ public sealed class NativeAlarmEventIdentityFieldDeliveryTests
    private const int MessageIndex = 4;
    private const int SeverityIndex = 5;

+    // Field indices in BuildConditionClassEventFilter's clause (#475). A SEPARATE clause on purpose: the
+    // ScadaBridge one above is mirrored field-for-field from the real consumer and its indices are load-bearing,
+    // so appending to it would silently shift them. Message is re-selected here only as the collector's filter key.
+    private const int ConditionClassIdIndex = 0;
+    private const int ConditionClassNameIndex = 1;
+    private const int ConditionClassMessageIndex = 2;
+
    /// <summary>A live native condition event delivers a populated EventType / SourceNode / SourceName to a
    /// Server-object subscriber using the standard BaseEventType select clause.</summary>
    [Fact]
@@ -67,7 +75,7 @@ public sealed class NativeAlarmEventIdentityFieldDeliveryTests
            var rawNs = (ushort)session.NamespaceUris.GetIndex(V3NodeIds.RawNamespaceUri);
            rawNs.ShouldBeGreaterThan((ushort)0);

-            var collector = new EventCollector();
+            var collector = new EventCollector(MessageIndex);
            var subscription = new Subscription(session.DefaultSubscription) { PublishingInterval = 100 };
            subscription.FastEventCallback = collector.OnEvents;
            session.AddSubscription(subscription);
@@ -75,7 +83,7 @@ public sealed class NativeAlarmEventIdentityFieldDeliveryTests

            // The collector filters by our unique Message, so the item's ClientHandle is not needed here
            // (unlike the sibling multi-notifier test, which tallies delivery per monitored item).
-            AddEventItem(subscription, ObjectIds.Server);
+            AddEventItem(subscription, ObjectIds.Server, BuildEventFilter());
            await subscription.ApplyChangesAsync(ct);

            sink.WriteAlarmCondition(RawAlarmPath, ActiveSnapshot(), DateTime.UtcNow, AddressSpaceRealm.Raw);
@@ -113,6 +121,57 @@ public sealed class NativeAlarmEventIdentityFieldDeliveryTests
        }
    }

+    /// <summary>Issue #475 — a live condition event delivers a populated ConditionClassId / ConditionClassName.
+    /// Both previously arrived unset (NodeId.Null / empty text) via the same mechanism as the #473 fields, so an
+    /// HMI bucketing alarms by condition class dropped every OtOpcUa alarm into an unclassified bin.</summary>
+    [Fact]
+    public async Task Condition_event_carries_populated_ConditionClass_fields_on_the_wire()
+    {
+        var pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-alarm-condclass-{Guid.NewGuid():N}");
+        var port = AllocateFreePort();
+        var ct = TestContext.Current.CancellationToken;
+        try
+        {
+            var (server, host) = await BootServerAsync(port, pkiRoot + "-srv", ConditionClassServerUri, ct);
+            await using var _ = host;
+
+            var sink = new SdkAddressSpaceSink(server.NodeManager!);
+            WireCondition(sink);
+
+            using var session = await OpenSessionAsync($"opc.tcp://127.0.0.1:{port}/OtOpcUa", ct);
+
+            var collector = new EventCollector(ConditionClassMessageIndex);
+            var subscription = new Subscription(session.DefaultSubscription) { PublishingInterval = 100 };
+            subscription.FastEventCallback = collector.OnEvents;
+            session.AddSubscription(subscription);
+            await subscription.CreateAsync(ct);
+
+            AddEventItem(subscription, ObjectIds.Server, BuildConditionClassEventFilter());
+            await subscription.ApplyChangesAsync(ct);
+
+            sink.WriteAlarmCondition(RawAlarmPath, ActiveSnapshot(), DateTime.UtcNow, AddressSpaceRealm.Raw);
+
+            await WaitUntilAsync(() => collector.Fields.Count >= 1, TimeSpan.FromSeconds(5));
+            var fields = collector.Fields.ShouldHaveSingleItem();
+
+            // ConditionClassId — a resolvable class NodeId, NOT NodeId.Null. We model no condition classes, so
+            // Part 9's "no class modelled" value (BaseConditionClassType) is the conformant report.
+            fields[ConditionClassIdIndex].Value.ShouldBe(ObjectTypeIds.BaseConditionClassType,
+                "ConditionClassId must carry a resolvable condition class, not null");
+
+            // ConditionClassName — the matching human-readable name, NOT empty text.
+            var className = fields[ConditionClassNameIndex].Value.ShouldBeOfType<LocalizedText>();
+            className.Text.ShouldBe("BaseConditionClass",
+                "ConditionClassName must name the reported condition class, not be empty");
+
+            await subscription.DeleteAsync(true, ct);
+        }
+        finally
+        {
+            SafeDelete(pkiRoot + "-srv");
+        }
+    }
+
    /// <summary>Materialise the raw device folder + the native condition at its RawPath, plus one referencing
    /// equipment folder wired as a notifier (the production topology).</summary>
    private static void WireCondition(SdkAddressSpaceSink sink)
@@ -127,13 +186,13 @@ public sealed class NativeAlarmEventIdentityFieldDeliveryTests
        new(Active: true, Acknowledged: false, Confirmed: true, Enabled: true,
            Shelving: AlarmShelvingKind.Unshelved, Severity: 700, Message: AlarmMessage);

-    private static MonitoredItem AddEventItem(Subscription subscription, NodeId source)
+    private static MonitoredItem AddEventItem(Subscription subscription, NodeId source, EventFilter filter)
    {
        var item = new MonitoredItem(subscription.DefaultItem)
        {
            StartNodeId = source,
            AttributeId = Attributes.EventNotifier,
-            Filter = BuildEventFilter(),
+            Filter = filter,
            QueueSize = 100,
            SamplingInterval = 0,
        };
@@ -154,9 +213,23 @@ public sealed class NativeAlarmEventIdentityFieldDeliveryTests
        return filter;
    }

+    /// <summary>#475's clause: [ConditionClassId, ConditionClassName, Message]. The two class fields are declared on
+    /// <c>ConditionType</c>, not BaseEventType, so they must be selected against that type or the server returns no
+    /// value for them regardless of the fix. Message rides along solely as the collector's filter key.</summary>
+    private static EventFilter BuildConditionClassEventFilter()
+    {
+        var filter = new EventFilter();
+        filter.AddSelectClause(ObjectTypes.ConditionType, BrowseNames.ConditionClassId);
+        filter.AddSelectClause(ObjectTypes.ConditionType, BrowseNames.ConditionClassName);
+        filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.Message);
+        return filter;
+    }
+
    /// <summary>Captures the delivered event field lists, filtered to our unique alarm Message so unrelated
    /// server events / refresh brackets never count.</summary>
-    private sealed class EventCollector
+    /// <param name="messageIndex">Position of the Message field in the select clause this collector is paired
+    /// with — the two clauses here place it differently, so it cannot be a shared constant.</param>
+    private sealed class EventCollector(int messageIndex)
    {
        private readonly object _gate = new();
        private readonly List<VariantCollection> _fields = new();
@@ -172,8 +245,8 @@ public sealed class NativeAlarmEventIdentityFieldDeliveryTests
            {
                foreach (var e in notification.Events)
                {
-                    if (e.EventFields.Count > MessageIndex &&
-                        e.EventFields[MessageIndex].Value is LocalizedText lt &&
+                    if (e.EventFields.Count > messageIndex &&
+                        e.EventFields[messageIndex].Value is LocalizedText lt &&
                        lt.Text == AlarmMessage)
                    {
                        _fields.Add(e.EventFields);
@@ -23,6 +23,12 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
 ///         scripted), matching <c>ConditionId</c>. The leaf/display name stays on <c>ConditionName</c>, so
 ///         no information is lost by SourceName carrying the unique id rather than the ambiguous leaf.
 ///     </para>
+///     <para>
+///         Issue #475 — the same mechanism leaves the mandatory <c>ConditionType</c> classification fields
+///         <c>ConditionClassId</c> / <c>ConditionClassName</c> unset. The contract locked in here: a server that
+///         does not model condition classes must still report <c>BaseConditionClassType</c> (Part 9's
+///         "no class modelled" value) rather than null. See <c>docs/AlarmTracking.md</c>.
+///     </para>
 /// </summary>
 public sealed class NodeManagerAlarmSourceFieldsTests : IDisposable
 {
@@ -144,6 +150,52 @@ public sealed class NodeManagerAlarmSourceFieldsTests : IDisposable

    }

+    /// <summary>#475 — a NATIVE condition (Raw realm) carries the mandatory ConditionType classification fields.
+    /// We do not model condition classes, so Part 9's "no class modelled" value — BaseConditionClassType — is the
+    /// conformant answer; the point is that it is a resolvable class NodeId a client can bucket on rather than a
+    /// null that drops the alarm into an unclassified bin.</summary>
+    [Trait("Category", "Unit")]
+    [Fact]
+    public async Task Native_condition_carries_ConditionClassId_and_ConditionClassName()
+    {
+        await using var host = await BootAsync();
+        var nm = host.Nm;
+
+        nm.EnsureFolder(RawDeviceFolder, parentNodeId: null, displayName: "dev1", realm: AddressSpaceRealm.Raw);
+        nm.MaterialiseAlarmCondition(RawAlarmPath, RawDeviceFolder, "HR200", "OffNormalAlarm", 700,
+            realm: AddressSpaceRealm.Raw, isNative: true);
+
+        var condition = nm.TryGetAlarmCondition(RawAlarmPath, AddressSpaceRealm.Raw);
+        condition.ShouldNotBeNull();
+
+        condition.ConditionClassId.ShouldNotBeNull();
+        condition.ConditionClassId.Value.ShouldBe(ObjectTypeIds.BaseConditionClassType);
+
+        condition.ConditionClassName.ShouldNotBeNull();
+        condition.ConditionClassName.Value.ShouldNotBeNull();
+        condition.ConditionClassName.Value.Text.ShouldBe("BaseConditionClass");
+    }
+
+    /// <summary>#475 — a SCRIPTED condition (UNS realm) carries the SAME classification fields: native and scripted
+    /// share the materialise path, so neither may regress independently.</summary>
+    [Trait("Category", "Unit")]
+    [Fact]
+    public async Task Scripted_condition_carries_ConditionClassId_and_ConditionClassName()
+    {
+        await using var host = await BootAsync();
+        var nm = host.Nm;
+
+        nm.EnsureFolder("eq-4", parentNodeId: null, displayName: "Station 4", realm: AddressSpaceRealm.Uns);
+        nm.MaterialiseAlarmCondition("tank-dry", "eq-4", "Tank Dry", "OffNormalAlarm", 700,
+            realm: AddressSpaceRealm.Uns, isNative: false);
+
+        var condition = nm.TryGetAlarmCondition("tank-dry", AddressSpaceRealm.Uns);
+        condition.ShouldNotBeNull();
+
+        condition.ConditionClassId!.Value.ShouldBe(ObjectTypeIds.BaseConditionClassType);
+        condition.ConditionClassName!.Value.Text.ShouldBe("BaseConditionClass");
+    }
+
    /// <summary>A booted server + its node manager, disposed via <c>await using</c> so an assertion failure
    /// cannot leak a live server (and its bound port) into the rest of the test run.</summary>
    private sealed class BootedServer(OpcUaApplicationHost host, OtOpcUaNodeManager nm) : IAsyncDisposable