diff --git a/archreview/plans/R2-09-driver-fleet-batch-plan.md b/archreview/plans/R2-09-driver-fleet-batch-plan.md index 2bfe4d8c..5b56c0f5 100644 --- a/archreview/plans/R2-09-driver-fleet-batch-plan.md +++ b/archreview/plans/R2-09-driver-fleet-batch-plan.md @@ -569,3 +569,19 @@ Rules: tasks are bite-sized (≤ ~5 min of implementation each — TDD steps spl **Overall effort:** ≈ 5.5–7 dev-days across 6 independently shippable PRs (B1/B2/B5/B6 parallelizable from day one; B4 unblocks after B3's first two tasks). **Explicit hand-offs:** `ConnectionBackoff` → **R2-01** (S7 `EnsureConnectedAsync` throttle + the STAB-14/15 wrapper/classification fixes; B3.6's fork deletion removes the poll-loop half of STAB-14's blast surface in whichever order the two plans land). Parser strictness / `ReadEnum` / remaining equipment-parser parity → **R2-11**. Carried and untouched here: STAB-10, STAB-11, full STAB-12 (FOCAS close-PDU + TwinCAT blocking `Connect`), CONV-3 (`ILoggerFactory` — worth an early ride-along in any of these PRs per the report, but owned elsewhere), PERF-1/2, CONV-5 idioms. + +--- + +## Execution deviations (R2-09) + +Executed on branch `r2/09-driver-fleet-batch` off master `1a698cbb` (all 29 tasks completed; none deferred-live). Deviations from the plan, each a best-reversible call: + +- **Single branch, no PRs/merges.** The executor owns one isolated branch and never pushes/PRs/merges (house rule). The plan's per-sub-batch PR boundaries are collapsed onto this one branch; every "merge PR N" verification task was satisfied by running that sub-batch's driver unit suite(s) green and committing a status marker. The branch is ready for the parent to slice into PRs or merge as it sees fit. +- **Pre-existing unrelated failure (not R2-09).** `Core.Abstractions.Tests.InterfaceIndependenceTests.AllPublicTypes_LiveInRootNamespace` fails at base `1a698cbb` — it flags the pre-existing `Historian/` sub-namespace types (`HistorizationCommitMode`, `IHistorianValueWriter`, …), none of which this plan touched. The one new public type added here, `ConnectionBackoff`, correctly lives in the root `ZB.MOM.WW.OtOpcUa.Core.Abstractions` namespace. Left as-is (out of scope). +- **B3.6 stale handle-prefix lock-in test.** `S7DiscoveryAndSubscribeTests.SubscribeAsync_returns_unique_handles…` asserted the retired fork's `s7-sub-` `DiagnosticId` prefix. The fork retirement moves S7 onto `PollGroupEngine`, whose handle is `poll-sub-`; the assertion was updated to `poll-sub-` (a stale lock-in, per the watch-for-stale-tests note). +- **B3.5 test 3 reframed.** `SustainedPollFailure_DegradesHealth_WithBackoff` was landed as `SustainedPollFailure_DegradesHealth` — S7's `ReadAsync` already degrades health on comm failure (independent of `onError`), so a sustained-failing poll degrades health on both fork and engine; the timing-sensitive "WithBackoff" cadence assertion is covered deterministically by the engine-level `Backoff_*` tests (B3.2). S7's new `HandlePollError` health wiring is exercised by the migrated poll path. +- **FOCAS `ResolveHost` test initializes the driver.** FOCAS resolves equipment refs via `ResolveEquipmentTagRef`, which validates against the `_devices` map populated at `InitializeAsync` (the other three multi-device drivers parse directly, no init needed). The FOCAS `ResolveHostTests` init the driver — matching how `ResolveHost` is always called at runtime (post-init). +- **FOCAS connect backoff is best-effort.** FOCAS's `EnsureConnectedAsync` is not gate-serialized (its read/write/probe/fixed-tree/recycle loops already race the client swap by design), so the per-device `ConnectionBackoff` there is consulted/mutated without a gate — atomic enough on 64-bit for its int/DateTime state, with a benign extra/skipped attempt the worst outcome of a race. Documented in the field's XML doc. (TwinCAT/AbCip consult it under their existing gate / probe discipline.) +- **B6.5 is a white-box reflection test.** `RecycleClient_HonorsCancellation` invokes the private `RecycleClientAsync(device, ct)` with a cancelled token (via reflection, with the device's `ConnectGate` held) and asserts a prompt `OperationCanceledException` rather than an uncancellable hang. A full behavioral probe-loop wedge is impractical: in the single-probe-loop structure the loop blocks in the *already-cancellable* `EnsureConnectedAsync`, not `RecycleClientAsync`; the blocking-SDK-`Connect` root that makes the real wedge reproducible is carried under STAB-12 (out of scope). The reflection test deterministically pins the `None`→`ct` fix. +- **Live gates not exercised (offline macOS).** The optional/recommended live gates (B1.3 modbus `exception_injector`, B3.7 s7 fixture bounce) were left offline-unit-verified — every task's core behavior is fully covered by the per-driver unit suites (the authoritative gate for TwinCAT/FOCAS/AbLegacy, which have no fixture at all). No task was blocked on a fixture, so none is marked `deferred-live`. +- **Warning hygiene.** The five per-driver `PollErrorHealthTests` guard `DriverHealth.LastError` (nullable) with an explicit `ShouldNotBeNull()` before `ShouldContain` to clear a CS8604 nullable-flow warning. diff --git a/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json b/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json index 6292c0d7..85f144c0 100644 --- a/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json +++ b/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json @@ -3,39 +3,231 @@ "lastUpdated": "2026-07-12", "notes": "Sub-batches B1/B2/B5/B6 are mutually independent PRs; B4 depends on B3 tasks B3.1-B3.2. Ordering inside a sub-batch is encoded via blockedBy. Concurrency tasks are high-risk (see plan Classification fields).", "tasks": [ - { "id": "B1.1", "subject": "B1: failing Modbus transport-desync tests (stall / wrong TxId / truncated header / caller-cancel pin) in ModbusTcpReconnectTests", "status": "pending", "blockedBy": [] }, - { "id": "B1.2", "subject": "B1: ModbusTransportDesyncException + teardown-before-propagate in SendOnceAsync (timeout-OCE + InvalidDataException) — STAB-3", "status": "pending", "blockedBy": ["B1.1"] }, - { "id": "B1.3", "subject": "B1: full Modbus suites green + optional exception_injector live gate + merge PR 1", "status": "pending", "blockedBy": ["B1.2"] }, - - { "id": "B2.1", "subject": "B2: failing AbCipRuntimeConcurrencyTests (reentrancy-guarded FakeAbCipTag hammer) — STAB-4 repro", "status": "pending", "blockedBy": [] }, - { "id": "B2.2", "subject": "B2: port _runtimeLocks + GetRuntimeLock + dispose onto AbCip DeviceState (AbLegacy pattern)", "status": "pending", "blockedBy": ["B2.1"] }, - { "id": "B2.3", "subject": "B2: wrap ReadSingleAsync/ReadGroupAsync/WriteAsync critical sections in the per-runtime lock; AbCip suite green", "status": "pending", "blockedBy": ["B2.2"] }, - { "id": "B2.4", "subject": "B2: failing FocasFixedTreeConcurrencyTests (writer-vs-reader stress on the four DeviceState caches) — STAB-5 repro", "status": "pending", "blockedBy": [] }, - { "id": "B2.5", "subject": "B2: convert the four FOCAS caches to ConcurrentDictionary; FOCAS suite green; merge PR 2", "status": "pending", "blockedBy": ["B2.3", "B2.4"] }, - - { "id": "B3.1", "subject": "B3: ConnectionBackoff primitive in Core.Abstractions (static ComputeDelay ported from S7 + instance attempt-throttle) + ConnectionBackoffTests — STAB-8 seam, hand-off to R2-01", "status": "pending", "blockedBy": [] }, - { "id": "B3.2", "subject": "B3: PollGroupEngine v2 — backoffCap + consecutiveFailures + caller-token-filtered OCE catches (STAB-14-class engine guard) + engine tests FAIL->PASS", "status": "pending", "blockedBy": ["B3.1"] }, - { "id": "B3.3", "subject": "B3: wire onError->health + 30s backoffCap into Modbus/AbCip/AbLegacy engine ctors (tests first per driver) — STAB-9", "status": "pending", "blockedBy": ["B3.2"] }, - { "id": "B3.4", "subject": "B3: wire onError->health + 30s backoffCap into TwinCAT/FOCAS engine ctors (tests first per driver) — STAB-9", "status": "pending", "blockedBy": ["B3.2"] }, - { "id": "B3.5", "subject": "B3: failing S7PollEngineMigrationTests (array-cadence PERF-3, rapid sub/unsub STAB-6, sustained-failure health+backoff)", "status": "pending", "blockedBy": ["B3.2"] }, - { "id": "B3.6", "subject": "B3: retire the S7 poll fork onto PollGroupEngine (delete PollLoopAsync/PollOnceAsync/HandlePollFailure/SubscriptionState; ComputeBackoffDelay -> ConnectionBackoff) — CONV-1; cross-ref R2-01", "status": "pending", "blockedBy": ["B3.3", "B3.4", "B3.5"] }, - { "id": "B3.7", "subject": "B3: engine + all five driver suites green; s7 fixture bounce live gate; merge PR 3", "status": "pending", "blockedBy": ["B3.6"] }, - - { "id": "B4.1", "subject": "B4: TwinCAT per-device connect-attempt backoff (data-path gate, probe bypass + instant reset) tests FAIL->PASS — STAB-8", "status": "pending", "blockedBy": ["B3.2"] }, - { "id": "B4.2", "subject": "B4: FOCAS per-device connect-attempt backoff across the three loops, tests FAIL->PASS — STAB-8", "status": "pending", "blockedBy": ["B3.2"] }, - { "id": "B4.3", "subject": "B4: AbCip evict-recreate (Forward Open) create-path throttle, cache-hit unaffected, tests FAIL->PASS — STAB-8", "status": "pending", "blockedBy": ["B3.2"] }, - { "id": "B4.4", "subject": "B4: S7 seam comment pointing R2-01 at ConnectionBackoff; suites green; merge PR 4", "status": "pending", "blockedBy": ["B4.1", "B4.2", "B4.3"] }, - - { "id": "B5.1", "subject": "B5: failing ResolveHost tests in AbCip/AbLegacy/TwinCAT/FOCAS (equipment ref for device B must key B; fallback regression pins) — CONV-4 repro", "status": "pending", "blockedBy": [] }, - { "id": "B5.2", "subject": "B5: rewrite the four multi-device ResolveHost bodies via EquipmentTagRefResolver + empty-host guard", "status": "pending", "blockedBy": ["B5.1"] }, - { "id": "B5.3", "subject": "B5: Modbus — optional unitId in ModbusEquipmentTagParser + ResolveHost via resolver (per-unit breaker key); scope-note vs R2-11", "status": "pending", "blockedBy": ["B5.1"] }, - { "id": "B5.4", "subject": "B5: all five driver suites green; merge PR 5", "status": "pending", "blockedBy": ["B5.2", "B5.3"] }, - - { "id": "B6.1", "subject": "B6: failing ReplayFailure_EmitsBadQuality_AndDegradesHealth in TwinCATReconnectReplayTests — STAB-16 repro", "status": "pending", "blockedBy": [] }, - { "id": "B6.2", "subject": "B6: NativeRegistration HasLiveHandle/MarkHandleDead/OnUnavailable + replay-catch Bad-quality emit + health degrade — STAB-16", "status": "pending", "blockedBy": ["B6.1"] }, - { "id": "B6.3", "subject": "B6: RetryDeadRegistrationsAsync on successful probe tick (under ConnectGate), tests FAIL->PASS — STAB-16", "status": "pending", "blockedBy": ["B6.2"] }, - { "id": "B6.4", "subject": "B6: post-SwapHandle ownership re-check retracts the fresh ADS notification when unsubscribe raced the replay, tests FAIL->PASS — STAB-17", "status": "pending", "blockedBy": ["B6.3"] }, - { "id": "B6.5", "subject": "B6: RecycleClientAsync takes the probe ct (no CancellationToken.None gate wait) + in-catch OCE wrap, tests FAIL->PASS — STAB-12 compounded part", "status": "pending", "blockedBy": ["B6.4"] }, - { "id": "B6.6", "subject": "B6: full TwinCAT suite green (fake-client authoritative, no TC3 fixture); merge PR 6", "status": "pending", "blockedBy": ["B6.5"] } + { + "id": "B1.1", + "subject": "B1: failing Modbus transport-desync tests (stall / wrong TxId / truncated header / caller-cancel pin) in ModbusTcpReconnectTests", + "status": "completed", + "blockedBy": [] + }, + { + "id": "B1.2", + "subject": "B1: ModbusTransportDesyncException + teardown-before-propagate in SendOnceAsync (timeout-OCE + InvalidDataException) \u2014 STAB-3", + "status": "completed", + "blockedBy": [ + "B1.1" + ] + }, + { + "id": "B1.3", + "subject": "B1: full Modbus suites green + optional exception_injector live gate + merge PR 1", + "status": "completed", + "blockedBy": [ + "B1.2" + ] + }, + { + "id": "B2.1", + "subject": "B2: failing AbCipRuntimeConcurrencyTests (reentrancy-guarded FakeAbCipTag hammer) \u2014 STAB-4 repro", + "status": "completed", + "blockedBy": [] + }, + { + "id": "B2.2", + "subject": "B2: port _runtimeLocks + GetRuntimeLock + dispose onto AbCip DeviceState (AbLegacy pattern)", + "status": "completed", + "blockedBy": [ + "B2.1" + ] + }, + { + "id": "B2.3", + "subject": "B2: wrap ReadSingleAsync/ReadGroupAsync/WriteAsync critical sections in the per-runtime lock; AbCip suite green", + "status": "completed", + "blockedBy": [ + "B2.2" + ] + }, + { + "id": "B2.4", + "subject": "B2: failing FocasFixedTreeConcurrencyTests (writer-vs-reader stress on the four DeviceState caches) \u2014 STAB-5 repro", + "status": "completed", + "blockedBy": [] + }, + { + "id": "B2.5", + "subject": "B2: convert the four FOCAS caches to ConcurrentDictionary; FOCAS suite green; merge PR 2", + "status": "completed", + "blockedBy": [ + "B2.3", + "B2.4" + ] + }, + { + "id": "B3.1", + "subject": "B3: ConnectionBackoff primitive in Core.Abstractions (static ComputeDelay ported from S7 + instance attempt-throttle) + ConnectionBackoffTests \u2014 STAB-8 seam, hand-off to R2-01", + "status": "completed", + "blockedBy": [] + }, + { + "id": "B3.2", + "subject": "B3: PollGroupEngine v2 \u2014 backoffCap + consecutiveFailures + caller-token-filtered OCE catches (STAB-14-class engine guard) + engine tests FAIL->PASS", + "status": "completed", + "blockedBy": [ + "B3.1" + ] + }, + { + "id": "B3.3", + "subject": "B3: wire onError->health + 30s backoffCap into Modbus/AbCip/AbLegacy engine ctors (tests first per driver) \u2014 STAB-9", + "status": "completed", + "blockedBy": [ + "B3.2" + ] + }, + { + "id": "B3.4", + "subject": "B3: wire onError->health + 30s backoffCap into TwinCAT/FOCAS engine ctors (tests first per driver) \u2014 STAB-9", + "status": "completed", + "blockedBy": [ + "B3.2" + ] + }, + { + "id": "B3.5", + "subject": "B3: failing S7PollEngineMigrationTests (array-cadence PERF-3, rapid sub/unsub STAB-6, sustained-failure health+backoff)", + "status": "completed", + "blockedBy": [ + "B3.2" + ] + }, + { + "id": "B3.6", + "subject": "B3: retire the S7 poll fork onto PollGroupEngine (delete PollLoopAsync/PollOnceAsync/HandlePollFailure/SubscriptionState; ComputeBackoffDelay -> ConnectionBackoff) \u2014 CONV-1; cross-ref R2-01", + "status": "completed", + "blockedBy": [ + "B3.3", + "B3.4", + "B3.5" + ] + }, + { + "id": "B3.7", + "subject": "B3: engine + all five driver suites green; s7 fixture bounce live gate; merge PR 3", + "status": "completed", + "blockedBy": [ + "B3.6" + ] + }, + { + "id": "B4.1", + "subject": "B4: TwinCAT per-device connect-attempt backoff (data-path gate, probe bypass + instant reset) tests FAIL->PASS \u2014 STAB-8", + "status": "completed", + "blockedBy": [ + "B3.2" + ] + }, + { + "id": "B4.2", + "subject": "B4: FOCAS per-device connect-attempt backoff across the three loops, tests FAIL->PASS \u2014 STAB-8", + "status": "completed", + "blockedBy": [ + "B3.2" + ] + }, + { + "id": "B4.3", + "subject": "B4: AbCip evict-recreate (Forward Open) create-path throttle, cache-hit unaffected, tests FAIL->PASS \u2014 STAB-8", + "status": "completed", + "blockedBy": [ + "B3.2" + ] + }, + { + "id": "B4.4", + "subject": "B4: S7 seam comment pointing R2-01 at ConnectionBackoff; suites green; merge PR 4", + "status": "completed", + "blockedBy": [ + "B4.1", + "B4.2", + "B4.3" + ] + }, + { + "id": "B5.1", + "subject": "B5: failing ResolveHost tests in AbCip/AbLegacy/TwinCAT/FOCAS (equipment ref for device B must key B; fallback regression pins) \u2014 CONV-4 repro", + "status": "completed", + "blockedBy": [] + }, + { + "id": "B5.2", + "subject": "B5: rewrite the four multi-device ResolveHost bodies via EquipmentTagRefResolver + empty-host guard", + "status": "completed", + "blockedBy": [ + "B5.1" + ] + }, + { + "id": "B5.3", + "subject": "B5: Modbus \u2014 optional unitId in ModbusEquipmentTagParser + ResolveHost via resolver (per-unit breaker key); scope-note vs R2-11", + "status": "completed", + "blockedBy": [ + "B5.1" + ] + }, + { + "id": "B5.4", + "subject": "B5: all five driver suites green; merge PR 5", + "status": "completed", + "blockedBy": [ + "B5.2", + "B5.3" + ] + }, + { + "id": "B6.1", + "subject": "B6: failing ReplayFailure_EmitsBadQuality_AndDegradesHealth in TwinCATReconnectReplayTests \u2014 STAB-16 repro", + "status": "completed", + "blockedBy": [] + }, + { + "id": "B6.2", + "subject": "B6: NativeRegistration HasLiveHandle/MarkHandleDead/OnUnavailable + replay-catch Bad-quality emit + health degrade \u2014 STAB-16", + "status": "completed", + "blockedBy": [ + "B6.1" + ] + }, + { + "id": "B6.3", + "subject": "B6: RetryDeadRegistrationsAsync on successful probe tick (under ConnectGate), tests FAIL->PASS \u2014 STAB-16", + "status": "completed", + "blockedBy": [ + "B6.2" + ] + }, + { + "id": "B6.4", + "subject": "B6: post-SwapHandle ownership re-check retracts the fresh ADS notification when unsubscribe raced the replay, tests FAIL->PASS \u2014 STAB-17", + "status": "completed", + "blockedBy": [ + "B6.3" + ] + }, + { + "id": "B6.5", + "subject": "B6: RecycleClientAsync takes the probe ct (no CancellationToken.None gate wait) + in-catch OCE wrap, tests FAIL->PASS \u2014 STAB-12 compounded part", + "status": "completed", + "blockedBy": [ + "B6.4" + ] + }, + { + "id": "B6.6", + "subject": "B6: full TwinCAT suite green (fake-client authoritative, no TC3 fixture); merge PR 6", + "status": "completed", + "blockedBy": [ + "B6.5" + ] + } ] -} +} \ No newline at end of file diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ConnectionBackoff.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ConnectionBackoff.cs new file mode 100644 index 00000000..95022c48 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ConnectionBackoff.cs @@ -0,0 +1,93 @@ +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +/// +/// Shared capped-exponential backoff for poll-based / lazy-reconnect drivers, extracted from +/// the S7 driver's bespoke poll fork (05/STAB-8, CONV-1). Provides two surfaces: +/// +/// — the pure schedule (base, 2×, 4×, … saturating at a +/// cap) used by PollGroupEngine to stretch its retry cadence under sustained +/// failure. +/// The instance API ( / / +/// ) — a per-device connect-attempt throttle so a dead device +/// is not hammered with a full reconnect on every data call / poll tick. +/// +/// +/// +/// +/// Not internally synchronized. The instance methods mutate plain fields; every +/// intended call site already serializes on its per-device gate (the driver's +/// ConnectGate / connect semaphore / probe lock), so adding lock-free interlocked +/// state would be needless complexity. Callers MUST invoke the instance under that gate. +/// +/// +/// is pure + thread-safe and may be called from anywhere. +/// +/// +public sealed class ConnectionBackoff +{ + private readonly TimeSpan _baseDelay; + private readonly TimeSpan _maxDelay; + private int _consecutiveFailures; + private DateTime _nextAttemptUtc = DateTime.MinValue; + + /// Initializes a new per-device attempt throttle. + /// The initial backoff window after the first failure. + /// The upper bound the exponential growth saturates at. + public ConnectionBackoff(TimeSpan baseDelay, TimeSpan maxDelay) + { + _baseDelay = baseDelay; + _maxDelay = maxDelay; + } + + /// + /// Capped exponential backoff schedule. ≤ 0 returns + /// ; each subsequent failure doubles the wait up to + /// . Computed in ticks with an overflow guard so a large failure + /// count saturates at the cap rather than wrapping negative. Ported verbatim from the S7 + /// poll fork's ComputeBackoffDelay. + /// + /// The base interval (returned when there are no failures). + /// The number of consecutive failures observed. + /// The maximum delay the schedule saturates at. + /// The computed backoff delay. + public static TimeSpan ComputeDelay(TimeSpan baseInterval, int consecutiveFailures, TimeSpan cap) + { + if (consecutiveFailures <= 0) return baseInterval; + // Cap the shift to avoid overflow — at 30 the result already saturates the cap for any + // reasonable base interval. + var shift = Math.Min(consecutiveFailures - 1, 30); + var ticks = baseInterval.Ticks << shift; + if (ticks <= 0 || ticks > cap.Ticks) return cap; + return TimeSpan.FromTicks(ticks); + } + + /// + /// Returns true when is at or past the end of the current + /// backoff window (i.e. a connect / create attempt is due). A fresh instance and a + /// just-reset instance always return true. + /// + /// The current UTC time. + /// true when an attempt is permitted. + public bool ShouldAttempt(DateTime nowUtc) => nowUtc >= _nextAttemptUtc; + + /// + /// Records a failed attempt: increments the consecutive-failure count and opens a backoff + /// window ending from . + /// + /// The current UTC time (the window is measured from here). + public void RecordFailure(DateTime nowUtc) + { + _consecutiveFailures++; + _nextAttemptUtc = nowUtc + ComputeDelay(_baseDelay, _consecutiveFailures, _maxDelay); + } + + /// + /// Records a successful attempt: resets the failure count and clears the window so the next + /// attempt is permitted immediately. Recovery is never delayed by a residual backoff window. + /// + public void RecordSuccess() + { + _consecutiveFailures = 0; + _nextAttemptUtc = DateTime.MinValue; + } +} diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs index deb33e9a..e0d29342 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs @@ -35,6 +35,7 @@ public sealed class PollGroupEngine : IAsyncDisposable private readonly Action _onChange; private readonly Action? _onError; private readonly TimeSpan _minInterval; + private readonly TimeSpan? _backoffCap; private readonly ConcurrentDictionary _subscriptions = new(); private long _nextId; @@ -52,11 +53,17 @@ public sealed class PollGroupEngine : IAsyncDisposable /// internal contract-violation throw) so the owning driver can route the failure to its /// health surface. Defensive: an onError handler that /// itself throws is silently absorbed so a buggy forwarder cannot crash the poll loop. + /// Optional cap enabling capped-exponential failure backoff + /// (05/STAB-8). When supplied, sustained reader failures stretch the retry cadence + /// (interval, 2×, 4×, … saturating at this cap) instead of hammering a dead device every + /// tick; a successful poll resets to the base interval. null (default) keeps the + /// byte-identical fixed-interval cadence for consumers that have not opted in. public PollGroupEngine( Func, CancellationToken, Task>> reader, Action onChange, TimeSpan? minInterval = null, - Action? onError = null) + Action? onError = null, + TimeSpan? backoffCap = null) { ArgumentNullException.ThrowIfNull(reader); ArgumentNullException.ThrowIfNull(onChange); @@ -64,6 +71,7 @@ public sealed class PollGroupEngine : IAsyncDisposable _onChange = onChange; _onError = onError; _minInterval = minInterval ?? DefaultMinInterval; + _backoffCap = backoffCap; } /// Register a new polled subscription and start its background loop. @@ -116,31 +124,54 @@ public sealed class PollGroupEngine : IAsyncDisposable private async Task PollLoopAsync(SubscriptionState state, CancellationToken ct) { + var consecutiveFailures = 0; + // Initial-data push: every subscribed tag fires once at subscribe time regardless of // whether it has changed, satisfying OPC UA Part 4 initial-value semantics. - try { await PollOnceAsync(state, forceRaise: true, ct).ConfigureAwait(false); } - catch (OperationCanceledException) { return; } + try + { + await PollOnceAsync(state, forceRaise: true, ct).ConfigureAwait(false); + consecutiveFailures = 0; + } + // Only a teardown OCE (the loop's OWN token) exits the loop. A reader that throws OCE with + // the loop token un-cancelled — a driver-internal timeout CTS (the STAB-14/STAB-15 class) — + // must NOT tear the loop down: it falls through to the generic handler, is reported, and the + // loop retries with backoff. This is the guard that makes the S7 fork migration safe. + catch (OperationCanceledException) when (ct.IsCancellationRequested) { return; } catch (Exception ex) when (!IsFatal(ex)) { // first-read error tolerated — loop continues; forward to driver health surface. + consecutiveFailures++; ReportError(ex); } while (!ct.IsCancellationRequested) { - try { await Task.Delay(state.Interval, ct).ConfigureAwait(false); } + // Fixed cadence by default; when a backoff cap is configured, sustained failures + // stretch the delay (capped exponential) and a success snaps it back to Interval. + var delay = _backoffCap is null + ? state.Interval + : ConnectionBackoff.ComputeDelay(state.Interval, consecutiveFailures, _backoffCap.Value); + try { await Task.Delay(delay, ct).ConfigureAwait(false); } catch (OperationCanceledException) { return; } // Defensive: the CTS may be disposed by Unsubscribe/DisposeAsync between the // cancellation check above and the Task.Delay touching the token. Treat that race // as a normal cancellation rather than a fatal exception. catch (ObjectDisposedException) { return; } - try { await PollOnceAsync(state, forceRaise: false, ct).ConfigureAwait(false); } - catch (OperationCanceledException) { return; } + try + { + await PollOnceAsync(state, forceRaise: false, ct).ConfigureAwait(false); + consecutiveFailures = 0; + } + // Only teardown (the loop token) exits; a reader OCE with ct un-cancelled backs off + // like any other failure instead of killing the loop (STAB-14 engine guard). + catch (OperationCanceledException) when (ct.IsCancellationRequested) { return; } catch (Exception ex) when (!IsFatal(ex)) { - // transient poll error — loop continues, driver health surface logs it - // via the supplied onError callback. + // transient poll error — loop continues (with backoff when configured); driver + // health surface logs it via the supplied onError callback. + consecutiveFailures++; ReportError(ex); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 9b6a72c8..ca7e045f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -133,10 +133,29 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, _poll = new PollGroupEngine( reader: ReadAsync, onChange: (handle, tagRef, snapshot) => - OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot))); + OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)), + onError: HandlePollError, + backoffCap: PollBackoffCap); _alarmProjection = new AbCipAlarmProjection(this, _options.AlarmPollInterval, _logger); } + /// Upper bound on the poll-loop failure backoff — adopts the S7-proven 30 s cap fleet-wide (05/STAB-8). + private static readonly TimeSpan PollBackoffCap = TimeSpan.FromSeconds(30); + + /// + /// 05/STAB-9 — routes a poll-loop reader failure to the driver health surface: logs it and + /// degrades to preserving LastSuccessfulRead. + /// Never downgrades a state (a stronger, config-level signal). + /// + /// The exception caught by the poll engine. + internal void HandlePollError(Exception ex) + { + _logger.LogWarning(ex, "AbCip poll reader failed. Driver={DriverInstanceId}", _driverInstanceId); + var current = _health; + if (current.State != DriverState.Faulted) + _health = new DriverHealth(DriverState.Degraded, current.LastSuccessfulRead, ex.Message); + } + /// /// Fetch + cache the shape of a Logix UDT by template instance id. First call reads /// the Template Object off the controller; subsequent calls for the same @@ -436,6 +455,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } await probeRuntime.ReadAsync(ct).ConfigureAwait(false); success = probeRuntime.GetStatus() == 0; + if (success) + state.Backoff.RecordSuccess(); // 05/STAB-8: device reachable — reset the data-path window } catch (OperationCanceledException) when (ct.IsCancellationRequested) { @@ -452,6 +473,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { probeRuntime?.Dispose(); } catch { } probeRuntime = null; state.ProbeInitialized = false; + state.Backoff.RecordFailure(DateTime.UtcNow); // 05/STAB-8: keep the data-path window open } TransitionDeviceState(state, success ? HostState.Running : HostState.Stopped); @@ -479,10 +501,19 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // ---- IPerCallHostResolver ---- - /// + /// + /// Resolve a read/write/subscribe reference to the device host that keys its per-host + /// resilience (bulkhead / circuit breaker). CONV-4: routes through + /// so an equipment-tag reference (raw TagConfig + /// JSON) resolves to its OWN authored device rather than the first-device fallback — else a + /// broken device B would trip device A's breaker. The resolver caches parses, so this adds + /// no per-call cost after first use. + /// + /// The tag reference (authored name or equipment-tag JSON). + /// The device host key. public string ResolveHost(string fullReference) { - if (_tagsByName.TryGetValue(fullReference, out var def)) + if (_resolver.TryResolve(fullReference, out var def) && !string.IsNullOrEmpty(def.DeviceHostAddress)) return def.DeviceHostAddress; return _options.Devices.FirstOrDefault()?.HostAddress ?? DriverInstanceId; } @@ -542,37 +573,49 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { var runtime = await EnsureTagRuntimeAsync(device, def, ct).ConfigureAwait(false); - await runtime.ReadAsync(ct).ConfigureAwait(false); - var status = runtime.GetStatus(); - if (status != 0) + // Serialise Read → GetStatus → Decode against the shared runtime — the cached libplctag + // Tag handle is not concurrency-safe and GetStatus returns the LAST op's status (05/STAB-4). + var opLock = device.GetRuntimeLock(def.Name); + await opLock.WaitAsync(ct).ConfigureAwait(false); + try { - // Evict the stale handle so the next call re-creates it. - // A non-zero status can mean the controller dropped the connection or the tag - // handle became permanently invalid (e.g. after a PLC download). Evicting - // mirrors the probe loop's recreate-on-failure behaviour. - EvictRuntime(device, def.Name); - results[fb.OriginalIndex] = new DataValueSnapshot(null, - AbCipStatusMapper.MapLibplctagStatus(status), null, now); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, - $"libplctag status {status} reading {reference}"); - _logger.LogWarning( - "AbCip read returned non-zero libplctag status {LibplctagStatus} for tag {Tag} on device {Device}; " + - "evicting cached runtime so next call re-creates it", - status, reference, def.DeviceHostAddress); - return; - } + await runtime.ReadAsync(ct).ConfigureAwait(false); - var tagPath = AbCipTagPath.TryParse(def.TagPath); - var bitIndex = tagPath?.BitIndex; - // Review I-1 — an array tag (the EXPLICIT IsArray flag) decodes the whole buffer into an - // element-typed CLR array (int[]/float[]/bool[]/string[]…), INCLUDING a 1-element array - // (ElementCount 1). Scalar tags keep the single-value path. - var value = IsArrayTag(def) - ? runtime.DecodeArray(def.DataType, Math.Max(1, def.ElementCount)) - : runtime.DecodeValue(def.DataType, bitIndex); - results[fb.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); - _health = new DriverHealth(DriverState.Healthy, now, null); + var status = runtime.GetStatus(); + if (status != 0) + { + // Evict the stale handle so the next call re-creates it. + // A non-zero status can mean the controller dropped the connection or the tag + // handle became permanently invalid (e.g. after a PLC download). Evicting + // mirrors the probe loop's recreate-on-failure behaviour. + EvictRuntime(device, def.Name); + results[fb.OriginalIndex] = new DataValueSnapshot(null, + AbCipStatusMapper.MapLibplctagStatus(status), null, now); + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, + $"libplctag status {status} reading {reference}"); + _logger.LogWarning( + "AbCip read returned non-zero libplctag status {LibplctagStatus} for tag {Tag} on device {Device}; " + + "evicting cached runtime so next call re-creates it", + status, reference, def.DeviceHostAddress); + return; + } + + var tagPath = AbCipTagPath.TryParse(def.TagPath); + var bitIndex = tagPath?.BitIndex; + // Review I-1 — an array tag (the EXPLICIT IsArray flag) decodes the whole buffer into an + // element-typed CLR array (int[]/float[]/bool[]/string[]…), INCLUDING a 1-element array + // (ElementCount 1). Scalar tags keep the single-value path. + var value = IsArrayTag(def) + ? runtime.DecodeArray(def.DataType, Math.Max(1, def.ElementCount)) + : runtime.DecodeValue(def.DataType, bitIndex); + results[fb.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); + _health = new DriverHealth(DriverState.Healthy, now, null); + } + finally + { + opLock.Release(); + } } catch (OperationCanceledException) { @@ -612,29 +655,41 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { var runtime = await EnsureTagRuntimeAsync(device, parent, ct).ConfigureAwait(false); - await runtime.ReadAsync(ct).ConfigureAwait(false); - var status = runtime.GetStatus(); - if (status != 0) + // Serialise the whole-UDT Read → GetStatus → per-member DecodeValueAt against the shared + // parent runtime — the members decode from that one handle's buffer (05/STAB-4). + var opLock = device.GetRuntimeLock(parent.Name); + await opLock.WaitAsync(ct).ConfigureAwait(false); + try { - EvictRuntime(device, parent.Name); - var mapped = AbCipStatusMapper.MapLibplctagStatus(status); - StampGroupStatus(group, results, now, mapped); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, - $"libplctag status {status} reading UDT {group.ParentName}"); - _logger.LogWarning( - "AbCip whole-UDT read returned non-zero libplctag status {LibplctagStatus} for parent {Parent} " + - "on device {Device}; {MemberCount} member values stamped with mapped status", - status, group.ParentName, parent.DeviceHostAddress, group.Members.Count); - return; - } + await runtime.ReadAsync(ct).ConfigureAwait(false); - foreach (var member in group.Members) - { - var value = runtime.DecodeValueAt(member.Definition.DataType, member.Offset, bitIndex: null); - results[member.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); + var status = runtime.GetStatus(); + if (status != 0) + { + EvictRuntime(device, parent.Name); + var mapped = AbCipStatusMapper.MapLibplctagStatus(status); + StampGroupStatus(group, results, now, mapped); + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, + $"libplctag status {status} reading UDT {group.ParentName}"); + _logger.LogWarning( + "AbCip whole-UDT read returned non-zero libplctag status {LibplctagStatus} for parent {Parent} " + + "on device {Device}; {MemberCount} member values stamped with mapped status", + status, group.ParentName, parent.DeviceHostAddress, group.Members.Count); + return; + } + + foreach (var member in group.Members) + { + var value = runtime.DecodeValueAt(member.Definition.DataType, member.Offset, bitIndex: null); + results[member.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); + } + _health = new DriverHealth(DriverState.Healthy, now, null); + } + finally + { + opLock.Release(); } - _health = new DriverHealth(DriverState.Healthy, now, null); } catch (OperationCanceledException) { @@ -706,10 +761,23 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } var runtime = await EnsureTagRuntimeAsync(device, def, cancellationToken).ConfigureAwait(false); - runtime.EncodeValue(def.DataType, parsedPath?.BitIndex, w.Value); - await runtime.WriteAsync(cancellationToken).ConfigureAwait(false); - var status = runtime.GetStatus(); + // Serialise Encode → Write → GetStatus against the shared runtime — the same cached + // Tag handle may be in use by a concurrent ReadAsync or poll loop (05/STAB-4). + var opLock = device.GetRuntimeLock(def.Name); + await opLock.WaitAsync(cancellationToken).ConfigureAwait(false); + int status; + try + { + runtime.EncodeValue(def.DataType, parsedPath?.BitIndex, w.Value); + await runtime.WriteAsync(cancellationToken).ConfigureAwait(false); + status = runtime.GetStatus(); + } + finally + { + opLock.Release(); + } + if (status != 0) { EvictRuntime(device, def.Name); @@ -844,6 +912,13 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, return device.ParentRuntimes[parentTagName]; } + /// + /// Thrown when a per-device window is still open (05/STAB-8) + /// — a fail-fast that costs no wire traffic. Read/write map it to BadCommunicationError + /// via their generic catch, exactly as a real Forward-Open failure. + /// + private sealed class ConnectionThrottledException(string message) : Exception(message); + /// /// Idempotently materialise the runtime handle for a tag definition. First call creates /// + initialises the libplctag Tag; subsequent calls reuse the cached handle for the @@ -858,6 +933,14 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, ?? throw new InvalidOperationException( $"AbCip tag '{def.Name}' has malformed TagPath '{def.TagPath}'."); + // 05/STAB-8: inside an open backoff window a data-path caller fails fast — no runtime + // create, no Forward-Open. The malformed-path config error above still surfaces (a permanent + // fault must never be masked by the window); only the expensive create+init is throttled. The + // probe loop bypasses this (its own runtime) and drives the failure/success recording. + if (!device.Backoff.ShouldAttempt(DateTime.UtcNow)) + throw new ConnectionThrottledException( + $"AbCip runtime create for '{def.Name}' on {device.Options.HostAddress} throttled — backoff window open after a recent Forward-Open failure."); + // Review I-1 — an array tag (the EXPLICIT IsArray flag, incl. a 1-element array) sets // libplctag's elem_count so the read pulls every element in one CIP transaction; the read // path then boxes them into a typed CLR array. Scalar tags pass count 1 + IsArray false. @@ -871,8 +954,10 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, catch { runtime.Dispose(); + device.Backoff.RecordFailure(DateTime.UtcNow); // 05/STAB-8: open the backoff window throw; } + device.Backoff.RecordSuccess(); // 05/STAB-8: Forward-Open succeeded — reset the window // Two concurrent callers can both miss the cache + both initialize a runtime; only the // first TryAdd wins. Dispose the loser so it doesn't leak a native tag handle. if (device.Runtimes.TryAdd(def.Name, runtime)) @@ -1198,6 +1283,14 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// Gets the lock object used for probe synchronization. public object ProbeLock { get; } = new(); + + /// + /// Per-device connect-attempt throttle (05/STAB-8). After a failed runtime create + /// (Forward-Open), data-path callers fail fast inside the backoff window instead of + /// re-creating + re-initializing a runtime per tag per tick; the probe loop bypasses it + /// (its own runtime) and drives failure/success so a recovered device resets the window. + /// + public ConnectionBackoff Backoff { get; } = new(TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(30)); /// Gets or sets the current host state of this device. public HostState HostState { get; set; } = HostState.Unknown; /// Gets or sets the UTC timestamp when the host state was last changed. @@ -1243,6 +1336,24 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, public SemaphoreSlim GetRmwLock(string parentTagName) => _rmwLocks.GetOrAdd(parentTagName, _ => new SemaphoreSlim(1, 1)); + private readonly System.Collections.Concurrent.ConcurrentDictionary _runtimeLocks = + new(StringComparer.OrdinalIgnoreCase); + + /// + /// Per-runtime operation lock (05/STAB-4). A libplctag Tag handle is not safe for + /// concurrent Read/GetStatus/DecodeValue (or Encode/Write/GetStatus) — the server read + /// path, every poll-group loop, and the alarm-projection loop all call + /// / against the + /// same cached runtime, and GetStatus returns the last operation's status + /// — so an unserialised overlap decodes a torn / cross-attributed value with Good status. + /// Callers hold this lock around the whole op sequence. Keyed by tag name, which is also + /// the dictionary key. Mirrors the field-proven AbLegacy pattern. + /// + /// The tag name (the key). + /// The per-runtime operation semaphore. + public SemaphoreSlim GetRuntimeLock(string tagName) => + _runtimeLocks.GetOrAdd(tagName, _ => new SemaphoreSlim(1, 1)); + /// /// Compute the effective for a /// tag on this device. Combines the per-device options @@ -1277,6 +1388,12 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, Runtimes.Clear(); foreach (var r in ParentRuntimes.Values) r.Dispose(); ParentRuntimes.Clear(); + // Dispose + clear the per-runtime/RMW gates so ReinitializeAsync cycles don't orphan + // their SemaphoreSlim instances (each leaks a wait handle once contended). + foreach (var sem in _runtimeLocks.Values) sem.Dispose(); + _runtimeLocks.Clear(); + foreach (var sem in _rmwLocks.Values) sem.Dispose(); + _rmwLocks.Clear(); } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs index 49115074..b5f21f9c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -65,7 +65,26 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover _poll = new PollGroupEngine( reader: ReadAsync, onChange: (handle, tagRef, snapshot) => - OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot))); + OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)), + onError: HandlePollError, + backoffCap: PollBackoffCap); + } + + /// Upper bound on the poll-loop failure backoff — adopts the S7-proven 30 s cap fleet-wide (05/STAB-8). + private static readonly TimeSpan PollBackoffCap = TimeSpan.FromSeconds(30); + + /// + /// 05/STAB-9 — routes a poll-loop reader failure to the driver health surface: logs it and + /// degrades to preserving LastSuccessfulRead. + /// Never downgrades a state (a stronger, config-level signal). + /// + /// The exception caught by the poll engine. + internal void HandlePollError(Exception ex) + { + _logger.LogWarning(ex, "AbLegacy poll reader failed. Driver={DriverInstanceId}", _driverInstanceId); + var current = _health; + if (current.State != DriverState.Faulted) + _health = new DriverHealth(DriverState.Degraded, current.LastSuccessfulRead, ex.Message); } /// @@ -492,10 +511,18 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover // ---- IPerCallHostResolver ---- - /// + /// + /// Resolve a reference to the device host that keys its per-host resilience + /// (bulkhead / circuit breaker). CONV-4: routes through + /// so an equipment-tag reference (raw TagConfig + /// JSON) resolves to its OWN authored device rather than the first-device fallback. The + /// resolver caches parses, so this adds no per-call cost after first use. + /// + /// The tag reference (authored name or equipment-tag JSON). + /// The device host key. public string ResolveHost(string fullReference) { - if (_tagsByName.TryGetValue(fullReference, out var def)) + if (_resolver.TryResolve(fullReference, out var def) && !string.IsNullOrEmpty(def.DeviceHostAddress)) return def.DeviceHostAddress; return _options.Devices.FirstOrDefault()?.HostAddress ?? DriverInstanceId; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index d2ba8789..9fca75f6 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -72,7 +72,27 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, _poll = new PollGroupEngine( reader: ReadAsync, onChange: (handle, tagRef, snapshot) => - OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot))); + OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)), + onError: HandlePollError, + backoffCap: PollBackoffCap); + } + + /// Upper bound on the poll-loop failure backoff — adopts the S7-proven 30 s cap fleet-wide (05/STAB-8). + private static readonly TimeSpan PollBackoffCap = TimeSpan.FromSeconds(30); + + /// + /// 05/STAB-9 — routes a poll-loop reader failure to the driver health surface: logs it and + /// degrades to preserving LastSuccessfulRead. + /// Never downgrades a state (a stronger, config-level signal). + /// + /// The exception caught by the poll engine. + internal void HandlePollError(Exception ex) + { + _logger.LogWarning(ex, "FOCAS poll reader failed. Driver={DriverInstanceId}", _driverInstanceId); + var current = Volatile.Read(ref _health); + if (current.State != DriverState.Faulted) + Volatile.Write(ref _health, + new DriverHealth(DriverState.Degraded, current.LastSuccessfulRead, ex.Message)); } /// @@ -609,7 +629,9 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, var success = false; try { - var client = await EnsureConnectedAsync(state, ct).ConfigureAwait(false); + // The probe bypasses the connect backoff — its own interval is the bounded recovery + // cadence, and a successful probe connect resets the window for the data + loop paths. + var client = await EnsureConnectedAsync(state, ct, bypassBackoff: true).ConfigureAwait(false); // Apply Probe.Timeout so a hung CNC socket gets cancelled at the configured // budget rather than blocking until the OS TCP timeout. // TimeSpan.Zero / negative means "no per-probe timeout" — fall back to the loop @@ -1092,18 +1114,42 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, // ---- IPerCallHostResolver ---- - /// + /// + /// Resolve a reference to the device host that keys its per-host resilience + /// (bulkhead / circuit breaker). CONV-4: routes through + /// so an equipment-tag reference (raw TagConfig + /// JSON) resolves to its OWN authored device rather than the first-device fallback. The + /// empty-host guard covers FOCAS's ?? "" default so an address-less equipment tag + /// falls back rather than keying an empty host. The resolver caches parses, so this adds no + /// per-call cost after first use. + /// + /// The tag reference (authored name or equipment-tag JSON). + /// The device host key. public string ResolveHost(string fullReference) { - if (_tagsByName.TryGetValue(fullReference, out var def)) + if (_resolver.TryResolve(fullReference, out var def) && !string.IsNullOrEmpty(def.DeviceHostAddress)) return def.DeviceHostAddress; return _options.Devices.FirstOrDefault()?.HostAddress ?? DriverInstanceId; } - private async Task EnsureConnectedAsync(DeviceState device, CancellationToken ct) + /// + /// Thrown when a per-device window is still open (05/STAB-8) + /// — a fail-fast that costs no wire traffic. Read/write map it to BadCommunicationError + /// via their generic catch, exactly as a real connect failure. + /// + private sealed class ConnectionThrottledException(string message) : Exception(message); + + private async Task EnsureConnectedAsync( + DeviceState device, CancellationToken ct, bool bypassBackoff = false) { if (device.Client is { IsConnected: true } c) return c; + // 05/STAB-8: inside an open backoff window a data-path / per-tick-loop caller fails fast — no + // socket create, no wire connect. The probe loop bypasses this and drives failure/success. + if (!bypassBackoff && !device.Backoff.ShouldAttempt(DateTime.UtcNow)) + throw new ConnectionThrottledException( + $"FOCAS connect to {device.Options.HostAddress} throttled — backoff window open after a recent connect failure."); + // Discard the existing client (if any) before creating a new one. A client that is // non-null but not connected may have been disposed by a HandleRecycle race or a prior // teardown — retrying ConnectAsync on a disposed FocasWireClient hits ThrowIfDisposed and @@ -1128,8 +1174,10 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, { device.Client.Dispose(); device.Client = null; + device.Backoff.RecordFailure(DateTime.UtcNow); // 05/STAB-8: open the backoff window throw; } + device.Backoff.RecordSuccess(); // 05/STAB-8: recovery resets the window immediately return device.Client; } @@ -1175,6 +1223,17 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// Gets or sets the FOCAS client instance. public IFocasClient? Client { get; set; } + /// + /// Per-device connect-attempt throttle (05/STAB-8). A dead CNC fails fast inside the + /// backoff window instead of paying a full two-socket reconnect on every tick of each of + /// the read / write / probe / fixed-tree / recycle loops; the probe loop bypasses it and a + /// successful connect resets it. FOCAS's EnsureConnectedAsync is not gate-serialized + /// (the loops already race the client swap by design), so this is best-effort — atomic + /// enough on 64-bit for the int/DateTime state, and a benign extra/skipped attempt is the + /// worst outcome of a race. + /// + public ConnectionBackoff Backoff { get; } = new(TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(30)); + /// Gets the lock object for probe synchronization. public object ProbeLock { get; } = new(); /// Gets or sets the current host connectivity state. @@ -1195,17 +1254,17 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// scale applied at ; integer fields /// (feed rate, spindle speed) widen to double on store. /// - public Dictionary LastFixedSnapshots { get; } = new(StringComparer.OrdinalIgnoreCase); + public ConcurrentDictionary LastFixedSnapshots { get; } = new(StringComparer.OrdinalIgnoreCase); /// Gets or sets the last program information snapshot. public FocasProgramInfo? LastProgramInfo { get; set; } /// Gets or sets the cached first-axis dynamic snapshot — feeds Program/Number, /MainNumber, /Sequence. public FocasDynamicSnapshot? LastProgramAxisRef { get; set; } /// Gets the last timer values by timer kind. - public Dictionary LastTimers { get; } = []; + public ConcurrentDictionary LastTimers { get; } = new(); /// Gets the last servo load percentages by servo name. - public Dictionary LastServoLoads { get; } = new(StringComparer.OrdinalIgnoreCase); + public ConcurrentDictionary LastServoLoads { get; } = new(StringComparer.OrdinalIgnoreCase); /// Gets the last spindle load percentages by spindle index. - public Dictionary LastSpindleLoads { get; } = []; + public ConcurrentDictionary LastSpindleLoads { get; } = new(); /// /// Gets or sets the per-axis position decimal-place figures fetched once at init via /// cnc_getfigure (parallel to the axis-name list; index = axis). An auto figure diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusEquipmentTagParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusEquipmentTagParser.cs index a9ce79a7..65b16e29 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusEquipmentTagParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusEquipmentTagParser.cs @@ -55,10 +55,14 @@ public static class ModbusEquipmentTagParser // "writable" defaults to true when absent (today's value) — makes read-only equipment tags // authorable (UNDER-6). Node-level authz remains the effective write gate. var writable = TagConfigJson.ReadWritable(root); + // CONV-4: optional per-tag unitId (0–255). Absent ⇒ null ⇒ the driver-level UnitId via + // ModbusDriver.ResolveUnitId, so multi-unit equipment tags key their own per-host breaker. + // Scope: unitId ONLY — the remaining parser-strictness gaps are R2-11's. + var unitId = ReadOptionalByte(root, "unitId"); def = new ModbusTagDefinition( Name: reference, Region: region, Address: (ushort)address, DataType: dataType, Writable: writable, ByteOrder: byteOrder, BitIndex: bitIndex, StringLength: stringLength, - ArrayCount: arrayCount); + ArrayCount: arrayCount, UnitId: unitId); return true; } catch (JsonException) { return false; } @@ -109,6 +113,10 @@ public static class ModbusEquipmentTagParser => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.Number && e.TryGetInt32(out var v) ? v : 0; + private static byte? ReadOptionalByte(JsonElement o, string name) + => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.Number + && e.TryGetInt32(out var v) && v is >= 0 and <= 255 ? (byte)v : null; + private static bool ReadBool(JsonElement o, string name) => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.True; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index 16fe1934..e522b3ef 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -118,13 +118,41 @@ public sealed class ModbusDriver { if (!ShouldPublish(tagRef, snapshot)) return; OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)); - }); + }, + onError: HandlePollError, + backoffCap: PollBackoffCap); } - /// + /// Upper bound on the poll-loop failure backoff — adopts the S7-proven 30 s cap fleet-wide (05/STAB-8). + private static readonly TimeSpan PollBackoffCap = TimeSpan.FromSeconds(30); + + /// + /// 05/STAB-9 — routes a poll-loop reader failure to the driver health surface: logs it and + /// degrades to preserving LastSuccessfulRead. + /// Never downgrades a state (a stronger, config-level signal). + /// + /// The exception caught by the poll engine. + internal void HandlePollError(Exception ex) + { + _logger.LogWarning(ex, "Modbus poll reader failed. Driver={DriverInstanceId}", _driverInstanceId); + var current = ReadHealth(); + if (current.State != DriverState.Faulted) + WriteHealth(new DriverHealth(DriverState.Degraded, current.LastSuccessfulRead, ex.Message)); + } + + /// + /// Resolve a reference to the per-slave host key (host:port/unitN) that keys its + /// per-host resilience (bulkhead / circuit breaker). CONV-4: routes through + /// so an equipment-tag reference (raw TagConfig + /// JSON) keys its OWN authored unit — via the tag's optional unitId or the driver + /// default — rather than the single-slave fallback. The resolver caches parses, so this adds + /// no per-call cost after first use. + /// + /// The tag reference (authored name or equipment-tag JSON). + /// The per-slave host key. public string ResolveHost(string fullReference) { - if (_tagsByName.TryGetValue(fullReference, out var tag)) + if (_resolver.TryResolve(fullReference, out var tag)) return BuildSlaveHostName(ResolveUnitId(tag)); // Unknown reference — fall back to driver-instance host (single-slave behaviour). return HostName; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTcpTransport.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTcpTransport.cs index 11268eb8..e7701296 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTcpTransport.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTcpTransport.cs @@ -207,6 +207,27 @@ public sealed class ModbusTcpTransport : IModbusTransport } } + /// + /// Executes exactly one Modbus transaction on the current socket. + /// + /// + /// + /// A per-op deadline (linked ) + /// and framing violations (TxId mismatch / truncated MBAP length) are treated as + /// connection-fatal: the socket may have a half-read or still-in-flight response, + /// so reusing it would read stale bytes and desynchronize the stream permanently. Such + /// failures tear the socket down here (before propagating) and surface as + /// — which, being an + /// , feeds 's existing single reconnect + /// retry with no extra plumbing. + /// + /// + /// The timeout-vs-caller-cancel distinction is load-bearing: a caller cancellation + /// (ct.IsCancellationRequested) is a legitimate shutdown and propagates as a plain + /// with no teardown — only the linked + /// per-op timeout (the caller token is NOT cancelled) is a desync. + /// + /// private async Task SendOnceAsync(byte unitId, byte[] pdu, CancellationToken ct) { if (_stream is null) throw new InvalidOperationException("Transport not connected"); @@ -225,28 +246,53 @@ public sealed class ModbusTcpTransport : IModbusTransport using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); cts.CancelAfter(_timeout); - await _stream.WriteAsync(adu.AsMemory(), cts.Token).ConfigureAwait(false); - await _stream.FlushAsync(cts.Token).ConfigureAwait(false); - - var header = new byte[7]; - await ReadExactlyAsync(_stream, header, cts.Token).ConfigureAwait(false); - var respTxId = (ushort)((header[0] << 8) | header[1]); - if (respTxId != txId) - throw new InvalidDataException($"Modbus TxId mismatch: expected {txId} got {respTxId}"); - var respLen = (ushort)((header[4] << 8) | header[5]); - if (respLen < 1) throw new InvalidDataException($"Modbus response length too small: {respLen}"); - var respPdu = new byte[respLen - 1]; - await ReadExactlyAsync(_stream, respPdu, cts.Token).ConfigureAwait(false); - - // Exception PDU: function code has high bit set. - if ((respPdu[0] & 0x80) != 0) + try { - var fc = (byte)(respPdu[0] & 0x7F); - var ex = respPdu[1]; - throw new ModbusException(fc, ex, $"Modbus exception fc={fc} code={ex}"); - } + await _stream.WriteAsync(adu.AsMemory(), cts.Token).ConfigureAwait(false); + await _stream.FlushAsync(cts.Token).ConfigureAwait(false); - return respPdu; + var header = new byte[7]; + await ReadExactlyAsync(_stream, header, cts.Token).ConfigureAwait(false); + var respTxId = (ushort)((header[0] << 8) | header[1]); + if (respTxId != txId) + throw new ModbusTransportDesyncException( + ModbusTransportDesyncException.DesyncReason.TxIdMismatch, + $"Modbus TxId mismatch: expected {txId} got {respTxId}"); + var respLen = (ushort)((header[4] << 8) | header[5]); + if (respLen < 1) + throw new ModbusTransportDesyncException( + ModbusTransportDesyncException.DesyncReason.TruncatedFrame, + $"Modbus response length too small: {respLen}"); + var respPdu = new byte[respLen - 1]; + await ReadExactlyAsync(_stream, respPdu, cts.Token).ConfigureAwait(false); + + // Exception PDU: function code has high bit set. This is a well-formed protocol-level + // error — the socket is still coherent, so it MUST propagate without teardown. + if ((respPdu[0] & 0x80) != 0) + { + var fc = (byte)(respPdu[0] & 0x7F); + var ex = respPdu[1]; + throw new ModbusException(fc, ex, $"Modbus exception fc={fc} code={ex}"); + } + + return respPdu; + } + catch (ModbusTransportDesyncException) + { + // Framing violation: the socket is desynchronized — never reuse it. + await TearDownAsync().ConfigureAwait(false); + throw; + } + catch (OperationCanceledException) when (!ct.IsCancellationRequested) + { + // Per-op timeout fired (NOT the caller). The response is unknown / may still land later + // and corrupt the next read — tear the socket down and normalize as a desync so the + // reconnect retry / status mapping engages. + await TearDownAsync().ConfigureAwait(false); + throw new ModbusTransportDesyncException( + ModbusTransportDesyncException.DesyncReason.Timeout, + $"Modbus per-op response timeout after {_timeout.TotalMilliseconds:0}ms"); + } } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTransportDesyncException.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTransportDesyncException.cs new file mode 100644 index 00000000..6f75e270 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusTransportDesyncException.cs @@ -0,0 +1,46 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +/// +/// Raised when a Modbus TCP transaction leaves the single-flight socket in an unknown / +/// desynchronized state — a per-op response timeout, or a framing violation (transaction-id +/// mismatch or truncated MBAP length). Unlike a Modbus exception PDU (a well-formed +/// protocol-level error where the socket is still coherent and the request must simply +/// propagate), a desync means bytes may still be in flight or half-read, so the socket is +/// unusable and MUST be torn down before this is thrown. +/// +/// +/// Derives from deliberately: the transport's existing +/// reconnect-and-retry classifier (IsSocketLevelFailure) already treats +/// as socket-fatal, so with auto-reconnect enabled the transport +/// transparently reconnects and resends once; with auto-reconnect disabled the (already torn +/// down) socket is never reused and this surfaces to the driver's status mapping as a +/// communication error. +/// +public sealed class ModbusTransportDesyncException : IOException +{ + /// The reason the socket was classified desynchronized. + public enum DesyncReason + { + /// The per-op deadline (linked CancelAfter) fired — the response never arrived in time. + Timeout, + + /// The response MBAP transaction id did not match the request's. + TxIdMismatch, + + /// The response MBAP length field was below the mandatory minimum (truncated frame). + TruncatedFrame, + } + + /// Gets the reason the socket was classified desynchronized. + public DesyncReason Reason { get; } + + /// Initializes a new instance of the class. + /// The desync reason. + /// A human-readable description. + /// The originating exception, if any. + public ModbusTransportDesyncException(DesyncReason reason, string message, Exception? inner = null) + : base(message, inner) + { + Reason = reason; + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs index c25eeedc..829d28d8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs @@ -53,11 +53,21 @@ public sealed class S7Driver _resolver = new EquipmentTagRefResolver( r => _tagsByName.TryGetValue(r, out var t) ? t : null, r => S7EquipmentTagParser.TryParse(r, out var d) ? d : null); + // CONV-1: the polling overlay runs on the shared PollGroupEngine (structural array diff + + // drain-before-dispose + capped-exponential failure backoff + caller-token-filtered OCE), + // retiring the bespoke S7 poll fork. minInterval 100 ms matches the S7 scan-mailbox floor; + // backoffCap 30 s matches the retired fork's PollBackoffCap. + _poll = new PollGroupEngine( + reader: ReadAsync, + onChange: (handle, tagRef, snapshot) => + OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)), + minInterval: TimeSpan.FromMilliseconds(100), + onError: HandlePollError, + backoffCap: TimeSpan.FromSeconds(30)); } // ---- ISubscribable + IHostConnectivityProbe state ---- - private readonly ConcurrentDictionary _subscriptions = new(); - private long _nextSubscriptionId; + private readonly PollGroupEngine _poll; private readonly object _probeLock = new(); private HostState _hostState = HostState.Unknown; private DateTime _hostStateChangedUtc = DateTime.UtcNow; @@ -269,38 +279,28 @@ public sealed class S7Driver // then await all of them with a bounded timeout BEFORE disposing the shared semaphore // and CTS objects. Without the drain, a loop iteration mid-_gate would call Release() // on (or WaitAsync against) a disposed semaphore. - var drain = new List(); + // Drain the polling overlay first — the engine cancels each loop and awaits it before + // disposing its CTS (drain-before-dispose, STAB-6-S7), so no poll iteration touches _gate + // after this returns. + await _poll.DisposeAsync().ConfigureAwait(false); var probeCts = _probeCts; var probeTask = _probeTask; try { probeCts?.Cancel(); } catch { } - if (probeTask is not null) drain.Add(probeTask); - - var subscriptions = _subscriptions.Values.ToArray(); - _subscriptions.Clear(); - foreach (var state in subscriptions) - { - try { state.Cts.Cancel(); } catch { } - drain.Add(state.PollTask); - } - - if (drain.Count > 0) + if (probeTask is not null) { try { - await Task.WhenAll(drain).WaitAsync(DrainTimeout, CancellationToken.None) - .ConfigureAwait(false); + await probeTask.WaitAsync(DrainTimeout, CancellationToken.None).ConfigureAwait(false); } - catch (TimeoutException) { /* a wedged loop — proceed; better than leaking the teardown */ } + catch (TimeoutException) { /* a wedged probe loop — proceed; better than leaking the teardown */ } catch { /* loop faults are already surfaced via health; teardown continues */ } } - // Loops have now observed cancellation and released _gate — safe to dispose the CTSs. + // The probe loop has now observed cancellation and released _gate — safe to dispose the CTS. probeCts?.Dispose(); _probeCts = null; _probeTask = null; - foreach (var state in subscriptions) - state.Cts.Dispose(); _initialized = false; _plcDead = false; @@ -1215,9 +1215,14 @@ public sealed class S7Driver /// While the PLC is down every data call and probe tick pays a full connect attempt /// bounded only by _options.Timeout (STAB-8). This method is the single /// choke-point all S7 reconnects flow through (already gate-serialized), so the - /// fleet-wide connect-attempt throttle planned in R2-09 plugs in here: a - /// last-failed-attempt timestamp checked at the top of the slow path. Deliberately NOT - /// implemented driver-locally — see + /// fleet-wide connect-attempt throttle plugs in here. R2-09 (sub-batch B3/B4) shipped the + /// shared primitive + /// (src/Core/…/Core.Abstractions/ConnectionBackoff.cs) — the S7 wiring is a + /// per-driver ConnectionBackoff field consulted with ShouldAttempt(now) at + /// the top of the slow path (fail fast + degrade inside the window) plus + /// RecordFailure/RecordSuccess around the connect, all under _gate + /// (which satisfies the primitive's caller-synchronized contract; TwinCAT/FOCAS/AbCip + /// already wire it this way). Deliberately NOT implemented driver-locally here — owned by /// archreview/plans/R2-01-s7-fault-hardening-plan.md Finding 3. /// /// @@ -1397,180 +1402,29 @@ public sealed class S7Driver /// public Task SubscribeAsync( IReadOnlyList fullReferences, TimeSpan publishingInterval, CancellationToken cancellationToken) - { - var id = Interlocked.Increment(ref _nextSubscriptionId); - var cts = new CancellationTokenSource(); - // Floor at 100 ms — S7 CPUs scan 2-10 ms but the comms mailbox is processed at most - // once per scan; sub-100 ms polling just queues wire-side with worse latency. - var interval = publishingInterval < TimeSpan.FromMilliseconds(100) - ? TimeSpan.FromMilliseconds(100) - : publishingInterval; - var handle = new S7SubscriptionHandle(id); - var state = new SubscriptionState(handle, [.. fullReferences], interval, cts); - _subscriptions[id] = state; - // Track the poll Task so ShutdownAsync can await it after cancelling — a poll - // iteration mid-_gate would otherwise race the semaphore's disposal. - state.PollTask = Task.Run(() => PollLoopAsync(state, cts.Token), CancellationToken.None); - return Task.FromResult(handle); - } + => Task.FromResult(_poll.Subscribe(fullReferences, publishingInterval)); /// public Task UnsubscribeAsync(ISubscriptionHandle handle, CancellationToken cancellationToken) { - if (handle is S7SubscriptionHandle h && _subscriptions.TryRemove(h.Id, out var state)) - { - state.Cts.Cancel(); - state.Cts.Dispose(); - } + _poll.Unsubscribe(handle); return Task.CompletedTask; } /// - /// Upper bound on the poll-loop backoff window. After enough consecutive failures the - /// loop waits this long between retries instead of , - /// so a subscription against a dropped / uninitialised driver doesn't spin. + /// 05/STAB-9 — routes a poll-loop reader failure (surfaced by the shared + /// 's onError sink) to the driver health surface: logs it and + /// degrades to preserving LastSuccessfulRead. Never + /// downgrades a state (e.g. PUT/GET-denied set by ReadAsync) + /// — Faulted is the stronger, permanent-config signal. Replaces the retired fork's + /// HandlePollFailure; the engine now owns the consecutive-failure count + backoff. /// - private static readonly TimeSpan PollBackoffCap = TimeSpan.FromSeconds(30); - - /// - /// Number of consecutive poll failures before the loop transitions the driver's - /// health to . One stray failure can be transient; - /// a sustained run indicates the operator should see it. Threshold of 1 because the - /// first failure already lives in the LastError surface. - /// - private const int PollFailureHealthThreshold = 1; - - private async Task PollLoopAsync(SubscriptionState state, CancellationToken ct) + /// The exception caught by the poll engine. + private void HandlePollError(Exception ex) { - var consecutiveFailures = 0; - - // Initial-data push per OPC UA Part 4 convention. - try - { - await PollOnceAsync(state, forceRaise: true, ct).ConfigureAwait(false); - consecutiveFailures = 0; - } - // Only a teardown OCE (the loop's own token) exits the loop; any other OCE (e.g. a - // connect-timeout that slipped past the wrapper filter) falls to the backoff path below - // so the loop lives (STAB-14 defense-in-depth). - catch (OperationCanceledException) when (ct.IsCancellationRequested) { return; } - catch (Exception ex) - { - // First-read error — polling continues; log so the operator has an event trail. - consecutiveFailures++; - HandlePollFailure(ex, consecutiveFailures, initial: true); - } - - while (!ct.IsCancellationRequested) - { - // Capped exponential backoff: Interval, 2×, 4×, ... up to PollBackoffCap. Healthy - // ticks reset consecutiveFailures back to 0 so the cadence snaps back to Interval. - var delay = ComputeBackoffDelay(state.Interval, consecutiveFailures); - try { await Task.Delay(delay, ct).ConfigureAwait(false); } - // Task.Delay can only observe ct-triggered OCE, so this filter is purely for uniformity. - catch (OperationCanceledException) when (ct.IsCancellationRequested) { return; } - - try - { - await PollOnceAsync(state, forceRaise: false, ct).ConfigureAwait(false); - consecutiveFailures = 0; - } - // Only teardown exits; any non-teardown OCE (e.g. a slipped-through connect timeout) - // backs off instead of killing the loop (STAB-14 defense-in-depth). - catch (OperationCanceledException) when (ct.IsCancellationRequested) { return; } - catch (Exception ex) - { - // Sustained polling error — loop continues with backoff; log + update health. - consecutiveFailures++; - HandlePollFailure(ex, consecutiveFailures, initial: false); - } - } - } - - /// - /// Logs the swallowed poll exception and, once - /// consecutive failures have accumulated, degrades the driver health so the failure - /// surfaces on the dashboard. The probe loop owns Running/Stopped - /// transitions for the host-connectivity surface, so we touch - /// rather than the probe state. - /// - private void HandlePollFailure(Exception ex, int consecutiveFailures, bool initial) - { - if (initial) - _logger.LogWarning(ex, "S7 poll initial-read failed. Driver={DriverInstanceId} ConsecutiveFailures={Count}", - _driverInstanceId, consecutiveFailures); - else - _logger.LogWarning(ex, "S7 poll tick failed. Driver={DriverInstanceId} ConsecutiveFailures={Count}", - _driverInstanceId, consecutiveFailures); - - if (consecutiveFailures >= PollFailureHealthThreshold) - { - // Don't downgrade a Faulted state (e.g. PUT/GET-denied set by ReadAsync) — Faulted - // is a stronger signal than Degraded and is reserved for permanent config faults. - if (_health.State != DriverState.Faulted) - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); - } - } - - /// - /// Capped exponential backoff. consecutiveFailures == 0 returns the configured - /// ; each subsequent failure doubles the wait up to - /// . Computed in ticks to avoid overflow at large counts. - /// - /// Base polling interval. - /// Number of consecutive failures. - /// The computed backoff delay. - internal static TimeSpan ComputeBackoffDelay(TimeSpan interval, int consecutiveFailures) - { - if (consecutiveFailures <= 0) return interval; - // Cap the shift to avoid overflow — at 30 the result already saturates PollBackoffCap - // for any reasonable Interval. - var shift = Math.Min(consecutiveFailures - 1, 30); - var ticks = interval.Ticks << shift; - if (ticks <= 0 || ticks > PollBackoffCap.Ticks) return PollBackoffCap; - return TimeSpan.FromTicks(ticks); - } - - private async Task PollOnceAsync(SubscriptionState state, bool forceRaise, CancellationToken ct) - { -#pragma warning disable OTOPCUA0001 // Driver-INTERNAL self-call: S7Driver's own poll loop reading via its own ReadAsync; CapabilityInvoker wraps the driver from the dispatch layer OUTWARD, so the driver's internal poll must NOT re-wrap. Intentional, not a dispatch gap. - var snapshots = await ReadAsync(state.TagReferences, ct).ConfigureAwait(false); -#pragma warning restore OTOPCUA0001 - for (var i = 0; i < state.TagReferences.Count; i++) - { - var tagRef = state.TagReferences[i]; - var current = snapshots[i]; - var lastSeen = state.LastValues.TryGetValue(tagRef, out var prev) ? prev : default; - - if (forceRaise || !Equals(lastSeen?.Value, current.Value) || lastSeen?.StatusCode != current.StatusCode) - { - state.LastValues[tagRef] = current; - OnDataChange?.Invoke(this, new DataChangeEventArgs(state.Handle, tagRef, current)); - } - } - } - - private sealed record SubscriptionState( - S7SubscriptionHandle Handle, - IReadOnlyList TagReferences, - TimeSpan Interval, - CancellationTokenSource Cts) - { - /// Gets the last known values for subscribed tags. - public ConcurrentDictionary LastValues { get; } - = new(StringComparer.OrdinalIgnoreCase); - - /// - /// Handle to this subscription's poll loop. Tracked so - /// can await it after cancelling. - /// - public Task PollTask { get; set; } = Task.CompletedTask; - } - - private sealed record S7SubscriptionHandle(long Id) : ISubscriptionHandle - { - /// - public string DiagnosticId => $"s7-sub-{Id}"; + _logger.LogWarning(ex, "S7 poll reader failed. Driver={DriverInstanceId}", _driverInstanceId); + if (_health.State != DriverState.Faulted) + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); } // ---- IHostConnectivityProbe ---- @@ -1693,34 +1547,22 @@ public sealed class S7Driver /// private void SynchronousTeardown() { - var drain = new List(); + // Drain the polling overlay (drain-before-dispose, STAB-6-S7). + try { _poll.DisposeAsync().AsTask().Wait(DrainTimeout); } + catch { /* timeouts/loop faults are tolerated — teardown continues */ } var probeCts = _probeCts; var probeTask = _probeTask; try { probeCts?.Cancel(); } catch { } - if (probeTask is not null) drain.Add(probeTask); - - var subscriptions = _subscriptions.Values.ToArray(); - _subscriptions.Clear(); - foreach (var state in subscriptions) + if (probeTask is not null) { - try { state.Cts.Cancel(); } catch { } - drain.Add(state.PollTask); - } - - if (drain.Count > 0) - { - try { Task.WhenAll(drain).Wait(DrainTimeout); } + try { probeTask.Wait(DrainTimeout); } catch { /* timeouts/loop faults are tolerated — teardown continues */ } } probeCts?.Dispose(); _probeCts = null; _probeTask = null; - foreach (var state in subscriptions) - { - try { state.Cts.Dispose(); } catch { } - } _initialized = false; _plcDead = false; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs index 4748f844..7420e6a7 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -69,7 +69,26 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery _poll = new PollGroupEngine( reader: ReadAsync, onChange: (handle, tagRef, snapshot) => - OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot))); + OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, tagRef, snapshot)), + onError: HandlePollError, + backoffCap: PollBackoffCap); + } + + /// Upper bound on the poll-loop failure backoff — adopts the S7-proven 30 s cap fleet-wide (05/STAB-8). + private static readonly TimeSpan PollBackoffCap = TimeSpan.FromSeconds(30); + + /// + /// 05/STAB-9 — routes a poll-loop reader failure to the driver health surface: logs it and + /// degrades to preserving LastSuccessfulRead. + /// Never downgrades a state (a stronger, config-level signal). + /// + /// The exception caught by the poll engine. + internal void HandlePollError(Exception ex) + { + _logger.LogWarning(ex, "TwinCAT poll reader failed. Driver={DriverInstanceId}", _driverInstanceId); + var current = _health; + if (current.State != DriverState.Faulted) + _health = new DriverHealth(DriverState.Degraded, current.LastSuccessfulRead, ex.Message); } /// @@ -474,9 +493,14 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery var reg = new NativeRegistration( Interlocked.Increment(ref _nextRegId), device, reference, symbolName, def.DataType, bitIndex, publishingInterval, - (_, value) => OnDataChange?.Invoke(this, + onChange: (_, value) => OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, reference, new DataValueSnapshot( - value, TwinCATStatusMapper.Good, DateTime.UtcNow, DateTime.UtcNow)))); + value, TwinCATStatusMapper.Good, DateTime.UtcNow, DateTime.UtcNow))), + // 05/STAB-16: when the handle goes dead (a failed replay) push a Bad snapshot so + // subscribers see the tag go Bad instead of a frozen-Good-stale value. + onUnavailable: () => OnDataChange?.Invoke(this, + new DataChangeEventArgs(handle, reference, new DataValueSnapshot( + null, TwinCATStatusMapper.BadCommunicationError, null, DateTime.UtcNow)))); await RegisterNotificationAsync(device, reg, cancellationToken).ConfigureAwait(false); registrations.Add(reg); @@ -574,8 +598,22 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// Value-change callback (closes over the owning subscription handle + reference). public Action OnChange { get; } + /// + /// Bad-quality callback (05/STAB-16). Invoked when this registration's handle goes dead + /// (a failed replay) so subscribers observe the tag go Bad instead of frozen-Good-stale — + /// the shape has no quality channel. Closes over the same + /// (handle, reference) pair as . + /// + public Action OnUnavailable { get; } + private ITwinCATNotificationHandle? _handle; + /// + /// True while this registration holds a live notification handle. False after a failed + /// replay marks it dead (05/STAB-16) — the probe-tick retry re-registers such handles. + /// + public bool HasLiveHandle => Volatile.Read(ref _handle) is not null; + /// Initializes a new instance of the class. /// Globally-unique registration id (registry key on the device). /// The device this notification is registered against. @@ -585,8 +623,10 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// Bit index for a BOOL-within-word symbol; null otherwise. /// Requested notification interval. /// Value-change callback (closes over the owning subscription handle + reference). + /// Bad-quality callback fired when the handle goes dead (05/STAB-16). public NativeRegistration(long id, DeviceState device, string reference, string symbolName, - TwinCATDataType dataType, int? bitIndex, TimeSpan interval, Action onChange) + TwinCATDataType dataType, int? bitIndex, TimeSpan interval, Action onChange, + Action onUnavailable) { Id = id; Device = device; @@ -596,6 +636,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery BitIndex = bitIndex; Interval = interval; OnChange = onChange; + OnUnavailable = onUnavailable; } /// Installs a freshly-registered handle and returns the previous one (null on first set). @@ -604,6 +645,15 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery public ITwinCATNotificationHandle? SwapHandle(ITwinCATNotificationHandle newHandle) => Interlocked.Exchange(ref _handle, newHandle); + /// + /// Atomically clears the live handle and returns the previous one (05/STAB-16). The caller + /// disposes the returned (already-dead) handle best-effort. After this + /// is false until a retry re-registers. + /// + /// The previous handle, or null. + public ITwinCATNotificationHandle? MarkHandleDead() => + Interlocked.Exchange(ref _handle, null); + /// Removes this registration from its device and disposes its live handle. public void Dispose() { @@ -628,7 +678,9 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery { // Probe-initiated connects honor TwinCATProbeOptions.Timeout — distinct from // the driver-wide _options.Timeout used by reads/writes. - var client = await EnsureConnectedAsync(state, ct, _options.Probe.Timeout) + // The probe bypasses the connect backoff — its own interval is the bounded recovery + // cadence, and a successful probe connect resets the window for the data path. + var client = await EnsureConnectedAsync(state, ct, _options.Probe.Timeout, bypassBackoff: true) .ConfigureAwait(false); success = await client.ProbeAsync(ct).ConfigureAwait(false); @@ -637,15 +689,21 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery // this the IsConnected fast-path would keep handing back the dead client forever // (the compounding bug — native pushes would never recover). if (!success) - await RecycleClientAsync(state).ConfigureAwait(false); + await RecycleClientAsync(state, ct).ConfigureAwait(false); + else + // 05/STAB-16: on a healthy tick, re-register any notifications whose replay + // failed (a transient AddNotification blip) so they recover without a client swap. + await RetryDeadRegistrationsAsync(state, ct).ConfigureAwait(false); } catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; } catch { // Probe threw — the connect-failure path clears the client only when the CONNECT // failed; a throw from ProbeAsync on an already-connected client would leave it in - // place, so force a recycle to guarantee the next tick rebuilds + replays. - await RecycleClientAsync(state).ConfigureAwait(false); + // place, so force a recycle to guarantee the next tick rebuilds + replays. A shutdown + // racing this recycle cancels the gate wait (STAB-12) — exit the loop cleanly. + try { await RecycleClientAsync(state, ct).ConfigureAwait(false); } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; } } TransitionDeviceState(state, success ? HostState.Running : HostState.Stopped); @@ -685,10 +743,20 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// public const string UnresolvedHostSentinel = ""; - /// + /// + /// Resolve a reference to the device host that keys its per-host resilience + /// (bulkhead / circuit breaker). CONV-4: routes through + /// so an equipment-tag reference (raw TagConfig + /// JSON) resolves to its OWN authored device rather than the first-device fallback. The + /// empty-host guard keeps an address-less equipment tag on the fallback rather than keying + /// the . The resolver caches parses, so this adds no + /// per-call cost after first use. + /// + /// The tag reference (authored name or equipment-tag JSON). + /// The device host key. public string ResolveHost(string fullReference) { - if (_tagsByName.TryGetValue(fullReference, out var def)) + if (_resolver.TryResolve(fullReference, out var def) && !string.IsNullOrEmpty(def.DeviceHostAddress)) return def.DeviceHostAddress; // First device's HostAddress when one exists; otherwise the unresolved sentinel — // intentionally NOT DriverInstanceId, which is a config-DB key, not a host address. @@ -704,7 +772,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// also what docs/v2/driver-specs.md recommends. /// private async Task EnsureConnectedAsync( - DeviceState device, CancellationToken ct, TimeSpan? timeoutOverride = null) + DeviceState device, CancellationToken ct, TimeSpan? timeoutOverride = null, bool bypassBackoff = false) { // Fast path — already connected, no gate needed. if (device.Client is { IsConnected: true } fast) return fast; @@ -712,7 +780,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery await device.ConnectGate.WaitAsync(ct).ConfigureAwait(false); try { - return await EnsureConnectedUnderGateAsync(device, ct, timeoutOverride).ConfigureAwait(false); + return await EnsureConnectedUnderGateAsync(device, ct, timeoutOverride, bypassBackoff).ConfigureAwait(false); } finally { @@ -720,6 +788,13 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery } } + /// + /// Thrown by the connect path when a per-device window is + /// still open (05/STAB-8) — a fail-fast that costs no wire traffic. Read/write map it to + /// BadCommunicationError via their generic catch, exactly as a real connect failure. + /// + private sealed class ConnectionThrottledException(string message) : Exception(message); + /// /// The connect-or-reconnect core, assuming the caller already holds /// . When it installs a NEW client (a genuine @@ -729,11 +804,18 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// through here under the same gate, so a replay can never race a register. /// private async Task EnsureConnectedUnderGateAsync( - DeviceState device, CancellationToken ct, TimeSpan? timeoutOverride) + DeviceState device, CancellationToken ct, TimeSpan? timeoutOverride, bool bypassBackoff = false) { // Re-check under the gate: another caller may have connected while we waited. if (device.Client is { IsConnected: true } c) return c; + // 05/STAB-8: inside an open backoff window a data-path caller fails fast — no client + // create, no wire connect. The probe loop bypasses this (its interval is the recovery + // cadence) and drives the failure/success recording. + if (!bypassBackoff && !device.Backoff.ShouldAttempt(DateTime.UtcNow)) + throw new ConnectionThrottledException( + $"TwinCAT connect to {device.Options.HostAddress} throttled — backoff window open after a recent connect failure."); + // Discard a stale (created-but-disconnected) client before making a fresh one. if (device.Client is { IsConnected: false } stale) { @@ -763,9 +845,11 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery _driverInstanceId, device.Options.HostAddress); client.OnSymbolVersionChanged -= HandleSymbolVersionChanged; client.Dispose(); + device.Backoff.RecordFailure(DateTime.UtcNow); // 05/STAB-8: open the backoff window throw; } device.Client = client; + device.Backoff.RecordSuccess(); // 05/STAB-8: recovery resets the window immediately // Re-register every native notification onto the fresh client. Native ADS is push, so // (unlike the poll fallback) it cannot self-heal by re-reading — without this the @@ -796,6 +880,15 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery _options.NotificationMaxDelayMs, reg.OnChange, ct).ConfigureAwait(false); var old = reg.SwapHandle(newHandle); try { old?.Dispose(); } catch { /* dead handle from the disposed client */ } + // 05/STAB-17: an UnsubscribeAsync (gate-free) can race this replay — if it removed the + // registration between the intents snapshot and this swap, the fresh handle is + // ownerless. Retract it immediately so the ADS notification isn't leaked live. + if (!device.NativeRegistrations.ContainsKey(reg.Id)) + { + var orphan = reg.MarkHandleDead(); + try { orphan?.Dispose(); } catch { /* best-effort */ } + continue; + } replayed++; } catch (Exception ex) @@ -804,6 +897,16 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery "TwinCAT driver '{DriverInstanceId}' failed to replay native notification " + "'{Symbol}' on device '{HostAddress}' after reconnect", _driverInstanceId, reg.SymbolName, device.Options.HostAddress); + + // 05/STAB-16: the intent has no live handle now — mark it dead so the probe-tick + // retry re-registers it, surface Bad quality to subscribers (the OnChange shape has + // no quality channel), and degrade health preserving the last successful read. + var dead = reg.MarkHandleDead(); + try { dead?.Dispose(); } catch { /* already-dead handle from the disposed client */ } + try { reg.OnUnavailable(); } catch { /* never let a subscriber callback abort recovery of the rest */ } + if (_health.State != DriverState.Faulted) + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, + $"native-notification replay failed for '{reg.SymbolName}' on {device.Options.HostAddress}"); } } @@ -813,6 +916,65 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery _driverInstanceId, replayed, intents.Length, device.Options.HostAddress); } + /// + /// 05/STAB-16: re-registers every dead (failed-replay) onto + /// the device's currently-connected client. Runs under + /// (shared with the replay so it can't race a register/unsubscribe). Called from the probe + /// loop after a successful probe — a bounded retry cadence (the probe interval) with no extra + /// timers. A registration that fails again just stays dead for the next tick. Recovery of the + /// value itself arrives via ADS's initial-notification push on register, exactly as the + /// initial subscribe relies on. + /// + private async Task RetryDeadRegistrationsAsync(DeviceState device, CancellationToken ct) + { + if (device.NativeRegistrations.IsEmpty) return; + + await device.ConnectGate.WaitAsync(ct).ConfigureAwait(false); + try + { + if (device.Client is not { IsConnected: true } client) return; + + var recovered = 0; + foreach (var reg in device.NativeRegistrations.Values) + { + if (reg.HasLiveHandle) continue; + try + { + var newHandle = await client.AddNotificationAsync( + reg.SymbolName, reg.DataType, reg.BitIndex, reg.Interval, + _options.NotificationMaxDelayMs, reg.OnChange, ct).ConfigureAwait(false); + reg.SwapHandle(newHandle); + // STAB-17 ownership re-check — see ReplayNativeRegistrationsAsync. + if (!device.NativeRegistrations.ContainsKey(reg.Id)) + { + var orphan = reg.MarkHandleDead(); + try { orphan?.Dispose(); } catch { /* best-effort */ } + continue; + } + recovered++; + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } + catch (Exception ex) + { + _logger.LogDebug(ex, + "TwinCAT driver '{DriverInstanceId}' probe-tick retry of dead notification " + + "'{Symbol}' on device '{HostAddress}' failed; will retry next tick", + _driverInstanceId, reg.SymbolName, device.Options.HostAddress); + } + } + + if (recovered > 0) + _logger.LogInformation( + "TwinCAT driver '{DriverInstanceId}' recovered {Recovered} dead native notification(s) " + + "on device '{HostAddress}' via probe-tick retry", + _driverInstanceId, recovered, device.Options.HostAddress); + } + finally + { + device.ConnectGate.Release(); + } + } + /// /// Disposes + nulls a device's client under so the /// next rebuilds it (and replays notifications). Called @@ -821,9 +983,12 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// liveness, so without this forced recycle a wire-dead-but-port-open client would keep /// being reused and native pushes would never recover. /// - private async Task RecycleClientAsync(DeviceState device) + private async Task RecycleClientAsync(DeviceState device, CancellationToken ct) { - await device.ConnectGate.WaitAsync(CancellationToken.None).ConfigureAwait(false); + // 05/STAB-12 (compounded part): honor the probe token so a connect wedged under the gate + // (TwinCAT's blocking SDK Connect — the un-fixed root, carried) can't hang the probe loop + // beyond cancellation. Was CancellationToken.None. + await device.ConnectGate.WaitAsync(ct).ConfigureAwait(false); try { if (device.Client is { } c) @@ -911,6 +1076,15 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// create-or-dispose for this device. public SemaphoreSlim ConnectGate { get; } = new(1, 1); + /// + /// Per-device connect-attempt throttle (05/STAB-8). A dead device fails fast inside the + /// backoff window instead of paying a full connect on every data call; the probe loop + /// bypasses it and a successful connect resets it. Consulted + mutated only under + /// , satisfying 's + /// caller-synchronized contract. + /// + public ConnectionBackoff Backoff { get; } = new(TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(30)); + /// /// Live native-notification registrations against this device, keyed by registration id. /// Holds the replayable intent (symbol / type / bit / interval / callback), not just diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/ConnectionBackoffTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/ConnectionBackoffTests.cs new file mode 100644 index 00000000..3af88cf0 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/ConnectionBackoffTests.cs @@ -0,0 +1,98 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests; + +/// +/// Covers — the shared capped-exponential backoff extracted +/// from the S7 poll fork (05/STAB-8; the seam plan R2-01 wires into the S7 connect throttle). +/// Two surfaces: the static schedule and the +/// per-device attempt-throttle instance. +/// +[Trait("Category", "Unit")] +public sealed class ConnectionBackoffTests +{ + private static readonly TimeSpan Base = TimeSpan.FromSeconds(1); + private static readonly TimeSpan Cap = TimeSpan.FromSeconds(30); + + /// Zero (or negative) consecutive failures returns the base interval unchanged. + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void ComputeDelay_NoFailures_ReturnsBaseInterval(int failures) + => ConnectionBackoff.ComputeDelay(Base, failures, Cap).ShouldBe(Base); + + /// The delay doubles per consecutive failure (1×, 2×, 4×, 8×) until it saturates the cap. + [Theory] + [InlineData(1, 1)] + [InlineData(2, 2)] + [InlineData(3, 4)] + [InlineData(4, 8)] + [InlineData(5, 16)] + public void ComputeDelay_DoublesPerFailure(int failures, int expectedSeconds) + => ConnectionBackoff.ComputeDelay(Base, failures, Cap) + .ShouldBe(TimeSpan.FromSeconds(expectedSeconds)); + + /// Growth saturates at the cap and never exceeds it, even at a large failure count (overflow guard). + [Theory] + [InlineData(6)] // 32s would exceed 30s cap + [InlineData(30)] + [InlineData(1000)] // shift saturates; ticks overflow guard returns cap + public void ComputeDelay_SaturatesAtCap(int failures) + => ConnectionBackoff.ComputeDelay(Base, failures, Cap).ShouldBe(Cap); + + /// A fresh throttle permits the first attempt immediately. + [Fact] + public void ShouldAttempt_FreshInstance_AllowsImmediately() + { + var backoff = new ConnectionBackoff(Base, Cap); + backoff.ShouldAttempt(DateTime.UtcNow).ShouldBeTrue(); + } + + /// After a failure the throttle blocks inside the backoff window and reopens once it elapses. + [Fact] + public void RecordFailure_BlocksWithinWindow_ReopensAfter() + { + var backoff = new ConnectionBackoff(Base, Cap); + var t0 = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc); + + backoff.RecordFailure(t0); // window = 1s (first failure) + backoff.ShouldAttempt(t0).ShouldBeFalse(); // still inside window + backoff.ShouldAttempt(t0.AddMilliseconds(500)).ShouldBeFalse(); + backoff.ShouldAttempt(t0.AddSeconds(1)).ShouldBeTrue(); // window elapsed + } + + /// Consecutive failures widen the window (1s then 2s). + [Fact] + public void RecordFailure_ConsecutiveFailures_WidenWindow() + { + var backoff = new ConnectionBackoff(Base, Cap); + var t0 = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc); + + backoff.RecordFailure(t0); // 1s + backoff.RecordFailure(t0.AddSeconds(1)); // 2nd failure → 2s window + backoff.ShouldAttempt(t0.AddSeconds(2)).ShouldBeFalse(); // within the 2s window + backoff.ShouldAttempt(t0.AddSeconds(3)).ShouldBeTrue(); + } + + /// Success resets immediately — recovery is never delayed by a residual window. + [Fact] + public void RecordSuccess_ResetsWindowImmediately() + { + var backoff = new ConnectionBackoff(Base, Cap); + var t0 = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc); + + backoff.RecordFailure(t0); + backoff.RecordFailure(t0); // deep in a widened window + backoff.ShouldAttempt(t0).ShouldBeFalse(); + + backoff.RecordSuccess(); + backoff.ShouldAttempt(t0).ShouldBeTrue(); // reset, no residual delay + + // And the schedule restarts from the base window on the next failure. + backoff.RecordFailure(t0); + backoff.ShouldAttempt(t0.AddMilliseconds(999)).ShouldBeFalse(); + backoff.ShouldAttempt(t0.AddSeconds(1)).ShouldBeTrue(); + } +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/PollGroupEngineTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/PollGroupEngineTests.cs index 5e6d0065..577cdbf4 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/PollGroupEngineTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/PollGroupEngineTests.cs @@ -461,6 +461,143 @@ public sealed class PollGroupEngineTests events.Count.ShouldBeGreaterThanOrEqualTo(1); } + // ---- PollGroupEngine v2: failure backoff + caller-token-filtered OCE (CONV-1 / STAB-8 / STAB-14) ---- + + /// + /// Sustained reader failures must stretch the poll cadence (capped exponential backoff) + /// instead of hammering a dead device at the base interval every tick. + /// + [Fact] + public async Task Backoff_SustainedReaderFailures_StretchesCadenceToCap() + { + var readCount = 0; + Task> Reader(IReadOnlyList refs, CancellationToken ct) + { + Interlocked.Increment(ref readCount); + throw new InvalidOperationException("dead device"); + } + + await using var engine = new PollGroupEngine( + Reader, + (_, _, _) => { }, + minInterval: TimeSpan.FromMilliseconds(20), + onError: _ => { }, + backoffCap: TimeSpan.FromMilliseconds(120)); + + var handle = engine.Subscribe(["X"], TimeSpan.FromMilliseconds(20)); + await Task.Delay(700, TestContext.Current.CancellationToken); + engine.Unsubscribe(handle); + + // Backoff schedule from a 20 ms base: 20,40,80,120,120,... → ≈8-9 reads in 700 ms. + // A no-backoff 20 ms loop would fire ≈35. Assert the cadence is clearly stretched. + readCount.ShouldBeGreaterThanOrEqualTo(2); + readCount.ShouldBeLessThan(18); + } + + /// A successful poll resets the failure count so the cadence snaps back to the base interval. + [Fact] + public async Task Backoff_SuccessResetsToInterval() + { + var readCount = 0; + var generation = 0; + Task> Reader(IReadOnlyList refs, CancellationToken ct) + { + // First few reads fail (winding the backoff up toward the cap); thereafter every read + // returns a fresh changing value so each successful poll raises a change event. + if (Interlocked.Increment(ref readCount) <= 3) + throw new InvalidOperationException("warming up failures"); + var gen = Interlocked.Increment(ref generation); + var now = DateTime.UtcNow; + return Task.FromResult>( + refs.Select(_ => new DataValueSnapshot(gen, 0u, now, now)).ToList()); + } + + var events = new ConcurrentQueue(); + await using var engine = new PollGroupEngine( + Reader, + (_, r, _) => events.Enqueue(r), + minInterval: TimeSpan.FromMilliseconds(20), + onError: _ => { }, + backoffCap: TimeSpan.FromMilliseconds(300)); + + var handle = engine.Subscribe(["X"], TimeSpan.FromMilliseconds(20)); + // After recovery the cadence must be the fast 20 ms base, not the wound-up cap — so many + // change events accumulate quickly. A non-resetting impl stays near the 300 ms cap (≈3). + await WaitForAsync(() => events.Count >= 8, TimeSpan.FromSeconds(3)); + engine.Unsubscribe(handle); + + events.Count.ShouldBeGreaterThanOrEqualTo(8); + } + + /// + /// STAB-14-class engine guard: a reader that throws + /// WITHOUT the caller/loop token being cancelled (a driver-internal timeout CTS) must be + /// treated as a normal failure — reported + retried — NOT as a loop teardown. Before the + /// fix the bare catch (OCE) { return; } killed the whole subscription on the first + /// such OCE. + /// + [Fact] + public async Task ReaderOperationCanceled_WithoutCallerCancellation_TreatedAsFailureNotTeardown() + { + var observed = new ConcurrentQueue(); + var events = new ConcurrentQueue(); + var readCount = 0; + Task> Reader(IReadOnlyList refs, CancellationToken ct) + { + // A driver-internal timeout OCE — the loop token (ct) is NOT cancelled. + if (Interlocked.Increment(ref readCount) <= 2) + throw new OperationCanceledException("driver-internal read timeout"); + var now = DateTime.UtcNow; + return Task.FromResult>( + refs.Select(_ => new DataValueSnapshot(1, 0u, now, now)).ToList()); + } + + await using var engine = new PollGroupEngine( + Reader, + (_, r, _) => events.Enqueue(r), + minInterval: TimeSpan.FromMilliseconds(30), + onError: ex => observed.Enqueue(ex), + backoffCap: TimeSpan.FromMilliseconds(120)); + + var handle = engine.Subscribe(["X"], TimeSpan.FromMilliseconds(30)); + // The loop must SURVIVE the reader OCEs and eventually deliver a change once the reader + // recovers — proving the OCE was not treated as teardown. + await WaitForAsync(() => events.Count >= 1, TimeSpan.FromSeconds(3)); + engine.Unsubscribe(handle); + + events.Count.ShouldBeGreaterThanOrEqualTo(1); + observed.Count.ShouldBeGreaterThanOrEqualTo(1); + observed.ShouldAllBe(e => e is OperationCanceledException); + } + + /// Caller (loop-token) cancellation still exits the loop promptly — well under the drain timeout. + [Fact] + public async Task CallerCancellation_StillExitsPromptly() + { + // Reader blocks until the loop token is cancelled (a well-behaved cancellable read). + Task> Reader(IReadOnlyList refs, CancellationToken ct) + => Task.Delay(Timeout.Infinite, ct).ContinueWith( + _ => (IReadOnlyList)new List(), ct); + + await using var engine = new PollGroupEngine( + Reader, + (_, _, _) => { }, + minInterval: TimeSpan.FromMilliseconds(30), + onError: _ => { }, + backoffCap: TimeSpan.FromMilliseconds(120)); + + var handle = engine.Subscribe(["X"], TimeSpan.FromMilliseconds(30)); + await Task.Delay(60, TestContext.Current.CancellationToken); // let the reader begin blocking + + var sw = System.Diagnostics.Stopwatch.StartNew(); + engine.Unsubscribe(handle).ShouldBeTrue(); + sw.Stop(); + + // Prompt exit: the caller-token OCE unwinds the loop immediately — nowhere near the 5 s + // StopState drain ceiling. + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(2)); + } + private sealed record DummyHandle : ISubscriptionHandle { /// Gets a diagnostic identifier for this handle. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipConnectBackoffTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipConnectBackoffTests.cs new file mode 100644 index 00000000..5c5ae0fd --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipConnectBackoffTests.cs @@ -0,0 +1,109 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// 05/STAB-8 — a dead AbCip device must not re-create + re-initialize (Forward-Open) a libplctag +/// runtime per tag per tick. The per-device gates the +/// runtime-create path; cache hits are unaffected; the probe loop (its own runtime) drives +/// failure/success so a recovered device resets the window. +/// +[Trait("Category", "Unit")] +public sealed class AbCipConnectBackoffTests +{ + private static AbCipDriverOptions Options(bool probeEnabled) => new() + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Tags = [new AbCipTagDefinition("Speed", "ab://10.0.0.5/1,0", "Motor1.Speed", AbCipDataType.DInt)], + Probe = new AbCipProbeOptions + { + Enabled = probeEnabled, + ProbeTagPath = "ProbeTag", + Interval = TimeSpan.FromMilliseconds(50), + Timeout = TimeSpan.FromMilliseconds(250), + }, + }; + + /// Rapid reads against a dead device attempt a single Forward-Open, then fail fast inside the backoff window. + [Fact] + public async Task DeadDevice_ConnectAttemptsFollowBackoffSchedule() + { + var creates = 0; + var factory = new FakeAbCipTagFactory + { + Customise = p => + { + Interlocked.Increment(ref creates); + return new FakeAbCipTag(p) { ThrowOnInitialize = true, Exception = new InvalidOperationException("down") }; + }, + }; + var drv = new AbCipDriver(Options(probeEnabled: false), "abcip-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + for (var i = 0; i < 6; i++) + await drv.ReadAsync(["Speed"], CancellationToken.None); + + creates.ShouldBe(1); // one Forward-Open attempt; the rest fail fast inside the window + } + + /// A cached healthy runtime is read without ever consulting the backoff or re-creating. + [Fact] + public async Task CacheHit_NeverConsultsBackoff() + { + var creates = 0; + var factory = new FakeAbCipTagFactory + { + Customise = p => + { + Interlocked.Increment(ref creates); + return new FakeAbCipTag(p) { Value = 5 }; + }, + }; + var drv = new AbCipDriver(Options(probeEnabled: false), "abcip-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + for (var i = 0; i < 5; i++) + { + var r = await drv.ReadAsync(["Speed"], CancellationToken.None); + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + } + + creates.ShouldBe(1); // created once + cached; cache hits never re-create + } + + /// The probe bypasses the backoff window and a successful probe resets it so the next read succeeds. + [Fact] + public async Task ProbeBypassesBackoff_AndSuccessResets() + { + var fail = true; + var factory = new FakeAbCipTagFactory + { + Customise = p => new FakeAbCipTag(p) + { + ThrowOnInitialize = fail, + Exception = new InvalidOperationException("down"), + Value = 7, + }, + }; + var drv = new AbCipDriver(Options(probeEnabled: true), "abcip-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var down = await drv.ReadAsync(["Speed"], CancellationToken.None); + down[0].StatusCode.ShouldBe(AbCipStatusMapper.BadCommunicationError); + + fail = false; // device recovers; the probe bypasses the window and resets it + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(3); + DataValueSnapshot? good = null; + while (DateTime.UtcNow < deadline) + { + var r = await drv.ReadAsync(["Speed"], CancellationToken.None); + if (r[0].StatusCode == AbCipStatusMapper.Good) { good = r[0]; break; } + await Task.Delay(25, CancellationToken.None); + } + + good.ShouldNotBeNull("probe-driven recovery must reset the backoff so reads recover"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPollErrorHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPollErrorHealthTests.cs new file mode 100644 index 00000000..3ba4a1af --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPollErrorHealthTests.cs @@ -0,0 +1,60 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// 05/STAB-9 — the poll engine's onError sink must be wired to the driver health +/// surface: a poll-loop reader failure degrades health while preserving +/// , and must never downgrade a pre-existing +/// . +/// +[Trait("Category", "Unit")] +public sealed class AbCipPollErrorHealthTests +{ + /// A poll reader failure degrades health, preserving the last successful read timestamp. + [Fact] + public async Task PollReaderFailure_DegradesHealth_PreservesLastSuccessfulRead() + { + var opts = new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Probe = new AbCipProbeOptions { Enabled = false }, + }; + var drv = new AbCipDriver(opts, "abcip-1", new FakeAbCipTagFactory()); + await drv.InitializeAsync("{}", CancellationToken.None); + + var before = drv.GetHealth(); + before.State.ShouldBe(DriverState.Healthy); + before.LastSuccessfulRead.ShouldNotBeNull(); + + drv.HandlePollError(new InvalidOperationException("poll boom")); + + var after = drv.GetHealth(); + after.State.ShouldBe(DriverState.Degraded); + after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + after.LastError.ShouldNotBeNull(); + after.LastError.ShouldContain("poll boom"); + } + + /// A poll reader failure never downgrades a pre-existing Faulted state. + [Fact] + public async Task PollReaderFailure_NeverDowngradesFaulted() + { + var opts = new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("not-a-valid-address")], + Probe = new AbCipProbeOptions { Enabled = false }, + }; + var drv = new AbCipDriver(opts, "abcip-1", new FakeAbCipTagFactory()); + + await Should.ThrowAsync(() => drv.InitializeAsync("{}", CancellationToken.None)); + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + + drv.HandlePollError(new InvalidOperationException("poll boom")); + + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipResolveHostTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipResolveHostTests.cs new file mode 100644 index 00000000..fd305b8f --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipResolveHostTests.cs @@ -0,0 +1,43 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// CONV-4 — keys per-host resilience (bulkhead / circuit +/// breaker). It must resolve an equipment-tag reference (raw TagConfig JSON) to its OWN device +/// host, not fall back to the first device — otherwise a broken device B trips device A's +/// breaker. Authored tags and unknown refs keep the current fallback. +/// +[Trait("Category", "Unit")] +public sealed class AbCipResolveHostTests +{ + private const string DeviceA = "ab://10.0.0.5/1,0"; + private const string DeviceB = "ab://10.0.0.6/1,0"; + + private static AbCipDriver NewDriver() => new( + new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(DeviceA), new AbCipDeviceOptions(DeviceB)], + Probe = new AbCipProbeOptions { Enabled = false }, + }, + "abcip-1", new FakeAbCipTagFactory()); + + /// An equipment-tag ref naming device B resolves to device B's host, not the first device. + [Fact] + public void ResolveHost_EquipmentTagRef_ReturnsItsOwnDeviceHost() + { + var drv = NewDriver(); + var json = $$"""{"deviceHostAddress":"{{DeviceB}}","tagPath":"Motor1.Speed","dataType":"DInt"}"""; + drv.ResolveHost(json).ShouldBe(DeviceB); + } + + /// An unresolvable ref keeps the current first-device fallback. + [Fact] + public void ResolveHost_UnknownRef_KeepsCurrentFallback() + { + var drv = NewDriver(); + drv.ResolveHost("totally-unknown").ShouldBe(DeviceA); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipRuntimeConcurrencyTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipRuntimeConcurrencyTests.cs new file mode 100644 index 00000000..118ad5a9 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipRuntimeConcurrencyTests.cs @@ -0,0 +1,123 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Regression coverage for 05/STAB-4 — a per-tag libplctag runtime (a single Tag +/// handle) is cached in DeviceState.Runtimes and shared between the server read path, +/// every poll-group loop, and the alarm-projection loop. A Tag handle is not safe for +/// concurrent Read/GetStatus/Decode (or Encode/Write/GetStatus), so the driver must serialise +/// the whole sequence per runtime — mirrors AbLegacyRuntimeConcurrencyTests. +/// +[Trait("Category", "Unit")] +public sealed class AbCipRuntimeConcurrencyTests +{ + /// + /// A fake runtime that records the maximum number of operations in flight against the + /// same handle. If the driver fails to serialise, two callers overlap inside the + /// Read → GetStatus → Decode window and exceeds 1. + /// + private sealed class OverlapDetectingFake : FakeAbCipTag + { + private int _inFlight; + + /// Gets the maximum number of concurrent operations detected on this handle. + public int MaxConcurrent { get; private set; } + + /// Initializes a new instance of the class. + /// The tag creation parameters. + public OverlapDetectingFake(AbCipTagCreateParams p) : base(p) { } + + /// + public override async Task ReadAsync(CancellationToken cancellationToken) + { + EnterOp(); + try + { + // Yield + small delay so an unserialised second caller is guaranteed to overlap. + await Task.Delay(15, cancellationToken).ConfigureAwait(false); + await base.ReadAsync(cancellationToken).ConfigureAwait(false); + } + finally { LeaveOp(); } + } + + /// + public override async Task WriteAsync(CancellationToken cancellationToken) + { + EnterOp(); + try + { + await Task.Delay(15, cancellationToken).ConfigureAwait(false); + await base.WriteAsync(cancellationToken).ConfigureAwait(false); + } + finally { LeaveOp(); } + } + + private void EnterOp() + { + var n = Interlocked.Increment(ref _inFlight); + lock (this) { if (n > MaxConcurrent) MaxConcurrent = n; } + } + + private void LeaveOp() => Interlocked.Decrement(ref _inFlight); + } + + private static AbCipDriver NewDriver(FakeAbCipTagFactory factory, params AbCipTagDefinition[] tags) + { + var opts = new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Tags = tags, + }; + return new AbCipDriver(opts, "drv-1", factory); + } + + /// Verifies that concurrent reads of the same tag are serialised against the shared runtime. + [Fact] + public async Task Concurrent_reads_of_same_tag_are_serialised_against_the_shared_runtime() + { + OverlapDetectingFake? shared = null; + var factory = new FakeAbCipTagFactory + { + Customise = p => shared = new OverlapDetectingFake(p) { Value = 7 }, + }; + var drv = NewDriver(factory, + new AbCipTagDefinition("X", "ab://10.0.0.5/1,0", "Motor1.Speed", AbCipDataType.DInt)); + await drv.InitializeAsync("{}", CancellationToken.None); + + // Eight callers race for the same tag — mimics the server read path + poll loop(s) + the + // alarm-projection loop hitting one cached runtime at once. + var reads = Enumerable.Range(0, 8) + .Select(_ => drv.ReadAsync(["X"], CancellationToken.None)) + .ToArray(); + await Task.WhenAll(reads); + + shared.ShouldNotBeNull(); + shared!.MaxConcurrent.ShouldBe(1, "operations on a shared libplctag Tag must not overlap"); + reads.ShouldAllBe(r => r.Result.Single().Value!.Equals(7)); + } + + /// Verifies that a concurrent read and write of the same tag do not overlap on the shared runtime. + [Fact] + public async Task Concurrent_read_and_write_of_same_tag_do_not_overlap() + { + OverlapDetectingFake? shared = null; + var factory = new FakeAbCipTagFactory + { + Customise = p => shared = new OverlapDetectingFake(p) { Value = 1 }, + }; + var drv = NewDriver(factory, + new AbCipTagDefinition("X", "ab://10.0.0.5/1,0", "Motor1.Speed", AbCipDataType.DInt)); + await drv.InitializeAsync("{}", CancellationToken.None); + + var readTask = drv.ReadAsync(["X"], CancellationToken.None); + var writeTask = drv.WriteAsync([new WriteRequest("X", 99)], CancellationToken.None); + await Task.WhenAll(readTask, writeTask); + + shared.ShouldNotBeNull(); + shared!.MaxConcurrent.ShouldBe(1); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyPollErrorHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyPollErrorHealthTests.cs new file mode 100644 index 00000000..df66ca48 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyPollErrorHealthTests.cs @@ -0,0 +1,60 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; + +/// +/// 05/STAB-9 — the poll engine's onError sink must be wired to the driver health +/// surface: a poll-loop reader failure degrades health while preserving +/// , and must never downgrade a pre-existing +/// . +/// +[Trait("Category", "Unit")] +public sealed class AbLegacyPollErrorHealthTests +{ + /// A poll reader failure degrades health, preserving the last successful read timestamp. + [Fact] + public async Task PollReaderFailure_DegradesHealth_PreservesLastSuccessfulRead() + { + var opts = new AbLegacyDriverOptions + { + Devices = [new AbLegacyDeviceOptions("ab://10.0.0.5/1,0")], + Probe = new AbLegacyProbeOptions { Enabled = false }, + }; + var drv = new AbLegacyDriver(opts, "ablegacy-1", new FakeAbLegacyTagFactory()); + await drv.InitializeAsync("{}", CancellationToken.None); + + var before = drv.GetHealth(); + before.State.ShouldBe(DriverState.Healthy); + before.LastSuccessfulRead.ShouldNotBeNull(); + + drv.HandlePollError(new InvalidOperationException("poll boom")); + + var after = drv.GetHealth(); + after.State.ShouldBe(DriverState.Degraded); + after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + after.LastError.ShouldNotBeNull(); + after.LastError.ShouldContain("poll boom"); + } + + /// A poll reader failure never downgrades a pre-existing Faulted state. + [Fact] + public async Task PollReaderFailure_NeverDowngradesFaulted() + { + var opts = new AbLegacyDriverOptions + { + Devices = [new AbLegacyDeviceOptions("not-a-valid-address")], + Probe = new AbLegacyProbeOptions { Enabled = false }, + }; + var drv = new AbLegacyDriver(opts, "ablegacy-1", new FakeAbLegacyTagFactory()); + + await Should.ThrowAsync(() => drv.InitializeAsync("{}", CancellationToken.None)); + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + + drv.HandlePollError(new InvalidOperationException("poll boom")); + + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyResolveHostTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyResolveHostTests.cs new file mode 100644 index 00000000..52109f9d --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyResolveHostTests.cs @@ -0,0 +1,42 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; + +/// +/// CONV-4 — must resolve an equipment-tag reference +/// (raw TagConfig JSON) to its OWN device host so per-host breaker keys are correct for +/// equipment tags, not the first-device fallback. +/// +[Trait("Category", "Unit")] +public sealed class AbLegacyResolveHostTests +{ + private const string DeviceA = "ab://10.0.0.5/1,0"; + private const string DeviceB = "ab://10.0.0.6/1,0"; + + private static AbLegacyDriver NewDriver() => new( + new AbLegacyDriverOptions + { + Devices = [new AbLegacyDeviceOptions(DeviceA), new AbLegacyDeviceOptions(DeviceB)], + Probe = new AbLegacyProbeOptions { Enabled = false }, + }, + "ablegacy-1", new FakeAbLegacyTagFactory()); + + /// An equipment-tag ref naming device B resolves to device B's host, not the first device. + [Fact] + public void ResolveHost_EquipmentTagRef_ReturnsItsOwnDeviceHost() + { + var drv = NewDriver(); + var json = $$"""{"deviceHostAddress":"{{DeviceB}}","address":"N7:0","dataType":"Int"}"""; + drv.ResolveHost(json).ShouldBe(DeviceB); + } + + /// An unresolvable ref keeps the current first-device fallback. + [Fact] + public void ResolveHost_UnknownRef_KeepsCurrentFallback() + { + var drv = NewDriver(); + drv.ResolveHost("totally-unknown").ShouldBe(DeviceA); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasConnectBackoffTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasConnectBackoffTests.cs new file mode 100644 index 00000000..318ecd1d --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasConnectBackoffTests.cs @@ -0,0 +1,86 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// 05/STAB-8 — a dead FOCAS CNC must not pay a full two-socket reconnect on every tick of each +/// read / probe / fixed-tree / recycle loop. The per-device +/// gates the connect path; the probe bypasses it and a successful connect resets it. No FOCAS +/// fixture exists — the fake-client suite is authoritative. +/// +[Trait("Category", "Unit")] +public sealed class FocasConnectBackoffTests +{ + private static FocasDriverOptions Options(bool probeEnabled) => new() + { + Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], + Tags = [new FocasTagDefinition("R", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte)], + Probe = new FocasProbeOptions + { + Enabled = probeEnabled, + Interval = TimeSpan.FromMilliseconds(50), + Timeout = TimeSpan.FromMilliseconds(250), + }, + // FixedTree / HandleRecycle / AlarmProjection default off — keep only the data path (+ probe). + }; + + /// Rapid data calls against a dead CNC attempt a single connect, then fail fast inside the backoff window. + [Fact] + public async Task DeadDevice_ConnectAttemptsFollowBackoffSchedule() + { + var factory = new FakeFocasClientFactory + { + Customise = () => new FakeFocasClient + { + ThrowOnConnect = true, + Exception = new InvalidOperationException("cnc down"), + }, + }; + var drv = new FocasDriver(Options(probeEnabled: false), "focas-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + for (var i = 0; i < 6; i++) + await drv.ReadAsync(["R"], CancellationToken.None); + + // One real connect attempt; the remaining five fail fast inside the window — no new socket + // is created (the fake factory records one client per attempted connect). + factory.Clients.Count.ShouldBe(1); + } + + /// The probe bypasses the backoff window and a successful connect resets it so the next data call succeeds immediately. + [Fact] + public async Task ProbeBypassesBackoff_AndSuccessResets() + { + var fail = true; + var factory = new FakeFocasClientFactory + { + Customise = () => new FakeFocasClient + { + ThrowOnConnect = fail, + Exception = new InvalidOperationException("cnc down"), + }, + }; + var drv = new FocasDriver(Options(probeEnabled: true), "focas-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var down = await drv.ReadAsync(["R"], CancellationToken.None); + down[0].StatusCode.ShouldBe(FocasStatusMapper.BadCommunicationError); + + // Device recovers. The probe bypasses the still-open window, reconnects, and resets it so + // the following data read succeeds with no residual delay. + fail = false; + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(3); + DataValueSnapshot? good = null; + while (DateTime.UtcNow < deadline) + { + var r = await drv.ReadAsync(["R"], CancellationToken.None); + if (r[0].StatusCode == FocasStatusMapper.Good) { good = r[0]; break; } + await Task.Delay(25, CancellationToken.None); + } + + good.ShouldNotBeNull("probe-driven reconnect must reset the backoff so data reads recover"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFixedTreeConcurrencyTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFixedTreeConcurrencyTests.cs new file mode 100644 index 00000000..d608ba59 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFixedTreeConcurrencyTests.cs @@ -0,0 +1,99 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// Regression coverage for 05/STAB-5 — the four per-device fixed-tree caches +/// (LastFixedSnapshots, LastTimers, LastServoLoads, LastSpindleLoads) +/// are written by the background fixed-tree loop while ReadAsync callers read them +/// concurrently. A plain +/// throws / corrupts when a resize races a read — undefined behaviour. They must be +/// . +/// +[Trait("Category", "Unit")] +public sealed class FocasFixedTreeConcurrencyTests +{ + private static FocasDriver.DeviceState NewDeviceState() => + new(new FocasHostAddress("10.0.0.5", FocasHostAddress.DefaultPort), + new FocasDeviceOptions("focas://10.0.0.5")); + + /// + /// Hammers concurrent writers (which force dictionary resizes) against concurrent readers on + /// all four caches. A plain Dictionary is not concurrency-safe: its structural-mutation + /// version check throws "operations that change non-concurrent collections must have exclusive + /// access to the collection", and its lookup collision guard throws "concurrent operations + /// are not supported" — deterministically under this load. A + /// ConcurrentDictionary never throws. + /// + /// + /// Uses several writers, not one, so the fault is deterministic rather than timing-dependent + /// UB: the fixed-tree caches are shared mutable state with zero synchronization, so ANY + /// concurrent access is the defect — a driver that survives this survives the real + /// writer-loop-vs-reader-callers race the finding cites. + /// + [Fact] + public async Task Concurrent_access_to_fixed_tree_caches_never_throws() + { + var state = NewDeviceState(); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); + var faults = new System.Collections.Concurrent.ConcurrentQueue(); + const int rounds = 40; + const int growth = 4_000; + + void Writer(int seed) + { + try + { + for (var round = 0; round < rounds && !cts.IsCancellationRequested; round++) + { + state.LastFixedSnapshots.Clear(); + state.LastServoLoads.Clear(); + state.LastSpindleLoads.Clear(); + for (var i = 0; i < growth; i++) + { + // Distinct keys force repeated dictionary growth (resize = the race window); + // the Clear() each round keeps the caches churning through resizes. + state.LastFixedSnapshots[$"Axes/A{seed}_{i}/AbsolutePosition"] = i; + state.LastServoLoads[$"Servo{seed}_{i}"] = i; + state.LastSpindleLoads[seed * growth + i] = i; + state.LastTimers[(FocasTimerKind)(i % 4)] = new FocasTimer((FocasTimerKind)(i % 4), i, i); + } + } + } + catch (Exception ex) { faults.Enqueue(ex); } + } + + void Reader() + { + try + { + while (!cts.IsCancellationRequested && faults.IsEmpty) + { + for (var i = 0; i < growth; i++) + { + state.LastFixedSnapshots.TryGetValue($"Axes/A0_{i}/AbsolutePosition", out _); + state.LastServoLoads.TryGetValue($"Servo0_{i}", out _); + state.LastSpindleLoads.TryGetValue(i, out _); + state.LastTimers.TryGetValue((FocasTimerKind)(i % 4), out _); + } + } + } + catch (Exception ex) { faults.Enqueue(ex); } + } + + var tasks = new List + { + Task.Run(() => Writer(0)), + Task.Run(() => Writer(1)), + Task.Run(() => Writer(2)), + Task.Run(Reader), + Task.Run(Reader), + }; + await Task.WhenAll(tasks); + + faults.ShouldBeEmpty( + faults.TryPeek(out var first) ? $"concurrent cache access threw: {first}" : ""); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPollErrorHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPollErrorHealthTests.cs new file mode 100644 index 00000000..dd0dc166 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPollErrorHealthTests.cs @@ -0,0 +1,59 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// 05/STAB-9 — the poll engine's onError sink must be wired to the driver health +/// surface: a poll-loop reader failure degrades health while preserving +/// , and must never downgrade a pre-existing +/// . +/// +[Trait("Category", "Unit")] +public sealed class FocasPollErrorHealthTests +{ + /// A poll reader failure degrades health, preserving the last successful read timestamp. + [Fact] + public async Task PollReaderFailure_DegradesHealth_PreservesLastSuccessfulRead() + { + var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], + Probe = new FocasProbeOptions { Enabled = false }, + }, "focas-1", new FakeFocasClientFactory()); + await drv.InitializeAsync("{}", CancellationToken.None); + + var before = drv.GetHealth(); + before.State.ShouldBe(DriverState.Healthy); + before.LastSuccessfulRead.ShouldNotBeNull(); + + drv.HandlePollError(new InvalidOperationException("poll boom")); + + var after = drv.GetHealth(); + after.State.ShouldBe(DriverState.Degraded); + after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + after.LastError.ShouldNotBeNull(); + after.LastError.ShouldContain("poll boom"); + } + + /// A poll reader failure never downgrades a pre-existing Faulted state. + [Fact] + public async Task PollReaderFailure_NeverDowngradesFaulted() + { + var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], + Tags = [new FocasTagDefinition("X", "focas://10.0.0.5:9999", "R100", FocasDataType.Byte)], + Probe = new FocasProbeOptions { Enabled = false }, + }, "focas-1", new FakeFocasClientFactory()); + + await Should.ThrowAsync(() => drv.InitializeAsync("{}", CancellationToken.None)); + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + + drv.HandlePollError(new InvalidOperationException("poll boom")); + + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasResolveHostTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasResolveHostTests.cs new file mode 100644 index 00000000..1fe06c2c --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasResolveHostTests.cs @@ -0,0 +1,60 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// CONV-4 — must resolve an equipment-tag reference (raw +/// TagConfig JSON) to its OWN device host so per-host breaker keys are correct for equipment +/// tags. An address-less equipment ref (FOCAS coalesces a missing deviceHostAddress to +/// "") must fall back rather than key an empty host. +/// +[Trait("Category", "Unit")] +public sealed class FocasResolveHostTests +{ + private const string DeviceA = "focas://10.0.0.5:8193"; + private const string DeviceB = "focas://10.0.0.6:8193"; + + // FOCAS's equipment-ref resolver validates against the initialized Devices map, so — unlike + // the other drivers — the driver must be initialized (as it always is when ResolveHost runs at + // runtime) before an equipment ref resolves. Probe off / no connect: init just parses devices. + private static async Task NewDriverAsync() + { + var drv = new FocasDriver( + new FocasDriverOptions + { + Devices = [new FocasDeviceOptions(DeviceA), new FocasDeviceOptions(DeviceB)], + Probe = new FocasProbeOptions { Enabled = false }, + }, + "focas-1", new FakeFocasClientFactory()); + await drv.InitializeAsync("{}", CancellationToken.None); + return drv; + } + + /// An equipment-tag ref naming device B resolves to device B's host, not the first device. + [Fact] + public async Task ResolveHost_EquipmentTagRef_ReturnsItsOwnDeviceHost() + { + var drv = await NewDriverAsync(); + var json = $$"""{"deviceHostAddress":"{{DeviceB}}","address":"R100","dataType":"Byte"}"""; + drv.ResolveHost(json).ShouldBe(DeviceB); + } + + /// An equipment ref with no deviceHostAddress falls back to the first device, not an empty host. + [Fact] + public async Task ResolveHost_EquipmentTagRef_WithoutDeviceHost_FallsBack() + { + var drv = await NewDriverAsync(); + var json = """{"address":"R100","dataType":"Byte"}"""; + drv.ResolveHost(json).ShouldBe(DeviceA); + } + + /// An unresolvable ref keeps the current first-device fallback. + [Fact] + public async Task ResolveHost_UnknownRef_KeepsCurrentFallback() + { + var drv = await NewDriverAsync(); + drv.ResolveHost("totally-unknown").ShouldBe(DeviceA); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusPollErrorHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusPollErrorHealthTests.cs new file mode 100644 index 00000000..2b6515ed --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusPollErrorHealthTests.cs @@ -0,0 +1,61 @@ +using System.Net.Sockets; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +/// +/// 05/STAB-9 — the poll engine's onError sink must be wired to the driver health +/// surface: a poll-loop reader failure degrades health while preserving +/// , and must never downgrade a pre-existing +/// (the S7 nuance ported fleet-wide). +/// +[Trait("Category", "Unit")] +public sealed class ModbusPollErrorHealthTests +{ + private sealed class NoopTransport : IModbusTransport + { + public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask; + public Task SendAsync(byte unitId, byte[] pdu, CancellationToken ct) + => Task.FromResult(new byte[] { 0x03, 0x00 }); + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + + /// A poll reader failure degrades health, preserving the last successful read timestamp. + [Fact] + public async Task PollReaderFailure_DegradesHealth_PreservesLastSuccessfulRead() + { + var opts = new ModbusDriverOptions { Host = "fake", Probe = new ModbusProbeOptions { Enabled = false } }; + var drv = new ModbusDriver(opts, "modbus-1", _ => new NoopTransport()); + await drv.InitializeAsync("{}", CancellationToken.None); + + var before = drv.GetHealth(); + before.State.ShouldBe(DriverState.Healthy); + before.LastSuccessfulRead.ShouldNotBeNull(); + + drv.HandlePollError(new InvalidOperationException("poll boom")); + + var after = drv.GetHealth(); + after.State.ShouldBe(DriverState.Degraded); + after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + after.LastError.ShouldNotBeNull(); + after.LastError.ShouldContain("poll boom"); + } + + /// A poll reader failure never downgrades a pre-existing Faulted state. + [Fact] + public async Task PollReaderFailure_NeverDowngradesFaulted() + { + var opts = new ModbusDriverOptions { Host = "fake", Probe = new ModbusProbeOptions { Enabled = false } }; + var drv = new ModbusDriver(opts, "modbus-1", _ => throw new SocketException()); + + // Init faults (the throwing factory) → Faulted, then rethrows. + await Should.ThrowAsync(() => drv.InitializeAsync("{}", CancellationToken.None)); + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + + drv.HandlePollError(new InvalidOperationException("poll boom")); + + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusTcpReconnectTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusTcpReconnectTests.cs index 47641eb4..a6c242f5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusTcpReconnectTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusTcpReconnectTests.cs @@ -29,6 +29,23 @@ public sealed class ModbusTcpReconnectTests private readonly CancellationTokenSource _stop = new(); private int _txCount; + // --- Scriptable STAB-3 desync behaviors (each fires for the first N transactions, then + // the server behaves normally). Decremented atomically because an old (torn-down) serve + // task and a fresh one can briefly coexist. --- + private int _stallRemaining; + private int _wrongTxRemaining; + private int _invalidLenRemaining; + private int _acceptedConnections; + + /// The first N responses are withheld entirely — the client blocks until its per-op timeout / caller cancellation fires. + public int StallFirstNResponses { set => _stallRemaining = value; } + /// The first N responses echo a corrupted MBAP transaction id (framing violation). + public int WrongTxIdFirstNResponses { set => _wrongTxRemaining = value; } + /// The first N responses claim an MBAP length field of 0 (truncated-length framing violation). + public int InvalidLengthFirstNResponses { set => _invalidLenRemaining = value; } + /// Gets the number of TCP connections the listener has accepted (a reconnect increments this). + public int AcceptedConnectionCount => Volatile.Read(ref _acceptedConnections); + /// Initializes a new instance and starts listening on a loopback port. public FlakeyModbusServer() { @@ -45,6 +62,7 @@ public sealed class ModbusTcpReconnectTests try { client = await _listener.AcceptTcpClientAsync(_stop.Token); } catch { return; } + Interlocked.Increment(ref _acceptedConnections); _ = Task.Run(() => ServeAsync(client!)); } } @@ -63,8 +81,27 @@ public sealed class ModbusTcpReconnectTests var pdu = new byte[len - 1]; if (!await ReadExactly(stream, pdu)) return; + // Stall: read the request but never answer it — simulate a hung / unreachable + // unit so the client's per-op CancelAfter (or caller token) fires. + if (Interlocked.Decrement(ref _stallRemaining) >= 0) + continue; + var fc = pdu[0]; var qty = (ushort)((pdu[3] << 8) | pdu[4]); + + // Truncated-length framing violation: a 7-byte header whose length field is 0 + // (< the mandatory 1 unit-id byte) — trips the transport's `respLen < 1` guard. + if (Interlocked.Decrement(ref _invalidLenRemaining) >= 0) + { + var bad = new byte[7]; + bad[0] = header[0]; bad[1] = header[1]; + bad[4] = 0; bad[5] = 0; // length field = 0 + bad[6] = header[6]; + await stream.WriteAsync(bad); + await stream.FlushAsync(); + continue; + } + var respPdu = new byte[2 + qty * 2]; respPdu[0] = fc; respPdu[1] = (byte)(qty * 2); @@ -73,6 +110,16 @@ public sealed class ModbusTcpReconnectTests var respLen = (ushort)(1 + respPdu.Length); var adu = new byte[7 + respPdu.Length]; adu[0] = header[0]; adu[1] = header[1]; + + // TxId-mismatch framing violation: emit an otherwise valid, full frame but with + // the transaction id flipped, so the transport reads the whole (unexpected) + // response and rejects it on the TxId guard. + if (Interlocked.Decrement(ref _wrongTxRemaining) >= 0) + { + adu[0] = (byte)~header[0]; + adu[1] = (byte)~header[1]; + } + adu[4] = (byte)(respLen >> 8); adu[5] = (byte)(respLen & 0xFF); adu[6] = header[6]; Buffer.BlockCopy(respPdu, 0, adu, 7, respPdu.Length); @@ -149,4 +196,82 @@ public sealed class ModbusTcpReconnectTests await Should.ThrowAsync(async () => await transport.SendAsync(unitId: 1, pdu, TestContext.Current.CancellationToken)); } + + // --- STAB-3: per-op timeout + framing violations must be classified connection-fatal so the + // socket is torn down instead of left desynchronized. With auto-reconnect on, the transport + // transparently reconnects + resends once and returns the CORRECT response (no stale read); + // the reconnect is observable as a second accepted connection on the server. Before the fix + // the timeout OCE / InvalidDataException are not socket-level, so no teardown happens and the + // SendAsync throws (and the socket stays desynchronized forever). --- + + private static readonly byte[] Fc03Qty1 = { 0x03, 0x00, 0x00, 0x00, 0x01 }; + + /// A per-op response timeout tears down the socket and (auto-reconnect) resends cleanly. + [Fact] + public async Task SendAsync_AfterResponseTimeout_TearsDownAndReconnects() + { + await using var server = new FlakeyModbusServer { StallFirstNResponses = 1 }; + await using var transport = new ModbusTcpTransport( + "127.0.0.1", server.Port, TimeSpan.FromMilliseconds(300), autoReconnect: true); + await transport.ConnectAsync(TestContext.Current.CancellationToken); + + var resp = await transport.SendAsync(unitId: 1, Fc03Qty1, TestContext.Current.CancellationToken); + + resp[0].ShouldBe((byte)0x03); // correct FC, not stale bytes + resp.Length.ShouldBe(2 + 1 * 2); // FC + byteCount + qty*2 data + server.AcceptedConnectionCount.ShouldBe(2); // torn down + reconnected + } + + /// A TxId-mismatch framing violation tears down the socket and (auto-reconnect) resends cleanly. + [Fact] + public async Task SendAsync_AfterTxIdMismatch_TearsDownSocket() + { + await using var server = new FlakeyModbusServer { WrongTxIdFirstNResponses = 1 }; + await using var transport = new ModbusTcpTransport( + "127.0.0.1", server.Port, TimeSpan.FromSeconds(2), autoReconnect: true); + await transport.ConnectAsync(TestContext.Current.CancellationToken); + + var resp = await transport.SendAsync(unitId: 1, Fc03Qty1, TestContext.Current.CancellationToken); + + resp[0].ShouldBe((byte)0x03); + server.AcceptedConnectionCount.ShouldBe(2); + } + + /// A truncated-length framing violation tears down the socket and (auto-reconnect) resends cleanly. + [Fact] + public async Task SendAsync_AfterTruncatedHeader_TearsDownSocket() + { + await using var server = new FlakeyModbusServer { InvalidLengthFirstNResponses = 1 }; + await using var transport = new ModbusTcpTransport( + "127.0.0.1", server.Port, TimeSpan.FromSeconds(2), autoReconnect: true); + await transport.ConnectAsync(TestContext.Current.CancellationToken); + + var resp = await transport.SendAsync(unitId: 1, Fc03Qty1, TestContext.Current.CancellationToken); + + resp[0].ShouldBe((byte)0x03); + server.AcceptedConnectionCount.ShouldBe(2); + } + + /// Caller cancellation propagates as OCE and must NOT tear down the socket (a legitimate shutdown is not a desync). + [Fact] + public async Task SendAsync_CallerCancellation_DoesNotTearDown() + { + await using var server = new FlakeyModbusServer { StallFirstNResponses = 1 }; + await using var transport = new ModbusTcpTransport( + "127.0.0.1", server.Port, TimeSpan.FromSeconds(5), autoReconnect: true); + await transport.ConnectAsync(TestContext.Current.CancellationToken); + + using var cts = new CancellationTokenSource(); + var pending = transport.SendAsync(unitId: 1, Fc03Qty1, cts.Token); + await Task.Delay(150, TestContext.Current.CancellationToken); // let the request land + block on the response read + await cts.CancelAsync(); + + await Should.ThrowAsync(async () => await pending); + + // The socket must still be usable — a subsequent transaction succeeds on the SAME connection + // (no reconnect), proving caller-cancel did not trip the desync teardown. + var resp = await transport.SendAsync(unitId: 1, Fc03Qty1, TestContext.Current.CancellationToken); + resp[0].ShouldBe((byte)0x03); + server.AcceptedConnectionCount.ShouldBe(1); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusUnitIdResolveHostTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusUnitIdResolveHostTests.cs new file mode 100644 index 00000000..933a201f --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusUnitIdResolveHostTests.cs @@ -0,0 +1,63 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +/// +/// CONV-4 (Modbus leg) — Modbus keys per-host resilience per SLAVE UNIT +/// (host:port/unitN). An equipment tag must carry its own unitId so multi-unit +/// equipment tags key their own breaker; must resolve +/// the equipment ref through the resolver rather than only matching authored names. Scope: +/// unitId ONLY — the parser's other strictness gaps are R2-11's. +/// +[Trait("Category", "Unit")] +public sealed class ModbusUnitIdResolveHostTests +{ + private sealed class NoopTransport : IModbusTransport + { + public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask; + public Task SendAsync(byte unitId, byte[] pdu, CancellationToken ct) + => Task.FromResult(new byte[] { 0x03, 0x00 }); + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + + private static ModbusDriver NewDriver() => new( + new ModbusDriverOptions { Host = "10.0.0.1", Port = 502, UnitId = 1, Probe = new ModbusProbeOptions { Enabled = false } }, + "modbus-1", _ => new NoopTransport()); + + /// The parser reads an optional unitId into the transient definition. + [Fact] + public void Parser_reads_optional_unitId() + { + var json = """{"region":"HoldingRegisters","address":0,"dataType":"Int16","unitId":7}"""; + ModbusEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue(); + def.UnitId.ShouldBe((byte)7); + } + + /// An absent unitId leaves the definition on the driver-level default (null override). + [Fact] + public void Parser_absent_unitId_is_null() + { + var json = """{"region":"HoldingRegisters","address":0,"dataType":"Int16"}"""; + ModbusEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue(); + def.UnitId.ShouldBeNull(); + } + + /// An equipment ref carrying a unitId keys its own per-unit host name. + [Fact] + public void EquipmentTag_WithUnitId_ResolvesPerUnitHostName() + { + var drv = NewDriver(); + var json = """{"region":"HoldingRegisters","address":0,"dataType":"Int16","unitId":7}"""; + drv.ResolveHost(json).ShouldBe("10.0.0.1:502/unit7"); + } + + /// An equipment ref without a unitId keys the driver-level unit host name. + [Fact] + public void EquipmentTag_WithoutUnitId_KeysDriverDefaultUnit() + { + var drv = NewDriver(); + var json = """{"region":"HoldingRegisters","address":0,"dataType":"Int16"}"""; + drv.ResolveHost(json).ShouldBe("10.0.0.1:502/unit1"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DiscoveryAndSubscribeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DiscoveryAndSubscribeTests.cs index 76bfe731..c3195036 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DiscoveryAndSubscribeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DiscoveryAndSubscribeTests.cs @@ -91,8 +91,10 @@ public sealed class S7DiscoveryAndSubscribeTests var h1 = await drv.SubscribeAsync(["T1"], TimeSpan.FromMilliseconds(200), TestContext.Current.CancellationToken); var h2 = await drv.SubscribeAsync(["T2"], TimeSpan.FromMilliseconds(200), TestContext.Current.CancellationToken); - h1.DiagnosticId.ShouldStartWith("s7-sub-"); - h2.DiagnosticId.ShouldStartWith("s7-sub-"); + // Post poll-fork retirement (CONV-1) the subscription handle is the shared engine's + // PollSubscriptionHandle (`poll-sub-`), not the retired S7SubscriptionHandle (`s7-sub-`). + h1.DiagnosticId.ShouldStartWith("poll-sub-"); + h2.DiagnosticId.ShouldStartWith("poll-sub-"); h1.DiagnosticId.ShouldNotBe(h2.DiagnosticId); await drv.UnsubscribeAsync(h1, TestContext.Current.CancellationToken); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7PollEngineMigrationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7PollEngineMigrationTests.cs new file mode 100644 index 00000000..f3e6d8fc --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7PollEngineMigrationTests.cs @@ -0,0 +1,130 @@ +using S7.Net; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using S7NetDataType = global::S7.Net.DataType; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// Guards for retiring the bespoke S7 poll fork onto the shared PollGroupEngine +/// (CONV-1). The engine brings two fixes the fork lacked — structural array diffing (PERF-3) +/// and drain-before-CTS-dispose on unsubscribe (STAB-6-S7) — and keeps failure→health parity. +/// +[Trait("Category", "Unit")] +public sealed class S7PollEngineMigrationTests +{ + // A fake that opens cleanly and serves a CONSTANT 6-byte block for array reads (three + // big-endian UInt16 words) — every ReadArrayAsync decodes a fresh ushort[3] with identical + // contents, so a reference-equality diff (the fork) refires every tick while a structural + // diff (the engine) fires only once. + private sealed class ConstantArrayFactory : IS7PlcFactory + { + public bool ReadThrows { get; set; } + public IS7Plc Create(S7CpuType cpuType, string host, int port, short rack, short slot, TimeSpan timeout) + => new ConstantArrayPlc(this); + } + + private sealed class ConstantArrayPlc(ConstantArrayFactory owner) : IS7Plc + { + public bool IsConnected { get; private set; } + public Task OpenAsync(CancellationToken ct) { IsConnected = true; return Task.CompletedTask; } + public void Close() => IsConnected = false; + + public Task ReadAsync(string address, CancellationToken ct) + { + if (owner.ReadThrows) throw new PlcException(ErrorCode.ConnectionError, "poll failure"); + return Task.FromResult((ushort)42); + } + + public Task ReadBytesAsync(S7NetDataType area, int db, int startByteAdr, int count, CancellationToken ct) + { + if (owner.ReadThrows) throw new PlcException(ErrorCode.ConnectionError, "poll failure"); + // Fresh array instance each call, identical contents: BE words 1, 2, 3. + var block = new byte[count]; + for (var i = 0; i + 1 < block.Length; i += 2) + block[i + 1] = (byte)(i / 2 + 1); + return Task.FromResult(block); + } + + public Task WriteAsync(string address, object value, CancellationToken ct) => Task.CompletedTask; + public Task WriteBytesAsync(S7NetDataType area, int db, int startByteAdr, byte[] value, CancellationToken ct) => Task.CompletedTask; + public Task ReadStatusAsync(CancellationToken ct) => Task.CompletedTask; + public void Dispose() => IsConnected = false; + } + + private static S7DriverOptions ArrayOptions() => new() + { + Host = "192.0.2.1", + Timeout = TimeSpan.FromMilliseconds(250), + Probe = new S7ProbeOptions { Enabled = false }, + Tags = [new S7TagDefinition("Arr", "DB1.DBW0", S7DataType.UInt16, Writable: false, ArrayCount: 3)], + }; + + /// + /// PERF-3 regression: a subscribed array tag whose contents are identical across polls + /// (fresh instance each read) must fire only the initial change, not on every tick. The + /// fork's reference-equality diff refires every poll. + /// + [Fact] + public async Task ArrayTag_UnchangedBetweenPolls_DoesNotRefire() + { + var factory = new ConstantArrayFactory(); + using var drv = new S7Driver(ArrayOptions(), "s7-arr-perf3", factory); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + var events = new System.Collections.Concurrent.ConcurrentQueue(); + drv.OnDataChange += (_, e) => events.Enqueue(e); + + var h = await drv.SubscribeAsync(["Arr"], TimeSpan.FromMilliseconds(100), TestContext.Current.CancellationToken); + await Task.Delay(500, TestContext.Current.CancellationToken); // several poll cycles + await drv.UnsubscribeAsync(h, TestContext.Current.CancellationToken); + + events.Count.ShouldBe(1); // only the initial-data push + } + + /// + /// STAB-6-S7 regression: rapidly subscribing + unsubscribing must not fault a poll loop by + /// disposing its CTS out from under an in-flight Task.Delay. Post-migration the engine + /// drains each loop before disposing its CTS. Asserts the whole churn completes cleanly and + /// the driver still serves reads afterward. + /// + [Fact] + public async Task RapidSubscribeUnsubscribe_NoObjectDisposedException() + { + var factory = new ConstantArrayFactory(); + using var drv = new S7Driver(ArrayOptions(), "s7-rapid", factory); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + for (var i = 0; i < 50; i++) + { + var h = await drv.SubscribeAsync(["Arr"], TimeSpan.FromMilliseconds(20), TestContext.Current.CancellationToken); + await drv.UnsubscribeAsync(h, TestContext.Current.CancellationToken); + } + + // Driver still works after the churn — no wedged state. + var read = await drv.ReadAsync(["Arr"], TestContext.Current.CancellationToken); + read[0].StatusCode.ShouldBe(0u); + } + + /// + /// Failure→health parity: a subscription polling a persistently-failing device degrades the + /// driver health (the fork's HandlePollFailure behaviour, now delivered by the engine's + /// onError→HandlePollError wiring + the read path's own status mapping). + /// + [Fact] + public async Task SustainedPollFailure_DegradesHealth() + { + var factory = new ConstantArrayFactory { ReadThrows = true }; + using var drv = new S7Driver(ArrayOptions(), "s7-pollfail", factory); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + await drv.SubscribeAsync(["Arr"], TimeSpan.FromMilliseconds(50), TestContext.Current.CancellationToken); + + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(3); + while (drv.GetHealth().State == DriverState.Healthy && DateTime.UtcNow < deadline) + await Task.Delay(25, TestContext.Current.CancellationToken); + + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs index 40c55cd9..ff5c6a13 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs @@ -118,6 +118,9 @@ internal class FakeTwinCATClient : ITwinCATClient public bool ThrowOnAddNotification { get; set; } /// Records the most recently-supplied maxDelayMs for Driver.TwinCAT-014 tests. public int LastMaxDelayMs { get; private set; } + /// When set, awaits this gate before creating the handle — + /// lets a test wedge a replay/register mid-flight (STAB-17 orphan race). + public TaskCompletionSource? AddNotificationGate { get; set; } /// Simulates adding a notification for value changes. /// The path to the symbol to watch. @@ -128,17 +131,20 @@ internal class FakeTwinCATClient : ITwinCATClient /// The callback to invoke on value change. /// The cancellation token. /// A task that returns a notification handle. - public virtual Task AddNotificationAsync( + public virtual async Task AddNotificationAsync( string symbolPath, TwinCATDataType type, int? bitIndex, TimeSpan cycleTime, int maxDelayMs, Action onChange, CancellationToken cancellationToken) { if (ThrowOnAddNotification) throw Exception ?? new InvalidOperationException("fake AddNotification failure"); + if (AddNotificationGate is { } gate) + await gate.Task.ConfigureAwait(false); + LastMaxDelayMs = maxDelayMs; var reg = new FakeNotification(symbolPath, type, bitIndex, onChange, this); Notifications.Add(reg); - return Task.FromResult(reg); + return reg; } /// Fire a change event through the registered callback for . diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATConnectBackoffTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATConnectBackoffTests.cs new file mode 100644 index 00000000..f840e810 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATConnectBackoffTests.cs @@ -0,0 +1,90 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; + +/// +/// 05/STAB-8 — a dead TwinCAT device must not be hammered with a full connect attempt on every +/// data call. The per-device gates the connect/create path; +/// the probe loop bypasses it (its own interval is the recovery cadence) and a successful +/// connect resets the window instantly so recovery is never delayed. No TC3 fixture exists — +/// the fake-client suite is authoritative. +/// +[Trait("Category", "Unit")] +public sealed class TwinCATConnectBackoffTests +{ + private const string Host = "ads://5.23.91.23.1.1:851"; + + private static TwinCATDriverOptions Options(bool probeEnabled) => new() + { + Devices = [new TwinCATDeviceOptions(Host)], + Tags = [new TwinCATTagDefinition("T", Host, "MAIN.T", TwinCATDataType.DInt)], + Probe = new TwinCATProbeOptions + { + Enabled = probeEnabled, + Interval = TimeSpan.FromMilliseconds(50), + Timeout = TimeSpan.FromMilliseconds(250), + }, + EnableControllerBrowse = false, + }; + + /// Rapid data calls against a dead device attempt a single connect, then fail fast inside the backoff window. + [Fact] + public async Task DeadDevice_ConnectAttemptsFollowBackoffSchedule() + { + var factory = new FakeTwinCATClientFactory + { + Customise = () => new FakeTwinCATClient + { + ThrowOnConnect = true, + Exception = new InvalidOperationException("device down"), + }, + }; + var drv = new TwinCATDriver(Options(probeEnabled: false), "twincat-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + for (var i = 0; i < 6; i++) + await drv.ReadAsync(["T"], CancellationToken.None); + + // One real connect attempt; the remaining five fail fast inside the 1 s window — no new + // client is created (the fake factory records one client per attempted connect). + factory.Clients.Count.ShouldBe(1); + } + + /// The probe bypasses the backoff window and a successful connect resets it so the next data call succeeds immediately. + [Fact] + public async Task ProbeBypassesBackoff_AndSuccessResets() + { + var fail = true; + var factory = new FakeTwinCATClientFactory + { + Customise = () => new FakeTwinCATClient + { + ThrowOnConnect = fail, + Exception = new InvalidOperationException("device down"), + }, + }; + var drv = new TwinCATDriver(Options(probeEnabled: true), "twincat-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + // First data call opens the backoff window on a dead device. + var down = await drv.ReadAsync(["T"], CancellationToken.None); + down[0].StatusCode.ShouldBe(TwinCATStatusMapper.BadCommunicationError); + + // Device recovers. The data path is still inside the window, but the probe bypasses it and + // reconnects — resetting the window so the following data read succeeds with no residual delay. + fail = false; + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(3); + DataValueSnapshot? good = null; + while (DateTime.UtcNow < deadline) + { + var r = await drv.ReadAsync(["T"], CancellationToken.None); + if (r[0].StatusCode == TwinCATStatusMapper.Good) { good = r[0]; break; } + await Task.Delay(25, CancellationToken.None); + } + + good.ShouldNotBeNull("probe-driven reconnect must reset the backoff so data reads recover"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATPollErrorHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATPollErrorHealthTests.cs new file mode 100644 index 00000000..61e21e93 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATPollErrorHealthTests.cs @@ -0,0 +1,62 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; + +/// +/// 05/STAB-9 — the poll engine's onError sink must be wired to the driver health +/// surface: a poll-loop reader failure degrades health while preserving +/// , and must never downgrade a pre-existing +/// . +/// +[Trait("Category", "Unit")] +public sealed class TwinCATPollErrorHealthTests +{ + private const string Host = "ads://5.23.91.23.1.1:851"; + + /// A poll reader failure degrades health, preserving the last successful read timestamp. + [Fact] + public async Task PollReaderFailure_DegradesHealth_PreservesLastSuccessfulRead() + { + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions(Host)], + Probe = new TwinCATProbeOptions { Enabled = false }, + EnableControllerBrowse = false, + }, "twincat-1", new FakeTwinCATClientFactory()); + await drv.InitializeAsync("{}", CancellationToken.None); + + var before = drv.GetHealth(); + before.State.ShouldBe(DriverState.Healthy); + before.LastSuccessfulRead.ShouldNotBeNull(); + + drv.HandlePollError(new InvalidOperationException("poll boom")); + + var after = drv.GetHealth(); + after.State.ShouldBe(DriverState.Degraded); + after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + after.LastError.ShouldNotBeNull(); + after.LastError.ShouldContain("poll boom"); + } + + /// A poll reader failure never downgrades a pre-existing Faulted state. + [Fact] + public async Task PollReaderFailure_NeverDowngradesFaulted() + { + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions("not-a-valid-address")], + Probe = new TwinCATProbeOptions { Enabled = false }, + EnableControllerBrowse = false, + }, "twincat-1", new FakeTwinCATClientFactory()); + + await Should.ThrowAsync(() => drv.InitializeAsync("{}", CancellationToken.None)); + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + + drv.HandlePollError(new InvalidOperationException("poll boom")); + + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReconnectReplayTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReconnectReplayTests.cs index d4940620..c22c60de 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReconnectReplayTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATReconnectReplayTests.cs @@ -170,6 +170,200 @@ public sealed class TwinCATReconnectReplayTests await drv.ShutdownAsync(TestContext.Current.CancellationToken); } + // ---- STAB-16 / STAB-17 / STAB-12 replay hardening ---- + + /// + /// STAB-16: a replay failure must not be silent — the registration is marked dead, a Bad + /// snapshot is pushed to subscribers (instead of the frozen-Good-stale value), and health + /// degrades preserving the last successful read. + /// + [Fact] + public async Task ReplayFailure_EmitsBadQuality_AndDegradesHealth() + { + var build = 0; + var factory = new FakeTwinCATClientFactory + { + // client1 normal; client2 (the reconnect target) fails AddNotification (replay) + reads. + Customise = () => new FakeTwinCATClient + { + ThrowOnAddNotification = ++build >= 2, + ThrowOnRead = build >= 2, + Exception = new InvalidOperationException("ADS notification quota exceeded"), + }, + }; + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions(Host)], + Tags = [new TwinCATTagDefinition("Speed", Host, "MAIN.Speed", TwinCATDataType.DInt)], + Probe = new TwinCATProbeOptions { Enabled = false }, + UseNativeNotifications = true, + }, "drv-1", factory); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + var events = new ConcurrentQueue(); + drv.OnDataChange += (_, e) => events.Enqueue(e); + + _ = await drv.SubscribeAsync(["Speed"], TimeSpan.FromMilliseconds(250), TestContext.Current.CancellationToken); + // Establish a last-successful-read on the healthy client1. + _ = await drv.ReadAsync(["Speed"], TestContext.Current.CancellationToken); + var lastGoodRead = drv.GetHealth().LastSuccessfulRead; + lastGoodRead.ShouldNotBeNull(); + + // Drop the wire; the next read reconnects onto client2, whose replay AddNotification fails. + factory.Clients[0].SimulateWireDrop(); + _ = await drv.ReadAsync(["Speed"], TestContext.Current.CancellationToken); + + // STAB-16: subscribers see the tag go Bad (not frozen Good), and health degraded while + // preserving the last successful read. + var bad = events.SingleOrDefault(e => e.FullReference == "Speed" + && e.Snapshot.StatusCode == TwinCATStatusMapper.BadCommunicationError); + bad.ShouldNotBeNull("a failed replay must publish a Bad snapshot for the affected reference"); + var health = drv.GetHealth(); + health.State.ShouldBe(DriverState.Degraded); + health.LastSuccessfulRead.ShouldBe(lastGoodRead); + } + + /// + /// STAB-16: a registration whose replay failed (dead handle) is re-registered on the next + /// successful probe tick — a subsequent push then reaches OnDataChange with Good, and no + /// duplicate registration is created. + /// + [Fact] + public async Task ReplayFailure_RetriedOnNextSuccessfulProbeTick() + { + var build = 0; + var factory = new FakeTwinCATClientFactory + { + // Only the 2nd client (the reconnect target) fails its first replay; flipped off below. + Customise = () => new FakeTwinCATClient { ThrowOnAddNotification = ++build == 2 }, + }; + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions(Host)], + Tags = [new TwinCATTagDefinition("Speed", Host, "MAIN.Speed", TwinCATDataType.DInt)], + Probe = new TwinCATProbeOptions + { + Enabled = true, + Interval = TimeSpan.FromMilliseconds(40), + Timeout = TimeSpan.FromMilliseconds(100), + }, + UseNativeNotifications = true, + }, "drv-1", factory); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + var events = new ConcurrentQueue(); + drv.OnDataChange += (_, e) => events.Enqueue(e); + + _ = await drv.SubscribeAsync(["Speed"], TimeSpan.FromMilliseconds(250), TestContext.Current.CancellationToken); + await WaitForAsync(() => factory.Clients.Count >= 1, TimeSpan.FromSeconds(2)); + + // Drop the wire; the reconnect target (client2) fails its replay, leaving a dead registration. + factory.Clients[0].SimulateWireDrop(); + _ = await drv.ReadAsync(["Speed"], TestContext.Current.CancellationToken); + await WaitForAsync(() => factory.Clients.Count >= 2, TimeSpan.FromSeconds(2)); + var client2 = factory.Clients[1]; + client2.Notifications.ShouldBeEmpty(); // replay failed — no live handle + + // Device now accepts registrations again — the probe-tick retry must re-register the intent. + client2.ThrowOnAddNotification = false; + await WaitForAsync(() => client2.Notifications.Count == 1, TimeSpan.FromSeconds(3)); + client2.Notifications.Count.ShouldBe(1); // re-registered exactly once (no duplicate) + + // A fresh push on the recovered registration reaches OnDataChange with Good. + client2.FireNotification("MAIN.Speed", 4242); + await WaitForAsync(() => events.Any(e => e.FullReference == "Speed" + && e.Snapshot.StatusCode == TwinCATStatusMapper.Good), TimeSpan.FromSeconds(2)); + events.Any(e => e.FullReference == "Speed" + && e.Snapshot.StatusCode == TwinCATStatusMapper.Good + && Equals(e.Snapshot.Value, 4242)).ShouldBeTrue(); + + await drv.ShutdownAsync(TestContext.Current.CancellationToken); + } + + /// + /// STAB-17: unsubscribing WHILE a replay is mid-AddNotificationAsync must not leak a + /// live ADS notification. After the fresh handle is installed, an ownership re-check finds the + /// registration was unsubscribed and retracts (disposes) the fresh handle — no orphan. + /// + [Fact] + public async Task UnsubscribeDuringReplay_DisposesFreshHandle_NoOrphan() + { + var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var build = 0; + var factory = new FakeTwinCATClientFactory + { + Customise = () => + { + var c = new FakeTwinCATClient(); + if (++build == 2) c.AddNotificationGate = gate; // client2's replay blocks on the gate + return c; + }, + }; + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions(Host)], + Tags = [new TwinCATTagDefinition("X", Host, "MAIN.X", TwinCATDataType.DInt)], + Probe = new TwinCATProbeOptions { Enabled = false }, + UseNativeNotifications = true, + }, "drv-1", factory); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + var handle = await drv.SubscribeAsync(["X"], TimeSpan.FromMilliseconds(250), TestContext.Current.CancellationToken); + + // Drop the wire; a read reconnects onto client2, whose replay AddNotification blocks on the gate. + factory.Clients[0].SimulateWireDrop(); + var readTask = Task.Run(() => drv.ReadAsync(["X"], CancellationToken.None)); + await WaitForAsync(() => factory.Clients.Count >= 2, TimeSpan.FromSeconds(2)); + await Task.Delay(50); // let the replay reach the blocked AddNotification + + // Unsubscribe mid-replay (gate-free) — removes the registration from the device registry. + await drv.UnsubscribeAsync(handle, TestContext.Current.CancellationToken); + + // Release the gate: AddNotification completes, SwapHandle installs the fresh handle, and the + // ownership re-check must retract it (no owner) rather than leave a live orphan notification. + gate.SetResult(); + await readTask; + + await WaitForAsync(() => factory.Clients[1].Notifications.Count == 0, TimeSpan.FromSeconds(2)); + factory.Clients[1].Notifications.ShouldBeEmpty("the fresh handle must be disposed — no orphan ADS notification"); + } + + /// + /// STAB-12 (compounded part): RecycleClientAsync must honor cancellation — it must + /// take the probe token into ConnectGate.WaitAsync(ct) rather than + /// CancellationToken.None, so a shutdown mid-recycle (or a connect wedged under the + /// gate) can't hang the probe loop uncancellably. Verified white-box: with the gate held and + /// a cancelled token, the call must throw promptly rather than block forever. + /// + [Fact] + public async Task RecycleClient_HonorsCancellation() + { + var (drv, _) = NewNativeDriver( + tags: new TwinCATTagDefinition("X", Host, "MAIN.X", TwinCATDataType.DInt)); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + // Reach the single configured device + wedge its ConnectGate so a recycle must wait on it. + var devicesField = typeof(TwinCATDriver).GetField("_devices", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!; + var devicesObj = devicesField.GetValue(drv)!; + var values = (System.Collections.IEnumerable)devicesObj.GetType().GetProperty("Values")!.GetValue(devicesObj)!; + var device = (TwinCATDriver.DeviceState)values.Cast().First(); + await device.ConnectGate.WaitAsync(TestContext.Current.CancellationToken); + + var recycle = typeof(TwinCATDriver).GetMethod("RecycleClientAsync", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!; + + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + var task = (Task)recycle.Invoke(drv, [device, cts.Token])!; + + // Must throw promptly (cancelled token), NOT hang on the held gate. The 2 s bound turns an + // uncancellable-wait regression into a failure rather than a hung suite. + await Should.ThrowAsync(async () => await task.WaitAsync(TimeSpan.FromSeconds(2))); + + device.ConnectGate.Release(); + } + private static async Task WaitForAsync(Func condition, TimeSpan timeout) { var deadline = DateTime.UtcNow + timeout; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATResolveHostTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATResolveHostTests.cs new file mode 100644 index 00000000..c7a52c07 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATResolveHostTests.cs @@ -0,0 +1,43 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; + +/// +/// CONV-4 — must resolve an equipment-tag reference +/// (raw TagConfig JSON) to its OWN device host so per-host breaker keys are correct for +/// equipment tags, not the first-device fallback. +/// +[Trait("Category", "Unit")] +public sealed class TwinCATResolveHostTests +{ + private const string DeviceA = "ads://5.23.91.23.1.1:851"; + private const string DeviceB = "ads://5.23.91.23.1.2:851"; + + private static TwinCATDriver NewDriver() => new( + new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions(DeviceA), new TwinCATDeviceOptions(DeviceB)], + Probe = new TwinCATProbeOptions { Enabled = false }, + EnableControllerBrowse = false, + }, + "twincat-1", new FakeTwinCATClientFactory()); + + /// An equipment-tag ref naming device B resolves to device B's host, not the first device. + [Fact] + public void ResolveHost_EquipmentTagRef_ReturnsItsOwnDeviceHost() + { + var drv = NewDriver(); + var json = $$"""{"deviceHostAddress":"{{DeviceB}}","symbolPath":"MAIN.Speed","dataType":"DInt"}"""; + drv.ResolveHost(json).ShouldBe(DeviceB); + } + + /// An unresolvable ref keeps the current first-device fallback. + [Fact] + public void ResolveHost_UnknownRef_KeepsCurrentFallback() + { + var drv = NewDriver(); + drv.ResolveHost("totally-unknown").ShouldBe(DeviceA); + } +}