dohertj2/lmxopcua
1
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases Wiki Activity

Universal discovery browser: full controller-tree render is live-verify-blocked by the ab_server fixture (no @tags walk) #468

New Issue
Closed
opened 2026-07-15 18:38:43 -04:00 by dohertj2 · 1 comment
dohertj2 commented 2026-07-15 18:38:43 -04:00
Owner
Copy Link
Copy Source

Context

The Wave-0 universal Discover-backed browser (DiscoveryDriverBrowser) merged to master (commit 056887d6) — one generic browser that captures any discovery-capable driver's ITagDiscovery.DiscoverAsync and serves it through the picker, lighting up AbCip/TwinCAT/FOCAS pickers with zero per-driver browser code. Design: docs/plans/2026-07-15-universal-discovery-browser-design.md; plan + live-gate result: docs/plans/2026-07-15-universal-discovery-browser-implementation.md (§ "Live-gate result").

What the Task 18 live gate DID verify (docker-dev)

  • AbCip picker renders the "Browse controller" button — full CanBrowse binding chain works end-to-end.
  • Clicking it runs the universal browser in production (central-2 logs: BrowserSessionService.OpenUniversalAsync -> DiscoveryDriverBrowser.OpenAsync -> RunCaptureAsync -> CaptureAsync -> DiscoverTreeAsync), with PatchForBrowse demonstrably applied — the controller @tags walk ran even though the authored config never set EnableControllerBrowse (that walk only runs when the flag is true).
  • Error surfaced as a graceful UI chip (no dead binding, crash, or hang); bounded shutdown ran.
  • Modbus picker shows no Browse button (negative).

What is BLOCKED (this issue)

A populated controller-tag tree render + leaf-commit round-trip could not be verified. The libplctag ab_server test sim (ControlLogix fixture at 10.100.0.35:44818, tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/) implements tag read/write but not the CIP Symbol Object controller enumeration — the @tags walk returns ErrorUnsupported. This is a fixture capability gap, not a code defect: the browse binding is proven alive and PatchForBrowse is proven to run in production.

Definition of done

Point the universal browser at a symbol-enumeration-capable Allen-Bradley backend and confirm:

  1. The AbCip picker renders a populated controller-tag tree (folders + leaves).
  2. Refresh re-captures.
  3. Picking a leaf writes the CIP tag path into TagConfig.FullName and the tag saves.

Options: a real ControlLogix over VPN/on-prem, or an ab_server/sim variant that serves @tags. TwinCAT (ADS symbol upload) and FOCAS (FixedTree UntilStable settle) have the same fixture-availability gap and can ride the same verification once a backend exists.

Related follow-up (already fixed)

Driver-config-page pickers not passing live config -> fixed in commit 10e23f8b (threaded GetConfigJson through AbCipDriverPage/TwinCATDriverPage/FOCASDriverPage).

