From 9c8e3da5da20ef3826e5c6f30fe7f4e2622b91f5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 15:38:52 -0400 Subject: [PATCH] fix(mtconnect): review rulings for CONDITION alarms (#626) - Unavailable keeps an ACTIVE condition active: quality-only BadNoCommunication; on data return same statement => quality-only Good, Normal => Clear, different => Retrigger. - A changed alarm reference set (alarm-bearing delta = full rebuild) clears the whole last-told record so every active condition is re-asserted; the same set keeps it. - Quality-only events go through the host's AlarmQualityUpdate path, never a projected snapshot (which could ack a retained unacked condition or rewrite message/severity). - Deterministic tie-break between equal-level conditions (ordinal nativeCode, then type). - Dedup key is level+nativeCode+nativeSeverity+qualifier; a text-only change updates nothing. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../MTConnectConditionAlarmMapper.cs | 140 ++++++++++----- .../MTConnectConditionTracker.cs | 31 ++-- .../MTConnectDriver.Alarms.cs | 36 ++-- .../Drivers/DriverHostActor.cs | 26 ++- .../MTConnectAlarmSourceTests.cs | 51 ++++++ .../MTConnectConditionAlarmMappingTests.cs | 161 ++++++++++++++---- .../DriverHostActorNativeAlarmTests.cs | 49 +++++- 7 files changed, 382 insertions(+), 112 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectConditionAlarmMapper.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectConditionAlarmMapper.cs index 3d2d3a55..fc257264 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectConditionAlarmMapper.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectConditionAlarmMapper.cs @@ -3,10 +3,18 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; +/// +/// What the server was last TOLD about one condition (#626): the last alarm statement it was given +/// (active or Normal — never Unavailable) and whether its Quality is currently annotated Bad. +/// +/// The last statement delivered; null when only an Unavailable was ever seen. +/// Whether the condition's Quality was last annotated BadNoCommunication. +internal sealed record MTConnectToldCondition(MTConnectConditionState? Statement, bool QualityBad); + /// /// The MTConnect CONDITION → OPC UA Part 9 mapping (#626), as one pure function: given what the server was /// last told about a condition and what the Agent says now, the single to -/// raise — or none. +/// raise — or none — and what the server has then been told. /// /// /// @@ -14,22 +22,31 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// Normalinactive (return to normal), quality Good /// Warningactive, severity 500 () /// Faultactive, severity 800 ( bucket) -/// Unavailableinactive, quality BadNoCommunication +/// Unavailablestate UNCHANGED, quality BadNoCommunication /// /// -/// Inactive → active is a ; active → a DIFFERENT active statement -/// (Warning → Fault, a new nativeCode, a changed message) is a -/// — active and unacknowledged again with the new severity -/// and message; active → Normal/Unavailable is a . A change that -/// moves only the quality (Normal ⇄ Unavailable) is a quality-only annotation -/// (). An unchanged statement -/// () emits nothing — which is what keeps a reconnect's -/// replayed /current from re-raising, and so un-acknowledging, an alarm the operator acked. +/// Unavailable is comms loss one hop further out, not a return to normal. A cppagent marks every +/// DataItem of an adapter UNAVAILABLE when the adapter socket drops, so it is treated exactly like the +/// host's own transport loss (#477/#481): an active condition STAYS active (and keeps its ack), only its +/// Quality goes Bad, as a quality-only annotation (, no +/// /alerts row). When data returns: the same statement ⇒ quality-only Good; Normal ⇒ Clear; a +/// different active statement ⇒ Retrigger. /// /// -/// The one deliberate exception is a resync: an unchanged Unavailable is -/// re-asserted on a (re)subscribe, because the host re-annotates every condition Good when the driver -/// reconnects (#477) and nothing else would put the per-condition Bad back. +/// Otherwise: inactive → active is a ; active → a DIFFERENT active +/// statement (Warning → Fault, a new winning nativeCode) is a +/// — active and unacknowledged again with the new severity +/// and message (so clearing the worst of several codes re-raises the next one down, un-acked, by +/// design: it is a statement the operator has not seen); active → Normal is a +/// . An unchanged statement +/// ( — text excluded) emits nothing, which keeps a +/// reconnect's replayed /current from re-raising, and so un-acknowledging, an acked alarm; a +/// text-only change likewise updates nothing, so the message shown stays the statement's first text. +/// +/// +/// Resync. On a (re)subscribe an unchanged Bad quality is re-asserted, because the host +/// re-annotates every condition Good when the driver reconnects (#477) and nothing else would put the +/// per-condition Bad back. /// /// internal static class MTConnectConditionAlarmMapper @@ -47,18 +64,18 @@ internal static class MTConnectConditionAlarmMapper private const uint BadNoCommunication = 0x80310000u; /// - /// The event moving the server from to , or - /// null when nothing needs to be said. + /// The event moving the server from to (or null + /// when nothing needs to be said), and the new told record. /// - /// What the server was last told for this condition; null when nothing yet. + /// What the server was last told for this condition; null when nothing yet. /// The Agent's current statement. /// The condition's ConditionId — the alarm-bearing raw tag's RawPath. /// The Agent DataItem id (carried as ). /// The alarm subscription the event belongs to. - /// True on a (re)subscribe: re-assert an unchanged Unavailable quality. - /// The event to raise, or null. - public static AlarmEventArgs? Transition( - MTConnectConditionState? delivered, + /// True on a (re)subscribe: re-assert an unchanged Bad quality. + /// The event to raise (or null) and what the server has been told afterwards. + public static (AlarmEventArgs? Event, MTConnectToldCondition Told) Transition( + MTConnectToldCondition? told, MTConnectConditionState current, string conditionId, string dataItemId, @@ -67,34 +84,47 @@ internal static class MTConnectConditionAlarmMapper { ArgumentNullException.ThrowIfNull(current); - var wasActive = delivered?.IsActive == true; + var statement = told?.Statement; + var bad = told?.QualityBad ?? false; - if (delivered is not null && delivered.SameAlarmAs(current)) + if (current.Level == MTConnectConditionLevel.Unavailable) { - return resync && current.Level == MTConnectConditionLevel.Unavailable - ? Build(current, delivered, conditionId, dataItemId, handle, AlarmTransitionKind.Unspecified) - : null; + // State untouched; only the quality moves (or is re-asserted on a resync). + var next = new MTConnectToldCondition(statement, QualityBad: true); + return !bad || resync + ? (QualityOnly(statement ?? current, current, conditionId, dataItemId, handle, BadNoCommunication), next) + : (null, next); } + var told2 = new MTConnectToldCondition(current, QualityBad: false); + + if (statement is not null && statement.SameAlarmAs(current)) + { + // Unchanged statement: keep the FIRST text the server was shown. Only a Bad quality is lifted. + var kept = told2 with { Statement = statement }; + return bad + ? (QualityOnly(statement, current, conditionId, dataItemId, handle, Good), kept) + : (null, kept); + } + + var wasActive = statement?.IsActive == true; + if (current.IsActive) { - return Build(current, delivered, conditionId, dataItemId, handle, - wasActive ? AlarmTransitionKind.Retrigger : AlarmTransitionKind.Raise); + return (Build(current, statement, conditionId, dataItemId, handle, + wasActive ? AlarmTransitionKind.Retrigger : AlarmTransitionKind.Raise), told2); } if (wasActive) { - return Build(current, delivered, conditionId, dataItemId, handle, AlarmTransitionKind.Clear); + return (Build(current, statement, conditionId, dataItemId, handle, AlarmTransitionKind.Clear), told2); } - // Inactive → inactive with a different level: only the quality moved. Nothing to say for a first-seen - // Normal — the host's connectivity annotation already confirms a materialised condition Good. - if (delivered is null && current.Level == MTConnectConditionLevel.Normal) - { - return null; - } - - return Build(current, delivered, conditionId, dataItemId, handle, AlarmTransitionKind.Unspecified); + // Normal after nothing / after Normal: nothing to say about the state — the host's connectivity + // annotation confirms a materialised condition Good — except lifting a Bad this driver put there. + return bad + ? (QualityOnly(current, current, conditionId, dataItemId, handle, Good), told2) + : (null, told2); } /// The Part 9 severity of an active level. @@ -124,19 +154,46 @@ internal static class MTConnectConditionAlarmMapper return builder.ToString(); } + /// + /// A quality-only annotation. The host writes ONLY the condition's Quality for it (never message, + /// severity or ack state), so the message/severity carried are informational: those of the statement the + /// server holds. + /// + private static AlarmEventArgs QualityOnly( + MTConnectConditionState shown, + MTConnectConditionState current, + string conditionId, + string dataItemId, + IAlarmSubscriptionHandle handle, + uint quality) + { + var severity = shown.IsActive ? SeverityOf(shown.Level) : WarningSeverity; + + return new AlarmEventArgs( + SubscriptionHandle: handle, + SourceNodeId: dataItemId, + ConditionId: conditionId, + AlarmType: shown.Detail.ConditionType ?? "CONDITION", + Message: MessageOf(shown), + Severity: severity >= FaultSeverity ? AlarmSeverity.High : AlarmSeverity.Medium, + SourceTimestampUtc: current.TimestampUtc, + Kind: AlarmTransitionKind.Unspecified, + OpcUaSeverity: severity, + QualityStatusCode: quality); + } + private static AlarmEventArgs Build( MTConnectConditionState current, - MTConnectConditionState? delivered, + MTConnectConditionState? previous, string conditionId, string dataItemId, IAlarmSubscriptionHandle handle, AlarmTransitionKind kind) { - // An inactive statement keeps the severity of the alarm it clears (or the one last shown), so a clear - // does not visibly re-grade the condition; a never-active condition reports Warning's 500. + // A clear keeps the severity of the alarm it clears, so it does not visibly re-grade the condition. var severity = current.IsActive ? SeverityOf(current.Level) - : delivered?.IsActive == true ? SeverityOf(delivered.Level) : WarningSeverity; + : previous?.IsActive == true ? SeverityOf(previous.Level) : WarningSeverity; return new AlarmEventArgs( SubscriptionHandle: handle, @@ -148,6 +205,7 @@ internal static class MTConnectConditionAlarmMapper SourceTimestampUtc: current.TimestampUtc, Kind: kind, OpcUaSeverity: severity, - QualityStatusCode: current.Level == MTConnectConditionLevel.Unavailable ? BadNoCommunication : Good); + // A real transition only ever happens on live data, so it also lifts any Bad annotation. + QualityStatusCode: Good); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectConditionTracker.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectConditionTracker.cs index 52bc2d04..ffa18d81 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectConditionTracker.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectConditionTracker.cs @@ -38,11 +38,13 @@ internal sealed record MTConnectConditionState( /// /// Whether two states are the SAME alarm statement — the dedup key the alarm path diffs on. Two active - /// states match only when level, nativeCode, nativeSeverity, qualifier and message - /// text all agree (a new code, an escalation, or a changed message is a new statement); two inactive - /// states match on the level alone, so a re-reported Normal never re-clears. The timestamp and the - /// active count are deliberately NOT part of it: a replayed /current after a reconnect carries the - /// same condition under a new sequence, and re-raising it would un-acknowledge an operator's ack. + /// states match when level, nativeCode, nativeSeverity and qualifier agree (a new + /// code or an escalation is a new statement); two inactive states match on the level alone, so a + /// re-reported Normal never re-clears. NOT part of it: the timestamp and the active count (a replayed + /// /current after a reconnect carries the same condition under a new sequence, and re-raising it + /// would un-acknowledge an operator's ack) and the message TEXT (an adapter that embeds a live value in + /// the text, e.g. "Spindle temp 82C", would otherwise re-raise and un-ack on every tick — the message + /// shown stays the statement's first text). /// /// The state to compare against. /// True when the alarm path must emit nothing to move from one to the other. @@ -53,8 +55,7 @@ internal sealed record MTConnectConditionState( return string.Equals(Detail.NativeCode, other.Detail.NativeCode, StringComparison.Ordinal) && string.Equals(Detail.NativeSeverity, other.Detail.NativeSeverity, StringComparison.Ordinal) - && string.Equals(Detail.Qualifier, other.Detail.Qualifier, StringComparison.Ordinal) - && string.Equals(Detail.Text, other.Detail.Text, StringComparison.Ordinal); + && string.Equals(Detail.Qualifier, other.Detail.Qualifier, StringComparison.Ordinal); } } @@ -75,7 +76,8 @@ internal sealed record MTConnectConditionState( /// Rules, per DataItem, applied in document order. Warning/Fault activates (or /// replaces) the entry keyed by its nativeCode (blank = one unnamed entry). Normal with a /// nativeCode clears that code; Normal without one clears every code. Unavailable -/// clears every code and marks the item unavailable until any other level arrives. A baseline +/// clears every code and marks the item unavailable until any other level arrives (the alarm path does +/// NOT treat that as a clear — see ). A baseline /// (/current, which lists every currently-active condition) first resets each DataItem it /// mentions, so an entry cleared while the driver was not listening cannot survive a re-baseline. An /// unrecognised (vendor) state word is treated as an active Warning — surfacing an unknown @@ -92,7 +94,6 @@ internal sealed class MTConnectConditionTracker private readonly Lock _lock = new(); private readonly Dictionary _items = new(StringComparer.Ordinal); - private long _order; /// /// Folds every CONDITION observation of into the tracked state. @@ -144,11 +145,13 @@ internal sealed class MTConnectConditionTracker if (item.Active.Count > 0) { - // Most severe wins; among equals, the most recently activated — the newest statement the - // operator has not yet seen. + // Most severe wins; among equals, a DETERMINISTIC tie-break (ordinal nativeCode, then type) — + // never document order, which an Agent may present differently on a re-baseline and which would + // then re-raise (and un-ack) an unchanged condition on every reconnect. var winner = item.Active.Values .OrderByDescending(e => e.Level) - .ThenByDescending(e => e.Order) + .ThenBy(e => e.Detail.NativeCode ?? string.Empty, StringComparer.Ordinal) + .ThenBy(e => e.Detail.ConditionType ?? string.Empty, StringComparer.Ordinal) .First(); return new MTConnectConditionState(winner.Level, winner.Detail, winner.TimestampUtc, item.Active.Count); @@ -185,7 +188,7 @@ internal sealed class MTConnectConditionTracker private void ApplyOne(ItemState item, MTConnectObservation observation) { var detail = observation.Condition!; - var entry = new Entry(LevelOf(observation.Value), detail, observation.TimestampUtc, ++_order); + var entry = new Entry(LevelOf(observation.Value), detail, observation.TimestampUtc); switch (entry.Level) { @@ -209,7 +212,7 @@ internal sealed class MTConnectConditionTracker } } - private sealed record Entry(MTConnectConditionLevel Level, MTConnectConditionDetail Detail, DateTime TimestampUtc, long Order); + private sealed record Entry(MTConnectConditionLevel Level, MTConnectConditionDetail Detail, DateTime TimestampUtc); private sealed class ItemState { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.Alarms.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.Alarms.cs index c21970af..6c82d568 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.Alarms.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.Alarms.cs @@ -48,11 +48,19 @@ public sealed partial class MTConnectDriver : IAlarmSource private readonly Lock _alarmLock = new(); /// - /// ConditionId (RawPath) → the state the server was last told. Guarded by . - /// Survives re-subscribes and lifecycle restarts on purpose — it describes the SERVER's condition node, - /// which outlives both. + /// ConditionId (RawPath) → what the server was last told. Guarded by . Survives a + /// re-subscribe with the SAME reference set (a reconnect) and lifecycle restarts — the server's condition + /// nodes outlive both, and re-raising there would un-acknowledge. Cleared WHOLE when the subscribed set + /// changes: only an alarm-bearing raw-tag delta changes it, that delta is a full address-space rebuild, + /// and a rebuild recreates every condition node inactive — so everything must be told again. /// - private readonly Dictionary _deliveredConditions = new(StringComparer.Ordinal); + private readonly Dictionary _deliveredConditions = new(StringComparer.Ordinal); + + /// + /// The reference set of the last , kept across an unsubscribe so a + /// release-then-resubscribe of an identical set is still recognised. Guarded by . + /// + private HashSet? _lastAlarmReferences; /// The CONDITION tracker for the current session; replaced alongside the observation index. private MTConnectConditionTracker _conditions = new(); @@ -100,12 +108,16 @@ public sealed partial class MTConnectDriver : IAlarmSource lock (_alarmLock) { - // A condition that left the set has its node torn down by the server; if it comes back it is a NEW - // node that has been told nothing, so forget what its predecessor was told. - foreach (var stale in _deliveredConditions.Keys.Where(k => !references.Contains(k)).ToList()) + // A CHANGED set means an alarm-bearing delta, which the server applies as a full rebuild: every + // condition node — including ones still in the set — was recreated inactive and has been told + // nothing. Re-tell all of them. The SAME set is a reconnect: keep the record, or every still-active + // alarm would be re-raised and lose its acknowledgement. + if (_lastAlarmReferences is null || !_lastAlarmReferences.SetEquals(references)) { - _deliveredConditions.Remove(stale); + _deliveredConditions.Clear(); } + + _lastAlarmReferences = references; } EmitConditionTransitions(touchedDataItemIds: null, resync: true); @@ -192,10 +204,10 @@ public sealed partial class MTConnectDriver : IAlarmSource var current = tracker.Get(dataItemId); if (current is null) continue; - _deliveredConditions.TryGetValue(conditionId, out var delivered); - var evt = MTConnectConditionAlarmMapper.Transition( - delivered, current, conditionId, dataItemId, subscription.Handle, resync); - _deliveredConditions[conditionId] = current; + _deliveredConditions.TryGetValue(conditionId, out var told); + var (evt, nowTold) = MTConnectConditionAlarmMapper.Transition( + told, current, conditionId, dataItemId, subscription.Handle, resync); + _deliveredConditions[conditionId] = nowTold; if (evt is null) continue; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index ac0cfbdd..33eafca8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -1789,6 +1789,25 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS _localNode, msg.DriverInstanceId, msg.Args.ConditionId); return; } + + // #626 — a QUALITY-ONLY annotation (an MTConnect condition gone Unavailable, or back) is not an alarm + // transition. It rides the SAME quality-only node path the #477 connectivity annotation uses + // (AlarmQualityUpdate → WriteAlarmQuality, which touches Quality and nothing else), never a projected + // snapshot: the projector's prior state is cleared on every apply, so a projected "keep the state" + // after a deploy would default to inactive+acked and silently acknowledge a retained, unacked + // condition — and would rewrite its message and severity. Ungated by role (a Secondary keeps quality + // warm, as for connectivity) and no alerts row. + if (msg.Args is { IsQualityOnly: true, QualityStatusCode: { } quality }) + { + foreach (var n in nodeIds) + { + _opcUaPublishActor.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.AlarmQualityUpdate( + n.NodeId, quality, msg.Args.SourceTimestampUtc, n.Realm)); + } + + return; + } + // PRIMARY GATE (archreview 03/S4) — decided ONCE for this transition: the OPC UA condition write // below stays UNGATED (a Secondary keeps its address space warm for failover), but only the Primary // publishes the single cluster-wide alerts copy. Denied on a Secondary/Detached, OR on a multi-driver @@ -1811,11 +1830,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS _opcUaPublishActor.Tell(new ZB.MOM.WW.OtOpcUa.Runtime.OpcUa.OpcUaPublishActor.AlarmStateUpdate( nodeId, snapshot, msg.Args.SourceTimestampUtc, n.Realm)); - // Only the Primary publishes the cluster-wide alerts transition (see the gate above). A quality-only - // annotation (#626: e.g. an MTConnect condition gone Unavailable while already Normal) is not an - // alarm transition at all — the condition write above carries it; an alerts row would read as - // "Activated" for something that never activated. - if (!serviceAlertsAsPrimary || msg.Args.IsQualityOnly) continue; + // Only the Primary publishes the cluster-wide alerts transition (see the gate above). + if (!serviceAlertsAsPrimary) continue; var meta = _alarmMetaByNodeId.TryGetValue(nodeId, out var m) ? m : (EquipmentId: nodeId, Name: nodeId, AlarmType: "AlarmCondition", HistorizeToAveva: (bool?)null, diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAlarmSourceTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAlarmSourceTests.cs index 38f637c3..ccabf8d7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAlarmSourceTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAlarmSourceTests.cs @@ -20,6 +20,7 @@ public sealed class MTConnectAlarmSourceTests private const string ConditionRawPath = "Plant/MT/dev1/SystemCond"; private const string ConditionValueRawPath = "Plant/MT/dev1/SystemCondText"; private const string ConditionId = "dev1_system_cond"; + private const string OtherConditionRawPath = "Plant/MT/dev1/OtherCond"; private const long FixtureInstanceId = 1655000000L; private const long RestartedInstanceId = 1655999999L; @@ -39,6 +40,7 @@ public sealed class MTConnectAlarmSourceTests // must not disturb the existing value surface. new RawTagEntry(ConditionRawPath, $$"""{"address":"{{ConditionId}}"}""", false, "dev1"), new RawTagEntry(ConditionValueRawPath, $$"""{"address":"{{ConditionId}}"}""", false, "dev1"), + new RawTagEntry(OtherConditionRawPath, """{"address":"dev1_other_cond"}""", false, "dev1"), ], }; @@ -125,6 +127,55 @@ public sealed class MTConnectAlarmSourceTests await driver.ShutdownAsync(Ct); } + [Fact] + public async Task An_ack_survives_an_adapter_unavailable_blip() + { + // A cppagent marks an adapter's items UNAVAILABLE when the adapter socket drops. The Fault must stay + // active (no Clear/Raise pair, which would reset the operator's ack); only its quality moves. + var (driver, agent, events) = await AlarmDriverAsync(); + await driver.SubscribeAlarmsAsync([ConditionRawPath], Ct); + + await agent.PumpAsync(Chunk(108, 109, Cond("Fault", "E17", "overtemp"))).WaitAsync(Watchdog, Ct); + await agent.PumpAsync(Chunk(109, 110, Cond("UNAVAILABLE"))).WaitAsync(Watchdog, Ct); + await agent.PumpAsync(Chunk(110, 111, Cond("Fault", "E17", "overtemp"))).WaitAsync(Watchdog, Ct); + + var seen = events.ToArray(); + seen.Select(e => e.Kind).ShouldBe( + [AlarmTransitionKind.Raise, AlarmTransitionKind.Unspecified, AlarmTransitionKind.Unspecified]); + seen[1].IsQualityOnly.ShouldBeTrue(); + seen[1].QualityStatusCode.ShouldBe(0x80310000u); + seen[2].IsQualityOnly.ShouldBeTrue(); + seen[2].QualityStatusCode.ShouldBe(0x00000000u); + + await driver.ShutdownAsync(Ct); + } + + [Fact] + public async Task A_changed_alarm_set_re_tells_every_condition_and_the_same_set_does_not() + { + // Review MAJOR 2: a changed set is an alarm-bearing delta ⇒ full rebuild ⇒ every condition node was + // recreated inactive, so an active condition still in the set must be re-asserted onto it. + var (driver, agent, events) = await AlarmDriverAsync(); + await driver.SubscribeAlarmsAsync([ConditionRawPath], Ct); + await agent.PumpAsync(Chunk(108, 109, Cond("Fault", "E17"))).WaitAsync(Watchdog, Ct); + events.ShouldHaveSingleItem().Kind.ShouldBe(AlarmTransitionKind.Raise); + + await driver.SubscribeAlarmsAsync([ConditionRawPath, OtherConditionRawPath], Ct); + events.Count.ShouldBe(2, "A is re-raised onto its rebuilt node"); + events.Last().ConditionId.ShouldBe(ConditionRawPath); + events.Last().Kind.ShouldBe(AlarmTransitionKind.Raise); + + await driver.SubscribeAlarmsAsync([OtherConditionRawPath, ConditionRawPath], Ct); + + // Positive control: the next real transition is the very next event. + await agent.PumpAsync(Chunk(109, 110, Cond("Normal"))).WaitAsync(Watchdog, Ct); + events.Select(e => e.Kind).ShouldBe( + [AlarmTransitionKind.Raise, AlarmTransitionKind.Raise, AlarmTransitionKind.Clear], + "the same set (a reconnect) must re-raise nothing"); + + await driver.ShutdownAsync(Ct); + } + [Fact] public async Task A_resubscribe_with_an_unchanged_active_state_raises_nothing() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectConditionAlarmMappingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectConditionAlarmMappingTests.cs index 68f698ba..7d3392c2 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectConditionAlarmMappingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectConditionAlarmMappingTests.cs @@ -113,6 +113,21 @@ public sealed class MTConnectConditionAlarmMappingTests MTConnectConditionTracker.LevelOf("fault").ShouldBe(MTConnectConditionLevel.Fault); } + + [Fact] + public void Equal_level_conditions_tie_break_deterministically_regardless_of_document_order() + { + // Review MINOR 2: an Agent that lists the same two Warnings in a different order on a re-baseline must + // not change the winner, or every reconnect would retrigger (and un-ack) the condition. + var forward = new MTConnectConditionTracker(); + forward.Apply(Doc(Cond("Warning", "B", at: 0), Cond("Warning", "A", at: 1)), baseline: true); + var reverse = new MTConnectConditionTracker(); + reverse.Apply(Doc(Cond("Warning", "A", at: 0), Cond("Warning", "B", at: 1)), baseline: true); + + forward.Get(Id).ShouldNotBeNull().Detail.NativeCode.ShouldBe("A"); + reverse.Get(Id).ShouldNotBeNull().Detail.NativeCode.ShouldBe("A"); + } + // ---- mapper: state mapping ---- [Fact] @@ -154,15 +169,14 @@ public sealed class MTConnectConditionAlarmMappingTests [Fact] public void Normal_then_fault_then_normal_is_raise_then_clear() { - var normal = State(MTConnectConditionLevel.Normal); - var fault = State(MTConnectConditionLevel.Fault, "overtemp", "E17"); + var events = Sequence( + State(MTConnectConditionLevel.Normal), + State(MTConnectConditionLevel.Fault, "overtemp", "E17"), + State(MTConnectConditionLevel.Normal)); - Map(null, normal).ShouldBeNull("a first-seen Normal has nothing to say — the node starts inactive"); - - var raise = Map(normal, fault).ShouldNotBeNull(); - raise.Kind.ShouldBe(AlarmTransitionKind.Raise); - - var clear = Map(fault, normal).ShouldNotBeNull(); + events[0].ShouldBeNull("a first-seen Normal has nothing to say — the node starts inactive"); + events[1].ShouldNotBeNull().Kind.ShouldBe(AlarmTransitionKind.Raise); + var clear = events[2].ShouldNotBeNull(); clear.Kind.ShouldBe(AlarmTransitionKind.Clear); clear.QualityStatusCode.ShouldBe(Good); clear.OpcUaSeverity.ShouldBe(800, "a clear keeps the severity of the alarm it clears"); @@ -171,11 +185,11 @@ public sealed class MTConnectConditionAlarmMappingTests [Fact] public void Warning_escalating_to_fault_is_a_retrigger_at_the_new_severity() { - var warning = State(MTConnectConditionLevel.Warning, "coolant low", "W1"); - var fault = State(MTConnectConditionLevel.Fault, "coolant empty", "F1"); - - var evt = Map(warning, fault).ShouldNotBeNull(); + var events = Sequence( + State(MTConnectConditionLevel.Warning, "coolant low", "W1"), + State(MTConnectConditionLevel.Fault, "coolant empty", "F1")); + var evt = events[1].ShouldNotBeNull(); evt.Kind.ShouldBe(AlarmTransitionKind.Retrigger); evt.OpcUaSeverity.ShouldBe(800); evt.Message.ShouldStartWith("coolant empty"); @@ -184,10 +198,9 @@ public sealed class MTConnectConditionAlarmMappingTests [Fact] public void A_different_active_native_code_at_the_same_level_is_a_retrigger() { - var evt = Map(State(MTConnectConditionLevel.Fault, "a", "A"), State(MTConnectConditionLevel.Fault, "b", "B")) - .ShouldNotBeNull(); - - evt.Kind.ShouldBe(AlarmTransitionKind.Retrigger); + // Includes the documented "clearing the worst code re-raises the next one down, un-acked" case. + Sequence(State(MTConnectConditionLevel.Fault, "a", "A"), State(MTConnectConditionLevel.Fault, "b", "B"))[1] + .ShouldNotBeNull().Kind.ShouldBe(AlarmTransitionKind.Retrigger); } [Fact] @@ -202,64 +215,140 @@ public sealed class MTConnectConditionAlarmMappingTests Map(before, replayed, resync: true).ShouldBeNull(); } + [Fact] + public void A_text_only_change_updates_nothing_and_the_first_text_is_kept() + { + // Review MINOR 4 / ruling 5: an adapter embedding a live value in the text must not re-raise (un-ack). + var first = State(MTConnectConditionLevel.Fault, "Spindle temp 82C", "E17"); + var (evt, told) = MTConnectConditionAlarmMapper.Transition( + new MTConnectToldCondition(first, false), State(MTConnectConditionLevel.Fault, "Spindle temp 83C", "E17"), + RawPath, Id, Handle, resync: false); + + evt.ShouldBeNull(); + told.Statement.ShouldNotBeNull().Detail.Text.ShouldBe("Spindle temp 82C"); + } + [Fact] public void A_repeated_normal_with_different_text_does_not_re_clear() { Map(State(MTConnectConditionLevel.Normal, "ok"), State(MTConnectConditionLevel.Normal, "still ok")).ShouldBeNull(); } + // ---- mapper: Unavailable (comms loss one hop out) ---- + [Fact] - public void An_active_condition_going_unavailable_clears_with_bad_quality() + public void An_active_condition_going_unavailable_stays_active_with_a_bad_quality_annotation() { var evt = Map(State(MTConnectConditionLevel.Fault, "x", "E1"), State(MTConnectConditionLevel.Unavailable)) .ShouldNotBeNull(); - evt.Kind.ShouldBe(AlarmTransitionKind.Clear); + evt.IsQualityOnly.ShouldBeTrue("not a Clear: the state is unknown, not normal (#477/#481 posture)"); evt.QualityStatusCode.ShouldBe(BadNoCommunication); - evt.Message.ShouldBe("SYSTEM UNAVAILABLE"); + evt.Message.ShouldBe("x [nativeCode=E1]", "the annotation describes the statement the server still holds"); + } + + [Fact] + public void An_ack_survives_an_unavailable_blip_when_the_same_fault_returns() + { + // Fault → UNAVAILABLE → the same Fault: two quality-only annotations, no Clear, no Raise — so the + // operator's acknowledgement is never reset. + var events = Sequence( + State(MTConnectConditionLevel.Fault, "x", "E1"), + State(MTConnectConditionLevel.Unavailable), + State(MTConnectConditionLevel.Fault, "x", "E1")); + + events[0].ShouldNotBeNull().Kind.ShouldBe(AlarmTransitionKind.Raise); + events[1].ShouldNotBeNull().IsQualityOnly.ShouldBeTrue(); + events[1]!.QualityStatusCode.ShouldBe(BadNoCommunication); + events[2].ShouldNotBeNull().IsQualityOnly.ShouldBeTrue(); + events[2]!.QualityStatusCode.ShouldBe(Good); + } + + [Fact] + public void An_active_condition_returning_normal_from_unavailable_is_a_clear() + { + var events = Sequence( + State(MTConnectConditionLevel.Fault, "x", "E1"), + State(MTConnectConditionLevel.Unavailable), + State(MTConnectConditionLevel.Normal)); + + var clear = events[2].ShouldNotBeNull(); + clear.Kind.ShouldBe(AlarmTransitionKind.Clear); + clear.QualityStatusCode.ShouldBe(Good); + } + + [Fact] + public void An_active_condition_returning_with_a_different_statement_is_a_retrigger() + { + var events = Sequence( + State(MTConnectConditionLevel.Warning, "w", "W1"), + State(MTConnectConditionLevel.Unavailable), + State(MTConnectConditionLevel.Fault, "f", "F1")); + + var evt = events[2].ShouldNotBeNull(); + evt.Kind.ShouldBe(AlarmTransitionKind.Retrigger); + evt.QualityStatusCode.ShouldBe(Good); + evt.OpcUaSeverity.ShouldBe(800); } [Fact] public void An_inactive_condition_going_unavailable_and_back_is_quality_only() { - var normal = State(MTConnectConditionLevel.Normal); - var unavailable = State(MTConnectConditionLevel.Unavailable); + var events = Sequence( + State(MTConnectConditionLevel.Normal), + State(MTConnectConditionLevel.Unavailable), + State(MTConnectConditionLevel.Normal)); - var lost = Map(normal, unavailable).ShouldNotBeNull(); - lost.IsQualityOnly.ShouldBeTrue(); - lost.QualityStatusCode.ShouldBe(BadNoCommunication); + events[1].ShouldNotBeNull().IsQualityOnly.ShouldBeTrue(); + events[1]!.QualityStatusCode.ShouldBe(BadNoCommunication); + events[2].ShouldNotBeNull().IsQualityOnly.ShouldBeTrue(); + events[2]!.QualityStatusCode.ShouldBe(Good); - var back = Map(unavailable, normal).ShouldNotBeNull(); - back.IsQualityOnly.ShouldBeTrue(); - back.QualityStatusCode.ShouldBe(Good); - - Map(null, unavailable).ShouldNotBeNull().IsQualityOnly.ShouldBeTrue("first-seen Unavailable must reach the node"); + Map(null, State(MTConnectConditionLevel.Unavailable)).ShouldNotBeNull().IsQualityOnly + .ShouldBeTrue("first-seen Unavailable must reach the node"); } [Fact] public void An_unchanged_unavailable_is_reasserted_only_on_a_resync() { // The host re-annotates every condition Good on reconnect (#477); only the resync puts the Bad back. + var told = new MTConnectToldCondition(State(MTConnectConditionLevel.Fault, "x", "E1"), QualityBad: true); var unavailable = State(MTConnectConditionLevel.Unavailable); - Map(unavailable, unavailable).ShouldBeNull(); - Map(unavailable, unavailable, resync: true).ShouldNotBeNull().IsQualityOnly.ShouldBeTrue(); + MTConnectConditionAlarmMapper.Transition(told, unavailable, RawPath, Id, Handle, resync: false).Event.ShouldBeNull(); + MTConnectConditionAlarmMapper.Transition(told, unavailable, RawPath, Id, Handle, resync: true).Event + .ShouldNotBeNull().IsQualityOnly.ShouldBeTrue(); } [Fact] - public void Unavailable_recovering_straight_to_fault_raises_with_good_quality() + public void A_first_seen_unavailable_recovering_straight_to_fault_raises_with_good_quality() { - var evt = Map(State(MTConnectConditionLevel.Unavailable), State(MTConnectConditionLevel.Fault, "x", "E1")) - .ShouldNotBeNull(); + var events = Sequence(State(MTConnectConditionLevel.Unavailable), State(MTConnectConditionLevel.Fault, "x", "E1")); + var evt = events[1].ShouldNotBeNull(); evt.Kind.ShouldBe(AlarmTransitionKind.Raise); evt.QualityStatusCode.ShouldBe(Good); } // ---- helpers ---- - private static AlarmEventArgs? Map(MTConnectConditionState? delivered, MTConnectConditionState current, bool resync = false) => - MTConnectConditionAlarmMapper.Transition(delivered, current, RawPath, Id, Handle, resync); + /// Maps one step from a told state of (Good quality). + private static AlarmEventArgs? Map(MTConnectConditionState? statement, MTConnectConditionState current, bool resync = false) => + MTConnectConditionAlarmMapper.Transition( + statement is null ? null : new MTConnectToldCondition(statement, false), current, RawPath, Id, Handle, resync).Event; + + /// Threads the told record through a sequence of Agent statements, returning each step's event. + private static AlarmEventArgs?[] Sequence(params MTConnectConditionState[] states) + { + MTConnectToldCondition? told = null; + var events = new AlarmEventArgs?[states.Length]; + for (var i = 0; i < states.Length; i++) + { + (events[i], told) = MTConnectConditionAlarmMapper.Transition(told, states[i], RawPath, Id, Handle, resync: false); + } + + return events; + } private static MTConnectConditionState State( MTConnectConditionLevel level, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs index 0a99914d..848ee47d 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmTests.cs @@ -273,7 +273,8 @@ public sealed class DriverHostActorNativeAlarmTests : RuntimeActorTestBase /// Gitea #626 — a QUALITY-ONLY native event (an kind /// carrying , e.g. an MTConnect condition gone Unavailable while - /// already Normal) re-annotates the condition's Quality through the ordinary state update, and publishes NO + /// already Normal) re-annotates the condition's Quality through the quality-only node path + /// ( — NOT a projected snapshot), and publishes NO /// alerts row — it is not an alarm transition. The positive control is the Raise that follows: it /// MUST publish, and it must be the FIRST alerts message, so the absence is proven rather than timed. [Fact] @@ -298,10 +299,10 @@ public sealed class DriverHostActorNativeAlarmTests : RuntimeActorTestBase Kind: AlarmTransitionKind.Unspecified, QualityStatusCode: OpcUaStatus.BadNoCommunication))); - var annotated = publish.ExpectMsg(TimeSpan.FromSeconds(5)); + var annotated = publish.ExpectMsg(TimeSpan.FromSeconds(5)); annotated.AlarmNodeId.ShouldBe(AlarmRawPath); - annotated.State.Active.ShouldBeFalse(); - annotated.State.StatusCode.ShouldBe(OpcUaStatus.BadNoCommunication); + annotated.Realm.ShouldBe(AddressSpaceRealm.Raw); + annotated.StatusCode.ShouldBe(OpcUaStatus.BadNoCommunication); // Positive control: a real transition after it DOES publish, and is the first thing on the topic. actor.Tell(new DriverInstanceActor.AttributeAlarmPublished("drv-1", new AlarmEventArgs( @@ -327,6 +328,46 @@ public sealed class DriverHostActorNativeAlarmTests : RuntimeActorTestBase evt.Severity.ShouldBe(800); } + /// Gitea #626 (review MINOR 1) — after a deploy (which clears the projector's prior state), a + /// quality-only event on a raised, UNACKNOWLEDGED condition must not reach the node as a snapshot: a projected + /// "keep the state" would default to inactive+acked, silently acknowledging it and rewriting its message and + /// severity. Only an (which writes Quality alone) may be + /// sent. Proven by order: the next message after it is the positive control's state update. + [Fact] + public void Quality_only_event_never_sends_a_state_snapshot_that_could_ack_or_rewrite() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedV3AlarmDeployment(db, RevA, Driver: "drv-1", Tag: "temp_hi"); + var (actor, publish) = SpawnHostAndApply(db, deploymentId); + + actor.Tell(new DriverInstanceActor.AttributeAlarmPublished("drv-1", new AlarmEventArgs( + new StubAlarmHandle(), "Temp", AlarmRawPath, "SYSTEM", "spindle fault", AlarmSeverity.High, Ts, + Kind: AlarmTransitionKind.Raise, OpcUaSeverity: 800, QualityStatusCode: OpcUaStatus.Good))); + publish.ExpectMsg(TimeSpan.FromSeconds(5)).State.Acknowledged.ShouldBeFalse(); + + // Whatever the projector's prior state is (any apply clears it), a quality-only event must never be + // projected — the routing below is the whole guard, so no deploy needs staging here. + + actor.Tell(new DriverInstanceActor.AttributeAlarmPublished("drv-1", new AlarmEventArgs( + new StubAlarmHandle(), "Temp", AlarmRawPath, "SYSTEM", "spindle fault", AlarmSeverity.High, Ts, + Kind: AlarmTransitionKind.Unspecified, OpcUaSeverity: 800, + QualityStatusCode: OpcUaStatus.BadNoCommunication))); + // Positive control. + actor.Tell(new DriverInstanceActor.AttributeAlarmPublished("drv-1", new AlarmEventArgs( + new StubAlarmHandle(), "Temp", AlarmRawPath, "SYSTEM", "control", AlarmSeverity.High, Ts, + Kind: AlarmTransitionKind.Retrigger, OpcUaSeverity: 800, QualityStatusCode: OpcUaStatus.Good))); + + var next = publish.FishForMessage( + m => m is OpcUaPublishActor.AlarmQualityUpdate or OpcUaPublishActor.AlarmStateUpdate, + TimeSpan.FromSeconds(5)); + next.ShouldBeOfType().StatusCode.ShouldBe(OpcUaStatus.BadNoCommunication); + var control = publish.FishForMessage( + m => m is OpcUaPublishActor.AlarmQualityUpdate or OpcUaPublishActor.AlarmStateUpdate, + TimeSpan.FromSeconds(5)); + control.ShouldBeOfType().State.Message.ShouldBe("control", + "the quality-only event must produce no state snapshot ahead of the control"); + } + /// #477 Layer 2 — a driver annotates the /// Quality of the driver's native conditions via : comms /// lost → Bad, restored → Good, at the raw condition NodeId + realm. No alarm transition is involved — this