bug(alarms): condition Quality is hardcoded Good regardless of the source tag's quality #477

New Issue

2026-07-17T00:54:11-04:00

dohertj2 commented

2026-07-17 00:54:11 -04:00

Summary

Found while fixing #475 (PR #476). Lower priority than #473 / #475, and a genuinely different bug class — worth being precise about why.

MaterialiseAlarmCondition never assigns AlarmConditionState.Quality, and neither does the WriteAlarmCondition transition path. Grep confirms no assignment anywhere in src/ (the .Quality hits are unrelated: Galaxy EventPump, historian sample mapping, the historization outbox).

Why this is NOT the same class as #473/#475

Those two shipped null on the wire and broke conformant clients. Quality does not:

QUALITY value=[0x00000000] type=[StatusCode] isGood=[True]

StatusCodes.Good is 0x00000000, so default(StatusCode) is Good. The field is accidentally valid — clients parse it fine. There is no null defect here, which is why it was deliberately left out of #475.

The actual defect

Every condition reports Good unconditionally, regardless of the backing tag's real quality. A native alarm condition is materialised at a raw tag; if that tag goes Bad (device offline / comms lost) or Uncertain, the condition still reports Quality = Good.

Part 9 defines ConditionType.Quality as the quality of the Condition's source data, so this is a wrong value rather than a missing one — arguably worse to detect, since nothing looks broken.

OT impact: an operator (or an HMI bucketing on Quality) cannot distinguish "this alarm is genuinely not active" from "we have lost contact with the device and have no idea whether it is active". Both read as a healthy, inactive, Good condition. That is a meaningful distinction in a plant context.

Why it is not a 2-line fix

Unlike #473/#475 — static values known at the materialise seam — Quality is dynamic: it must track the source tag's live status code, which means plumbing the driver's quality through to the condition and updating it on quality transitions (not just alarm-state transitions).

The raw tag's quality already reaches the node manager via the value path (WriteValue carries OpcUaQuality), but an alarm-bearing raw tag materialises only a condition and no sibling value variable, so the quality currently has nowhere to land. Design questions to settle:

Does an alarm-bearing raw tag get its quality delivered at all today, given it has no value variable? (Probably not — needs checking.)
Should the condition's Quality follow the source tag's status code directly, or map through a policy (e.g. Bad -> Uncertain)?
Should a Bad-quality source also affect Retain / suppress transitions, or only annotate Quality?
Scripted alarms: what is the quality of a script-computed condition — the worst of its input tags?

Suggested test

Extend NativeAlarmEventIdentityFieldDeliveryTests (the wire-level guard from #474/#476) with a case that drives the source tag Bad and asserts the condition event reports non-Good Quality.

References

#473 — the BaseEventType identity fields (null; fixed, PR #474).
#475 — ConditionClassId/ConditionClassName (null; fixed, PR #476).
This is the same function, but a wrong-value bug rather than a null-value bug, and needs a design decision rather than a default.

## Summary Found while fixing #475 (PR #476). **Lower priority than #473 / #475, and a genuinely different bug class** — worth being precise about why. `MaterialiseAlarmCondition` never assigns `AlarmConditionState.Quality`, and neither does the `WriteAlarmCondition` transition path. Grep confirms no assignment anywhere in `src/` (the `.Quality` hits are unrelated: Galaxy `EventPump`, historian sample mapping, the historization outbox). ## Why this is NOT the same class as #473/#475 Those two shipped **null** on the wire and broke conformant clients. `Quality` does not: ``` QUALITY value=[0x00000000] type=[StatusCode] isGood=[True] ``` `StatusCodes.Good` is `0x00000000`, so `default(StatusCode)` **is** `Good`. The field is *accidentally valid* — clients parse it fine. There is no null defect here, which is why it was deliberately left out of #475. ## The actual defect Every condition reports **`Good` unconditionally, regardless of the backing tag's real quality**. A native alarm condition is materialised at a raw tag; if that tag goes **Bad** (device offline / comms lost) or **Uncertain**, the condition still reports `Quality = Good`. Part 9 defines `ConditionType.Quality` as the quality of the Condition's source data, so this is a wrong value rather than a missing one — arguably worse to detect, since nothing looks broken. **OT impact:** an operator (or an HMI bucketing on Quality) cannot distinguish *"this alarm is genuinely not active"* from *"we have lost contact with the device and have no idea whether it is active"*. Both read as a healthy, inactive, Good condition. That is a meaningful distinction in a plant context. ## Why it is not a 2-line fix Unlike #473/#475 — static values known at the materialise seam — `Quality` is **dynamic**: it must track the source tag's live status code, which means plumbing the driver's quality through to the condition and updating it on quality transitions (not just alarm-state transitions). The raw tag's quality already reaches the node manager via the value path (`WriteValue` carries `OpcUaQuality`), but an alarm-bearing raw tag materialises **only a condition and no sibling value variable**, so the quality currently has nowhere to land. Design questions to settle: 1. Does an alarm-bearing raw tag get its quality delivered at all today, given it has no value variable? (Probably not — needs checking.) 2. Should the condition's `Quality` follow the source tag's status code directly, or map through a policy (e.g. Bad -> Uncertain)? 3. Should a Bad-quality source also affect `Retain` / suppress transitions, or only annotate `Quality`? 4. Scripted alarms: what is the quality of a script-computed condition — the worst of its input tags? ## Suggested test Extend `NativeAlarmEventIdentityFieldDeliveryTests` (the wire-level guard from #474/#476) with a case that drives the source tag Bad and asserts the condition event reports non-Good `Quality`. ## References - #473 — the BaseEventType identity fields (null; fixed, PR #474). - #475 — ConditionClassId/ConditionClassName (null; fixed, PR #476). - This is the same *function*, but a wrong-value bug rather than a null-value bug, and needs a design decision rather than a default.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#477