Commit Graph

38 Commits

Author SHA1 Message Date
Joseph Doherty 1982af8155 docs(twincat): native array notifications, sub-range / size-cap rules, live TC2 gate record
Removes the 'use UseNativeNotifications=false for arrays' workaround and
documents the raw-bytes array model shared by poll and notification: element
checks, STRING arrays, multi-dim flattening, 0-based publication, leading
sub-range for a shorter arrayLength, refusals (struct/UDT arrays, too long,
scalar over array), the 128 KiB native cap (BadOutOfRange), the bit-path fix,
and the 2026-10-07 read-only live gate on Z33856 / Z33733.
2026-10-07 14:56:03 -04:00
Joseph Doherty dcc35f3481 feat(twincat): native ADS notifications for array tags, decoded from raw bytes
An array tag subscribed natively used to register only its declared scalar
(publishing element 0 at Good) and was then refused wholesale with
BadTypeMismatch. The driver now hands the authored ArrayLength to the client,
which registers the whole array as raw bytes and decodes every notification.

- TwinCATTypeGuard.CheckArray plans the layout from the RESOLVED array type:
  element checked like a scalar notification (width / float kind / string
  kind); STRING(n)/WSTRING(n) elements stride n+1 / 2(n+1); multi-dim arrays
  flattened in PLC memory order; an authored ArrayLength shorter than the PLC
  array reads the leading sub-range, a longer one is refused; struct / pointer
  / nested-array elements, non-array symbols and VAR_IN_OUT refs are refused.
- CheckNotificationSize caps a native array at 128 KiB (largest registration
  proven on a live TC 2.11 target); above it the tag is refused BadOutOfRange.
- TwinCATArrayCodec decodes the bytes into the element-typed CLR array.
- The poll read uses the same plan + codec instead of the Beckhoff typed array
  read, which failed live for most real arrays (exact CLR element type, rank-N
  for multi-dim, member arrays sliced by the under-reported TC2 symbol size).
- TwinCATNotificationRefusedException carries the refusal's status; the type
  mismatch derives from it and the driver isolates either per tag.
2026-10-07 14:45:35 -04:00
Joseph Doherty 8060cfd2a5 fix(twincat): size strings by the resolved type and refuse VAR_IN_OUT references (live TC2 gate)
Found by the read-only live gate against the TwinCAT 2.11 Elma cleaners
Z33856 / Z33733:

- The Beckhoff library reports a TC2 dotted MEMBER symbol's ByteSize as if the
  PLC's byte count were a bit count (STRING(80) -> 11, INT -> 1, UDINT -> 1)
  while DataType.ByteSize is right (81/2/4, raw SYM_INFOBYNAMEEX says 81). The
  previous commit's typeof(byte[]) read sliced by the symbol size, so
  .OPC_MACHINEDATA.PROCESSSTATUS read "ready for o" at Good; the notification
  width check would also have refused every numeric member. The shape now takes
  the resolved type's size, and string reads go by index group/offset (or a
  fresh handle for 0xF005) at that width.
- P_INITINPUT_ACTION.SLOTID is "STRING(20) (VAR_IN_OUT)" - a 4-byte reference -
  but resolves to a plain STRING(20); a String tag read 21 bytes at the pointer
  as text at Good. The symbol's own VAR_IN_OUT marker now classifies it
  NonScalar -> BadTypeMismatch.
2026-10-07 14:27:22 -04:00
Joseph Doherty 83a314e734 fix(twincat): string subscriptions, Windows-1252 STRING decode, and declared-type kind checks (ignitionoee 0.19.2-0.19.4)
Port of the ignitionoee TwinCAT string + type-reconciliation fixes onto the
Beckhoff-library client:

- STRING/WSTRING subscriptions never worked: AddDeviceNotificationExAsync was
  called with typeof(string) and no length, and the Beckhoff any-type
  marshaller throws ArgumentNullException for a string without one, so one
  string tag tore down its whole subscribe call. A string now registers the
  PLC's own byte width as raw bytes (0.19.2: STRING(n) of any n).