## Context The Wave-0 universal Discover-backed browser (`DiscoveryDriverBrowser`) merged to master (commit `056887d6`) — one generic browser that captures any discovery-capable driver's `ITagDiscovery.DiscoverAsync` and serves it through the picker, lighting up AbCip/TwinCAT/FOCAS pickers with zero per-driver browser code. Design: `docs/plans/2026-07-15-universal-discovery-browser-design.md`; plan + live-gate result: `docs/plans/2026-07-15-universal-discovery-browser-implementation.md` (§ "Live-gate result"). ## What the Task 18 live gate DID verify (docker-dev) - AbCip picker renders the **"Browse controller"** button — full `CanBrowse` binding chain works end-to-end. - Clicking it runs the universal browser **in production** (central-2 logs: `BrowserSessionService.OpenUniversalAsync -> DiscoveryDriverBrowser.OpenAsync -> RunCaptureAsync -> CaptureAsync -> DiscoverTreeAsync`), with `PatchForBrowse` **demonstrably applied** — the controller `@tags` walk ran even though the authored config never set `EnableControllerBrowse` (that walk only runs when the flag is true). - Error surfaced as a graceful UI chip (no dead binding, crash, or hang); bounded shutdown ran. - Modbus picker shows **no** Browse button (negative). ## What is BLOCKED (this issue) A **populated controller-tag tree render + leaf-commit round-trip** could not be verified. The libplctag `ab_server` test sim (ControlLogix fixture at `10.100.0.35:44818`, `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/`) implements tag read/write but **not** the CIP Symbol Object controller enumeration — the `@tags` walk returns `ErrorUnsupported`. This is a **fixture capability gap, not a code defect**: the browse binding is proven alive and `PatchForBrowse` is proven to run in production. ## Definition of done Point the universal browser at a symbol-enumeration-capable Allen-Bradley backend and confirm: 1. The AbCip picker renders a populated controller-tag tree (folders + leaves). 2. Refresh re-captures. 3. Picking a leaf writes the CIP tag path into `TagConfig.FullName` and the tag saves. Options: a real ControlLogix over VPN/on-prem, or an `ab_server`/sim variant that serves `@tags`. TwinCAT (ADS symbol upload) and FOCAS (FixedTree `UntilStable` settle) have the same fixture-availability gap and can ride the same verification once a backend exists. ## Related follow-up (already fixed) Driver-config-page pickers not passing live config -> fixed in commit `10e23f8b` (threaded `GetConfigJson` through `AbCipDriverPage`/`TwinCATDriverPage`/`FOCASDriverPage`).
dohertj2 commented 2026-08-01 12:25:45 -04:00
Author
Owner
Copy Link
Copy Source

Closed by c2c3c522 — via the second option in the definition of done (a sim that serves @tags), since no real ControlLogix is reachable.

What changed

The fixture now applies a vendored patch (tests/Drivers/…AbCip.IntegrationTests/Docker/patches/) teaching ab_server to answer CIP 0x55 on class 0x6B, with partial-transfer paging. Checked against libplctag's own list_tags_logix rather than against our decoder — our decoder is the thing under test, so its agreement would have proved nothing. The clone is also pinned to v2.7.0; the Dockerfile described --branch release as "pinned" while tracking a branch that moves.

Definition of done — all three green on docker-dev

  1. Populated controller-tag tree — all nine seeded tags render as leaves under AbCip → Device1 → Discovered, in wire order: Motor_Run, Line1_Count, Pump_1_Speed, TestArray, TestString, TestSINT, TestBOOL, TestREAL, TestDINT.
  2. Refresh re-captures — collapses to root, re-expansion returns the same nine from a fresh capture.
  3. Leaf commit — Pump_1_Speed + TestDINT committed into the raw tree with their CIP tag paths in TagConfig and saved.

The gate found a real defect

CipSymbolObjectDecoder advanced to an even byte boundary after each symbol name. Entries are packed with no padding — libplctag walks them by sizeof(entry) + string_len, and a captured reply confirms it (281 bytes for nine entries; an even-padded stream would be 285). So any odd-length name desynchronised every entry behind it.

Motor_Run is 9 bytes and first on the wire, so the browse in leg 1 would have shown that one tag and silently dropped the other eight.

Every existing multi-entry test passed the whole time, because the test builder padded exactly as the decoder did — the pair agreed with each other and neither agreed with a controller. The builder no longer pads, and the layout is now pinned by the captured reply rather than by a restatement of the assumption. Restoring the alignment step reddens exactly the three new tests and none of the 29 others.

Two things the gate surfaced that it was not asked about

  • #536 — browse-commit discards the discovered data type, so a browsed REAL commits as "dataType":"DInt" and the driver decodes off that field, not the row's DataType. Filed rather than folded in here.
  • Test Connect fails against this fixture regardless of the device — the probe falls back to @raw_cpu_type when no tags are authored, which ab_server does not implement. Fixture limitation, documented in the fixture README; browse is unaffected.

Explicitly NOT closed

