Commit Graph

10 Commits

Author SHA1 Message Date
Joseph Doherty 6b3da93151 fix(browse): commit the browsed data type, and fix the probe classification the fixture was hiding (#536)
v2-ci / build (push) Successful in 4m17s
v2-ci / unit-tests (push) Failing after 16m47s
A browse-committed AB CIP / TwinCAT / FOCAS tag bound with its model's DEFAULT
data type, not the one the device reported. A browsed REAL committed as
`{"tagPath":"Pump_1_Speed","dataType":"DInt"}` — and those three drivers decode
the wire bytes by that field, not by the raw row's OPC UA DataType — so the tag
read four bytes of a float through an integer decoder and published a plausible
wrong number with no error raised anywhere. FOCAS was missed when #536 was filed.

The type is CARRIED, not derived. Reverse-mapping the leaf's driver-agnostic
DriverDataType looks like the smaller fix and is wrong: SInt/Int/DInt/USInt/UInt
all collapse into Int32 on the way out of the driver, so that version would bind
a 16-bit INT as a 32-bit DINT while appearing to fix the REAL. Drivers now state
their own type name on DriverAttributeInfo.NativeDataType, which reaches the
picker as AttributeInfo.AddressFields["dataType"] and the commit mapper from
there. A leaf that states nothing keeps the model default — the pre-#536
behaviour, kept deliberately as the fallback rather than a guess.

Numeric strings are refused before parsing: Enum.TryParse accepts "9" for any
enum and Enum.IsDefined then confirms it, so an unguarded parse would bind a tag
to a type nobody named. A test caught this in the first version of the guard.

--- and the other half, which the fixture was hiding ---

Test Connect on a device with NO authored tags — every AB CIP device at the
moment an operator first presses the button — reported "CIP handshake failed"
against a healthy controller. That was written down as a fixture limitation
("ab_server doesn't implement @raw_cpu_type"). Both halves of that were wrong:

  * @raw_cpu_type is not a libplctag special tag at all (only @raw, @tags and
    @udt/ are), so it goes out as an ordinary symbolic name that no controller
    defines, and the controller's "tag does not exist" IS the successful answer.
  * ab_server answered an unresolvable tag path with CIP 0x03 where a Logix
    controller answers 0x04 — ErrorBadParam vs ErrorNotFound, which the probe
    classifies oppositely. Fixed in vendored patch 0002.
  * With the fixture then telling the truth, the real defect surfaced:
    AbCipDriverProbe.IsReachableException matched the native C status spellings
    ("STATUS_NOT_FOUND"), but libplctag.NET throws with the MANAGED enum name
    ("ErrorNotFound"). Those substrings can never match, so the reachable
    classification could not fire on the exception path — which is the path every
    no-authored-tags probe takes. This was failing on real hardware too, not just
    against the sim. It now recovers the Status and defers to IsReachableStatus,
    so there is one classification rather than two that can drift.

Rejected: switching the fallback to @tags. libplctag builds a tag-listing tag
with NO network I/O, so it reports OK against any TCP listener — measured green
against a Mosquitto broker.

Verification: full solution suite green. Falsification runs confirm exactly the
intended tests go red — 9 for the type carry, 3 (the reachable statuses only) for
the probe classification. Live on docker-dev: browse-committed TestString/
Motor_Run/TestSINT now store String/Bool/SInt, alongside last session's
Pump_1_Speed still holding the wrong DInt. TestSINT is the falsifier — its row
DataType is Int32, identical to TestDINT's, so no reverse map could have produced
"SInt".

Known consequence: pre-existing browse-committed tags keep their wrong dataType;
the fix applies to new commits only.

Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
2026-08-01 15:20:07 -04:00
Joseph Doherty 8d9155682d fix(mqtt): browse-commit writes a bindable address + a Request-rebirth affordance
Two gaps the Task 23/24 review found, both blocking Task 26's live gate.

Gap 1 — browse-commit produced a silently dead MQTT tag. RawBrowseCommitMapper
had no `Mqtt` case, so a committed leaf fell through to the generic
`{"address": …}` key. Neither MqttTagDefinitionFactory entry point reads
`address`: the tag deployed clean and reported BadNodeIdUnknown forever, with
no signal at commit time. Predates Task 23 (Plain was affected too), but Task 23
built the Sparkplug metric tree precisely so an operator could browse and commit
a binding.

The address is a DESCRIPTOR, not a reference string, and it cannot be recovered
from the browse node id: `{group}/{node}[/{device}]::{metric}` where a metric
name legitimately contains `/` (`Node Control/Rebirth`) — the ambiguity
MetricSeparator's remarks already name. So the session STATES it, via a new
`AttributeInfo.AddressFields` seam, and the mapper reads it. Which keys are
emitted is also how the mapper learns Plain vs Sparkplug — the driver type
reaching it is just `Mqtt`, and the mode lives on the driver config. Key names
are single-sourced in the new `MqttTagConfigKeys` (producer, mapper, factory),
with the literals pinned by a test so a symmetric rename cannot silently unbind
already-persisted blobs. A leaf with no stated address is refused at commit in
words rather than committed dead.

Gap 2 — RequestRebirthAsync had no UI. Task 23 shipped it backend-only; Task 26
step 1 assumes the button, and Task 25's runbook notes the picker tree stays
empty until a birth lands, so it is the only way to fill it on demand. Added to
the /raw browse modal: scope = the clicked tree node (device/metric resolve up to
their edge node, group fans out and is refused whole past 32), an explicit
two-click confirm naming the resolved scope and its consequence, the outcome or
the refusal shown rather than swallowed. Offered only for a Sparkplug session
(new `IRebirthCapableBrowseSession.RebirthAvailable` — one session class serves
both modes, so a type test alone would draw a button that can only throw) and
only to a DriverOperator; the server-side gate remains the boundary.

Tests: MQTT 545 → 581, AdminUI 781 → 800. The round-trip tests feed the emitted
blob to the REAL factory and were falsified by mutating the emitted key name.

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
2026-07-24 23:43:07 -04:00
Joseph Doherty 64ec7aa43a feat(mqtt): Sparkplug birth-driven browser tree + scoped Request-rebirth action
Unseals the Sparkplug branch of the MQTT address-picker browser that Task 10
deliberately sealed shut, and adds the first (and only) sanctioned publish on a
browse session.

- MqttBrowseSession serves Group -> EdgeNode -> [Device] -> Metric in Sparkplug
  mode, decoded from observed NBIRTH/DBIRTH certificates (the only Sparkplug
  messages that name their metrics). Node-level metrics hang directly under
  their edge node -- no synthesised device folder, because the authored address
  tuple genuinely has deviceId = null for them. Metric node ids use a '::'
  separator: a metric name may contain '/', so slash-joining would make a
  node-level metric indistinguishable from a device folder.
- AttributesAsync is purely birth-derived in Sparkplug mode. The plain path's
  UTF-8 inference / payload-snippet machinery does not run and no payload bytes
  are retained: a Sparkplug body is protobuf and would be reported as
  "(N bytes, binary)" for every metric in the plant. A datatype ToDriverDataType
  cannot map is reported as the Sparkplug type name, never coerced.
- RequestRebirthAsync(scope) is the one publishing member, reached only by an
  explicit operator action. It routes through the counted PublishAsync
  chokepoint via a private RebirthTransport adapter, so PublishCountForTest
  still proves that OpenAsync/RootAsync/ExpandAsync/AttributesAsync/Observe/
  DisposeAsync publish nothing -- now in both modes. A device or metric scope
  resolves up to its owning edge node (NCMD is node-addressed); group scope
  enumerates observed edge nodes and is refused whole past
  MaxGroupRebirthNodes = 32.
- BrowserSessionService.RequestRebirthAsync gates on the same DriverOperator
  policy that gates the picker, evaluated server-side where the action happens
  rather than only at render time, fails closed, and Info-logs the scope and the
  published count. Exposed through the new IRebirthCapableBrowseSession contract
  so "does this session write?" stays a type question.
- ToBrowseOptions' Last-Will landmine re-verified: MqttDriverOptions still
  carries nothing will-shaped (an NDEATH is a broker-published will and would be
  invisible to PublishCountForTest), now pinned by a reflection guard.

Falsifiability: injecting a publish into RootAsync reddens the Sparkplug and
plain passivity tests; double-publishing per target reddens all four
one-NCMD-per-node assertions. 572/572 MQTT tests, 24/24 AdminUI browsing tests.

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
2026-07-24 22:51:16 -04:00
Joseph Doherty bbbda99719 feat(browse): Wave-0 Batch F — capture coalescing/cap + AddAdminUI DI registration
Task 10: in-flight coalescing (Lazy<Task<CapturedTree>> keyed on driverType+SHA256(config))
         so repeat Browse clicks share one device walk; global SemaphoreSlim cap
         (MaxConcurrentCaptures=4) queues excess; per-caller ct via WaitAsync leaves the shared
         capture (bounded by open-timeout, CancellationToken.None) unaffected; in-flight-only map
         (Refresh = fresh capture). 7 concurrency tests green (158 total in Commons.Tests).
Task 12: AddAdminUI registers IUniversalDriverBrowser->DiscoveryDriverBrowser with
         NullDriverFactory fallback (standalone AdminUI => CanBrowse false => manual entry).
         AdminUI.Tests 542 green.
2026-07-15 17:37:02 -04:00
Joseph Doherty 698703744f feat(browse): Wave-0 Batch E — UntilStable settle loop + BrowserSessionService fallback
Task 9: DiscoverTreeAsync UntilStable settle loop (FOCAS) — re-capture on a 1s interval until
        the node set is non-empty and stable across two passes, bounded by open-timeout; on
        timeout with a prior non-empty capture, return it (best-effort) instead of failing.
Task 11: BrowserSessionService resolves bespoke-first, falls back to IUniversalDriverBrowser
         when no bespoke browser matches and CanBrowse is true; new CanBrowse on the service.
16 unit tests green (4 settle + 12 service).
2026-07-15 17:31:50 -04:00
Joseph Doherty fa339a5565 feat(browse): Wave-0 Batch D — IUniversalDriverBrowser + DiscoveryDriverBrowser core
Task 8: IUniversalDriverBrowser + DiscoveryDriverBrowser open path — construct(+PatchForBrowse)
        -> connect -> one-shot capture -> bounded (10s) shutdown; CanBrowse gate (catch-throwing
        TryCreate, defensive teardown of the throwaway instance); deep-merge browse patch.
        Commons gains Microsoft.Extensions.Logging.Abstractions (ILogger doesn't flow transitively)
        + InternalsVisibleTo(Commons.Tests). OTOPCUA0001 suppressed on the deliberate one-shot
        browse-capture DiscoverAsync (not a runtime dispatch path). 12 unit tests green.
2026-07-15 17:27:04 -04:00
Joseph Doherty 15da8d4f0d feat(browse): Wave-0 Batch C — CapturingAddressSpaceBuilder + CapturedTreeBrowseSession
Task 6: CapturedTree/CapturedNode model + CapturingAddressSpaceBuilder — an in-memory
        IAddressSpaceBuilder that records a driver's DiscoverAsync stream (folder nesting,
        leaf FullName as node id, alarm mark, node-cap truncation, no-op alarm sink).
Task 7: CapturedTreeBrowseSession — serves the captured snapshot through IBrowseSession
        (Root/Expand/Attributes from memory, truncation marker, shared-immutable-tree dispose).
13 unit tests green.
2026-07-15 17:22:00 -04:00
Joseph Doherty 9cad9ed0fc docs(xmldoc): fill missing XML docs + strip tracking-ID comments across src
v2-ci / build (push) Failing after 41s
v2-ci / unit-tests (tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests) (push) Has been skipped
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests) (push) Has been skipped
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests) (push) Has been skipped
Adds <summary>/<param>/<returns>/<inheritdoc> where missing and removes
project bookkeeping IDs (task/tracking refs) from shipped code comments,
so the docs read cleanly and CommentChecker is quiet except for known
false positives (PLC/protocol terms, event/IEqualityComparer inheritdoc).
Doc/comment-only; no logic changed; solution builds clean.
2026-07-07 12:38:39 -04:00
Joseph Doherty 069a5f3165 feat(adminui): Galaxy picker pre-fills native-alarm fields from IsAlarm 2026-06-16 16:55:05 -04:00
Joseph Doherty 81f09a7054 feat(commons): add IDriverBrowser/IBrowseSession/BrowseNode abstractions 2026-05-28 15:32:01 -04:00