diff --git a/CLAUDE.md b/CLAUDE.md index 71fe1cd6..c2f6b1db 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1121,12 +1121,14 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa the `alarm` object = ONE condition at the RawPath (either alone is skipped + reported). Overlapping selectors all fire; `OTHER` = what no type selector claims. ⚠️ A failed `cnc_rdalmmsg2` THROWS (it used to return `[]` ⇒ mass clear + un-acked re-raise) — and annotates its conditions `BadNoCommunication` once per outage (quality-only) and - degrades driver health with `LastSuccessfulRead` frozen, so an alarm-only driver trips the #527 watchdog. The told + degrades driver health with `LastSuccessfulRead` frozen, so an alarm-only driver trips the #527 watchdog. ⚠️ A CNC + REFUSAL (non-OK rc, transport fine) is `BadNotSupported` + one Warning and does NOT touch health — else the + watchdog reconnect-loops a production control. Health = worst of the value and alarm paths, computed together. The told record lives on the driver INSTANCE (reconnect / in-place re-init re-raise nothing); a deploy RESPAWNS the driver, and its Raise for a still-active alarm is a host-side re-statement (below). Type names are the 16i-era table only (30i codes unconfirmed, not invented — use `T` there). Read-only; ack server-side. `AlarmProjection.Enabled` is a no-op. E2E: `Runtime.Tests/FocasNativeAlarmEndToEndTests` (incl. a respawn). `docs/drivers/FOCAS.md`. -- **A Raise on an already-active condition is a RE-STATEMENT (#647 review):** `NativeAlarmProjector` keeps its ack and +- **A Raise on an already-active condition with the SAME message is a RE-STATEMENT (#647 review):** `NativeAlarmProjector` keeps its ack and `ForwardNativeAlarm` writes no second "Activated" `/alerts` row — every native driver's told-record dies with its instance, and a deploy respawns the driver. Only `Retrigger` (or inactive → active) un-acks, so a source whose UPSTREAM is authoritative for the ack sends Retrigger for an unacked re-statement: OpcUaClient on first sight of a diff --git a/docs/AlarmTracking.md b/docs/AlarmTracking.md index 65906ac1..294476d1 100644 --- a/docs/AlarmTracking.md +++ b/docs/AlarmTracking.md @@ -57,9 +57,15 @@ same condition is wired as an **event notifier of every referencing equipment fo - **Routing is an exact `ConditionId == RawPath` match** (`DriverHostActor.ForwardNativeAlarm`). A driver that decorates the id has every transition dropped as "no alarm condition" — AbCip did (`"{ref}#active"`, fixed under #641), and so did FOCAS (`"{host}#{Type}:{Number}"`, fixed under #647 — one condition per `ALARM:` tag, see - `drivers/FOCAS.md`). A `Raise` on a condition already projected Active is a **re-statement** (#647 review): the - ack is kept and no second "Activated" row is published — a respawned driver re-reading a still-active alarm sends - exactly that. Only `Retrigger` (or inactive → active) un-acks. A native source must also set `Kind`: an + `drivers/FOCAS.md`). A `Raise` on a condition already projected Active **with the same message** is a + **re-statement** (#647 review): the ack is kept and no second "Activated" row is published — a respawned driver + re-reading a still-active alarm sends exactly that. A Raise with a DIFFERENT message is a new statement and un-acks: + a respawned driver cannot tell "the same alarm" from "a different one that appeared while it was down", so the + host compares what the condition shows. Only `Retrigger` (or inactive → active, or a changed-message Raise) un-acks. + ⚠️ **Residual:** the comparison is only as good as the source's message determinism. FOCAS and MTConnect messages + are deterministic for an unchanged statement; a source whose message carries volatile text (a timestamp, a + counter) would un-ack on every respawn — conservative, never hiding an alarm — and a source whose message omits + what changed (a new alarm under identical text) would keep a stale ack across a respawn. A native source must also set `Kind`: an `Unspecified` kind projects no state change. ### Native condition state survives a full rebuild (Gitea #641) diff --git a/docs/drivers/FOCAS.md b/docs/drivers/FOCAS.md index 5840ae7a..1df7fd0b 100644 --- a/docs/drivers/FOCAS.md +++ b/docs/drivers/FOCAS.md @@ -405,16 +405,28 @@ it, then author `ALARM:T`. Number ranges are series-independent. - **…but an unreachable source says so.** The first failed read of an outage annotates every subscribed condition on that device `BadNoCommunication` through the #626 quality-only path (no state change, no `/alerts` row) and logs ONE Warning naming `cnc_rdalmmsg2`; the next successful read lifts it to Good (before any transition it - also finds, e.g. Good then Clear for an alarm that cleared during the outage). Failed alarm reads also reach + also finds, e.g. Good then Clear for an alarm that cleared during the outage). Transport failures also reach driver health: `Degraded` with `LastSuccessfulRead` frozen (only a successful read advances it), so a driver whose only tags are `ALARM:` tags trips the comms-loss watchdog (#527) and the host's driver-wide #477 annotation like any other. +- **A CNC refusal is not comms loss.** A non-OK FOCAS return from `cnc_rdalmmsg2` (the CNC answered and declined, + e.g. `EW_FUNC`) marks the conditions `BadNotSupported` (quality-only, once), logs one Warning, and **leaves driver + health alone** — polling continues at the normal cadence. Treating it as comms loss would make the #527 watchdog + reconnect a production control every 90 s forever. Only a transport fault (session down, wire fault, malformed + reply) degrades. +- **Health is the worst of the value path and the alarm path, computed together** (`GetHealth`). A failing alarm + read keeps a mixed driver `Degraded` however many value reads succeed (so `/hosts` does not flip per poll), and a + good alarm read cannot mask a value-path `Degraded`. `LastSuccessfulRead` is the later of the two paths' last + successes, so the watchdog fires only when both have stopped. - **Diffed against what the server was TOLD**, recorded only while a handler is attached. The record lives on the driver instance: a reconnect (same-set re-subscribe) and an in-place re-initialise (Force Reconnect, #630) re-raise nothing. **A deploy that changes the driver's config respawns it** (the planner stops + recreates the driver, a new instance with an empty record), and its first poll sends `Raise` for every still-active alarm. The host treats a - `Raise` on a condition it already shows Active as a **re-statement**: the acknowledgement is kept and no second - "Activated" `/alerts` row is written (`NativeAlarmProjector`, #647 review). Only `Retrigger` — or inactive → + `Raise` on a condition it already shows Active **with the same message** as a **re-statement**: the + acknowledgement is kept and no second "Activated" `/alerts` row is written (`NativeAlarmProjector`, #647 review). + The condition message is a deterministic function of the matched set (winner, type code, axes, `[+N more]`), so a + true re-statement matches exactly, while an alarm set that changed during the respawn — including one that only + LOST a member — reads as a new statement and un-acks (the conservative direction). Only `Retrigger` — or inactive → active — un-acknowledges, which is why a genuinely new CNC alarm in a matched set is sent as `Retrigger`. - **Polling:** one `cnc_rdalmmsg2` per `AlarmProjection.PollInterval` (default 2 s) for each device that owns a **subscribed** alarm tag; devices without one are never polled for alarms. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasAlarmProjection.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasAlarmProjection.cs index 7fb8e026..b44d6905 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasAlarmProjection.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasAlarmProjection.cs @@ -57,22 +57,37 @@ internal sealed class FocasAlarmProjection : IAsyncDisposable /// Devices whose last read came back at the per-read cap. Guarded by . private readonly HashSet _truncationWarned = new(StringComparer.OrdinalIgnoreCase); - /// Devices whose alarm read is currently failing (one Warning per episode). Guarded by . - private readonly HashSet _failingDevices = new(StringComparer.OrdinalIgnoreCase); + /// + /// Devices whose alarm read is currently failing → the quality that failure maps to (one Warning per episode + /// and per kind: a refusal turning into a transport fault warns again). Guarded by . + /// + private readonly Dictionary _failingDevices = new(StringComparer.OrdinalIgnoreCase); /// /// ConditionIds this projection has annotated BadNoCommunication and not yet lifted. Guarded by /// . Kept apart from : a quality annotation never touches a /// condition's Active/Acked state. /// - private readonly HashSet _qualityBadTold = new(StringComparer.Ordinal); + private readonly Dictionary _qualityBadTold = new(StringComparer.Ordinal); /// Canonical Opc.Ua.StatusCodes.Good. private const uint QualityGood = 0x00000000u; - /// Canonical Opc.Ua.StatusCodes.BadNoCommunication. + /// Canonical Opc.Ua.StatusCodes.BadNoCommunication — the CNC could not be reached. private const uint QualityBadNoCommunication = 0x80310000u; + /// + /// Whether an alarm-read failure is a CNC REFUSAL (Gitea #647 re-review R2): the CNC answered with a non-OK + /// FOCAS return code, so the transport is fine and the control simply will not serve cnc_rdalmmsg2 + /// (e.g. EW_FUNC, as cnc_rdparam does on the Makino). That is a capability answer, not comms + /// loss: it must NOT degrade driver health, or an alarm-only driver on such a control would be reconnected by + /// the #527 watchdog every 90 s forever. Everything else (session down, wire fault, malformed reply, backoff) + /// is a transport failure. + /// + /// The alarm-read failure. + /// for a refusal. + internal static bool IsCncRefusal(Exception error) => error is Wire.FocasWireException { Rc: not null }; + /// Serializes loop start/stop. private readonly SemaphoreSlim _loopGate = new(1, 1); @@ -246,13 +261,15 @@ internal sealed class FocasAlarmProjection : IAsyncDisposable catch (Exception ex) { // Not "no alarms": keep every condition on this device exactly as the server was last told, but say - // its source is unreachable — and let the failure reach driver health (an alarm-only driver has no - // other read to trip the comms-loss watchdog with). - _driver.NoteAlarmReadFailed(host, ex); + // why it cannot be read. A transport failure reaches driver health (an alarm-only driver has no other + // read to trip the comms-loss watchdog with); a CNC refusal does not (R2). Polling continues at the + // normal cadence either way. + if (!IsCncRefusal(ex)) _driver.NoteAlarmReadFailed(host, ex); + else _driver.NoteAlarmReadRefused(host); // the CNC answered: the transport to it is fine TickFailed(sub, host, tags, ex); continue; } - _driver.NoteAlarmReadSucceeded(); + _driver.NoteAlarmReadSucceeded(host); Tick(sub, host, alarms, tags); } } @@ -290,14 +307,14 @@ internal sealed class FocasAlarmProjection : IAsyncDisposable var handlers = _driver.AlarmHandlers; if (handlers is null) return; - if (_failingDevices.Remove(deviceHostAddress)) + if (_failingDevices.Remove(deviceHostAddress, out _)) _logger.LogInformation("FOCAS alarm read for {Host} recovered", deviceHostAddress); foreach (var tag in deviceTags.Where(t => sub.References.Contains(t.RawPath))) { // Lift a Bad this projection put there BEFORE the normal diff, so a recovery that also changed the // alarm state (cleared during the outage) lands as Good then Clear. - if (_qualityBadTold.Remove(tag.RawPath)) + if (_qualityBadTold.Remove(tag.RawPath, out _)) Deliver(handlers, QualityOnly(tag, sub.Handle, QualityGood, now), tag.RawPath); var view = FocasAlarmMatcher.Evaluate(tag.Selector, alarms, deviceSelectors, truncated); @@ -312,8 +329,9 @@ internal sealed class FocasAlarmProjection : IAsyncDisposable /// /// A FAILED read of (Gitea #647 review): conditions keep their state, but - /// each subscribed one is annotated BadNoCommunication once per outage through the #626 quality-only - /// path (no Active/Acked change, no /alerts row), and the first failure of the episode logs one + /// each subscribed one is annotated once per outage through the #626 quality-only path (no Active/Acked change, + /// no /alerts row) — BadNoCommunication for a transport failure, BadNotSupported for a CNC + /// refusal (R2, ) — and the first failure of the episode (per kind) logs one /// Warning. The next successful read lifts the Bad in . /// /// The subscription the read was taken for; a superseded one emits nothing. @@ -328,10 +346,21 @@ internal sealed class FocasAlarmProjection : IAsyncDisposable { if (!ReferenceEquals(Volatile.Read(ref _current), sub)) return; - if (_failingDevices.Add(deviceHostAddress)) - _logger.LogWarning(error, - "FOCAS alarm read (cnc_rdalmmsg2) failed for {Host}; its alarm conditions keep their state and are " + - "marked BadNoCommunication until a read succeeds", deviceHostAddress); + var refusal = IsCncRefusal(error); + var quality = refusal ? FocasStatusMapper.BadNotSupported : QualityBadNoCommunication; + if (!_failingDevices.TryGetValue(deviceHostAddress, out var was) || was != quality) + { + _failingDevices[deviceHostAddress] = quality; + if (refusal) + _logger.LogWarning(error, + "FOCAS CNC {Host} refuses the alarm read (cnc_rdalmmsg2); its alarm conditions keep their state " + + "and are marked BadNotSupported. This is a capability answer, not comms loss — driver health is " + + "unaffected and polling continues", deviceHostAddress); + else + _logger.LogWarning(error, + "FOCAS alarm read (cnc_rdalmmsg2) failed for {Host}; its alarm conditions keep their state and " + + "are marked BadNoCommunication until a read succeeds", deviceHostAddress); + } else _logger.LogDebug(error, "FOCAS alarm read for {Host} still failing", deviceHostAddress); @@ -343,8 +372,9 @@ internal sealed class FocasAlarmProjection : IAsyncDisposable string.Equals(t.DeviceHostAddress, deviceHostAddress, StringComparison.OrdinalIgnoreCase) && sub.References.Contains(t.RawPath))) { - if (!_qualityBadTold.Add(tag.RawPath)) continue; - Deliver(handlers, QualityOnly(tag, sub.Handle, QualityBadNoCommunication, now), tag.RawPath); + if (_qualityBadTold.TryGetValue(tag.RawPath, out var told) && told == quality) continue; + _qualityBadTold[tag.RawPath] = quality; + Deliver(handlers, QualityOnly(tag, sub.Handle, quality, now), tag.RawPath); } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index 7245578e..54d97740 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -200,6 +200,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, // AdoptConfigDuringInit calls this without a teardown first). Not ShutdownAsync: that also drops _poll // and subscriptions, which are owned by the shutdown path (the actor re-subscribes after every successful init). await ReleaseDeviceStatesAsync().ConfigureAwait(false); + ResetAlarmHealth(); Volatile.Write(ref _health, new DriverHealth(DriverState.Initializing, null, null)); try { @@ -475,11 +476,32 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, _resolver.Clear(); // no-op in v3 (the resolver reads the live table by closure); kept for symmetry _parsedAddressesByTagName.Clear(); _fixedTreeActive = false; + ResetAlarmHealth(); Volatile.Write(ref _health, new DriverHealth(DriverState.Unknown, Volatile.Read(ref _health).LastSuccessfulRead, null)); } /// - public DriverHealth GetHealth() => Volatile.Read(ref _health); + /// + /// The worst of the value path () and the alarm path (), + /// computed together (Gitea #647 re-review R3). is the later of + /// the two paths' last successes: while either path still reads, the comms-loss watchdog stays quiet; when both + /// have stopped it is frozen. Faulted / Unknown / Initializing come from the value (lifecycle) path unchanged. + /// + public DriverHealth GetHealth() + { + var value = Volatile.Read(ref _health); + var ticks = Interlocked.Read(ref _alarmLastSuccessTicks); + DateTime? alarmLast = ticks == 0 ? null : new DateTime(ticks, DateTimeKind.Utc); + var last = value.LastSuccessfulRead is { } v && (alarmLast is null || v >= alarmLast) ? v : alarmLast; + + if (value.State is not (DriverState.Healthy or DriverState.Degraded)) return value; + if (value.State == DriverState.Healthy && !_alarmTransportFailures.IsEmpty) + { + var reason = _alarmTransportFailures.OrderBy(kv => kv.Key, StringComparer.OrdinalIgnoreCase).First().Value; + return new DriverHealth(DriverState.Degraded, last, reason); + } + return value.LastSuccessfulRead == last ? value : value with { LastSuccessfulRead = last }; + } /// Gets the number of configured devices. internal int DeviceCount => _devices.Count; @@ -1700,31 +1722,50 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// - /// A successful alarm read is a successful read (Gitea #647 review): it advances - /// exactly as a Good value read does, so an alarm-only driver's - /// health tracks its CNC. Never lifts a (a config-level signal). + /// The ALARM-path half of driver health (Gitea #647 re-review R3): device host → why its alarm read is failing + /// at the TRANSPORT. Kept apart from (the value path) and combined with it in + /// , so neither path can overwrite the other: a good value read cannot mask a failing + /// alarm read (and flip /hosts back and forth every poll), and a good alarm read cannot mask a value-path + /// Degraded. A CNC refusal never lands here (R2 — see ). /// - internal void NoteAlarmReadSucceeded() + private readonly ConcurrentDictionary _alarmTransportFailures = new(StringComparer.OrdinalIgnoreCase); + + /// UTC ticks of the last successful alarm read (0 = none) — the alarm path's LastSuccessfulRead. + private long _alarmLastSuccessTicks; + + /// + /// A successful alarm read of : clears that device's alarm-path failure and + /// advances the alarm path's LastSuccessfulRead, so an alarm-only driver's health tracks its CNC. + /// + /// The device read. + internal void NoteAlarmReadSucceeded(string hostAddress) { - var current = Volatile.Read(ref _health); - if (current.State != DriverState.Faulted) - Volatile.Write(ref _health, new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null)); + _alarmTransportFailures.TryRemove(hostAddress, out _); + Interlocked.Exchange(ref _alarmLastSuccessTicks, DateTime.UtcNow.Ticks); } /// - /// A failed alarm read degrades driver health, PRESERVING — the - /// conjunction the comms-loss watchdog (#527) trips on. Without it a driver whose only tags are ALARM: - /// tags stayed Healthy through a total outage, the watchdog never fired, and the host's #477 - /// BadNoCommunication annotation never ran (Gitea #647 review). + /// The CNC REFUSED the alarm read (R2): the transport is fine, so any alarm-path transport failure of the + /// device is over — but nothing was read, so LastSuccessfulRead does not advance. + /// + /// The device that answered. + internal void NoteAlarmReadRefused(string hostAddress) => _alarmTransportFailures.TryRemove(hostAddress, out _); + + /// + /// A TRANSPORT failure of an alarm read: the alarm path is Degraded until the device's next good read, and its + /// LastSuccessfulRead stays frozen — the conjunction the comms-loss watchdog (#527) trips on, which a driver + /// whose only tags are ALARM: tags otherwise never reached (Gitea #647 review). /// /// The device whose alarm read failed. /// Why. - internal void NoteAlarmReadFailed(string hostAddress, Exception error) + internal void NoteAlarmReadFailed(string hostAddress, Exception error) => + _alarmTransportFailures[hostAddress] = $"FOCAS alarm read failed for {hostAddress}: {error.Message}"; + + /// Clears the alarm-path health (a fresh device set at initialise / shutdown). + private void ResetAlarmHealth() { - var current = Volatile.Read(ref _health); - if (current.State != DriverState.Faulted) - Volatile.Write(ref _health, new DriverHealth(DriverState.Degraded, current.LastSuccessfulRead, - $"FOCAS alarm read failed for {hostAddress}: {error.Message}")); + _alarmTransportFailures.Clear(); + Interlocked.Exchange(ref _alarmLastSuccessTicks, 0); } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs index f35d4992..3701d578 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs @@ -119,13 +119,16 @@ public sealed class WireFocasClient : IFocasClient /// Gitea #647: every failure THROWS — a disconnected session, a non-OK return code and a wire fault /// alike. This used to answer [] for all three, which the alarm projection could not tell /// from "no alarms active": every comms blip cleared every active alarm, and recovery re-raised them - /// un-acknowledged. propagates as-is. + /// un-acknowledged. propagates as-is; a refusal (non-OK rc) is thrown carrying its Rc. /// public async Task> ReadAlarmsAsync(CancellationToken cancellationToken) { RequireConnected(); var result = await _wire.ReadAlarmsAsync(FocasAlarmType.All, MaxAlarmsPerRead, cancellationToken).ConfigureAwait(false); - ThrowIfRcNonZero(result.Rc, "cnc_rdalmmsg2", result.IsOk); + // A non-OK return code is the CNC REFUSING the call over a healthy transport — thrown carrying the rc, so + // the alarm projection can tell it from comms loss and keep it out of driver health (#647 re-review R2). + if (!result.IsOk) + throw new FocasWireException($"cnc_rdalmmsg2 refused by the CNC (EW_{result.Rc}).", result.Rc); return (result.Value ?? []).Select(Map).ToList(); static FocasActiveAlarm Map(WireAlarm a) => new( diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs index b8392f77..4b91fead 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs @@ -12,7 +12,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Drivers; /// public sealed class NativeAlarmProjector { - private readonly Dictionary _prior = new(StringComparer.Ordinal); + private readonly Dictionary _prior = new(StringComparer.Ordinal); /// Project an alarm transition onto the full condition snapshot for . /// The materialised condition node's id (the projection's state key). @@ -22,7 +22,8 @@ public sealed class NativeAlarmProjector /// /// Project an alarm transition, reporting whether it was a re-statement (Gitea #647 review): a - /// on a condition already projected Active. That is what a driver + /// on a condition already projected Active WITH THE SAME MESSAGE + /// (re-review R1 — a different message is a different alarm, and un-acks). That is what a driver /// whose told-record died with its instance sends after a deploy respawns it — the alarm it re-reads is the /// one the server already shows, so the acknowledgement is KEPT and the host writes no second "Activated" /// row. Only (or inactive → active) un-acknowledges; a source @@ -34,19 +35,25 @@ public sealed class NativeAlarmProjector /// The full Part 9 condition snapshot to write to the node. public AlarmConditionSnapshot Project(string nodeId, AlarmEventArgs e, out bool restatement) { - var prev = _prior.TryGetValue(nodeId, out var p) ? p : (Active: false, Acked: true); + var prev = _prior.TryGetValue(nodeId, out var p) ? p : (Active: false, Acked: true, Message: (string?)null); // #626: a source with a finer scale than the four buckets supplies the exact Part 9 severity. var sev = e.OpcUaSeverity is { } exact ? (ushort)Math.Clamp(exact, 1, 1000) : MapSeverity(e.Severity); - restatement = e.Kind == AlarmTransitionKind.Raise && prev.Active; + // Gitea #647 re-review R1: only a Raise saying the SAME thing the condition already shows is a re-statement. + // A respawned driver cannot tell "the alarm I was showing" from "a different one that appeared while I was + // down", so a different message is treated as a new statement — un-acked, with its alerts row. Comparing the + // message is conservative: any difference (a sources' messages are deterministic for an unchanged statement) + // errs toward showing the operator an unacknowledged alarm, never toward hiding one behind a stale ack. + restatement = e.Kind == AlarmTransitionKind.Raise && prev.Active + && string.Equals(prev.Message?.Trim(), e.Message.Trim(), StringComparison.Ordinal); var (active, acked) = e.Kind switch { - AlarmTransitionKind.Raise when prev.Active => (true, prev.Acked), + AlarmTransitionKind.Raise when restatement => (true, prev.Acked), AlarmTransitionKind.Raise or AlarmTransitionKind.Retrigger => (true, false), AlarmTransitionKind.Acknowledge => (prev.Active, true), AlarmTransitionKind.Clear => (false, prev.Acked), _ => (prev.Active, prev.Acked), }; - _prior[nodeId] = (active, acked); + _prior[nodeId] = (active, acked, e.Message); return new AlarmConditionSnapshot( Active: active, Acknowledged: acked, Confirmed: true, Enabled: true, Shelving: AlarmShelvingKind.Unshelved, Severity: sev, Message: e.Message, @@ -64,7 +71,7 @@ public sealed class NativeAlarmProjector /// The acknowledged condition node id. public void MarkAcknowledged(string nodeId) { - if (_prior.TryGetValue(nodeId, out var p)) _prior[nodeId] = (p.Active, true); + if (_prior.TryGetValue(nodeId, out var p)) _prior[nodeId] = (p.Active, true, p.Message); } /// Clears all tracked per-node state. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FakeFocasClient.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FakeFocasClient.cs index 8105d7c8..00cf61d5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FakeFocasClient.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FakeFocasClient.cs @@ -126,6 +126,9 @@ internal class FakeFocasClient : IFocasClient /// Gets or sets a value indicating whether the alarm read throws (Gitea #647: a failed read). public bool ThrowOnReadAlarms { get; set; } + /// Gets or sets an exception the alarm read throws (e.g. a CNC refusal carrying an rc). + public Exception? AlarmReadException { get; set; } + /// Gets the number of alarm reads served (successful or not). public int AlarmReadCount => Volatile.Read(ref _alarmReadCount); @@ -143,6 +146,7 @@ internal class FakeFocasClient : IFocasClient public virtual Task> ReadAlarmsAsync(CancellationToken ct) { Interlocked.Increment(ref _alarmReadCount); + if (AlarmReadException is { } refusal) throw refusal; if (ThrowOnReadAlarms) throw new InvalidOperationException("cnc_rdalmmsg2 failed (fake)"); lock (Alarms) return Task.FromResult>([.. Alarms]); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAlarmProjectionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAlarmProjectionTests.cs index 40d5801f..e06eacd1 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAlarmProjectionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAlarmProjectionTests.cs @@ -545,6 +545,111 @@ public sealed class FocasAlarmProjectionTests } } + // ---- Gitea #647 re-review R1–R3 ------------------------------------------------------------------------- + + /// + /// R1 — the host treats a respawned driver's Raise as a re-statement only when its message matches what the + /// condition shows, so the message must be a deterministic function of the matched SET: two fresh driver + /// instances reading the same alarms in a different wire (and axis) order say exactly the same thing, and a + /// different set says something different. + /// + [Fact] + public async Task The_condition_message_is_deterministic_for_the_same_alarm_set_across_driver_instances() + { + async Task FirstRaiseMessage(params FocasActiveAlarm[] alarms) + { + var (drv, factory) = NewDriver(Tag(AnyPath, "ALARM:*")); + await InitAndSubscribeForTicksAsync(drv, factory, AnyPath); + var events = Capture(drv); + Tick(drv, alarms); + return events.Single().Message; + } + + var a = await FirstRaiseMessage(Servo401(axis: 2), Overheat700, Servo401(axis: 1), Overtravel506); + var b = await FirstRaiseMessage(Overtravel506, Servo401(axis: 1), Overheat700, Servo401(axis: 2)); + b.ShouldBe(a); + (await FirstRaiseMessage(Overtravel506, Servo401(axis: 1))).ShouldNotBe(a, "a different set must not read as a re-statement"); + } + + private static Wire.FocasWireException CncRefusal() => new("cnc_rdalmmsg2 refused EW_FUNC", rc: 1); + + /// + /// R2 — a CNC REFUSAL of cnc_rdalmmsg2 (an rc; the transport is fine) is a capability answer, not comms loss: + /// health stays Healthy (so the #527 watchdog can never reconnect-loop a production control), the conditions + /// are annotated BadNotSupported once, one Warning is logged, and polling continues at the normal cadence. + /// Falsification control: a TRANSPORT failure on the same driver does degrade. + /// + [Fact] + public async Task A_CNC_refusal_marks_conditions_not_supported_without_degrading_health() + { + var logger = new ListLogger(); + var factory = new FakeFocasClientFactory { Customise = () => new FakeFocasClient { AlarmReadException = CncRefusal() } }; + var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions(Host, DeviceName: "m1")], + RawTags = [Tag(ServoPath, "ALARM:Servo")], + Probe = new FocasProbeOptions { Enabled = false }, + AlarmProjection = new FocasAlarmProjectionOptions { PollInterval = TimeSpan.FromMilliseconds(20) }, + }, "drv-1", factory, logger); + await drv.InitializeAsync("{}", CancellationToken.None); + var events = Capture(drv); + await drv.SubscribeAlarmsAsync([ServoPath], CancellationToken.None); + await WaitFor(() => factory.Clients.Count > 0 && factory.Clients[0].AlarmReadCount >= 5); + + drv.GetHealth().State.ShouldBe(DriverState.Healthy, "a refusal is not a comms fault"); + lock (events) + { + events.Count.ShouldBe(1); + events[0].IsQualityOnly.ShouldBeTrue(); + events[0].QualityStatusCode.ShouldBe(FocasStatusMapper.BadNotSupported); + } + logger.Warnings().ShouldBe(1); + factory.Clients.Count.ShouldBe(1, "a refusal does not tear the session down"); + + // Control: the same driver losing its transport DOES degrade, and re-annotates the condition. + factory.Clients[0].AlarmReadException = null; + factory.Clients[0].ThrowOnReadAlarms = true; + await WaitFor(() => drv.GetHealth().State == DriverState.Degraded); + lock (events) events[^1].QualityStatusCode.ShouldBe(BadNoCommunication); + await drv.ShutdownAsync(CancellationToken.None); + } + + /// + /// R3 — on a mixed driver health is the WORST of the value path and the alarm path, computed together: a + /// failing alarm read keeps the driver Degraded however many value reads succeed (no per-poll flip on + /// /hosts), and a successful alarm read cannot mask a value-path Degraded. + /// + [Fact] + public async Task Health_is_the_worst_of_the_value_and_alarm_paths() + { + const string valuePath = "Plant/Cnc/m1/r100"; + var (drv, factory) = NewDriver( + new RawTagEntry(valuePath, """{"address":"R100","dataType":"Byte"}""", false, "m1"), + Tag(ServoPath, "ALARM:Servo")); + await drv.InitializeAsync("{}", CancellationToken.None); + + // Value reads good, alarm path failing ⇒ Degraded, and it stays Degraded across further good value reads. + (await drv.ReadAsync([valuePath], CancellationToken.None))[0].StatusCode.ShouldBe(FocasStatusMapper.Good); + drv.NoteAlarmReadFailed(Host, new InvalidOperationException("socket closed")); + for (var i = 0; i < 3; i++) + { + await drv.ReadAsync([valuePath], CancellationToken.None); + drv.GetHealth().State.ShouldBe(DriverState.Degraded, "a good value read must not mask the failing alarm path"); + } + drv.GetHealth().LastSuccessfulRead.ShouldNotBeNull("value reads still succeed, so the watchdog is not tripped"); + + // Alarm path recovers ⇒ Healthy again. + drv.NoteAlarmReadSucceeded(Host); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + + // Value path Degraded, alarm path good ⇒ Degraded: an alarm success cannot mask it. + factory.Clients.Single().ReadStatuses["R100"] = FocasStatusMapper.BadOutOfRange; + await drv.ReadAsync([valuePath], CancellationToken.None); + drv.NoteAlarmReadSucceeded(Host); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + await drv.ShutdownAsync(CancellationToken.None); + } + private static int Count(List events) { lock (events) return events.Count; diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/FocasNativeAlarmEndToEndTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/FocasNativeAlarmEndToEndTests.cs index 0e3e20af..bd3406c7 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/FocasNativeAlarmEndToEndTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/FocasNativeAlarmEndToEndTests.cs @@ -98,8 +98,15 @@ public sealed class FocasNativeAlarmEndToEndTests : RuntimeActorTestBase /// sends Raise; the host must treat that as a re-statement: the operator's acknowledgement survives and no /// second "Activated" alerts row is written. Driven through the real host with a FRESH driver instance. /// - [Fact] - public async Task A_respawned_driver_re_reading_an_acknowledged_active_alarm_keeps_the_ack() + /// + /// Re-review R1 — the other half: a DIFFERENT alarm that appeared while the driver was down (here a second + /// servo alarm, so the condition's displayed set changed) is a new statement even though the fresh driver + /// can only say Raise. It must un-acknowledge and write its alerts row. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_respawned_driver_re_reading_an_acknowledged_active_alarm_keeps_the_ack(bool newAlarmDuringRespawn) { var db = NewInMemoryDbFactory(); var blob = BuildArtifact(); @@ -130,14 +137,27 @@ public sealed class FocasNativeAlarmEndToEndTests : RuntimeActorTestBase coordinator.FishForMessage(a => a.Outcome == ApplyAckOutcome.Applied, TimeSpan.FromSeconds(5)); publish.FishForMessage(_ => true, TimeSpan.FromSeconds(5)); + if (newAlarmDuringRespawn) + cnc.SetAlarms( + new FocasActiveAlarm(401, FocasAlarmType.Servo, 1, "SERVO V-READY OFF"), + new FocasActiveAlarm(410, FocasAlarmType.Servo, 2, "SERVO EXCESS ERROR")); + var respawned = await StartDriverAsync(rawTags, cnc, host); try { var restated = publish.FishForMessage(_ => true, TimeSpan.FromSeconds(5)); restated.AlarmNodeId.ShouldBe(ExpectedRawPath); restated.State.Active.ShouldBeTrue(); - restated.State.Acknowledged.ShouldBeTrue("a respawned driver's re-statement must not un-acknowledge"); - alerts.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); + if (newAlarmDuringRespawn) + { + restated.State.Acknowledged.ShouldBeFalse("an alarm the operator has not seen must not inherit the old ack"); + alerts.ExpectMsg(TimeSpan.FromSeconds(5)); + } + else + { + restated.State.Acknowledged.ShouldBeTrue("a respawned driver's re-statement must not un-acknowledge"); + alerts.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); + } } finally { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs index 12b878f8..66ccd2a9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs @@ -237,7 +237,7 @@ public class NativeAlarmProjectorTests first.ShouldBeFalse("inactive → active is a real transition"); sut.MarkAcknowledged("n1"); - var restated = sut.Project("n1", Evt(AlarmTransitionKind.Raise, msg: "again"), out var restatement); + var restated = sut.Project("n1", Evt(AlarmTransitionKind.Raise), out var restatement); restatement.ShouldBeTrue(); restated.Active.ShouldBeTrue(); restated.Acknowledged.ShouldBeTrue("a re-statement must not un-acknowledge"); @@ -247,6 +247,27 @@ public class NativeAlarmProjectorTests retriggered.Acknowledged.ShouldBeFalse("a Retrigger is a new statement and un-acks"); } + /// + /// Gitea #647 re-review R1 — a respawned driver cannot tell "the same alarm" from "a different one that + /// appeared while it was down"; its Raise is a re-statement only when its message matches what the condition + /// shows. A different message is a new statement: it un-acks and is not flagged (so the alerts row is written). + /// + [Fact] + public void A_Raise_on_an_active_condition_with_a_different_message_is_a_new_statement() + { + var sut = new NativeAlarmProjector(); + sut.Project("n1", Evt(AlarmTransitionKind.Raise, msg: "401 SERVO (type 4 Servo, axis 1)")); + sut.MarkAcknowledged("n1"); + + var same = sut.Project("n1", Evt(AlarmTransitionKind.Raise, msg: "401 SERVO (type 4 Servo, axis 1)"), out var r1); + r1.ShouldBeTrue(); + same.Acknowledged.ShouldBeTrue(); + + var other = sut.Project("n1", Evt(AlarmTransitionKind.Raise, msg: "506 OVER TRAVEL (type 2 Overtravel, axis 2)"), out var r2); + r2.ShouldBeFalse("a different alarm is not a re-statement"); + other.Acknowledged.ShouldBeFalse("the operator has not seen this alarm"); + } + private static AlarmEventArgs Evt( AlarmTransitionKind kind, AlarmSeverity sev = AlarmSeverity.High,