From e784545844e538497045098212e2390986a38867 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 06:08:44 -0400 Subject: [PATCH 1/9] docs(plan): MTConnect write-back design + implementation plan; MQTT P3 recorded as already shipped --- .../2026-08-05-mtconnect-writeback-design.md | 151 +++++++++++++++ docs/plans/2026-08-05-mtconnect-writeback.md | 183 ++++++++++++++++++ ...26-08-05-mtconnect-writeback.md.tasks.json | 13 ++ 3 files changed, 347 insertions(+) create mode 100644 docs/plans/2026-08-05-mtconnect-writeback-design.md create mode 100644 docs/plans/2026-08-05-mtconnect-writeback.md create mode 100644 docs/plans/2026-08-05-mtconnect-writeback.md.tasks.json diff --git a/docs/plans/2026-08-05-mtconnect-writeback-design.md b/docs/plans/2026-08-05-mtconnect-writeback-design.md new file mode 100644 index 00000000..addbcea5 --- /dev/null +++ b/docs/plans/2026-08-05-mtconnect-writeback-design.md @@ -0,0 +1,151 @@ +# MTConnect write-back + MQTT write-through record reconciliation — design + +**Status:** DRAFT 2026-08-05. + +## 1. Scope correction: the MQTT half of this request is already shipped + +The request ("MQTT P3 write-through plus its two documented browse-UI gaps, MTConnect write-back") +came from `docs/plans/2026-07-24-driver-expansion-tracking.md` §Next actions (lines 288–289) — which +is **stale**. On master today: + +| Item | Status | Evidence | +|---|---|---| +| MQTT P3 write-through (`IWritable`: plain `writeTopic` publish + Sparkplug NCMD/DCMD) | **Shipped, live-gated** — #508, `72be1922`, 2026-07-31 | `MqttDriver.cs:65-67` implements `IWritable`; `Mqtt.md:16-20` status banner | +| Browse-UI gap 1 (empty tree cannot arm a rebirth) | **Fixed** — #514, `7f53bd96` | `RawBrowseModal.razor:127-157` manual `{group}/{edgeNode}` scope input | +| Browse-UI gap 2 (slashed metric name refused at browse-commit) | **Fixed** — #509, `7f53bd96` | `RawBrowseCommitMapper.ToRawSegment` (`:107`), collision refusal (`:117`) | + +So the only MQTT work left is **documentation reconciliation** — three stale records that now +contradict the code and `Mqtt.md`'s own closure line (`:597-602`): + +1. `docs/drivers/Mqtt.md:550-556` — still prints the #514 rebirth gap as an open "Known gap". +2. `docs/drivers/Mqtt.md:586-587` — "Read-only. No writes reach the broker; every node is ViewOnly" + contradicts §Write-through (`:407-480`) in the same file. +3. `docs/plans/2026-07-24-driver-expansion-tracking.md:288-289` §Next actions still lists MQTT P3 + + the two gaps as remaining work; the P2 section (`:239-263`) still describes the gaps as + "documented not fixed". +4. `deferment.md:132-133` — "`IWritable` is absent from MTConnect, MQTT, Sql and Calculation" was + audited pre-#508 and is wrong about MQTT (and, after this branch, about MTConnect). + +## 2. MTConnect write-back — the real work + +### 2.1 What was deferred, and what mechanism to build on + +The driver shipped deliberately read-only (`MTConnectDriver.cs:63` — no `IWritable`; +`docs/plans/2026-07-15-mtconnect-driver-design.md` §3.6). The deferral named **MTConnect +Interfaces** (a request/response state-machine handshake between two agents) as the only write-back +shape and judged it misleading to model as `IWritable` — "revisit only on a concrete deployment +need". This request is that need, but Interfaces is still the wrong mechanism: it is rarely +deployed, requires adapter-side cooperation, and is a state machine rather than a value write. + +The mechanism this design uses instead is the **Agent's HTTP PUT/POST surface** — a standard +cppagent feature, off by default, enabled by `AllowPut = true` (optionally scoped by +`AllowPutFrom = `) in `agent.cfg`. With it enabled, `PUT`/`POST +{agent}/{deviceName}?{dataItemId}={value}` sets the named observation in the agent's buffer exactly +as if an adapter had reported it. The repo's own integration fixture already carries the key — +currently `AllowPut = false` with a "must not accept writes" comment +(`tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/agent.cfg:25-26`). + +### 2.2 Honest semantics — this is observation injection, not machine command + +A PUT write updates **what the agent reports**, not the machine: the value lands in the agent's +observation buffer, is visible to every agent consumer, and the next adapter/SHDR update for that +data item overwrites it. That is genuinely useful (operator-set EVENT values, external systems +feeding data items the adapter does not own, test rigs) but it must never be documented as +commanding the CNC. The docs section this branch writes states this prominently, and the feature is +**opt-in at two independent layers**: the agent (`AllowPut`) and the driver config +(`write.enabled`, default `false`). Either alone leaves today's read-only behaviour untouched. + +### 2.3 Driver design + +**Config.** `MTConnectDriverConfigDto` (+ typed `MTConnectDriverOptions`) gains a `Write` block: + +```json +"write": { "enabled": true } +``` + +Default absent/`false` — nothing changes for existing deployments. No per-tag key: writability is +derived (below), matching how the driver derives everything else from the probe model. + +**Capability.** `MTConnectDriver` adds `IWritable`. The factory already returns the concrete type +(`MTConnectDriverFactoryExtensions.cs:33-39` documents why), so capability dispatch picks it up +with no registry change. + +**`WriteAsync` shape** (mirrors `MqttDriver.cs:670-745` — never throws, batch shape preserved, +per-slot codes): + +- `write.enabled == false` → every slot `BadNotWritable` (0x803B0000). +- No live client (pre-Initialize / post-Shutdown) → `BadNotConnected`, matching `ReadAsync`'s + convention (`MTConnectDriver.cs:628-695`). +- RawPath does not resolve via `TagBinding.ToDataItemId` → `BadNodeIdUnknown`. +- Data item is CONDITION-category, or the value is null / not representable as an invariant string + → `BadNotWritable` / `BadTypeMismatch` respectively. (Condition PUTs use a different level|text + wire format and carry alarm semantics — excluded from v1.) +- HTTP call: `POST {agentBase}/{deviceName}?{dataItemId}={value}` via the agent-client seam. The + owning `deviceName` comes from a `dataItemId → deviceName` map built at bind time from the cached + probe model (falling back to `options.DeviceName`); a multi-device agent is addressed correctly + without config. +- Agent refusal (non-2xx, or an `MTConnectError` document in a 200 body) → `BadCommunicationError` + with the agent's own error text logged (never in `LastError` — type-only discipline). +- `OperationCanceledException` fills remaining slots and returns; any other exception → log warning + + `BadCommunicationError`. +- Values format with `CultureInfo.InvariantCulture`; booleans as the raw invariant string. + +**Seam extension.** `IMTConnectAgentClient` gains +`Task WriteObservationAsync(string deviceName, string dataItemId, string value, CancellationToken ct)` +(outcome = accepted / agent-error-with-text / transport-error). `MTConnectAgentClient` implements +it over `_http` (the unary client with the bounded timeout) using `HttpMethod.Post`; the stub +`HttpMessageHandler` ctor seam (`MTConnectAgentClient.cs:107`) covers response-shape unit tests +with no socket. The unit-test `CannedAgentClient` records write calls and returns scripted +outcomes. + +**Discovery stamping.** `DiscoverAsync` currently stamps every leaf +`SecurityClassification.ViewOnly` (`MTConnectDriver.cs:1040-1042`) with a comment saying that is +the only honest tier for a driver with no `IWritable`. That derivation becomes conditional: +`CanWrite(dataItem)` (= `write.enabled` && category is not CONDITION) → `Operate`, else `ViewOnly` +— the same "a node cannot advertise writability the driver would refuse" principle MQTT pinned +(`MqttDriver.cs:504`, `CanWrite` at `:625-627`). The implementer must verify how materialization +derives the `CurrentWrite` access bit (capability presence + SecurityClass) so a stamped-Operate +node is actually writable over OPC UA, and a ViewOnly node is not. + +**Doc comments to update in the same change:** the class remarks +(`MTConnectDriver.cs:46-53` "deliberately no IWritable"), the factory remark +(`MTConnectDriverFactoryExtensions.cs:39`), and `MTConnectTagDefinition.cs:10-14` ("no +corresponding write path"). + +### 2.4 Out of scope + +- **MTConnect Interfaces** — still deferred, same rationale as §3.6; this design does not touch it. +- Asset writes (`POST /asset`), per-tag write opt-outs, write retry (`isIdempotent` stays `false` + at `DriverInstanceActor.cs:776`), and PUT `time=` timestamps — YAGNI. +- Any AdminUI surface — the driver form gains nothing; `write.enabled` is authored in the existing + driver-config JSON. (No chrome gate needed for this branch.) + +### 2.5 Testing + +1. **Unit (offline, falsifiable).** New `MTConnectWriteTests`: batch contract + every refusal leg + via `CannedAgentClient` (disabled-by-default, unknown RawPath, condition category, null value, + no client, cancellation fill, agent-error outcome). Encoder-free — the write is a URL, so the + URL/verb/device-resolution assertions live in `MTConnectAgentClient` stub-handler tests + (MTConnectError-in-200 detection included). Discovery stamping test: same probe fixture browsed + with write on/off flips leaf SecurityClass, CONDITION stays ViewOnly. +2. **Integration (env-gated, docker fixture).** Flip the fixture to `AllowPut = true` (update the + comment to say writes are now in scope) and add a write leg: write via the driver → read + `/current` back → the observation carries the written value; a second leg proves an + `AllowPut = false`-style refusal maps to `BadCommunicationError` is *not* needed (we cannot run + two agent configs in one fixture) — instead a bad-dataItemId write asserts the agent-error + mapping. Remove the "(b) Writes" not-covered remark + (`MTConnectAgentIntegrationTests.cs:64-65`). +3. **Live gate (no chrome).** Against the docker-dev rig + the shared fixture agent: author an + MTConnect driver with `write.enabled`, deploy, `Client.CLI write` to a writable node, confirm + via `/current` and via OPC UA read-back. Runnable headless by the executing session. + +## 3. Bookkeeping (both halves) + +- `docs/drivers/MTConnect.md`: capability surface (`:71-77`), Deferred (`:295-301`), new + §Write-back section with the §2.2 semantics warning. +- `docs/plans/2026-07-15-mtconnect-driver-design.md` §3.6: superseded note pointing here. +- `docs/plans/2026-07-24-driver-expansion-tracking.md`: MQTT P2 gap bullets + §Next actions + rewritten to record #508/#509/#514 closure and this branch's MTConnect write-back. +- `docs/drivers/Mqtt.md`: delete stale blocks `:550-556` and `:586-587`. +- `deferment.md`: correct `:132-133` (post-branch: `IWritable` absent from Sql and Calculation + only) and the MTConnect row (`:126`). diff --git a/docs/plans/2026-08-05-mtconnect-writeback.md b/docs/plans/2026-08-05-mtconnect-writeback.md new file mode 100644 index 00000000..e73fced1 --- /dev/null +++ b/docs/plans/2026-08-05-mtconnect-writeback.md @@ -0,0 +1,183 @@ +# MTConnect write-back Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development +> (opus implementers per user instruction). + +**Goal:** Implement `IWritable` on `MTConnectDriver` via the Agent's HTTP PUT/POST surface +(opt-in at agent `AllowPut` AND driver `write.enabled`), and reconcile the stale MQTT +write-through/browse-gap records. + +**Architecture:** See `docs/plans/2026-08-05-mtconnect-writeback-design.md` (in this branch). +Write = observation injection into the agent buffer, never machine command. Batch-safe +`WriteAsync` mirroring `MqttDriver.cs:670-745`; new seam method on `IMTConnectAgentClient`; +discovery stamps `Operate` only for leaves the driver would accept. + +**Tech Stack:** .NET 10, xUnit v3 + Shouldly, existing `CannedAgentClient` / stub +`HttpMessageHandler` seams. No new packages. + +**Worktree:** `/Users/dohertj2/Desktop/OtOpcUa/.claude/worktrees/mqtt-mtconnect-write` +(branch `feature/mqtt-mtconnect-write`). All paths below relative to it. + +--- + +### Task 0: Write options block (config + parse) + +**Classification:** small +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 1 + +**Files:** +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectDriverOptions.cs` +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs` (config DTO region + ~:3257-3345 + `ParseOptions` ~:2222) +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDriverOptionsTests.cs`, + `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectFactoryTests.cs` + +TDD: failing test first — `{"write":{"enabled":true}}` parses to `Options.Write.Enabled == true`; +absent block defaults to `false`; malformed value behaves like the sibling `Probe`/`Reconnect` +blocks (match their existing tolerance/failure semantics exactly — read them first). Add +`MTConnectWriteOptions` (sealed record, `Enabled`, default false) on `MTConnectDriverOptions` +following the `Probe`/`Reconnect` pattern, a nullable `WriteDto` on the config DTO, wire through +`ParseOptions`. Commit `feat(mtconnect): parse the write.enabled config block`. + +### Task 1: Agent-client write seam + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 0 + +**Files:** +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs` +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs` +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs` (new) + +Add to the seam: + +```csharp +Task WriteObservationAsync( + string deviceName, string dataItemId, string value, CancellationToken cancellationToken); +``` + +with `MTConnectWriteOutcome` a small readonly record: `Accepted` bool + `AgentError` string? +(agent-produced MTConnectError text) — transport failures THROW (HttpRequestException etc.), the +driver maps them; agent refusals return `Accepted=false` with the text. Implementation in +`MTConnectAgentClient` over `_http` (the bounded-timeout unary client): `HttpMethod.Post` to +`{base}/{deviceName}?{Uri.EscapeDataString(dataItemId)}={Uri.EscapeDataString(value)}` built with +the same left-part logic as `BuildRequestUri` (:382-403). A 2xx whose body parses as an +`MTConnectError` document is a refusal (reuse the LocalName-only XML helpers in +`MTConnectXml.cs`); non-2xx is a refusal carrying the status; empty 2xx is accepted. + +TDD via the internal `HttpMessageHandler` ctor seam (`MTConnectAgentClient.cs:107`), the same +pattern `MTConnectDriverProbeTests.cs:22-28` uses: assert verb, exact URI (device path + escaped +query), accepted on empty 200, refusal on MTConnectError-in-200 (agent text surfaced), refusal on +404, throw on handler-thrown transport error. Commit +`feat(mtconnect): agent-client observation write (HTTP POST, AllowPut surface)`. + +### Task 2: `MTConnectDriver` implements `IWritable` + discovery stamping + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** none + +**Files:** +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs` (class decl :63, + remarks :46-53, WriteAsync new, `TagBinding` device-name map ~:3170-3213, `DiscoverAsync` + stamping :1030-1046) +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriverFactoryExtensions.cs` + (remark :33-39) +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinition.cs` + (remark :10-14) +- Modify: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/CannedAgentClient.cs` + (record writes, scripted outcomes) +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs` (new), + `MTConnectDiscoverTests.cs` (stamping cases) + +Per design §2.3. Status-code constants: follow MQTT's named-const pattern +(`MqttDriver.cs:76-89`) — `StatusBadNodeIdUnknown 0x80340000`, `StatusBadNotWritable 0x803B0000`, +`StatusBadTypeMismatch 0x80740000`, `StatusBadCommunicationError 0x80050000`, plus +`BadNotConnected` matching `ReadAsync`'s existing constant. Refusal order per slot: disabled → +BadNotWritable; no client → BadNotConnected; unresolved RawPath → BadNodeIdUnknown; CONDITION +category → BadNotWritable; null/unrepresentable value → BadTypeMismatch; then the seam call — +`Accepted=false` or thrown transport error → BadCommunicationError (log agent text at Warning, +never into `LastError`). `OperationCanceledException` fills remaining slots with +BadCommunicationError and returns. Never throw. Success = `WriteResult(0u)`. + +Device-name resolution: extend `TagBinding` (or a parallel FrozenDictionary built in +`BuildBinding` ~:2887-2980) with `dataItemId → owning deviceName` from the probe model; fall back +to `options.DeviceName`; unresolvable device → BadNodeIdUnknown. Values format via +`Convert.ToString(value, CultureInfo.InvariantCulture)`. + +Discovery: `CanWrite(dataItem)` = `options.Write.Enabled && !IsCondition(category)` → +`SecurityClassification.Operate` else `ViewOnly`; update the :1040-1042 comment. **Verify** how +materialization derives `CurrentWrite` (search for how MQTT's Operate-stamped nodes get the write +bit — SecurityClass + capability) and state the finding in the commit message; if a server-side +gate keys on the driver capability alone, note that existing ViewOnly-committed raw tags keep +their authored SecurityClass. + +TDD: every refusal leg observed red first where feasible; the batch-shape test (one bad slot +doesn't fail the batch) mirrors `MqttDriverWriteTests.cs:74`. The publish itself is proven by +Task 3 live legs, not by asserting a fake was called beyond the seam-call recording (same +philosophy as `MqttDriverWriteTests.cs:11-15`). Commit +`feat(mtconnect): IWritable via agent observation writes (#468-era deferral closed)`. + +### Task 3: Integration fixture + write legs + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 4 + +**Files:** +- Modify: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/Docker/agent.cfg` + (`AllowPut = true` + comment rewrite) +- Modify: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs` + (remove the "(b) Writes" not-covered remark :64-65; add write legs) + +Legs (env-gated by the existing `MTCONNECT_AGENT_ENDPOINT` fixture, `Assert.Skip` clean): +1. Driver with `write.enabled` writes a distinct value to a writable EVENT data item (pick one + from `Devices.xml` the SHDR adapter does not continuously overwrite — read `adapter.py` to + choose; if every item is pumped, add a static EVENT item to `Devices.xml`) → poll `/current` + until the observation carries the value (bounded wait, `StreamWait` pattern). +2. A write naming a nonexistent dataItemId → `BadCommunicationError` (agent refusal mapping). +3. `write.enabled` absent → same write returns `BadNotWritable` without touching the agent. +Commit `test(mtconnect): live write legs against the AllowPut fixture`. + +### Task 4: Docs reconciliation (MTConnect + stale MQTT records) + +**Classification:** small +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 3 + +**Files:** +- Modify: `docs/drivers/MTConnect.md` (capability :71-77, Deferred :295-301, new §Write-back) +- Modify: `docs/plans/2026-07-15-mtconnect-driver-design.md` (§3.6 superseded note) +- Modify: `docs/drivers/Mqtt.md` (delete stale :550-556 and :586-587) +- Modify: `docs/plans/2026-07-24-driver-expansion-tracking.md` (P2 gap bullets :239-263 → closed + #509/#510/#513/#514; §Next actions :288-289 rewritten) +- Modify: `deferment.md` (:126 MTConnect row, :132-133 four-driver IWritable line → Sql + + Calculation only) + +Per design §1 + §3. The Write-back section MUST carry the §2.2 semantics warning (observation +injection, adapter overwrites, two-layer opt-in, `AllowPutFrom` pointer). Commit +`docs: MTConnect write-back recorded; stale MQTT write-through/browse-gap records reconciled`. + +### Task 5: Full-suite verification + +**Classification:** small +**Estimated implement time:** ~4 min +**Parallelizable with:** none + +`dotnet build ZB.MOM.WW.OtOpcUa.slnx` (0 errors) + full MTConnect unit suite + AdminUI suite + +`dotnet test --filter "FullyQualifiedName~Mqtt"` (docs-only but cheap insurance). Integration +suite compiles; live legs skip offline. Fix fallout, commit if changes. + +### Task 6: Live gate (main session, headless — NO chrome) + +**Classification:** standard +**Estimated implement time:** ~10 min (main session, after merge) + +Main session, not a subagent: bring up the MTConnect fixture (`ssh dohertj2@10.100.0.35` compose +in `/opt/otopcua-mtconnect/` — `lmxopcua-fix sync` equivalent via rsync first, since agent.cfg +changed), run the Task 3 legs live (`dotnet test --filter` the new write tests with +`MTCONNECT_AGENT_ENDPOINT` set), then the docker-dev rig leg: author + deploy an MTConnect driver +with `write.enabled`, `Client.CLI write` a value, read it back via Client.CLI and `/current`. +Record results in the design doc §Outcome. diff --git a/docs/plans/2026-08-05-mtconnect-writeback.md.tasks.json b/docs/plans/2026-08-05-mtconnect-writeback.md.tasks.json new file mode 100644 index 00000000..cf23733a --- /dev/null +++ b/docs/plans/2026-08-05-mtconnect-writeback.md.tasks.json @@ -0,0 +1,13 @@ +{ + "planPath": "docs/plans/2026-08-05-mtconnect-writeback.md", + "tasks": [ + { "id": 0, "subject": "Task 0: Write options block (config + parse)", "status": "pending" }, + { "id": 1, "subject": "Task 1: Agent-client write seam", "status": "pending" }, + { "id": 2, "subject": "Task 2: MTConnectDriver IWritable + discovery stamping", "status": "pending", "blockedBy": [0, 1] }, + { "id": 3, "subject": "Task 3: Integration fixture + write legs", "status": "pending", "blockedBy": [2] }, + { "id": 4, "subject": "Task 4: Docs reconciliation (MTConnect + stale MQTT records)", "status": "pending", "blockedBy": [2] }, + { "id": 5, "subject": "Task 5: Full-suite verification", "status": "pending", "blockedBy": [3, 4] }, + { "id": 6, "subject": "Task 6: Live gate (main session, headless)", "status": "pending", "blockedBy": [5] } + ], + "lastUpdated": "2026-08-05T00:00:00Z" +} From b5e39e1f328ad86ec33146885e157b4069669e90 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 06:13:30 -0400 Subject: [PATCH 2/9] feat(mtconnect): parse the write.enabled config block --- .../MTConnectDriverOptions.cs | 19 ++++++++ .../MTConnectDriver.cs | 14 ++++++ .../MTConnectDriverOptionsTests.cs | 14 ++++++ .../MTConnectFactoryTests.cs | 47 +++++++++++++++++++ 4 files changed, 94 insertions(+) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectDriverOptions.cs index da548efa..6c601e76 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectDriverOptions.cs @@ -109,6 +109,12 @@ public sealed class MTConnectDriverOptions /// /sample streaming connection. /// public MTConnectReconnectOptions Reconnect { get; init; } = new(); + + /// + /// Write-back settings. Off by default — the driver shipped read-only, and enabling this is + /// only half the gate: the Agent's own AllowPut must be on too. + /// + public MTConnectWriteOptions Write { get; init; } = new(); } /// Background connectivity-probe knobs, mirroring ModbusProbeOptions. @@ -132,3 +138,16 @@ public sealed class MTConnectReconnectOptions /// Multiplier applied each retry. Default 2.0 doubles each step. public double BackoffMultiplier { get; init; } = 2.0; } + +/// +/// Write-back knobs. A write is observation injection via the Agent's AllowPut +/// surface — it sets what the Agent reports for a DataItem, not the machine itself (design §2.2). +/// +public sealed class MTConnectWriteOptions +{ + /// + /// Gets a value indicating whether write-back is enabled. Default false — an existing + /// deployment that never authors this block keeps today's read-only behaviour exactly. + /// + public bool Enabled { get; init; } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index e811179c..53e41b9d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -2276,6 +2276,13 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis MaxBackoffMs = dto.Reconnect?.MaxBackoffMs ?? 30_000, BackoffMultiplier = dto.Reconnect?.BackoffMultiplier ?? 2.0, }, + + // Absent block == disabled: write-back is opt-in, and the agent's own AllowPut has to be + // on as well, so neither layer can arm the other by omission. + Write = new MTConnectWriteOptions + { + Enabled = dto.Write?.Enabled ?? false, + }, }; } @@ -3272,6 +3279,7 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis public List? RawTags { get; init; } public MTConnectProbeDto? Probe { get; init; } public MTConnectReconnectDto? Reconnect { get; init; } + public MTConnectWriteDto? Write { get; init; } } /// @@ -3347,4 +3355,10 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis public int? MaxBackoffMs { get; init; } public double? BackoffMultiplier { get; init; } } + + /// Write-back knobs — the driver-side half of the two-layer opt-in on observation injection. + private sealed class MTConnectWriteDto + { + public bool? Enabled { get; init; } + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDriverOptionsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDriverOptionsTests.cs index 91d1c4ca..aa2d989f 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDriverOptionsTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDriverOptionsTests.cs @@ -56,6 +56,20 @@ public sealed class MTConnectDriverOptionsTests reconnect.BackoffMultiplier.ShouldBeGreaterThan(1.0); } + /// + /// Write-back is off unless authored: the driver shipped read-only, and the agent's + /// PUT surface is itself off by default, so a deployment that never mentions write + /// must keep exactly today's behaviour. + /// + [Fact] + public void Write_defaults_to_disabled() + { + var write = new MTConnectDriverOptions { AgentUri = "http://agent:5000" }.Write; + + write.ShouldNotBeNull(); + write.Enabled.ShouldBeFalse(); + } + /// /// defaults to an empty collection, never /// null — a null collection here is an operator-authorable NullReferenceException at diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectFactoryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectFactoryTests.cs index 49dea03c..6184bc5f 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectFactoryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectFactoryTests.cs @@ -349,4 +349,51 @@ public sealed class MTConnectFactoryTests // above) hold for both entry points. MTConnectDriverFactoryExtensions.CreateInstance("mt1", json).DriverInstanceId.ShouldBe("mt1"); } + + /// + /// The write block reaches the options — the opt-in half of the two-layer gate on + /// write-back (the agent's own AllowPut is the other). Authored true arms it; + /// an authored false and an omitted block are the same thing, and both leave the + /// driver read-only. + /// + [Fact] + public void Write_section_round_trips_and_defaults_to_disabled_when_absent() + { + MTConnectDriver.ParseOptions( + "mt1", """{ "agentUri": "http://x:5000", "write": { "enabled": true } }""") + .Write.Enabled.ShouldBeTrue(); + + MTConnectDriver.ParseOptions( + "mt1", """{ "agentUri": "http://x:5000", "write": { "enabled": false } }""") + .Write.Enabled.ShouldBeFalse(); + + MTConnectDriver.ParseOptions("mt1", """{ "agentUri": "http://x:5000" }""") + .Write.Enabled.ShouldBeFalse(); + + // The factory shares that parse, so an armed instance is authorable through it too. + MTConnectDriverFactoryExtensions.CreateInstance( + "mt1", """{ "agentUri": "http://x:5000", "write": { "enabled": true } }""") + .DriverInstanceId.ShouldBe("mt1"); + } + + /// + /// A malformed write block fails exactly as its siblings do — the whole + /// document is one JsonSerializer.Deserialize, so a non-boolean enabled faults + /// the parse rather than silently defaulting to off. The probe leg is the control: it + /// pins that this is the surrounding convention and not stricter behaviour invented here. + /// + [Fact] + public void Malformed_write_block_faults_exactly_like_the_sibling_probe_block() + { + Should.Throw(() => MTConnectDriver.ParseOptions( + "mt1", """{ "agentUri": "http://x:5000", "probe": { "enabled": "yes" } }""")); + + Should.Throw(() => MTConnectDriver.ParseOptions( + "mt1", """{ "agentUri": "http://x:5000", "write": { "enabled": "yes" } }""")); + + // A null block is not malformed — the same "omitted means default" reading the sibling + // blocks get, since every DTO property is nullable. + MTConnectDriver.ParseOptions("mt1", """{ "agentUri": "http://x:5000", "write": null }""") + .Write.Enabled.ShouldBeFalse(); + } } From 9f17d7efa204fb1345ae30fde7f2d3ddc07056ae Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 06:16:35 -0400 Subject: [PATCH 3/9] feat(mtconnect): agent-client observation write (HTTP POST, AllowPut surface) --- .../IMTConnectAgentClient.cs | 85 ++++- .../MTConnectAgentClient.cs | 169 +++++++++- .../CannedAgentClient.cs | 26 ++ .../MTConnectAgentClientWriteTests.cs | 307 ++++++++++++++++++ 4 files changed, 580 insertions(+), 7 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs index 1d41561e..09543a11 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs @@ -1,5 +1,27 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; +/// +/// The result of a call, as the +/// Agent answered it. A transport failure is never represented here — it throws (see the +/// method's contract), so an outcome always means "the Agent replied, and this is what it said". +/// +/// +/// true when the Agent took the observation: a 2xx response whose body is not an +/// MTConnectError document. false is a refusal, and +/// then always carries text. +/// +/// +/// Why the Agent refused, in its own words where it supplied any — the joined +/// errorCode: text of the MTConnectError document, or the HTTP status line for a +/// non-2xx answer. null if and only if is true. +/// +/// This is Agent-authored text destined for a log line, never for a driver's +/// LastError (which is type-only by repo-wide discipline) and never for an OPC UA +/// status code's own message. +/// +/// +public sealed record MTConnectWriteOutcome(bool Accepted, string? AgentError); + /// /// The seam between the MTConnect driver and an MTConnect Agent's HTTP surface /// (/probe, /current, /sample). This interface is transport-neutral by @@ -17,7 +39,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// /sample long poll. (Task 0 planned to build on the TrakHound MTConnect.NET libraries; /// Tasks 6 and 7 proved they can neither parse a document nor frame the stream socket-free, and /// dropped the references — see the Task 0 CORRECTION block in the plan.) A fake for tests only -/// needs to implement these three instance members; is a static +/// needs to implement these four instance members; is a static /// protocol rule that lives here, rather than on the implementation, so a consumer written /// against this seam never has to reference the concrete client. /// @@ -88,6 +110,67 @@ public interface IMTConnectAgentClient : IDisposable /// Cancellation token; cancelling is the contracted way to end the stream. IAsyncEnumerable SampleAsync(long from, CancellationToken ct); + /// + /// Sets the Agent's observation for one DataItem, through the Agent's AllowPut + /// surface (POST {agent}/{deviceName}?{dataItemId}={value}). + /// + /// + /// + /// This is observation injection, not machine command. The value lands in the + /// Agent's observation buffer exactly as if an adapter had reported it — every consumer + /// of that Agent then sees it, and the next adapter/SHDR update for the same DataItem + /// overwrites it. Nothing is sent to the machine. It is also gated twice and off at both + /// gates by default: the Agent's own AllowPut (agent.cfg) and the driver's + /// write.enabled. + /// + /// + /// A refusal is a return value; a transport failure is an exception. The split is + /// the whole point of the outcome type, and it is what lets the driver map the two to + /// different OPC UA status codes without inspecting exception text: + /// + /// + /// The Agent answered and took the observation (2xx, body empty or not an + /// MTConnectError) ⇒ . An + /// empty 2xx body is acceptance — that is what a cppagent actually sends, + /// so treating a body-less answer as suspect would refuse every successful write. + /// + /// + /// The Agent answered and refused — a non-2xx status, or an + /// MTConnectError document served (as Agents do) under HTTP 200 ⇒ + /// not accepted, with the Agent's own text in + /// . The 200-with-error shape is + /// why the status code alone can never decide this. + /// + /// + /// The Agent could not be reached or did not answer in time ⇒ throws + /// (, or a for + /// the per-call deadline; when + /// is cancelled or the client is disposed). + /// Folding these into a false outcome would make "the machine's Agent is + /// unreachable" indistinguishable from "the Agent rejected this DataItem" — + /// identical to the caller, opposite in what an operator must do about it. + /// + /// + /// + /// + /// The write is a URL, so escaping is this seam's job, not the caller's. + /// and are passed unescaped + /// and are percent-encoded by the implementation. A caller that pre-escaped would + /// double-encode and write a literal %20 into the Agent's buffer, which the Agent + /// accepts — so the corruption is silent and shows up only as a wrong value on read-back. + /// + /// + /// + /// The device that owns the DataItem — the path segment of the request. Required even on a + /// single-device Agent, because the Agent's PUT surface is addressed per device. + /// + /// The DataItem's id, as declared by /probe. Unescaped. + /// The value to report, already rendered to its invariant string form. Unescaped. + /// Cancellation token. + /// What the Agent said — see the remarks. + Task WriteObservationAsync( + string deviceName, string dataItemId, string value, CancellationToken cancellationToken); + /// /// Has the Agent's ring buffer overflowed past the sequence the caller expected next? True /// when the chunk's oldest retained sequence is strictly newer than diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs index 8ce654b1..a3b39450 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs @@ -1,6 +1,8 @@ using System.Globalization; using System.Runtime.CompilerServices; using System.Text; +using System.Xml; +using System.Xml.Linq; using Microsoft.Extensions.Logging; namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; @@ -75,6 +77,7 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable private readonly ILogger? _logger; private readonly Uri _probeUri; private readonly Uri _currentUri; + private readonly string _agentRoot; private readonly string _sampleUriBase; private readonly string _sampleQuerySuffix; private readonly TimeSpan _requestTimeout; @@ -133,6 +136,7 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable // Agent and an idle one both emit at least one chunk per heartbeat. _streamIdleTimeout = TimeSpan.FromMilliseconds((2L * options.HeartbeatMs) + options.RequestTimeoutMs); + _agentRoot = ResolveAgentRoot(options); _probeUri = BuildRequestUri(options, "probe"); _currentUri = BuildRequestUri(options, "current"); _sampleUriBase = BuildRequestUri(options, "sample").AbsoluteUri; @@ -242,6 +246,139 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable } } + /// + /// + /// Issued on _http, the unary client — the write is a short request/response + /// and must be bounded by RequestTimeoutMs like the other unary legs. Sending it on + /// the streaming client would inherit that client's infinite timeout, and a frozen Agent + /// would park a write forever with nothing to bound it (the streaming leg's heartbeat + /// watchdog protects a multipart body, not a one-shot POST). + /// + public async Task WriteObservationAsync( + string deviceName, string dataItemId, string value, CancellationToken cancellationToken) + { + ObjectDisposedException.ThrowIf(_disposed, this); + ArgumentException.ThrowIfNullOrWhiteSpace(deviceName); + ArgumentException.ThrowIfNullOrWhiteSpace(dataItemId); + ArgumentNullException.ThrowIfNull(value); + + var uri = BuildWriteUri(deviceName, dataItemId, value); + + using var lifetime = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, _disposeCts.Token); + using var deadline = CancellationTokenSource.CreateLinkedTokenSource(lifetime.Token); + deadline.CancelAfter(_requestTimeout); + + using var request = new HttpRequestMessage(HttpMethod.Post, uri); + + HttpResponseMessage response; + try + { + response = await _http.SendAsync(request, deadline.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) when (!lifetime.IsCancellationRequested) + { + throw TimedOut(uri); + } + + using (response) + { + string body; + try + { + body = await response.Content.ReadAsStringAsync(deadline.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) when (!lifetime.IsCancellationRequested) + { + throw TimedOut(uri); + } + + // Deliberately NOT EnsureSuccessStatusCode: a refused write is an answer, and the caller + // needs it as a value it can map to a per-tag status code rather than as an exception + // indistinguishable from the Agent being unreachable. + var agentError = DescribeAgentError(body); + if (!response.IsSuccessStatusCode) + { + var status = $"HTTP {(int)response.StatusCode} {response.ReasonPhrase}".TrimEnd(); + + return new MTConnectWriteOutcome( + Accepted: false, agentError is null ? status : $"{status}: {agentError}"); + } + + // An MTConnectError document under HTTP 200 is how an Agent actually refuses; the status + // code alone would report it as a success. + return agentError is null + ? new MTConnectWriteOutcome(Accepted: true, AgentError: null) + : new MTConnectWriteOutcome(Accepted: false, agentError); + } + } + + /// + /// The Agent's AllowPut URL: the device as a path segment, the DataItem id and value + /// as a query pair. Every caller-supplied part is percent-encoded here — see the seam's + /// contract for why escaping cannot be the caller's job. + /// + private Uri BuildWriteUri(string deviceName, string dataItemId, string value) + { + var device = Uri.EscapeDataString(deviceName.Trim()); + var query = $"{Uri.EscapeDataString(dataItemId)}={Uri.EscapeDataString(value)}"; + + return new Uri($"{_agentRoot}/{device}?{query}", UriKind.Absolute); + } + + /// + /// The Agent's own refusal text when is an MTConnectError + /// document, else null (including for an empty body, a non-XML body, and any other + /// MTConnect document). + /// + /// + /// Read by only, like every other document this driver parses: + /// the namespace URI carries the MTConnect version, so a qualified match would silently stop + /// recognising errors the day an Agent is upgraded — and an unrecognised error reads as a + /// successful write. + /// + private static string? DescribeAgentError(string body) + { + if (string.IsNullOrWhiteSpace(body)) + { + return null; + } + + XElement? root; + try + { + root = XDocument.Parse(body).Root; + } + catch (XmlException) + { + // Not XML at all — an Agent that answered 2xx with a plain-text or HTML body has not + // refused anything, so this is not the place to invent a failure. + return null; + } + + if (root is null || !MTConnectXml.IsNamed(root, "MTConnectError")) + { + return null; + } + + var errors = root.Descendants() + .Where(e => MTConnectXml.IsNamed(e, "Error")) + .Select(e => + { + var code = MTConnectXml.OptionalAttribute(e, "errorCode"); + var text = e.Value.Trim(); + + return code is null ? text : $"{code}: {text}"; + }) + .Where(text => text.Length > 0) + .ToList(); + + // An error document with no readable detail is still a refusal; reporting it as null would + // turn it back into an acceptance. + return errors.Count == 0 + ? "The Agent answered with an MTConnectError document carrying no error detail." + : string.Join("; ", errors); + } + /// public void Dispose() { @@ -379,7 +516,31 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable return boundary; } - private static Uri BuildRequestUri(MTConnectDriverOptions options, string request) + /// + /// A read path's request URI, scoped to the configured device. Reads , + /// so it may only be called after that field is assigned — which the constructor does first. + /// + private Uri BuildRequestUri(MTConnectDriverOptions options, string request) + { + var deviceName = options.DeviceName?.Trim(); + var deviceSegment = string.IsNullOrEmpty(deviceName) ? string.Empty : $"/{Uri.EscapeDataString(deviceName)}"; + + return new Uri($"{_agentRoot}{deviceSegment}/{request}", UriKind.Absolute); + } + + /// + /// Validates and reduces it to the base every + /// request path is appended to: its left part up to and including the path (so a proxied + /// Agent keeps its prefix), with any trailing slash removed so callers can append + /// /segment unconditionally. + /// + /// + /// Extracted rather than duplicated because the write path resolves its device at call time, + /// not at construction: two copies of this rule would drift, and the drift would surface as + /// writes landing on a URL the reads never used — against an Agent that answers 200 either + /// way. + /// + private static string ResolveAgentRoot(MTConnectDriverOptions options) { var agentUri = options.AgentUri?.Trim(); if (string.IsNullOrEmpty(agentUri)) @@ -395,11 +556,7 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable $"{nameof(MTConnectDriverOptions.AgentUri)} '{agentUri}' is not an absolute http(s) URI.", nameof(options)); } - var root = parsed.GetLeftPart(UriPartial.Path).TrimEnd('/'); - var deviceName = options.DeviceName?.Trim(); - var deviceSegment = string.IsNullOrEmpty(deviceName) ? string.Empty : $"/{Uri.EscapeDataString(deviceName)}"; - - return new Uri($"{root}{deviceSegment}/{request}", UriKind.Absolute); + return parsed.GetLeftPart(UriPartial.Path).TrimEnd('/'); } /// diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/CannedAgentClient.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/CannedAgentClient.cs index 521e0e98..d6627661 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/CannedAgentClient.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/CannedAgentClient.cs @@ -355,6 +355,32 @@ internal sealed class CannedAgentClient : IMTConnectAgentClient, IDisposable } } + /// + /// Every write this Agent was asked to take, in order — the record a driver-level write test + /// asserts against (which DataItem, on which device, carrying what). + /// + public ConcurrentQueue<(string DeviceName, string DataItemId, string Value)> Writes { get; } = new(); + + /// + /// What answers with. Defaults to acceptance; set it to a + /// refusal to script the Agent's MTConnectError leg. + /// + public MTConnectWriteOutcome WriteOutcome { get; set; } = new(Accepted: true, AgentError: null); + + /// When set, a write throws this instead of answering — the transport-failure leg. + public Exception? WriteFailure { get; set; } + + /// + public Task WriteObservationAsync( + string deviceName, string dataItemId, string value, CancellationToken cancellationToken) + { + ObjectDisposedException.ThrowIf(IsDisposed, this); + cancellationToken.ThrowIfCancellationRequested(); + Writes.Enqueue((deviceName, dataItemId, value)); + + return WriteFailure is null ? Task.FromResult(WriteOutcome) : throw WriteFailure; + } + /// /// Queues a chunk onto the scripted /sample stream and waits until the consumer has /// processed it. This is the deterministic replacement for "wait a bit and hope the pump diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs new file mode 100644 index 00000000..cadf4af1 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs @@ -0,0 +1,307 @@ +using System.Net; +using System.Text; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests; + +/// +/// The wire contract of — the Agent's +/// AllowPut surface. Everything here runs through a stubbed +/// via the internal test-seam constructor, so the verb, the +/// exact request URI, and each response shape are asserted with no socket and no Agent. +/// +/// +/// +/// The URI assertions are the point of this class, not incidental detail. A write is +/// nothing but a URL: get the escaping or the device segment wrong and the Agent still +/// answers 200, having stored a value under a DataItem id nobody asked about — a corruption +/// with no error anywhere, visible only on a later read-back. There is no encoder to unit +/// test, so this is the only place that shape is pinned. +/// +/// +/// The refusal legs pin the split the seam contract rests on: an Agent that answered +/// and said no is a returned outcome, while an Agent that could not be reached throws. The +/// 200-carrying-an-MTConnectError case is why the status code alone can never decide +/// it — that is the shape a real cppagent uses to report every objection. +/// +/// +[Trait("Category", "Unit")] +public sealed class MTConnectAgentClientWriteTests +{ + // ── stub handler ───────────────────────────────────────────────────────── + + /// Answers every request with a canned response (or throws) via a delegate, with no socket. + private sealed class StubHandler(Func> respond) + : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken ct) => + respond(request, ct); + } + + /// Records the requests it is handed, then answers each with . + private sealed class RecordingHandler(Func respond) : HttpMessageHandler + { + public List<(HttpMethod Method, Uri Uri)> Requests { get; } = []; + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken ct) + { + Requests.Add((request.Method, request.RequestUri!)); + + return Task.FromResult(respond(request)); + } + } + + private static MTConnectDriverOptions Options(string agentUri = "http://stub-agent:5000") => + new() { AgentUri = agentUri }; + + private static HttpResponseMessage Ok(string body = "") => new(HttpStatusCode.OK) + { + Content = new StringContent(body, Encoding.UTF8, "text/plain"), + }; + + private static HttpResponseMessage XmlOk(string xml) => new(HttpStatusCode.OK) + { + Content = new StringContent(xml, Encoding.UTF8, "text/xml"), + }; + + /// Exactly the shape a cppagent uses to refuse: an error document served under HTTP 200. + private const string MTConnectErrorXml = + """ + + +
+ + Cannot find device data item: nosuchitem + + + """; + + // ── verb + URI ─────────────────────────────────────────────────────────── + + [Fact] + public async Task A_write_is_issued_as_a_POST() + { + var handler = new RecordingHandler(_ => Ok()); + using var client = new MTConnectAgentClient(Options(), handler); + + await client.WriteObservationAsync("StubDevice", "dev1_msg", "hello", CancellationToken.None); + + handler.Requests.ShouldHaveSingleItem().Method.ShouldBe(HttpMethod.Post); + } + + /// + /// The device is a path segment and the DataItem id/value are a query pair — + /// the Agent's documented AllowPut shape. Asserted as the whole URI rather than in parts, + /// because a fragment-wise assertion passes on a URI that is assembled in the wrong order. + /// + [Fact] + public async Task The_request_uri_is_the_agents_AllowPut_shape() + { + var handler = new RecordingHandler(_ => Ok()); + using var client = new MTConnectAgentClient(Options(), handler); + + await client.WriteObservationAsync("StubDevice", "dev1_msg", "hello", CancellationToken.None); + + handler.Requests.ShouldHaveSingleItem().Uri.AbsoluteUri + .ShouldBe("http://stub-agent:5000/StubDevice?dev1_msg=hello"); + } + + /// + /// A value carrying a space, an & and a = — each of which silently ends the + /// query pair early if it reaches the wire raw — plus a DataItem id needing the same + /// treatment. Unescaped, the Agent would take a 200 and store the truncated value. + /// + [Fact] + public async Task The_dataItemId_and_value_are_percent_encoded() + { + var handler = new RecordingHandler(_ => Ok()); + using var client = new MTConnectAgentClient(Options(), handler); + + await client.WriteObservationAsync( + "Stub Device", "dev1 msg&x", "Coolant low = 20% & rising", CancellationToken.None); + + handler.Requests.ShouldHaveSingleItem().Uri.AbsoluteUri.ShouldBe( + "http://stub-agent:5000/Stub%20Device?dev1%20msg%26x=Coolant%20low%20%3D%2020%25%20%26%20rising"); + } + + /// + /// The base URI's own path is preserved (an Agent behind a reverse-proxy prefix) and a + /// trailing slash does not double up — the same left-part rule the read paths use. + /// + [Fact] + public async Task A_based_agent_uri_keeps_its_path_prefix_and_does_not_double_its_slash() + { + var handler = new RecordingHandler(_ => Ok()); + using var client = new MTConnectAgentClient(Options("http://stub-agent:5000/mtconnect/"), handler); + + await client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None); + + handler.Requests.ShouldHaveSingleItem().Uri.AbsoluteUri + .ShouldBe("http://stub-agent:5000/mtconnect/StubDevice?dev1_msg=hi"); + } + + /// + /// The deviceName argument decides the path — not the options' device scope. A + /// multi-device Agent is addressed per write, so an implementation that reused the read + /// paths' scoped URI would write every DataItem onto one device. + /// + [Fact] + public async Task The_device_segment_comes_from_the_argument_not_the_configured_device_scope() + { + var handler = new RecordingHandler(_ => Ok()); + using var client = new MTConnectAgentClient( + new MTConnectDriverOptions { AgentUri = "http://stub-agent:5000", DeviceName = "ScopedDevice" }, + handler); + + await client.WriteObservationAsync("OtherDevice", "dev2_msg", "hi", CancellationToken.None); + + handler.Requests.ShouldHaveSingleItem().Uri.AbsoluteUri + .ShouldBe("http://stub-agent:5000/OtherDevice?dev2_msg=hi"); + } + + // ── acceptance ─────────────────────────────────────────────────────────── + + /// + /// An empty 2xx body is what a real cppagent sends on success. Treating a body-less answer + /// as anything but acceptance would refuse every write that actually worked. + /// + [Fact] + public async Task An_empty_200_is_accepted() + { + using var client = new MTConnectAgentClient(Options(), new StubHandler((_, _) => Task.FromResult(Ok()))); + + var outcome = await client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None); + + outcome.Accepted.ShouldBeTrue(); + outcome.AgentError.ShouldBeNull(); + } + + /// A 200 carrying a non-MTConnect body is still acceptance — only an error document refuses. + [Fact] + public async Task A_200_whose_body_is_not_an_error_document_is_accepted() + { + using var client = new MTConnectAgentClient( + Options(), new StubHandler((_, _) => Task.FromResult(Ok("OK")))); + + var outcome = await client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None); + + outcome.Accepted.ShouldBeTrue(); + outcome.AgentError.ShouldBeNull(); + } + + // ── refusal ────────────────────────────────────────────────────────────── + + /// + /// The load-bearing refusal: HTTP 200 with an MTConnectError body. + /// EnsureSuccessStatusCode never fires here, so an implementation that trusted the + /// status code would report every rejected write as a success. + /// + [Fact] + public async Task An_MTConnectError_under_200_is_a_refusal_carrying_the_agents_own_text() + { + using var client = new MTConnectAgentClient( + Options(), new StubHandler((_, _) => Task.FromResult(XmlOk(MTConnectErrorXml)))); + + var outcome = await client.WriteObservationAsync( + "StubDevice", "nosuchitem", "hi", CancellationToken.None); + + outcome.Accepted.ShouldBeFalse(); + outcome.AgentError.ShouldNotBeNull(); + outcome.AgentError.ShouldContain("UNSUPPORTED"); + outcome.AgentError.ShouldContain("Cannot find device data item: nosuchitem"); + } + + [Fact] + public async Task A_non_success_status_is_a_refusal_naming_the_status() + { + using var client = new MTConnectAgentClient( + Options(), + new StubHandler((_, _) => Task.FromResult(new HttpResponseMessage(HttpStatusCode.NotFound) + { + Content = new StringContent(string.Empty), + }))); + + var outcome = await client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None); + + outcome.Accepted.ShouldBeFalse(); + outcome.AgentError.ShouldNotBeNull(); + outcome.AgentError.ShouldContain("404"); + } + + // ── transport failure throws, never a false outcome ────────────────────── + + /// + /// "Unreachable" must stay distinguishable from "rejected". Returning + /// Accepted = false here would collapse a comms fault into a per-tag refusal, which + /// the driver maps to a different status code and an operator resolves differently. + /// + [Fact] + public async Task A_transport_failure_propagates_rather_than_becoming_a_refusal() + { + var boom = new HttpRequestException("dial failed"); + using var client = new MTConnectAgentClient( + Options(), new StubHandler((_, _) => Task.FromException(boom))); + + var thrown = await Should.ThrowAsync( + () => client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None)); + + thrown.ShouldBeSameAs(boom); + } + + /// + /// A frozen Agent must surface as a bounded naming the URI, + /// not park the caller — the same per-call deadline discipline the read legs carry + /// (arch-review R2-01). + /// + [Fact(Timeout = 10000)] + public async Task A_frozen_agent_times_out_within_the_per_call_deadline() + { + using var client = new MTConnectAgentClient( + new MTConnectDriverOptions { AgentUri = "http://stub-agent:5000", RequestTimeoutMs = 300 }, + new StubHandler(async (_, ct) => + { + await Task.Delay(Timeout.Infinite, ct); + + throw new UnreachableException(); + })); + + await Should.ThrowAsync( + () => client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None)); + } + + /// Local stand-in so the stub handler's "never returns" branch has a throw statement to satisfy flow analysis. + private sealed class UnreachableException : Exception; + + /// Cancelling is cancellation, never a refusal — the caller's own token wins over the deadline. + [Fact] + public async Task A_cancelled_token_surfaces_as_cancellation_not_as_a_refusal() + { + using var cts = new CancellationTokenSource(); + using var client = new MTConnectAgentClient( + Options(), + new StubHandler(async (_, ct) => + { + await Task.Delay(Timeout.Infinite, ct); + + throw new UnreachableException(); + })); + + var write = client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", cts.Token); + await cts.CancelAsync(); + + await Should.ThrowAsync(() => write); + } + + // ── disposal ───────────────────────────────────────────────────────────── + + [Fact] + public async Task Writing_through_a_disposed_client_throws_ObjectDisposedException() + { + var client = new MTConnectAgentClient(Options(), new StubHandler((_, _) => Task.FromResult(Ok()))); + client.Dispose(); + + await Should.ThrowAsync( + () => client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None)); + } +} From 3b8b5a77e1e742f811762bcebbb93464a48f32cb Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 06:23:08 -0400 Subject: [PATCH 4/9] fix(mtconnect): share MTConnectError text extraction; pin hollow-error refusal --- .../MTConnectAgentClient.cs | 27 ++++------ .../MTConnectXml.cs | 38 ++++++++++++-- .../MTConnectAgentClientWriteTests.cs | 49 +++++++++++++++++++ 3 files changed, 93 insertions(+), 21 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs index a3b39450..f40bf3e1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs @@ -334,7 +334,10 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable /// Read by only, like every other document this driver parses: /// the namespace URI carries the MTConnect version, so a qualified match would silently stop /// recognising errors the day an Agent is upgraded — and an unrecognised error reads as a - /// successful write. + /// successful write. The error-text walk itself is + /// , shared with the read paths' + /// so the two cannot drift on what counts + /// as an Agent objection. /// private static string? DescribeAgentError(string body) { @@ -360,23 +363,11 @@ public sealed class MTConnectAgentClient : IMTConnectAgentClient, IDisposable return null; } - var errors = root.Descendants() - .Where(e => MTConnectXml.IsNamed(e, "Error")) - .Select(e => - { - var code = MTConnectXml.OptionalAttribute(e, "errorCode"); - var text = e.Value.Trim(); - - return code is null ? text : $"{code}: {text}"; - }) - .Where(text => text.Length > 0) - .ToList(); - - // An error document with no readable detail is still a refusal; reporting it as null would - // turn it back into an acceptance. - return errors.Count == 0 - ? "The Agent answered with an MTConnectError document carrying no error detail." - : string.Join("; ", errors); + // An error document with no readable detail is still a refusal, so the shared helper's null + // — which means "nothing readable in here", NOT "this was not an error" — must become text + // rather than being passed on. Returning it as null would turn the refusal into acceptance. + return MTConnectXml.ExtractErrorText(root) + ?? "The Agent answered with an MTConnectError document carrying no error detail."; } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs index 63c31262..d43c4f65 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs @@ -127,6 +127,40 @@ internal static class MTConnectXml return message; } + var errors = ExtractErrorText(root); + + return errors is null ? message : $"{message} The Agent reported: {errors}"; + } + + /// + /// The Agent's own words from an MTConnectError document: every <Error> + /// beneath , rendered errorCode: text where a code is present + /// and joined with "; ". null when the document carries no readable error + /// detail at all (no <Error> elements, or only blank ones). + /// + /// + /// + /// Shared because both callers must read the same document the same way. An + /// Agent's objection reaches this driver through two unrelated doors — a read path that + /// got an error document where it expected a model + /// (), and the write path deciding whether a 200 + /// actually accepted the observation. Two copies of this walk would drift on the same + /// two silent rules the type-level remarks describe, and on the write side a missed + /// <Error> does not merely degrade a message: it reads as a successful + /// write. + /// + /// + /// null means "no readable detail", NOT "not an error document". Only the + /// caller knows which document it is holding — check the root name first. A caller that + /// treats a null here as "no error occurred" turns a hollow + /// <MTConnectError/> into a success. + /// + /// + /// The MTConnectError document's root element. + public static string? ExtractErrorText(XElement root) + { + ArgumentNullException.ThrowIfNull(root); + var errors = root.Descendants() .Where(e => IsNamed(e, "Error")) .Select(e => @@ -139,8 +173,6 @@ internal static class MTConnectXml .Where(text => text.Length > 0) .ToList(); - return errors.Count == 0 - ? message - : $"{message} The Agent reported: {string.Join("; ", errors)}"; + return errors.Count == 0 ? null : string.Join("; ", errors); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs index cadf4af1..ea7d20ec 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectAgentClientWriteTests.cs @@ -212,6 +212,55 @@ public sealed class MTConnectAgentClientWriteTests outcome.AgentError.ShouldContain("Cannot find device data item: nosuchitem"); } + /// + /// An MTConnectError document carrying no readable <Error> detail is + /// still a refusal. The Agent said no; that it said so unhelpfully changes nothing. The trap + /// this pins is that the natural implementation — "return the joined error text, or null if + /// there is none" — hands back the same null that means "this body was not an error + /// document at all", silently converting the refusal into an acceptance. + /// + [Fact] + public async Task An_error_document_with_no_readable_detail_is_still_a_refusal() + { + const string HollowError = + """ + + +
+ + + """; + + using var client = new MTConnectAgentClient( + Options(), new StubHandler((_, _) => Task.FromResult(XmlOk(HollowError)))); + + var outcome = await client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None); + + outcome.Accepted.ShouldBeFalse(); + outcome.AgentError.ShouldNotBeNullOrWhiteSpace(); + } + + /// The same, for an <Error> element that is present but blank. + [Fact] + public async Task An_error_document_whose_error_element_is_blank_is_still_a_refusal() + { + const string BlankError = + """ + + + + + """; + + using var client = new MTConnectAgentClient( + Options(), new StubHandler((_, _) => Task.FromResult(XmlOk(BlankError)))); + + var outcome = await client.WriteObservationAsync("StubDevice", "dev1_msg", "hi", CancellationToken.None); + + outcome.Accepted.ShouldBeFalse(); + outcome.AgentError.ShouldNotBeNullOrWhiteSpace(); + } + [Fact] public async Task A_non_success_status_is_a_refusal_naming_the_status() { From b29623cf99f1508ea91e96beb0642f23616ae5db Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 06:36:08 -0400 Subject: [PATCH 5/9] feat(mtconnect): IWritable via agent observation writes MTConnectDriver now implements IWritable over the Task-1 agent-client seam (WriteObservationAsync -> the Agent's AllowPut POST surface). A write is OBSERVATION INJECTION, not machine command: it sets what the Agent reports for a DataItem, is visible to every consumer of that Agent, and is overwritten by the next adapter/SHDR update. Off at two independent gates, both off by default - the Agent's own AllowPut and the driver's write.enabled (Task 0). Per-slot refusal order (never throws, strict N-in/N-out batch shape): write disabled -> BadNotWritable no live client -> BadNotConnected (ReadAsync's existing convention) reference unresolved -> BadNodeIdUnknown no resolvable device -> BadNodeIdUnknown CONDITION category -> BadNotWritable null / non-scalar value -> BadTypeMismatch agent refusal OR throw -> BadCommunicationError OperationCanceledException fills the remaining slots and returns. The Agent's refusal text is logged at Warning and never enters DriverHealth.LastError (type-only discipline); a refused write leaves health untouched. Device-name resolution: TagBinding gains WriteTargetsByDataItemId, built in BuildBinding from a probe-model walk that remembers which device each DataItem was declared under (name, or id when nameless) plus its category. Falls back to options.DeviceName; unresolvable -> BadNodeIdUnknown rather than a guess. Values render via Convert.ToString(..., InvariantCulture) - so a Boolean writes the invariant string "True"/"False", NOT "1"/"0". Arrays are refused rather than rendered: Convert.ToString would emit the literal "System.Double[]", which the Agent stores happily and which is invisible until read-back. FINDING - the CurrentWrite bit does NOT key on SecurityClassification, and the discovery stamping below is therefore cosmetic. Traced end to end: a browsed leaf's SecurityClass survives into the browse session's AttributeInfo (CapturedTreeBrowseSession.cs:53) and stops there - RawBrowseModal never reads it, RawBrowseCommitMapper.MapLeaf hardcodes TagAccessLevel.Read, and Tag has no SecurityClass column. AddressSpaceComposer:746/:809 sets Writable purely from Tag.AccessLevel == ReadWrite; AddressSpaceApplier ANDs !IsArray; OtOpcUaNodeManager.ComposeAccessLevel:2002 composes CurrentWrite from that alone. Driver capability is never consulted at compose time either - IWritable is checked only at runtime in DriverInstanceActor:755. Consequences: (a) raw tags already committed keep their authored AccessLevel, so nothing this commit does makes an existing node writable; (b) an operator must still set the tag to ReadWrite for an MTConnect write to be reachable over OPC UA; (c) conversely a ReadWrite tag on a non-IWritable driver has always advertised CurrentWrite and failed only at the actor - pre-existing, not touched here. Discovery stamps CanWrite(writeEnabled, dataItem) = write.enabled && !CONDITION -> Operate, else ViewOnly - kept honest so the browse never advertises a tier the write path would refuse (MQTT's CanWrite principle), with the caveat above recorded verbatim at the call site. Status-code constants are spelled WITHOUT MqttDriver's "Status" prefix, to match the two this file already declares (BadCommunicationError, BadNotConnected). StatusCodeParityTests strips either spelling and passes. TDD: WriteAsync landed first as a NotImplementedException scaffold - 19 of 20 new tests red on behaviour, 1 (the capability assertion) green on the scaffold - then implemented. MTConnectFactoryTests' "not_writable" characterization test is rewritten to pin the new truth, and is the guard for the concrete-return contract the factory remark explains. Tests: 532 passed / 0 failed (was ~509). Full solution build: 0 errors, 0 warnings; OTOPCUA0001 clean. --- .../MTConnectTagDefinition.cs | 9 +- .../MTConnectDriver.cs | 450 +++++++++++++++++- .../MTConnectDriverFactoryExtensions.cs | 14 +- .../MTConnectDiscoverTests.cs | 55 ++- .../MTConnectFactoryTests.cs | 16 +- .../MTConnectWriteTests.cs | 356 ++++++++++++++ 6 files changed, 868 insertions(+), 32 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinition.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinition.cs index c956f809..aaf040b8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinition.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinition.cs @@ -8,9 +8,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// current value from the Agent's /current snapshot / /sample stream by that id. ///
/// -/// The MTConnect DataItem's id attribute (the Agent's /probe response). This is -/// the driver's reference for read/subscribe — MTConnect is a read-only source protocol, so -/// there is no corresponding write path. +/// The MTConnect DataItem's id attribute (the Agent's /probe response). This is the +/// driver's reference for read, subscribe and write — the write being an observation +/// injection through the Agent's AllowPut surface +/// (POST {agent}/{device}?{id}={value}), which sets what the Agent reports for the DataItem +/// rather than commanding the machine. It is off unless both the Agent's AllowPut and the +/// driver's write.enabled say otherwise, and CONDITION-category items are excluded. /// /// Logical data type the DataItem's value is coerced to. /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index 53e41b9d..d8324306 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -44,12 +44,23 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// /// /// Scope. This type implements , , -/// , , +/// , , , /// and — the last two /// built on state this class already caches (the Agent instanceId and the probe -/// model). There is deliberately no IWritable: MTConnect is a read-only protocol, -/// which is also why every discovered leaf is -/// . +/// model). +/// +/// +/// is observation injection, not machine command, and it is +/// off by default. A write goes to the Agent's AllowPut surface +/// (POST {agent}/{device}?{dataItemId}={value}) and sets what the Agent reports +/// for that DataItem — visible to every consumer of that Agent, and overwritten by the next +/// adapter/SHDR update. Nothing reaches the CNC. It is therefore gated twice, and off at both +/// gates by default: the Agent's own AllowPut (agent.cfg) and this driver's +/// . Either gate alone leaves the shipped read-only +/// behaviour byte-for-byte unchanged. CONDITION data items are excluded — they carry alarm +/// semantics and a different level|text wire format. MTConnect Interfaces, the +/// protocol's own request/response handshake, remains deferred and is a different mechanism +/// entirely (see docs/plans/2026-08-05-mtconnect-writeback-design.md §2.1). /// /// /// The read path never writes the shared observation index — see @@ -60,15 +71,43 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// (see ). /// /// -public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDiscovery, IHostConnectivityProbe, IRediscoverable +public sealed class MTConnectDriver + : IDriver, IReadable, IWritable, ISubscribable, ITagDiscovery, IHostConnectivityProbe, IRediscoverable { /// /// The Agent could not be reached, answered unusably, or blew the per-call deadline. Distinct /// from the index's BadNoCommunication, which means the Agent answered and said the - /// machine has no value — the two failures need different operator responses. + /// machine has no value — the two failures need different operator responses. On the write + /// path it also covers an Agent that answered and refused — see . /// private const uint BadCommunicationError = 0x80050000u; + // ---- Write status codes ---- + // + // NAMED constants, never inline literals: StatusCodeParityTests checks each against the pinned SDK + // by reflection and can only see a named const (Gitea #497/#519). The names must match + // Opc.Ua.StatusCodes members exactly. Spelled WITHOUT the "Status" prefix MqttDriver uses, to match + // the two this file already declares (the parity guard strips either spelling). + + /// + /// The reference names no DataItem this driver serves, or names one whose owning device cannot + /// be resolved — there is no Agent URL a write could be addressed to. + /// + private const uint BadNodeIdUnknown = 0x80340000u; + + /// + /// Writing is switched off (write.enabled), or the DataItem is CONDITION-category. The + /// honest answer to "can I write this?", and deliberately not a communication failure: nothing + /// was attempted and no Agent was involved. + /// + private const uint BadNotWritable = 0x803B0000u; + + /// + /// The supplied value has no invariant string form a PUT could carry — null, or a + /// non-scalar (see ). + /// + private const uint BadTypeMismatch = 0x80740000u; + /// /// There is no Agent client at all: the driver has not been initialized, its initialize /// faulted, or it has been shut down. Deliberately not one of the index's codes — none of @@ -694,6 +733,200 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis } } + // ---- IWritable: observation injection through the Agent's AllowPut surface ---- + + /// + /// Sets the Agent's reported observation for each requested reference, through its + /// AllowPut surface. Returns one status per request, in request order. + /// + /// The requested writes. + /// Cancellation for the batch. + /// One per request, in request order. + /// + /// + /// This writes the Agent, not the machine — see the class remarks. Good here means + /// "the Agent took the observation", which is a stronger statement than MQTT's optimistic + /// "published" (the Agent answers) and a weaker one than a device acknowledgement (no + /// MTConnect Agent speaks for the machine). The next adapter update for that DataItem + /// overwrites what was written, and the ordinary /sample ingest path is what + /// corrects the node. + /// + /// + /// Never throws, and one bad slot never fails the batch — matching + /// . Failing the whole call would let one stale reference reject + /// every other write in the same OPC UA request. Even cancellation returns a full-length + /// batch rather than propagating, so the node-manager can tell which slots were attempted. + /// + /// + /// Refusal order is fixed, and each step answers a different operator question: + /// writing switched off ⇒ BadNotWritable; no live client ⇒ BadNotConnected + /// (this driver is not running — 's convention); the reference + /// resolves to no DataItem, or to one whose owning device cannot be named ⇒ + /// BadNodeIdUnknown; CONDITION category ⇒ BadNotWritable; no invariant string + /// form ⇒ BadTypeMismatch; and only then the Agent, whose refusal or whose + /// unreachability ⇒ BadCommunicationError. + /// + /// + /// The Agent's refusal text is logged, never stored. It goes to a Warning line and + /// nowhere near , which is type-only by repo-wide + /// discipline — and a refused write is not evidence the read path degraded, so health is + /// left exactly as it is (unlike ). + /// + /// + /// is null — a caller bug. + public async Task> WriteAsync( + IReadOnlyList writes, CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(writes); + + if (writes.Count == 0) + { + return []; + } + + // Read once each, for the same reason ReadAsync does: every slot in this batch is judged + // against ONE options document and ONE tag table, never half against a table a concurrent + // re-initialize just replaced. + var options = _options; + var client = Volatile.Read(ref _session)?.Client; + var binding = Volatile.Read(ref _binding); + + var results = new WriteResult[writes.Count]; + + for (var i = 0; i < writes.Count; i++) + { + var request = writes[i]; + + if (!options.Write.Enabled) + { + results[i] = new WriteResult(BadNotWritable); + continue; + } + + if (client is null) + { + results[i] = new WriteResult(BadNotConnected); + continue; + } + + if (!binding.TryResolveWriteTarget(request.FullReference, out var dataItemId, out var target) + || target.DeviceName is null) + { + results[i] = new WriteResult(BadNodeIdUnknown); + continue; + } + + if (target.IsCondition) + { + results[i] = new WriteResult(BadNotWritable); + continue; + } + + if (!TryFormatValue(request.Value, out var text)) + { + results[i] = new WriteResult(BadTypeMismatch); + continue; + } + + try + { + var outcome = await client + .WriteObservationAsync(target.DeviceName, dataItemId, text, cancellationToken) + .ConfigureAwait(false); + + if (outcome.Accepted) + { + results[i] = new WriteResult(0u); + continue; + } + + _logger.LogWarning( + "MTConnect driver {DriverInstanceId}: {AgentUri} refused the write of '{Value}' to DataItem '{DataItemId}' on device '{DeviceName}' — {AgentError}. Check the Agent's AllowPut / AllowPutFrom settings and that the DataItem exists.", + _driverInstanceId, options.AgentUri, text, dataItemId, target.DeviceName, outcome.AgentError); + + results[i] = new WriteResult(BadCommunicationError); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + // Caller-cancelled: fill the remainder rather than throwing, so the batch shape holds. + for (var j = i; j < writes.Count; j++) + { + results[j] = new WriteResult(BadCommunicationError); + } + + return results; + } + catch (Exception ex) + { + // A transport failure, a blown per-call deadline, or a disposed client. Logged with the + // exception (its TYPE is the diagnosis) and reported per slot; health is untouched. + _logger.LogWarning( + ex, + "MTConnect driver {DriverInstanceId} could not write DataItem '{DataItemId}' on device '{DeviceName}' to {AgentUri}.", + _driverInstanceId, dataItemId, target.DeviceName, options.AgentUri); + + results[i] = new WriteResult(BadCommunicationError); + } + } + + return results; + } + + /// + /// Renders a value to the invariant string a PUT query carries, or reports that it has no such + /// form. + /// + /// The value the OPC UA client supplied. + /// The rendered value; empty when this returns . + /// for a value that must be refused as a type mismatch. + /// + /// + /// is load-bearing. A comma decimal + /// separator produces a value the Agent accepts and every consumer reads back as a + /// different number. Booleans render as True/False — + /// is culture-independent, so that IS the invariant form, and + /// it is what an MTConnect consumer will see verbatim. + /// + /// + /// Arrays are refused rather than rendered. + /// falls back to + /// for a non- value, so an array + /// would be written as the literal text System.Double[] — which the Agent stores + /// happily, making the corruption invisible until someone reads the observation back. + /// TIME_SERIES / DATA_SET writes are out of scope for v1. + /// + /// + private static bool TryFormatValue(object? value, out string text) + { + text = string.Empty; + + // A PUT query has no null to send, and the empty string is a real observation rather than an + // absence — so "no value" is refused instead of being silently turned into one. + if (value is null or Array) + { + return false; + } + + string? rendered; + try + { + rendered = Convert.ToString(value, CultureInfo.InvariantCulture); + } + catch (Exception ex) when (ex is FormatException or InvalidCastException or OverflowException) + { + return false; + } + + if (rendered is null) + { + return false; + } + + text = rendered; + + return true; + } + // ---- ISubscribable: ONE shared /sample long poll behind every handle ---- /// @@ -940,7 +1173,11 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis // Read once: a concurrent re-initialize must not scope half this walk to one device and half // to another. - var deviceScope = _options.DeviceName?.Trim(); + // Both read once, together: a concurrent re-initialize must not scope half this walk to one + // device and half to another, nor stamp half its leaves writable and half not. + var options = _options; + var deviceScope = options.DeviceName?.Trim(); + var writeEnabled = options.Write.Enabled; var devices = 0; var variables = 0; @@ -966,7 +1203,7 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis } devices++; - variables += StreamContainer(device, builder.Folder(name, name)); + variables += StreamContainer(device, builder.Folder(name, name), writeEnabled); } // A configured scope that matches NO device is an authoring error — almost always a typo or @@ -1003,7 +1240,14 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis /// picker exists to show. Recursion depth is bounded by the parser, which refuses a probe /// document nested beyond its own limit. /// - private static int StreamContainer(IMTConnectComponentContainer container, IAddressSpaceBuilder scope) + /// The device or component whose data items are being streamed. + /// The builder folder they belong under. + /// + /// , read once by the caller so one walk cannot + /// stamp half its leaves under an old options document and half under a new one. + /// + private static int StreamContainer( + IMTConnectComponentContainer container, IAddressSpaceBuilder scope, bool writeEnabled) { var streamed = 0; @@ -1037,9 +1281,21 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis IsArray: inferred.IsArray, ArrayDim: inferred.ArrayDim, - // MTConnect is read-only and this driver implements no IWritable, so the tier - // that is read-only from OPC UA is the only honest one. - SecurityClass: SecurityClassification.ViewOnly, + // Operate exactly for the leaves WriteAsync would accept — the "a node cannot + // advertise writability the driver would refuse" principle. With write.enabled + // off (the default) every leaf stays ViewOnly, which is what this driver shipped. + // + // ⚠️ This does NOT make the committed tag writable over OPC UA, and must not be + // read as doing so. Nothing persists SecurityClass: RawBrowseCommitMapper.MapLeaf + // hardcodes TagAccessLevel.Read, Tag has no SecurityClass column, and the + // CurrentWrite bit is composed from Tag.AccessLevel alone + // (OtOpcUaNodeManager.ComposeAccessLevel). The stamp reaches the browse session's + // attribute side-channel and stops there, so an operator still has to author + // ReadWrite on the tag. It is kept honest anyway — a browse that advertised + // ViewOnly for a leaf this driver now writes would be a lie an operator reads. + SecurityClass: CanWrite(writeEnabled, dataItem) + ? SecurityClassification.Operate + : SecurityClassification.ViewOnly, // Historization is authored per tag, never inferred from a device model. IsHistorized: false, @@ -1067,12 +1323,26 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis continue; } - streamed += StreamContainer(component, scope.Folder(name, name)); + streamed += StreamContainer(component, scope.Folder(name, name), writeEnabled); } return streamed; } + /// + /// Whether would accept a write to — the + /// browse-time answer, asked of the same two facts the runtime asks of. + /// + /// Whether write.enabled is on. + /// The declared data item. + /// + /// The device-name leg of the runtime refusal is not asked here and does not need to be: a + /// leaf reached by this walk is inside a device the model declares, so its owning device + /// is known by construction. + /// + private static bool CanWrite(bool writeEnabled, MTConnectDataItem dataItem) => + writeEnabled && !IsCondition(dataItem.Category); + /// /// Whether a device is in this instance's configured scope. An unset scope is agent-wide. /// @@ -2933,10 +3203,13 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis if (options.RawTags.Count == 0) { + MTConnectTagDefinition[] only = [.. definitions.Values]; + return new TagBinding( FrozenDictionary.Empty, FrozenDictionary>.Empty, - [.. definitions.Values]); + only, + BuildWriteTargets(only, probeModel, options.DeviceName)); } var declared = IndexProbeDataItems(probeModel); @@ -2982,13 +3255,106 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis } } + MTConnectTagDefinition[] indexTags = [.. definitions.Values]; + return new TagBinding( idByRawPath.ToFrozenDictionary(StringComparer.Ordinal), rawPathsById.ToFrozenDictionary( pair => pair.Key, pair => (IReadOnlyList)pair.Value, StringComparer.Ordinal), - [.. definitions.Values]); + indexTags, + BuildWriteTargets(indexTags, probeModel, options.DeviceName)); + } + + /// + /// Derives the write-addressing table: for every DataItem this driver serves, the device the + /// Agent's AllowPut URL must name, and whether the item is CONDITION-category. + /// + /// Every DataItem the binding serves (the union of both authoring surfaces). + /// The Agent's device model when one has been fetched; else null. + /// + /// — the fallback for a DataItem the + /// /probe model does not declare. + /// + /// + /// + /// The device name is derived, not authored. The Agent's PUT surface is addressed + /// per device even on a single-device Agent, and the probe model already says which device + /// owns which DataItem — so a multi-device Agent is addressed correctly with no extra + /// config. Config is the fallback, for the dataItemId-keyed authoring surface (the driver + /// CLI) and for a DataItem the Agent has since stopped declaring. + /// + /// + /// A null is a refusal, not a default. Neither + /// the model nor config names a device, so there is no URL to POST to; guessing one yields + /// a 404 an operator reads as "the Agent is broken". reports it as + /// BadNodeIdUnknown. + /// + /// + /// The category comes from the probe first, the authored blob second. The Agent is + /// the authority on what a DataItem is; a blob's mtCategory is optional metadata + /// that may be absent (browse-commit writes only the address) or stale. + /// + /// + private static FrozenDictionary BuildWriteTargets( + IReadOnlyList definitions, + MTConnectProbeModel? probeModel, + string? configuredDeviceName) + { + if (definitions.Count == 0) + { + return FrozenDictionary.Empty; + } + + var owners = IndexProbeOwners(probeModel); + var fallback = string.IsNullOrWhiteSpace(configuredDeviceName) ? null : configuredDeviceName.Trim(); + var targets = new Dictionary(definitions.Count, StringComparer.Ordinal); + + foreach (var definition in definitions) + { + if (definition is null || string.IsNullOrWhiteSpace(definition.FullName)) continue; + + var owner = owners.TryGetValue(definition.FullName, out var declared) ? declared : null; + + targets[definition.FullName] = new WriteTarget( + owner?.DeviceName ?? fallback, + IsCondition(owner?.Category ?? definition.MtCategory)); + } + + return targets.ToFrozenDictionary(StringComparer.Ordinal); + } + + /// + /// Flattens a device model into a dataItemId → owning device lookup; null ⇒ empty. + /// Separate from because this one must remember which device + /// the walk was inside, which a flat DataItem lookup has thrown away. + /// + /// The Agent's device model, or null. + private static FrozenDictionary IndexProbeOwners(MTConnectProbeModel? probeModel) + { + if (probeModel is null) return FrozenDictionary.Empty; + + var map = new Dictionary(StringComparer.Ordinal); + foreach (var device in probeModel.Devices) + { + if (device is null) continue; + + // The same name/id fallback the browse tree uses: an Agent addresses a device by its + // name, and by its id when it declares no name. + var deviceName = FolderName(device.Name, device.Id); + var owner = new DataItemOwner( + string.IsNullOrWhiteSpace(deviceName) ? null : deviceName, Category: null); + + foreach (var dataItem in device.AllDataItems()) + { + if (dataItem is null || string.IsNullOrWhiteSpace(dataItem.Id)) continue; + + map[dataItem.Id] = owner with { Category = dataItem.Category }; + } + } + + return map.ToFrozenDictionary(StringComparer.Ordinal); } /// @@ -3174,11 +3540,45 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis /// . This is how a raw tag's declared (or /// probe-inferred) DriverDataType reaches the coercion. /// + /// + /// How each DataItem is addressed on the Agent's AllowPut surface, and whether it is + /// CONDITION-category — see . Keyed by DataItem id, so it + /// doubles as the write path's "is this a reference I serve at all?" test: a miss is + /// BadNodeIdUnknown, exactly as an index miss is on the read path. + /// private sealed record TagBinding( FrozenDictionary DataItemIdByRawPath, FrozenDictionary> RawPathsByDataItemId, - IReadOnlyList IndexTags) + IReadOnlyList IndexTags, + FrozenDictionary WriteTargetsByDataItemId) { + /// + /// Resolves a write reference to the DataItem id and the Agent address it writes through. + /// + /// The caller's reference (a RawPath under v3, or a DataItem id). + /// The resolved DataItem id; empty when this returns false. + /// Where and whether it can be written; default when false. + /// when no DataItem this driver serves answers to that reference. + /// + /// Stricter than , deliberately. A read may fall through to an + /// index miss and report itself; a write that fell through would POST an arbitrary + /// operator string at the Agent as if it were a DataItem id — the Agent's answer to which + /// is a refusal that reads like a comms fault. Unknown is answered here, before the wire. + /// + public bool TryResolveWriteTarget(string? reference, out string dataItemId, out WriteTarget target) + { + dataItemId = ToDataItemId(reference); + + if (dataItemId.Length != 0 && WriteTargetsByDataItemId.TryGetValue(dataItemId, out target)) + { + return true; + } + + target = default; + + return false; + } + /// /// Translates a subscriber/read reference to the DataItem id the index is keyed by. /// @@ -3218,6 +3618,24 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis } } + /// + /// Everything needs to decide one slot before it touches the network. + /// + /// + /// The device path segment of the Agent PUT URL, or null when neither the probe model + /// nor config names one — which is a refusal, not a default (see ). + /// + /// + /// Whether the DataItem is CONDITION-category. Excluded from v1 writes: a condition PUT carries + /// alarm semantics and a different level|text wire format. + /// + private readonly record struct WriteTarget(string? DeviceName, bool IsCondition); + + /// The device a DataItem was declared under, and the category it was declared with. + /// The owning device's name (or id); null when it declares neither. + /// The DataItem's category attribute as the Agent spelled it. + private sealed record DataItemOwner(string? DeviceName, string? Category); + /// /// One live subscription: its handle and the references it wants. Both are effectively /// immutable once registered, so the pump reads without a lock — diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriverFactoryExtensions.cs index 40faa2bd..cd33a5a0 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriverFactoryExtensions.cs @@ -31,12 +31,20 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// /// /// The instance is returned as its concrete type. The runtime resolves every optional -/// capability (, , +/// capability (, , , /// , , /// ) by pattern-matching the object /// hands back, so nothing here may narrow it — a -/// narrowed return is how a capability ends up implemented but never dispatched. There is -/// deliberately no IWritable: the MTConnect Agent surface is read-only. +/// narrowed return is how a capability ends up implemented but never dispatched. +/// is the standing example: it was added to +/// with no change whatsoever to this file, and would have been +/// silently undispatched had the return type ever been narrowed to . +/// +/// +/// Registering the factory does not enable writing. An MTConnect write is observation +/// injection through the Agent's AllowPut surface and is off at two independent gates +/// — the Agent's agent.cfg and the driver's own write.enabled (default +/// ). See 's remarks. /// /// public static class MTConnectDriverFactoryExtensions diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDiscoverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDiscoverTests.cs index ea6eef6a..0c6da662 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDiscoverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDiscoverTests.cs @@ -45,12 +45,16 @@ public sealed class MTConnectDiscoverTests "dev1_system_cond", ]; - private static MTConnectDriverOptions Opts(string? deviceName = null) => + private static MTConnectDriverOptions Opts(string? deviceName = null, bool writeEnabled = false) => new() { AgentUri = AgentUri, DeviceName = deviceName, + // Off unless a test says otherwise — the shipped default, and what every pre-write-back + // expectation in this file was written against. + Write = new MTConnectWriteOptions { Enabled = writeEnabled }, + // Discovery is a pure read of the /probe device model: it must enumerate what the Agent // declares, NOT what an operator has already authored. Left empty on purpose so a driver // that streamed its authored tag table instead could not pass. @@ -324,10 +328,10 @@ public sealed class MTConnectDiscoverTests } /// - /// MTConnect is a read-only protocol and this driver implements no IWritable, so every - /// browsed leaf is — the tier that is read-only - /// from OPC UA. Nothing is historized by discovery either: historization is authored per tag, - /// never inferred from a device model. + /// Write-back is off by default, so every browsed leaf is + /// — the tier that is read-only from OPC UA, and + /// exactly what this driver shipped. Nothing is historized by discovery either: historization + /// is authored per tag, never inferred from a device model. /// [Fact] public async Task Discover_streams_every_leaf_view_only_and_not_historized() @@ -341,6 +345,47 @@ public sealed class MTConnectDiscoverTests cap.Variables.ShouldAllBe(v => v.Attr.Source == NodeSourceKind.Driver); } + /// + /// With write.enabled on, the same probe model browses + /// — but only for the leaves WriteAsync would actually accept. A browse that advertised a + /// tier the write path refuses is the "silent no-op in a new costume" failure MQTT's + /// CanWrite pins against; the flag is the ONLY difference between this test and the one + /// above, so a driver that ignored it could not pass both. + /// + [Fact] + public async Task Discover_streams_writable_leaves_as_operate_when_write_is_enabled() + { + var (driver, _) = await InitializedDriverAsync(Opts(writeEnabled: true)); + + var cap = await DiscoverAsync(driver); + + var writable = cap.Variables.Where(v => !v.Attr.IsAlarm).ToList(); + + // Counted, not just ShouldAllBe: an empty sequence satisfies ShouldAllBe vacuously, so a + // discovery that streamed nothing at all would otherwise pass this test. + writable.Count.ShouldBe(FixtureDataItemIds.Length - 1); + writable.ShouldAllBe(v => v.Attr.SecurityClass == SecurityClassification.Operate); + } + + /// + /// A CONDITION data item stays in both + /// modes: a condition PUT carries alarm semantics and a different level|text wire + /// format, so the write path refuses it and the browse must say so. + /// + /// Whether the driver's write-back block is switched on. + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Discover_streams_a_condition_leaf_view_only_whether_or_not_write_is_enabled(bool writeEnabled) + { + var (driver, _) = await InitializedDriverAsync(Opts(writeEnabled: writeEnabled)); + + var cap = await DiscoverAsync(driver); + + cap.Variables.Single(v => v.Attr.FullName == "dev1_system_cond") + .Attr.SecurityClass.ShouldBe(SecurityClassification.ViewOnly); + } + // ---- browse names ---- /// diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectFactoryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectFactoryTests.cs index 6184bc5f..0f7ee5c7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectFactoryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectFactoryTests.cs @@ -207,19 +207,25 @@ public sealed class MTConnectFactoryTests } /// - /// The other half of the capability pin: MTConnect's Agent surface is read-only, so the - /// instance must NOT be . If it ever became writable, the write gate - /// would start offering an operation the protocol cannot honour. + /// The other half of the capability pin: the instance IS , so the + /// runtime's write gate dispatches to it. This is the seam the concrete return type exists for + /// — the capability was added to the driver with no change to the factory, and a narrowed + /// return would have left it implemented but never dispatched. /// + /// + /// Dispatchable is not enabled. The config below authors no write block, so this + /// instance refuses every write with BadNotWritable — and the Agent's own + /// AllowPut is a second, independent gate. See MTConnectWriteTests. + /// [Fact] - public void Registered_delegate_returns_an_instance_that_is_not_writable() + public void Registered_delegate_returns_an_instance_that_dispatches_writes() { var registry = new DriverFactoryRegistry(); MTConnectDriverFactoryExtensions.Register(registry); IDriver driver = registry.TryGet("MTConnect")!("mt1", "{\"agentUri\":\"http://a:5000\"}"); - (driver is IWritable).ShouldBeFalse("the MTConnect Agent surface is read-only"); + (driver is IWritable).ShouldBeTrue("MTConnect must dispatch writes (the Agent AllowPut surface)"); } /// Registering into a null registry is a wiring bug, not a silent no-op. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs new file mode 100644 index 00000000..89b30f14 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs @@ -0,0 +1,356 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests; + +/// +/// 's leg — the batch contract, the per-slot +/// refusals, and the one thing the driver has to get right before the Agent is even dialled: which +/// device a DataItem belongs to. +/// +/// +/// +/// The write itself is the live gate's job. Everything below stops at the +/// seam and asserts what the driver asked the Agent +/// for (), never that a fake was called for its own sake — +/// the same philosophy as MqttDriverWriteTests. What IS worth pinning offline is +/// everything that decides whether a write should reach an Agent at all, because that is where +/// a silent no-op would hide. +/// +/// +/// A write is observation injection, not machine command (design §2.2): it sets what the +/// Agent reports for a DataItem, and the next adapter update overwrites it. That is why the +/// feature is off at two independent gates — the Agent's AllowPut, and this driver's +/// write.enabled — and why the disabled leg below is the first test in the file. +/// +/// +public sealed class MTConnectWriteTests +{ + private const string AgentUri = "http://fixture-agent:5000"; + + /// The device the canned Fixtures/probe.xml declares — id="dev1" name="VMC-3Axis". + private const string FixtureDeviceName = "VMC-3Axis"; + + private const string PositionDataItemId = "dev1_pos"; + private const string ConditionDataItemId = "dev1_system_cond"; + + private const string PositionRawPath = "Plant/MTConnect/dev1/Position"; + private const string ConditionRawPath = "Plant/MTConnect/dev1/SystemCondition"; + private const string UnknownRawPath = "Plant/MTConnect/dev1/NotATag"; + + private const uint Good = 0x00000000u; + private const uint BadNodeIdUnknown = 0x80340000u; + private const uint BadNotWritable = 0x803B0000u; + private const uint BadTypeMismatch = 0x80740000u; + private const uint BadCommunicationError = 0x80050000u; + private const uint BadNotConnected = 0x808A0000u; + + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + /// One authored raw tag bound to a DataItem id, as the browse-commit flow writes it. + private static RawTagEntry Entry(string rawPath, string dataItemId) => + new(rawPath, $$"""{"fullName":"{{dataItemId}}"}""", WriteIdempotent: false, DeviceName: "dev1"); + + private static MTConnectDriverOptions Opts( + bool writeEnabled = true, string? deviceName = null, params RawTagEntry[] rawTags) => + new() + { + AgentUri = AgentUri, + DeviceName = deviceName, + Tags = [], + RawTags = rawTags, + Write = new MTConnectWriteOptions { Enabled = writeEnabled }, + }; + + private static (MTConnectDriver Driver, CannedAgentClient Client) NewDriver( + MTConnectDriverOptions options, RecordingDriverLogger? logger = null) + { + var client = CannedAgentClient.FromFixtures(); + + return (new MTConnectDriver(options, "mt1", _ => client, logger), client); + } + + private static async Task<(MTConnectDriver Driver, CannedAgentClient Client)> InitializedAsync( + MTConnectDriverOptions options, RecordingDriverLogger? logger = null) + { + var (driver, client) = NewDriver(options, logger); + await driver.InitializeAsync("{}", Ct); + + return (driver, client); + } + + /// The ordinary rig: write on, one writable position tag and the fixture's CONDITION item. + private static Task<(MTConnectDriver Driver, CannedAgentClient Client)> WritableDriverAsync( + RecordingDriverLogger? logger = null) => + InitializedAsync( + Opts( + writeEnabled: true, + deviceName: null, + Entry(PositionRawPath, PositionDataItemId), + Entry(ConditionRawPath, ConditionDataItemId)), + logger); + + // ---- the capability itself ---- + + [Fact] + public void MTConnectDriver_implements_IWritable() + // The interface's presence IS the feature: without it the runtime's capability dispatch never + // offers this driver a write, whatever a node advertises. + => NewDriver(Opts()).Driver.ShouldBeAssignableTo(); + + // ---- the two independent gates ---- + + [Fact] + public async Task A_driver_with_write_disabled_refuses_every_slot_without_dialling_the_agent() + { + // The default. An existing deployment that never authors the block keeps today's read-only + // behaviour EXACTLY — including making no request an Agent could log or accept. + var (driver, client) = await InitializedAsync( + Opts(writeEnabled: false, deviceName: null, Entry(PositionRawPath, PositionDataItemId))); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct); + + results.Count.ShouldBe(1); + results[0].StatusCode.ShouldBe(BadNotWritable); + client.Writes.ShouldBeEmpty(); + } + + [Fact] + public async Task A_write_before_initialize_reports_not_connected() + { + // Matches ReadAsync: "this driver is not running" is the only true statement available, and it + // is NOT the same operator problem as an Agent that answered and refused. + var (driver, _) = NewDriver(Opts(true, null, Entry(PositionRawPath, PositionDataItemId))); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct); + + results[0].StatusCode.ShouldBe(BadNotConnected); + } + + [Fact] + public async Task A_write_after_shutdown_reports_not_connected() + { + var (driver, _) = await WritableDriverAsync(); + await driver.ShutdownAsync(Ct); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct); + + results[0].StatusCode.ShouldBe(BadNotConnected); + } + + // ---- per-slot refusals ---- + + [Fact] + public async Task An_unknown_reference_is_refused_in_its_own_slot() + { + var (driver, client) = await WritableDriverAsync(); + + var results = await driver.WriteAsync([new WriteRequest(UnknownRawPath, 1.0)], Ct); + + results[0].StatusCode.ShouldBe(BadNodeIdUnknown); + client.Writes.ShouldBeEmpty(); + } + + [Fact] + public async Task A_condition_data_item_is_refused_as_not_writable() + { + // CONDITION PUTs carry alarm semantics and a different level|text wire format — excluded from + // v1. BadNotWritable, not BadTypeMismatch: the value was never the problem. + var (driver, client) = await WritableDriverAsync(); + + var results = await driver.WriteAsync([new WriteRequest(ConditionRawPath, "NORMAL")], Ct); + + results[0].StatusCode.ShouldBe(BadNotWritable); + client.Writes.ShouldBeEmpty(); + } + + [Fact] + public async Task A_null_value_is_refused_as_a_type_mismatch() + { + // A write IS a URL query value; there is no null to send. Refusing beats writing the empty + // string, which the Agent would accept as a real observation. + var (driver, client) = await WritableDriverAsync(); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, null)], Ct); + + results[0].StatusCode.ShouldBe(BadTypeMismatch); + client.Writes.ShouldBeEmpty(); + } + + [Fact] + public async Task An_array_value_is_refused_as_a_type_mismatch() + { + // Convert.ToString would happily render "System.Double[]" and the Agent would happily store it: + // a silent corruption visible only on read-back. Array/TIME_SERIES writes are out of scope. + var (driver, client) = await WritableDriverAsync(); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, new[] { 1.0, 2.0 })], Ct); + + results[0].StatusCode.ShouldBe(BadTypeMismatch); + client.Writes.ShouldBeEmpty(); + } + + // ---- what actually reaches the seam ---- + + [Fact] + public async Task An_accepted_write_reports_good_and_names_the_owning_device_and_data_item() + { + // The device NAME is resolved from the /probe model, not from config: the Agent's PUT surface + // is addressed per device, and a multi-device Agent must be addressed correctly with no extra + // authoring. The RawPath is translated to the DataItem id, exactly as the read path does. + var (driver, client) = await WritableDriverAsync(); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct); + + results[0].StatusCode.ShouldBe(Good); + client.Writes.ShouldHaveSingleItem() + .ShouldBe((FixtureDeviceName, PositionDataItemId, "42.5")); + } + + [Fact] + public async Task A_value_is_rendered_with_the_invariant_culture() + { + // A comma decimal separator would be accepted by the Agent and read back as a different number. + var (driver, client) = await WritableDriverAsync(); + + await driver.WriteAsync([new WriteRequest(PositionRawPath, 1234.5678d)], Ct); + + client.Writes.ShouldHaveSingleItem().Value.ShouldBe("1234.5678"); + } + + [Fact] + public async Task A_boolean_is_rendered_as_its_invariant_true_false_string() + { + // Boolean.ToString is culture-independent, so the invariant rendering is "True"/"False" — NOT + // "1"/"0". Documented because it is the value an MTConnect consumer will see verbatim. + var (driver, client) = await WritableDriverAsync(); + + await driver.WriteAsync([new WriteRequest(PositionRawPath, true)], Ct); + + client.Writes.ShouldHaveSingleItem().Value.ShouldBe("True"); + } + + // ---- device-name resolution ---- + + [Fact] + public async Task A_data_item_the_probe_does_not_declare_falls_back_to_the_configured_device() + { + // The dataItemId-keyed authoring surface (the driver CLI) and any tag whose DataItem the Agent + // has since dropped: config is the only device name available, and it is a real one. + var (driver, client) = await InitializedAsync( + Opts(writeEnabled: true, deviceName: "ConfiguredDevice", Entry(PositionRawPath, "not_in_probe"))); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 7)], Ct); + + results[0].StatusCode.ShouldBe(Good); + client.Writes.ShouldHaveSingleItem().ShouldBe(("ConfiguredDevice", "not_in_probe", "7")); + } + + [Fact] + public async Task A_data_item_with_no_resolvable_device_is_refused_as_an_unknown_node() + { + // Neither the probe model nor config names a device, so there is no URL to POST to. Refused + // rather than guessed: a wrong device path is a 404 the operator reads as "the Agent is broken". + var (driver, client) = await InitializedAsync( + Opts(writeEnabled: true, deviceName: null, Entry(PositionRawPath, "not_in_probe"))); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 7)], Ct); + + results[0].StatusCode.ShouldBe(BadNodeIdUnknown); + client.Writes.ShouldBeEmpty(); + } + + // ---- how the Agent's own answers map ---- + + [Fact] + public async Task An_agent_refusal_maps_to_a_communication_error_and_logs_the_agents_own_text() + { + var logger = new RecordingDriverLogger(); + var (driver, client) = await WritableDriverAsync(logger); + client.WriteOutcome = new MTConnectWriteOutcome(Accepted: false, AgentError: "UNSUPPORTED: no such device"); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct); + + results[0].StatusCode.ShouldBe(BadCommunicationError); + logger.WarningsSnapshot().ShouldContain(w => w.Contains("UNSUPPORTED: no such device", StringComparison.Ordinal)); + } + + [Fact] + public async Task An_agent_refusal_never_reaches_the_health_LastError() + { + // Type-only discipline: LastError carries an exception TYPE, never Agent-authored text, and a + // refused write is not evidence the driver's read path is degraded either. + var (driver, client) = await WritableDriverAsync(); + var before = driver.GetHealth(); + client.WriteOutcome = new MTConnectWriteOutcome(Accepted: false, AgentError: "UNSUPPORTED: no such device"); + + await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct); + + var after = driver.GetHealth(); + after.LastError.ShouldBe(before.LastError); + after.State.ShouldBe(before.State); + } + + [Fact] + public async Task A_transport_failure_maps_to_a_communication_error_without_throwing() + { + var (driver, client) = await WritableDriverAsync(); + client.WriteFailure = new HttpRequestException("connection refused"); + + var results = await driver.WriteAsync([new WriteRequest(PositionRawPath, 42.5)], Ct); + + results[0].StatusCode.ShouldBe(BadCommunicationError); + } + + // ---- batch contract ---- + + [Fact] + public async Task Cancellation_fills_the_remaining_slots_and_returns_the_batch() + { + // Never throws, even cancelled: the caller gets one status per request or the node-manager + // cannot tell which writes were attempted. + var (driver, client) = await WritableDriverAsync(); + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + var results = await driver.WriteAsync( + [new WriteRequest(PositionRawPath, 1.0), new WriteRequest(PositionRawPath, 2.0)], cts.Token); + + results.Count.ShouldBe(2); + results.ShouldAllBe(r => r.StatusCode == BadCommunicationError); + client.Writes.ShouldBeEmpty(); + } + + [Fact] + public async Task One_bad_request_does_not_fail_the_others_in_the_batch() + { + // Mirrors ReadAsync: failing the whole call would let one stale reference reject every other + // write in the same OPC UA request. + var (driver, client) = await WritableDriverAsync(); + + var results = await driver.WriteAsync( + [ + new WriteRequest(UnknownRawPath, 1.0), + new WriteRequest(ConditionRawPath, "NORMAL"), + new WriteRequest(PositionRawPath, 42.5), + ], + Ct); + + // Order-preserving, one status per request, each naming its own distinct cause. + results.Count.ShouldBe(3); + results[0].StatusCode.ShouldBe(BadNodeIdUnknown); + results[1].StatusCode.ShouldBe(BadNotWritable); + results[2].StatusCode.ShouldBe(Good); + client.Writes.ShouldHaveSingleItem().DataItemId.ShouldBe(PositionDataItemId); + } + + [Fact] + public async Task An_empty_batch_is_answered_with_an_empty_result() + => (await NewDriver(Opts()).Driver.WriteAsync([], Ct)).ShouldBeEmpty(); + + [Fact] + public async Task WriteAsync_rejects_a_null_batch() + => await Should.ThrowAsync( + async () => await NewDriver(Opts()).Driver.WriteAsync(null!, Ct)); +} From b833dda673a8c0040ecefba3655b0ae9473d89b1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 06:48:49 -0400 Subject: [PATCH 6/9] fix(mtconnect): a throwing value formatter cannot escape the write batch TryFormatValue's catch filter named only FormatException / InvalidCastException / OverflowException, and the call sits OUTSIDE WriteAsync's per-slot try. But Convert.ToString falls back to the value's OWN ToString/IFormattable - arbitrary caller code that may throw anything - so any other exception escaped WriteAsync uncaught and failed the entire batch, breaking the documented never-throws per-slot contract. Broadened to a catch-all; the slot gets BadTypeMismatch and the exception is logged at Warning (TryFormatValue became an instance method to reach _logger). One unformattable value now costs its own slot and nothing more. Falsification checked, not assumed: restoring the narrow filter makes the new test fail with a raw "System.InvalidOperationException : this formatter is broken" propagating out of WriteAsync - i.e. it fails for the contract-breach reason, not on an assertion. Two tests added: - a value whose ToString throws InvalidOperationException is refused BadTypeMismatch while the *following* good slot still writes (the load-bearing half - it proves the batch survived). - mid-batch cancellation: cancellation injected from inside slot 1's own formatting, so slot 0 has provably already reached the Agent. It keeps its real Good outcome while slots 1-2 fill BadCommunicationError. Reporting a landed write as failed is worse than reporting a failure, and a fill that started at 0 instead of i would do exactly that. Tests: 534 passed / 0 failed (was 532). Solution build: 0 errors, no warnings in any MTConnect project. --- .../MTConnectDriver.cs | 24 ++++++- .../MTConnectWriteTests.cs | 69 +++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index d8324306..1a2e8285 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -895,8 +895,16 @@ public sealed class MTConnectDriver /// happily, making the corruption invisible until someone reads the observation back. /// TIME_SERIES / DATA_SET writes are out of scope for v1. /// + /// + /// Every formatting throw is caught, not just the documented conversion ones. The + /// rendering runs the value's own / + /// , which is arbitrary code — and this runs OUTSIDE + /// 's per-slot try, so anything that escapes here escapes + /// the whole batch and breaks its never-throws contract. It costs its own slot a + /// BadTypeMismatch and is logged. + /// /// - private static bool TryFormatValue(object? value, out string text) + private bool TryFormatValue(object? value, out string text) { text = string.Empty; @@ -912,8 +920,20 @@ public sealed class MTConnectDriver { rendered = Convert.ToString(value, CultureInfo.InvariantCulture); } - catch (Exception ex) when (ex is FormatException or InvalidCastException or OverflowException) + catch (Exception ex) { + // Catch-all, and deliberately so. Convert.ToString falls back to the value's OWN + // ToString/IFormattable, which is arbitrary caller code that may throw ANYTHING — and this + // call sits outside WriteAsync's per-slot try, so a narrower filter (FormatException / + // InvalidCastException / OverflowException) let every other exception escape WriteAsync + // uncaught and break the documented never-throws contract for the whole batch. One + // unformattable value must cost its own slot, nothing more. + _logger.LogWarning( + ex, + "MTConnect driver {DriverInstanceId} could not render a write value of type {ValueType} to its invariant string form; the write is refused as a type mismatch.", + _driverInstanceId, + value.GetType()); + return false; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs index 89b30f14..936f3b6b 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectWriteTests.cs @@ -191,6 +191,36 @@ public sealed class MTConnectWriteTests client.Writes.ShouldBeEmpty(); } + [Fact] + public async Task A_value_whose_own_formatter_throws_costs_its_slot_and_nothing_more() + { + // Convert.ToString runs the value's OWN ToString — arbitrary code that may throw anything — + // and it runs OUTSIDE WriteAsync's per-slot try. A narrow catch filter let every exception + // other than Format/InvalidCast/Overflow escape WriteAsync and fail the WHOLE batch, breaking + // the never-throws contract. The load-bearing half of this test is the second slot: the good + // write still happens. + var (driver, client) = await WritableDriverAsync(); + + var results = await driver.WriteAsync( + [ + new WriteRequest(PositionRawPath, new ThrowingFormatter()), + new WriteRequest(PositionRawPath, 42.5), + ], + Ct); + + results.Count.ShouldBe(2); + results[0].StatusCode.ShouldBe(BadTypeMismatch); + results[1].StatusCode.ShouldBe(Good); + client.Writes.ShouldHaveSingleItem().Value.ShouldBe("42.5"); + } + + /// A value whose throws something no conversion filter would name. + private sealed class ThrowingFormatter + { + public override string ToString() => + throw new InvalidOperationException("this formatter is broken"); + } + // ---- what actually reaches the seam ---- [Fact] @@ -322,6 +352,45 @@ public sealed class MTConnectWriteTests client.Writes.ShouldBeEmpty(); } + [Fact] + public async Task Cancellation_mid_batch_leaves_the_slots_already_written_with_their_real_outcomes() + { + // The fill must start at the cancelled slot, not overwrite the batch. Slot 0 completed against + // the Agent before the token was cancelled, and reporting it BadCommunicationError would tell + // an operator a write failed that actually landed — the one thing worse than reporting a + // failure. Cancellation is injected from inside slot 1's own formatting, so the ordering is a + // fact of the run rather than a timing hope. + var (driver, client) = await WritableDriverAsync(); + using var cts = new CancellationTokenSource(); + + var results = await driver.WriteAsync( + [ + new WriteRequest(PositionRawPath, 42.5), + new WriteRequest(PositionRawPath, new CancellingFormatter(cts)), + new WriteRequest(PositionRawPath, 7.5), + ], + cts.Token); + + results.Count.ShouldBe(3); + results[0].StatusCode.ShouldBe(Good); + results[1].StatusCode.ShouldBe(BadCommunicationError); + results[2].StatusCode.ShouldBe(BadCommunicationError); + + // Exactly the pre-cancellation slot reached the Agent. + client.Writes.ShouldHaveSingleItem().Value.ShouldBe("42.5"); + } + + /// Cancels the batch from inside its own rendering, then renders normally. + private sealed class CancellingFormatter(CancellationTokenSource cts) + { + public override string ToString() + { + cts.Cancel(); + + return "99"; + } + } + [Fact] public async Task One_bad_request_does_not_fail_the_others_in_the_batch() { From 203000a80aca269072bfae443fcc247c0a9cb8cf Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 06:55:14 -0400 Subject: [PATCH 7/9] docs: MTConnect write-back recorded; stale MQTT write-through/browse-gap records reconciled --- deferment.md | 71 +++++++-- docs/drivers/MTConnect.md | 135 ++++++++++++++++-- docs/drivers/Mqtt.md | 15 +- .../2026-07-15-mtconnect-driver-design.md | 15 ++ .../2026-07-24-driver-expansion-tracking.md | 96 +++++++++---- 5 files changed, 270 insertions(+), 62 deletions(-) diff --git a/deferment.md b/deferment.md index d0499579..7d9a8bd7 100644 --- a/deferment.md +++ b/deferment.md @@ -123,14 +123,21 @@ reflection in `tests/Core/…Core.Abstractions.Tests/DriverTypeNamesGuardTests.c | Driver | Verdict | Merged | Capabilities | Notes | |---|---|---|---|---| -| **MTConnect** | Complete-with-gaps | `90bdaa44` (#506) | `IDriver, IReadable, ISubscribable, ITagDiscovery, IHostConnectivityProbe, IRediscoverable` (`MTConnectDriver.cs:63`) | No `IWritable` **by design** (`:50`). No device form. | -| **MQTT / Sparkplug B** | **Complete** | `c3a2b0f7` | `+ IRediscoverable, IAsyncDisposable` (`MqttDriver.cs:65-66`) | Cleanest of the five: picker + driver form + device form + tag editor all present. 8 open follow-up issues. | +| **MTConnect** | Complete-with-gaps | `90bdaa44` (#506) | `IDriver, IReadable, ISubscribable, ITagDiscovery, IHostConnectivityProbe, IRediscoverable` (`MTConnectDriver.cs:63`) | ~~No `IWritable` **by design** (`:50`).~~ ✅ **`IWritable` added 2026-08-05 (`b29623cf`)** — observation injection via the Agent's `AllowPut` surface, off by default at two gates (`Write.Enabled` + the Agent's own `AllowPut`); MTConnect *Interfaces* stays deferred. No device form. | +| **MQTT / Sparkplug B** | **Complete** | `c3a2b0f7` | `+ IRediscoverable, IAsyncDisposable` (`MqttDriver.cs:65-66`) — **and `IWritable` since `72be1922`** | Cleanest of the five: picker + driver form + device form + tag editor all present. ~~8 open follow-up issues.~~ ✅ All eight (#508–#515) **closed 2026-07-31**. | | **Sql** | Complete-with-gaps | `4ad54037`, follow-ups `28c28667` | `IDriver, ITagDiscovery, IReadable, ISubscribable, IHostConnectivityProbe` (`SqlDriver.cs:28-29`) | No device form; no browse-commit branch; stale comments. | | **Modbus RTU-over-TCP** | **Complete** | `0f38f486` (#495) | transport mode inside Modbus (`ModbusTransportMode.cs`, `ModbusRtuOverTcpTransport.cs`) | Parses transport **by name** (`ModbusDriverFactoryExtensions.cs:77-78`) — dodges the systemic numeric-enum authoring bug. | | **Calculation** | **Partial (authoring gap)** | pre-existing | `IDriver, IDependencyConsumer, ISubscribable, IReadable` (`CalculationDriver.cs:38`) | **Registered + offered in the picker, but has no config form.** See §1.2. | -`IWritable` is absent from MTConnect, MQTT, Sql and Calculation. In all four this is **structural and -documented**, not a stub (`MTConnectDriver.cs:50`, `MqttDriver.cs:483`, `SqlDriver.cs:15`). +~~`IWritable` is absent from MTConnect, MQTT, Sql and Calculation. In all four this is **structural and +documented**, not a stub (`MTConnectDriver.cs:50`, `MqttDriver.cs:483`, `SqlDriver.cs:15`).~~ + +✅ **Two of the four gained it since the audit. As of 2026-08-05 `IWritable` is absent from Sql and +Calculation only** — and in both that absence is still structural and documented (`SqlDriver.cs:15`). +**MQTT** gained it 2026-07-31 (`72be1922`, +[#508](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/508), live-gated — plain `writeTopic` +publish + Sparkplug NCMD/DCMD); **MTConnect** gained it 2026-08-05 (`b29623cf` — observation injection +through the Agent's `AllowPut` surface, off by default at two independent gates). **Zero `NotImplementedException`** across all five drivers. Every `NotSupportedException` is either a `catch` filter or a deliberate domain error. @@ -195,14 +202,14 @@ incomplete), 2 CANNOT VERIFY (correctly filed as hardware/fixture-gated). | **519** | No Bad/Uncertain **sub-code** reaches a client | **CONFIRMED** | Collapse at `DriverInstanceActor.cs:1033-1041` (`statusCode >> 30`), re-expand at `OtOpcUaNodeManager.cs:3318-3322`. Enum is 3-state at `IOpcUaAddressSpaceSink.cs:165`. **Wider than filed** — `StatusFromQuality` also used at `:429/:506/:576/:761`, so alarm-condition Quality collapses too. | Design consequence | | **517** | Health published AFTER teardown | **CONFIRMED, fleet-wide** | `ModbusDriver.cs:244-250`, `MTConnectDriver.cs:579-590`, `S7Driver.cs:292-326` — all `Teardown…` then `WriteHealth(Unknown)`; `GetHealth()` is a plain field read | Defect (race) | | **516** ✅ | Driver config edits silently discarded — **RESOLVED `d32d89c3` + `5184a2e1`: seam respawn + in-place re-parse; all 12 drivers now honour a changed config. Stale-open on tracker.** | **CONFIRMED — survey now complete** | `DriverInstanceActor.cs:316` (only `_driver` assignment) + `DriverHostActor.cs:2565` (`Tell` existing child, no respawn) + `:653/:660` (green seal). **Affected: Modbus, FOCAS, OpcUaClient, + AbLegacy (issue missed it), + Sql (deliberate/documented).** AbCip/TwinCAT re-parse ✅; Galaxy fails **loud** (throws, `GalaxyDriver.cs:600-641`) | Defect | -| **515** | Sparkplug `DataSet`/`Template`/`PropertySet` | **CONFIRMED** | `SparkplugCodec.cs:207-210` decode to `Unsupported` — visible refusal, not silent drop. *(`PropertySet` isn't a `ValueOneofCase`; it rides `metric.Properties`)* | Deferred feature | -| **514** | Rebirth unarmable on a plant that hasn't birthed | **CONFIRMED** | `RawBrowseModal.razor:119-120` disabled; `_rebirthTarget` set only by tree-click handlers (`:432`, `:437`, `:439-451`). Empty tree ⇒ permanently disabled | Defect (chicken-and-egg) | -| **513** | Meaningless `payloadFormat` in Sparkplug blob | **CONFIRMED** | `MqttTagConfigModel.cs:210` unconditional `Set`, vs. mode-aware `:216` | Defect (cosmetic) | -| **512** | `.Browser` → `.Driver` layering | **CONFIRMED** | `…Mqtt.Browser.csproj:43` (**not :44**), boxed "KNOWN, DELIBERATE EXCEPTION" at `:16-42` | Deferred refactor | -| **511** | `ActAsPrimaryHost` does nothing | **CONFIRMED** | Option at `MqttDriverOptions.cs:193`, round-tripped in the UI, handled only by a warning at `MqttDriver.cs:933-948`. No STATE publish, no Last Will | Deferred feature | -| **510** | `_canRebirth` captured at browse-open | **CONFIRMED** | `RawBrowseModal.razor:377` sole `true` assignment; failure path `:516-539` never re-evaluates | Defect (stale affordance) | -| **509** | Metric name with `/` unbrowse-committable | **CONFIRMED** | `RawBrowseCommitMapper.cs:63` uses browse name verbatim → `RawPaths.cs:39` rejects `/` → `RawTreeService.cs:976` **all-or-nothing** kills the whole batch | Defect | -| **508** | MQTT write-through | **CONFIRMED** | `MqttDriver.cs:66` — no `IWritable`; consequence at `DriverInstanceActor.cs:673-677` | Deferred feature | +| **515** ✅ | Sparkplug `DataSet`/`Template`/`PropertySet` — **RESOLVED `c74ccc55`: structured metrics flattened to sibling tags; live-gated.** | **CONFIRMED** | `SparkplugCodec.cs:207-210` decode to `Unsupported` — visible refusal, not silent drop. *(`PropertySet` isn't a `ValueOneofCase`; it rides `metric.Properties`)* | Deferred feature | +| **514** ✅ | Rebirth unarmable on a plant that hasn't birthed — **RESOLVED `7f53bd96`: the browse modal takes a typed `group/edgeNode` scope when no tree scope is armed (edge node only, never a group broadcast).** | **CONFIRMED** | `RawBrowseModal.razor:119-120` disabled; `_rebirthTarget` set only by tree-click handlers (`:432`, `:437`, `:439-451`). Empty tree ⇒ permanently disabled | Defect (chicken-and-egg) | +| **513** ✅ | Meaningless `payloadFormat` in Sparkplug blob — **RESOLVED `7f53bd96`.** | **CONFIRMED** | `MqttTagConfigModel.cs:210` unconditional `Set`, vs. mode-aware `:216` | Defect (cosmetic) | +| **512** ✅ | `.Browser` → `.Driver` layering — **RESOLVED `7f53bd96`.** | **CONFIRMED** | `…Mqtt.Browser.csproj:43` (**not :44**), boxed "KNOWN, DELIBERATE EXCEPTION" at `:16-42` | Deferred refactor | +| **511** ✅ | `ActAsPrimaryHost` does nothing — **RESOLVED `7f2108dc`: Sparkplug primary-host STATE publishing + Will, Primary-gated; live-gated.** | **CONFIRMED** | Option at `MqttDriverOptions.cs:193`, round-tripped in the UI, handled only by a warning at `MqttDriver.cs:933-948`. No STATE publish, no Last Will | Deferred feature | +| **510** ✅ | `_canRebirth` captured at browse-open — **RESOLVED `7f53bd96`.** | **CONFIRMED** | `RawBrowseModal.razor:377` sole `true` assignment; failure path `:516-539` never re-evaluates | Defect (stale affordance) | +| **509** ✅ | Metric name with `/` unbrowse-committable — **RESOLVED `7f53bd96`: `RawBrowseCommitMapper.ToRawSegment` sanitises the segment and refuses a collision in words.** | **CONFIRMED** | `RawBrowseCommitMapper.cs:63` uses browse name verbatim → `RawPaths.cs:39` rejects `/` → `RawTreeService.cs:976` **all-or-nothing** kills the whole batch | Defect | +| **508** ✅ | MQTT write-through — **RESOLVED `72be1922`: `IWritable` shipped (plain `writeTopic` publish + Sparkplug NCMD/DCMD), live-gated 2026-07-31. The evidence column is the audit snapshot; `MqttDriver.cs:66` now names `IWritable`.** | **CONFIRMED** | `MqttDriver.cs:66` — no `IWritable`; consequence at `DriverInstanceActor.cs:673-677` | Deferred feature | | **507**→ see above | | | | | | **491** | Continuous-historization live gate | **CANNOT VERIFY** (needs VPN'd gateway) | Code **is** complete: `AddressSpaceApplier.cs:238`→`:540`→`ActorHistorizedTagSubscriptionSink.cs:27-38`→`ContinuousHistorizationRecorder.cs:84`, wired `ServiceCollectionExtensions.cs:435` | Deferred test | | **489** | Hot-rebind renamed raw tag | **CONFIRMED** | No implementation exists | Deferred feature | @@ -1250,6 +1257,46 @@ drivers), 3 carried-open archreview findings led by 01/S-1 (`AddressSpaceApplier --- +### 2026-08-05 — MTConnect write-back; the MQTT closure records reconciled + +Recorded from branch `feature/mqtt-mtconnect-write`. This is exactly the drift shape the 2026-08-04 +entry named — **work that closes an item elsewhere leaves the register asserting the item is open** — +so it is written back here in the same change as the work. + +**§1.1 corrected on two rows.** `IWritable` is no longer absent from four drivers; **after this branch +it is absent from Sql and Calculation only**, and in both that is still structural. + +| Driver | Gained `IWritable` | Commit | Shape | +|---|---|---|---| +| **MQTT / Sparkplug B** | 2026-07-31 ([#508](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/508), live-gated) | `72be1922` | Plain `writeTopic` publish + Sparkplug NCMD/DCMD | +| **MTConnect** | 2026-08-05 | `b29623cf` | **Observation injection** through the Agent's optional `AllowPut` surface — off by default at two independent gates (`Write.Enabled` and the Agent's own `AllowPut`), CONDITION items refused | + +⚠️ **MTConnect write-back is not a machine command, and the register should not be read as saying the +driver can now command a CNC.** A write sets what the Agent *reports* for a DataItem; the next +adapter/SHDR update overwrites it. MTConnect *Interfaces* — the mechanism the original deferral +(`2026-07-15-mtconnect-driver-design.md` §3.6) assumed write-back would need — **remains deferred**, and +§3.6 now carries a superseded note saying so. Design: +`docs/plans/2026-08-05-mtconnect-writeback-design.md`. + +⚠️ **It also does not make any existing node writable.** The OPC UA `CurrentWrite` bit is composed from +the authored `Tag.AccessLevel` alone; driver capability is never consulted at compose time. An operator +must author the raw tag **ReadWrite**. Browse's new `Operate` stamp on writable leaves is honest +labelling only — nothing persists `SecurityClass`. + +**§2's eight MQTT rows (#508–#515) marked ✅ in place** with the commit that resolved each (`72be1922`, +`7f53bd96`, `7f2108dc`, `c74ccc55`), following the register's existing convention. Their evidence +columns are left as the audit snapshot — #508's `MqttDriver.cs:66 — no IWritable` in particular is now +false of the tree, and is annotated as such rather than rewritten. + +**Docs reconciled in the same change** (they contradicted the code, and each other): +`docs/drivers/MTConnect.md` (capability surface, new §Write-back, Deferred, the `DeviceName` +double-duty note), `docs/drivers/Mqtt.md` (deleted the #514 "empty tree cannot arm a rebirth" known-gap +block and the "Read-only. No writes reach the broker" operational note — both contradicted by that +file's own closure line), and `docs/plans/2026-07-24-driver-expansion-tracking.md` (P2 gap bullets + +§Next actions, which still listed MQTT P3 and both browse gaps as remaining work). + +--- + --- *§1–§8 generated 2026-07-27 against `master` @ `90bdaa44` by five parallel source-verification agents. diff --git a/docs/drivers/MTConnect.md b/docs/drivers/MTConnect.md index e694b5eb..4756c00a 100644 --- a/docs/drivers/MTConnect.md +++ b/docs/drivers/MTConnect.md @@ -15,7 +15,7 @@ fixture recipe read ## What it talks to -A **MTConnect Agent** — the vendor-neutral read-only telemetry endpoint that +A **MTConnect Agent** — the vendor-neutral telemetry endpoint that front-ends a machine tool (or fronts an adapter that itself talks to the machine over SHDR). The driver speaks plain HTTP + XML against the Agent's three standard REST paths: @@ -25,8 +25,13 @@ three standard REST paths: - `/sample` — a `multipart/x-mixed-replace` long-poll stream of observation deltas, keyed by a monotonic sequence number -v1 is **agent-first and read-only** — Discover + Read + Subscribe, no Write — -because the mainstream Agent surface has no "set value" operation. +Those three are the whole data plane: **agent-first** — Discover + Read + +Subscribe. There is a fourth path, and it is not part of the data plane: the +Agent's optional **`AllowPut`** surface, off by default in `agent.cfg`, which +this driver's `IWritable` uses (`b29623cf`, 2026-08-05). It sets what the Agent +*reports* for a DataItem; nothing reaches the machine. Read +[Write-back](#write-back--observation-injection-off-by-default) before enabling +it. ## Built vs. planned — read this before trusting the design doc's §2 @@ -68,18 +73,22 @@ below). ## Capability surface -`MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDiscovery, IHostConnectivityProbe, IRediscoverable` +`MTConnectDriver : IDriver, IReadable, IWritable, ISubscribable, ITagDiscovery, IHostConnectivityProbe, IRediscoverable` (`src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs`). -Deliberately **not** `IWritable` — the Agent surface is read-only by design. -Write-back exists only via optional, rarely-deployed MTConnect *Interfaces* -(a request/response handshake, not a "set value" operation), and is out of -scope for this build — see [Deferred](#deferred-not-in-this-build). +`IWritable` landed 2026-08-05 (`b29623cf`), against the Agent's `AllowPut` +surface rather than MTConnect *Interfaces*. It is **observation injection, not +machine command**, and is off at two independent gates by default — read +[Write-back](#write-back--observation-injection-off-by-default) before +authoring a writable MTConnect tag. *Interfaces* itself — the protocol's own +request/response handshake — is a different mechanism and remains deferred; see +[Deferred](#deferred-not-in-this-build). | Capability | Path | Notes | |------------|------|-------| | `ITagDiscovery` | `DiscoverAsync` — streams `/probe`'s Device→Component→DataItem tree into the address-space builder | `SupportsOnlineDiscovery = true`, `RediscoverPolicy = Once` | | `IReadable` | `ReadAsync` → one `/current` per call | **Not the production data path** — see below | +| `IWritable` | `WriteAsync` → `POST {agent}/{device}?{dataItemId}={value}` on the Agent's `AllowPut` surface | **Off by default** — needs `Write.Enabled` **and** the Agent's `AllowPut`; CONDITION items are refused | | `ISubscribable` | `SubscribeAsync`/`OnDataChange` — the shared `/sample` long-poll pump | **The production data path** | | `IHostConnectivityProbe` | periodic `/probe` under `Probe.*` | No consumer wires it today — see [Known limitations](#known-limitations) | | `IRediscoverable` | watches the Agent's `Header.instanceId` | No consumer wires it today — see [Known limitations](#known-limitations) | @@ -105,6 +114,7 @@ correct and tested, just not on the hot path. "HeartbeatMs": 10000, "Probe": { "Enabled": true, "IntervalMs": 5000, "TimeoutMs": 2000 }, "Reconnect": { "MinBackoffMs": 0, "MaxBackoffMs": 30000, "BackoffMultiplier": 2.0 }, + "Write": { "Enabled": false }, "Tags": [] } } @@ -123,13 +133,14 @@ Read off `MTConnectDriverOptions` / `MTConnectDriverConfigDto` | Key | Default | Notes | |---|---|---| | `AgentUri` | — (required) | Agent base URI; the driver appends `/probe`, `/current`, `/sample` | -| `DeviceName` | `null` | Scopes requests to `{AgentUri}/{DeviceName}/...`; `null` = whole Agent | +| `DeviceName` | `null` | **Double duty.** (1) Scopes requests to `{AgentUri}/{DeviceName}/...`; `null` = whole Agent. (2) The write path's fallback device — the device a write is addressed to when the probe model does not say which device declared the DataItem (see [Write-back](#write-back--observation-injection-off-by-default)) | | `RequestTimeoutMs` | `5000` | Per-call deadline for `/probe` and `/current` | | `SampleIntervalMs` | `1000` | The Agent's `/sample?interval=` query param | | `SampleCount` | `1000` | The Agent's `/sample?count=` query param | | `HeartbeatMs` | `10000` | The Agent's `/sample?heartbeat=` query param — also the watchdog's liveness window | | `Probe.Enabled` / `Probe.IntervalMs` / `Probe.TimeoutMs` | `true` / `5000` / `2000` | Background connectivity-probe knobs (mirrors `ModbusProbeOptions`) | | `Reconnect.MinBackoffMs` / `MaxBackoffMs` / `BackoffMultiplier` | `0` / `30000` / `2.0` | Geometric backoff after a failed request or a dropped `/sample` stream | +| `Write.Enabled` | `false` | The driver-side half of the write-back opt-in. An absent block means disabled — see [Write-back](#write-back--observation-injection-off-by-default) | | `Tags[]` | `[]` | Pre-v3 / CLI authoring surface — one `MTConnectTagDefinition` per DataItem `id` | | `RawTags[]` | `[]` (deploy-injected) | The v3 data-plane binding — see below | @@ -224,6 +235,84 @@ the two need disambiguating, not just OR-ing together). On a changed `/hosts` page — see [Known limitations](#known-limitations) for what that does and does not do. +## Write-back — observation injection, off by default + +> ⚠️ **A write sets what the Agent *reports*, not what the machine does.** The value +> lands in the Agent's observation buffer exactly as if an adapter had reported it — +> visible to **every** consumer of that Agent, and **overwritten by the next +> adapter/SHDR update for that DataItem**. Nothing reaches the CNC. Never document, +> script, or operate this as a machine command. It is genuinely useful for +> operator-set EVENT values, for external systems feeding DataItems the adapter does +> not own, and for test rigs — and nothing else. + +The mechanism is the Agent's **`AllowPut`** surface (`POST +{agent}/{device}?{dataItemId}={value}`), a standard cppagent feature that is **off by +default**. It is not MTConnect *Interfaces* — see [Deferred](#deferred-not-in-this-build) +for why Interfaces was rejected as the write-back shape. Design record: +[`2026-08-05-mtconnect-writeback-design.md`](../plans/2026-08-05-mtconnect-writeback-design.md) +§2.2; implementation `b29623cf`. + +### Two independent gates, both off by default + +1. **The Agent** — `AllowPut = true` in `agent.cfg`. Optionally scope it with + `AllowPutFrom = ` so only named hosts may write; that list is the Agent's + own access control and this driver neither reads nor influences it. +2. **The driver** — `"Write": { "Enabled": true }` in the driver config (default + `false`; an absent block is disabled). + +Either gate alone leaves the shipped read-only behaviour unchanged: with `Write.Enabled` +off every slot is refused `BadNotWritable` without an Agent being touched, and with +`AllowPut` off the Agent refuses and the driver reports `BadCommunicationError`. + +### The OPC UA write bit is authored, not derived — read this before filing a bug + +**Enabling write-back does not make any existing node writable over OPC UA.** The +`CurrentWrite` access bit is composed from the **authored `Tag.AccessLevel`** alone +(`AddressSpaceComposer` → `AddressSpaceApplier`, which additionally requires a +non-array tag → `OtOpcUaNodeManager.ComposeAccessLevel`); driver capability is never +consulted at compose time. An operator must set the raw tag to **ReadWrite** — a +browse-committed tag is created `Read` (`RawBrowseCommitMapper.MapLeaf`) whatever the +browse said. + +Browse *does* stamp writable leaves `SecurityClassification.Operate` when +`Write.Enabled` is on (and `ViewOnly` otherwise, CONDITION always `ViewOnly`), on the +same "a node cannot advertise writability the driver would refuse" principle MQTT +pinned. But that stamp is **honest labelling, not the access bit**: nothing persists +`SecurityClass` — `Tag` has no such column — so it reaches the browse session's +attribute side-panel and stops there. + +### What a write does, slot by slot + +`WriteAsync` never throws and one bad slot never fails the batch (matching `ReadAsync`); +even cancellation returns a full-length result set. The refusal order is fixed, and each +step answers a different operator question: + +| Condition | Status | Meaning | +|---|---|---| +| `Write.Enabled` off, **or** the DataItem is CONDITION-category | `BadNotWritable` (`0x803B0000`) | Nothing was attempted; no Agent was involved | +| No live Agent client (pre-`InitializeAsync`, faulted initialize, post-shutdown) | `BadNotConnected` (`0x808A0000`) | This driver is not running | +| The reference resolves to no DataItem, or to one whose owning device cannot be named | `BadNodeIdUnknown` (`0x80340000`) | There is no Agent URL the write could be addressed to | +| The value has no invariant string form — `null`, or non-scalar | `BadTypeMismatch` (`0x80740000`) | Arrays are refused rather than rendered (`Convert.ToString` would emit the literal `System.Double[]`, which the Agent stores happily) | +| The Agent refused, or could not be reached | `BadCommunicationError` (`0x80050000`) | Includes an `MTConnectError` document returned under HTTP 200 | + +**CONDITION DataItems are excluded deliberately** — they carry alarm semantics and a +different `level|text` wire format, and are out of scope for v1. + +**The Agent's refusal text is logged at Warning and never stored.** It stays out of +`DriverHealth.LastError`, which is type-only by repo-wide discipline, and a refused write +is not evidence the read path degraded — so a refusal leaves driver health untouched. + +**Device resolution needs no extra config.** `TagBinding.WriteTargetsByDataItemId`, +built at bind time from a walk of the cached probe model, remembers which device declared +each DataItem, so a multi-device Agent is addressed correctly; the fallback is the +configured `DeviceName`, and an unresolvable device is `BadNodeIdUnknown` rather than a +guess. + +**Values are rendered with `CultureInfo.InvariantCulture`.** A Boolean therefore writes +the invariant string `"True"`/`"False"`, **not** `"1"`/`"0"` — if the consuming adapter or +Agent vocabulary expects something else, author the tag as a `String` and write the exact +token. + ## Browse — free via the universal browser MTConnect ships **no bespoke browser project**. Setting @@ -272,6 +361,13 @@ for anything beyond values-and-conditions. driver instance both resolve to the same one Agent connection. Real per-device routing (a client per device) would be a design change, not a bug fix. + + **The write path is the one place a device name is resolved per DataItem**, + and it does not use `RawTagEntry.DeviceName` either: it reads the owning + device out of the cached probe model and falls back to the top-level + `DeviceName` (see [Write-back](#write-back--observation-injection-off-by-default)). + So `MTConnectDriverOptions.DeviceName` does double duty — request scope for + reads/browse, fallback write target for writes. 3. **Nothing validates a raw tag's declared OPC UA `DataType` against the blob's coercion type.** The `/probe`-sourced inference (`MTConnectDataTypeInference`) narrows how often an author picks a wrong @@ -294,11 +390,17 @@ for anything beyond values-and-conditions. ## Deferred (not in this build) -- **Write-back (MTConnect Interfaces).** Design - [§3.6](../plans/2026-07-15-mtconnect-driver-design.md#36-iwritable--not-implemented-v1): - the mainstream Agent surface is read-only by design; Interfaces is a rare, - optional request/response handshake that would mislead if modelled as - `IWritable`. Revisit only on a concrete deployment need. +- **MTConnect *Interfaces*.** Still deferred, and for the reason design + [§3.6](../plans/2026-07-15-mtconnect-driver-design.md#36-iwritable--not-implemented-v1) + gave: it is a rare, optional request/response **state-machine handshake** + requiring adapter-side cooperation, not a "set value" operation, and would + mislead if modelled as `IWritable`. + **Write-back itself is no longer deferred** — `b29623cf` shipped `IWritable` + over the Agent's `AllowPut` observation-injection surface instead, a different + mechanism entirely. See [Write-back](#write-back--observation-injection-off-by-default) + and + [`2026-08-05-mtconnect-writeback-design.md`](../plans/2026-08-05-mtconnect-writeback-design.md) + §2.1. - **P1.5 fast-follow** (design [§9](../plans/2026-07-15-mtconnect-driver-design.md#9-phasing--effort)): CONDITION → native OPC UA Part 9 alarms via `IAlarmSource` (the Galaxy @@ -317,7 +419,10 @@ for anything beyond values-and-conditions. discovery tree shape, `MTConnectDataTypeInference`, observation indexing, `UNAVAILABLE` → `BadNoCommunication`, CONDITION state-word mapping, multipart chunk framing, and ring-buffer re-baseline paging (both the - sequence-gap and the `OUT_OF_RANGE` legs). + sequence-gap and the `OUT_OF_RANGE` legs). The write path adds + `MTConnectAgentClientWriteTests` (`9f17d7ef` — verb, URL, agent-error-in-200 + detection) and `MTConnectWriteTests` (`b29623cf` — every refusal leg, the + batch shape, the browse-stamping flip). - **Integration tests** — `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/` — env-gated on `MTCONNECT_AGENT_ENDPOINT` (default `http://10.100.0.35:5000`), skips diff --git a/docs/drivers/Mqtt.md b/docs/drivers/Mqtt.md index c01aeb83..1b08ba9c 100644 --- a/docs/drivers/Mqtt.md +++ b/docs/drivers/Mqtt.md @@ -521,6 +521,11 @@ and only** browse action that publishes anything. - **Scope** is the last node clicked in the tree. A device or metric resolves **up to its owning edge node** (Sparkplug has no device-scoped rebirth); a group fans out to every observed edge node beneath it and is refused **whole** past 32, publishing nothing. +- **An empty tree can still arm a rebirth** ([#514](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/514), + fixed `7f53bd96`): when no scope is selected the panel offers a `group/edgeNode` input, so a plant + whose edge nodes all birthed before the window opened — the exact case the affordance exists for — + is reachable without a tree click. Deliberately an **edge node**, never a group: a typed group + broadcast is a self-inflicted broker storm on a large plant. - **Two clicks, never one** — `Request rebirth…` arms a confirm panel naming the resolved scope and its consequence; changing the selection disarms it. - **Gated on `DriverOperator`**, enforced server-side in `BrowserSessionService.RequestRebirthAsync` @@ -547,14 +552,6 @@ in the rebuilt tree. > affected **every** driver's picker since 2026-05-28; it surfaced here because choosing a rebirth > scope is the first flow that requires clicking a label rather than the ▶ toggle. -> ⚠️ **Known gap — a completely empty tree cannot arm a rebirth.** The rebirth scope is taken from the -> last node clicked *in the tree*, so on a plant that has not birthed since the window opened there is -> nothing to click and `Request rebirth…` stays disabled — the exact case the affordance exists for. -> `MqttBrowseSession.ResolveRebirthTargets` already accepts a bare `{group}/{edgeNode}` for a node the -> window has never seen (it calls that "the prime rebirth target"); what is missing is a UI path to -> enter one, or to default the scope to the driver's own configured `GroupId`. Until then: use -> **Refresh** after any birth, or author the tags by **Manual entry**, which is always available. - ## Test Connect `MqttDriverProbe` performs **one CONNECT** (no SUBSCRIBE, no publish) under a `-probe-` client id and @@ -583,8 +580,6 @@ identical words. See [TestConnectProbes.md](TestConnectProbes.md). ## Operational Notes -- **Read-only.** No writes reach the broker; every node is ViewOnly. Sparkplug NCMD is used for - **rebirth requests only** — never to command a device. - **No alarms, no driver-side history.** Historization is a server-side concern (`isHistorized`). - **Both pair nodes subscribe independently** — MQTT fan-out is per-connection, and the Primary gate applies downstream, not to ingest. This is also why `clientId` must stay unset (above). diff --git a/docs/plans/2026-07-15-mtconnect-driver-design.md b/docs/plans/2026-07-15-mtconnect-driver-design.md index f7c0e5ae..e993f6b2 100644 --- a/docs/plans/2026-07-15-mtconnect-driver-design.md +++ b/docs/plans/2026-07-15-mtconnect-driver-design.md @@ -114,6 +114,21 @@ Pure function in `.Contracts` (shared by driver, browser-commit, and the typed e ### 3.6 `IWritable` — **not implemented (v1)** +> **SUPERSEDED 2026-08-05 — write-back shipped, by a different mechanism.** +> `MTConnectDriver` now implements `IWritable` (`b29623cf`) over the Agent's optional +> **`AllowPut`** surface (`POST {agent}/{device}?{dataItemId}={value}`), gated off by +> default at both the Agent (`AllowPut`) and the driver (`Write.Enabled`). That is +> **observation injection into the Agent's buffer, not machine command** — see +> [`2026-08-05-mtconnect-writeback-design.md`](2026-08-05-mtconnect-writeback-design.md) +> §2.2 and `docs/drivers/MTConnect.md` §Write-back. +> **The paragraph below is retained because its rationale still holds for *Interfaces*, +> which remains deferred** (design §2.1 of the write-back doc re-affirms it: rarely +> deployed, needs adapter-side cooperation, a state machine rather than a value write). +> Only its two incidental claims about this build are out of date — the driver is no +> longer `IWritable`-less, and nodes no longer materialize without `CurrentWrite` *because +> of a missing capability*; the `CurrentWrite` bit was in fact always composed from the +> authored `Tag.AccessLevel` alone, never from driver capability. + Justified per research §2.1: the MTConnect Agent surface is read-only by design; write-back exists only via optional, rarely-deployed MTConnect *Interfaces* (a request/response state-machine handshake, not a "set value" — modelling it as `IWritable` would mislead). Capability interfaces are composable, so omitting `IWritable` is idiomatic (Galaxy's write path is even fire-and-forget). Nodes materialize **without** the `AccessLevels.CurrentWrite` bit automatically. Revisit Interfaces only if a concrete deployment needs it. ### 3.7 `IHostConnectivityProbe` + `IRediscoverable` diff --git a/docs/plans/2026-07-24-driver-expansion-tracking.md b/docs/plans/2026-07-24-driver-expansion-tracking.md index f9df628f..f15a8ac4 100644 --- a/docs/plans/2026-07-24-driver-expansion-tracking.md +++ b/docs/plans/2026-07-24-driver-expansion-tracking.md @@ -179,7 +179,8 @@ Both CI-simulatable on the shared docker host, no hardware. surface, data-plane precedence rules, quality-code mapping, and (most load-bearing) the **Known limitations** section. - **Scope shipped:** `IDriver`+`ITagDiscovery`+`IReadable`+`ISubscribable`+`IHostConnectivityProbe`+`IRediscoverable` - (deliberately no `IWritable` — read-only Agent surface), browse **free via the Wave-0 universal + (P1 shipped deliberately without `IWritable`; **`IWritable` added 2026-08-05, `b29623cf`** — see the + Deferred line below), browse **free via the Wave-0 universal browser** (`SupportsOnlineDiscovery=true`, no browser project), typed AdminUI tag editor, `UNAVAILABLE→BadNoCommunication` mapping, CONDITION-as-`String`, ring-buffer re-baseline paging (both the sequence-gap and `OUT_OF_RANGE` legs). @@ -191,7 +192,18 @@ Both CI-simulatable on the shared docker host, no hardware. - **Test results:** MTConnect unit suite 491/491; integration suite 12/12 against a real Agent (`mtconnect/agent:2.7.0.12`, **not** `mtconnect/cppagent` — that Docker Hub repo doesn't exist); skips cleanly (12 Skipped) offline. AdminUI 749/749; full solution 0 build errors. -- **Deferred (documented, not built):** write-back via MTConnect Interfaces (design §3.6); P1.5 +- **Write-back — SHIPPED 2026-08-05, not via Interfaces** (`b29623cf`, branch + `feature/mqtt-mtconnect-write`; design + [`2026-08-05-mtconnect-writeback-design.md`](2026-08-05-mtconnect-writeback-design.md)). + `IWritable` over the Agent's optional **`AllowPut`** surface + (`POST {agent}/{device}?{dataItemId}={value}`) — **observation injection into the Agent's + buffer, never a machine command**, gated off by default at both the Agent (`AllowPut`) and the + driver (`Write.Enabled`), CONDITION items refused. **MTConnect *Interfaces* remains deferred** + — design §3.6's rationale still holds for it (rare, adapter-side cooperation, a state machine + rather than a value write), and it is a different mechanism, not this one. Caveat recorded in + the driver guide: the OPC UA `CurrentWrite` bit still comes from the authored + `Tag.AccessLevel` alone, so an operator must author **ReadWrite**. +- **Deferred (documented, not built):** MTConnect *Interfaces* (design §3.6, above); P1.5 CONDITION→native Part-9 alarms, `TIME_SERIES` array materialization, EVENT→enum vocab (design §9); P2 SHDR adapter ingest (design §9). Also two pre-existing fleet-wide gaps this build surfaced rather than caused: `IRediscoverable`/`IHostConnectivityProbe` have no server-side consumer (eight @@ -210,7 +222,11 @@ Both CI-simulatable on the shared docker host, no hardware. `OnDataChange`, retained last-value read, `#`-observation browser); P2 Sparkplug B ingest (vendored Tahu proto + `Grpc.Tools` codegen, birth/alias/seq-gap/rebirth state machine, death→STALE, birth-driven browse tree, scoped Request-rebirth NCMD, typed tag editor). - **Write-through (`IWritable`) is deferred to P3** — every MQTT node materializes read-only. + **P3 write-through (`IWritable`) shipped 2026-07-31** — `72be1922` + ([#508](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/508), live-gated): plain + `writeTopic` publish + Sparkplug NCMD/DCMD, with a node discovered `ViewOnly` when the encoder + would refuse the write. Superseded the "every MQTT node materializes read-only" line this entry + carried; see `docs/drivers/Mqtt.md` §Write-through. - **Fixture:** Mosquitto TLS+auth broker + JSON publisher sidecar, **live** at `10.100.0.35:8883` (`:1883` plaintext-but-authenticated); stack `/opt/otopcua-mqtt`, compose in `tests/Drivers/…​.Driver.Mqtt.IntegrationTests/Docker/`. Env-gated live suite @@ -236,13 +252,31 @@ Both CI-simulatable on the shared docker host, no hardware. Sparkplug keys, merge-not-replace on save, group-id segment validation) + 8 falsifiable `MqttDriverFormModelTests` cases. **Three gates, three instances of the same defect class: a hand-maintained AdminUI surface left behind by a driver-side feature.** -- **P2 live gate — two open UI gaps, documented not fixed** (see `docs/drivers/Mqtt.md` §Known gaps): - - **A completely empty browse tree cannot arm a rebirth.** The scope comes from a tree click, so on - a plant that has not birthed since the window opened `Request rebirth…` stays disabled — the very - case the affordance exists for. `MqttBrowseSession.ResolveRebirthTargets` already accepts a bare - `{group}/{edgeNode}` ("the prime rebirth target"); only a UI path to enter one is missing. - Mitigation shipped: a **Refresh** button on the browse tree (it re-reads the same session, which - previously rendered exactly once at open and never again). +- **P2 live gate — four UI gaps recorded, ALL FOUR NOW CLOSED 2026-07-31** (`7f53bd96`; see + `docs/drivers/Mqtt.md` §Known gaps, which lists the closures). Recorded as found, with what + actually fixed each: + - **A completely empty browse tree cannot arm a rebirth** + ([#514](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/514)). The scope comes from a tree + click, so on a plant that has not birthed since the window opened `Request rebirth…` stays + disabled — the very case the affordance exists for. `MqttBrowseSession.ResolveRebirthTargets` + already accepts a bare `{group}/{edgeNode}` ("the prime rebirth target"); only a UI path to + enter one was missing. Mitigation shipped at the time: a **Refresh** button on the browse tree + (it re-reads the same session, which previously rendered exactly once at open and never again). + **FIXED —** `RawBrowseModal` now offers a `group/edgeNode` scope input whenever no tree scope is + armed; edge-node only, never a typed group broadcast. + - **A metric name containing `/` cannot be browse-committed** + ([#509](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/509)). The derived raw **tag Name** + is a RawPath segment and may not contain `/`, while `metricName` legitimately may — so the + spec-mandatory `Node Control/Rebirth` was refused at commit (`Row N: Name must not contain '/'`). + The refusal was loud and all-or-nothing, never a silently mis-bound tag, and **Manual entry** was + the working path. Fixing it needed a name-sanitisation policy with a collision answer, so it was + recorded rather than guessed at. **FIXED —** `RawBrowseCommitMapper.ToRawSegment` sanitises the + segment and refuses a collision explicitly. + - **`_canRebirth` is captured at browse-open**, so a reaped session still drew the button + ([#510](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/510)). **FIXED.** + - **The tag editor injects the Plain-only `payloadFormat` key into a Sparkplug blob** + ([#513](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/513)) — cosmetic; the factory + routes on the Sparkplug tuple and ignored it. **FIXED.** - **P2 live gate also found a PRE-EXISTING, cross-driver AdminUI defect — FIXED:** every node label in the shared `DriverBrowseTree` was an ``. In a Blazor Web App, `blazor.web.js`'s enhanced-navigation click interceptor resolves a bare `#` against ``, so **clicking @@ -252,19 +286,14 @@ Both CI-simulatable on the shared docker host, no hardware. now `