From a1f20e78f905bd0b5411847e8c71b508366967d8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 15:20:51 -0400 Subject: [PATCH] fix(s7): a CPU refusal is per-tag Bad and counts as answered (R07:H4) A deterministic PLC refusal (object does not exist, address out of range, type inconsistent...) degraded driver health, and an all-refused driver (a wrong DB number) sat Degraded with LastSuccessfulRead frozen, so the 90 s comms-loss watchdog reconnected it forever against a reachable PLC. - IsRefusal / IsAccessDenied classify over the whole exception chain. Live probe on the snap7 fixture: S7.Net's READ paths raise the item error as a BARE Exception ("Received error from PLC: ..."), its write paths as PlcException(WriteData). The old PlcException-only match missed the read shape entirely, so a real read access-denied was BadCommunicationError + Degraded instead of the Faulted PUT/GET alert; the Test Connect probe had the same blind spot. - A refusal (read or write) is BadDeviceFailure with a once-per-tag Warning, no health write and no reopen; it counts as answered, so it advances LastSuccessfulRead and restores Healthy after a comms fault. An undecodable buffer (S7ValueRangeException) also counts as answered. - A write batch the CPU answered without a failure advances LastSuccessfulRead and retires a Degraded the write side itself published. - Tests that used ReadData as their generic "device fault" now use SendData so their Degraded leg is still exercised. Refs #742 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- docs/drivers/S7.md | 15 +- .../ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs | 192 ++++++++++++++--- .../S7DriverProbe.cs | 10 +- .../S7_1500/S7_1500RefusalTests.cs | 83 ++++++++ .../CapturingS7Logger.cs | 32 +++ .../S7RefusalHealthTests.cs | 193 ++++++++++++++++++ .../S7SpanReadParityTests.cs | 22 +- .../S7TsapAndPduTests.cs | 17 ++ 8 files changed, 515 insertions(+), 49 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500RefusalTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/CapturingS7Logger.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7RefusalHealthTests.cs diff --git a/docs/drivers/S7.md b/docs/drivers/S7.md index 18e07821..448c7689 100644 --- a/docs/drivers/S7.md +++ b/docs/drivers/S7.md @@ -312,7 +312,8 @@ Portal under *Protection & Security* for the CPU. | Read-only tag written | `BadNotWritable` | unchanged | | Unimplemented data type | `BadNotSupported` | unchanged | | PUT/GET denied | `BadNotSupported` | `Faulted` (config alert) | -| CPU / hardware fault | `BadDeviceFailure` | `Degraded` | +| CPU refusal of the item (object does not exist, address out of range, type inconsistent, hardware fault) | `BadDeviceFailure` | unchanged — counts as *answered*, advances `LastSuccessfulRead`; one Warning per tag | +| Other device-layer fault (`PlcException` that is neither a refusal nor connection-fatal) | `BadDeviceFailure` | `Degraded` | | Socket / timeout | `BadCommunicationError` | `Degraded` | | Reconnect suppressed (backoff window) | `BadCommunicationError` | `Degraded` | | Array write of the wrong shape / length / element type | `BadTypeMismatch` | unchanged | @@ -323,7 +324,17 @@ accumulate their outcomes and write health once (`Healthy < Degraded < Faulted`) `Faulted` item is not erased by a later `Degraded` or successful sibling. The write-side `Faulted` is **sticky against reads**: a clean read pass (reads often work against a write-protected CPU) leaves `Faulted` in place and only advances `LastSuccessfulRead`; the next write that succeeds clears it. A -transient `Degraded` write failure is still cleared by a clean read, as before. +transient `Degraded` write failure is cleared by a clean read, or by the next write batch the CPU +answers without a failure. + +**A refusal is an answer (R07:H4).** S7.Net's read paths raise the CPU's item error as a bare +`Exception` ("Received error from PLC: Object does not exist." — live-probed on the snap7 fixture), its +write paths as a `PlcException(WriteData)`; both are classified over the whole exception chain, and the +access-denied text inside either is the `Faulted` alert. A refusal is deterministic, so it neither +degrades health nor reopens the connection, and because it proves the CPU is reachable it advances +`LastSuccessfulRead` and restores `Healthy` after an earlier comms fault. Before this, a driver whose +every tag was refused (a wrong DB number) sat `Degraded` with a frozen timestamp and the comms-loss +watchdog reconnected it every 90 s against a perfectly reachable PLC. **Connect backoff.** A dead PLC costs one connect attempt per backoff window (1 s doubling to a 30 s cap), not one per read / poll tick / probe tick: inside the window `EnsureConnectedAsync` fails fast 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 acade74e..f1136908 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs @@ -208,6 +208,19 @@ public sealed class S7Driver /// Guarded by . /// private (DriverState State, string Message)? _writeFault; + + /// + /// The message of the last Degraded the WRITE side published, or null. A clean write batch + /// retires a Degraded carrying exactly this message (a good write proves the comms fault it + /// reported is gone) and leaves any other Degraded to the read pass. Guarded by . + /// + private string? _writeOwnedError; + + /// + /// Tags whose refusal has already been logged this generation — the once-per-tag Warning. Cleared + /// by every initialize publish. + /// + private readonly ConcurrentDictionary _refusalWarned = new(StringComparer.Ordinal); private bool _disposed; /// @@ -797,7 +810,7 @@ public sealed class S7Driver } catch (Exception ex) { - results[i] = new DataValueSnapshot(null, RecordReadFailure(ex, outcome), null, now); + results[i] = new DataValueSnapshot(null, RecordReadFailure(ex, outcome, [resolved[i]!.Name]), null, now); } } } @@ -903,6 +916,13 @@ public sealed class S7Driver /// Records that one tag read successfully. public void RecordSuccess() => AnySuccess = true; + /// + /// Records that the CPU answered for one tag without serving it (a refusal, an undecodable + /// buffer). Not a failure — it ranks nothing — but it IS proof of a live device, so it + /// advances LastSuccessfulRead exactly like a success. + /// + public void RecordAnswered() => AnySuccess = true; + /// /// Records one failed tag, keeping the worst state and the message that explains it. /// @@ -1098,7 +1118,7 @@ public sealed class S7Driver { // Decoding is pure and synchronous — no cancellation can surface here, so there // is deliberately no OperationCanceledException arm to shadow the request one. - results[index] = new DataValueSnapshot(null, RecordReadFailure(ex, outcome), null, now); + results[index] = new DataValueSnapshot(null, RecordReadFailure(ex, outcome, [tag.Name]), null, now); } } } @@ -1195,7 +1215,7 @@ public sealed class S7Driver } catch (Exception ex) { - var status = RecordReadFailure(ex, outcome); + var status = RecordReadFailure(ex, outcome, span.Members.Select(m => m.Tag.Name)); foreach (var member in span.Members) results[member.OriginalIndex] = new DataValueSnapshot(null, status, null, now); return; @@ -1214,7 +1234,7 @@ public sealed class S7Driver // Decoding is pure and synchronous — no cancellation can surface here, so there is // deliberately no OperationCanceledException arm to shadow the block-read one above. results[member.OriginalIndex] = - new DataValueSnapshot(null, RecordReadFailure(ex, outcome), null, now); + new DataValueSnapshot(null, RecordReadFailure(ex, outcome, [member.Tag.Name]), null, now); } } } @@ -1289,8 +1309,10 @@ public sealed class S7Driver /// /// The exception observed on a read. /// The pass's outcome accumulator this failure is recorded into. + /// The tags the failure belongs to (every member of a failed span), for the + /// once-per-tag refusal Warning. /// The OPC UA status code every affected tag must be stamped with. - private uint RecordReadFailure(Exception ex, ReadPassOutcome outcome) + private uint RecordReadFailure(Exception ex, ReadPassOutcome outcome, IEnumerable tagNames) { switch (ex) { @@ -1299,19 +1321,33 @@ public sealed class S7Driver case S7ValueRangeException: // A PLC buffer the type codec refuses (uninitialised DTL, non-BCD S5TIME…) is a data - // problem, not a transport fault — no health change, no connection teardown. + // problem, not a transport fault — no health change, no connection teardown. The + // bytes came off the wire, so the CPU answered. + outcome.RecordAnswered(); return StatusBadOutOfRange; - case PlcException pex when IsAccessDenied(pex): + case var _ when IsAccessDenied(ex): // PUT/GET-disabled (S7-1200/1500) / access-protection — a permanent configuration // fault, NOT a transient one. Blind retry is wasted effort, so map it to // BadNotSupported and flag the driver as a config alert (Faulted) rather than - // Degraded — per driver-specs.md §5. + // Degraded — per driver-specs.md §5. Matched over the whole chain: S7.Net's READ + // paths raise the CPU's item error as a bare Exception, not a PlcException. outcome.RecordFailure(DriverState.Faulted, "S7 access denied — enable PUT/GET communication in TIA Portal " + - $"(Protection & Security) for this CPU. PLC reported: {pex.Message}"); + $"(Protection & Security) for this CPU. PLC reported: {ex.Message}"); return StatusBadNotSupported; + case var _ when IsRefusal(ex): + // The CPU answered and refused the item (object does not exist, address out of + // range, type inconsistent...) — deterministic, so a retry, a reopen or a Degraded + // would all be wrong: per-tag Bad, a once-per-tag Warning, no health write, and it + // counts as answered so LastSuccessfulRead advances (R07:H4). An all-refused driver + // (wrong DB number) used to sit Degraded with a frozen timestamp and trip the + // comms-loss watchdog every 90 s against a reachable PLC. + outcome.RecordAnswered(); + foreach (var name in tagNames) WarnRefusalOnce(name, "read", ex); + return StatusBadDeviceFailure; + case PlcException pex: // A genuine device-layer fault (CPU error, hardware fault) — transient enough to // keep retrying; report BadDeviceFailure and degrade health. A connection-fatal @@ -1969,7 +2005,7 @@ public sealed class S7Driver // Client-supplied value of the wrong shape — not a device or comms fault. results[i] = new WriteResult(StatusBadTypeMismatch); } - catch (PlcException pex) when (IsAccessDenied(pex)) + catch (Exception ex) when (IsAccessDenied(ex)) { // PUT/GET-disabled / access-protection on write — same permanent // configuration fault as on read. BadNotSupported + @@ -1977,7 +2013,15 @@ public sealed class S7Driver results[i] = new WriteResult(StatusBadNotSupported); outcome.RecordFailure(DriverState.Faulted, "S7 access denied — enable PUT/GET communication in TIA Portal " + - $"(Protection & Security) for this CPU. PLC reported: {pex.Message}"); + $"(Protection & Security) for this CPU. PLC reported: {ex.Message}"); + } + catch (Exception ex) when (IsRefusal(ex)) + { + // The CPU answered and refused the item — per-tag Bad, once-per-tag Warning, no + // Degraded, and it counts as answered (R07:H4). + results[i] = new WriteResult(StatusBadDeviceFailure); + outcome.RecordAnswered(); + WarnRefusalOnce(tag.Name, "write", ex); } catch (PlcException pex) { @@ -2029,16 +2073,24 @@ public sealed class S7Driver else if (outcome.AnySuccess) _writeFault = null; // a write got through — the protection fault is gone + var answeredAt = outcome.AnyAnswered ? DateTime.UtcNow : (DateTime?)null; if (worst == DriverState.Healthy) { - // Success-only batch: no health write, except to retire a Faulted that WE (the write - // side) were holding — a clean write is the only evidence the alert has cleared. - // Only a Faulted the write side owns: a read-side Faulted (e.g. access denied on another DB) is the - // read pass's to clear, and Health still carries the write fault's own message iff it is ours. - if (outcome.AnySuccess && _writeFault is null && priorWriteFault is { } owned) - TryUpdateHealth(gen, h => h.State == DriverState.Faulted && h.LastError == owned.Message - ? new DriverHealth(DriverState.Healthy, h.LastSuccessfulRead, null) - : null); + // Clean batch (successes and/or refusals): the CPU answered, so LastSuccessfulRead + // advances, and a non-Healthy state the WRITE side owns is retired — a Faulted only by a + // real success (a refusal does not prove PUT/GET is enabled), a write-side comms Degraded + // by any answer. A state the read side owns (its own message) is the read pass's to clear. + var ownedFault = outcome.AnySuccess && _writeFault is null ? priorWriteFault?.Message : null; + var ownedDegraded = _writeOwnedError; + _writeOwnedError = null; + TryUpdateHealth(gen, h => + { + var lsr = answeredAt ?? h.LastSuccessfulRead; + if ((h.State == DriverState.Faulted && ownedFault is not null && h.LastError == ownedFault) + || (h.State == DriverState.Degraded && ownedDegraded is not null && h.LastError == ownedDegraded)) + return new DriverHealth(DriverState.Healthy, lsr, null); + return lsr == h.LastSuccessfulRead ? null : h with { LastSuccessfulRead = lsr }; + }); return; } @@ -2050,7 +2102,8 @@ public sealed class S7Driver state = wf.State; error = wf.Message; } - TryUpdateHealth(gen, h => new DriverHealth(state, h.LastSuccessfulRead, error)); + if (state == DriverState.Degraded) _writeOwnedError = error; + TryUpdateHealth(gen, h => new DriverHealth(state, answeredAt ?? h.LastSuccessfulRead, error)); } /// @@ -2068,10 +2121,18 @@ public sealed class S7Driver public DriverState WorstFailure => _inner.WorstFailure; /// The message explaining . public string? WorstError => _inner.WorstError; - /// True once the batch observed a success or a failure. - public bool HasObservations => _inner.HasObservations; + /// True once the batch observed a success, a refusal or a failure. + public bool HasObservations => _inner.HasObservations || AnyAnswered; + /// True once the CPU answered any item (a success or a refusal) — proof of a live device. + public bool AnyAnswered { get; private set; } /// Records one successful write. - public void RecordSuccess() => _inner.RecordSuccess(); + public void RecordSuccess() + { + _inner.RecordSuccess(); + AnyAnswered = true; + } + /// Records a write the CPU answered by refusing it: not a success, not a failure. + public void RecordAnswered() => AnyAnswered = true; /// Records one failed write. /// The state the failure would have stamped alone. /// The operator-facing message. @@ -2407,19 +2468,82 @@ public sealed class S7Driver } /// - /// Detects an S7 PUT/GET-disabled / access-protection fault inside an S7.Net - /// . S7.Net's read/write paths wrap every PLC-side error in a - /// PlcException with / ; - /// the response-code validator throws a plain for the S7 - /// AccessingObjectNotAllowed status, which lands as the inner exception. There is - /// no typed error code for it, so the inner message is the only discriminator - /// S7.Net exposes. + /// True when is a deterministic PLC REFUSAL of the item: the CPU answered + /// with an S7 item error (object does not exist, address out of range, data type inconsistent / + /// not supported, hardware fault, or an unknown status), or S7.Net raised a + /// / over a response it could + /// not use. Excludes the access-denied refusal (its own sticky Faulted config alert) and + /// anything carrying a socket / timeout / framing / cancellation fault in its chain — those + /// wrap a transport failure, not an answer. /// - /// The exception raised by S7.Net. - /// when the PLC refused access to the object. - internal static bool IsAccessDenied(PlcException pex) + /// + /// Live-probed against the snap7 fixture (2026-10-22): S7.Net's READ paths + /// (ReadAsync(string), ReadBytesAsync) raise the item error as a BARE + /// — "Received error from PLC: Object does not exist." — not wrapped in + /// a ; its write paths wrap whatever they hit in a + /// PlcException(WriteData). Both shapes are matched. + /// + /// The exception raised by S7.Net. + /// when the CPU answered and refused. + internal static bool IsRefusal(Exception ex) { - for (Exception? e = pex; e is not null; e = e.InnerException) + if (IsAccessDenied(ex) || IsS7ConnectionFatal(ex) || HasCancellationInChain(ex)) return false; + if (ex is PlcException { ErrorCode: ErrorCode.ReadData or ErrorCode.WriteData }) return true; + for (Exception? e = ex; e is not null; e = e.InnerException) + if (IsS7ItemError(e)) return true; + return false; + } + + /// + /// True for the bare S7.Net's response-code validator raises for an S7 + /// item error ("Received error from PLC: …" / "Invalid response from PLC: statusCode=…"). + /// Exact type only: a derived exception carrying similar words is not S7.Net's validator. + /// + /// One exception of a chain. + /// for the validator's item-error shape. + private static bool IsS7ItemError(Exception e) + => e.GetType() == typeof(Exception) + && (e.Message.StartsWith("Received error from PLC", StringComparison.Ordinal) + || e.Message.StartsWith("Invalid response from PLC", StringComparison.Ordinal)); + + /// True when a cancellation is buried in the exception chain (a half-read PDU, not a refusal). + /// The exception to inspect. + /// when any inner exception is an . + private static bool HasCancellationInChain(Exception ex) + { + for (var e = ex.InnerException; e is not null; e = e.InnerException) + if (e is OperationCanceledException) return true; + return false; + } + + /// Logs a refusal once per tag per generation. + /// The refused tag. + /// "read" or "write". + /// The refusal. + private void WarnRefusalOnce(string tagName, string operation, Exception ex) + { + if (!_refusalWarned.TryAdd(tagName, 0)) return; + _logger.LogWarning( + "S7 CPU refused the {Operation} of tag {Tag} ({Reason}); it reads Bad until the tag or the PLC program is fixed. " + + "Driver health is unaffected. Further refusals of this tag are not logged. Driver={DriverInstanceId}", + operation, tagName, ex.InnerException?.Message ?? ex.Message, _driverInstanceId); + } + + /// + /// Detects an S7 PUT/GET-disabled / access-protection fault. S7.Net's response-code validator + /// throws a plain for the S7 AccessingObjectNotAllowed status — + /// bare on the read paths, wrapped in a () + /// on the write paths. There is no typed error code for it, so the message is the only + /// discriminator S7.Net exposes. The top-level exception must be S7.Net-shaped (a + /// or the validator's bare item error), so an unrelated exception that + /// happens to say "not allowed" is not mistaken for it. + /// + /// The exception raised by S7.Net. + /// when the PLC refused access to the object. + internal static bool IsAccessDenied(Exception ex) + { + if (ex is not PlcException && !IsS7ItemError(ex)) return false; + for (Exception? e = ex; e is not null; e = e.InnerException) { if (e.Message.Contains("Accessing object not allowed", StringComparison.OrdinalIgnoreCase) || e.Message.Contains("not allowed", StringComparison.OrdinalIgnoreCase)) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverProbe.cs index d936b98a..13925989 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverProbe.cs @@ -128,15 +128,17 @@ public sealed class S7DriverProbe : IDriverProbe return null; } catch (OperationCanceledException) { throw; } - catch (PlcException pex) when (S7Driver.IsAccessDenied(pex)) + // S7.Net's read paths raise the CPU's item error as a BARE Exception ("Received error from + // PLC: …"), not a PlcException, so both classifiers walk the whole chain (the driver's own). + catch (Exception ex) when (S7Driver.IsAccessDenied(ex)) { return new(false, "S7 connected but data access is denied — enable PUT/GET communication in TIA Portal " + - $"(Protection & Security) for this CPU. PLC reported: {pex.Message}", null); + $"(Protection & Security) for this CPU. PLC reported: {ex.Message}", null); } - catch (PlcException pex) when (pex.ErrorCode == ErrorCode.ReadData) + catch (Exception ex) when (S7Driver.IsRefusal(ex)) { - return null; // address not available on this CPU — not a connection problem + return null; // address not available on this CPU — the CPU answered; not a connection problem } catch (Exception ex) { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500RefusalTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500RefusalTests.cs new file mode 100644 index 00000000..709be4c2 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500RefusalTests.cs @@ -0,0 +1,83 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests.S7_1500; + +/// +/// Live gate for R07:H4 — a real CPU refusal over real S7comm. The snap7 server has no DB99, so it +/// answers a read of DB99.DBW0 with an S7 item error. That is a per-tag Bad with no health +/// impact (and LastSuccessfulRead advances), not a Degraded that would trip the comms-loss watchdog +/// against a reachable PLC. +/// +[Collection(Snap7ServerCollection.Name)] +[Trait("Category", "Integration")] +[Trait("Device", "S7_1500")] +public sealed class S7_1500RefusalTests(Snap7ServerFixture sim) +{ + /// A refused DB read is per-tag Bad, keeps the driver Healthy, and its neighbour reads Good. + [Fact] + public async Task A_read_of_a_missing_DB_is_refused_per_tag_without_degrading() + { + if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + var ct = TestContext.Current.CancellationToken; + + var baseOptions = S7_1500Profile.BuildOptions(sim.Host, sim.Port); + var options = new S7DriverOptions + { + Host = baseOptions.Host, Port = baseOptions.Port, CpuType = baseOptions.CpuType, + Rack = baseOptions.Rack, Slot = baseOptions.Slot, Timeout = baseOptions.Timeout, + Probe = baseOptions.Probe, + RawTags = [.. baseOptions.RawTags, .. S7RawTags.Entries( + new S7TagDefinition("Missing/Db99", "DB99.DBW0", S7DataType.UInt16, Writable: false))], + }; + await using var drv = new S7Driver(options, driverInstanceId: "s7-refusal-live"); + await drv.InitializeAsync("{}", ct); + var before = drv.GetHealth().LastSuccessfulRead!.Value; + + // Read the refused tag ALONE first: an all-refused pass is exactly the case that used to + // freeze LastSuccessfulRead and latch Degraded. + var refused = await drv.ReadAsync(["Missing/Db99"], ct); + refused[0].StatusCode.ShouldNotBe(0u); + refused[0].Value.ShouldBeNull(); + var health = drv.GetHealth(); + health.State.ShouldBe(DriverState.Healthy, "a CPU refusal is not a comms fault"); + health.LastSuccessfulRead!.Value.ShouldBeGreaterThanOrEqualTo(before); + + // Positive control: the same connection still serves the seeded neighbour. + var good = await drv.ReadAsync([S7_1500Profile.ProbeTag], ct); + good[0].StatusCode.ShouldBe(0u); + Convert.ToInt32(good[0].Value).ShouldBe(S7_1500Profile.ProbeSeedValue); + } + + /// A refused write is per-tag Bad and keeps the driver Healthy; a good write still lands. + [Fact] + public async Task A_write_to_a_missing_DB_is_refused_per_tag_without_degrading() + { + if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + var ct = TestContext.Current.CancellationToken; + + var baseOptions = S7_1500Profile.BuildOptions(sim.Host, sim.Port); + var options = new S7DriverOptions + { + Host = baseOptions.Host, Port = baseOptions.Port, CpuType = baseOptions.CpuType, + Rack = baseOptions.Rack, Slot = baseOptions.Slot, Timeout = baseOptions.Timeout, + Probe = baseOptions.Probe, + RawTags = [.. baseOptions.RawTags, .. S7RawTags.Entries( + new S7TagDefinition("Missing/Db99W", "DB99.DBW0", S7DataType.UInt16, Writable: true))], + }; + await using var drv = new S7Driver(options, driverInstanceId: "s7-refusal-write-live"); + await drv.InitializeAsync("{}", ct); + + var refused = await drv.WriteAsync([new WriteRequest("Missing/Db99W", (ushort)5)], ct); + refused[0].StatusCode.ShouldNotBe(0u); + drv.GetHealth().State.ShouldBe(DriverState.Healthy, "a CPU write refusal is not a comms fault"); + + // Positive control: a write the CPU accepts still lands (and reads back). + var ok = await drv.WriteAsync([new WriteRequest(S7_1500Profile.WriteScratchTag, (ushort)4711)], ct); + ok[0].StatusCode.ShouldBe(0u); + var back = await drv.ReadAsync([S7_1500Profile.WriteScratchTag], ct); + Convert.ToInt32(back[0].Value).ShouldBe(4711); + await drv.WriteAsync([new WriteRequest(S7_1500Profile.WriteScratchTag, (ushort)0)], ct); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/CapturingS7Logger.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/CapturingS7Logger.cs new file mode 100644 index 00000000..c2664116 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/CapturingS7Logger.cs @@ -0,0 +1,32 @@ +using Microsoft.Extensions.Logging; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// An that records (level, message) pairs. +internal sealed class CapturingS7Logger : ILogger +{ + private readonly List<(LogLevel Level, string Message)> _entries = []; + + /// A snapshot of every entry logged so far. + public IReadOnlyList<(LogLevel Level, string Message)> Entries + { + get { lock (_entries) return [.. _entries]; } + } + + /// The Warning messages logged so far. + public IReadOnlyList Warnings + => [.. Entries.Where(e => e.Level == LogLevel.Warning).Select(e => e.Message)]; + + /// + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + /// + public bool IsEnabled(LogLevel logLevel) => true; + + /// + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + { + lock (_entries) _entries.Add((logLevel, formatter(state, exception))); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7RefusalHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7RefusalHealthTests.cs new file mode 100644 index 00000000..06aa3277 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7RefusalHealthTests.cs @@ -0,0 +1,193 @@ +using S7.Net; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// R07:H4 — a deterministic PLC refusal (S7.Net ReadData / WriteData carrying an S7 +/// item error such as "object does not exist", not a socket/timeout/framing fault) proves the CPU +/// answered. It is per-tag Bad with a once-per-tag Warning and NO Degraded; it advances +/// LastSuccessfulRead, so a driver whose every tag is refused (wrong DB number) no longer +/// trips the 90 s comms-loss watchdog against a reachable PLC. +/// +[Trait("Category", "Unit")] +public sealed class S7RefusalHealthTests +{ + private const uint BadDeviceFailure = 0x808B0000u; + + /// + /// The shape S7.Net's READ paths really raise for a CPU item error — a BARE + /// , live-probed against the snap7 fixture (not a PlcException). + /// + private static Exception ReadRefusal() => new("Received error from PLC: Object does not exist."); + + private static PlcException WriteRefusal() => + new(ErrorCode.WriteData, new Exception("Received error from PLC: Address out of range.")); + + private static S7DriverOptions Options() => new() + { + Host = "192.0.2.1", + Timeout = TimeSpan.FromSeconds(5), + Probe = new S7ProbeOptions { Enabled = false }, + // Grouping off: one wire request per tag keeps each tag's outcome its own. + SpanReadMaxGapBytes = -1, + RawTags = S7RawTags.Entries( + new S7TagDefinition("Missing", "DB99.DBW0", S7DataType.UInt16), + new S7TagDefinition("Good", "DB1.DBW0", S7DataType.UInt16)), + }; + + private static async Task<(S7Driver Driver, ScriptedS7PlcFactory Factory, CapturingS7Logger Log)> StartAsync() + { + var factory = new ScriptedS7PlcFactory(); + var log = new CapturingS7Logger(); + var drv = new S7Driver(Options(), "s7-refusal", factory, log); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + return (drv, factory, log); + } + + /// A refused tag is Bad, health stays Healthy, LastSuccessfulRead advances, and it warns once. + [Fact] + public async Task A_read_refusal_is_per_tag_bad_with_no_health_impact_and_one_warning() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory, log) = await StartAsync(); + using var _ = drv; + factory.Read = (addr, _) => addr == "DB99.DBW0" ? throw ReadRefusal() : Task.FromResult((ushort)7); + + var before = drv.GetHealth().LastSuccessfulRead!.Value; + for (var pass = 0; pass < 3; pass++) + { + var r = await drv.ReadAsync(["Missing"], ct); + r[0].StatusCode.ShouldBe(BadDeviceFailure); + drv.GetHealth().State.ShouldBe(DriverState.Healthy, "a refusal is not a comms fault"); + } + + drv.GetHealth().LastSuccessfulRead!.Value.ShouldBeGreaterThanOrEqualTo(before, + "the CPU answered, so the all-refused pass still advances LastSuccessfulRead"); + log.Warnings.Count(w => w.Contains("Missing")).ShouldBe(1, "one Warning per refused tag, not one per poll"); + + // Positive control: the good tag still reads, and nothing reopened the connection. + (await drv.ReadAsync(["Good"], ct))[0].StatusCode.ShouldBe(0u); + factory.Created.Count.ShouldBe(1); + } + + /// + /// A refusal after a comms fault restores Healthy — the CPU is back and answering. Before the + /// fix the refusal latched Degraded with LastSuccessfulRead frozen, which is exactly the + /// watchdog's trip condition. + /// + [Fact] + public async Task A_refusal_after_a_comms_fault_restores_healthy() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory, _) = await StartAsync(); + using var __ = drv; + + // Positive control: a genuine comms fault DOES degrade. + factory.Read = (_, _) => throw new TimeoutException("frozen peer"); + await drv.ReadAsync(["Missing"], ct); + var degraded = drv.GetHealth(); + degraded.State.ShouldBe(DriverState.Degraded); + + factory.Read = (addr, _) => addr == "DB99.DBW0" ? throw ReadRefusal() : Task.FromResult((ushort)7); + var r = await drv.ReadAsync(["Missing"], ct); + r[0].StatusCode.ShouldBe(BadDeviceFailure); + var after = drv.GetHealth(); + after.State.ShouldBe(DriverState.Healthy); + after.LastSuccessfulRead!.Value.ShouldBeGreaterThanOrEqualTo(degraded.LastSuccessfulRead!.Value); + } + + /// The ReadData-wrapped shape of a refusal is classified the same way. + [Fact] + public async Task A_read_data_wrapped_refusal_is_also_a_refusal() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory, _) = await StartAsync(); + using var __ = drv; + factory.Read = (_, _) => throw new PlcException(ErrorCode.ReadData, ReadRefusal()); + + (await drv.ReadAsync(["Missing"], ct))[0].StatusCode.ShouldBe(BadDeviceFailure); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// The bare read-path shape of access denied (PUT/GET disabled) is the Faulted config alert. + /// Before, only a PlcException-wrapped "not allowed" was recognised, so a real CPU's read + /// refusal fell through to BadCommunicationError + Degraded. + /// + [Fact] + public async Task A_bare_access_denied_read_is_the_faulted_config_alert() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory, _) = await StartAsync(); + using var __ = drv; + factory.Read = (_, _) => throw new Exception("Received error from PLC: Accessing object not allowed."); + + (await drv.ReadAsync(["Good"], ct))[0].StatusCode.ShouldBe(0x803D0000u); // BadNotSupported + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + drv.GetHealth().LastError!.ShouldContain("PUT/GET"); + } + + /// A ReadData wrapping a socket fault is still a comms fault (Degraded), not a refusal. + [Fact] + public async Task A_read_data_wrapping_a_socket_fault_still_degrades() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory, _) = await StartAsync(); + using var __ = drv; + factory.Read = (_, _) => throw new PlcException(ErrorCode.ReadData, new System.IO.IOException("connection reset")); + + await drv.ReadAsync(["Good"], ct); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + } + + /// A write refusal is per-tag Bad, counts as answered and leaves health alone. + [Fact] + public async Task A_write_refusal_is_per_tag_bad_and_counts_as_answered() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory, log) = await StartAsync(); + using var _ = drv; + factory.Write = (addr, _, _) => addr == "DB99.DBW0" ? throw WriteRefusal() : Task.CompletedTask; + + // Put the driver into a comms-fault Degraded first; the refusal must lift it. + factory.Read = (_, _) => throw new TimeoutException("frozen peer"); + await drv.ReadAsync(["Good"], ct); + var degraded = drv.GetHealth(); + degraded.State.ShouldBe(DriverState.Degraded); + + for (var i = 0; i < 2; i++) + { + var w = await drv.WriteAsync([new WriteRequest("Missing", (ushort)1)], ct); + w[0].StatusCode.ShouldBe(BadDeviceFailure); + } + + var after = drv.GetHealth(); + after.LastSuccessfulRead!.Value.ShouldBeGreaterThanOrEqualTo(degraded.LastSuccessfulRead!.Value); + log.Warnings.Count(m => m.Contains("Missing")).ShouldBe(1); + } + + /// + /// A good write clears a Degraded the write side wrote. Before the fix a success-only batch + /// never wrote health, so a transient write fault latched Degraded until some read pass ran. + /// + [Fact] + public async Task A_good_write_clears_a_write_side_degraded() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory, _) = await StartAsync(); + using var _ = drv; + + factory.Write = (_, _, _) => throw new System.IO.IOException("connection reset"); + var bad = await drv.WriteAsync([new WriteRequest("Good", (ushort)1)], ct); + bad[0].StatusCode.ShouldNotBe(0u); + drv.GetHealth().State.ShouldBe(DriverState.Degraded, "positive control: a comms fault on write degrades"); + + factory.Write = (_, _, _) => Task.CompletedTask; + var good = await drv.WriteAsync([new WriteRequest("Good", (ushort)2)], ct); + good[0].StatusCode.ShouldBe(0u); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs index 2dc2152f..ad23e466 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs @@ -202,9 +202,10 @@ public sealed class S7SpanReadParityTests public async Task A_span_read_failure_degrades_every_member_like_the_per_tag_path() { var ct = TestContext.Current.CancellationToken; - // ErrorCode.ReadData with a message that is NOT access-denied ("not allowed") — a genuine - // device-layer fault, which the driver maps to BadDeviceFailure + Degraded. - var fault = new PlcException(ErrorCode.ReadData, "S7 CPU reported a device fault"); + // A device-layer fault that is neither a refusal (a ReadData item error is per-tag Bad with + // no health impact since R07:H4, and a refused span falls back per member since R07:P2) nor + // connection-fatal — the driver maps it to BadDeviceFailure + Degraded on every member. + var fault = new PlcException(ErrorCode.SendData, "S7 CPU reported a device fault"); var groupingFactory = new ImageS7PlcFactory(Images()) { ReadFault = fault }; var perTagFactory = new ImageS7PlcFactory(Images()) { ReadFault = fault }; @@ -468,8 +469,11 @@ public sealed class S7SpanReadParityTests } /// - /// The SPAN path obeys the same rule: one member that fails to decode leaves the pass - /// Degraded even though a later member of the same block decoded fine. + /// The SPAN path guards each member's decode: one member that fails to decode is Bad on its + /// own while a later member of the same block decodes fine. S7.Net rejects the malformed + /// STRING with a ReadData — the CPU answered, the buffer is + /// what is wrong — so since R07:H4 it is a per-tag refusal with no Degraded (like the + /// codec's own BadOutOfRange), and LastSuccessfulRead still advances. /// /// /// The block read itself succeeds — this is the per-member decode guard, the span-path @@ -480,7 +484,7 @@ public sealed class S7SpanReadParityTests /// last-writer-wins used to erase. /// [Fact] - public async Task One_undecodable_span_member_leaves_the_pass_Degraded() + public async Task One_undecodable_span_member_is_bad_alone_without_degrading() { var ct = TestContext.Current.CancellationToken; // The STRING occupies DB1 bytes 100..111 (StringLength + 2), so the word at 112 is @@ -516,7 +520,7 @@ public sealed class S7SpanReadParityTests results[1].Value.ShouldNotBeNull(); var health = driver.GetHealth(); - health.State.ShouldBe(DriverState.Degraded, "a span member that failed to decode must not be erased"); + health.State.ShouldBe(DriverState.Healthy, "an undecodable buffer is a data problem, not a comms fault"); health.LastSuccessfulRead!.Value.ShouldBeGreaterThan(before!.Value); } @@ -542,7 +546,7 @@ public sealed class S7SpanReadParityTests // which is the Faulted config alert. B: an ordinary device-layer fault, merely Degraded, // observed AFTER it. C: no fault at all. factory.PerTagFaults["DB1.DBW0"] = new PlcException(ErrorCode.ReadData, "Accessing object not allowed"); - factory.PerTagFaults["DB1.DBW2"] = new PlcException(ErrorCode.ReadData, "S7 CPU reported a device fault"); + factory.PerTagFaults["DB1.DBW2"] = new PlcException(ErrorCode.SendData, "S7 CPU reported a device fault"); // maxGap -1 turns grouping off entirely, so every tag takes the per-tag read path and the // faults can be aimed at individual addresses. using var driver = new S7Driver(Options(NarrowTags(), maxGap: -1), "s7-access-denied", factory); @@ -584,7 +588,7 @@ public sealed class S7SpanReadParityTests var factory = new ImageS7PlcFactory(Images()); // A: an ordinary device-layer fault (Degraded) FIRST. B: the access-denied config alert // (Faulted) after it. C: no fault at all. - factory.PerTagFaults["DB1.DBW0"] = new PlcException(ErrorCode.ReadData, "S7 CPU reported a device fault"); + factory.PerTagFaults["DB1.DBW0"] = new PlcException(ErrorCode.SendData, "S7 CPU reported a device fault"); factory.PerTagFaults["DB1.DBW2"] = new PlcException(ErrorCode.ReadData, "Accessing object not allowed"); using var driver = new S7Driver(Options(NarrowTags(), maxGap: -1), "s7-escalation-message", factory); await driver.InitializeAsync("{}", ct); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TsapAndPduTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TsapAndPduTests.cs index 4f9bd37f..a69cd4a1 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TsapAndPduTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TsapAndPduTests.cs @@ -224,6 +224,23 @@ public sealed class S7TsapAndPduTests (await S7DriverProbe.CheckDataAccessAsync(_ => throw missing, TestContext.Current.CancellationToken)).ShouldBeNull(); } + /// + /// The shapes a real read raises — S7.Net's read paths throw the CPU's item error as a BARE + /// Exception (live-probed on the snap7 fixture). Before, the probe only matched PlcException, so a + /// PUT/GET-disabled CPU got the generic "test read failed" text and a missing address failed the probe. + /// + [Fact] + public async Task Probe_classifies_the_bare_read_path_shapes() + { + var ct = TestContext.Current.CancellationToken; + var denied = await S7DriverProbe.CheckDataAccessAsync( + _ => throw new Exception("Received error from PLC: Accessing object not allowed."), ct); + denied.ShouldNotBeNull().Message!.ShouldContain("PUT/GET"); + + (await S7DriverProbe.CheckDataAccessAsync( + _ => throw new Exception("Received error from PLC: Object does not exist."), ct)).ShouldBeNull(); + } + [Fact] public async Task Probe_reports_any_other_read_fault_as_a_failure() {