design(phase4b): Mac-verifiable driver gaps (Modbus reconcile + Galaxy nesting + FOCAS auto-scale)

This commit is contained in:
Joseph Doherty
2026-06-16 19:17:19 -04:00
parent c081917a69
commit f90017bc9a
@@ -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<IReadOnlyList<int>> 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.