This issue's last paragraph noted that TwinCAT (ADS symbol upload) and FOCAS (FixedTree UntilStable settle) have the same fixture gap and could ride the same verification. They cannot ride this one — the patch teaches a CIP server to serve CIP symbol enumeration, and neither of those speaks CIP. Both still have no browsable backend in test, and #536 flags TwinCAT as sharing the data-type defect by inspection only for that reason.

Closed by `c2c3c522` — via the second option in the definition of done (a sim that serves `@tags`), since no real ControlLogix is reachable. ## What changed The fixture now applies a vendored patch (`tests/Drivers/…AbCip.IntegrationTests/Docker/patches/`) teaching `ab_server` to answer **CIP 0x55 on class 0x6B**, with partial-transfer paging. Checked against libplctag's own `list_tags_logix` rather than against our decoder — our decoder is the thing under test, so its agreement would have proved nothing. The clone is also pinned to `v2.7.0`; the Dockerfile described `--branch release` as "pinned" while tracking a branch that moves. ## Definition of done — all three green on docker-dev 1. **Populated controller-tag tree** — all nine seeded tags render as leaves under `AbCip → Device1 → Discovered`, in wire order: `Motor_Run`, `Line1_Count`, `Pump_1_Speed`, `TestArray`, `TestString`, `TestSINT`, `TestBOOL`, `TestREAL`, `TestDINT`. 2. **Refresh re-captures** — collapses to root, re-expansion returns the same nine from a fresh capture. 3. **Leaf commit** — `Pump_1_Speed` + `TestDINT` committed into the raw tree with their CIP tag paths in `TagConfig` and saved. ## The gate found a real defect `CipSymbolObjectDecoder` advanced to an **even byte boundary** after each symbol name. Entries are packed with **no padding** — libplctag walks them by `sizeof(entry) + string_len`, and a captured reply confirms it (281 bytes for nine entries; an even-padded stream would be 285). So any odd-length name desynchronised every entry behind it. `Motor_Run` is 9 bytes and first on the wire, so **the browse in leg 1 would have shown that one tag and silently dropped the other eight.** Every existing multi-entry test passed the whole time, because the test builder padded exactly as the decoder did — the pair agreed with each other and neither agreed with a controller. The builder no longer pads, and the layout is now pinned by the captured reply rather than by a restatement of the assumption. Restoring the alignment step reddens exactly the three new tests and none of the 29 others. ## Two things the gate surfaced that it was not asked about - **#536** — browse-commit discards the discovered data type, so a browsed REAL commits as `"dataType":"DInt"` and the driver decodes off that field, not the row's `DataType`. Filed rather than folded in here. - **Test Connect fails against this fixture regardless of the device** — the probe falls back to `@raw_cpu_type` when no tags are authored, which `ab_server` does not implement. Fixture limitation, documented in the fixture README; browse is unaffected. ## Explicitly NOT closed This issue's last paragraph noted that **TwinCAT (ADS symbol upload) and FOCAS (`FixedTree` `UntilStable` settle) have the same fixture gap and could ride the same verification.** They cannot ride this one — the patch teaches a CIP server to serve CIP symbol enumeration, and neither of those speaks CIP. Both still have no browsable backend in test, and #536 flags TwinCAT as sharing the data-type defect by inspection only for that reason.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
auto-managed
queue management
 cross-driver
queue management
 driver/abcip
queue management
 driver/ablegacy
queue management
 driver/focas
queue management
 driver/mqtt
MQTT / Sparkplug B broker-subscribe driver
 driver/opcuaclient
queue management
 driver/s7
queue management
 driver/twincat
queue management
 phase/1
queue management
 phase/2
queue management
 phase/3
queue management
 phase/4
queue management
 phase/5
queue management
 phase/6
queue management
 queue/blocked
queue management
 queue/done
queue management
 queue/failed
queue management
 queue/in-progress
queue management
 queue/queued
queue management
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#468
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?