Skip to content

fix(common): coerce null/empty/whitespace observation Result to UNAVAILABLE - #193

Open
ottobolyos wants to merge 9 commits into
TrakHound:masterfrom
ottobolyos:fix/agent-empty-result-unavailable
Open

fix(common): coerce null/empty/whitespace observation Result to UNAVAILABLE#193
ottobolyos wants to merge 9 commits into
TrakHound:masterfrom
ottobolyos:fix/agent-empty-result-unavailable

Conversation

@ottobolyos

@ottobolyos ottobolyos commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

MTConnectAgent.AddObservation accepted observations whose Result value was null, the empty string "", or whitespace-only and forwarded that value verbatim onto every wire transport (HTTP /current, /sample, MQTT JSON-cppagent, SHDR). MTConnect Part 1 §Observation Information Model — Representation — Observation Values mandates "UNAVAILABLE" as the sole valid representation of a missing or undetermined observation value:

"If an Agent cannot determine a Valid Data Value for a DataItem, the value returned for the Result for the Data Entity MUST be reported as UNAVAILABLE."

The empty string is, by definition, not a Valid Data Value (it parses as no value at all under every typed DataItem schema). This PR adds an unconditional coerce step in the canonical AddObservation(string deviceKey, IObservationInput, …) overload — null / empty / whitespace Result is rewritten to Observation.Unavailable before validation, so:

  • Ignore / Warning / Remove: publishes UNAVAILABLE rather than the empty value, satisfying the spec mandate on the wire.
  • Strict: coerces to UNAVAILABLE, the observation lands rather than being silently dropped.

The coerce skips CONDITION observations (their Level enum cannot carry the empty pathology). DATA_SET / TABLE / TIME_SERIES Representation pre-fill of Count / SampleCount runs after the coerce — semantics for those representations are unchanged.

No opt-out flag is exposed: the spec mandate is MUST, so a way to opt out of compliance would itself be non-conformant.

Behaviour change

Input Pre-fix wire payload Post-fix wire payload
Result = null "" (empty) "UNAVAILABLE"
Result = "" "" "UNAVAILABLE"
Result = " " (whitespace) " " "UNAVAILABLE"
Result = "AVAILABLE" (concrete) "AVAILABLE" "AVAILABLE" (unchanged)
Result = "" under Strict observation silently dropped lands with "UNAVAILABLE"

Tests

Unit (NUnit, tests/MTConnect.NET-Common-Tests/Agents/AddObservationEmptyResultCoerceTests.cs, 10 cases):

  • empty-string Result under every non-Strict InputValidationLevel
  • the null / "" / whitespace family under Warning
  • empty Result under Strict (lands, not silently dropped)
  • concrete-value preservation (negative — the coerce does not substitute for a Valid Data Value)

Wire-level E2E (xUnit, [Trait("Category", "E2E")]):

tests/MTConnect.NET-Integration-Tests/Workflows/AddObservationEmptyResultUnavailableWorkflowTests.cs — HTTP /current:

  • null / empty / spaces / tab / newline / CRLF Result → AVAILABILITY element body is UNAVAILABLE (6-case [Theory])
  • concrete value → AVAILABILITY element body is verbatim
  • concrete → empty sequence → AVAILABILITY element reflects the latest (UNAVAILABLE)
  • empty Result under Strict validation → AVAILABILITY element body is UNAVAILABLE (lands, not silently dropped; spins its own agent + server harness)

tests/MTConnect.NET-Integration-Tests/Workflows/AddObservationEmptyResultUnavailableSampleStreamWorkflowTests.cs — HTTP /sample stream:

  • null / empty / spaces / tab / newline / CRLF Result → latest AVAILABILITY sample is UNAVAILABLE (6-case [Theory])
  • concrete value → latest AVAILABILITY sample is verbatim
  • concrete → empty sequence → two distinct samples in stream order (concrete first, then UNAVAILABLE); proves the coerce does not silently drop the second observation

All wire surfaces fed by the agent's buffer read the same coerced payload — every transport (HTTP, MQTT relay, SHDR adapter output) inherits the fix because the coerce sits above the buffer.

Files touched

  • libraries/MTConnect.NET-Common/Agents/MTConnectAgent.cs — invoke the coerce in AddObservation; add IsEmptyResult + CoerceEmptyResultToUnavailable helpers.
  • tests/MTConnect.NET-Common-Tests/Agents/AddObservationEmptyResultCoerceTests.cs — new unit fixture.
  • tests/MTConnect.NET-Integration-Tests/Workflows/AddObservationEmptyResultUnavailableWorkflowTests.cs — new wire-level E2E fixture against HTTP /current.
  • tests/MTConnect.NET-Integration-Tests/Workflows/AddObservationEmptyResultUnavailableSampleStreamWorkflowTests.cs — new wire-level E2E fixture against HTTP /sample stream.
  • .github/workflows/dotnet.yml — drop matrix expressions from the two matrix jobs' name: fields so skipped-state status checks render cleanly (build-and-test / route-check-e2e instead of build-and-test-${{ matrix.os }}).
  • docs/reference/configuration.md — regenerated to surface the new deviceValidationLevel config key on the docs site (IAgentConfiguration + AgentConfiguration sections). Same regenerator (docs/scripts/generate-reference.sh) that drives the docs-site drift gate.

