From aba3aa7a8db0c016f482eb49069edfb5e92c0995 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 16:19:27 -0400 Subject: [PATCH] fix(opcua): condition Severity is carried verbatim, not re-bucketed to 5 points OtOpcUaNodeManager.MapSeverity re-bucketed every projected severity onto the SDK's five EventSeverity values (<800 -> 700, else 900), defeating #626's exact OpcUaSeverity: an MTConnect Fault (800) read and evented as 900; a scripted alarm graded 1000 published 900 while /alerts said 1000; a native Low (200) published 300. EventSeverity merely names points on the 1..1000 scale, so the node now carries the value clamped to 1..1000 (ToPart9Severity), at materialise, projection and in the delta gate. MTConnect wire test now asserts Fault ActiveState=true/Severity=800 and the Normal clear ActiveState=false/Retain=false/Severity=800. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/ScriptedAlarms.md | 27 ++++------- .../OtOpcUaNodeManager.cs | 46 ++++++++++--------- .../MTConnectConditionAlarmWireTests.cs | 8 ++++ .../SdkAddressSpaceSinkTests.cs | 4 +- 4 files changed, 45 insertions(+), 40 deletions(-) diff --git a/docs/ScriptedAlarms.md b/docs/ScriptedAlarms.md index eeebc949..18d257f8 100644 --- a/docs/ScriptedAlarms.md +++ b/docs/ScriptedAlarms.md @@ -274,9 +274,10 @@ overwrites an already-authored alarm section. #### Severity mapping The authored `severity` (1–1000) seeds the OPC UA condition node at materialisation -time. Every native transition maps the driver's `AlarmSeverity` to one of four fixed -buckets via `NativeAlarmProjector.MapSeverity`, overriding the authored `severity` seed -from the first transition onward: +time. Every native transition replaces it from the first transition onward. A source with a +finer scale supplies the exact Part 9 severity (`AlarmEventArgs.OpcUaSeverity`, e.g. MTConnect +Fault = 800, #626); otherwise `NativeAlarmProjector.MapSeverity` maps the driver's +`AlarmSeverity` to one of four fixed values: | `AlarmSeverity` enum | Projected value | |---|---| @@ -286,20 +287,12 @@ from the first transition onward: | `Critical` | 900 | | *(default/unspecified)* | 500 | -That projected integer is then mapped to the OPC UA SDK `EventSeverity` enum -by `OtOpcUaNodeManager.MapSeverity` on each `AlarmStateUpdate` write: - -| Projected value range | `EventSeverity` | -|---|---| -| < 200 | `Low` | -| 200 – 399 | `MediumLow` | -| 400 – 599 | `Medium` | -| 600 – 799 | `MediumHigh` | -| ≥ 800 | `High` | - -Consequently the four authored buckets land as: Low→`MediumLow`, Medium→`Medium`, -High→`MediumHigh`, Critical→`High`. The authored `severity` field is overridden by -live driver events on every transition. +The condition node carries that projected integer **verbatim** (clamped to 1–1000), on a direct +Read and on every event. It used to be re-bucketed onto the SDK's five `EventSeverity` points +(< 200 → 100, < 400 → 300, < 600 → 500, < 800 → 700, else 900) by the node manager — so an +MTConnect Fault read and evented as 900, a `Low` native alarm as 300, and a scripted alarm the +engine graded 1000 published 900 while its `/alerts` row said 1000. That re-bucketing is gone +(the `EventSeverity` enum merely names points on the 1–1000 scale; any value is valid). ### Runtime flow diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 4c90c906..925bf37d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -888,7 +888,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 oneShot: state.Shelving == AlarmShelvingKind.OneShot, shelvingTime: state.Shelving == AlarmShelvingKind.Timed ? RemainingShelveMs(state.UnshelveAtUtc) : 0); } - condition.SetSeverity(SystemContext, MapSeverity(state.Severity)); + condition.SetSeverity(SystemContext, ToPart9Severity(state.Severity)); condition.Message.Value = new LocalizedText(state.Message); // #477 — project the source-data quality. A Good→Bad bucket change is a genuine condition @@ -1075,9 +1075,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// condition event AND that an can change. As a record, two /// instances compare by value, so is a plain inequality. /// - /// Severity is stored as the MAPPED bucket (a - /// ) — the same value the node holds after SetSeverity — so two - /// raw severities that fall in the same bucket are correctly treated as "no change". The + /// Severity is stored as the value the node holds after SetSeverity — the exact + /// Part 9 severity clamped to 1..1000 (; it used to be one of five + /// buckets, see there) — so a severity change fires an event. The /// Shelving kind is read back from / mapped to the SDK's shelving state machine so the /// live node and the snapshot compare on the same 3-way (Unshelved/OneShot/Timed) axis. /// @@ -1134,8 +1134,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// Read the gate-relevant slice off the LIVE condition node. Mandatory children /// (Active/Acked/Enabled) are always present; Confirmed/Shelving are optional and null-guarded - /// (a leaner child set ⇒ treat as the unset default). Severity is read as the already-mapped - /// bucket the node stores, and shelving is mapped from the shelving + /// (a leaner child set ⇒ treat as the unset default). Severity is read as the already-clamped + /// value the node stores, and shelving is mapped from the shelving /// state machine's CurrentState so it lines up with . private static AlarmConditionDelta ReadConditionDelta(AlarmConditionState condition) => new( Active: condition.ActiveState?.Id?.Value ?? false, @@ -1150,8 +1150,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 Quality: condition.Quality?.Value ?? StatusCodes.Good); /// Build the gate-relevant slice from the incoming snapshot, normalising the two fields that - /// the node stores in a derived form: Severity is run through so it matches - /// the bucket the node holds (the projection calls SetSeverity(MapSeverity(...))), and an + /// the node stores in a derived form: Severity is run through so it matches + /// the value the node holds (the projection calls SetSeverity(ToPart9Severity(...))), and an /// optional Confirmed/Shelving that the node can't actually hold (missing child) is folded to the /// node's read-back default so it never spuriously registers as a delta. private static AlarmConditionDelta ToConditionDelta(AlarmConditionSnapshot state, AlarmConditionState condition) => new( @@ -1164,7 +1164,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Likewise for shelving: without a ShelvingState child the projection can't apply, so fold to the // node's read-back default (Unshelved). Shelving: condition.ShelvingState is not null ? state.Shelving : AlarmShelvingKind.Unshelved, - MappedSeverity: (ushort)MapSeverity(state.Severity), + MappedSeverity: (ushort)ToPart9Severity(state.Severity), Message: state.Message ?? string.Empty, // If the node has no Quality child, WriteAlarmCondition's projection is a no-op there; fold to the // node's read-back default (Good) so a snapshot Quality can't register a spurious delta. @@ -1200,7 +1200,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// The equipment folder node id the condition parents under (null/unknown ⇒ root). /// Human-readable condition name (BrowseName / DisplayName / Message / ConditionName). /// Domain alarm type — maps to the SDK condition subtype (see remarks). - /// Domain severity (treated as an OPC UA 1..1000 severity); mapped to . + /// Domain severity (an OPC UA 1..1000 severity); written verbatim, clamped to 1..1000. /// True when the condition is a NATIVE (driver-fed, e.g. Galaxy) alarm rather than a scripted one. /// /// AlarmType → SDK subtype mapping. Script-driven alarms have no OPC limit / @@ -1327,7 +1327,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // refused and Acknowledge re-asserts Confirmed — so start it Confirmed, the value NativeAlarmProjector // supplies on every transition, rather than the SDK's default false until the first projection. if (isNative && alarm.ConfirmedState is not null) alarm.SetConfirmedState(SystemContext, true); - alarm.SetSeverity(SystemContext, MapSeverity(severity)); + alarm.SetSeverity(SystemContext, ToPart9Severity(severity)); // Gitea #587 (found by its TimedShelve test): Create() leaves MaxTimeShelved at 0, and the SDK's // TimedShelve handler rejects any ShelvingTime above it — so every client TimedShelve failed with // BadShelvingTimeOutOfRange. The bound becomes the longest shelve the SDK's own shelving timer can @@ -2618,16 +2618,20 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 folder.ClearChangeMasks(SystemContext, includeChildren: false); } - /// Map an integer domain severity (treated as the OPC UA 1..1000 scale) onto the - /// enum buckets the SDK's SetSeverity expects. - private static EventSeverity MapSeverity(int severity) => severity switch - { - < 200 => EventSeverity.Low, - < 400 => EventSeverity.MediumLow, - < 600 => EventSeverity.Medium, - < 800 => EventSeverity.MediumHigh, - _ => EventSeverity.High, - }; + /// + /// The Part 9 Severity (1..1000) a condition carries for a domain severity, which is already on + /// that scale: the value itself, clamped. SetSeverity takes an , but + /// that enum merely names five points on the 1..1000 scale — any value in range is a valid Severity. + /// + /// + /// This used to re-bucket onto those five points (< 800 ⇒ 700, else 900), so the condition + /// node was the one seam that disagreed with every other: an MTConnect Fault (#626's exact 800) read and + /// evented as 900, a scripted alarm the engine graded 1000 published 900 on the node while its + /// /alerts row and historian event said 1000, and a bucket-only native driver's Low (200) became 300. + /// + /// The domain severity. + /// The severity the node carries. + private static EventSeverity ToPart9Severity(int severity) => (EventSeverity)Math.Clamp(severity, 1, 1000); /// /// Ensure a folder node exists at with the given display diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/MTConnectConditionAlarmWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/MTConnectConditionAlarmWireTests.cs index fb249c56..35d6eed5 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/MTConnectConditionAlarmWireTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/MTConnectConditionAlarmWireTests.cs @@ -78,6 +78,9 @@ public sealed class MTConnectConditionAlarmWireTests var fault = events.ConditionEvents(folderItem, FaultMessage).ShouldHaveSingleItem(); fault[RecordedEvents.AckedField].Value.ShouldBe(false, "a fresh fault is unacknowledged"); + fault[RecordedEvents.ActiveField].Value.ShouldBe(true, "a fault is active"); + fault[RecordedEvents.SeverityField].Value.ShouldBe((ushort)800, "Fault is the exact Part 9 severity 800, not the 900 bucket"); + fault[RecordedEvents.RetainField].Value.ShouldBe(true); // An operator acknowledges it (applied server-side, as the Part 9 method does), then a deploy clears // the host's projector prior state. @@ -96,10 +99,15 @@ public sealed class MTConnectConditionAlarmWireTests foreach (var blip in events.ConditionEvents(folderItem, FaultMessage).Skip(2)) { blip[RecordedEvents.AckedField].Value.ShouldBe(true, "an Unavailable blip must not reset the ack"); + blip[RecordedEvents.ActiveField].Value.ShouldBe(true, "an Unavailable blip must not clear the fault"); } agent.Push(Condition("Normal", null, null, 111)); await RecordedEvents.WaitUntilAsync(() => events.Has(folderItem, "SYSTEM Normal")); + var normal = events.ConditionEvents(folderItem, "SYSTEM Normal").ShouldHaveSingleItem(); + normal[RecordedEvents.ActiveField].Value.ShouldBe(false, "Normal clears the condition"); + normal[RecordedEvents.RetainField].Value.ShouldBe(false, "inactive + acknowledged is no longer retained"); + normal[RecordedEvents.SeverityField].Value.ShouldBe((ushort)800, "a clear keeps the severity of the alarm it clears"); lock (routed) routed.ShouldAllBe(id => id == RawAlarmPath, "ConditionId must be the alarm tag's RawPath"); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs index 0e5ae264..2e07da49 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SdkAddressSpaceSinkTests.cs @@ -398,8 +398,8 @@ public sealed class SdkAddressSpaceSinkTests : IDisposable // SetShelvingState(shelved:true, oneShot:false) drives the shelving sub-state machine into a // shelved state; CurrentState is populated (TimedShelved). condition.ShelvingState!.CurrentState.Id.Value.ShouldNotBeNull(); - // Severity 850 → High bucket (>= 800) → EventSeverity.High (the SDK stamps the ushort value). - condition.Severity.Value.ShouldBe((ushort)EventSeverity.High); + // Severity 850 is carried verbatim — not re-bucketed onto EventSeverity.High (900). + condition.Severity.Value.ShouldBe((ushort)850); condition.Message.Value.Text.ShouldBe("Pressure above limit"); condition.Retain.Value.ShouldBeTrue(); // active || !acked ⇒ retain