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
2026-10-08 16:19:27 -04:00
parent 49a470a080
commit aba3aa7a8d
4 changed files with 45 additions and 40 deletions
@@ -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

@@ -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 <see cref="AlarmConditionSnapshot"/> can change. As a record, two
    ///     instances compare by value, so <see cref="ShouldFireConditionEvent"/> is a plain inequality.
    ///     <para>
-    ///         <b>Severity</b> is stored as the MAPPED <see cref="EventSeverity"/> bucket (a
-    ///         <see cref="ushort"/>) — the same value the node holds after <c>SetSeverity</c> — so two
-    ///         raw severities that fall in the same bucket are correctly treated as "no change". The
+    ///         <b>Severity</b> is stored as the value the node holds after <c>SetSeverity</c> — the exact
+    ///         Part 9 severity clamped to 1..1000 (<see cref="ToPart9Severity"/>; it used to be one of five
+    ///         <see cref="EventSeverity"/> buckets, see there) — so a severity change fires an event. The
    ///         <b>Shelving</b> 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.
    ///     </para>
@@ -1134,8 +1134,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2

    /// <summary>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
-    /// <see cref="EventSeverity"/> 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 <see cref="ToConditionDelta"/>.</summary>
    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);

    /// <summary>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 <see cref="MapSeverity"/> so it matches
-    /// the bucket the node holds (the projection calls <c>SetSeverity(MapSeverity(...))</c>), and an
+    /// the node stores in a derived form: Severity is run through <see cref="ToPart9Severity"/> so it matches
+    /// the value the node holds (the projection calls <c>SetSeverity(ToPart9Severity(...))</c>), 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.</summary>
    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
    /// <param name="equipmentNodeId">The equipment folder node id the condition parents under (null/unknown ⇒ root).</param>
    /// <param name="displayName">Human-readable condition name (BrowseName / DisplayName / Message / ConditionName).</param>
    /// <param name="alarmType">Domain alarm type — maps to the SDK condition subtype (see remarks).</param>
-    /// <param name="severity">Domain severity (treated as an OPC UA 1..1000 severity); mapped to <see cref="EventSeverity"/>.</param>
+    /// <param name="severity">Domain severity (an OPC UA 1..1000 severity); written verbatim, clamped to 1..1000.</param>
    /// <param name="isNative">True when the condition is a NATIVE (driver-fed, e.g. Galaxy) alarm rather than a scripted one.</param>
    /// <remarks>
    ///     <para><b>AlarmType → SDK subtype mapping.</b> 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);
    }

-    /// <summary>Map an integer domain severity (treated as the OPC UA 1..1000 scale) onto the
-    /// <see cref="EventSeverity"/> enum buckets the SDK's <c>SetSeverity</c> expects.</summary>
-    private static EventSeverity MapSeverity(int severity) => severity switch
-    {
-        < 200 => EventSeverity.Low,
-        < 400 => EventSeverity.MediumLow,
-        < 600 => EventSeverity.Medium,
-        < 800 => EventSeverity.MediumHigh,
-        _ => EventSeverity.High,
-    };
+    /// <summary>
+    ///     The Part 9 <c>Severity</c> (1..1000) a condition carries for a domain severity, which is already on
+    ///     that scale: the value itself, clamped. <c>SetSeverity</c> takes an <see cref="EventSeverity"/>, but
+    ///     that enum merely names five points on the 1..1000 scale — any value in range is a valid Severity.
+    /// </summary>
+    /// <remarks>
+    ///     This used to re-bucket onto those five points (<c>&lt; 800</c> ⇒ 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
+    ///     <c>/alerts</c> row and historian event said 1000, and a bucket-only native driver's Low (200) became 300.
+    /// </remarks>
+    /// <param name="severity">The domain severity.</param>
+    /// <returns>The severity the node carries.</returns>
+    private static EventSeverity ToPart9Severity(int severity) => (EventSeverity)Math.Clamp(severity, 1, 1000);

    /// <summary>
    ///     Ensure a folder node exists at <paramref name="folderNodeId"/> with the given display
@@ -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");

@@ -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