fix(mtconnect): on a changed alarm set forget only the conditions that left (#641)

The driver cleared its whole "last told" record whenever the subscribed alarm
set changed, assuming the rebuild that accompanies an alarm-bearing delta had
reset every condition node. The server now carries each surviving native
condition's state (acknowledgement included) across the rebuild, so re-raising a
survivor would un-acknowledge it. Only entries that left the set are dropped; a
condition that comes back later is told again onto its fresh node.

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-08 17:06:57 -04:00
parent 11f13f6326
commit 3fe6cbb905
3 changed files with 30 additions and 31 deletions
+6 -5
View File
@@ -462,11 +462,12 @@ and records a state only when an alarm handler was attached to receive it. Conse
condition the driver owns `BadNoCommunication` on the connectivity edge (#477) and keeps its Active
state; the reconnect edge re-annotates Good, after which the driver re-asserts any still-`Unavailable`
condition's Bad quality on its re-subscribe.
- A re-subscribe with a **changed** alarm set clears the whole told record: only an alarm-bearing raw-tag
delta changes the set, the server applies that as a full rebuild, and a rebuild recreates every
condition node inactive — so every active condition is re-asserted onto its new node. The **same** set
(a reconnect) keeps the record. (A rebuild that leaves this driver's alarm set unchanged triggers no
re-subscribe; that is a host-wide gap shared with every native source.)
- A re-subscribe with a **changed** alarm set drops the told record of only the conditions that **left**
the set. The server carries every native condition's state (acknowledgement included) across a full
rebuild itself (Gitea #641 — see `docs/AlarmTracking.md`), so re-raising a surviving condition would
only un-acknowledge it; a condition that left and later comes back was materialised fresh and is told
again. The **same** set (a reconnect) keeps the whole record. A rebuild that leaves the alarm set
unchanged triggers no re-subscribe, and needs none — the server-side carry-over covers it.
- An alarm-only driver (no value tags) still runs the `/sample` pump.
**EVENT controlled vocabularies as OPC UA enumerations — won't fix.** EVENT values (`Execution`,
@@ -50,18 +50,13 @@ public sealed partial class MTConnectDriver : IAlarmSource
/// <summary>
/// ConditionId (RawPath) → what the server was last told. Guarded by <see cref="_alarmLock"/>. 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.
/// nodes outlive both, and re-raising there would un-acknowledge. When the subscribed set changes (an
/// alarm-bearing raw-tag delta, i.e. a full rebuild) only the entries that LEFT the set are dropped: the
/// server carries every surviving condition's state across the rebuild (Gitea #641), so a survivor needs
/// no re-tell, while a condition that comes back later was materialised fresh and is told again.
/// </summary>
private readonly Dictionary<string, MTConnectToldCondition> _deliveredConditions = new(StringComparer.Ordinal);
/// <summary>
/// The reference set of the last <see cref="SubscribeAlarmsAsync"/>, kept across an unsubscribe so a
/// release-then-resubscribe of an identical set is still recognised. Guarded by <see cref="_alarmLock"/>.
/// </summary>
private HashSet<string>? _lastAlarmReferences;
/// <summary>The CONDITION tracker for the current session; replaced alongside the observation index.</summary>
private MTConnectConditionTracker _conditions = new();
@@ -108,16 +103,15 @@ public sealed partial class MTConnectDriver : IAlarmSource
lock (_alarmLock)
{
// 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))
// Forget only the conditions that LEFT the set (Gitea #641). A changed set is an alarm-bearing delta,
// which the server applies as a full rebuild — but the server carries every surviving condition's
// state (acknowledgement included) across that rebuild, so re-raising a survivor would only
// un-acknowledge it. A condition that left and later comes back was materialised fresh and has no
// record, so it is told again. The SAME set (a reconnect) prunes nothing.
foreach (var conditionId in _deliveredConditions.Keys.Where(k => !references.Contains(k)).ToList())
{
_deliveredConditions.Clear();
_deliveredConditions.Remove(conditionId);
}
_lastAlarmReferences = references;
}
EmitConditionTransitions(touchedDataItemIds: null, resync: true);
@@ -151,27 +151,31 @@ public sealed class MTConnectAlarmSourceTests
}
[Fact]
public async Task A_changed_alarm_set_re_tells_every_condition_and_the_same_set_does_not()
public async Task A_changed_alarm_set_re_tells_only_conditions_that_left_and_came_back()
{
// 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.
// Gitea #641: a changed set is an alarm-bearing delta ⇒ a full rebuild, but the server now carries every
// surviving condition's state (acknowledgement included) across it. Re-raising a survivor would
// UN-acknowledge it, so only the record of a condition that LEFT the set is dropped: one that comes back
// was materialised fresh and must be told again.
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");
await driver.SubscribeAlarmsAsync([ConditionRawPath, OtherConditionRawPath], Ct); // A survives: silent
await driver.SubscribeAlarmsAsync([OtherConditionRawPath, ConditionRawPath], Ct); // same set: silent
events.Count.ShouldBe(1, "a surviving condition is not re-raised");
await driver.SubscribeAlarmsAsync([OtherConditionRawPath], Ct); // A removed
await driver.SubscribeAlarmsAsync([OtherConditionRawPath, ConditionRawPath], Ct); // A re-added: fresh node
events.Count.ShouldBe(2, "a condition that left the set and came back is told again");
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");
[AlarmTransitionKind.Raise, AlarmTransitionKind.Raise, AlarmTransitionKind.Clear]);
await driver.ShutdownAsync(Ct);
}