Additional scope (from cascade rebase)

Includes maintainer commit a61a3647 Added separate DeviceValidationLevel propery and enum to handle MTConnectDevices validation:

  • New enum MTConnect.Agents.DeviceValidationLevel (Ignore / Warning / Remove / Strict) — controls how the agent reacts when MTConnectDevices shape data fails validation against the standard.
  • New AgentConfiguration.DeviceValidationLevel / IAgentConfiguration.DeviceValidationLevel property (default Warning). Serialised key: deviceValidationLevel.
  • MTConnectAgent.NormalizeDevice — the three invalid-Component / -Composition / -DataItem branches now consult DeviceValidationLevel instead of InputValidationLevel, splitting device-shape validation from observation validation. Enables InputValidationLevel = Strict (spec-conformant observations) with DeviceValidationLevel = Warning (accept non-standard device shapes).

Orthogonal to the empty-Result coerce contract above; the coerce runs unconditionally in AddObservation regardless of either level, per the Part 1 MUST.

@ottobolyos
ottobolyos force-pushed the fix/agent-empty-result-unavailable branch 3 times, most recently from 66db93a to 0a58cfb Compare June 11, 2026 16:10
@ottobolyos
ottobolyos marked this pull request as ready for review June 11, 2026 17:35
@ottobolyos
ottobolyos marked this pull request as draft June 11, 2026 17:52
@ottobolyos
ottobolyos force-pushed the fix/agent-empty-result-unavailable branch from a2c41aa to a90e23b Compare June 11, 2026 17:57
@ottobolyos
ottobolyos marked this pull request as ready for review June 11, 2026 18:03
@ottobolyos
ottobolyos marked this pull request as draft June 11, 2026 18:10
@ottobolyos
ottobolyos force-pushed the fix/agent-empty-result-unavailable branch from a90e23b to 9d1e172 Compare June 11, 2026 18:17
@ottobolyos
ottobolyos marked this pull request as ready for review June 11, 2026 18:21
@ottobolyos

Copy link
Copy Markdown
Contributor Author

@PatrickRitchie, this PR fixes a Part 1 spec violation that touches every wire output: MTConnectAgent.AddObservation was forwarding null / empty / whitespace Result values verbatim, where Part 1 §Observation Information Model — Representation — Observation Values mandates UNAVAILABLE as the sole valid representation of a missing value. The fix is small and self-contained—4 files across 4 commits, with 10 unit + 17 wire-level E2E cases pinning the contract—but the bug class is broad enough (every HTTP envelope, MQTT relay, and SHDR adapter output benefits from the coerce at the buffer entry) that a review at your convenience would be appreciated.

@ottobolyos

Copy link
Copy Markdown
Contributor Author

@PatrickRitchie — heads-up: while validating PR #189's dime-connector build I caught an unrelated multi-TFM Release-pack regression introduced by an earlier warnings sweep. The minimal fix lands in PR #194 (just opened as draft); the broader warnings cleanup plus a Release-pack CI gate ships in PR #195.

@PatrickRitchie

Copy link
Copy Markdown
Contributor

Updates

I updated the PR to allow this feature to be toggled on/off based on the 'InputValidationLevel = strict' configuration parameter. This should allow any existing implementation to still work while allowing a strict adherence to the standard when required.

I also added a separate DeviceValidationLevel configuration parameter in order to allow a 'non-standard' DataItem or Component to work with the 'strict' InputValidationLevel. This is common for end users to use a DataItem with a custom Type or SubType.

Tests

I'm not sure extactly how this affects the tests but I saw where InputValidationLevel is used a few times so that may be why it was failing.

@ottobolyos
ottobolyos marked this pull request as draft July 15, 2026 07:52
@ottobolyos
ottobolyos force-pushed the fix/agent-empty-result-unavailable branch from a61a364 to 6ab2685 Compare July 15, 2026 07:54
@ottobolyos
ottobolyos force-pushed the fix/agent-empty-result-unavailable branch from 6ab2685 to b788148 Compare July 15, 2026 20:53
@ottobolyos

Copy link
Copy Markdown
Contributor Author

@PatrickRitchie, following up on the empty-Result coerce in MTConnectAgent.AddObservation (line 2161 of libraries/MTConnect.NET-Common/Agents/MTConnectAgent.cs) — commit 8dd28c6b Updated to toggle on/off based on input validation level set to 'strict' narrowed the coerce guard to fire only when InputValidationLevel == Strict, and that introduces a real MTConnect standard violation on the other three levels.

