All 4 Criticals complete. crit4 branch af318fb4: replayable-intent store +
reconnect-replay + probe-recycle, 4 unit guards + 174/174 (unit-only, no TC3 fixture).
Next: systemic guards (task 6 forwarding test first) + Critical-1 failover harness (task 9).
51 KiB
Design + Implementation Plan — 05 Protocol Drivers
Status (2026-07-08): BOTH Criticals DONE. STAB-1 (S7 reconnect, task 4) —
fix/archreview-crit3-s7-reconnect25c0c6f6;IS7Plc/IS7PlcFactoryseam + lazyEnsureConnectedAsync; 4 unit guards + LIVE-verified against real snap7 (container bounce → recovers, no redeploy). STAB-2 (TwinCAT orphaned ADS subs, task 5) —fix/archreview-crit4-twincat-replayaf318fb4; replayable-intent store + reconnect replay + probe-recycle; 4 unit guards + 174/174 (unit-only — no TwinCAT docker fixture). Remaining (non-Critical): STAB-4/5 (AbCip lock, FOCAS caches) + Part B shared-template consolidation. SeeSTATUS.md.
Source report: archreview/05-protocol-drivers.md
Review commit: 9cad9ed0 (== current HEAD, so no drift)
Verification date: 2026-07-08
Scope: Modbus, S7, AbCip, AbLegacy, TwinCAT, FOCAS, OpcUaClient (+ Contracts, + Cli)
Verification summary
Every finding in the report was opened at its cited file:line and checked against the current tree. 37 actionable findings; 37 CONFIRMED; 0 stale. (STAB-13 is a "positive" observation carrying no work.) The line references were accurate to within 1–2 lines throughout. The zero-drift result is expected: the review commit is the current HEAD.
Confirmed counts by section: Stability 12 actionable (STAB-1…12) + 1 positive (STAB-13); Performance 7 (PERF-1…7); Conventions 8 (CONV-1…8); Underdeveloped 9 (UNDER-1…9).
Top 3 by priority (align with OVERALL actions #3/#4 and report remediation order):
- STAB-1 — S7 no reconnect path (Critical).
Plc.OpenAsynconly atS7Driver.cs:165; grep confirms it is the sole open site, noEnsureConnected. - STAB-2 — TwinCAT native subscriptions orphaned after reconnect (Critical).
NativeSubscription(TwinCATDriver.cs:517-519) stores only disposable handles, not replayable intent;EnsureConnectedAsyncswaps the client (:614-644) and nothing re-runsAddNotificationAsync;AdsTwinCATClient.Disposeclears_notifications(:426). - STAB-4 + STAB-5 — silent-corruption class. AbCip runs Read→GetStatus→Decode
on shared libplctag handles with no per-runtime lock (
AbCipDriver.cs:544-575; onlyGetRmwLockexists, bit-writes only), while AbLegacy guards the identical hazard withGetRuntimeLock(AbLegacyDriver.cs:786, used at:227/:331). FOCAS's fixed-tree caches are plainDictionary(FocasDriver.cs:1184,1190,1192,1194) read/written across threads.
The two systemic threads the report calls out — theme #2 (fixes don't flow back to shared templates) and theme #6 (equipment-tag/TagConfig paths are second-class) — are both real and are given a dedicated consolidation section (Part B) so the fleet fixes land in one home rather than being re-patched per driver.
Part A — Per-driver Criticals & Highs
A1. STAB-1 (Critical) — S7 has no reconnect path
Restatement: the S7 Plc is opened once in InitializeAsync; a transient PLC
reboot/network blip permanently kills the instance until redeploy.
Verification: CONFIRMED. grep OpenAsync S7Driver.cs → single hit at line 165,
inside InitializeAsync. No EnsureConnected/Reconnect symbol. Reads/writes call
RequirePlc() under _gate and reuse the dead handle
(S7Driver.cs:479-483, 995-1001). The probe loop detects the outage and transitions
Stopped but never recycles. ReinitializeAsync (:212) just re-calls InitializeAsync
(externally driven only). Zero S7 reconnect tests (tests/Drivers/.../S7.Tests has no
reconnect file; only Modbus, OpcUaClient do).
Root cause: the Plc is a concrete S7.Net type new-ed inline
(S7Driver.cs:156, field at :122), with no factory seam and no lazy-connect
gate. The template calls for a lazy EnsureConnectedAsync (TwinCAT/AbCip have one;
S7 never got it).
Design (mirror TwinCAT + OpcUaClient):
- Introduce a testable seam:
IS7Plc(wrapping the members S7Driver uses:OpenAsync,Close,ReadBytesAsync/ReadMultiple,Write*,IsConnected) andIS7PlcFactorywithCreate(cpu, host, port, rack, slot). Default impl wrapsS7.Net.Plc; the ctor takes an optionalIS7PlcFactory? = nulldefaulting to the real one — exactly TwinCAT'sITwinCATClientFactorypattern (TwinCATDriver.cs:20,58,64). - Add
private async Task<IS7Plc> EnsureConnectedAsync(CancellationToken ct)guarded by the existing_gate: fast-path returns a livePlc; under the gate, if thePlcis null or a prior socket-levelPlcExceptionmarked it dead, dispose and re-OpenAsynca fresh instance (with the same timeout-linked CTS as:163-164). - Route
ReadAsync/WriteAsync/PollOnceAsyncthroughEnsureConnectedAsyncinstead ofRequirePlc(). On a socket-levelPlcExceptionduring a read/write, null thePlc(mark dead) so the next call reopens, and setDegraded(keepLastSuccessfulRead) rather thanFaulted. Preserve the existingFaultedescalation for PUT/GET-denied (S7Driver.cs:921-1006) — that is a permanent config fault, not a transient drop. - Classify socket-level vs protocol-level
PlcExceptionin a smallIsS7ConnectionFatal(Exception)helper (S7.Net surfacesErrorCode.ConnectionError/WriteData/ReadData/ IO/socket exceptions). Do not reopen on a data-address/type error.
Alternatives considered: (a) let the probe loop own recycle-on-Stopped — rejected
as sole mechanism because reads between probe ticks would keep failing on the dead
handle; the lazy EnsureConnectedAsync repairs on the very next data call and the
probe becomes a backstop. (b) Reconnect handler à la OpcUaClient's
SessionReconnectHandler — overkill; S7.Net has no session-transfer semantics, a
plain reopen is correct.
Implementation steps:
- New file
Driver.S7/IS7Plc.cs(+S7PlcAdapter,IS7PlcFactory,S7PlcFactory). ChangeS7Driver.Plcfield type toIS7Plc?. - Thread
IS7PlcFactorythrough the ctor +S7DriverFactoryExtensions.CreateInstance. - Add
EnsureConnectedAsync+IsS7ConnectionFatal; convert the twoRequirePlccall clusters andPollOnceAsync(:1295-1309). - Optionally: on probe transition to
Stopped, null thePlcso the next data call reconnects even if no read failed in between.
Tests:
- Unit (new
S7DriverReconnectTests.cs): fakeIS7PlcFactorywhoseIS7Plcthrows a connection-fatalPlcExceptionon read N, succeeds on read N+1; assert the driver creates a secondIS7Plc, health goesDegraded→Healthy, and the tag recovers to Good. Assert a protocol error (illegal address) does not trigger a reopen. - Integration (
S7.IntegrationTests, skip-gated onSnap7ServerFixture): subscribe, bounce the snap7 container (lmxopcua-fix down s7 s7_1500/up), assert values resume without redeploy. Note: no per-test container hook exists, so gate this as a manual/[Trait("Category","Reconnect")]recipe.
Effort: M. Risk: Medium — touches the hot read/write path; the IS7Plc seam
is mechanical but broad. Mitigated by the fake-factory unit tests (which also close
the TEST-1 gap).
A2. STAB-2 (Critical) — TwinCAT native subscriptions orphaned after reconnect
Restatement: native ADS notifications (the default mode) silently stop after a client swap — no Bad status, no error.
Verification: CONFIRMED. SubscribeAsync registers AddNotificationAsync once
per tag (TwinCATDriver.cs:472) and stores the resulting handles in
NativeSubscription(Handle, Registrations) (:495, :517-519) — no record of the
symbol/type/interval/handler needed to replay. EnsureConnectedAsync disposes a
stale client and creates a fresh one (:614-644); AdsTwinCATClient.Dispose clears
_notifications (:426). Compounding: the fast path keys on IsConnected (:606),
the local AMS-port state, not wire liveness; and a probe failure only transitions
state, never forces a client recycle (ProbeLoopAsync :540-547).
Root cause: subscription state is stored as opaque handles rather than replayable intent, and there is no "client changed → replay" hook. Native ADS is push, so unlike the poll fallback it cannot self-heal by re-reading.
Design:
- Change
NativeSubscriptionto store replayable registrations:record NativeRegistrationIntent(string DeviceHostAddress, string Reference, string SymbolName, TwinCATDataType Type, int? BitIndex, TimeSpan Interval, Action<string,object?> OnChange)plus the liveITwinCATNotificationHandle. - Give
DeviceStatean epoch/generation counter incremented every timeEnsureConnectedAsyncinstalls a new client (:644). After installing a new client, replay everyNativeRegistrationIntentwhoseDeviceHostAddressmatches: callAddNotificationAsyncagain and swap the stored live handle. Do this insideEnsureConnectedAsyncunderConnectGateso it cannot race a concurrent subscribe. - Fix the liveness signal: the probe should call a real read (
ProbeAsyncalready does a symbol read) and, on failure, dispose+null the device client soEnsureConnectedAsyncrebuilds — i.e. make probe-detected outage force a recycle (ProbeLoopAsynccatch block at:541-545should nulldevice.Client). Consider keying the fast path (:606) additionally on a "known-good since last probe" flag so a locally-connected-but-wire-dead port still triggers the rebuild-and-replay.
Alternatives: switch the default to the poll fallback (self-healing) — rejected; native push is the performance-correct default and the report explicitly wants replay, not abandonment.
Implementation steps:
- Introduce a per-device
ConcurrentDictionary<subId, List<NativeRegistrationIntent>>(or hang intents offDeviceState). Populate inSubscribeAsync. - Extract the per-tag registration into
RegisterNotificationAsync(device, intent)used by both initial subscribe and replay. - In
EnsureConnectedAsync, afterdevice.Client = client(:644), iterate the device's intents and replay; log a re-registration count. - In
ProbeLoopAsync, on probe failure null the device client (force rebuild).
Tests:
- Unit (new
TwinCATReconnectReplayTests.cs) using the existing fakeITwinCATClientFactory/ITwinCATClient: subscribe a tag, simulate a client that reportsIsConnected=false, forceEnsureConnectedAsyncto build a replacement, assertAddNotificationAsyncis re-invoked with the same symbol/type/interval and that a subsequent fake push reachesOnDataChange. Assert the old handle is disposed. - Unit: probe-failure recycles the client (assert a new client instance built).
- Note there is no TwinCAT docker fixture (13 integration tests need a real TC3 XAR); the fake-client unit tests are the only automated coverage — make them authoritative.
Effort: M. Risk: Medium — concurrency around ConnectGate/replay; must not
double-register. The fake client makes it fully unit-testable (the report's key gap).
A3. STAB-3 (High) — Modbus transport desyncs after a response timeout
Restatement: a per-op timeout or TxId mismatch leaves the single-flight socket half-read; the next txn reads a stale response and the stream stays desynchronized.
Verification: CONFIRMED. IsSocketLevelFailure (ModbusTcpTransport.cs:257-261)
matches only EndOfStreamException/IOException/SocketException/
ObjectDisposedException. The per-op deadline (:226-227 linked CTS CancelAfter)
surfaces as OperationCanceledException; the TxId guard throws InvalidDataException
(:234-235); the truncated-length guard throws InvalidDataException (:237) — none
of which are socket-level, so the reconnect-and-retry catch (:155-165) is skipped
and the socket is retained mid-desync.
Root cause: the transport conflates "protocol-layer error, keep the socket" with
"framing/timeout error, the socket is unusable." Timeout and any framing violation
leave unknown bytes in the receive buffer; the connection is fatally desynchronized
even though the exception type is not IOException.
Design: treat per-op timeout and any framing violation as connection-fatal:
tear the socket down before propagating, so the next SendAsync reconnects clean.
- In
SendOnceAsync, wrap the read/write in a try that, onOperationCanceledExceptionwhere the caller'sctis NOT cancelled (i.e. the timeout linked-CTS fired), and onInvalidDataException, callsTearDownAsync()then rethrows a normalizedModbusTransportDesyncException : IOException(subclassing IOException lets the existing:155catch optionally reconnect-and-retry once). Distinguish caller cancellation (propagate as OCE, no teardown) from timeout (teardown). - Simplest correct implementation: after
TearDownAsync, rethrow asIOExceptionso both the retry classifier and the desync-repair share one path.
Alternatives: add timeout/framing to IsSocketLevelFailure directly — works but
must still tear down the socket first (the current retry path does TearDownAsync
before reconnect, so adding the types and ensuring teardown covers it). Preferred to
keep IsSocketLevelFailure about exception type and add an explicit teardown at the
throw site for framing/timeout, since those need teardown even when auto-reconnect is
off.
Implementation steps:
- In
SendOnceAsync, catchOperationCanceledException(timeout branch) andInvalidDataException;await TearDownAsync(); rethrow asIOException(ModbusTransportDesyncException). - Keep the outer
SendAsyncsingle-retry (:155-165) — it now covers the desync. - Ensure teardown even when
_autoReconnectis false (the socket must not be reused desynced regardless).
Tests:
- Unit (
ModbusTcpReconnectTests.csextension): a fake stream that (a) stalls past the deadline, (b) returns a wrong TxId, (c) returns a truncated header. Assert each triggersTearDownAsync(socket disposed) and the following call reconnects. Assert a caller-cancellation (external ct) does NOT tear the socket down. - Integration (
Modbus.IntegrationTests+exception_injector): the existing rig already forces FC exceptions; add a timeout scenario (non-responding unit) and assert recovery on the next poll.
Effort: S. Risk: Low-Medium — must get the timeout-vs-cancellation distinction right or a legitimate shutdown could look like a desync (harmless, just an extra reconnect).
A4. STAB-4 (High) — AbCip concurrent ops on shared libplctag handles, no lock
Restatement: AbCip read/group/write run Read→GetStatus→Decode /
Encode→Write→GetStatus on cached IAbCipTagRuntime with zero serialization while the
server read path, poll loops, and the alarm-projection loop can hit the same handle.
Verification: CONFIRMED. ReadSingleAsync (AbCipDriver.cs:544-575),
ReadGroupAsync (:614-637), WriteAsync (:708-726) take no runtime lock; the only
locks are GetRmwLock (bit-in-DINT writes, :1243) and a creation lock. AbLegacy
documents the exact hazard and guards it: GetRuntimeLock (AbLegacyDriver.cs:786),
applied on both read (:227) and write (:331), comment "A libplctag Tag handle is
not safe for concurrent Read/GetStatus/DecodeValue." AbCipAlarmProjection.cs:149/228
reads through the unlocked path. libplctag's GetStatus returns the last operation's
status, so concurrent Read/GetStatus can cross-attribute results → torn values with
Good status.
Root cause: the AbLegacy fix never propagated to AbCip (theme #2). AbCip caches a per-tag runtime but never serializes operations on it.
Design: port AbLegacy's per-runtime operation lock verbatim.
- Add
GetRuntimeLock(string tagName)to AbCip'sDeviceState(aConcurrentDictionary<string,SemaphoreSlim>keyed by tag name, matchingAbLegacyDriver.cs:786). - Wrap the whole Read/GetStatus/Decode critical section in
ReadSingleAsyncandReadGroupAsync, and the Encode/Write/GetStatus section inWriteAsync, inawait opLock.WaitAsync(ct)/finally Release. The existingGetRmwLock(bit RMW) must nest inside or replace-by the runtime lock consistently — since RMW does read-modify-write on the same handle, take the runtime lock for the whole RMW too (this also fixes STAB-11's "direct parent write bypasses the lock" for AbCip: a direct parent write and an RMW now serialize on the same per-tag runtime lock).
Alternatives: a single per-device lock — rejected; too coarse, serializes independent tags. Per-runtime lock matches AbLegacy and preserves cross-tag parallelism.
Implementation steps:
- Add
_runtimeLocks+GetRuntimeLockto AbCipDeviceState. - Wrap the three op bodies. Ensure eviction-on-failure (
EvictRuntime,:554) also disposes/removes the tag's lock or leaves it (a fresh runtime under the same name reuses the lock — fine). - Reconcile RMW: make
WriteBitInDIntAsyncacquire the per-runtime lock instead of (or in addition to)GetRmwLock; collapse to one lock domain per parent to also close STAB-11.
Tests:
- Unit (new
AbCipRuntimeConcurrencyTests.cs, mirrorAbLegacyRuntimeConcurrencyTests): a fakeIAbCipTagRuntimethat asserts no overlapping Read/GetStatus (e.g. a reentrancy counter that throws if >1); hammer concurrentReadAsync+WriteAsync+alarm-read on one tag; assert serialization and no torn/cross-attributed status. - Integration (
abcipfixture on:44818): concurrent subscribe + write to overlapping tags, assert Good values stay coherent.
Effort: S. Risk: Low — additive locking, blast radius is AbCip only.
A5. STAB-5 (High) — FOCAS fixed-tree caches are plain Dictionaries mutated across threads
Restatement: LastFixedSnapshots/LastServoLoads/LastSpindleLoads/LastTimers
are plain Dictionary, written by FixedTreeLoopAsync and read on ReadAsync threads.
Verification: CONFIRMED. Fields at FocasDriver.cs:1184 (Dictionary<string,double>),
:1190 (Dictionary<FocasTimerKind,FocasTimer> = []), :1192
(Dictionary<string,double>), :1194 (Dictionary<int,int> = []). Writes at
:752, :793, :836-839; reads at :912, :920, :945. Contrast the neighbouring
_parsedAddressesByTagName = new ConcurrentDictionary (:40) — that field got the
treatment, these did not. Concurrent write-during-Dictionary-resize is UB (throw or
corruption).
Root cause: partial application of the concurrent-collection fix (theme #2).
Design: convert the four caches to ConcurrentDictionary<,>. TryGetValue and
indexer-set are the only operations used, both supported. LastTimers and
LastSpindleLoads collection-initializer [] becomes new().
- Alternative (immutable snapshot swap): build a fresh dictionary per loop pass and
Volatile.Writethe reference; readersVolatile.Read. Cleaner isolation but more churn;ConcurrentDictionaryis the lower-risk minimal change and matches the in-tree precedent (:40).
Implementation steps: change the four field types (on DeviceState) + collection
initializers; no call-site changes needed (indexer + TryGetValue are identical).
Tests:
- Unit (
FocasDriver.Tests): a stress test spinningFixedTreeLoopAsync-style writes while readers callTryReadFixedTree; assert no exception over N iterations (would reliably throw on the plainDictionarytoday).
Effort: S. Risk: Very low — type-only change, FOCAS-local.
A6. STAB-6 (High) — AbLegacy probe-loop shutdown race; S7 unsubscribe race
Verification: CONFIRMED both.
- AbLegacy: probe task fire-and-forgot
_ = Task.Run(...)(AbLegacyDriver.cs:118);ShutdownAsync(:156-170) cancels then immediately disposesProbeCts+ runtimes without awaiting loop exit — a winding-down loop can raiseOnHostStatusChangedand touch a disposed CTS. AbCip already fixed this: retainedProbeTask(:298) + phased shutdown withawait probeTask.WaitAsync(10s)(:332-365). - S7:
UnsubscribeAsync(:1185-1193) cancels + disposes the CTS without drainingPollTask;Task.Delayon the disposed source can throwObjectDisposedExceptionthe loop only catches as OCE (:1233-1234).ShutdownAsyncdoes drain PollTask (:239), so the bug is confined to theUnsubscribepath.
Root cause: cancel-then-dispose-without-await; PollGroupEngine.StopState
(PollGroupEngine.cs:99-112) already solves this by awaiting the loop (bounded) before
disposing the CTS — neither bespoke path adopted it.
Design:
- AbLegacy: retain the probe task in
DeviceState.ProbeTaskand phase shutdown like AbCip (await probeTask.WaitAsync(bounded)beforeProbeCts.Dispose()). - S7: in
UnsubscribeAsync,Cancel(), thenstate.PollTask.Wait(bounded)(or await), thenCts.Dispose()— mirroringStopState. Better: once S7 migrates toPollGroupEngine(see B1) this deletes itself.
Tests: unit — subscribe/unsubscribe rapidly in a loop and assert no
ObjectDisposedException escapes; shutdown while a probe tick is mid-transition.
Effort: S. Risk: Low.
A7. UNDER-1 (High) — FOCAS advertises writes it cannot perform
Verification: CONFIRMED. WireFocasClient.WriteAsync returns
FocasStatusMapper.BadNotWritable for every address (WireFocasClient.cs:71-73), yet
FocasDriverOptions.Writable defaults true (FocasDriverOptions.cs:168) and
FocasEquipmentTagParser hard-codes Writable: true (FocasEquipmentTagParser.cs:35).
The driver's write exception-mapping (FocasDriver.cs:353-381) is dead code against
this backend. Net: equipment tags surface as Operate-writable OPC UA nodes whose writes
always fail.
Root cause: the writable default was written speculatively (doc cites "write tests") but the only real backend is read-only.
Design (force writable-false at the seams until PMC writes exist):
FocasEquipmentTagParser.cs:35→Writable: false.FocasDriverOptions.Writabledefault →false(and/or force it false at discovery materialization). Keep the option field for the future PMC-write feature but do not advertise writable nodes today.- Leave the write status-mapping in place (it becomes live if/when PMC writes ship).
- Alternative (larger): implement PMC writes in
WireFocasClientviapmc_wrpmcrng— a separate feature, out of scope for a hardening pass.
Tests: unit — assert a FOCAS equipment/authored tag materializes without the
CurrentWrite access bit; assert a write attempt returns BadNotWritable (guards the
regression if someone flips the default back).
Effort: S. Risk: Very low (removes a false capability).
A8. UNDER-2 / UNDER-3 (High) — OpcUaClient: dead UnsMappingTable gate; broken alarm-filter encoding
UNDER-2 verification: CONFIRMED. ValidateNamespaceKind hard-fails Equipment-kind
configs lacking UnsMappingTable (OpcUaClientDriver.cs:335-338) but grep shows the
table is read nowhere except that gate and the options DTO
(OpcUaClientDriverOptions.cs:169,188). DiscoverAsync renders refs purely from remote
browse via StableReference (:808-809). Operators must author a table with zero
runtime effect.
UNDER-3 verification: CONFIRMED. Persisted refs are stable nsu= strings
(:808-809), but the alarm source filter is an ordinal HashSet of caller refs
(:1337) matched against EventFields[SourceNode].ToString() — a session-relative
ns=N;… rendering (:1538-1539). The two encodings never match, so any non-empty
filter drops every event; only empty-filter subscriptions work.
Root cause: UNDER-2 is a validation gate for an unbuilt feature; UNDER-3 is an
encoding mismatch between the stored nsu= form and the runtime ns=N form.
Design:
- UNDER-2: delete the gate (remove the Equipment-kind
UnsMappingTablemandatory check) — the honest minimal change, since the feature does not exist. Keep the option DTO field (forward-compatible) but stop forcing operators to author dead config. Alternative (build the feature): haveDiscoverAsynctranslate remote refs → UNS refs via the table — a real feature, larger, defer. - UNDER-3: normalize both sides to the same encoding before compare. Convert the
incoming
EventFields[SourceNode]NodeIdto the stablensu=reference via the driver'sNamespaceMap.ToStableReference(the same function used at:808-809) before theHashSet.Containscheck; build the filter set from stable refs. Then a non-empty filter works.
Implementation steps: remove ValidateNamespaceKind's Equipment branch (or make it
a no-op warning); in OnEventNotification, map the source NodeId through
_namespaceMap before comparing (:1538-1539); build sourceFilter from stable refs
(:1337).
Tests: unit — event with a known source arrives, non-empty stable-ref filter matches and the event is delivered (today it is dropped); Equipment config without a mapping table no longer throws at validation.
Effort: S (both). Risk: Low — OpcUaClient-local; UNDER-3 fix is a behavior change that enables previously-dead filtering (verify no consumer depended on the "filter silently drops all" bug).
A9. STAB-13 (Positive) — no action
Write-outcome surfacing is honoured by every protocol driver (per-item WriteResult
with typed exception→status), and OpcUaClient's reconnect machine is exemplary
(double-arm guard, re-arm on exhaustion, session swap under _probeLock,
:1908-2058). CONFIRMED. Use OpcUaClient's machine as the reference for A1/A2. One
caveat surfaced: Session.TransferSubscriptionsOnReconnect is left unset
(mentioned only in the doc at :1948), so subscription transfer rests on the SDK
default — worth explicitly setting it (folds into UNDER-9 cleanup).
Part B — Shared-template consolidation (themes #2 and #6)
The report's cross-cutting message: every High is a fix present in one sibling and absent in another. These four items give the fleet a single home for the fixes so they stop being re-patched per driver. Do these after the per-driver criticals but treat them as the highest-leverage structural work.
B1. PollGroupEngine v2 — absorb S7's fork; wire onError; add backoff (CONV-1, STAB-9, STAB-8, PERF-3, STAB-6-S7)
Verification of the pieces:
PollGroupEnginealready has theonErrorsink (PollGroupEngine.cs:36,55-67,164-169),StopStatedraining (:99-112), and structural array compare (:203-209). It does not have failure backoff —PollLoopAsyncuses a fixedTask.Delay(state.Interval)(:131).- STAB-9 CONFIRMED: no consumer passes
onError— the five constructions (ModbusDriver.cs:115,AbCipDriver.cs:133,AbLegacyDriver.cs:65,TwinCATDriver.cs:69,FocasDriver.cs:72) omit it, so the sink is dead fleet-wide. - CONV-1 / PERF-3 / STAB-6-S7 CONFIRMED: S7 re-ships the loop (
S7Driver.cs:1165-1307), which has backoff (ComputeBackoffDelay :1284-1293) and failure-health (HandlePollFailure :1258-1274) the engine lacks, but lacks the structural array diff (usesEquals(lastSeen?.Value, current.Value):1304→ arrays fire every tick) and the drain-before-dispose (UnsubscribeAsync :1189-1190).
Design (single home for the poll loop):
- Add optional backoff to
PollGroupEngine. Port S7'sComputeBackoffDelay(capped exponential, ticks-based, saturating at aPollBackoffCap) into the engine. Constructor gainsTimeSpan? backoffCap = null(null ⇒ current fixed-cadence behavior, preserving every current consumer).PollLoopAsynctracksconsecutiveFailures, resets on success, and delaysComputeBackoffDelay(interval, failures). - Route failures to health. The engine already forwards to
onError; make each of the five drivers pass anonErrorthat degrades_health(keepingLastSuccessfulRead) + logs — i.e. lift S7'sHandlePollFailureinto a small sharedAction<Exception>per driver. Consider makingonErrorrequired (the report suggests it) once all five pass it. - Migrate S7 onto the engine and delete the fork. Replace
S7Driver's_subscriptionsdict +PollLoopAsync/PollOnceAsync/ComputeBackoffDelay/HandlePollFailure(:1162-1310) with aPollGroupEngineinstance (reader =ReadAsync, onChange = forwardOnDataChange, onError = degrade health, backoffCap = 30 s). This deletes PERF-3 (structural compare now inherited) and STAB-6-S7 (StopState drain now inherited) for free.
Implementation steps:
- Extend
PollGroupEnginector +PollLoopAsyncwith backoff; add unit tests forComputeBackoffDelayparity with S7's (move the existing S7 test asserts over). - Add an
onErrorhandler in Modbus/AbCip/AbLegacy/TwinCAT/FOCAS constructions that degrades health; assert via unit test that a throwing reader degrades health. - S7: construct the engine, delete the fork, keep the 100 ms floor and the
PollFailureHealthThreshold/Degraded-not-Faultednuance in the onError handler.
Tests:
- Engine unit: backoff schedule (0 failures = interval, N failures = capped exponential); onError invoked once per caught reader exception; structural array equality still suppresses no-change arrays; StopState drains before CTS dispose.
- S7 unit: array tag no longer fires every tick (regression for PERF-3); rapid
subscribe/unsubscribe no
ObjectDisposedException(STAB-6); sustained poll failure degrades health with backoff.
Effort: M (engine change is S; S7 migration + 5 onError wirings is M). Risk: Medium — S7 migration touches its subscription surface; the array-diff change alters publish cadence (intended). Gate behind the existing S7 subscribe tests + a new array-cadence test.
B2. Shared strict ReadEnum for equipment-tag parsers (CONV-2, UNDER-6)
Verification: CONFIRMED. An identical private generic ReadEnum<TEnum>(…, TEnum fallback) with silent fallback is copy-pasted six times — a typo'd dataType
quietly becomes the default type and reads/writes the wrong width with Good status:
ModbusEquipmentTagParser.cs:65-67, S7EquipmentTagParser.cs:59-61,
AbCipEquipmentTagParser.cs:86-88, AbLegacyEquipmentTagParser.cs:60-62,
TwinCATEquipmentTagParser.cs:47-49, FocasEquipmentTagParser.cs:43-45 (all
byte-identical bodies). Contrast the factory's fail-loud ParseEnum
(ModbusDriverFactoryExtensions.cs:191-201, lists valid names). Probe-parity is also
broken for Modbus: ModbusDriverProbe binds the runtime ModbusDriverOptions
(ModbusDriverProbe.cs:36) not the factory DTO, so timeoutMs is silently dropped.
Root cause: duplication-by-convention (theme #2); the three parse tiers (factory strict / probe permissive / parser silent) diverged.
Design:
- Hoist one strict helper into
Core.Abstractions, living besideEquipmentTagRefResolver(theme #6's home): e.g.TagConfigJson.ReadEnum<TEnum>(JsonElement o, string name, TEnum @default)with the rule present-but-invalid ⇒ throw (parse failure), absent ⇒ default. The parser wraps the throw so the resolver's_parseRefreturns null on a bad enum, and the driver surfacesBadNodeIdUnknowninstead of silently reading the wrong width. - Delete all six private copies; the parsers call the shared helper. Enums stay
per-driver (the helper is generic over
TEnum), so no coupling of driver vocab. - Probe parity (Modbus): make
ModbusDriverProbedeserialize the same factory DTO shape the factory uses (or share aTryParseConfigused by both), sotimeoutMsbinds identically. OpcUaClient already documents this rule (OpcUaClientDriverProbe.cs:22); Modbus should follow.
Writable/capability parity (UNDER-6, folds in here): Modbus/AbLegacy/FOCAS parsers
hard-code Writable: true (ModbusEquipmentTagParser.cs:54-57,
AbLegacyEquipmentTagParser.cs:52, FocasEquipmentTagParser.cs:35), so read-only
equipment tags are unauthorable on three drivers (AbCip honours a writable key,
AbCipEquipmentTagParser.cs:53-54). FOCAS additionally must force writable-false
(A7). Add a shared ReadBool(o, "writable", default) helper and honour it in all six
parsers; FOCAS overrides to false until PMC writes exist. FOCAS equipment tags also
bypass the FocasCapabilityMatrix gate authored tags get (FocasDriver.cs:243-247 vs
:115-119) — route equipment-tag resolution through the same matrix validate.
Implementation steps:
- New
Core.Abstractions/TagConfigJson.cswith strictReadEnum+ReadBool. - Replace the six
ReadEnumcopies; wrap parse failure so_parseRefreturns null. - Honour
writablein the three hard-coded parsers; FOCAS → false + capability gate. - Fix
ModbusDriverProbeto parse the factory DTO shape.
Tests:
- Shared conformance test (the report explicitly wants one): a parameterized suite
run per driver asserting (a) unknown
dataType⇒ resolve failure ⇒BadNodeIdUnknown(not silent default); (b)writable:falsehonoured; (c) absent enum ⇒ documented default. Put it in a shared test helper consumed by each driver's.Tests. - Modbus probe: a config with
timeoutMsround-trips the timeout.
Effort: M. Risk: Medium — changing silent-default to hard-fail is a behavior change: a deployment with a currently-silently-defaulted typo will start returning Bad. That is the correct outcome, but call it out in the migration notes and verify live on the rig (author a typo'd tag, confirm Bad rather than wrong-width Good).
B3. ResolveHost via the resolver — fix per-host breaker isolation (CONV-4, theme #6)
Verification: CONFIRMED. Every multi-device driver's ResolveHost consults only
_tagsByName then falls back to the first device: AbCipDriver.cs:485-487,
AbLegacyDriver.cs:498-500, TwinCATDriver.cs:587-591, FocasDriver.cs:1082-1087
(Modbus :125-131 same shape, but per-tag UnitId isn't in its equipment parser).
An equipment-tag reference (raw TagConfig JSON, resolved via _resolver, not in
_tagsByName) misses and returns the first device's host — so the per-host Polly
bulkhead/circuit-breaker key is wrong for equipment tags, and device B's outage can trip
device A's breaker. Since equipment tags are the primary post-Galaxy authoring model,
the IPerCallHostResolver is effectively inert for the flagship path.
Root cause: ResolveHost never learned about the equipment-tag path; the resolver
resolves defs but exposes nothing host-specific (EquipmentTagRefResolver is generic
over TDef and never inspects it — confirmed).
Design: route ResolveHost through EquipmentTagRefResolver.TryResolve, then
derive the host from the parsed def. The per-driver TDef records already carry the
host: AbCipTagDefinition.DeviceHostAddress, AbLegacyTagDefinition.DeviceHostAddress,
TwinCATTagDefinition.DeviceHostAddress, FocasTagDefinition.DeviceHostAddress (all
parsed by the equipment parsers). So:
- Each driver's
ResolveHost(ref)becomes:if (_resolver.TryResolve(ref, out var def)) return def.DeviceHostAddress; return <first-device-or-sentinel>. This covers both authored (_byName) and equipment (_parseRef) paths through one call and reuses the parse cache. - Optional shared sugar: add a
string? ResolveHost(string ref, Func<TDef,string> host)overload toEquipmentTagRefResolverso the pattern is one call. Keeps the host extraction driver-specific (the resolver stays generic) while giving theme #6 a single home.
Implementation steps: rewrite the four ResolveHost bodies to call
_resolver.TryResolve; add the resolver overload if desired. Modbus: note UnitId
isn't in the equipment parser — either add unitId to ModbusEquipmentTagParser
(UNDER-6) or document that Modbus per-tag host resolution is out of scope.
Tests: unit per driver — an equipment-tag ref for device B resolves to device B's host (today returns device A/first); assert the Polly breaker key differs per device.
Effort: S-M. Risk: Low — corrects a mis-key; verify the breaker-key wiring downstream consumes the corrected host.
B4. Shared reconnect/backoff primitive (STAB-8, supports A1/A2)
Verification: CONFIRMED. No reconnect backoff exists except the Modbus transport's
geometric backoff (ModbusTcpTransport.cs:179-208) and S7's poll-loop backoff. Dead
devices pay full reconnect cost per tick: AbCip full Tag create + Forward Open per tag
per tick (AbCipDriver.cs:891-897 evict-recreate), FOCAS full two-socket reconnect per
tick of each of three loops (FocasDriver.cs:1089-1120), TwinCAT connect-attempt per
call (:602-651).
Design: a small shared ConnectionBackoff primitive in Core.Abstractions
(capped exponential, the same ComputeBackoffDelay math B1 adds to the poll engine —
factor it out so both use it). Lazy-reconnect drivers gate their
EnsureConnected/evict-recreate behind a per-device throttle: record
_lastConnectAttemptUtc + a growing backoff, and short-circuit a reconnect attempt that
is inside the backoff window (return the current Bad/Degraded state instead of hammering).
Implementation steps:
- Extract
ComputeBackoffDelayintoCore.Abstractions/ConnectionBackoff.cs(shared by PollGroupEngine v2 and the lazy-reconnect drivers). - Add a per-device backoff gate to TwinCAT
EnsureConnectedAsync, FOCASEnsureConnectedAsync, and AbCip's evict-recreate path (skip the recreate if within the backoff window; reset on success).
Tests: unit — a device that fails to connect is not retried more often than the backoff schedule; success resets the schedule.
Effort: M. Risk: Low-Medium — must not delay recovery when the device comes back (reset promptly on the first success).
Part C — Remaining Mediums & Lows (batched)
All CONFIRMED. Grouped for efficient sweeps.
C1. One-liner conventions sweep (CONV-3, CONV-5)
- CONV-3 (Medium): S7/TwinCAT/AbCip/FOCAS
Registertake noILoggerFactory(S7DriverFactoryExtensions.cs:19,TwinCATDriverFactoryExtensions.cs:18,AbCipDriverFactoryExtensions.cs:21,FocasDriverFactoryExtensions.cs:36), soDriverFactoryBootstrap.Register(:100,102,106,107) can't pass one and all four log toNullLoggerin production — their poll-failure/prohibition/live-risk warnings are invisible. AbLegacy/Galaxy/Modbus/OpcUaClient already have the overload (:101,103,104,105). Fix: add theILoggerFactory? = nullparam + thread to the driver ctor in the four factories; update the four bootstrap call sites. Effort S, risk very low. (Also unblocks A1/A2/B1 logging.) - CONV-5 (Medium): four health-idioms (
Volatilevsvolatilevs plain field); AbLegacy no health-refresh on successful write (AbLegacyDriver.cs:344-346vs AbCip:725); AbLegacy silently clobbers duplicate tag names (:91) where AbCip fails fast (:249-256); TwinCAT declaresHealthyat init with zero wire contact (:115) and reads host state without the probe lock (:524-525). Fix: standardize onVolatile.Read/Write(orvolatilerecord ref) across the fleet; add write-success health refresh to AbLegacy; make AbLegacy fail-fast on duplicate names; make TwinCAT prove the connection beforeHealthy. Effort S-M, risk low.
C2. Per-protocol read batching (PERF-1, PERF-2) — the scalability lever
-
PERF-1 (High): five of seven drivers do one tag per round-trip. Priority order per the report: TwinCAT ADS sum-read (
TwinCATDriver.cs:203-248), S7 multi-var (ReadMultipleVarsAsync, unused — S7 loops per tag:443-484under one_gate), FOCAS PMC range reads (WireFocasClient.cs:296-334widens to one value width), AbLegacy file-element spans (N7:0..N7:2). Each is a separate protocol feature. -
PERF-2 (High): AbCip's whole-UDT batched read is half-built — the planner collapses N members via declaration-order layout, off by default (safe-but-unused,
AbCipDriverOptions.cs:72defaultfalse), while true member offsets are already fetched via the CIP Template Object (AbCipDriver.cs:151-182,AbCipTemplateCache) but consulted only by discovery, never the planner (AbCipUdtReadPlanner.cs:69). Highest-ROI batching win (design already in-tree): feedAbCipUdtShapetemplate offsets into the planner, retire the unsafe declaration-order mode. Effort M-L, risk Medium (correctness of offset mapping — test against the abcip fixture with a known UDT).Sequencing: do PERF-2 first (design exists), then TwinCAT sum-read + S7 multi-var. Each is independently shippable. Effort L overall.
C3. OpcUaClient performance + monolith (PERF-4, PERF-5, UNDER-9)
- PERF-4 (Medium): all traffic serializes on one
SemaphoreSlim(1,1)_gate(12WaitAsyncsites,OpcUaClientDriver.cs:78,626,713,836,1186,1377,1451,1653,1825). OPC UA sessions multiplex natively; a multi-second HistoryRead/re-discovery blocks every read/write. The gate is only needed for session-swap consistency, which_probeLock+ re-read-in-critical-section already provide. Fix: scope the gate to session-swap only; let reads/writes/history run concurrently on the session. Effort M, risk Medium — must preserve session-swap safety; test the reconnect race. - PERF-5 (Medium):
EnrichAndRegisterVariablesAsyncsendspending.Count*4ReadValueIds in one call (:1029-1046); a server enforcingMaxNodesPerReadreturnsBadTooManyOperationsand the blanket catch (:1049-1057) silently registers every var as Int32/ViewOnly/non-historized with no log/health. Fix: chunk client-side (respectMaxNodesPerRead/ a conservative cap); log + health-signal the fallback. Effort S, risk low. - UNDER-9 (Low): 2154-line monolith; extract the reconnect state machine for isolated
testability; set
Session.TransferSubscriptionsOnReconnectexplicitly (:1946-1949— currently unset, relies on SDK default);BuildCertificateIdentityloads only PKCS#12 (:483-484) while the option doc promises PEM (OpcUaClientDriverOptions.cs:68-75) — either implement PEM load or fix the doc. Effort M (extraction) / S (the two nits), risk low.
C4. Remaining stability/correctness Mediums (STAB-10, STAB-11, STAB-12)
- STAB-10 (Medium): AbLegacy never evicts failed tag handles on the data path
(
AbLegacyDriver.cs:246-259,269-274) though AbCip does (AbCipDriver.cs:891-897) and AbLegacy's own probe loop does (:456-461). Fix: evict-recreate on non-zero status/transport exception in the data path (port AbCip'sEvictRuntime). Pairs with A4/A6 as an AbLegacy/AbCip parity pass. Effort S, risk low. - STAB-11 (Medium): RMW locks don't cover direct parent writes (AbCip/AbLegacy/
TwinCAT) → lost update when a direct parent write interleaves an in-flight RMW; AbCip
RMW hard-codes DINT while
AbCipTagPathaccepts.Non any parent (AbCipTagPath.cs:17-19); TwinCAT RMW reads the parent as 4-byteUDInt(AdsTwinCATClient.cs:96-107), an unverified assumption that will failDeviceInvalidSizeon WORD/BYTE parents. Fix: make direct parent writes take the same per-parent/per-runtime lock (AbCip's is subsumed by A4's runtime lock); width-aware RMW for TwinCAT (read the parent as its actual declared width). Effort M, risk Medium (TwinCAT width needs real-hardware verification — flag under UNDER-7). - STAB-12 (Medium): FOCAS sync
Dispose()does a close-PDU exchange (SendPdu+ReadPdu) on aNetworkStreamwith noReadTimeout/token (FocasWireClient.cs:164-175) — a stalled CNC wedgesShutdownAsync. TwinCATConnectAsynccalls the blocking SDKConnectignoring token+timeout (AdsTwinCATClient.cs:73-80). Fix: bound the FOCAS close read with aReadTimeout; run TwinCATConnecton a worker with the timeout/token honoured (or set_client.Timeoutand wrap inTask.Run+WaitAsync(ct)). Effort S, risk low.
C5. Perf Lows (PERF-6, PERF-7)
- PERF-6 (Low): TwinCAT re-parses
TwinCATSymbolPathper call (TwinCATDriver.cs:220,284,468) — cache like S7's_parsedByName; Modbus freshDriverHealthper read (:285); AbCipBuildArray<T>boxes per element. None urgent. - PERF-7 (Low): deadband +
WriteOnChangeOnlyexist only in Modbus (ModbusDriver.cs:136-159); generalize into/besidePollGroupEngine(driver-agnostic publish suppression) so noisy analog signals on the other poll drivers don't publish every jitter. Natural PollGroupEngine-v2 add-on (B1). Effort M, risk low.
C6. Underdeveloped Mediums (UNDER-4, UNDER-5, UNDER-7, UNDER-8)
- UNDER-4 (Medium, S7):
S7TagDtohas noArrayCountandBuildTagnever sets it (S7DriverFactoryExtensions.cs:80-90,137-151) → driver-config array tags silently scalar (arrays only work as equipment tags,S7EquipmentTagParser.cs:73-87); array writes throwNotSupportedException(S7Driver.cs:1014-1017);UInt32surfaces lossily asInt32(:1149-1152). Fix: addArrayCountto the DTO +BuildTag; implement array writes; mapUInt32→UInt32/Int64. Effort M. - UNDER-5 (Medium, AbCip):
AcknowledgeAsyncfire-and-forgets write results (AbCipAlarmProjection.cs:149) → a failed ack is invisible to operator/health/Part 9 caller; withEnableControllerBrowseon, one unreachable PLC faults the entire multi-deviceDiscoverAsync(uncaughtawait foreach,AbCipDriver.cs:970-1037); browsed tags default writable/Operate (CipSymbolObjectDecoder.cs:91). Fix: surface ack outcomes to health + the caller; per-device try/catch in discovery so one dead PLC doesn't fault all; default browsed tags read-only. Effort M. - UNDER-7 (Medium): aggregate the honest "unverified against real hardware" blocks
into a "needs bench time" checklist — TwinCAT array-read shape
(
AdsTwinCATClient.cs:109-116), Flat-modeSubSymbolspopulation (:263-277), bit-RMW width (STAB-11); AbLegacy array-decode's five unproven layout assumptions (LibplctagLegacyTagRuntime.cs:64-88); FOCAScnc_getfigureid + servo-load scaling (FocasWireClient.cs:391-395,943-947). Deliverable: a doc checklist, not code. Effort S (doc). - UNDER-8 (Medium): test coverage is codec-heavy, failure-path-light. The concrete
gaps this plan closes with new tests: S7 reconnect (A1), TwinCAT replay (A2), AbCip
concurrency (A4), FOCAS cache concurrency (A5), Modbus desync (A3), shared
parser-strictness conformance (B2). Also:
Cli.Common.DriverCommandBasehas no direct tests — add a small suite.
C7. Lows — hygiene (CONV-6, CONV-7, CONV-8)
- CONV-6 (Low/Medium):
Driver.OpcUaClient.Contractsdrags the full OPC UA SDK becauseNamespaceMaplives there (OpcUaClient.Contracts.csproj:9OPCFoundation.NetStandard.Opc.Ua.Client); every options-DTO consumer transitively loads the SDK. This is the previously-deferred OpcUaClient.Contracts-002 finding — moveNamespaceMapinto the driver project (or a new.Namespaceproject), leaving Contracts POCO-only like the other six. Effort M (new project boundary), risk low. - CONV-7 (Low/Medium): CLI divergences —
ParseValue/ParseBoolcopy-pasted six times (*/Commands/WriteCommand.cs); optionValidate()absent in AbLegacy (AbLegacyCommandBase.cs—--timeout-ms 0reaches the driver);Timeoutinit-setter throws in AbCip (AbCipCommandBase.cs:43) but no-ops in five others; only TwinCAT exposesbrowsethough AbCip supports controller browse (AbCipCommandBase.cs:63hardcodesEnableControllerBrowse=false); OpcUaClient has no CLI. Fix: hoist value parsing + a standardValidate()intoCli.Common; add an OpcUaClient CLI or document the gap. Effort M, risk low. - CONV-8 (Low): dead/no-op knobs + stale docs —
DisableFC23no-op (ModbusDriverOptions.cs:83-89); AbCipConnectionSizeplumbed-never-applied (:97-103); AbLegacy class doc says read/write "ship in PRs 2 and 3" (AbLegacyDriver.cs:9-11); OpcUaClient header says "PR 66 ships the scaffold: IDriver only" (:12-14). No options class hasValidate(). Fix: delete/wire the dead knobs; refresh the stale headers; optionally add per-driver optionsValidate()(S7's init guards:314-385are the best-of-breed template). Effort S, risk very low.
Sequencing & effort roll-up
Aligns with the report's "Recommended remediation order" and OVERALL actions #3/#4.
| # | Item | Findings | Effort | Risk | Notes |
|---|---|---|---|---|---|
| 1 | S7 reconnect (IS7Plc seam + EnsureConnected) + reconnect test |
STAB-1, UNDER-8 | M | Med | Critical; template = TwinCAT/OpcUaClient |
| 2 | TwinCAT native-sub replay on client swap + probe recycle | STAB-2 | M | Med | Critical; fake-client unit tests are the only automated coverage |
| 3 | AbCip per-runtime op lock + FOCAS ConcurrentDictionary caches | STAB-4, STAB-5 | S each | Low | Silent-corruption class; port AbLegacy's GetRuntimeLock |
| 4 | Modbus timeout/framing ⇒ connection teardown | STAB-3 | S | Low-Med | Distinguish timeout vs caller-cancel |
| 5 | Shutdown/unsub race fixes | STAB-6 | S | Low | Deleted for S7 by item 8 |
| 6 | FOCAS writable-false; OpcUaClient dead gate + alarm-filter encoding | UNDER-1, UNDER-2, UNDER-3 | S each | Low | Kill false capabilities |
| 7 | Factory ILoggerFactory on 4 drivers |
CONV-3 | S | V.Low | Unblocks item 1/2/8 logging |
| 8 | PollGroupEngine v2: backoff + onError-health + migrate S7 off fork | CONV-1, STAB-8, STAB-9, PERF-3, STAB-6-S7 | M | Med | Single home for poll fixes |
| 9 | Shared strict ReadEnum/ReadBool + probe parity + writable parity |
CONV-2, UNDER-6 | M | Med | Behavior change: typo ⇒ Bad; live-verify |
| 10 | ResolveHost via resolver (per-host breaker isolation) |
CONV-4 | S-M | Low | Flagship equipment-tag path |
| 11 | Shared reconnect/backoff primitive for lazy drivers | STAB-8 | M | Low-Med | Extract ConnectionBackoff; don't delay recovery |
| 12 | Per-protocol batching: AbCip planner⇄template-cache, TwinCAT sum-read, S7 multi-var | PERF-1, PERF-2 | L | Med | Biggest scalability lever; AbCip design in-tree |
| 13 | OpcUaClient gate-scoping + chunk discovery + monolith split | PERF-4, PERF-5, UNDER-9 | M | Med | Preserve session-swap safety |
| 14 | Remaining mediums: evict-on-fail, RMW parent lock/width, blocking teardown | STAB-10, STAB-11, STAB-12 | M | Med | TwinCAT width needs bench time |
| 15 | Underdeveloped: S7 arrays/UInt32, AbCip acks/discovery, bench checklist | UNDER-4, UNDER-5, UNDER-7, UNDER-8 | M | Low-Med | |
| 16 | Lows: Contracts SDK split, CLI hoist, dead knobs/docs, perf lows | CONV-6/7/8, PERF-6/7 | M | Low |
Where shared primitives live: Core.Abstractions — PollGroupEngine (v2 with
backoff, item 8), ConnectionBackoff (item 11, shared with the engine),
TagConfigJson.ReadEnum/ReadBool (item 9, beside EquipmentTagRefResolver), and the
ResolveHost-via-resolver pattern (item 10). This is the concrete answer to themes #2
and #6: fleet fixes get one home instead of six copies.
Live-verify recipes (docker rig on 10.100.0.35): Modbus :5020 +
exception_injector for STAB-3 (item 4) and write-failure paths; S7 :1102
(Snap7ServerFixture, bounce via lmxopcua-fix down/up s7 s7_1500) for STAB-1 (item 1);
AbCip :44818 for STAB-4 concurrency + PERF-2 UDT (items 3, 12). TwinCAT has no docker
fixture (needs a real TC3 XAR) — the fake-ITwinCATClient unit tests are authoritative
for STAB-2/item 2.