dohertj2/lmxopcua
1
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(abcip): keep the ALMD last-told state across a re-subscribe so a still-faulted alarm is not re-raised (#641 review)

Browse Source 
Now that ALMD transitions route, a re-subscribe (every reconnect, every alarm-set change)
started from an empty per-subscription record and re-RAISED every still-faulted alarm,
which the host projects as Active+unacked - un-acknowledging a condition the server had
just carried across the rebuild with its client ack. The record is now driver-owned and
pruned to the new set, mirroring MTConnect's _deliveredConditions.

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-08 17:17:55 -04:00
parent bd23f557f3
commit 5c88de078c
3 changed files with 107 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs
+44 -4
View File
@@ -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<Subscription> 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
/// <summary>Gets or sets the polling loop task.</summary>
public Task Loop { get; set; } = Task.CompletedTask;
}
}
/// <summary>Gets the dictionary tracking the last known InFaulted state for each node.</summary>
public Dictionary<string, bool> LastInFaulted { get; } = new(StringComparer.Ordinal);
/// <summary>
/// Gitea #641 review — the last <c>InFaulted</c> the ALMD projection told the server, per alarm RawPath. Owned by
/// the <see cref="AbCipDriver"/> 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.
/// </summary>
internal sealed class AbCipAlarmFaultRecord
{
private readonly Dictionary<string, bool> _lastInFaulted = new(StringComparer.Ordinal);
private readonly Lock _lock = new();
/// <summary>Records <paramref name="nowFaulted"/> for <paramref name="nodeId"/>, returning the prior value
/// (<c>false</c> when none).</summary>
/// <param name="nodeId">The alarm RawPath.</param>
/// <param name="nowFaulted">The state just read.</param>
/// <returns>The previously recorded state.</returns>
public bool Exchange(string nodeId, bool nowFaulted)
{
lock (_lock)
{
var was = _lastInFaulted.GetValueOrDefault(nodeId, false);
_lastInFaulted[nodeId] = nowFaulted;
return was;
}
}
/// <summary>Forgets every ref not in <paramref name="nodeIds"/>.</summary>
/// <param name="nodeIds">The newly subscribed refs.</param>
public void RetainOnly(IReadOnlyList<string> nodeIds)
{
var keep = new HashSet<string>(nodeIds, StringComparer.Ordinal);
lock (_lock)
{
foreach (var k in _lastInFaulted.Keys.Where(k => !keep.Contains(k)).ToList()) _lastInFaulted.Remove(k);
}
}
}
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs
+4
View File
@@ -118,6 +118,10 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
private readonly ILogger<AbCipDriver> _logger;
private AbCipAlarmProjection _alarmProjection;
/// <summary>Gitea #641 review — the ALMD projection's last-told InFaulted per alarm ref; outlives every
/// projection and subscription (see <see cref="AbCipAlarmFaultRecord"/>).</summary>
internal AbCipAlarmFaultRecord AlarmFaultRecord { get; } = new();
private DriverHealth _health = new(DriverState.Unknown, null, null);
/// <summary>Occurs when a subscribed tag's value changes.</summary>
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs
+59
View File
@@ -206,6 +206,65 @@ public sealed class AbCipAlarmProjectionTests
cleared.Kind.ShouldBe(AlarmTransitionKind.Clear);
}
/// <summary>
/// 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.
/// </summary>
[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<AlarmEventArgs>();
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);
}
/// <summary>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.</summary>
[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<AlarmEventArgs>();
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);
}
/// <summary>Verifies that unsubscribing stops the alarm poll loop.</summary>
[Fact]
public async Task Unsubscribe_Stops_The_Poll_Loop()