Under Ignore, Warning, and Remove (with Warning being the default), a null / empty-string / whitespace-only Result now flows verbatim onto every wire transport — HTTP /current and /sample, MQTT JSON-cppagent, SHDR.

The Part 1 mandate this repo already recognises verbatim (see libraries/MTConnect.NET-Common/Observations/Observation.cs:20–27):

If an Agent cannot determine a Valid Data Value for a DataItem, the value returned for the Result for the Data Entity MUST be reported as UNAVAILABLE.

The empty string is by definition not a Valid Data Value under any typed DataItem schema:

  • Every MTConnectStreams_*.xsd restriction that permits UNAVAILABLE does so as an enumerated string token, never as an empty content model — see e.g. tests/Compliance/MTConnect-Compliance-Tests/Schemas/v2_2/MTConnectStreams_2.2.xsd:4567 and every enumerated Event/Sample restriction on that schema.
  • On xs:decimal-typed Samples the empty string flat-out fails XSD validation ("" is not a valid xs:decimal lexical form).
  • On Events with typed enums the empty string is not among the enumerated tokens either.
  • cppagent — the MTConnect reference implementation — coerces or rejects empty Results at ingress and never emits them on the wire.

The 8 red tests on AddObservation_NullEmptyOrWhitespaceResult_Coerced_To_Unavailable (null / "" / " " / "\t" / "\n" / "\r\n" / …) pin the pre-8dd28c6b behaviour and are what caught this. Under the current guard they all fail because the coerce is skipped.

Proposal

Restore the unconditional coerce — drop the && _configuration.InputValidationLevel == InputValidationLevel.Strict clause so the guard reads dataItem.Category != DataItemCategory.CONDITION && IsEmptyResult(input). Every level then publishes the spec-mandated UNAVAILABLE; Strict still gains the original benefit (the empty observation lands rather than being silently dropped by the validator). All 8 tests stay green as they are.

If you want the pre-coerce raw-empty behaviour preserved as an opt-in — e.g. for a downstream tool that inspects the raw payload — we can add an AgentConfiguration.CoerceEmptyResultToUnavailable boolean defaulting to true. Flipping it to false produces an agent that is not fully conformant with MTConnect Part 1, so we'd log a startup warning describing the non-conformance and gate the flip behind that warning, but the mechanism is there for downstream consumers who genuinely need it.

Happy to push the unconditional-coerce fix directly. #193 stays draft until we agree on the shape.

ottobolyos and others added 9 commits July 23, 2026 20:10
This is often used for messages and program names. SHDR should pass the value as is to the Agent and the Agent should then decide (based on validation level) whether to accept the value or not.
Although, based on the MTConnect Standard, a value should never be 'empty', an 'empty' value should be able to be accepted as many adpaters/agents don't adhere to this restriction and could cause issues with real world implementations.

An InputValidationLevel set to 'strict' should rewrite an 'empty' value as 'UNAVAILABLE' so that it strictly conforms to the standard.
…nectDevices validation. This allows a device to be validated at a different level than observations/assets
The new `DeviceValidationLevel` property + enum added on this branch adds
one config key row to `IAgentConfiguration` and `AgentConfiguration`; the
drift gate `docs/scripts/generate-reference.sh --check` reports DRIFT
until the generated `docs/reference/configuration.md` catches up. Runs
the generator to produce the current output.
Commit 8dd28c6 narrowed the empty-Result coerce guard on
`MTConnectAgent.AddObservation` from unconditional to
`InputValidationLevel == Strict`-only. On the three other levels
(`Ignore`, `Warning`, `Remove`) a null / empty-string / whitespace-only
Result then flowed verbatim onto every wire transport, contradicting the
MTConnect Part 1 mandate this repo already encodes verbatim as
`Observation.UnavailableDescription`:

    If an Agent cannot determine a Valid Data Value for a DataItem, the
    value returned for the Result for the Data Entity MUST be reported
    as UNAVAILABLE.

The empty string is not a Valid Data Value under any typed DataItem
schema — `MTConnectStreams_*.xsd` enumerates `UNAVAILABLE` as a schema
token, never as an empty content model; `xs:decimal` samples reject the
empty lexical form outright; cppagent, the reference implementation,
coerces or rejects empty Results at ingress.

Restores the pre-8dd28c6b guard: coerce fires whenever the item is
non-CONDITION and the Result is empty, regardless of validation level.
Strict still gains the original benefit — the empty observation lands
rather than being silently dropped by the validator. The 8 red tests on
`AddObservation_NullEmptyOrWhitespaceResult_Coerced_To_Unavailable` were
pinning exactly this contract and go green with the restored guard.
@ottobolyos
ottobolyos force-pushed the fix/agent-empty-result-unavailable branch from 5fc3041 to e64c5f8 Compare July 23, 2026 18:10
@ottobolyos
ottobolyos marked this pull request as ready for review July 24, 2026 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Reviewing

Development

Successfully merging this pull request may close these issues.

2 participants