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

No driver's Bad/Uncertain status sub-code reaches an OPC UA client (3-state OpcUaQuality collapse) #519

New Issue
Closed
opened 2026-07-27 13:52:12 -04:00 by dohertj2 · 1 comment
dohertj2 commented 2026-07-27 13:52:12 -04:00
Owner
Copy Link
Copy Source

Found during the MTConnect live gate (PR #506, 2026-07-27) and confirmed by direct code inspection. Not MTConnect-specific and out of scope for that plan.

Symptom

MTConnect maps the Agent's UNAVAILABLE sentinel to BadNoCommunication (0x80310000) at MTConnectObservationIndex.cs:255. An OPC UA client reading that node gets 0x80000000 (generic StatusCodes.Bad).

Verified live: ns=2;s=mtc-1/vmc/fixture_asset_changed read 0x80000000 while carrying the Agent's exact source timestamp — which proves the publish landed and that only the status differs.

Cause — two hops

  1. DriverInstanceActor.QualityFromStatus (src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs:1032-1041) projects the driver's uint status onto the 3-state OpcUaQuality enum using only the top two severity bits:

    var severity = statusCode >> 30;
return severity switch { 0 => Good, 1 => Uncertain, _ => Bad };

    Every Bad sub-code collapses to OpcUaQuality.Bad.

  2. OtOpcUaNodeManager.StatusFromQuality (src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs:3318-3322) re-expands Bad → StatusCodes.Bad = 0x80000000.

Every message type between the two is already enum-typed (AttributeValuePublished → AttributeValueUpdate → IOpcUaAddressSpaceSink.WriteValue), so nothing in between could have preserved the sub-code.

Not a null-value artifact — there is no null/Variant special-casing on the path. A Bad status carrying a non-null value collapses identically.

It is deliberate — but the consequence looks undocumented

OpcUaQuality's own doc comment (src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs:164) says: "Real SDK has finer-grained codes; the engine actors only need this 3-state classification."

So the design decision is intentional. What appears unwritten anywhere is the client-visible consequence: no driver's Bad/Uncertain sub-code ever reaches an OPC UA client.

That interacts with recent work. #497 corrected 16 wrong status constants across 6 drivers, and StatusCodeParityTests now guards them by reflection — both valuable for internal consistency — but a client still cannot distinguish BadNoCommunication from BadTypeMismatch from BadNotSupported. Diagnostic intent survives in logs only.

One sub-code does survive: BadWaitingForInitialData (0x80320000), because it is written directly at the node (OtOpcUaNodeManager.cs:1806 / :1901) rather than routed through the quality projection.

Options

  • (a) Accept and document — state in docs/ that driver status sub-codes are log-only, so driver authors don't assume clients can act on them. Cheap and honest.
  • (b) Widen the path — carry the uint through AttributeValuePublished → AttributeValueUpdate → IOpcUaAddressSpaceSink.WriteValue → WriteValue, touching the sink interface and its three implementations (Sdk, Deferred, Null). The real fix if clients need to act on sub-codes.

Worth an explicit decision either way, since right now the constants are maintained as if they mattered end-to-end.

Found during the MTConnect live gate (PR #506, 2026-07-27) and confirmed by direct code inspection. Not MTConnect-specific and out of scope for that plan. ## Symptom MTConnect maps the Agent's `UNAVAILABLE` sentinel to `BadNoCommunication` (`0x80310000`) at `MTConnectObservationIndex.cs:255`. An OPC UA client reading that node gets **`0x80000000`** (generic `StatusCodes.Bad`). Verified live: `ns=2;s=mtc-1/vmc/fixture_asset_changed` read `0x80000000` **while carrying the Agent's exact source timestamp** — which proves the publish landed and that only the status differs. ## Cause — two hops 1. `DriverInstanceActor.QualityFromStatus` (`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs:1032-1041`) projects the driver's `uint` status onto the 3-state `OpcUaQuality` enum using only the top two severity bits: ```csharp var severity = statusCode >> 30; return severity switch { 0 => Good, 1 => Uncertain, _ => Bad }; ``` Every `Bad` sub-code collapses to `OpcUaQuality.Bad`. 2. `OtOpcUaNodeManager.StatusFromQuality` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs:3318-3322`) re-expands `Bad` → `StatusCodes.Bad` = `0x80000000`. Every message type between the two is already enum-typed (`AttributeValuePublished` → `AttributeValueUpdate` → `IOpcUaAddressSpaceSink.WriteValue`), so nothing in between could have preserved the sub-code. **Not a null-value artifact** — there is no null/Variant special-casing on the path. A `Bad` status carrying a non-null value collapses identically. ## It is deliberate — but the consequence looks undocumented `OpcUaQuality`'s own doc comment (`src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs:164`) says: *"Real SDK has finer-grained codes; the engine actors only need this 3-state classification."* So the design decision is intentional. What appears unwritten anywhere is the client-visible consequence: **no driver's `Bad`/`Uncertain` sub-code ever reaches an OPC UA client.** That interacts with recent work. #497 corrected 16 wrong status constants across 6 drivers, and `StatusCodeParityTests` now guards them by reflection — both valuable for internal consistency — but a client still cannot distinguish `BadNoCommunication` from `BadTypeMismatch` from `BadNotSupported`. Diagnostic intent survives in logs only. **One sub-code does survive:** `BadWaitingForInitialData` (`0x80320000`), because it is written directly at the node (`OtOpcUaNodeManager.cs:1806` / `:1901`) rather than routed through the quality projection. ## Options - **(a) Accept and document** — state in `docs/` that driver status sub-codes are log-only, so driver authors don't assume clients can act on them. Cheap and honest. - **(b) Widen the path** — carry the `uint` through `AttributeValuePublished` → `AttributeValueUpdate` → `IOpcUaAddressSpaceSink.WriteValue` → `WriteValue`, touching the sink interface and its three implementations (`Sdk`, `Deferred`, `Null`). The real fix if clients need to act on sub-codes. Worth an explicit decision either way, since right now the constants are maintained as if they mattered end-to-end.
dohertj2 commented 2026-07-31 05:26:43 -04:00
Author
Owner
Copy Link
Copy Source

Resolved — master 3207089f (commit 2d20aeb7). Option (b), chosen by the maintainer over "accept and document".

Everything the issue diagnosed was correct: both hops, the BadWaitingForInitialData exception, and the not-a-null-value-artifact finding all held up under source verification.

Option (b) was larger than costed, and the reason is worth recording

The issue put it at "the sink interface and its three implementations." The real surface was 41 files / ~205 references, because OpcUaQuality had producers that are not drivers and therefore had no uint to carry:

Producer Code it now names
VirtualTagActor (script failed/timed out) BadInternalError
DriverHostActor (comms lost) BadNoCommunication
AlarmConditionSnapshot (worst-of-input) the engine's WorstInputStatusCode, verbatim
AddressSpaceApplier (removed node) generic Bad — deliberately unchanged, see below

Two of those already computed a uint and threw it away: ScriptedAlarmHostActor held a pair of mutually-inverse uint→enum→uint mappers wrapped around an engine that carried the code all along. Both are deleted.

AddressSpaceApplier's four removal writes keep the generic Bad. A removed node could say something truer (BadNodeIdUnknown), but this issue was about not losing a code the driver already had; inventing a new one for the deploy path is a separate judgement call and was left out of scope.

Behaviour change worth knowing

WriteAlarmQuality now fires its Part 9 event on a status-code change, where it fired on a severity-bucket change before. While the annotation was 3-state those were the same predicate; now a move between two Bad reasons genuinely changes what a subscriber reads, so suppressing it would leave clients holding a stale reason. Documented at the sink, the message, and the node manager.

New constants + the guard

Commons.OpcUa.OpcUaStatus holds the SDK-free named constants the engine needs, since Commons and Runtime deliberately do not reference the OPC UA stack. StatusCodeParityTests is widened to cover it — with a positive control asserting the Commons rows were actually discovered, because widening a glob is worthless if the assembly does not deploy and the theory then passes silently over the driver rows alone.

Per #497's lesson: add codes there, never as an inline literal — the guard is reflection-driven and can only see a named constant.

Verification

Two tests, one per hop, each asserting two different Bad sub-codes — under the old projection both collapsed to the same value, so a single-code assertion could have passed against a hard-coded constant. Both falsified by restoring the collapse and confirming red.

One self-inflicted defect en route, caught by the suite: the bulk .Quality → .StatusCode rename also hit BrowseNames.Quality (an SDK constant) and a .Quality inside a script-source string literal. A mechanical rewrite needs its output audited by receiver, not just compiled.

Resolved — master `3207089f` (commit `2d20aeb7`). **Option (b)**, chosen by the maintainer over "accept and document". Everything the issue diagnosed was correct: both hops, the `BadWaitingForInitialData` exception, and the not-a-null-value-artifact finding all held up under source verification. ## Option (b) was larger than costed, and the reason is worth recording The issue put it at "the sink interface and its three implementations." The real surface was **41 files / ~205 references**, because `OpcUaQuality` had producers that are **not drivers** and therefore had no `uint` to carry: | Producer | Code it now names | |---|---| | `VirtualTagActor` (script failed/timed out) | `BadInternalError` | | `DriverHostActor` (comms lost) | `BadNoCommunication` | | `AlarmConditionSnapshot` (worst-of-input) | the engine's `WorstInputStatusCode`, verbatim | | `AddressSpaceApplier` (removed node) | generic `Bad` — deliberately unchanged, see below | Two of those **already computed a `uint` and threw it away**: `ScriptedAlarmHostActor` held a pair of mutually-inverse `uint`→enum→`uint` mappers wrapped around an engine that carried the code all along. Both are deleted. `AddressSpaceApplier`'s four removal writes keep the generic `Bad`. A removed node could say something truer (`BadNodeIdUnknown`), but this issue was about not **losing** a code the driver already had; inventing a new one for the deploy path is a separate judgement call and was left out of scope. ## Behaviour change worth knowing `WriteAlarmQuality` now fires its Part 9 event on a **status-code** change, where it fired on a severity-bucket change before. While the annotation was 3-state those were the same predicate; now a move between two Bad reasons genuinely changes what a subscriber reads, so suppressing it would leave clients holding a stale reason. Documented at the sink, the message, and the node manager. ## New constants + the guard `Commons.OpcUa.OpcUaStatus` holds the SDK-free named constants the engine needs, since Commons and Runtime deliberately do not reference the OPC UA stack. `StatusCodeParityTests` is widened to cover it — with a positive control asserting the Commons rows were actually discovered, because widening a glob is worthless if the assembly does not deploy and the theory then passes silently over the driver rows alone. Per #497's lesson: **add codes there, never as an inline literal** — the guard is reflection-driven and can only see a named constant. ## Verification Two tests, one per hop, each asserting **two different** Bad sub-codes — under the old projection both collapsed to the same value, so a single-code assertion could have passed against a hard-coded constant. Both falsified by restoring the collapse and confirming red. One self-inflicted defect en route, caught by the suite: the bulk `.Quality` → `.StatusCode` rename also hit `BrowseNames.Quality` (an SDK constant) and a `.Quality` inside a script-source string literal. A mechanical rewrite needs its output audited by receiver, not just compiled.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
auto-managed
queue management
 cross-driver
queue management
 driver/abcip
queue management
 driver/ablegacy
queue management
 driver/focas
queue management
 driver/mqtt
MQTT / Sparkplug B broker-subscribe driver
 driver/opcuaclient
queue management
 driver/s7
queue management
 driver/twincat
queue management
 phase/1
queue management
 phase/2
queue management
 phase/3
queue management
 phase/4
queue management
 phase/5
queue management
 phase/6
queue management
 queue/blocked
queue management
 queue/done
queue management
 queue/failed
queue management
 queue/in-progress
queue management
 queue/queued
queue management
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#519
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?