- Strings are read as raw bytes and decoded by TwinCATStringCodec. The
  Beckhoff marshaller already cut a read at the first NUL (0.19.3 / TwinCAT-007
  were not defects on the read path); the codec keeps that rule for both STRING
  and WSTRING and extends it to the new notification path. A STRING now decodes
  as Windows-1252 (TwinCAT-010) - or UTF-8 when the target reports UTF-8
  strings - instead of the host culture's ANSI code page the Beckhoff
  marshaller falls back to (1250 on a Polish host, 1251 Cyrillic, ...).
- TwinCATTypeGuard (TwinCAT-002/-008): a read refuses a numeric tag over a PLC
  string (and vice versa / STRING vs WSTRING / string over a container) with
  BadTypeMismatch instead of handing back a string object at Good; a
  notification - raw bytes decoded as the DECLARED type - additionally refuses
  width and float/integer mismatches (DINT over REAL reinterpreted the bits).
- A refused notification is isolated: that tag publishes BadTypeMismatch with
  one WARN, keeps a dead intent for the probe-tick retry, and the rest of the
  subscription registers.
2026-10-07 14:05:55 -04:00
Joseph Doherty 97902d6879 fix(drivers): overrun-meter tag keys align with the Meter's snake_case convention (driver_type/driver_instance) 2026-08-17 03:54:53 -04:00
Joseph Doherty e22ecea008 feat(drivers): per-driver poll-overrun meter on the shared OtOpcUa Meter; NoteOverrun + backoffCap doc notes 2026-08-17 03:46:27 -04:00
Joseph Doherty 9cf400f1e8 perf(drivers): fixed-rate poll pacing with overrun counter; non-blocking unsubscribe 2026-08-16 04:26:20 -04:00
Joseph Doherty dd900f970a fix(hosts): a driver that never built a tag table reports NO ANSWER, not zero skips
Found on the docker-dev live gate, not in tests. Two drivers that faulted
during initialise (rtu-gate, opcua1) each rendered "0 tags" on /hosts against
two authored tags apiece, because a ledger that had never committed returned
SkippedTagReport.Empty.

A zero report is a positive claim -- "this driver skipped nothing" -- and
making it on behalf of a driver that never got as far as looking is exactly
the quiet inaccuracy #541 exists to remove. GetSkippedTags() and
SkippedTagLedger.Snapshot() now return null until the first committed build,
so the column renders the same em-dash it already used for a driver that
cannot report at all. Both are 'no answer'; neither is 'nothing skipped'.

