feat(alarms): condition Quality tracks source connectivity (#477)
v2-ci / build (pull_request) Successful in 4m49s
v2-ci / unit-tests (pull_request) Failing after 3h11m25s

Part 9 ConditionType.Quality was never assigned; default(StatusCode)==Good
so every native + scripted condition reported Good unconditionally — a
comms-lost device still showed a healthy, inactive, Good condition (a
wrong-VALUE bug, distinct from the null-value #473/#475). Clients (and HMIs
bucketing on IsGood) could not tell "genuinely inactive" from "lost contact".

Layer 1 — make Quality a real, plumbed field:
- AlarmConditionSnapshot gains OpcUaQuality Quality (default Good).
- MaterialiseAlarmCondition sets it (native BadWaitingForInitialData, scripted Good).
- WriteAlarmCondition projects snapshot.Quality; the delta-gate gains a Quality
  member so a quality-bucket change fires a Part 9 event.

Layer 2 — drive native quality from driver connectivity (a comms-lost driver
emits no alarm transitions, and an alarm-bearing raw tag has no value variable,
so quality can't come from either existing channel):
- DriverInstanceActor Tells parent ConnectivityChanged on Connected/Reconnecting.
- DriverHostActor fans it to every native condition the driver owns as
  OpcUaPublishActor.AlarmQualityUpdate (Good on connect, Bad on disconnect).
- New dedicated IOpcUaAddressSpaceSink.WriteAlarmQuality sets ONLY Quality and
  fires only on a bucket change — never touches Active/Acked/Retain (an active
  alarm that loses comms stays active). Not a full-snapshot re-projection, so it
  can't clobber severity/message and works for a never-fired condition.
  Forwarded through DeferredAddressSpaceSink (F10b trap; auto-verified by the
  reflection forwarding guard). Ungated by redundancy role; no /alerts row.

Scripted conditions stay Good; worst-of-input quality deferred to #478 (Layer 3).

Tests: node-level (materialise/project/no-clobber/unknown-node no-op),
NativeAlarmProjector, DriverInstanceActor connectivity emission, DriverHostActor
fan-out, OpcUaPublishActor routing, and the wire-level guard
(Condition_event_Quality_tracks_source_connectivity_on_the_wire) — RED-verified
against a simulated pre-fix always-Good server. Existing DriverInstanceActor
parent probes ignore the new ConnectivityChanged.

Docs: docs/AlarmTracking.md §"Condition source-data Quality (#477)";
design doc docs/plans/2026-07-17-alarm-condition-quality-477-design.md.
This commit is contained in:
Joseph Doherty
2026-07-17 15:10:04 -04:00
parent f6a3c31b60
commit db751d12a5
30 changed files with 752 additions and 7 deletions
+38
View File
@@ -68,6 +68,7 @@ are set explicitly. Leaving them unset shipped them as **null** on every event
| `ConditionName` | the leaf / display name (e.g. `HR200`) | Where the short human-readable name lives |
| `ConditionClassId` | always **`BaseConditionClassType`** | Part 9's "no condition class modelled" value. Unset shipped `NodeId.Null` (#475) |
| `ConditionClassName` | always **`"BaseConditionClass"`** | Matches `ConditionClassId`. Unset shipped empty text (#475) |
| `Quality` | the condition's **source-data quality** — tracks the native source's connectivity (`Good` / `Bad`) | A pure annotation; never alters Active/Acked/Retain. Unset shipped the accidentally-Good default (#477) — see below |
**Why `BaseConditionClassType` and not `ProcessConditionClassType`.** We hold no per-alarm
classification at the materialize seam, and `ConditionClassId` is a wire contract clients bucket on.
@@ -111,6 +112,43 @@ that `ConditionClassId` / `ConditionClassName` do too. The two class fields are
> a live↔history join key. Correlate on `ConditionId` / the RawPath instead. (Pre-existing; the
> live-path fix above does not change the history path.)
### Condition source-data Quality (#477)
`ConditionType.Quality` reports the quality of the condition's source data. It was never assigned, and
because `StatusCodes.Good == 0x00000000` an unassigned `StatusCode` **is** `Good` — so every condition
reported `Good` unconditionally (a wrong *value*, not a null like #473/#475). A native alarm whose device
went offline still read `Good`, so an operator could not tell *"genuinely inactive"* from *"we have lost
contact and do not know"*.
**How it is driven now (native alarms).** An alarm-bearing raw tag materializes a condition with **no
sibling value variable**, so the value/quality path (`WriteValue`) never touches it, and a comms-lost
driver emits **no alarm transitions** (the feed goes silent). The quality therefore comes from the
**driver's connectivity**, out of band from alarm transitions:
- `DriverInstanceActor` Tells its host `ConnectivityChanged(driverInstanceId, connected)` on every
transition into `Connected` (`true`) / `Reconnecting` (`false`).
- `DriverHostActor.OnDriverConnectivityChanged` fans that out to **every** native condition the driver
owns as an `OpcUaPublishActor.AlarmQualityUpdate` (`Good` on connect, `Bad` on disconnect).
- `OtOpcUaNodeManager.WriteAlarmQuality` sets **only** the condition's `Quality` and fires a Part 9
event **only on a quality-bucket change** — it never touches Active/Acked/Severity/Retain (an active
alarm that loses comms stays active). This is a dedicated path, *not* a full-snapshot re-projection, so
it cannot clobber a condition's severity/message and works for a condition that never fired a transition.
- A freshly materialized native condition starts `BadWaitingForInitialData` (the "no driver data yet"
convention value variables use); the first `Connected` confirms it `Good`.
- The connectivity annotation is **ungated by redundancy role** (a Secondary keeps its condition quality
warm for failover) and publishes **no `/alerts` row** — driver comms health already has its own status
surface (`IDriverHealthPublisher`); a row per condition would be alarm-fatigue.
**Scripted alarms** are script-computed and always live in this scope, so they materialize `Good` and are
not connectivity-driven. Deriving a scripted condition's quality from the worst of its input tags'
qualities is a deferred follow-up (Layer 3).
Wire-level guard: `NativeAlarmEventIdentityFieldDeliveryTests.Condition_event_Quality_tracks_source_connectivity_on_the_wire`
subscribes with a `[Quality, Message]` clause (Quality is declared on `ConditionType`) and asserts a
healthy source reports `Good`, a comms-lost source reports non-`Good`, and recovery returns to `Good` — on
a real client subscription. `NodeManagerAlarmSourceFieldsTests` guards the node itself + the no-clobber /
unknown-node-no-op invariants.
## Galaxy driver path (driver-native)
Restored in PR B.2 of the epic. `GalaxyDriver` implements
@@ -0,0 +1,127 @@
# Alarm condition Quality (issue #477) — design
**Status:** implemented (L1+L2) · **Date:** 2026-07-17 · **Issue:** #477 (follow-up chain #473#475#477)
**Scope decision:** Layer 1 + Layer 2, Bad-direct, annotate-only. Layer 3 (scripted worst-of-input) deferred → **#478**.
## Problem
`AlarmConditionState.Quality` is never assigned anywhere in `src/` — neither by
`OtOpcUaNodeManager.MaterialiseAlarmCondition` nor by the `WriteAlarmCondition` transition path.
Because `StatusCodes.Good == 0x00000000`, `default(StatusCode)` **is** `Good`, so the field is
*accidentally valid* — clients parse it, but it reports **`Good` unconditionally regardless of the
backing tag's real quality**.
This is a wrong-*value* bug, not the null-value bug class of #473/#475. Part 9 defines
`ConditionType.Quality` as "the quality of the Condition's source data". OT impact: when a native
alarm's device goes offline (comms lost) the condition still reports `Quality = Good`, so an operator
(or an HMI bucketing on `IsGood`) cannot distinguish *"genuinely not active"* from *"we have lost
contact and do not know"*.
## Why it isn't a 2-line default (confirmed by code)
1. **Alarm-bearing raw tags have no value variable.** `AddressSpaceApplier` materialises a raw tag as
*either* a condition node (`tag.Alarm is not null`) *or* a value variable (`else`) — never both,
since they'd share the same `s=<RawPath>` NodeId. So `WriteValue` (the only path carrying
`OpcUaQuality`) is never invoked for an alarm node. Quality has nowhere to land today.
2. **The alarm channel is quality-blind.** `AlarmEventArgs` (driver → host) and `AlarmConditionSnapshot`
(host → SDK sink) both carry no quality field.
3. **On comms-loss the alarm feed goes silent.** `DriverInstanceActor` on `DisconnectObserved` detaches
the alarm subscription and re-enters `Reconnecting` — no transition event ever arrives to carry Bad.
So the "device offline" signal must come from **driver connectivity**, independently of alarm
transitions.
## Decisions (the issue's open questions)
| # | Question | Decision | Rationale |
|---|----------|----------|-----------|
| 1 | Does an alarm tag get quality today? | No | Confirmed above — new plumbing required. |
| 2 | Direct status code vs. policy map | **Direct Bad** on comms-loss; Good on reconnect | Matches how a value variable would read; unambiguous for `IsGood` bucketing. |
| 3 | Does Bad also suppress transitions / touch Retain? | **No — annotate only** | A comms-lost *active* condition must stay active + retained. Silently clearing an active alarm on comms-loss is the unsafe direction. Quality is a pure annotation; the Active/Ack/Retain state machine is untouched. |
| 4 | Scripted alarms: worst-of-inputs quality? | **Deferred (Layer 3)** | Scripted conditions stay `Good`. Filed as a follow-up issue. |
## Architecture — reuse the existing publish path, add no sink method
The key move: **do not add a new `IOpcUaAddressSpaceSink` method.** A new sink-interface surface would
have to be forwarded through `DeferredAddressSpaceSink` or it is inert on driver hosts (the F10b
prod-inertness trap). Instead the `NativeAlarmProjector` becomes the single owner of per-condition
state *and* quality, and a connectivity change re-projects the *last* snapshot with a swapped quality
through the **existing** `AlarmStateUpdate → OpcUaPublishActor → WriteAlarmCondition` path.
### Layer 1 — make Quality a real, plumbed field
- `AlarmConditionSnapshot` (Commons) gains `OpcUaQuality Quality` (last positional param, default
`OpcUaQuality.Good` so scripted callers and existing tests keep compiling; Commons already knows
`OpcUaQuality` via `IOpcUaAddressSpaceSink`).
- `MaterialiseAlarmCondition` sets `alarm.Quality.Value` at build time:
**native → `BadWaitingForInitialData`** (honest until connectivity confirms Good, matching the
value-variable "waiting for initial data" convention), **scripted → `Good`** (script-computed, always
live in this scope).
- `WriteAlarmCondition` projects `StatusFromQuality(state.Quality)` onto `condition.Quality.Value`
(+ `SourceTimestamp`).
- The delta-gate (`AlarmConditionDelta` / `ReadConditionDelta` / `ToConditionDelta`) gains a `Quality`
member, so a Good→Bad bucket change is a genuine delta and **fires a Part 9 condition event**.
### Layer 2 — drive native quality from driver connectivity
- `DriverInstanceActor`: new `public sealed record ConnectivityChanged(string DriverInstanceId, bool Connected)`.
`Context.Parent.Tell` it on `Become(Connected)` entry (`true`) and on the transitions into
`Reconnecting` (`DisconnectObserved` / `ForceReconnect`) (`false`). Fire-and-forget, mirrors
`DeltaApplied`.
- `NativeAlarmProjector`: per-node state becomes `(bool Active, bool Acked, OpcUaQuality Quality)`.
`Project(transition)` preserves the current quality; new `ProjectQuality(nodeId, quality)` preserves
Active/Acked and swaps only the quality, returning a full snapshot.
- `DriverHostActor`: `Receive<ConnectivityChanged>` iterates `_alarmNodeIdByDriverRef` for that driver
instance and Tells one `AlarmStateUpdate` per condition with the re-projected snapshot
(`connected ? Good : Bad`). **Ungated** — both redundancy nodes track their own driver's comms, matching
the existing "condition write stays ungated (Secondary keeps its address space warm)" rule.
**No `/alerts` row** for a quality-only change — driver health already has its own status/alerts surface
via `IDriverHealthPublisher`; a row here would be alarm-fatigue.
Scripted alarms are unaffected: they are not driver instances, receive no `ConnectivityChanged`, and
their snapshot quality stays `Good`.
## Files
**Layer 1**
- `src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/AlarmConditionSnapshot.cs`
- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs` (`MaterialiseAlarmCondition`,
`WriteAlarmCondition`, `AlarmConditionDelta`/`ReadConditionDelta`/`ToConditionDelta`)
- `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs` (`ToSnapshot` — Quality=Good, or rely on default)
**Layer 2**
- `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs`
- `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs`
- `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs`
## Tests (TDD, RED-first)
1. **Wire-level (the issue's suggested guard)** — extend `NativeAlarmEventIdentityFieldDeliveryTests`
(OpcUaServer.IntegrationTests): active alarm → event `Quality.IsGood`; driver disconnect → condition
event `Quality.IsGood == false`; reconnect → Good. Verify RED against pre-fix.
2. **Node-level**`NodeManagerAlarmSourceFieldsTests`: materialise sets Quality (native
`BadWaitingForInitialData`, scripted `Good`); `WriteAlarmCondition` projects snapshot quality and
fires on a quality-bucket change only.
3. **`NativeAlarmProjector`** unit: `ProjectQuality` keeps Active/Acked + swaps quality; `Project`
preserves quality.
4. **`DriverInstanceActor`**: `Connected` entry Tells `ConnectivityChanged(true)`; `DisconnectObserved`
Tells `ConnectivityChanged(false)`.
5. **`DriverHostActor`**: `ConnectivityChanged(false)` pushes a Bad-quality `AlarmStateUpdate` to every
condition of that driver instance.
## Deferred / notes
- **Layer 3** (scripted worst-of-input quality) → **Gitea #478**.
- **Implementation note:** L2 uses a **dedicated `IOpcUaAddressSpaceSink.WriteAlarmQuality`** path (not a
full-snapshot re-projection). Rationale: a connectivity change must set *only* Quality; re-projecting a full
snapshot would clobber a cold condition's severity/message and can't annotate a condition that never fired a
transition. The new sink method is forwarded through `DeferredAddressSpaceSink` (the F10b inertness trap) —
auto-verified by `DeferredSinkForwardingReflectionTests` (reflection guard) + its realm-discriminator guard.
- **Test-harness note:** the new `DriverInstanceActor → parent` `ConnectivityChanged` Tell polluted existing
parent-`TestProbe` assertions in 3 `DriverInstanceActor*Tests` files; those tests now
`parent.IgnoreMessages(m => m is ConnectivityChanged)` since they assert on data/alarm/discovery forwards,
not connectivity.
- `Bad_NoCommunication` vs generic `Bad`: v1 maps `OpcUaQuality.Bad → StatusCodes.Bad`; refining
`StatusFromQuality` to emit `BadNoCommunication` for the comms-loss case is a one-line nicety, noted in
the issue.
- `docs/AlarmTracking.md` §"Condition event identity fields" gains a Quality subsection (Good/Bad
semantics, annotation-not-state-change, quality-bucket change fires an event).