bug(alarms): condition events emit null ConditionClassId/ConditionClassName #475

New Issue

2026-07-17T00:37:44-04:00

dohertj2 commented

2026-07-17 00:37:44 -04:00

Summary

Sibling of #473, found by the high-effort review on PR #474, in the same function and via the exact same mechanism.

MaterialiseAlarmCondition (src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs) never assigns the mandatory Part 9 ConditionType fields ConditionClassId / ConditionClassName, so they arrive unset on every condition event — native and scripted.

#473 fixed the three BaseEventType identity fields (EventType/SourceNode/SourceName) but deliberately scoped itself to those; these two were left as-is.

Evidence

ConditionClassId and ConditionClassName are assigned nowhere in src/ or tests/:

$ grep -rn "ConditionClassId\|ConditionClassName" --include="*.cs" src/ tests/
src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IAlarmSource.cs:79:  ... Maps to OPC UA ConditionClassName downstream when ...

The single hit is an aspirational doc comment — the mapping it describes is not implemented.

An empirical probe against the real materialise path (during the PR #474 review) dumped the children Create() builds:

PROBE ConditionClassId   child null? False   value: i=0        (NodeId.Null)
PROBE ConditionClassName child null? False   value: <empty>

Same mechanism as #473: Create() builds the mandatory children from the type's embedded definition but leaves .Value unset, and nothing downstream synthesises them (ReportEvent / InstanceStateSnapshot copy children verbatim).

Impact

An alarm HMI or Part 9 client that selects ConditionClassId to bucket alarms into Process / Maintenance / System — the standard OPC UA categorization — gets null for every OtOpcUa alarm and drops them all into an unclassified bucket.

Part 9 requires a server that does not model condition classes to still report BaseConditionClassType rather than null. The SDK's own AlarmConditionServer sample assigns alarm.ConditionClassId.Value = ObjectTypeIds.ProcessConditionClassType.

Suggested fix

In MaterialiseAlarmCondition, alongside the #473 assignments:

alarm.ConditionClassId.Value   = ObjectTypeIds.BaseConditionClassType;
alarm.ConditionClassName.Value = new LocalizedText("BaseConditionClass");

Worth deciding deliberately (why this was not folded into #473):

Which class? BaseConditionClassType is the safe spec-conformant default. ProcessConditionClassType is arguably truer for device alarms — but it is a wire contract clients bucket on, so it deserves an explicit call rather than a default picked in passing.
Should the driver's alarm category drive it? IAlarmSource.cs:79 already claims the alarm's category (Process / Maintenance / System / Diagnostics) "maps to OPC UA ConditionClassName downstream". Wiring that through would make the doc comment true, but it needs a category -> ConditionClass mapping and a way to carry the category to the materialise seam (it is not on the composition today).

Why this survived

Same reason as #473: no test asserts these field values. The new wire-level NativeAlarmEventIdentityFieldDeliveryTests (PR #474) selects only the six BaseEventType fields, so it would not catch these either.

A fix should extend that wire-level test's select clause to assert ConditionClassId / ConditionClassName are populated on a live condition event.

References

#473 — the BaseEventType identity fields (same function, same mechanism).
PR #474 — the #473 fix; its review surfaced this.

## Summary Sibling of #473, found by the high-effort review on PR #474, in the **same function** and via the **exact same mechanism**. `MaterialiseAlarmCondition` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs`) never assigns the mandatory Part 9 `ConditionType` fields `ConditionClassId` / `ConditionClassName`, so they arrive unset on every condition event — native and scripted. #473 fixed the three `BaseEventType` identity fields (`EventType`/`SourceNode`/`SourceName`) but deliberately scoped itself to those; these two were left as-is. ## Evidence `ConditionClassId` and `ConditionClassName` are assigned **nowhere** in `src/` or `tests/`: ``` $ grep -rn "ConditionClassId\|ConditionClassName" --include="*.cs" src/ tests/ src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IAlarmSource.cs:79: ... Maps to OPC UA ConditionClassName downstream when ... ``` The single hit is an **aspirational doc comment** — the mapping it describes is not implemented. An empirical probe against the real materialise path (during the PR #474 review) dumped the children `Create()` builds: ``` PROBE ConditionClassId child null? False value: i=0 (NodeId.Null) PROBE ConditionClassName child null? False value: <empty> ``` Same mechanism as #473: `Create()` builds the mandatory children from the type's embedded definition but leaves `.Value` unset, and nothing downstream synthesises them (`ReportEvent` / `InstanceStateSnapshot` copy children verbatim). ## Impact An alarm HMI or Part 9 client that selects `ConditionClassId` to bucket alarms into Process / Maintenance / System — the standard OPC UA categorization — gets null for every OtOpcUa alarm and drops them all into an unclassified bucket. Part 9 requires a server that does not model condition classes to still report `BaseConditionClassType` rather than null. The SDK's own `AlarmConditionServer` sample assigns `alarm.ConditionClassId.Value = ObjectTypeIds.ProcessConditionClassType`. ## Suggested fix In `MaterialiseAlarmCondition`, alongside the #473 assignments: ```csharp alarm.ConditionClassId.Value = ObjectTypeIds.BaseConditionClassType; alarm.ConditionClassName.Value = new LocalizedText("BaseConditionClass"); ``` **Worth deciding deliberately** (why this was not folded into #473): 1. **Which class?** `BaseConditionClassType` is the safe spec-conformant default. `ProcessConditionClassType` is arguably truer for device alarms — but it is a wire contract clients bucket on, so it deserves an explicit call rather than a default picked in passing. 2. **Should the driver's alarm category drive it?** `IAlarmSource.cs:79` already claims the alarm's category (`Process` / `Maintenance` / `System` / `Diagnostics`) "maps to OPC UA `ConditionClassName` downstream". Wiring that through would make the doc comment true, but it needs a category -> ConditionClass mapping and a way to carry the category to the materialise seam (it is not on the composition today). ## Why this survived Same reason as #473: no test asserts these field values. The new wire-level `NativeAlarmEventIdentityFieldDeliveryTests` (PR #474) selects only the six BaseEventType fields, so it would not catch these either. A fix should extend that wire-level test's select clause to assert `ConditionClassId` / `ConditionClassName` are populated on a live condition event. ## References - #473 — the BaseEventType identity fields (same function, same mechanism). - PR #474 — the #473 fix; its review surfaced this.

dohertj2 referenced this issue from a commit

2026-07-17 00:49:46 -04:00

fix(alarms): populate ConditionClassId/ConditionClassName on conditions (#475)

dohertj2 referenced a pull request that will close this issue

2026-07-17 00:50:20 -04:00

fix(alarms): populate ConditionClassId/ConditionClassName on condition events (#475) #476

dohertj2 referenced this issue

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

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

Sign in to join this conversation.