Knock-on: a FOCAS driver whose only init threw now reports null rather than a
zero report, which is strictly more honest than the assertion the test
originally made.
2026-08-05 20:16:31 -04:00
Joseph Doherty 39db75083d feat(drivers): every driver reports the authored tags it skipped (Gitea #541)
A driver builds its RawPath -> definition table at init and tolerates a
per-tag miss: an unmappable TagConfig, an unparseable address, a
capability-matrix rejection. That tolerance is right -- one bad tag must not
take down the other few thousand. What was missing is any statement of the
fact afterwards. A skip is not an error and happens before any read, so the
driver stays Healthy, errors/5 min stays 0, Last error stays empty, and Last
read advances live off the tags that did map. On the wonder Makino in #540
that reading held while focas-z-34184 ignored 20 of its 22 tags.

New optional capability ISkippedTagReporter, backed by a shared
SkippedTagLedger so the reset/commit/cap rules are one implementation rather
than one per driver. The report -- authored count, exact skipped count, and a
capped sample of RawPath + reason -- rides DriverHealthChanged to a new Tags
column on /hosts, over the Phase 5 telemetry stream. No table: a driver-only
node has no ConfigDb to write to, the lesson from #521.

The issue named eight drivers. It is twelve -- every driver in
DriverTypeNames.All has an authored-tag skip path. Calculation (no scriptId),
MTConnect (no readable DataItem id), Galaxy (no attributeRef) and OpcUaClient
(no nodeId) were unlisted, and OpcUaClient's was the quietest of the twelve:
NamespaceMap drops such an entry with a bare `continue` and not even a log
line. SkippedTagReporterFleetTests pins the invariant reflectively over every
IDriver in src/Drivers/, so a thirteenth cannot re-introduce the silence.

Sql drops a tag at TWO points and its report is the union of both: a blob
that does not map, AND an identifier the 8.1 catalog gate rejects against the
live catalog. The second is what schema drift produces -- a renamed column, a
dropped view -- months after a deployment sealed green, and reporting only
the first would have that driver claim "0 skipped" while polling half its
tags, which is worse than reporting nothing because the column would then be
actively reassuring.

Deliberately additive to driver state, never a change of it. Latching
non-Healthy for a config problem would satisfy both halves of the #527
comms-loss conjunction -- 90 s without a successful read AND non-Healthy --
and a driver whose tags were ALL skipped has no successful reads by
construction, so it would reconnect-loop forever against a healthy device.

Three details that are load-bearing rather than incidental:

- The skip digest is part of the publish fingerprint. A skipped tag leaves
  every other fingerprint component identical to a clean driver's, so without
  it PublishHealthSnapshot's dedup swallows exactly the publish carrying the
  signal -- the trap that already bit the rediscovery and host-status
  channels. The digest covers the sample, not just the counts: a redeploy
  that fixes one tag and breaks another leaves both counts unchanged.
- The count is exact; only the sample is capped, and the tooltip says so when
  it is showing fewer than it counted. A truncated list that looked complete
  would have an operator fix twenty tags and believe the job done.
- The ledger stages and commits, so an init that throws part-way leaves the
  previous report standing. A partial snapshot would read as "we now skip
  fewer tags", the opposite of the truth.

The proto field needs no companion presence flag, unlike host_statuses: a
singular message field carries real presence in proto3, and a has_ flag would
collide with protoc's generated HasSkippedTags.

Verified red-first three ways: dropping SkippedTagDigest from the fingerprint
fails the two dedup tests, removing the seam from one driver fails the fleet
guard, and dropping the catalog-gate leg fails the Sql union test. Full
solution builds with zero new warnings against 4e22f024.
2026-08-05 19:28:46 -04:00
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 b815446940 refactor(drivers): delete GetMemoryFootprint / FlushOptionalCachesAsync (#525)
Both `IDriver` members had no production call site. Their only consumer was
`MemoryTracking`, deleted with the driver-tier recycle machinery in #522, and
`GetMemoryFootprint`'s doc-comment claimed "polled every 30s by Core" — which
had not been true since v3, even before #522 removed the poller.

Owner decision on #525 was (a) delete rather than (b) build the process-wide
watchdog that would consume them. The driver-side implementations are in git
history and were mostly reusable; what was always missing is the trigger, and a
member nobody calls is how the misleading doc-comment survived this long.

Removed from `IDriver`, all 12 driver implementations, and every test stub —
plus the support code that existed only to feed them, which is the part a
member-only deletion would have left behind as dead weight:

- MTConnect's three per-entry byte-weight constants, its cached
  `ProbeModelDataItemCount`, and `CountDataItems` — a full device-tree walk run
  on every cache fill to populate a number with no remaining reader.
- OpcUaClient's `_discoveredNodeCount` field and the discovery-pass write to it.
- MQTT's `ApproxBytesPerAuthoredTag`.

Two behavioural notes, neither a change in what the software does:

- `AbCipTemplateCache` now has no production reset path. Its only caller was
  AB CIP's flush, which itself had no caller — so nothing changes except how
  visible the fact is. Recorded on the type, along with the correction that its
  comment's claim about a device `ReinitializeAsync` also flushing it was never
  true. A UDT redefined on a live controller keeps its stale shape until the
  driver instance is rebuilt.
- MTConnect's `/probe` cache is still dropped, by `InvalidateProbeModel()` on an
  Agent restart. That is now `internal` rather than `private` so the tests which
  used the flush purely to empty the cache drive the real path instead — they
  now cover a case that actually occurs in production. Several unrelated doc
  comments cited `FlushOptionalCachesAsync` as the canonical CAS example and are
  repointed to it.

Tests: the ones that existed only to assert on the removed members are deleted
(3 OpcUaClient, 3 Galaxy, 2 MQTT, 1 MTConnect, 1 AbCip). Worth noting one was
vacuous — `FlushOptionalCachesAsync_resets_footprint_counter` asserted a
footprint of 0 before the flush and 0 after with no discovery in between, so it
would have passed against a flush that did nothing at all.

Full suite green except 7 failures verified identical on master before and
after (4 AbLegacy integration with the compose profile down, 3
Host.IntegrationTests), checked by name rather than assumed.

Docs corrected where they described this as live: `DriverLifecycle.md` (which
carried the #525 placeholder), `drivers/AbCip.md`, `drivers/AbLegacy.md`, and
`v2/driver-stability.md` — whose "polls this every 30 s" + per-driver
`DriverConfig` memory budget were never built as written and are now banner-ed
as design intent rather than description.

Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
2026-07-31 09:07:11 -04:00
Joseph Doherty 5fff597324 revert(drivers): remove the periodic 5-minute device re-browse (#523)
Removes the tag-set drift detector shipped in e77c8a35. The design was sound
-- the tautology drivers were correctly gated out, and the structurally
undetectable "vanished" direction was declared rather than faked -- but none of
that is the objection. The feature browsed real plant equipment on a recurring
timer, at a frequency no operator could configure (the interval was a
constructor parameter with no appsettings key), and that outbound traffic was
never once observed against a real PLC, CNC or MTConnect Agent.

Removed: TagSetDrift/TagSetDriftDetector; DriverInstanceActor's drift timer,
tick message, Props/ctor parameter, gate, handler and leaf-collector;
ITagDiscovery.AuthoredDiscoveryRefs + .DiscoveryStreamIncludesAuthoredTags and
their four implementations; 14 tests.

Kept: ITagDiscovery.SupportsOnlineDiscovery (it predates this and drives the
universal browse picker), and the whole IRediscoverable consumption from
09a401b8 -- the driver tells us, we do not poll.

CONSEQUENCE, deliberately accepted: AbCip and FOCAS browse a live backend but
implement no IRediscoverable, so they now have NO tag-change signal at all -- a
PLC re-download is invisible until someone re-browses by hand. Recorded in
CLAUDE.md so the next reader does not have to rediscover it, along with the
shape to reach for if that coverage is wanted back (event-driven, or triggered
by a reconnect/deploy, not a wall-clock timer).

Runtime.Tests 476 pass / 13 skipped (skip set unchanged); FOCAS 275, TwinCAT
192, MTConnect 491, AbCip 342, Core.Abstractions 266 all green.

Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
2026-07-28 13:59:06 -04:00
Joseph Doherty e77c8a3569 feat(drivers): tag-set drift detection, gated on SupportsOnlineDiscovery
The second thing #516/§8.2 deliberately did not build. The original objection
stands — Modbus, S7, MQTT, AbLegacy and Sql echo authored config from
DiscoverAsync, so diffing discovered against authored is a tautology for them
— but it is now ENCODED rather than used as a reason not to build.

The discriminator already existed: ITagDiscovery.SupportsOnlineDiscovery is
documented as "enumerates the tag set from the live backend rather than
replaying pre-declared/authored tags", and it separates the fleet cleanly —
FOCAS, TwinCAT, MTConnect and AbCip true; the five config-echo drivers
explicitly false. The check runs only for a driver that is
SupportsOnlineDiscovery AND reports the new AuthoredDiscoveryRefs. That is
nullable rather than empty-by-default on purpose: an empty collection
legitimately means "nothing authored, everything discovered is new", so
conflating the two would report total drift for a driver that never opted in.

Where the value is: AbCip and FOCAS browse a live backend but implement no
IRediscoverable, so before this they had no change signal at all. A periodic
compare is the only way to notice a PLC re-download.

A finding that changed the design: FOCAS, TwinCAT and AbCip all re-emit their
authored tags into the same DiscoverAsync stream as device-derived ones, so
the browse picker can show an operator their existing tags. Discovered is
therefore always a superset of authored, and a VANISHED tag can never appear
missing — one whole direction is structurally undetectable for three of the
four. Shipping a Vanished list that is permanently empty for them would have
been the half-inert seam this whole exercise removes. So the driver declares
DiscoveryStreamIncludesAuthoredTags, the detector does not compute the
undetectable half, and the operator message says "missing-tag detection
unavailable for this driver" rather than letting the absence of a missing-tag
clause read as reassurance. MTConnect is the only driver where both directions
work — its stream is purely probe-model-derived.

Behaviour: a failed browse is not drift (an unreachable device would otherwise
report every authored tag as vanished); a truncated capture is skipped for the
same reason; an unchanged drift is reported once rather than re-stamping its
timestamp; drift that resolves clears the prompt. Interval defaults to 5
minutes — it browses a real device, and a tag set changing is an engineering
event, not a runtime one. It reuses the Stage-2 surfacing, raising the same
/hosts re-browse prompt, and never rebuilds the served address space.

Comparison logic is a pure, separately-tested type rather than actor-inline.
14 new tests. The gate was verified load-bearing by deleting the
SupportsOnlineDiscovery half: the tautology-driver test goes red, and it
asserts discovery was never invoked rather than merely "no prompt raised",
which would have passed for the wrong reason if the timer never fired.

Full suite green except the same three pre-existing fixture-gated integration
suites, identical counts.
2026-07-28 00:48:39 -04:00
Joseph Doherty a53be2af65 v3(b1-twincat): apply Modbus exemplar pattern to TwinCAT (multi-device)
- Rename TwinCATEquipmentTagParser -> TwinCATTagDefinitionFactory; TryParse ->
  FromTagConfig(tagConfig, rawPath, out def). Drop leading-'{' heuristic + the
  deviceHostAddress key; def.Name = rawPath. Keep strict-enum + Inspect; add a
  Structure guard (sole tag path now) + inverse ToTagConfig.
- Options: Tags -> RawTags (RawTagEntry); keep Devices + protocol fields.
- Driver: _tagsByRawPath (Ordinal); byName-only resolver; build table from
  _options.RawTags via FromTagConfig, threading WriteIdempotent + DeviceName.
  Multi-device: ResolveDeviceHost matches entry.DeviceName against options.Devices
  (by name, then host) -> def.DeviceHostAddress (TODO(v3 WaveC): live host from the
  Device row's DeviceConfig). DiscoverAsync now emits from the table.
- Factory: retire pre-declared tags[] DTO path; bind rawTags; keep Devices.
- Cli: BuildOptions serialises typed defs -> RawTagEntry via ToTagConfig.
- Tests: TwinCATRawTags helper; migrate Tags= -> RawTags=; rename parser tests to
  FromTagConfig; equipment/resolve-host/array tests delivered via RawTags.

Gate: Contracts + Driver build green; Driver.TwinCAT.Tests 192 pass, Cli.Tests 70 pass.
2026-07-15 20:11:23 -04:00
Joseph Doherty 679484ae78 feat(browse): Wave-0 Batch B — AbCip/TwinCAT/FOCAS opt into online discovery
Task 3/4/5: SupportsOnlineDiscovery => true on AbCipDriver, TwinCATDriver, FocasDriver
(their device enumeration is real, config-gated on EnableControllerBrowse / FixedTree.Enabled
which PatchForBrowse guarantees at open). One gate fact per driver test project.
2026-07-15 17:18:02 -04:00
Joseph Doherty 502a327f25 fix(twincat): probe recycle honors cancellation — no uncancellable gate wait (05/STAB-12 compounded part); B6 complete (TwinCAT suite 189 green) 2026-07-13 12:43:06 -04:00
Joseph Doherty 082ee99b6b fix(twincat): retract the fresh ADS notification when unsubscribe raced the replay (05/STAB-17) 2026-07-13 12:39:39 -04:00
Joseph Doherty de29759643 fix(twincat): retry failed replay intents on the next successful probe tick (05/STAB-16) 2026-07-13 12:37:20 -04:00
Joseph Doherty a06515ae91 fix(twincat): replay failure marks the registration dead, publishes Bad quality, degrades health (05/STAB-16) 2026-07-13 12:34:43 -04:00
Joseph Doherty 9262dd25b7 fix(drivers): ResolveHost resolves equipment-tag refs via EquipmentTagRefResolver — per-host breaker isolation (CONV-4) 2026-07-13 12:26:42 -04:00
Joseph Doherty 962afed241 fix(twincat): per-device connect-attempt backoff, probe-driven recovery (05/STAB-8) 2026-07-13 12:13:23 -04:00
Joseph Doherty b494d09d9b fix(drivers): wire PollGroupEngine onError->health + 30s poll backoff (05/STAB-9) [twincat,focas] 2026-07-13 11:59:49 -04:00
Joseph Doherty 3b5ef439d4 docs(xmldoc): fix genuine doc issues surfaced by /fixdocs (17 fixes)
v2-ci / build (push) Successful in 3m11s
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests) (push) Failing after 3m51s
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests) (push) Failing after 1m50s
v2-ci / unit-tests (push) Failing after 8m37s
- CapabilityInvoker: 3 methods that implement IDriverCapabilityInvoker now use
  <inheritdoc /> instead of duplicating the interface docs (MissingInheritDoc).
- TwinCAT NativeRegistration ctor: added the 8 missing <param> tags (MissingParam).
- Stripped the arch-review bookkeeping IDs STAB-1 (IS7Plc, S7Driver) + STAB-2
  (TwinCATDriver x4) from comments, keeping the descriptive text.

Left the other 86 flagged items untouched — they are checker false positives on
legitimate technical content: interface-implemented events / IEqualityComparer /
IDisposable members whose <inheritdoc/> IS correct (10 InheritDocMisused), and domain
terms the heuristic misreads as tracking IDs (IEEE-754, S7-1200/300/400/1500, PLC-5,
D2-260, S7/Melsec memory addresses, OPC UA Part 9, phase-N design-doc filename refs,
UNS level-N descriptions, and prose like 'the #1 bug source' / failover 'pick #3').
2026-07-10 06:23:12 -04:00
Joseph Doherty af318fb442 fix(twincat): replay native ADS notifications after reconnect (archreview STAB-2 / Critical 4)
Native ADS notifications (the default subscribe mode) were stored as opaque handles
with no record of the symbol/type/interval/handler needed to replay. On a client swap
(EnsureConnectedAsync building a fresh client after a drop) the notifications were
silently orphaned — no Bad status, no error, pushes just stopped until redeploy.
Compounding: the IsConnected fast-path keys on AMS-port state, not wire liveness, and
a probe failure only transitioned state without recycling the dead client.

Fix:
- Store REPLAYABLE INTENT: NativeRegistration (symbol/type/bit/interval/onChange +
  swappable live handle) hung off DeviceState.NativeRegistrations, populated by
  SubscribeAsync via RegisterNotificationAsync (under ConnectGate).
- Split EnsureConnectedAsync into a gate wrapper + EnsureConnectedUnderGateAsync core;
  when the core installs a NEW client it replays every stored intent onto it and swaps
  the live handle (disposing the dead one). Register + replay both run under ConnectGate
  so they can't race.
- Probe loop: on a wire-probe failure (false or throw) RecycleClientAsync disposes+nulls
  the client so the next tick rebuilds + replays — closes the fast-path-keys-on-port-state
  compounding bug.

No TwinCAT docker fixture exists (integration needs a real TC3 XAR), so the fake-client
unit tests are the authoritative coverage:
- 4 new guards in TwinCATReconnectReplayTests (replay-onto-fresh-client + push reaches
  OnDataChange + old handle disposed; replay-all-tags; unsubscribe-after-reconnect stops
  replaying; probe-failure recycles+rebuilds).
- Full TwinCAT unit suite 174/174 green; full solution builds 0 errors.
2026-07-08 17:20:10 -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 a378b572af feat(otopcua): add ITagDiscovery.RediscoverPolicy + per-driver assignments (follow-up B) 2026-06-26 12:18:44 -04:00
Joseph Doherty 340c145e87 fix(twincat): dispose bit-RMW locks on shutdown + guard null parent read (review) 2026-06-17 12:01:55 -04:00
Joseph Doherty a73e20fb28 feat(twincat): BOOL-within-word writes via driver-level parent-word RMW 2026-06-17 11:55:44 -04:00
Joseph Doherty 3e74239532 feat(twincat): 1-D array symbol read via ADS + IsArray discovery 2026-06-16 21:59:17 -04:00
Joseph Doherty 5ebf541f54 feat(twincat): resolve equipment-tag refs (read + write) via EquipmentTagRefResolver 2026-06-13 11:24:12 -04:00
Joseph Doherty 64e3fbe035 docs: backfill XML documentation across 756 files
v2-ci / build (push) Failing after 1m43s
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>, <typeparam>, and <inheritdoc/> tags to public
members surfaced by commentchecker — resolves 5,847 of 5,869 issues
(99.6%) across three /fixdocs passes.
2026-05-28 08:10:17 -04:00
Joseph Doherty 3c75db7eb6 fix(driver-twincat): resolve Low code-review findings (Driver.TwinCAT-004,006,014,015,016)
- Driver.TwinCAT-004: corrected the IEC time-type inline comments;
  documented that the driver currently surfaces them as raw UInt32
  counters.
- Driver.TwinCAT-006: ResolveHost returns a documented UnresolvedHost
  sentinel when no devices are configured instead of returning the
  logical DriverInstanceId (which never matches GetHostStatuses).
- Driver.TwinCAT-014: wired Probe.Timeout into the probe-loop call and
  added a NotificationMaxDelayMs config knob threaded through
  AddNotificationAsync.
- Driver.TwinCAT-015: Dispose() runs a genuinely synchronous teardown
  with bounded waits (no sync-over-async deadlock pattern).
- Driver.TwinCAT-016: pinned the Structure-tag rejection and the
  probe-loop vs read disposal race with regression tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 08:17:42 -04:00
Joseph Doherty 3f6b61133e fix(driver-twincat): resolve Medium code-review finding (Driver.TwinCAT-012)
GetMemoryFootprint now returns tagsByName * 256 + nativeSubs * 512 bytes
instead of a hard-coded 0; document that the stream-and-discard symbol
browse leaves no flushable cache so FlushOptionalCachesAsync is a
deliberate no-op.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-22 10:50:28 -04:00
Joseph Doherty 40b28e8820 fix(driver-twincat): resolve Medium code-review finding (Driver.TwinCAT-011)
Confirm AdsErrorCode values from Beckhoff.TwinCAT.Ads 7.0.172 and rewrite
MapAdsError with 20 explicit cases. Fix critical bug: AdsSymbolVersionChanged
was 0x0702 (DeviceInvalidGroup) but DeviceSymbolVersionInvalid is 1809
(0x0711); correct constant and all comments. Add BadOutOfService for
DeviceNotReady and BadInvalidState for DeviceInvalidState/PLC-in-Config.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-22 10:49:38 -04:00
Joseph Doherty 98d8df4adf fix(driver-twincat): resolve Medium code-review finding (Driver.TwinCAT-009)
Swap _devices and _tagsByName to ConcurrentDictionary so ShutdownAsync
Clear() no longer races concurrent TryGetValue calls; store ProbeTask
on DeviceState and await it in ShutdownAsync before disposing the client
and gate, eliminating the probe-disposal race.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-22 10:43:35 -04:00
Joseph Doherty 40aa27b64b fix(driver-twincat): resolve Medium code-review finding (Driver.TwinCAT-005)
Inject optional ILogger<TwinCATDriver> (NullLogger default) and log
connect success/failure, ADS read errors, symbol-browse fallback,
native-notification registration failures, and host-state transitions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-22 10:42:08 -04:00
Joseph Doherty 5197b6c237 fix(driver-twincat): resolve High code-review findings (Driver.TwinCAT-001, -002, -007, -008, -013)
Driver.TwinCAT-001 — InitializeAsync/ReinitializeAsync ignored driverConfigJson.
Extracted the DTO-to-options parse into a shared TwinCATDriverFactoryExtensions.ParseOptions;
InitializeAsync now re-parses driverConfigJson into a mutable _options field, so a config
generation pushed via ReinitializeAsync (added/removed devices, tags, probe settings) is
actually applied at runtime.

Driver.TwinCAT-002 — LInt/ULInt narrowed to Int32. ToDriverDataType now maps LInt to Int64,
ULInt to UInt64, UDInt to UInt32, UInt/USInt to UInt16, Int/SInt to Int16, and the IEC
TIME/DATE/DT/TOD types to UInt32 (their raw UDINT counter). Removed the stale "Int64 gap"
comment — no truncation or sign flips at the OPC UA encode layer.

Driver.TwinCAT-007 — EnsureConnectedAsync was not thread-safe. Connect/reconnect is now
serialized per device by a SemaphoreSlim (DeviceState.ConnectGate) with a double-checked
connect, mirroring the S7 driver. Concurrent read/write/probe callers can no longer leak a
client or race a create-vs-dispose.

Driver.TwinCAT-008 — native ADS notification callbacks ran driver logic on the AMS router
thread. AdsTwinCATClient now enqueues AdsNotificationEx callbacks onto a bounded Channel
drained by a dedicated managed task; the router-thread callback only does a non-blocking
TryWrite, so a slow consumer cannot stall ADS notification delivery process-wide.

Driver.TwinCAT-013 — TwinCATDriver did not implement IRediscoverable. The driver now
implements IRediscoverable; AdsTwinCATClient detects ADS 0x0702 (symbol-version-changed) on
read/write paths and raises OnSymbolVersionChanged, which the driver forwards as
OnRediscoveryNeeded so Core rebuilds the address space after a PLC program re-download.

Adds TwinCATHighFindingsRegressionTests covering all five fixes; updates the data-type
mapping assertion in TwinCATDriverTests. TwinCAT driver builds clean; 119 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 06:37:05 -04:00
Joseph Doherty a25593a9c6 chore: organize solution into module folders (Core/Server/Drivers/Client/Tooling)
Group all 69 projects into category subfolders under src/ and tests/ so the
Rider Solution Explorer mirrors the module structure. Folders: Core, Server,
Drivers (with a nested Driver CLIs subfolder), Client, Tooling.

- Move every project folder on disk with git mv (history preserved as renames).
- Recompute relative paths in 57 .csproj files: cross-category ProjectReferences,
  the lib/ HintPath+None refs in Driver.Historian.Wonderware, and the external
  mxaccessgw refs in Driver.Galaxy and its test project.
- Rebuild ZB.MOM.WW.OtOpcUa.slnx with nested solution folders.
- Re-prefix project paths in functional scripts (e2e, compliance, smoke SQL,
  integration, install).

Build green (0 errors); unit tests pass. Docs left for a separate pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 01:55:28 -04:00