Joseph Doherty
163 lines
11 KiB
Markdown
163 lines
11 KiB
Markdown
# MTConnect write-back + MQTT write-through record reconciliation — design
|
||
|
||
**Status:** DONE 2026-08-05 — merged to master (`0e3381d5`); unit suites green (MTConnect 534/534,
|
||
MQTT 658/658, AdminUI 1064/1064); **live gate PASSED** (§4): the full integration suite ran live
|
||
against the shared fixture (`10.100.0.35:5000`, AllowPut now on) — 15/15, 0 skipped — covering the
|
||
write round-trip (value read back from `/current`), the real agent-refusal → `BadCommunicationError`
|
||
mapping (with the no-`deviceName` → `BadNodeIdUnknown` falsifiability control), and the disabled
|
||
gate (`BadNotWritable`, agent untouched). The same legs also ran against a local OrbStack fixture
|
||
during development with agent-log corroboration. **One leg is deliberately deferred to the
|
||
operator's chrome session:** the docker-dev rig OPC UA leg (author an MTConnect driver with
|
||
`write.enabled` + a `ReadWrite` tag in AdminUI, deploy, `Client.CLI write`, read back) — authoring
|
||
requires the AdminUI, and per the operator's instruction chrome-driven gates run separately.
|
||
Remember the CurrentWrite caveat when running it: the tag must be authored `ReadWrite`
|
||
(browse-commit defaults to `Read`).
|
||
|
||
## 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 = <host list>`) 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<MTConnectWriteOutcome> 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`).
|