fix(alarms): native conditions support Acknowledge/AddComment only; report only real method changes (#587 review)

Review of 167a48bf (FIX-FIRST): making native Confirmed/Shelving method-owned produced over-the-wire
regressions — a native TimedShelve never expired, a native OneShotShelve was silently undone by
SetActiveState, an OPC UA ack left a native alarm Unconfirmed forever (ScadaBridge maps ConfirmedState),
and any local state was per-node, never reaching the redundant peer.

- Reverted: the driver projection owns native Confirmed/Shelving again.
- Native Confirm / OneShotShelve / TimedShelve / Unshelve return BadNotSupported with no event, as native
  Enable/Disable already did. Scripted alarms keep every method.
- A native Acknowledge re-asserts ConfirmedState=true and the projection's Retain rule (Active || !Acked)
  before its single report. The native condition starts Confirmed.
- The method path reports only when the state actually moved (before/after). A re-ack is silent and
  restores the Confirmed/Retain that the SDK flips, so a scripted node never drifts from its engine.
  AddComment always reports.
- Docs: native alarms support Acknowledge/AddComment only; the Secondary-node native-ack flap is documented.

Tests: refused native methods (5), native ack stays Confirmed through echo/clear/reactivate, scripted
TimedShelve expires with exactly one unshelve event, re-ack silent (native + scripted). Each fix falsified.

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-08 01:19:38 -04:00
parent 167a48bffa
commit e2663643dd
5 changed files with 334 additions and 93 deletions
+40 -16
View File
@@ -314,9 +314,30 @@ from the scripted `AlarmCommandRouter`:
OPC UA client (mirrors the Galaxy write-outcome limitation; the local
`AlarmConditionState` SDK update already committed at step 1).
Only the **Acknowledge** is routed to the driver. `Confirm` / `AddComment`
/ `Shelve` operations on a native condition stay on the scripted
`AlarmCommandRouter` path (Phase 3 scope is Acknowledge → AVEVA only).
Only the **Acknowledge** is routed to the driver. **A native condition
supports Acknowledge and AddComment only** (Gitea #587 review): `Confirm`,
`OneShotShelve` / `TimedShelve` / `Unshelve` and `Enable` / `Disable` return
`BadNotSupported` and report no event. There is no upstream path for them and
no pair replication of a local state, and the driver projection
(`NativeAlarmProjector`: always `Confirmed`, `Unshelved`) would silently undo
one at the next transition. `AddComment` stays on the scripted
`AlarmCommandRouter` path (Phase 3 scope is Acknowledge → AVEVA only) and is
applied locally.
A native condition is **never Unconfirmed**: it starts Confirmed, and because
the SDK's own Acknowledge sets `ConfirmedState = false` on any condition type
that supports Confirm, the node manager re-asserts `Confirmed = true` (and the
projection's Retain rule, `Active || !Acked`) after the SDK handler, before the
one ack event. ScadaBridge maps `ConfirmedState` into its alarm state, so this
is client-visible.
> **Secondary-node ack flap.** On a node that is not the redundancy Primary,
> an OPC UA Acknowledge is applied to the local condition and reported (one
> event, Acked), but `DriverHostActor`'s Primary gate drops the route upstream.
> The driver never acknowledges, so its next transition for that alarm is
> projected with `Acked = false` and the node reverts — with its own event. A
> client acking against the Secondary sees acked → unacked. Ack against the
> Primary (the higher ServiceLevel) for the ack to stick.
> **Best-effort during reconnect.** If a native ack arrives while the owning
> driver is `Connecting`/`Reconnecting` (or on a non-Primary node), it is
@@ -399,19 +420,22 @@ reports nothing.
and suppresses. The whole method call runs under the node-manager lock, so
a fast re-projection cannot slip in between the SDK applying the state and
the report.
- **Native conditions: Confirmed and Shelving are method-owned.**
`NativeAlarmProjector` models neither (it always says `Confirmed = true`,
`Unshelved`), while the SDK's Acknowledge sets `Confirmed = false`. So for a
native condition `WriteAlarmCondition` neither projects nor compares those
two fields — otherwise the driver's echo of an ack would report a second
event, and the next driver transition would undo (and report) a local
shelve. A native condition starts Confirmed; Confirm and the shelve methods
are still not routed to the driver (local only, as before), but they now
stick. Scripted conditions are unchanged: the engine models both fields.
- **TimedShelve works.** `Create()` left `MaxTimeShelved` at 0, and the SDK
rejects any shelving time above it, so every client TimedShelve failed with
`BadShelvingTimeOutOfRange`. It is now `int.MaxValue` ms (≈24.8 days — the
longest the SDK's own shelving timer can hold).
- **Only a real change is reported.** The condition's state is compared
before and after the SDK handler; a call that changed nothing reports
nothing. The SDK accepts acknowledging an acknowledged condition and sets
`Confirmed = false` again, so a re-ack also restores the pre-call Confirmed
and Retain — silent, and a scripted node never drifts from its engine.
AddComment always reports (the comment is the change).
- **Native conditions: Acknowledge and AddComment only** — see §"Native alarm
acknowledge → AVEVA" above. The other methods are refused
(`BadNotSupported`, no event), and a native ack keeps the condition
Confirmed.
- **TimedShelve works (scripted).** `Create()` left `MaxTimeShelved` at 0, and
the SDK rejects any shelving time above it, so every client TimedShelve
failed with `BadShelvingTimeOutOfRange`. It is now `int.MaxValue` ms (≈24.8
days). The node applies the shelve with no SDK timer of its own; the engine
owns the expiry and its Unshelved re-projection reports the one expiry
event.
- Method **audit** events (`AuditConditionAcknowledgeEventType`, …) are still
the SDK's, delivered only while the server is auditing over an encrypted
channel.