From 57d9f1b38e9c4d96da133bb8894d1cce9bcb5fdc Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 16 Jun 2026 05:18:19 -0400 Subject: [PATCH] =?UTF-8?q?docs(phase4):=20data-type=20tier=20design=20?= =?UTF-8?q?=E2=80=94=20Modbus=20Int64/UInt64=20node=20type,=20FOCAS=20fail?= =?UTF-8?q?-fast+scaling,=20Historian=20Total+dead-letter=20(S7+arrays=20d?= =?UTF-8?q?eferred)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...pending-phase-4-driver-datatypes-design.md | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 docs/plans/2026-06-16-stillpending-phase-4-driver-datatypes-design.md diff --git a/docs/plans/2026-06-16-stillpending-phase-4-driver-datatypes-design.md b/docs/plans/2026-06-16-stillpending-phase-4-driver-datatypes-design.md new file mode 100644 index 00000000..12d54f70 --- /dev/null +++ b/docs/plans/2026-06-16-stillpending-phase-4-driver-datatypes-design.md @@ -0,0 +1,128 @@ +# Still-Pending Phase 4 (data-type tier) — Driver data-type & robustness coverage — design + +> **Status:** approved 2026-06-16. Parent roadmap: `docs/plans/2026-06-15-stillpending-backlog-design.md` (Phase 4). +> Source backlog: `stillpending.md` §2 (driver-layer gaps). Branch `feat/stillpending-phase-4-driver-datatypes` +> off master `7eeb9fb0`. Phases 0–3 already shipped. + +## Goal + +Close the cleanly-achievable **scalar data-type and robustness** gaps in `stillpending.md` §2 across three +disjoint driver projects — **no Configuration entity / EF migration, no contract churn, no bUnit.** Phase 4's +roadmap entry listed nine per-driver sub-areas; grounding (this session) showed the heavy structural items +(S7 wide types, cross-driver arrays, UDT member paths, bit-index writes, Galaxy nesting, OpcUaClient +event-history) need read-path refactors / contract changes / Phase-6 UI that don't fit a `standard` tier. This +branch is the **data-type tier**; everything structural is deferred to **Phase 4b** (recorded below, not dropped). + +## Grounding that reshaped the scope + +1. **`DriverDataType.Int64`/`UInt64` already exist** (`Core.Abstractions/DriverDataType.cs:17,20`) and the live + equipment-node path already resolves the strings `"Int64"`/`"UInt64"` → correct OPC UA `DataTypeIds` + (`OtOpcUaNodeManager.ResolveBuiltInDataType:1364-1365`). So the Modbus "node misreported as Int32" gap is + **purely** the driver's `MapDataType` returning `Int32` — a small forward-fix, not a missing enum member. +2. **The array item is mis-scoped for a data-type tier.** The live equipment-node materialiser `EnsureVariable` + hard-wires `ValueRank = Scalar` with no array params (`OtOpcUaNodeManager.cs:1327`); `EquipmentTagPlan` + carries no array metadata at all; and S7 + AbLegacy have no array *read* support. Surfacing arrays end-to-end + is a coherent cross-cutting feature (plan metadata + `EnsureVariable` contract change + per-driver read + + Phase-6 authoring UI), not a per-driver flag flip. **Deferred to its own slice.** +3. **S7 wide types are the riskiest, least-verifiable in-tier item.** The driver reads via S7.Net + **address-strings** (`plc.ReadAsync("DB1.DBW0")` → boxed 4-byte); wide types need a byte-buffer + + `S7.Net.Types` decode branch, and the dev sim is **python-snap7** with unconfirmed 8-byte/string support + (no real PLC). **All S7 deferred to Phase 4b** (one S7-focused branch with a proper read-path refactor). + +## In scope — five changes (parallelizable, three disjoint projects) + +### 1. Modbus Int64 / UInt64 node DataType — `small` — live-verifiable +- **File:** `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs`. +- `MapDataType:1517` currently `ModbusDataType.Int64 or ModbusDataType.UInt64 => DriverDataType.Int32`. + Split: `Int64 => DriverDataType.Int64`, `UInt64 => DriverDataType.UInt64`. +- The wire codec (`DecodeRegister`/`EncodeRegister`) already round-trips `long`/`ulong` — **no codec change.** +- Remove the now-stale `Driver.Modbus-007` XML-doc caveat (`:1496-1506`) + the inline comment (`:1510-1516`). +- **Test** (`ModbusDataTypeTests`): `MapDataType(Int64)⇒DriverDataType.Int64`, `(UInt64)⇒UInt64`; a discovery + test that an `Int64` tag surfaces `DriverDataType.Int64` on its `DriverAttributeInfo`. +- **Live** (Modbus sim `10.100.0.35:5020`): author an Int64 tag, confirm the node advertises Int64 and a value + outside the 32-bit range reads back correctly. + +### 2. FOCAS fail-fast factory — `small` — unit-only +- **Files:** `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs` (factory iface + + `UnimplementedFocasClientFactory`), `FocasDriver.cs` (`InitializeAsync`). +- Today `UnimplementedFocasClientFactory.Create()` throws only on the **first read** (lazy `Create()` at + `FocasDriver.cs:1079`) — so an `unimplemented`/`none`/`stub` backend reports **healthy** then faults every + read/write/subscribe (operator footgun, §2). +- Add `IFocasClientFactory.EnsureUsable()` (default no-op; `UnimplementedFocasClientFactory` overrides to throw + the same clear message **without** creating a live wire client) and call it early in `InitializeAsync` so the + driver **faults at config/init time** with the actionable message instead of masquerading healthy. +- **Test** (`FocasScaffoldingTests`): a driver with `Backend:"unimplemented"` throws/faults at + `InitializeAsync`, not at first read; the `wire` backend still initialises clean. + +### 3. FOCAS position scaling — `standard` — unit-only (no CNC) +- **Files:** FOCAS device-config record (add `PositionDecimalPlaces`, default `0`), `FocasDriver.cs` + (`PublishAxisSnapshot`). +- `cnc_rddynamic2` returns **scaled integers**; the `10^DecimalPlaces` divide is never applied, so positions are + published in CNC-internal units (§2). Auto-fetching the figure via `cnc_getfigure` is wire-gated → deferred. +- Add a per-device `PositionDecimalPlaces` config knob (default `0` = today's behaviour, fully backward + compatible). When `> 0`, apply `value / 10^dp` to the **four position fields only** (Absolute / Machine / + Relative / DistanceToGo) at the `PublishAxisSnapshot` seam, which feeds both the subscribe poll and the cached + `ReadAsync`. **No node-type change** — axis variables are already `DriverDataType.Float64` + (`FocasDriver.cs:519`), so a fractional double is type-coherent. FeedRate / SpindleSpeed / ServoLoad are **not** + position-scaled. +- **Test** (`FocasReadWriteTests` or new): `dp=3` → raw `12345` publishes `12.345`; `dp=0` → `12345` unchanged; + FeedRate/SpindleSpeed never scaled. + +### 4. Historian `Total` aggregate — `standard` — unit-only (sidecar-gated) +- **File:** `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client/WonderwareHistorianClient.cs`. +- `MapAggregate:519` throws `NotSupported` for `HistoryAggregateType.Total` (Wonderware AnalogSummary has no + Total column). Implement it **client-side** via the time-integral identity: for a time-weighted average, + `Total = Average × interval-duration`. In `ReadProcessedAsync`, when the requested aggregate is `Total`, issue + the wire query with the `Average` column and **post-multiply** each returned bucket's value by + `interval.TotalSeconds`; quality/status carries from the Average query. +- Document the approximation (time-weighted Average × seconds) in the method remarks. +- **Test** (`WonderwareHistorianClientTests`): replace `ReadProcessedAsync_TotalAggregate_ThrowsNotSupported` + with one asserting `Total = Average × interval-seconds` (FakeSidecarServer returns Average values; client scales). + +### 5. Historian poison-event dead-letter — `standard` — unit-only +- **File:** `src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs`. +- The sidecar reply carries only a per-event bool (no transient-vs-permanent), so every failure maps to + `RetryPlease`; the drain loop bumps `AttemptCount` but **never caps it** → a permanently-malformed event + retries forever at the 60 s backoff floor (§2 "finding 002"). +- Add a `maxAttempts` ctor knob (default e.g. `10`; mirrors `capacity`/`deadLetterRetention`; `Validate`/ctor + guard warns or rejects `≤ 0`). Extend `QueueRow` + the `ReadBatch` SELECT to carry `AttemptCount` (the column + already exists — schema `:23,689`). In the `RetryPlease` branch, dead-letter the row once + `AttemptCount + 1 ≥ maxAttempts` (reason `"max attempts exceeded"`) instead of bumping again. +- **Test** (`SqliteStoreAndForwardSinkTests`): a writer that always returns `RetryPlease` → after `maxAttempts` + drains the row moves to dead-letter (`DeadLetterDepth` +1, `QueueDepth` −1); below the cap it stays queued. + +## Out of scope — deferred, recorded as follow-ups (not dropped) + +- **Phase 4b (S7):** S7 Int64/UInt64/LReal/String/DateTime + Timer/Counter — needs the `S7.Net.Types` + byte-buffer read-path refactor. +- **Dedicated array slice:** cross-driver `IsArray` surfacing + `EnsureVariable`/`EquipmentTagPlan` array + plumbing + Modbus String/BitInRegister array decode + per-driver array reads + Phase-6 authoring UI. +- **Phase 4b (structural):** AbCip/TwinCAT UDT member paths, AbLegacy/TwinCAT bit-index RMW writes, Galaxy + nested gobject hierarchy + writer item-handle cache, OpcUaClient `ReadEventsAsync`, FOCAS `cnc_getfigure` + auto-scale, Historian oversized tie-cluster paging (task #400). + +## Architecture / risk notes + +- Three disjoint projects (Modbus / FOCAS / Historian+AlarmHistorian) → per-driver independence. T1, T2, T4, T5 + dispatch concurrently; T3 follows T2 (both edit `FocasDriver.cs`). +- The only shared-infra touch is `Core.AlarmHistorian` (item 5, the durable store-and-forward path) — additive + threshold + ctor knob, no schema change (column pre-exists). Treated as the highest-care item. +- No actor-model, redundancy, security, or address-space contract changes. No `IOpcUaAddressSpaceSink` touch + (that was the deferred array work). `DriverDataType` enum unchanged (Int64/UInt64 already present). + +## Testing & verification + +- TDD red→green, xUnit + Shouldly, per existing driver test patterns (`ModbusDataTypeTests`, + `FocasScaffoldingTests`/`FocasReadWriteTests`, `WonderwareHistorianClientTests`, + `SqliteStoreAndForwardSinkTests`). **No bUnit.** +- `dotnet build` clean (production projects are `TreatWarningsAsErrors`) + full `dotnet test` green before merge. +- Final integration review (the durable-store item + the cross-item docs). +- **Live `/run`:** Modbus Int64 on the docker-dev rig (the one live-verifiable item). FOCAS (no CNC) and + Historian (sidecar-gated) are unit-proven; their live gates are honestly recorded as deferred. + +## Hard constraints (carried from the parent roadmap) + +- **NO Configuration entity / EF migration.** Stage by path — never `git add .`. Never stage `sql_login.txt`, + `src/Server/.../Host/pki/`, `pending.md`, `current.md`, `docker-dev/docker-compose.yml`, `stillpending.md`. + Never echo or commit secrets. No force-push, no `--no-verify`. +- Finish = merge to master + push (every phase).