diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs index ac429441..73f6261b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs @@ -70,6 +70,12 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable // prior subscriptions before starting the new one is semantically faithful. Snapshotting + // clearing under the same lock DisposeAsync uses guarantees the stale subs can never be // double-owned (and thus never double-disposed) by a racing dispose. + // Gitea #641 review — forget only the refs that LEFT the set. A survivor keeps its last-seen InFaulted, so + // a re-subscribe (every reconnect, every alarm-set change) does not re-RAISE a still-faulted alarm: the + // server carries the condition across a rebuild, and a Raise would un-acknowledge it. A ref that left and + // comes back was materialised fresh, so it starts from "not faulted" and is raised again. + _driver.AlarmFaultRecord.RetainOnly(sub.SourceNodeIds); + List stale; lock (_subsLock) { @@ -190,8 +196,7 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable // is intentional and matches the documented behaviour. var severity = ToInt(severityDv.Value); - var wasFaulted = sub.LastInFaulted.GetValueOrDefault(nodeId, false); - sub.LastInFaulted[nodeId] = nowFaulted; + var wasFaulted = _driver.AlarmFaultRecord.Exchange(nodeId, nowFaulted); // ConditionId is the subscribed reference itself — the alarm tag's RawPath, the key the host routes // native transitions on — and Kind tells the host's projector what changed (Gitea #641). This used @@ -303,9 +308,44 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable /// Gets or sets the polling loop task. public Task Loop { get; set; } = Task.CompletedTask; + } +} - /// Gets the dictionary tracking the last known InFaulted state for each node. - public Dictionary LastInFaulted { get; } = new(StringComparer.Ordinal); +/// +/// Gitea #641 review — the last InFaulted the ALMD projection told the server, per alarm RawPath. Owned by +/// the rather than a subscription or a projection, so it survives a re-subscribe and a +/// re-initialise (both of which happen on every reconnect): the server's condition nodes outlive them, and a +/// re-raise there would un-acknowledge. Thread-safe — a retiring poll loop may overlap the new one for a tick. +/// +internal sealed class AbCipAlarmFaultRecord +{ + private readonly Dictionary _lastInFaulted = new(StringComparer.Ordinal); + private readonly Lock _lock = new(); + + /// Records for , returning the prior value + /// (false when none). + /// The alarm RawPath. + /// The state just read. + /// The previously recorded state. + public bool Exchange(string nodeId, bool nowFaulted) + { + lock (_lock) + { + var was = _lastInFaulted.GetValueOrDefault(nodeId, false); + _lastInFaulted[nodeId] = nowFaulted; + return was; + } + } + + /// Forgets every ref not in . + /// The newly subscribed refs. + public void RetainOnly(IReadOnlyList nodeIds) + { + var keep = new HashSet(nodeIds, StringComparer.Ordinal); + lock (_lock) + { + foreach (var k in _lastInFaulted.Keys.Where(k => !keep.Contains(k)).ToList()) _lastInFaulted.Remove(k); + } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index a80a86e0..2fade80c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -118,6 +118,10 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, private readonly ILogger _logger; private AbCipAlarmProjection _alarmProjection; + + /// Gitea #641 review — the ALMD projection's last-told InFaulted per alarm ref; outlives every + /// projection and subscription (see ). + internal AbCipAlarmFaultRecord AlarmFaultRecord { get; } = new(); private DriverHealth _health = new(DriverState.Unknown, null, null); /// Occurs when a subscribed tag's value changes. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs index 558554f5..ba312794 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs @@ -206,6 +206,65 @@ public sealed class AbCipAlarmProjectionTests cleared.Kind.ShouldBe(AlarmTransitionKind.Clear); } + /// + /// Gitea #641 review — the server carries every surviving native condition's state (a client's + /// acknowledgement included) across an address-space rebuild, and the host re-subscribes alarms on every + /// reconnect and every alarm-set change. A re-subscribe used to start from an empty "last seen" record, so + /// a still-faulted ALMD was re-RAISED, which the host projects as Active+UNacked — un-acknowledging an alarm + /// an operator had already acknowledged. The record is the driver's, kept for refs that stay in the set. + /// + [Fact] + public async Task A_resubscribe_does_not_re_raise_a_still_faulted_alarm() + { + var factory = new FakeAbCipTagFactory(); + var drv = new AbCipDriver(Options(enabled: true), "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var raises = new List(); + drv.OnAlarmEvent += (_, e) => { if (e.Kind == AlarmTransitionKind.Raise) lock (raises) raises.Add(e); }; + + var sub1 = await drv.SubscribeAlarmsAsync(["HighTemp"], CancellationToken.None); + await WaitForTagCreation(factory, "HighTemp"); + factory.Tags["HighTemp"].ValuesByOffset[8] = 500; + factory.Tags["HighTemp"].ValuesByOffset[0] = 1; + await WaitForEventAsync(raises, _ => true); + + // A reconnect / alarm-set change: the same ref, still faulted. + var sub2 = await drv.SubscribeAlarmsAsync(["HighTemp"], CancellationToken.None); + await WaitForMoreReadsAsync(factory, "HighTemp", 6); + + lock (raises) raises.Count.ShouldBe(1, "a re-subscribe must not re-raise (and so un-acknowledge) a still-faulted alarm"); + + await drv.UnsubscribeAlarmsAsync(sub1, CancellationToken.None); + await drv.UnsubscribeAlarmsAsync(sub2, CancellationToken.None); + await drv.ShutdownAsync(CancellationToken.None); + } + + /// Gitea #641 review — positive control: a ref that LEFT the subscribed set was materialised fresh + /// (inactive) when it came back, so it is raised again. + [Fact] + public async Task A_ref_that_left_the_set_and_came_back_is_raised_again() + { + var factory = new FakeAbCipTagFactory(); + var drv = new AbCipDriver(Options(enabled: true), "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var raises = new List(); + drv.OnAlarmEvent += (_, e) => { if (e.Kind == AlarmTransitionKind.Raise) lock (raises) raises.Add(e); }; + + await drv.SubscribeAlarmsAsync(["HighTemp"], CancellationToken.None); + await WaitForTagCreation(factory, "HighTemp"); + factory.Tags["HighTemp"].ValuesByOffset[8] = 500; + factory.Tags["HighTemp"].ValuesByOffset[0] = 1; + await WaitForEventAsync(raises, _ => true); + + await drv.SubscribeAlarmsAsync([], CancellationToken.None); + await drv.SubscribeAlarmsAsync(["HighTemp"], CancellationToken.None); + await WaitForEventAsync(raises, _ => raises.Count >= 2); + + await drv.ShutdownAsync(CancellationToken.None); + } + /// Verifies that unsubscribing stops the alarm poll loop. [Fact] public async Task Unsubscribe_Stops_The_Poll_Loop()