From e3cb4f0c7b6909bb5610edd1cc0c4c49f13577ec Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 15:45:04 -0400 Subject: [PATCH] fix(ablegacy): opaque replies are neither answers nor faults; round-2 minors Review r1 N1 (Important): the I1 fix made IsDeviceRefusal default-open and mapped libplctag's ERR_REMOTE_ERR to an answer. libplctag folds EVERY non-zero PCCC STS and CIP status into that code, including a routing hop's "unconnected send timed out" / "connection lost" / "remote node missing", and native BadReply covers truncated / mismatched / unmapped replies - so a powered-off PLC behind a bridge read as reachable: Healthy, LastSuccessfulRead advancing, the comms-loss watchdog unable to fire. Three classes now, each pinned both ways: - no reply (Timeout/BadConnection/Abort/Pending/unknown): comms fault; - answered - an ALLOW-LIST again (NotFound/NotAllowed/OutOfBounds/TooLarge/ TooSmall/TypeMismatch/DeviceStateConflict): proves reachability; - OPAQUE (BadReply/Partial; ERR_REMOTE_ERR/ERR_BAD_STATUS now translate to the existing BadReply, OPC UA BadCommunicationError as before r0): per-tag Bad, handle kept, coalescing fallback, never fail-fast, never an answer. Option (a): a device whose pass/call produced only opaque outcomes is Degraded with LastSuccessfulRead not advanced, so the watchdog reaches a dead routed PLC. The probe and index reads classify the same way. Minors: - N2: a mismatched array (type vs data file) READS BadTypeMismatch instead of decoding the file at the wrong width as Good. - N3: a swept indirect resolution is never reconnected under a forgotten gate; that one call answers neutrally. - N4: an index read answered Unsupported is neutral, not a transport fault. - N5: ArrayFileMismatch checks indirect addresses (the file letter is static). - Test nit: the evicted-while-waiting test signals gate arrival via a seam instead of sleeping. Live: 51/51 on ab_server (slc500/micrologix/plc5 x both transports), incl. an opaque-only libplctag device staying Degraded with LastSuccessfulRead frozen. Refs #738 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../AbLegacyTagDefinitionFactory.cs | 4 +- .../AbLegacyDriver.cs | 167 ++++++++++++++---- .../LibplctagLegacyStatusTranslation.cs | 17 +- .../AbLegacyReadCoalescingIntegrationTests.cs | 41 ++++- .../AbLegacyAnsweredStatusTests.cs | 166 ++++++++++++++--- .../AbLegacyArrayWriteTests.cs | 15 ++ .../AbLegacyHealthEvidenceTests.cs | 96 +++++++++- .../AbLegacyHostAndStatusTests.cs | 3 - 8 files changed, 434 insertions(+), 75 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs index ea87f079..c10832d7 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyTagDefinitionFactory.cs @@ -289,7 +289,9 @@ public static class AbLegacyTagDefinitionFactory public static string? ArrayFileMismatch(AbLegacyDataType type, string address) { if (!IsArrayElementTypeSupported(type)) return null; // structured arrays have their own refusal - if (AbLegacyAddress.TryParse(address) is not { } a || a.IsIndirect) return null; + // The file LETTER is static even for an indirect address (N7:[N7:0], N[N7:0]:5) — only the numbers move — + // so the width rule applies to it too (review 07 r1 N5). + if (AbLegacyAddress.TryParse(address) is not { } a) return null; var file = ArrayFileElementType(a); var ok = type switch { 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 28b6fbe7..2444f3c1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -468,7 +468,11 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag /// evidence for the backoff); = nothing reached the wire or it says nothing about /// reachability (a local refusal, a bad index value). /// - private enum OpVerdict { Neutral, Answered, Fault, Throttled } + /// = a reply arrived that cannot be classified (libplctag ERR_REMOTE_ERR, native + /// BadReply/Partial): it may be the device refusing OR a routing hop reporting that the end device is + /// gone, so it is NOT an answer (no backoff reset, no LastSuccessfulRead) and NOT a fault (no fail-fast); + /// a pass that produced only opaque outcomes for a device degrades (review 07 r1 N1). + private enum OpVerdict { Neutral, Answered, Fault, Throttled, Opaque } /// The health evidence one device pass produced (collected, then written once per read). private struct DeviceReadOutcome @@ -476,6 +480,18 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag public bool AnyGood; public bool AnyAnswered; public string? FaultMessage; + public string? OpaqueMessage; + + /// + /// Folds an opaque-only device into the fault class for STATE (Degraded, LastSuccessfulRead not advanced + /// by it) — option (a) of review 07 r1 N1: a dead PLC behind a bridge answers every request with a fast remote + /// error, and only this lets the comms-loss watchdog (non-Healthy AND frozen LastSuccessfulRead) reach + /// it. Cost: a single-tag device whose only tag draws an opaque refusal is re-initialised every 90 s. + /// + public void FoldOpaque() + { + if (OpaqueMessage is not null && !AnyAnswered) FaultMessage ??= OpaqueMessage; + } } /// @@ -641,8 +657,12 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag case OpVerdict.Throttled: outcome.FaultMessage ??= message; break; + case OpVerdict.Opaque: + outcome.OpaqueMessage ??= message; + break; } } + outcome.FoldOpaque(); return outcome; } @@ -664,11 +684,22 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag results[item.Index] = new DataValueSnapshot(null, res.Status, null, now); return res.TransportFault ? (OpVerdict.Fault, $"transport fault resolving indirect address for {reference}") - : (OpVerdict.Neutral, null); + : res.Opaque + ? (OpVerdict.Opaque, $"unclassifiable reply resolving indirect address for {reference}") + : (OpVerdict.Neutral, null); } def = res.Def; } + // A mis-authored array (its type does not match its data file) would decode the file at the wrong width + // and serve garbage as Good (review 07 r1 N2): answered BadTypeMismatch, warned once, no health write. + if (IsArrayTag(def) && AbLegacyTagDefinitionFactory.ArrayFileMismatch(def.DataType, def.Address) is { } widthError) + { + WarnRefusalOnce(device, def, "read", widthError, AbLegacyStatusMapper.BadTypeMismatch); + results[item.Index] = new DataValueSnapshot(null, AbLegacyStatusMapper.BadTypeMismatch, null, now); + return (OpVerdict.Neutral, null); + } + var runtime = await EnsureTagRuntimeAsync(device, def, ct).ConfigureAwait(false); int status; @@ -677,10 +708,17 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag // runtime — the server read path and the poll loop both call ReadAsync against // the same cached libplctag Tag handle, which is not concurrency-safe. var opLock = device.GetRuntimeLock(def.Name); + device.BeforeOperationGate?.Invoke(def.Name); await opLock.WaitAsync(ct).ConfigureAwait(false); try { - runtime = await CurrentRuntimeUnderLockAsync(device, def, runtime, ct).ConfigureAwait(false); + if (await CurrentRuntimeUnderLockAsync(device, def, runtime, ct).ConfigureAwait(false) is not { } live) + { + // A swept indirect resolution (the index moved under us): never resurrect it under a forgotten gate. + results[item.Index] = new DataValueSnapshot(null, AbLegacyStatusMapper.BadCommunicationError, null, now); + return (OpVerdict.Neutral, null); + } + runtime = live; await runtime.ReadAsync(ct).ConfigureAwait(false); status = runtime.GetStatus(); var parsed = AbLegacyAddress.TryParse(def.Address, device.Options.PlcFamily); @@ -713,6 +751,12 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag WarnRefusalOnce(device, def, "read", "status Unsupported", AbLegacyStatusMapper.BadNotSupported); return (OpVerdict.Neutral, null); } + if (IsOpaque((AbStatus)status)) + { + // A reply we cannot classify: per-tag Bad, keep the handle, warn once — neither an answer nor a fault. + WarnRefusalOnce(device, def, "read", $"unclassifiable reply {(AbStatus)status}", results[item.Index].StatusCode); + return (OpVerdict.Opaque, $"unclassifiable reply {(AbStatus)status} reading {reference}"); + } // Evict the stale handle so the next call re-creates it (mirrors AbCip). EvictRuntime(device, def.Name); if (!device.FirstNonZeroStatusLogged) @@ -752,6 +796,13 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag results[item.Index] = new DataValueSnapshot(null, AbLegacyStatusMapper.MapStatus(ex.Status), null, now); return (OpVerdict.Answered, null); } + catch (AbTransportException ex) when (IsOpaque(ex.Status)) + { + // A truncated / mismatched / unmapped reply (native BadReply, Partial): not proof the device answered. + WarnRefusalOnce(device, def, "read", ex.Message, AbLegacyStatusMapper.MapStatus(ex.Status)); + results[item.Index] = new DataValueSnapshot(null, AbLegacyStatusMapper.MapStatus(ex.Status), null, now); + return (OpVerdict.Opaque, ex.Message); + } catch (NotSupportedException nse) { // A deterministic decode refusal (review 07 H16) — the same answer every poll. BadNotSupported, keep @@ -781,23 +832,34 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag !Enum.IsDefined(status) || status is AbStatus.Timeout or AbStatus.BadConnection or AbStatus.Abort or AbStatus.Pending; /// - /// True for a non-zero status that proves the DEVICE answered and refused this one operation — the file or - /// element does not exist, the access is not allowed, a size / type is wrong, a remote error (libplctag - /// ERR_REMOTE_ERR, translated to ), or a reply the transport - /// could not map (, ). Such a status is per-tag Bad, - /// keeps the handle, and is evidence of reachability. is classified - /// separately (: it says nothing about reachability). + /// True for a status that PROVES the device answered and refused this one operation — an ALLOW-LIST (review 07 + /// r1 N1): the file or element does not exist, the access is not allowed, it is out of bounds, a size or type is + /// wrong, or the processor is in a conflicting state. Per-tag Bad, keeps the handle, and is evidence of + /// reachability (backoff reset, LastSuccessfulRead, Healthy). /// /// - /// ⚠️ Depends on the transport's PCCC STS table (AbStatusCodes.FromPccc, Lane K): today it maps STS 0x30 - /// ("remote node missing" — a dead DH+/DH-485 node behind a bridge) and 0x20 to answered values, so on the - /// NATIVE transport such a dead node reads as answered and never trips the backoff or the comms-loss watchdog. - /// The remap is follow-up F8 (review 07 r0 I4); this classification needs no change once it lands. + /// ⚠️ Inherits the native transport's PCCC STS table (AbStatusCodes.FromPccc, Lane K): it maps STS 0x30 + /// ("remote node missing" — a dead DH+/DH-485 node behind a bridge) and 0x20 to NotAllowed/NotFound, + /// so on the NATIVE transport such a dead node still reads as answered. Follow-up F8 (review 07 r0 I4); this + /// classification needs no change once it lands. /// /// The transport status. /// for a device refusal. - internal static bool IsDeviceRefusal(AbStatus status) => - status is not (AbStatus.Ok or AbStatus.Unsupported) && !IsNoReply(status); + internal static bool IsDeviceRefusal(AbStatus status) => status is + AbStatus.NotFound or AbStatus.NotAllowed or AbStatus.OutOfBounds or AbStatus.TooLarge or AbStatus.TooSmall + or AbStatus.TypeMismatch or AbStatus.DeviceStateConflict; + + /// + /// True for a reply that arrived but cannot be classified (review 07 r1 N1): — + /// libplctag's opaque ERR_REMOTE_ERR/ERR_BAD_STATUS, which folds EVERY non-zero CIP general status + /// and PCCC STS together (including a routing hop's "unconnected send timed out", "connection lost" and "remote + /// node missing"), and the native transport's unmapped / truncated / mismatched reply — and + /// . Opaque statuses are per-tag Bad and trigger the coalescing fallback, but are + /// neither an answer nor a comms fault; see for the health rule. + /// + /// The transport status. + /// for an unclassifiable reply. + internal static bool IsOpaque(AbStatus status) => status is AbStatus.BadReply or AbStatus.Partial; // ---- IWritable ---- @@ -866,6 +928,8 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag device.RecordCommsFault(Clock()); outcome.FaultMessage ??= $"transport fault resolving indirect address for {w.FullReference}"; } + else if (res.Opaque) + outcome.OpaqueMessage ??= $"unclassifiable reply resolving indirect address for {w.FullReference}"; continue; } def = res.Def; @@ -943,7 +1007,12 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag await opLock.WaitAsync(cancellationToken).ConfigureAwait(false); try { - runtime = await CurrentRuntimeUnderLockAsync(device, def, runtime, cancellationToken).ConfigureAwait(false); + if (await CurrentRuntimeUnderLockAsync(device, def, runtime, cancellationToken).ConfigureAwait(false) is not { } live) + { + results[i] = new WriteResult(AbLegacyStatusMapper.BadCommunicationError); + continue; // the index moved under us (N3): not written, no health evidence + } + runtime = live; if (arrayValue is not null) runtime.EncodeArray(arrayFileType, arrayValue); else runtime.EncodeValue(def.DataType, parsed.BitIndex, w.Value); await runtime.WriteAsync(cancellationToken).ConfigureAwait(false); @@ -973,6 +1042,12 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag WarnRefusalOnce(device, def, "write", ex.Message, AbLegacyStatusMapper.MapStatus(ex.Status)); results[i] = new WriteResult(AbLegacyStatusMapper.MapStatus(ex.Status)); } + catch (AbTransportException ex) when (IsOpaque(ex.Status)) + { + outcome.OpaqueMessage ??= ex.Message; + WarnRefusalOnce(device, def, "write", ex.Message, AbLegacyStatusMapper.MapStatus(ex.Status)); + results[i] = new WriteResult(AbLegacyStatusMapper.MapStatus(ex.Status)); + } catch (NotSupportedException nse) { // Deterministic (review 07 H16): nothing reached the wire, so it says nothing about the device. @@ -1004,6 +1079,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag } } + outcome.FoldOpaque(); ApplyHealth(epoch, outcome.FaultMessage, anyGood: outcome.AnyAnswered, anyAnswered: outcome.AnyAnswered, DateTime.UtcNow); return results; } @@ -1028,6 +1104,12 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag device.RecordCommsFault(Clock()); outcome.FaultMessage ??= $"transport status {status} writing {def.Name}"; } + else if (IsOpaque(s)) + { + // Neither an answer nor a fault (review 07 r1 N1). + WarnRefusalOnce(device, def, "write", $"unclassifiable reply {s}", AbLegacyStatusMapper.MapStatus(status)); + outcome.OpaqueMessage ??= $"unclassifiable reply {s} writing {def.Name}"; + } } // ---- ITagDiscovery ---- @@ -1120,7 +1202,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag // The probe's verdict feeds the data path's backoff (review 07 P7): an answer resets the window so a // recovered device is read again at once; a comms fault counts toward marking it down. if (success || IsDeviceRefusal((AbStatus)status)) state.RecordAnswered(); - else if ((AbStatus)status != AbStatus.Unsupported) state.RecordCommsFault(Clock()); + else if (IsNoReply((AbStatus)status)) state.RecordCommsFault(Clock()); // opaque / Unsupported: neutral } catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; } catch (Exception ex) @@ -1129,7 +1211,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag probeRuntime = null; state.ProbeInitialized = false; if (ex is AbTransportException ate && IsDeviceRefusal(ate.Status)) state.RecordAnswered(); - else if (!IsLocalRefusal(ex)) state.RecordCommsFault(Clock()); + else if (ex is not AbTransportException { Status: var st } || IsNoReply(st)) state.RecordCommsFault(Clock()); } TransitionDeviceState(state, success ? HostState.Running : HostState.Stopped); @@ -1320,7 +1402,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag if (parsed.IndirectFileSource is { } fileSrc) { var r = await ReadIndexAsync(device, fileSrc, ct).ConfigureAwait(false); - if (r.Status != 0) return new IndirectResolution(null, r.Status, r.TransportFault); + if (r.Status != 0) return new IndirectResolution(null, r.Status, r.TransportFault, r.Opaque); if (r.Value is < 0 or > MaxIndirectFile) return new IndirectResolution(null, AbLegacyStatusMapper.BadOutOfRange, false); fileNumber = (int)r.Value!; @@ -1328,7 +1410,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag if (parsed.IndirectWordSource is { } wordSrc) { var r = await ReadIndexAsync(device, wordSrc, ct).ConfigureAwait(false); - if (r.Status != 0) return new IndirectResolution(null, r.Status, r.TransportFault); + if (r.Status != 0) return new IndirectResolution(null, r.Status, r.TransportFault, r.Opaque); if (r.Value < 0 || r.Value > maxElement) return new IndirectResolution(null, AbLegacyStatusMapper.BadOutOfRange, false); word = (int)r.Value!; @@ -1369,13 +1451,13 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag } /// Reads one direct index address as an integer through the shared parent-runtime cache. - private async Task<(long? Value, uint Status, bool TransportFault)> ReadIndexAsync( + private async Task<(long? Value, uint Status, bool TransportFault, bool Opaque)> ReadIndexAsync( DeviceState device, AbLegacyAddress inner, CancellationToken ct) { AbLegacyDataType type; if (inner.BitIndex is not null) type = AbLegacyDataType.Bit; else if (inner.FileLetter == "L") type = AbLegacyDataType.Long; - else if (inner.FileLetter is "F" or "ST") return (null, AbLegacyStatusMapper.BadOutOfRange, false); + else if (inner.FileLetter is "F" or "ST") return (null, AbLegacyStatusMapper.BadOutOfRange, false, false); else type = AbLegacyDataType.Int; // A bit-addressed index reads its parent word and decodes the bit, like the RMW path does. @@ -1394,40 +1476,49 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag catch (ConnectionThrottledException) { throw; } // the caller answers it as a fail-fast catch (AbTransportException ex) when (IsLocalRefusal(ex)) { - return (null, AbLegacyStatusMapper.BadNotSupported, false); + return (null, AbLegacyStatusMapper.BadNotSupported, false, false); } catch (AbTransportException ex) when (IsDeviceRefusal(ex.Status)) { // The device answered: the index file/element does not exist. Per-tag, not a comms fault. - return (null, AbLegacyStatusMapper.MapStatus(ex.Status), false); + return (null, AbLegacyStatusMapper.MapStatus(ex.Status), false, false); + } + catch (AbTransportException ex) when (IsOpaque(ex.Status)) + { + return (null, AbLegacyStatusMapper.MapStatus(ex.Status), false, true); } catch { EvictParentRuntime(device, runtimeName); - return (null, AbLegacyStatusMapper.BadCommunicationError, true); + return (null, AbLegacyStatusMapper.BadCommunicationError, true, false); } var status = runtime.GetStatus(); if (status != 0 && IsDeviceRefusal((AbStatus)status)) - return (null, AbLegacyStatusMapper.MapStatus(status), false); - if (status != 0) + return (null, AbLegacyStatusMapper.MapStatus(status), false, false); + if (status != 0 && IsOpaque((AbStatus)status)) + return (null, AbLegacyStatusMapper.MapStatus(status), false, true); + // Only a NO-REPLY status is a transport fault (review 07 r1 N4: Unsupported used to fall in here). + if (status != 0 && IsNoReply((AbStatus)status)) { EvictParentRuntime(device, runtimeName); - return (null, AbLegacyStatusMapper.MapStatus(status), true); + return (null, AbLegacyStatusMapper.MapStatus(status), true, false); } + if (status != 0) + return (null, AbLegacyStatusMapper.MapStatus(status), false, false); var raw = runtime.DecodeValue(type, inner.BitIndex); try { return raw switch { - bool b => (b ? 1L : 0L, 0u, false), - null => (null, AbLegacyStatusMapper.BadOutOfRange, false), - _ => (Convert.ToInt64(raw), 0u, false), + bool b => (b ? 1L : 0L, 0u, false, false), + null => (null, AbLegacyStatusMapper.BadOutOfRange, false, false), + _ => (Convert.ToInt64(raw), 0u, false, false), }; } catch (Exception ex) when (ex is FormatException or InvalidCastException or OverflowException) { - return (null, AbLegacyStatusMapper.BadOutOfRange, false); + return (null, AbLegacyStatusMapper.BadOutOfRange, false, false); } } finally @@ -1444,7 +1535,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag } } - private sealed record IndirectResolution(AbLegacyTagDefinition? Def, uint Status, bool TransportFault); + private sealed record IndirectResolution(AbLegacyTagDefinition? Def, uint Status, bool TransportFault, bool Opaque = false); private async Task EnsureParentRuntimeAsync( AbLegacyDriver.DeviceState device, string parentName, CancellationToken ct) @@ -1611,11 +1702,15 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag /// the wait may since have been evicted and disposed (a failed sibling call, or an indirect index sweep), and /// using it would fail spuriously as a comms fault. Returns the live cached handle, or connects a fresh one. /// - private async Task CurrentRuntimeUnderLockAsync( + private async Task CurrentRuntimeUnderLockAsync( DeviceState device, AbLegacyTagDefinition def, IAbLegacyTagRuntime fetched, CancellationToken ct) { if (device.Runtimes.TryGetValue(def.Name, out var live)) return live; + // A resolved INDIRECT key that is gone was swept because the index moved (review 07 r1 N3). Reconnecting it + // would resurrect a stale resolution under a gate the sweep already forgot (a later caller would get a fresh + // gate for the same handle). Null = the caller answers this one call neutrally; the next poll re-resolves. + if (def.Name.Contains("=>", StringComparison.Ordinal)) return null; // Evicted between the fetch and the lock: a native runtime refused at init is cached too, so absence means // it really was released. Connect again (single-flight per device). // A retired (torn-down) state never reconnects: the in-flight call finishes on what it has. @@ -1886,6 +1981,10 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag /// The number of per-key gates currently held (runtime + RMW). Tests only. internal int GateCount => _runtimeLocks.Count + _rmwLocks.Count; + /// Test seam: invoked with the runtime key just before an operation waits on its gate (after the + /// handle was fetched), so a test can act at exactly that point instead of sleeping. + internal Action? BeforeOperationGate { get; set; } + private volatile bool _retired; /// True once this state has been unpublished and released; nothing may be cached on it any more. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyStatusTranslation.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyStatusTranslation.cs index 281c4ebd..45855904 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyStatusTranslation.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyStatusTranslation.cs @@ -23,14 +23,15 @@ internal static class LibplctagLegacyStatusTranslation Status.ErrorPartial => AbStatus.Partial, Status.ErrorAbort => AbStatus.Abort, Status.ErrorBadReply => AbStatus.BadReply, - // The PLC (or a CIP hop) ANSWERED with an error: libplctag's PCCC layer turns a non-zero STS — a file or - // element that does not exist, a write the processor refuses — into PLCTAG_ERR_REMOTE_ERR (and a non-zero - // CIP general status into ERR_REMOTE_ERR / ERR_BAD_STATUS). AbStatus has no dedicated "remote error" value and - // Driver.Ab.Transport is not this driver's to extend (Lane K), so this maps to the existing value whose meaning - // is closest — "the device is reachable but refused the operation" — DeviceStateConflict (OPC UA - // BadDeviceFailure). It MUST NOT fall into the BadConnection default: that is a no-reply comms fault, which - // evicts handles, degrades health and counts toward marking the device down (review 07 r0 I1). - Status.ErrorRemoteErr or Status.ErrorBadStatus => AbStatus.DeviceStateConflict, + // A reply ARRIVED but carries an error libplctag does not break out: it folds EVERY non-zero PCCC STS and CIP + // general status into PLCTAG_ERR_REMOTE_ERR / ERR_BAD_STATUS. That covers the PLC refusing (element past the + // end of the file) AND a routing hop reporting the end device is gone (CIP 0x01/0x0204 unconnected send timed + // out, 0x07 connection lost, PCCC STS 0x30 remote node missing). So it is neither a no-reply comms fault + // (review 07 r0 I1 — it must not fall into the BadConnection default, which black-outs coalesced neighbours and + // trips fail-fast) nor proof of reachability (review 07 r1 N1). AbStatus has no dedicated "opaque remote error" + // value and Driver.Ab.Transport is Lane K's, so this maps to the existing BadReply — "a reply we cannot + // classify" — which the driver treats as OPAQUE (AbLegacyDriver.IsOpaque). OPC UA: BadCommunicationError. + Status.ErrorRemoteErr or Status.ErrorBadStatus => AbStatus.BadReply, _ => AbStatus.BadConnection, }; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadCoalescingIntegrationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadCoalescingIntegrationTests.cs index e3aa58d2..8542eb22 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadCoalescingIntegrationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadCoalescingIntegrationTests.cs @@ -101,9 +101,46 @@ public sealed class AbLegacyReadCoalescingIntegrationTests(AbLegacyServerFixture r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: neighbour A"); r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: neighbour B"); r[2].StatusCode.ShouldNotBe(AbLegacyStatusMapper.Good, $"poll {poll}: the past-the-end tag"); - r[2].StatusCode.ShouldNotBe(AbLegacyStatusMapper.BadCommunicationError, - $"poll {poll}: the PLC answered — not a comms fault (status 0x{r[2].StatusCode:X8})"); } + // The refusal (libplctag: an opaque ERR_REMOTE_ERR; native: a mapped STS) is never a comms fault: the + // answering neighbours keep the driver Healthy and the device is never failed fast. drv.GetHealth().State.ShouldBe(DriverState.Healthy); } + + /// + /// Review 07 r1 N1, on the wire: libplctag reports the past-the-end refusal as the OPAQUE ERR_REMOTE_ERR — + /// the same code a bridge returns for a powered-off PLC behind it — so a device whose ONLY traffic is that + /// reply must not look reachable: Degraded, LastSuccessfulRead frozen (the comms-loss watchdog's + /// evidence). The native transport maps the STS itself and is only checked to stay non-Good. + /// + /// The simulator profile. + /// The transport stack the driver runs on. + [AbLegacyTheory] + [MemberData(nameof(TransportMatrix.Profiles), MemberType = typeof(TransportMatrix))] + public async Task An_opaque_only_device_never_proves_reachability(AbLegacyServerProfile profile, AbLegacyTransport transport) + { + if (sim.SkipReasonFor(profile) is { } skip) Assert.Skip(skip); + var ct = TestContext.Current.CancellationToken; + var uri = $"ab://{sim.Host}:{sim.PortFor(profile)}/{sim.CipPath}"; + await using var drv = new AbLegacyDriver(new AbLegacyDriverOptions + { + Devices = [new AbLegacyDeviceOptions(uri, profile.Family)], + RawTags = AbLegacyRawTags.Entries(new AbLegacyTagDefinition("PastEnd", uri, "N7:12", AbLegacyDataType.Int)), + Timeout = TimeSpan.FromSeconds(5), + Probe = new AbLegacyProbeOptions { Enabled = false }, + Transport = transport, + }, driverInstanceId: $"ablegacy-opaque-{profile.Family}-{transport}"); + await drv.InitializeAsync("{}", ct); + var initLastRead = drv.GetHealth().LastSuccessfulRead; + await Task.Delay(20, ct); + + for (var poll = 0; poll < 2; poll++) + (await drv.ReadAsync(["PastEnd"], ct))[0].StatusCode.ShouldNotBe(AbLegacyStatusMapper.Good); + + if (transport == AbLegacyTransport.Libplctag) + { + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + drv.GetHealth().LastSuccessfulRead.ShouldBe(initLastRead); + } + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyAnsweredStatusTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyAnsweredStatusTests.cs index 8cbaba2a..1b209131 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyAnsweredStatusTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyAnsweredStatusTests.cs @@ -7,18 +7,24 @@ using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; /// -/// Review 07 r0 I1: on the default libplctag transport a PLC's own refusal (a non-zero PCCC STS — e.g. an element -/// past the end of the file) arrives as ERR_REMOTE_ERR, which used to translate to BadConnection: a -/// no-reply comms fault. A coalesced block holding one such member then failed as a group every poll, and two -/// refused tags in a row tripped the device's fail-fast window. Statuses the device RETURNED are now answers; -/// only no-reply statuses are comms faults. +/// Pins BOTH sides of the reply-classification line (review 07 r0 I1 + r1 N1). +/// +/// No reply (Timeout / BadConnection / Abort / Pending / unknown) — comms fault: evict, Degraded, +/// counts toward fail-fast. +/// Answered (an allow-list: NotFound / NotAllowed / OutOfBounds / TooLarge / TooSmall / TypeMismatch / +/// DeviceStateConflict) — proves reachability: per-tag Bad, Healthy, LastSuccessfulRead advances. +/// Opaque (BadReply / Partial — incl. libplctag's ERR_REMOTE_ERR, which folds a PLC refusal +/// and a routing hop's "end device gone" together) — neither: per-tag Bad, never fail-fast, never blacks out +/// coalesced neighbours, but NEVER advances LastSuccessfulRead; an opaque-only device degrades so the +/// comms-loss watchdog can reach a dead PLC behind a bridge. +/// /// [Trait("Category", "Unit")] public sealed class AbLegacyAnsweredStatusTests { private const string Host = "ab://10.0.0.5/1,0"; - /// What the libplctag runtime's GetStatus() reports for a PLC refusal (through the real translation). + /// What the libplctag runtime's GetStatus() reports for ERR_REMOTE_ERR (through the real translation). private static readonly int RemoteErr = (int)LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorRemoteErr); private static AbLegacyDriver NewDriver(FakeAbLegacyTagFactory factory, bool coalesce, params AbLegacyTagDefinition[] tags) => @@ -30,37 +36,75 @@ public sealed class AbLegacyAnsweredStatusTests CoalesceReads = coalesce, }, "drv-i1", factory); - [Fact] - public void Remote_error_translates_to_an_answered_status_not_a_comms_fault() + // ---- the classification table ---- + + [Theory] + [InlineData(AbStatus.NotFound)] + [InlineData(AbStatus.NotAllowed)] + [InlineData(AbStatus.OutOfBounds)] + [InlineData(AbStatus.TooLarge)] + [InlineData(AbStatus.TooSmall)] + [InlineData(AbStatus.TypeMismatch)] + [InlineData(AbStatus.DeviceStateConflict)] + public void The_answered_class_is_an_allow_list(AbStatus status) { - var s = LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorRemoteErr); - AbLegacyDriver.IsNoReply(s).ShouldBeFalse(); - AbLegacyDriver.IsDeviceRefusal(s).ShouldBeTrue(); - AbLegacyDriver.IsDeviceRefusal(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorBadStatus)).ShouldBeTrue(); - // Positive control: real no-reply statuses still are. - AbLegacyDriver.IsNoReply(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorTimeout)).ShouldBeTrue(); - AbLegacyDriver.IsNoReply(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorBadConnection)).ShouldBeTrue(); + AbLegacyDriver.IsDeviceRefusal(status).ShouldBeTrue(); + AbLegacyDriver.IsOpaque(status).ShouldBeFalse(); + AbLegacyDriver.IsNoReply(status).ShouldBeFalse(); } [Theory] - [InlineData(AbStatus.BadReply)] // native: an STS the transport does not map + [InlineData(AbStatus.BadReply)] [InlineData(AbStatus.Partial)] - [InlineData(AbStatus.NotFound)] - [InlineData(AbStatus.DeviceStateConflict)] - public void Statuses_the_device_returned_are_answers(AbStatus status) => - AbLegacyDriver.IsDeviceRefusal(status).ShouldBeTrue(); + public void Unclassifiable_replies_are_opaque_not_answers(AbStatus status) + { + AbLegacyDriver.IsOpaque(status).ShouldBeTrue(); + AbLegacyDriver.IsDeviceRefusal(status).ShouldBeFalse(); + AbLegacyDriver.IsNoReply(status).ShouldBeFalse(); + } [Theory] [InlineData(AbStatus.Timeout)] [InlineData(AbStatus.BadConnection)] [InlineData(AbStatus.Abort)] + [InlineData(AbStatus.Pending)] [InlineData((AbStatus)(-99))] // a code this build does not know: conservatively no reply public void Only_no_reply_statuses_are_comms_faults(AbStatus status) { AbLegacyDriver.IsNoReply(status).ShouldBeTrue(); AbLegacyDriver.IsDeviceRefusal(status).ShouldBeFalse(); + AbLegacyDriver.IsOpaque(status).ShouldBeFalse(); } + [Fact] + public void Every_non_Ok_status_lands_in_at_most_one_class_and_Unsupported_in_none() + { + foreach (var s in Enum.GetValues().Where(s => s != AbStatus.Ok)) + { + var classes = new[] { AbLegacyDriver.IsNoReply(s), AbLegacyDriver.IsDeviceRefusal(s), AbLegacyDriver.IsOpaque(s) } + .Count(c => c); + if (s == AbStatus.Unsupported) classes.ShouldBe(0, "Unsupported is neutral"); + else classes.ShouldBe(1, s.ToString()); + } + } + + [Fact] + public void libplctag_remote_errors_translate_to_opaque_not_to_an_answer_or_a_fault() + { + foreach (var st in new[] { Status.ErrorRemoteErr, Status.ErrorBadStatus }) + { + var s = LibplctagLegacyStatusTranslation.ToAbStatus(st); + AbLegacyDriver.IsOpaque(s).ShouldBeTrue(st.ToString()); + AbLegacyDriver.IsDeviceRefusal(s).ShouldBeFalse(st.ToString()); + AbLegacyDriver.IsNoReply(s).ShouldBeFalse(st.ToString()); + } + // Positive controls on the other two sides. + AbLegacyDriver.IsNoReply(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorTimeout)).ShouldBeTrue(); + AbLegacyDriver.IsDeviceRefusal(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorNotFound)).ShouldBeTrue(); + } + + // ---- behaviour: the I1 side (an opaque member never blacks out its neighbours, never trips fail-fast) ---- + [Fact] public async Task A_remote_error_member_never_blacks_out_its_coalesced_neighbours() { @@ -82,15 +126,15 @@ public sealed class AbLegacyAnsweredStatusTests var r = await drv.ReadAsync(["A", "B", "C"], CancellationToken.None); r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}"); r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}"); - r[2].StatusCode.ShouldBe(AbLegacyStatusMapper.BadDeviceFailure, $"poll {poll}"); + r[2].StatusCode.ShouldNotBe(AbLegacyStatusMapper.Good, $"poll {poll}"); } - drv.GetHealth().State.ShouldBe(DriverState.Healthy); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); // the neighbours answered drv.GetDeviceState(Host)!.IsThrottled(DateTime.UtcNow).ShouldBeFalse(); await drv.ShutdownAsync(CancellationToken.None); } [Fact] - public async Task Consecutive_refused_tags_do_not_trip_fail_fast_on_an_answering_device() + public async Task Consecutive_opaque_replies_do_not_trip_fail_fast_on_an_answering_device() { var factory = new FakeAbLegacyTagFactory { @@ -106,10 +150,82 @@ public sealed class AbLegacyAnsweredStatusTests for (var poll = 0; poll < 3; poll++) { var r = await drv.ReadAsync(["R1", "R2", "R3", "Good"], CancellationToken.None); - r[3].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: an answering device is never failed fast"); + r[3].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: never failed fast"); } factory.Tags["N7:9"].ReadCount.ShouldBe(3); - factory.Tags["N7:0"].InitializeCount.ShouldBe(1); // refused handles are kept, not churned + factory.Tags["N7:0"].InitializeCount.ShouldBe(1); // opaque handles are kept, not churned + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + await drv.ShutdownAsync(CancellationToken.None); + } + + // ---- behaviour: the N1 side (an opaque reply never proves reachability) ---- + + [Fact] + public async Task Remote_error_only_reads_never_advance_LastSuccessfulRead_and_degrade() + { + // A PLC behind a bridge is powered off: every request comes back fast with ERR_REMOTE_ERR. + var factory = new FakeAbLegacyTagFactory { Customise = p => new FakeAbLegacyTag(p) { Value = 1 } }; + var drv = NewDriver(factory, coalesce: false, + new AbLegacyTagDefinition("X", Host, "N7:0", AbLegacyDataType.Int), + new AbLegacyTagDefinition("Y", Host, "N7:5", AbLegacyDataType.Int)); + await drv.InitializeAsync("{}", CancellationToken.None); + (await drv.ReadAsync(["X", "Y"], CancellationToken.None)).ShouldAllBe(s => s.StatusCode == AbLegacyStatusMapper.Good); + var healthyAt = drv.GetHealth().LastSuccessfulRead; // positive control: Good reads DID advance it + healthyAt.ShouldNotBeNull(); + foreach (var t in factory.Tags.Values) t.Status = RemoteErr; + await Task.Delay(5); + + for (var poll = 0; poll < 3; poll++) + (await drv.ReadAsync(["X", "Y"], CancellationToken.None)).ShouldAllBe(s => s.StatusCode == AbLegacyStatusMapper.BadCommunicationError); + + var h = drv.GetHealth(); + h.LastSuccessfulRead.ShouldBe(healthyAt); // frozen: the comms-loss watchdog can reach this device + h.State.ShouldBe(DriverState.Degraded); + drv.GetDeviceState(Host)!.IsThrottled(DateTime.UtcNow).ShouldBeFalse(); // but no fail-fast window + factory.Tags["N7:0"].ReadCount.ShouldBe(4); // every poll still asked + await drv.ShutdownAsync(CancellationToken.None); + } + + private sealed class ThrowingTag(AbLegacyTagCreateParams p, AbStatus status) : FakeAbLegacyTag(p) + { + public override Task ReadAsync(CancellationToken ct) => throw new AbTransportException(status, $"native {status}"); + } + + [Theory] + [InlineData(AbStatus.BadReply)] + [InlineData(AbStatus.Partial)] + public async Task A_native_unclassifiable_reply_is_not_counted_as_an_answer(AbStatus status) + { + var factory = new FakeAbLegacyTagFactory { Customise = p => new FakeAbLegacyTag(p) { Value = 1 } }; + var drv = NewDriver(factory, coalesce: false, new AbLegacyTagDefinition("X", Host, "N7:0", AbLegacyDataType.Int)); + await drv.InitializeAsync("{}", CancellationToken.None); + (await drv.ReadAsync(["X"], CancellationToken.None))[0].StatusCode.ShouldBe(AbLegacyStatusMapper.Good); + var healthyAt = drv.GetHealth().LastSuccessfulRead; + drv.GetDeviceState(Host)!.Runtimes.TryRemove("X", out _); + factory.Customise = p => new ThrowingTag(p, status); + await Task.Delay(5); + + var r = await drv.ReadAsync(["X"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.MapStatus(status)); + drv.GetHealth().LastSuccessfulRead.ShouldBe(healthyAt); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + await drv.ShutdownAsync(CancellationToken.None); + } + + [Fact] + public async Task Positive_control_a_real_device_refusal_alone_still_proves_reachability() + { + var factory = new FakeAbLegacyTagFactory { Customise = p => new FakeAbLegacyTag(p) { Status = (int)AbStatus.NotFound } }; + var drv = NewDriver(factory, coalesce: false, new AbLegacyTagDefinition("X", Host, "N7:0", AbLegacyDataType.Int)); + await drv.InitializeAsync("{}", CancellationToken.None); + var before = drv.GetHealth().LastSuccessfulRead; + await Task.Delay(5); + + (await drv.ReadAsync(["X"], CancellationToken.None))[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadNodeIdUnknown); + + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + drv.GetHealth().LastSuccessfulRead!.Value.ShouldBeGreaterThan(before!.Value); await drv.ShutdownAsync(CancellationToken.None); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyArrayWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyArrayWriteTests.cs index 20d9f203..f9a23fe3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyArrayWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyArrayWriteTests.cs @@ -123,6 +123,21 @@ public sealed class AbLegacyArrayWriteTests .ShouldContain(w => w.Contains("element width")); // and the deploy gate names it } + /// Review 07 r1 N5: the file letter of an indirect address is static, so the deploy gate checks it too. + [Theory] + [InlineData("N7:[N7:0]", "Float", true)] + [InlineData("F[N7:0]:2", "Int", true)] + [InlineData("N7:[N7:0]", "Int", false)] + [InlineData("F[N7:0]:2", "Float", false)] + public void An_indirect_array_type_file_mismatch_is_reported_at_the_gate(string address, string dataType, bool mismatch) + { + var json = $$"""{"address":"{{address}}","dataType":"{{dataType}}","isArray":true,"arrayLength":2}"""; + Enum.TryParse(dataType, out var type).ShouldBeTrue(); + + (AbLegacyTagDefinitionFactory.ArrayFileMismatch(type, address) is not null).ShouldBe(mismatch); + AbLegacyTagDefinitionFactory.Inspect(json).Any(w => w.Contains("element width")).ShouldBe(mismatch); + } + [Fact] public async Task Positive_control_a_Float_array_on_an_F_file_is_written() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHealthEvidenceTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHealthEvidenceTests.cs index c65004f0..e5e15fcf 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHealthEvidenceTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHealthEvidenceTests.cs @@ -243,8 +243,12 @@ public sealed class AbLegacyHealthEvidenceTests var gate = state.GetRuntimeLock("X"); await gate.WaitAsync(); - var read = drv.ReadAsync(["X"], CancellationToken.None); // fetches `first`, then parks on the gate - await Task.Delay(30); + // Signalled at exactly "handle fetched, about to wait on the gate" — no sleep, so the eviction below always + // lands between the reader's fetch and its use (review 07 r1 test nit: the sleep could pass vacuously). + var atGate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + state.BeforeOperationGate = key => { if (key == "X") atGate.TrySetResult(); }; + var read = drv.ReadAsync(["X"], CancellationToken.None); + await atGate.Task.WaitAsync(TimeSpan.FromSeconds(5)); state.Runtimes.TryRemove("X", out var evicted).ShouldBeTrue(); evicted!.Dispose(); gate.Release(); @@ -255,4 +259,92 @@ public sealed class AbLegacyHealthEvidenceTests drv.GetHealth().State.ShouldBe(DriverState.Healthy); await drv.ShutdownAsync(CancellationToken.None); } + + /// + /// Review 07 r1 N3: a reader parked on a resolved INDIRECT key's gate while the index sweep evicted that key and + /// forgot its gate used to RECONNECT it under the lock — resurrecting a stale resolution under a gate nobody + /// else would get. It now answers that one call neutrally and leaves the key gone. + /// + [Fact] + public async Task A_swept_indirect_resolution_is_never_resurrected_under_a_forgotten_gate() + { + var creates = new System.Collections.Concurrent.ConcurrentQueue(); + var f = new SwitchableFactory + { + Make = p => + { + creates.Enqueue(p.TagName); + return new FakeAbLegacyTag(p) { Value = p.TagName == "N7:0" ? (short)5 : (short)42 }; + }, + }; + var drv = NewDriver(f, new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int)); + await drv.InitializeAsync("{}", CancellationToken.None); + (await drv.ReadAsync(["Ind"], CancellationToken.None))[0].Value.ShouldBe(42); // positive control + var state = drv.GetDeviceState(Host)!; + const string key = "Ind=>N7:5"; + state.Runtimes.ContainsKey(key).ShouldBeTrue(); + + var gate = state.GetRuntimeLock(key); + await gate.WaitAsync(); + var atGate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + state.BeforeOperationGate = k => { if (k == key) atGate.TrySetResult(); }; + var read = drv.ReadAsync(["Ind"], CancellationToken.None); + await atGate.Task.WaitAsync(TimeSpan.FromSeconds(5)); + // What the sweep does when the index moves: evict the resolution, then forget its gate. + state.Runtimes.TryRemove(key, out var swept).ShouldBeTrue(); + swept!.Dispose(); + state.ForgetGates(key); + gate.Release(); + var r = await read; + + r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadCommunicationError); + state.Runtimes.ContainsKey(key).ShouldBeFalse(); // not resurrected + creates.Count(n => n == "N7:5").ShouldBe(1); // no reconnect of the stale resolution + drv.GetHealth().State.ShouldBe(DriverState.Healthy); // neutral: no health evidence either way + await drv.ShutdownAsync(CancellationToken.None); + } + + /// Review 07 r1 N4: an index read answered Unsupported is neutral, not a transport fault. + [Fact] + public async Task An_index_read_answered_Unsupported_is_neutral() + { + var f = new SwitchableFactory + { + Make = p => p.TagName == "N7:0" + ? new FakeAbLegacyTag(p) { Status = (int)AbStatus.Unsupported } + : new FakeAbLegacyTag(p) { Value = 1 }, + }; + var drv = NewDriver(f, + new AbLegacyTagDefinition("Ind", Host, "N7:[N7:0]", AbLegacyDataType.Int), + new AbLegacyTagDefinition("Plain", Host, "N9:0", AbLegacyDataType.Int)); + await drv.InitializeAsync("{}", CancellationToken.None); + + var r = await drv.ReadAsync(["Ind", "Plain"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadNotSupported); + r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good); // positive control + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + drv.GetDeviceState(Host)!.ParentRuntimes.ContainsKey("N7:0").ShouldBeTrue(); // handle kept, not evicted + await drv.ShutdownAsync(CancellationToken.None); + } + + /// Review 07 r1 N2: a persisted mismatched array READS BadTypeMismatch instead of garbage as Good. + [Fact] + public async Task A_mismatched_array_reads_BadTypeMismatch_not_garbage() + { + var f = new SwitchableFactory { Make = p => new FakeAbLegacyTag(p) { ArrayValue = new[] { 1f, 2f } } }; + var drv = NewDriver(f, + new AbLegacyTagDefinition("Bad", Host, "F8:0", AbLegacyDataType.Int, ArrayLength: 2), + new AbLegacyTagDefinition("Good", Host, "F9:0", AbLegacyDataType.Float, ArrayLength: 2)); + await drv.InitializeAsync("{}", CancellationToken.None); + + var r = await drv.ReadAsync(["Bad", "Good"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadTypeMismatch); + f.Tags.ContainsKey("F8:0").ShouldBeFalse(); // nothing read at the wrong width + r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good); // positive control + r[1].Value.ShouldBe(new[] { 1f, 2f }); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + await drv.ShutdownAsync(CancellationToken.None); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHostAndStatusTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHostAndStatusTests.cs index 79cd5d80..8338dc3b 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHostAndStatusTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHostAndStatusTests.cs @@ -89,9 +89,6 @@ public sealed class AbLegacyHostAndStatusTests Status.ErrorNotAllowed => AbLegacyStatusMapper.BadNotWritable, Status.ErrorOutOfBounds or Status.ErrorTooLarge or Status.ErrorTooSmall => AbLegacyStatusMapper.BadOutOfRange, Status.ErrorUnsupported or Status.ErrorNotImplemented => AbLegacyStatusMapper.BadNotSupported, - // DELIBERATE change (review 07 r0 I1): a PLC's own refusal (PCCC STS / CIP status → ERR_REMOTE_ERR / - // ERR_BAD_STATUS) is "device answered", not a comms error. - Status.ErrorRemoteErr or Status.ErrorBadStatus => AbLegacyStatusMapper.BadDeviceFailure, _ => AbLegacyStatusMapper.BadCommunicationError, };