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

fix(abcip): ALMA review round 1 + ALMD polls InAlarm (not InFaulted)

Browse Source 
M1 drop-out is a no-row Unspecified+IsRestatement update (projector case); M2 ack judged over every declared
limit; L1 ActiveAcknowledged for an already-acked new limit; L2 Bad Acked/severity carried/skipped; L3 host
end-to-end test; L4 BOOL member-by-member tests + mixed ALMD/ALMA; L5 detection requires no plain Acked.
ALMD active bit is InAlarm per Rockwell's ALMD reference; InFaulted is input health.

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-09 04:22:45 -04:00
parent b3838fcce9
commit 83d8de303c
13 changed files with 641 additions and 161 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs
+1 -1
View File
@@ -79,7 +79,7 @@ public sealed class AbCipDriverOptions
/// <summary>
/// When <c>true</c>, declared ALMD tags are surfaced as alarm conditions
/// via <c>IAlarmSource</c>; the driver polls each subscribed
/// alarm's <c>InFaulted</c> + <c>Severity</c> members + fires <c>OnAlarmEvent</c> on
/// alarm's <c>InAlarm</c> + <c>Severity</c> members + fires <c>OnAlarmEvent</c> on
/// state transitions. Default <c>false</c> — operators explicitly opt in because
/// projection semantics don't exactly mirror Rockwell FT Alarm &amp; Events; shops
/// running FT Live should keep this off + take alarms through the native route.
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs
+94 -50
View File
@@ -6,7 +6,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip;
/// <summary>
/// Projects AB Logix ALMD (digital) and ALMA (analog, <c>ALARM_ANALOG</c>) alarm instructions onto the OPC UA
/// alarm surface by polling the ALMD UDT's <c>InFaulted</c> / <c>Acked</c> / <c>Severity</c> members (or the
/// alarm surface by polling the ALMD UDT's <c>InAlarm</c> / <c>Severity</c> members (or the
/// ALMA limit/ack/severity members) at a
/// configurable interval + translating state transitions into <c>OnAlarmEvent</c>
/// callbacks on the owning <see cref="AbCipDriver"/>. Feature-flagged off by default via
@@ -160,8 +160,14 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
// failures don't poison the batch. Swallow the return so a single faulted ack
// doesn't bubble out of the caller's batch expectation.
#pragma warning disable OTOPCUA0001 // Driver-INTERNAL self-call: the projection is owned by AbCipDriver; CapabilityInvoker wraps the driver from the dispatch layer OUTWARD, so a driver's own internal ack path must NOT re-wrap. Intentional, not a dispatch gap.
_ = await _driver.WriteAsync(requests, cancellationToken).ConfigureAwait(false);
var written = await _driver.WriteAsync(requests, cancellationToken).ConfigureAwait(false);
#pragma warning restore OTOPCUA0001
// An ALMA ref whose OperAckAll member is not declared (or an ALMA-looking user UDT) fails here as
// BadNodeIdUnknown; surface it rather than letting the ack vanish.
for (var i = 0; i < written.Count && i < requests.Length; i++)
if (written[i].StatusCode != AbCipStatusMapper.Good)
_logger.LogWarning("AbCip alarm acknowledge write to {Reference} failed with status 0x{Status:X8}",
requests[i].FullReference, written[i].StatusCode);
}
/// <summary>Releases all resources associated with this alarm projection.</summary>
@@ -179,7 +185,7 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
}
/// <summary>
/// Poll-tick body — reads <c>InFaulted</c> + <c>Severity</c> for every source node id
/// Poll-tick body — reads <c>InAlarm</c> + <c>Severity</c> for every source node id
/// in the subscription, diffs each against last-seen state, fires raise/clear events.
/// Extracted so tests can drive one tick without standing up the Task.Run loop.
/// </summary>
@@ -187,7 +193,7 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
/// <param name="results">The data values read from the subscription source nodes.</param>
internal void Tick(Subscription sub, IReadOnlyList<DataValueSnapshot> results)
{
// results index layout: per sourceNode, ALMD = [InFaulted, Severity]; ALMA = the plan's member list.
// results index layout: per sourceNode, ALMD = [InAlarm, Severity]; ALMA = the plan's member list.
var cursor = 0;
for (var i = 0; i < sub.SourceNodeIds.Count; i++)
{
@@ -199,16 +205,16 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
cursor += plan.Alma.RefCount;
continue;
}
var inFaultedDv = results[cursor];
var inAlarmDv = results[cursor];
var severityDv = results[cursor + 1];
cursor += 2;
if (inFaultedDv.StatusCode != AbCipStatusMapper.Good) continue;
if (inAlarmDv.StatusCode != AbCipStatusMapper.Good) continue;
var nowFaulted = ToBool(inFaultedDv.Value);
var nowFaulted = ToBool(inAlarmDv.Value);
// severityDv.StatusCode is not checked here. When the Severity read is Bad (value null),
// ToInt(null) returns 0 and MapSeverity buckets it as Low. This is acceptable because
// InFaulted and Severity are members of the same ALMD UDT read in one batch, so a Good
// InFaulted almost always implies a Good Severity. The "unknown severity → Low" fallback
// InAlarm and Severity are members of the same ALMD UDT read in one batch, so a Good
// InAlarm almost always implies a Good Severity. The "unknown severity → Low" fallback
// is intentional and matches the documented behaviour.
var severity = ToInt(severityDv.Value);
@@ -248,7 +254,10 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
{
var nodeId = sub.SourceNodeIds[i];
if (sub.Plans[i].Alma is { } alma) { refs.AddRange(alma.Refs(nodeId)); continue; }
refs.Add($"{nodeId}.InFaulted");
// InAlarm is ALMD's alarm-active output (Rockwell ALMD reference). InFaulted (Status.1) is the INPUT
// health flag — it is not the alarm. Until hw4 this polled InFaulted, so a live ALMD would have raised on
// input faults and never on the alarm condition.
refs.Add($"{nodeId}.InAlarm");
refs.Add($"{nodeId}.Severity");
}
@@ -291,70 +300,96 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
/// <summary>
/// ALMA tick for one node. Active = any polled limit bit set; the condition's severity/message come from the
/// MOST SEVERE active limit (ties: HH, LL, H, L, ROC+, ROC-). Transitions: none-&gt;active Raise (or
/// ActiveAcknowledged when every active limit is already acked), active-&gt;none Clear, a NEWLY entered limit
/// while active Retrigger (a new, unacknowledged occurrence), a pure shrink/severity change an
/// Unspecified-kind update (state kept), and an all-acked flip while active an Acknowledge.
/// MOST SEVERE active limit (ties: HH, LL, H, L, ROC+, ROC-). What is told to the host:
/// <list type="bullet">
/// <item>none-&gt;active: Raise, or the atomic ActiveAcknowledged when every declared limit is acked.</item>
/// <item>active-&gt;none: Clear.</item>
/// <item>a limit NEWLY entering: Retrigger (a new unacknowledged occurrence) — or ActiveAcknowledged when
/// nothing is pending ack (AckRequired false), never Retrigger-then-Acknowledge (#656).</item>
/// <item>no new limit, every declared limit now acked while the condition was unacked: Acknowledge.</item>
/// <item>no new limit, the governing limit or its severity changed (a drop-out): an
/// <c>Unspecified</c> + <c>IsRestatement</c> state update — the projector keeps active/acked and writes
/// NO alerts row; anything else here would render a false "Activated".</item>
/// </list>
/// "Acked" is judged over EVERY declared limit's <c>*Acked</c> bit, active or not: in Logix an occurrence that
/// went back to normal unacknowledged keeps its bit false, so it must keep the condition unacknowledged
/// rather than being silently acked by a surviving, already-acked limit. A Bad ack/severity/limit read judges
/// nothing it could corrupt (the tick is skipped, or the ack state carried forward).
/// </summary>
private void TickAlma(Subscription sub, string nodeId, AlmaPlan plan, IReadOnlyList<DataValueSnapshot> results, int at)
{
var mask = 0;
var ackedAll = true;
var ackKnown = true;
AbCipAlmaLimit? top = null;
var topSev = int.MinValue;
for (var k = 0; k < plan.Limits.Count; k++)
{
var lim = plan.Limits[k];
var active = results[at + plan.InAlarmIndex(k)];
// A Bad limit bit leaves the whole node unjudged this tick (matches ALMD skipping a Bad InFaulted).
// A Bad limit bit leaves the whole node unjudged this tick (matches ALMD skipping a Bad InAlarm).
if (active.StatusCode != AbCipStatusMapper.Good) return;
if (!ToBool(active.Value)) continue;
var lim = plan.Limits[k];
mask |= 1 << (int)lim;
if (plan.AckedIndex(k) is { } ai)
var sev = 1;
if (plan.SeverityIndex(k) is { } si)
{
var acked = results[at + ai];
if (acked.StatusCode != AbCipStatusMapper.Good) ackKnown = false;
else if (!ToBool(acked.Value)) ackedAll = false;
var sevDv = results[at + si];
// A Bad severity must not change the told message/severity: skip the tick.
if (sevDv.StatusCode != AbCipStatusMapper.Good) return;
sev = Math.Clamp(ToInt(sevDv.Value), 1, 1000);
}
else ackKnown = false;
var sev = plan.SeverityIndex(k) is { } si ? Math.Clamp(ToInt(results[at + si].Value), 1, 1000) : 1;
if (sev > topSev) { topSev = sev; top = lim; }
}
var nowAcked = mask != 0 && ackKnown && ackedAll;
var (wasMask, wasAcked) = _driver.AlarmAnalogRecord.Exchange(nodeId, mask, nowAcked);
if (mask == 0 && wasMask == 0) return;
// Ack state over every declared *Acked bit. Unknown (none declared, or any read Bad) ⇒ carry the told value.
var ackKnown = plan.HasAcked;
var allAcked = true;
for (var k = 0; k < plan.Limits.Count && ackKnown; k++)
{
if (plan.AckedIndex(k) is not { } ai) continue;
var dv = results[at + ai];
if (dv.StatusCode != AbCipStatusMapper.Good) ackKnown = false;
else if (!ToBool(dv.Value)) allAcked = false;
}
AlarmEventArgs Build(AlarmTransitionKind kind, string message, int sev) => new(
var prior = _driver.AlarmAnalogRecord.Get(nodeId);
var nowAcked = mask != 0 && (ackKnown ? allAcked : prior.Acked);
var topId = top is { } t0 ? (int)t0 : -1;
if (mask == 0 && prior.Mask == 0) { _driver.AlarmAnalogRecord.Set(nodeId, 0, false, -1, 0); return; }
AlarmEventArgs Build(AlarmTransitionKind kind, string message, int sev, bool restatement = false) => new(
sub.Handle, nodeId, ConditionId: nodeId, AlarmType: "ALMA", Message: message,
Severity: MapSeverity(sev), SourceTimestampUtc: DateTime.UtcNow, Kind: kind, OpcUaSeverity: sev);
Severity: MapSeverity(sev), SourceTimestampUtc: DateTime.UtcNow, Kind: kind, OpcUaSeverity: sev,
IsRestatement: restatement);
if (mask == 0)
{
_driver.AlarmAnalogRecord.Set(nodeId, 0, false, -1, 0);
_driver.InvokeAlarmEvent(Build(AlarmTransitionKind.Clear, $"ALMA {nodeId} cleared", 1));
return;
}
var message = $"ALMA {nodeId} {top!.Value.Label()} alarm";
if (wasMask == 0)
if (prior.Mask == 0)
{
_driver.AlarmAnalogRecord.Set(nodeId, mask, nowAcked, topId, topSev);
_driver.InvokeAlarmEvent(Build(
nowAcked ? AlarmTransitionKind.ActiveAcknowledged : AlarmTransitionKind.Raise, message, topSev));
return;
}
if ((mask & ~wasMask) != 0)
if ((mask & ~prior.Mask) != 0)
{
_driver.InvokeAlarmEvent(Build(AlarmTransitionKind.Retrigger, message, topSev));
if (nowAcked) _driver.InvokeAlarmEvent(Build(AlarmTransitionKind.Acknowledge, message, topSev));
_driver.AlarmAnalogRecord.Set(nodeId, mask, nowAcked, topId, topSev);
_driver.InvokeAlarmEvent(Build(
nowAcked ? AlarmTransitionKind.ActiveAcknowledged : AlarmTransitionKind.Retrigger, message, topSev));
return;
}
if (mask != wasMask)
_driver.InvokeAlarmEvent(Build(AlarmTransitionKind.Unspecified, message, topSev));
if (nowAcked && !wasAcked)
_driver.AlarmAnalogRecord.Set(nodeId, mask, nowAcked, topId, topSev);
if (nowAcked && !prior.Acked)
_driver.InvokeAlarmEvent(Build(AlarmTransitionKind.Acknowledge, message, topSev));
else if (topId != prior.Top || topSev != prior.TopSeverity)
_driver.InvokeAlarmEvent(Build(AlarmTransitionKind.Unspecified, message, topSev, restatement: true));
}
/// <summary>Maps a raw severity value to an <see cref="AlarmSeverity"/> enum value.</summary>
@@ -401,6 +436,9 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
public int RefCount { get; private set; }
/// <summary>True when the tag declares at least one <c>*Acked</c> member.</summary>
public bool HasAcked => _acked.Any(a => a is not null);
public static AlmaPlan From(AbCipTagDefinition def)
{
var names = def.Members!.Select(m => m.Name).ToHashSet(StringComparer.OrdinalIgnoreCase);
@@ -460,7 +498,7 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
}
/// <summary>
/// Gitea #641 review — the last <c>InFaulted</c> the ALMD projection told the server, per alarm RawPath. Owned by
/// Gitea #641 review — the last <c>InAlarm</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.
@@ -529,18 +567,21 @@ internal static class AbCipAlmaLimitExtensions
/// </summary>
internal sealed class AbCipAlarmAnalogRecord
{
private readonly Dictionary<string, (int Mask, bool Acked)> _last = new(StringComparer.Ordinal);
internal readonly record struct State(int Mask, bool Acked, int Top, int TopSeverity);
private readonly Dictionary<string, State> _last = new(StringComparer.Ordinal);
private readonly Lock _lock = new();
/// <summary>Records the state just read, returning the prior one (<c>(0, false)</c> when none).</summary>
public (int Mask, bool Acked) Exchange(string nodeId, int mask, bool acked)
/// <summary>The state last told for <paramref name="nodeId"/> (all-zero when none).</summary>
public State Get(string nodeId)
{
lock (_lock)
{
var was = _last.GetValueOrDefault(nodeId, (0, false));
_last[nodeId] = (mask, acked);
return was;
}
lock (_lock) return _last.GetValueOrDefault(nodeId, new State(0, false, -1, 0));
}
/// <summary>Records the state just told.</summary>
public void Set(string nodeId, int mask, bool acked, int top, int topSeverity)
{
lock (_lock) _last[nodeId] = new State(mask, acked, top, topSeverity);
}
/// <summary>Forgets every ref not in <paramref name="nodeIds"/>.</summary>
@@ -570,7 +611,7 @@ public static class AbCipAlarmDetector
{
/// <summary>
/// <c>true</c> when <paramref name="tag"/> is a Structure whose declared members match
/// the ALMD signature (<c>InFaulted</c> + <c>Acked</c> present). ALMA is detected by
/// the ALMD signature (<c>InAlarm</c> + <c>Acked</c> present). ALMA is detected by
/// <see cref="IsAlma"/>.
/// </summary>
/// <param name="tag">The tag definition to check for ALMD signature.</param>
@@ -579,13 +620,13 @@ public static class AbCipAlarmDetector
{
if (tag.DataType != AbCipDataType.Structure || tag.Members is null) return false;
var names = tag.Members.Select(m => m.Name).ToHashSet(StringComparer.OrdinalIgnoreCase);
return names.Contains("InFaulted") && names.Contains("Acked");
return names.Contains("InAlarm") && names.Contains("Acked");
}
/// <summary>
/// <c>true</c> when <paramref name="tag"/> is a Structure whose declared members include at least one ALMA
/// per-limit status bit (<c>HHInAlarm</c>/<c>HInAlarm</c>/<c>LInAlarm</c>/<c>LLInAlarm</c>/
/// <c>ROCPosInAlarm</c>/<c>ROCNegInAlarm</c>). Checked BEFORE <see cref="IsAlmd"/> by the projection — an
/// <c>ROCPosInAlarm</c>/<c>ROCNegInAlarm</c>) and no plain <c>Acked</c> (an ALMD member). Checked BEFORE <see cref="IsAlmd"/> by the projection — an
/// ALMA also exposes <c>InFaulted</c>, and the limit bits are what ALMD does not have.
/// </summary>
/// <param name="tag">The tag definition to check.</param>
@@ -594,6 +635,9 @@ public static class AbCipAlarmDetector
{
if (tag.DataType != AbCipDataType.Structure || tag.Members is null) return false;
var names = tag.Members.Select(m => m.Name).ToHashSet(StringComparer.OrdinalIgnoreCase);
return Enum.GetValues<AbCipAlmaLimit>().Any(l => names.Contains(l.InAlarmMember()));
// ALMA-specific: a per-limit *InAlarm bit, and NOT ALMD's plain Acked (ALMA has only per-limit *Acked). A user
// UDT that is ALMD-shaped (plain InAlarm/Acked) stays ALMD even if it also names an "HInAlarm".
return !names.Contains("Acked")
&& Enum.GetValues<AbCipAlmaLimit>().Any(l => names.Contains(l.InAlarmMember()));
}
}
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs
+1 -1
View File
@@ -119,7 +119,7 @@ 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
/// <summary>Gitea #641 review — the ALMD projection's last-told InAlarm per alarm ref; outlives every
/// projection and subscription (see <see cref="AbCipAlarmFaultRecord"/>).</summary>
internal AbCipAlarmFaultRecord AlarmFaultRecord { get; } = new();