From f90017bc9a6846913921e029ec6db1385cc55d3c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 16 Jun 2026 19:17:19 -0400 Subject: [PATCH] design(phase4b): Mac-verifiable driver gaps (Modbus reconcile + Galaxy nesting + FOCAS auto-scale) --- ...tillpending-phase-4b-driver-gaps-design.md | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 docs/plans/2026-06-16-stillpending-phase-4b-driver-gaps-design.md diff --git a/docs/plans/2026-06-16-stillpending-phase-4b-driver-gaps-design.md b/docs/plans/2026-06-16-stillpending-phase-4b-driver-gaps-design.md new file mode 100644 index 00000000..6f6df9dd --- /dev/null +++ b/docs/plans/2026-06-16-stillpending-phase-4b-driver-gaps-design.md @@ -0,0 +1,158 @@ +# Phase 4b — Mac-verifiable driver gaps (design) + +**Status:** approved 2026-06-16. Branch `feat/stillpending-phase-4b-driver-gaps` off master `c081917a`. + +**Goal:** Close three independent, contract-free `stillpending.md` §2 driver gaps that are +shippable on their own and (for the first two) live-verifiable from this Mac: + +1. **Modbus driver-type-string reconcile** — `"ModbusTcp"` (AdminUI/probe) vs `"Modbus"` + (runtime factory + seed) → rig Modbus drivers fall to the raw-JSON editor; AdminUI-created + Modbus drivers store an unloadable type string. +2. **Galaxy nested gobject hierarchy** — `GalaxyDiscoverer` renders the gobject tree flat. +3. **FOCAS `cnc_getfigure` auto-scale** — axis positions need the decimal-place figure + hand-configured; the CNC reports it via `cnc_getfigure`. + +The three touch disjoint projects (AdminUI + Modbus driver / Galaxy driver / FOCAS driver), +so implementers can run concurrently. + +**Hard constraints (every task):** stage by explicit path, never `git add .`; never stage +`sql_login.txt` / `src/Server/ZB.MOM.WW.OtOpcUa.Host/pki/` / `pending.md` / `current.md` / +`docker-dev/docker-compose.yml` / `stillpending.md`; no force-push / no `--no-verify`; +**NO EF migration; NO Commons wire/proto contract change; NO bUnit** (Razor proven by live +`/run`). The `IFocasClient` addition is a driver-internal interface, not a wire contract; the +Galaxy proto is unchanged. + +--- + +## Component 1 — Modbus driver-type-string reconcile + +### Problem +The Modbus driver-type string is inconsistent: +- **Runtime canonical = `"Modbus"`** — `ModbusDriver.DriverType` (`ModbusDriver.cs:185`) and the + factory registration key `ModbusDriverFactoryExtensions.DriverTypeName` (`:16`). +- **AdminUI = `"ModbusTcp"`** — `DriverIdentitySection.razor`, `DriverTypePicker.razor`, + `DriverEditRouter.razor`, `ModbusDriverPage.razor`, `Uns/TagEditors/TagConfigEditorMap.cs`, + `Uns/TagEditors/TagConfigValidator.cs`, + ~13 AdminUI.Tests rows. +- **Probe = `"ModbusTcp"`** — `ModbusDriverProbe.cs:29` (a third spelling). + +Effect: the rig's `MAIN-modbus-eq` is seeded `DriverType="Modbus"`, so the `/uns` Tag modal's +`TagConfigEditorMap.Resolve` (keyed on `"ModbusTcp"`) misses → raw-JSON editor (and Phase 6's +Build-address button is unreachable). An AdminUI-created Modbus driver stores `"ModbusTcp"`, +which the runtime factory (`"Modbus"`) cannot instantiate. + +### Approach (chosen): canonicalize on `"Modbus"` +`"Modbus"` is the runtime factory registration key — the hard runtime dependency. Canonicalizing +on it means **never touching the factory key or `ModbusDriver.DriverType`** and **no data +migration** (the dev-rig seed is already `"Modbus"`). + +Changes — change the 7 outliers + tests to `"Modbus"`: +- `ModbusDriverProbe.cs:29` `DriverType => "ModbusTcp"` → `"Modbus"`. +- The 6 AdminUI files above: every `"ModbusTcp"` stored/dispatched value → `"Modbus"`. +- ~13 AdminUI.Tests rows seeding `DriverType="ModbusTcp"` → `"Modbus"`. + +A friendly **display label** ("Modbus TCP") stays in the picker UI; only the *stored/dispatched +value* canonicalizes. Any legacy `"ModbusTcp"` `DriverInstance.DriverType` row (none on the rig) +is a documented one-line SQL `UPDATE` — not a migration. + +*Rejected:* canonicalize on `"ModbusTcp"` (forces a runtime-factory-key change + seed + DB +migration — touches the load-bearing dependency); dual-alias registration (YAGNI machinery). + +### Tests +Unit: `TagConfigEditorMap.Resolve("Modbus")` returns the Modbus editor; `TagConfigValidator` +validates `"Modbus"`; the picker/router dispatch on `"Modbus"`. Update the ~13 AdminUI.Tests rows. +**Live `/run`:** the rig's seeded `MAIN-modbus-eq` now opens the typed Modbus editor + Build-address +button in the `/uns` Tag modal (the exact thing Phase 6 couldn't drive). + +--- + +## Component 2 — Galaxy nested gobject hierarchy + +### Problem +`GalaxyDiscoverer.DiscoverAsync` (`Driver.Galaxy/Browse/GalaxyDiscoverer.cs`) calls +`builder.Folder(...)` at root level for every gobject, then adds the gobject's variables into it. +Large Galaxy models browse as a flat list. + +### Approach (chosen): nest by `parent_gobject_id` (driver-internal, no gateway change) +The gateway proto `GalaxyObject` already carries the parentage we need: +`gobject_id` (1), `parent_gobject_id` (5), `is_area` (6), `hosted_by_gobject_id` (8). +`IAddressSpaceBuilder.Folder(...)` **returns `IAddressSpaceBuilder`**, so folders already nest +(`parentFolder.Folder(child, child)`) — no contract change. + +Rewrite `DiscoverAsync` as a two-pass build: +1. **Pass 1 — folder map:** create a `gobject_id → folder-builder` entry for every gobject + (browse name = `contained_name` ?? `tag_name`, as today). Don't attach to a parent yet. +2. **Pass 2 — link + populate:** for each gobject, resolve its folder under its parent's folder + when `parent_gobject_id != 0` **and** that parent id is in the map; otherwise attach to the + driver root (the current flat behaviour). Then add the gobject's variables into its own folder + (unchanged per-attribute logic, incl. alarm-ref marking and the `[]`-suffix strip). + +Properties: +- **Order-independent** — children may appear before parents (build the map first, link second). +- **Degrades to flat** — gateways that don't populate `parent_gobject_id` (returns 0) or models + with the parent outside the returned set fall back to root attachment. No regression for the + current flat data. +- Nest by the **model/area** relationship (`parent_gobject_id`), not deployment hosting + (`hosted_by_gobject_id`) — that is AVEVA's natural browse tree. +- Cycle/self-parent guard: if `parent_gobject_id == gobject_id` or a cycle is detected, attach to + root (defensive; real Galaxy models are a DAG/tree). + +### Tests +Unit (Galaxy driver test project) — fake `IGalaxyHierarchySource` returns canned multi-level +`GalaxyObject` sets through a capturing `IAddressSpaceBuilder`: +- two-level nest (child folder under parent folder, variables in the right folder); +- **order-independence** (child returned before parent → still nests); +- **degrade-to-flat** (`parent_gobject_id=0`, and parent-id-not-in-set → both attach to root); +- defensive self-parent/cycle → root. +**Live `/run`:** Client.CLI browse the Galaxy driver tree vs the gateway on `10.100.0.48` → +nested folders instead of flat. + +--- + +## Component 3 — FOCAS `cnc_getfigure` auto-scale + +### Problem +`FocasDriver.PublishAxisSnapshot` divides axis positions by `10^PositionDecimalPlaces` +(`FocasDriver.cs:823-829`), where `PositionDecimalPlaces` is a **manual** per-device config knob +(default 0, added in Phase 4). The CNC reports the real figure via `cnc_getfigure` +(`IFocasClient.cs:219-221` notes the auto path "once that export lands"). + +### Approach (chosen): add a `cnc_getfigure` binding; auto wins, manual is the fallback +- Add a binding to `IFocasClient` (e.g. `Task> GetPositionFiguresAsync(CancellationToken)` + returning the per-axis decimal-place figure). The **Wire** client P/Invokes `cnc_getfigure`; + the **Fake** and **Unimplemented** clients return an empty/unsupported result. +- At init (alongside the existing axis-name/sysinfo reads), fetch the per-axis figures and cache + them on the driver state. +- **Precedence (approved): auto wins; manual is the fallback.** When `cnc_getfigure` returns a + usable figure for an axis, that figure is authoritative for that axis's scale. When it returns + nothing / is unavailable (older FWLIB, Fake/Unimplemented backend, per-axis gap), fall back to + the configured `PositionDecimalPlaces`. This keeps existing manual-knob configs working when the + CNC doesn't report and matches the "auto-scale" intent (the CNC's own scaling is the truth). +- Resolve the effective decimal places **per axis** (the figure can differ by axis), so + `PublishAxisSnapshot` divides each axis by its own `10^figure`. + +*Rejected:* drop the manual knob entirely (loses the fallback for backends/sims that can't report); +leave manual-only (that's the current gap). + +### Tests +Unit (FOCAS driver test project) with a fake `IFocasClient`: +- auto wins — fake returns figures → driver scales by the auto figure, ignoring a differing manual + config; +- fallback — fake returns nothing/unsupported → driver uses the manual `PositionDecimalPlaces`; +- per-axis — different figures per axis scale independently; +- regression — the existing manual-only path (no getfigure) still divides correctly. +**Unit-proven only** (no CNC on this Mac — honestly operator-gated for a live read). + +--- + +## Out of scope (deferred) +- S7 wide types / Timer-Counter, the cross-driver **array slice** (sink-contract change), + AbCip/TwinCAT UDT member-paths, AbLegacy/TwinCAT bit-RMW writes, OpcUaClient `ReadEventsAsync`, + Historian tie-cluster paging (#400). These remain the next slices of Phase 4b's theme. + +## Done criteria +- `dotnet build ZB.MOM.WW.OtOpcUa.slnx` clean. +- Affected suites green: AdminUI.Tests (Modbus reconcile), Galaxy driver tests (nesting), + FOCAS driver tests (auto-scale). +- **Live `/run`:** Modbus typed editor + Build-address reachable on the rig's seeded driver; + Galaxy tree browses nested vs the gateway. FOCAS unit-proven (operator-gated live). +- Subagent-driven, per-task review by classification, final integration review, then merge + push.