diff --git a/docs/drivers/S7.md b/docs/drivers/S7.md index 02d27885..39788f8c 100644 --- a/docs/drivers/S7.md +++ b/docs/drivers/S7.md @@ -88,6 +88,9 @@ After the handshake, Test Connect reads 2 bytes from `MB0`. A CPU with PUT/GET d default) completes the handshake but denies every read, so the probe fails with an actionable "enable PUT/GET communication" message instead of a false green tick. A CPU that merely rejects that address (no `MB0`) still passes — the connection is fine; any other read fault fails the probe with its message. +Both are recognised in the bare-`Exception` shape S7.Net's read path really raises. The handshake and +test read are bounded by the config's `timeoutMs` (the driver's own per-operation timeout), capped by the +Test Connect timeout — before R07:H7 the config value was ignored. ## Address forms @@ -103,9 +106,18 @@ Addresses use Siemens TIA-Portal / STEP 7 Classic syntax, parsed by | Timer | `T0` / `T15` | Timer area (read-only — see Wide types section) | | Counter | `C0` / `C10` | Counter area (read-only — see Wide types section) | -Parsing is strict and runs once at `InitializeAsync` so a config typo fails fast -at load instead of surfacing as `BadInternalError` on every read. Bit offsets -must be 0-7, byte offsets non-negative, DB numbers >= 1. +Parsing is strict. Bit offsets must be 0-7, byte offsets non-negative, DB numbers >= 1. + +**Tag shape rules are judged per tag, at every gate** (`S7TagShapeRules`, called by +`S7TagDefinitionFactory.FromTagConfig`): the AdminUI editor refuses the tag, the deploy gate +(`Inspect`) warns, and the driver's init **skips** it (reported through `ISkippedTagReporter`; it reads +`BadNodeIdUnknown`) while every other tag keeps working. Refused: an address that does not parse, a +wide/structured type that is an array or is not byte-addressed (`DBB`/`MB`/`IB`/`QB`), a narrow type at the +wrong access width (`Int32` on `DBW0`, `Bool` on `DBB0`...), a Timer not typed `Float64` or a Counter not typed +`Int32`, a writable or array Timer/Counter, a writable `Bool` array, and a `Bool` array that does not start +at bit 0 (a Bool array reads bit 0 of each byte, so `DB1.DBX0.3` used to silently return bit 0 of bytes +0..n). These used to be init guards that THREW, so one typo faulted every tag on the PLC and the actor +retried the same init every 10 s forever (R07:H3). ## Data types @@ -277,7 +289,7 @@ AdminUI `/hosts` page (Gitea #640). See `docs/Telemetry.md` ("Driver diagnostics | `IWritable` | `WriteAsync` → `S7.Net.Plc.WriteAsync` | Read-only tags (`Writable=false`) return `BadNotWritable` | | `ITagDiscovery` | `DiscoverAsync` | Emits a flat `S7/` folder of the configured tags — no live browse | | `ISubscribable` | per-tag poll loop with capped exponential backoff | S7 has no push model; floor is 100 ms (the CPU services the comms mailbox once per scan) | -| `IHostConnectivityProbe` | periodic `S7.Net.Plc.ReadStatusAsync` (CPU-status PDU) | `host:port` host key; `Running`/`Stopped` transitions raise `OnHostStatusChanged` | +| `IHostConnectivityProbe` | periodic `S7.Net.Plc.ReadStatusAsync` (CPU-status PDU) | `host:port` host key; `Running`/`Stopped` transitions raise `OnHostStatusChanged`. A tick that finds the gate held past the probe deadline (a long read pass) gives **no verdict** — it no longer reports `Stopped` for a connection that is busy, not lost (R07:H7) | ### Single-connection policy @@ -302,8 +314,9 @@ Portal under *Protection & Security* for the CPU. | Tag not in config | `BadNodeIdUnknown` | unchanged | | Read-only tag written | `BadNotWritable` | unchanged | | Unimplemented data type | `BadNotSupported` | unchanged | -| PUT/GET denied | `BadNotSupported` | `Faulted` (config alert) | -| CPU / hardware fault | `BadDeviceFailure` | `Degraded` | +| PUT/GET denied | `BadNotSupported` | `Faulted` (config alert) — also counts as *answered*, advances `LastSuccessfulRead` (no watchdog reconnect loop) | +| 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 | @@ -314,7 +327,36 @@ 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 (S7.Net's exact "Accessing object not allowed", and never when the chain +also carries a transport fault) is the `Faulted` alert — which is ALSO an answer: it advances +`LastSuccessfulRead`, so a factory-default CPU with PUT/GET disabled stays `Faulted` instead of being +reconnected by the comms-loss watchdog every 90 s. A multi-var chunk that falls back per tag memoises the +members refused on their own, like a refused span. 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. + +**A dead handle ends the pass's wire traffic (R07:P1).** Once a read pass or write batch marks the +connection dead (a wire op blew its deadline on a frozen-but-established peer, a socket dropped), the +remaining spans / multi-var chunks / tags / writes of that pass are stamped `BadCommunicationError` +without I/O; the next call reopens. A multi-var chunk that dies on a transport loss no longer falls back +to one doomed per-tag read per member. Without this an op that ignores its cancellation token would cost +one full deadline per remaining item under the gate, and `Degraded` (and so the comms-loss watchdog's +clock) only became visible after the whole pass. + +**A refused span falls back per member (R07:P2).** A grouped block read covers the union of its members' +ranges, so one member the CPU refuses (running past the DB's real end, an over-declared `stringLength`) +used to make every neighbour in the block Bad on every poll. When the CPU refuses a multi-member span the +driver re-reads its members one at a time for that pass and remembers which tags must not be grouped — +the members refused on their own, or every member when none was — so later passes group only the rest. +The memo lasts until the next initialize. A span failure that is not a refusal still stamps every member. **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 @@ -359,7 +401,7 @@ normal dev (the S7 sim is on the shared Docker host but currently offline). **Array writes (S7-016)** — a writable `Byte` / `Int16` / `UInt16` / `Int32` / `UInt32` / `Float32` array is written as ONE contiguous `WriteBytesAsync` of `count x element-bytes` (the inverse of the read decode), and the value must be an array of exactly `arrayLength` elements (else `BadTypeMismatch`, no -health impact). **Writable `Bool` arrays are refused at init** (`NotSupportedException`): the array +health impact). **Writable `Bool` arrays are refused per tag** (see the tag shape rules above): the array read is byte-granular, so a matching write would overwrite the 7 neighbouring bits of every byte it touches, and a safe bit-packed write needs read-modify-write. Mark Bool arrays `writable: false`. A `null` array element is rejected (`BadTypeMismatch`, nothing written) rather than coerced to 0. An array write larger than one PDU is split by S7.Net into several writes and is diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7AddressParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7AddressParser.cs similarity index 100% rename from src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7AddressParser.cs rename to src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7AddressParser.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagDefinitionFactory.cs index a10c2c7d..94457fa5 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagDefinitionFactory.cs @@ -48,8 +48,23 @@ public static class S7TagDefinitionFactory /// The mapped definition when this returns . /// when is a valid S7 address object. public static bool FromTagConfig(string tagConfig, string rawPath, out S7TagDefinition def) + => FromTagConfig(tagConfig, rawPath, out def, out _); + + /// + /// , also reporting WHY a structurally + /// valid blob was refused by (an unparseable address, or a shape the + /// driver cannot serve). stays for a JSON-level + /// refusal (missing keys, bad enum), which explains on its own. + /// + /// The authored equipment-tag TagConfig JSON. + /// The tag's RawPath — becomes the definition's identity (Name). + /// The mapped definition when this returns . + /// The shape/address rejection reason, when that is why it refused. + /// when the driver can serve the tag. + public static bool FromTagConfig(string tagConfig, string rawPath, out S7TagDefinition def, out string? shapeRejection) { def = null!; + shapeRejection = null; if (string.IsNullOrWhiteSpace(tagConfig)) return false; try { @@ -78,7 +93,7 @@ public static class S7TagDefinitionFactory // "writable" defaults to true when absent (today's value); node-level authz still governs // writes. Honouring the key makes read-only equipment tags authorable. var writable = TagConfigJson.ReadWritable(root); - def = new S7TagDefinition( + var mapped = new S7TagDefinition( Name: rawPath, Address: address, DataType: dataType, @@ -86,6 +101,15 @@ public static class S7TagDefinitionFactory StringLength: stringLength == 0 ? MaxStringLength : stringLength, WriteIdempotent: false, ArrayCount: arrayCount); + // The address must parse and the shape must be one the driver can serve. These used to be + // init-time guards that THREW and faulted every tag on the PLC; refusing here skips only + // this tag, and the editor + deploy gate (which ask this method) refuse it too. + if (S7TagShapeRules.Validate(mapped, out _) is { } reason) + { + shapeRejection = reason; + return false; + } + def = mapped; return true; } catch (JsonException) { return false; } @@ -162,7 +186,9 @@ public static class S7TagDefinitionFactory /// reject — a structurally unparseable or non-object document, an absent / non-string / /// blank address, an absent dataType (see ), a /// present-but-invalid dataType (the runtime enum read is STRICT, so a - /// typo rejects the tag), or an out-of-range stringLength on a String tag. Such a tag + /// typo rejects the tag), an out-of-range stringLength on a String tag, or a shape + /// refuses (an unparseable address, a wrong access width, a mis-typed + /// Timer/Counter, an unsupported array). Such a tag /// is skipped by the driver and reads BadNodeIdUnknown forever. Empty when the mapper accepts /// the blob, when it is blank, or when it is not an equipment-tag object. Never throws. /// @@ -184,7 +210,13 @@ public static class S7TagDefinitionFactory // The accept gate: whatever the driver reads draws nothing. The RAW reference is handed to the // mapper — the driver reads the stored blob, not a trimmed copy of it. - if (FromTagConfig(reference, InspectionRawPath, out _)) return warnings; + if (FromTagConfig(reference, InspectionRawPath, out _, out var shapeRejection)) return warnings; + + if (shapeRejection is not null) + { + warnings.Add(shapeRejection + " The driver will skip this tag and it will read BadNodeIdUnknown."); + return warnings; + } try { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagShapeRules.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagShapeRules.cs new file mode 100644 index 00000000..17a9bf48 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7TagShapeRules.cs @@ -0,0 +1,149 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.S7; + +/// +/// The single authority on which S7 tag shapes the driver can serve. Consulted by +/// , so the AdminUI editor, the deploy gate +/// () and the driver's own init all refuse the same +/// tags with the same words. +/// +/// +/// +/// These rules used to be init-time guards in S7Driver that THREW. One mis-authored tag +/// (DB1.DBZ0, DB1.DBW0 typed Float64, T5 typed Int32) faulted the +/// whole driver, and the actor retried that same init every 10 s forever, darkening every tag on +/// the PLC. The editor and the deploy gate checked none of it, and a CSV import or API blob +/// bypasses the editor entirely. A rejected tag is now skipped on its own (it reads +/// BadNodeIdUnknown) and the deploy gate warns about it. +/// +/// +/// Every rule rejects a tag that could never be served correctly. None of them can newly break +/// a tag that used to work: each one either faulted init, or failed (or silently mis-decoded) +/// every read of that tag. +/// +/// +public static class S7TagShapeRules +{ + /// + /// S7 data types addressed by START BYTE (the B suffix) rather than by an access-width + /// suffix. The driver decodes them from a contiguous byte buffer, so a wide tag authored against a + /// non-byte suffix (DBW/DBD/...) would mis-frame its value. + /// + private static readonly HashSet ByteAnchoredDataTypes = + [ + S7DataType.Int64, + S7DataType.UInt64, + S7DataType.Float64, + S7DataType.String, + S7DataType.DateTime, + S7DataType.WString, + S7DataType.Char, + S7DataType.WChar, + S7DataType.Dtl, + S7DataType.Date, + S7DataType.Tod, + S7DataType.Time, + S7DataType.S5Time, + ]; + + /// True when is a wide/structured type read from a byte-anchored block. + /// The declared data type. + /// for the byte-anchored types. + public static bool IsByteAnchored(S7DataType dataType) => ByteAnchoredDataTypes.Contains(dataType); + + /// + /// Judges one tag definition. Returns when the driver can serve it, or + /// the operator-facing reason it cannot. + /// + /// The mapped tag definition. + /// The parsed address when this returns . + /// The rejection reason, or when the tag is servable. + public static string? Validate(S7TagDefinition def, out S7ParsedAddress parsed) + { + ArgumentNullException.ThrowIfNull(def); + parsed = default; + + S7ParsedAddress addr; + try + { + addr = S7AddressParser.Parse(def.Address); + } + catch (FormatException ex) + { + return $"S7 address '{def.Address}' does not parse: {ex.Message}"; + } + + var isWide = IsByteAnchored(def.DataType); + var isArray = def.ArrayCount is >= 1; + + // Wide-type arrays have no decoder. + if (isWide && isArray) + return $"S7 {def.DataType} arrays are not supported. Use a scalar {def.DataType} tag, or an " + + "Int16/UInt16/Int32/UInt32/Float32/Byte/Bool element type for an array."; + + // Timer / Counter type constraints: a timer decodes to Float64 seconds, a counter to an Int32 count. + if (addr.Area is S7Area.Timer && def.DataType != S7DataType.Float64) + return $"S7 Timer address '{def.Address}' is typed {def.DataType}. Timer tags must be " + + "DataType=Float64 (decoded to seconds, read-only)."; + if (addr.Area is S7Area.Counter && def.DataType != S7DataType.Int32) + return $"S7 Counter address '{def.Address}' is typed {def.DataType}. Counter tags must be " + + "DataType=Int32 (decoded to a count, read-only)."; + if (addr.Area is S7Area.Timer or S7Area.Counter) + { + if (isArray) + return $"S7 Timer/Counter address '{def.Address}' cannot be an array."; + if (def.Writable) + return $"S7 Timer/Counter address '{def.Address}' is declared writable. Timer/Counter tags " + + "are read-only; set Writable=false."; + parsed = addr; + return null; + } + + // Wide/structured scalars on DB/M/I/Q must be byte-anchored (DBB/MB/IB/QB). + if (isWide) + { + if (addr.Size != S7Size.Byte) + return $"S7 {def.DataType} at '{def.Address}' is not byte-addressed. Wide/structured types " + + "must be byte-addressed (DBB/MB/IB/QB), e.g. 'DB1.DBB0', 'MB4'."; + parsed = addr; + return null; + } + + // Narrow types: the declared type must match the address's access width, or every read fails + // (or decodes garbage) with a type mismatch. + var required = def.DataType switch + { + S7DataType.Bool => S7Size.Bit, + S7DataType.Byte => S7Size.Byte, + S7DataType.Int16 or S7DataType.UInt16 => S7Size.Word, + _ => S7Size.DWord, // Int32 / UInt32 / Float32 + }; + if (addr.Size != required) + return $"S7 {def.DataType} at '{def.Address}' has the wrong access width ({addr.Size}). " + + $"A {def.DataType} tag needs a {SuffixHint(required)} address."; + + if (isArray && def.DataType == S7DataType.Bool) + { + // Element i of a Bool array is bit 0 of byte (start + i): the array is byte-granular. A + // non-zero bit offset used to be silently ignored (DB1.DBX0.3 returned bit 0 of bytes 0..n). + if (addr.BitOffset != 0) + return $"S7 Bool array at '{def.Address}' starts at bit {addr.BitOffset}. A Bool array reads " + + "one byte per element (bit 0 of each byte), so it must start at bit 0 (e.g. 'DB1.DBX0.0')."; + // A byte-granular write would overwrite the 7 neighbouring bits of every byte it touches. + if (def.Writable) + return $"S7 Bool array at '{def.Address}' is declared writable. Bool array writes are not " + + "supported (a byte-granular write would clobber neighbouring bits); set Writable=false " + + "or write the bits as individual scalar Bool tags."; + } + + parsed = addr; + return null; + } + + private static string SuffixHint(S7Size size) => size switch + { + S7Size.Bit => "bit (DBX/M/I/Q n.b)", + S7Size.Byte => "byte (DBB/MB/IB/QB)", + S7Size.Word => "word (DBW/MW/IW/QW)", + _ => "double-word (DBD/MD/ID/QD)", + }; +} 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 8659565a..cbf94e6b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs @@ -59,7 +59,7 @@ public sealed class S7Driver _plcFactory = plcFactory ?? new S7PlcFactory(); _logger = logger ?? NullLogger.Instance; _resolver = new EquipmentTagRefResolver( - r => _tagsByRawPath.TryGetValue(r, out var t) ? t : null); + r => TagsByRawPath.TryGetValue(r, out var t) ? t : null); // 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; @@ -128,13 +128,6 @@ public sealed class S7Driver /// OPC UA StatusCode used for a genuine device fault (CPU error, hardware fault). private const uint StatusBadDeviceFailure = 0x808B0000u; - // v3: the RawPath → definition table. Keyed by RawPath (the v3 tag identity, == def.Name), - // case-sensitive ordinal. Built in InitializeAsync from _options.RawTags via the pure factory. - private readonly Dictionary _tagsByRawPath = new(StringComparer.Ordinal); - // Pre-parsed S7 address per tag, keyed by RawPath (== def.Name). Populated alongside - // _tagsByRawPath at init so a config typo fails fast here rather than as a per-read fault. - private readonly Dictionary _parsedByRawPath = new(StringComparer.Ordinal); - // Resolves a read/write/subscribe fullReference (a RawPath in v3) to a tag definition by a single // authored-table hit; a miss is a miss (the driver surfaces BadNodeIdUnknown, never a parse attempt). private readonly EquipmentTagRefResolver _resolver; @@ -208,6 +201,12 @@ public sealed class S7Driver /// Clock the connect backoff is measured on. Internal so tests step it deterministically. internal Func BackoffClock { get; set; } = static () => DateTime.UtcNow; + /// + /// Clock the read/write passes and the init publish stamp timestamps and + /// LastSuccessfulRead with. Internal so a test can step it and assert a STRICT advance. + /// + internal Func UtcNow { get; set; } = static () => DateTime.UtcNow; + /// /// The sticky write-side config fault (access denied / PUT-GET disabled), or null. A read /// pass that finds everything readable must NOT erase it (reads can succeed while writes are @@ -215,6 +214,27 @@ 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); + + /// + /// Tags the span planner must not group (R07:P2): members of a span the CPU refused as a block — + /// the ones that were refused on their own, or every member when none was. They are read + /// individually for the rest of the generation; a re-initialize (a corrected DB, a fixed tag) + /// groups everything again. Guarded by . + /// + private readonly HashSet _noGroup = new(StringComparer.Ordinal); private bool _disposed; /// @@ -225,62 +245,84 @@ public sealed class S7Driver /// public async Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) { - // InitializeAsync is reachable on a live driver with no teardown first (the IDriver contract - // allows it, and a reconnect retry can overlap a running attempt). Stop the previous probe loop and drop the previous connection - // before they are overwritten below. Deliberately NOT ShutdownAsync: that also drops _poll and the - // subscriptions, which are owned by the shutdown path (the actor re-subscribes after every successful init). - await ReleaseConnectionAsync().ConfigureAwait(false); - Health = new DriverHealth(DriverState.Initializing, null, null); + // Serialised against every other lifecycle operation: DriverInstanceActor dispatches initialize + // on a fire-and-forget Task.Run and a reconnect retry can start a second attempt while the + // first is still inside OpenAsync. Without the gate attempt A's freshly opened Plc and probe + // loop were overwritten by attempt B and never released (R07:H5) — the Modbus pattern. + await _lifecycleGate.WaitAsync(cancellationToken).ConfigureAwait(false); try { - // Re-parse the supplied DriverConfig JSON so a config change delivered through the - // IDriver contract is honoured. An empty / placeholder document - // (e.g. the "{}" some unit tests pass) keeps the constructor-supplied options. - if (HasConfigBody(driverConfigJson)) - _options = S7DriverFactoryExtensions.ParseOptions(_driverInstanceId, driverConfigJson); + await InitializeCoreAsync(driverConfigJson, cancellationToken).ConfigureAwait(false); + } + finally + { + _lifecycleGate.Release(); + } + } - // v3: build the RawPath → definition table from the authored raw tags BEFORE the config - // guards run (they now iterate the mapped table). Each entry's TagConfig blob is mapped by - // the pure factory; the entry's WriteIdempotent flag is threaded onto the def (it lives on - // the RawTagEntry, not inside the blob). A mapper miss is skipped (logged), never thrown — - // one bad tag must not fail the whole driver init. - _tagsByRawPath.Clear(); - _parsedByRawPath.Clear(); - _resolver.Clear(); // no-op in v3 (the resolver reads the live table by closure); kept for symmetry - _skippedTags.BeginRebuild(_options.RawTags.Count); - foreach (var entry in _options.RawTags) + /// The lifecycle-gated body of . + /// The driver config. + /// Caller cancellation. + /// A task that completes when the attempt has published, been superseded, or failed. + private async Task InitializeCoreAsync(string driverConfigJson, CancellationToken cancellationToken) + { + // This attempt's generation. Every health write and table/connection publish below is stamped + // with it and dropped once the generation has moved on: an attempt abandoned by its caller can + // still be running when a ShutdownAsync (which only waits a bounded time for the lifecycle + // gate) starts, and it must never publish over it. + var gen = BeginGeneration(); + + // InitializeAsync is reachable on a live driver with no teardown first (the IDriver contract + // allows it). Stop the previous probe loop and drop the previous connection before they are + // replaced below. Deliberately NOT a full teardown: that also drops _poll and the subscriptions, + // which are owned by the shutdown path (the actor re-subscribes after every successful init). + await ReleaseConnectionAsync(gen).ConfigureAwait(false); + TryWriteHealth(gen, new DriverHealth(DriverState.Initializing, null, null)); + + // Re-parse the supplied DriverConfig JSON so a config change delivered through the IDriver + // contract is honoured. An empty / placeholder document (e.g. the "{}" some unit tests pass) + // keeps the constructor-supplied options. Local until published: a superseded attempt must + // not leave its options behind for the live connection to reopen against. + var options = _options; + IS7Plc? plc = null; + try + { + if (HasConfigBody(driverConfigJson)) + options = S7DriverFactoryExtensions.ParseOptions(_driverInstanceId, driverConfigJson); + + // v3: build the RawPath → definition table from the authored raw tags. Each entry's TagConfig + // blob is mapped by the pure factory, which also applies S7TagShapeRules (the address must + // parse; wide arrays, mis-typed Timer/Counter, wrong access widths, writable Timer/Counter or + // Bool arrays and offset Bool arrays are refused). A refused tag is SKIPPED (logged, reported + // through ISkippedTagReporter, reads BadNodeIdUnknown), never thrown: these used to be init + // guards that faulted every tag on the PLC and made the actor retry the same init forever. + // The entry's WriteIdempotent flag is threaded onto the def (it lives on the RawTagEntry). + // Built into fresh dictionaries and published by ONE reference swap — a retained poll pass + // reading the old table never sees it half-cleared. + var tags = new Dictionary(StringComparer.Ordinal); + var parsed = new Dictionary(StringComparer.Ordinal); + var skipped = new List<(string RawPath, string Reason)>(); + foreach (var entry in options.RawTags) { - if (S7TagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var def)) - _tagsByRawPath[entry.RawPath] = def with { WriteIdempotent = entry.WriteIdempotent }; + if (S7TagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var def, out var shapeRejection) + && S7AddressParser.TryParse(def.Address, out var parsedAddress)) + { + tags[entry.RawPath] = def with { WriteIdempotent = entry.WriteIdempotent }; + parsed[entry.RawPath] = parsedAddress; + } else { - _skippedTags.Skip(entry.RawPath, "TagConfig did not map to a definition"); - _logger.LogWarning( - "S7 tag config did not map to a definition; skipping. Driver={DriverInstanceId} RawPath={RawPath}", - _driverInstanceId, entry.RawPath); + skipped.Add((entry.RawPath, shapeRejection ?? "TagConfig did not map to a definition")); } } - _skippedTags.Commit(); - - // Phase 4d combined config guard. Timer/Counter addresses and the wide/structured - // types (Int64/UInt64/Float64/String/DateTime) are now SUPPORTED, but only in - // specific shapes — this rejects the genuinely-unsupported configurations - // (wide-type arrays, non-byte-addressed wide scalars, wrong Timer/Counter DataType) - // so a config typo fails fast at init instead of as a misleading per-read fault. - RejectUnsupportedTagConfigs(); - - // Legacy data-type seam. The UnimplementedDataTypes set is now - // empty (Phase 4d unblocked all five wide types), so this rejects nothing — kept as - // the single grep target / future seam should a type ever need re-gating at init. - RejectUnsupportedTagDataTypes(); // Build the connection through the factory (the same seam EnsureConnectedAsync's reopen // uses) so the read/write timeouts are baked in identically on connect and reconnect. - var plc = _plcFactory.Create(_options); + plc = _plcFactory.Create(options); try { using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - cts.CancelAfter(_options.Timeout); + cts.CancelAfter(options.Timeout); await plc.OpenAsync(cts.Token).ConfigureAwait(false); } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) @@ -288,97 +330,160 @@ public sealed class S7Driver // The linked CancelAfter fired — a connect TIMEOUT, not a caller cancellation. // Surface it as a TimeoutException so init-time and reconnect-time timeouts report // the same exception type (for consistency); the outer catch still faults health. - try { plc.Dispose(); } catch { } throw new TimeoutException( - $"S7 connect to {_options.Host}:{_options.Port} timed out after " + - $"{(int)_options.Timeout.TotalMilliseconds} ms."); + $"S7 connect to {options.Host}:{options.Port} timed out after " + + $"{(int)options.Timeout.TotalMilliseconds} ms."); } - catch + + // Publish everything this attempt built under _gate (no pass can be mid-flight against a + // half-published state) and only while this attempt is still the current generation. + await _gate.WaitAsync(cancellationToken).ConfigureAwait(false); + try { - // Dispose the partially-constructed connection so a failed init doesn't leak it - // before the outer catch nulls Plc (Plc isn't assigned yet, so it can't). - try { plc.Dispose(); } catch { } - throw; + if (!IsCurrentGeneration(gen)) + { + // The outer finally disposes the unpublished plc; the catch writes no health (the + // generation moved on). Thrown, not returned: a caller must never read "initialized" + // into an attempt that published nothing. + throw new InvalidOperationException( + "S7 initialize was superseded by a newer lifecycle operation before it published; " + + "its connection was discarded."); + } + + _options = options; + Volatile.Write(ref _tables, new TagTables(tags, parsed)); + _refusalWarned.Clear(); + _noGroup.Clear(); + _skippedTags.BeginRebuild(options.RawTags.Count); + foreach (var (rawPath, reason) in skipped) + { + _skippedTags.Skip(rawPath, reason); + _logger.LogWarning( + "S7 tag skipped: {Reason} Driver={DriverInstanceId} RawPath={RawPath}", + reason, _driverInstanceId, rawPath); + } + _skippedTags.Commit(); + + Plc = plc; + plc = null; // published — no longer this method's to dispose + _plcDead = false; + _connectBackoff.RecordSuccess(); + Volatile.Write(ref _negotiatedPduSize, Plc.MaxPduSize); + _initialized = true; + TryWriteHealth(gen, new DriverHealth(DriverState.Healthy, UtcNow(), null)); + + // Kick off the probe loop once the connection is up. Initial HostState stays Unknown + // until the first probe tick succeeds — avoids broadcasting a premature Running + // transition before any PDU round-trip has happened. Tracked (not fire-and-forget) so + // a teardown can await it before disposing its CTS; Task.Run gets None so the handle + // is always awaitable — the loop's own token check handles cancellation. + if (options.Probe.Enabled) + { + var probeCts = new CancellationTokenSource(); + _probeCts = probeCts; + _probeTask = Task.Run(() => ProbeLoopAsync(probeCts.Token), CancellationToken.None); + } } - - Plc = plc; - _plcDead = false; - _connectBackoff.RecordSuccess(); - Volatile.Write(ref _negotiatedPduSize, plc.MaxPduSize); - - // Parse every tag's address once at init so config typos fail fast here instead - // of surfacing as BadInternalError on every Read against the bad tag. The parser - // also rejects bit-offset > 7, DB 0, unknown area letters, etc. Keyed by RawPath - // (== def.Name) so Read/Write can pick up the pre-parsed address. - foreach (var t in _tagsByRawPath.Values) + finally { - _parsedByRawPath[t.Name] = S7AddressParser.Parse(t.Address); // throws FormatException + _gate.Release(); } - _initialized = true; - Health = new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null); _logger.LogInformation("S7Driver connected. Driver={DriverInstanceId} Host={Host} CPU={CpuType} PduSize={PduSize} Tags={TagCount}", - _driverInstanceId, _options.Host, _options.CpuType, _negotiatedPduSize, _tagsByRawPath.Count); - - // Kick off the probe loop once the connection is up. Initial HostState stays - // Unknown until the first probe tick succeeds — avoids broadcasting a premature - // Running transition before any PDU round-trip has happened. - if (_options.Probe.Enabled) - { - _probeCts = new CancellationTokenSource(); - // Track the probe Task (not fire-and-forget) so ShutdownAsync can await it - // before disposing _gate / _probeCts. Pass None to Task.Run so - // the delegate always runs and the handle is always awaitable; the loop's own - // token check handles cancellation. - _probeTask = Task.Run(() => ProbeLoopAsync(_probeCts.Token), CancellationToken.None); - } + _driverInstanceId, options.Host, options.CpuType, _negotiatedPduSize, tags.Count); } catch (Exception ex) { - // Clean up a partially-constructed Plc so a retry from the caller doesn't leak - // the TcpClient. Dispose is best-effort and idempotent. - try { Plc?.Dispose(); } catch { } - Plc = null; - Volatile.Write(ref _negotiatedPduSize, 0); - _initialized = false; - _plcDead = false; - Health = new DriverHealth(DriverState.Faulted, null, ex.Message); - _logger.LogError(ex, "S7Driver connect failed. Driver={DriverInstanceId} Host={Host}", _driverInstanceId, _options.Host); + // A superseded attempt reports nothing: the generation that replaced it owns health. + if (TryWriteHealth(gen, new DriverHealth(DriverState.Faulted, null, ex.Message))) + { + _options = options; + _logger.LogError(ex, "S7Driver connect failed. Driver={DriverInstanceId} Host={Host}", _driverInstanceId, options.Host); + } throw; } + finally + { + // A connection this attempt opened but did not publish (failed or superseded) is released + // here, so a retry from the caller never leaks the TcpClient. + if (plc is not null) { try { plc.Dispose(); } catch { /* best-effort */ } } + } } /// public async Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) { - // InitializeAsync re-parses driverConfigJson, so a config change delivered here is + // Holds the lifecycle gate across BOTH halves so no other lifecycle operation slips between + // them. InitializeCoreAsync re-parses driverConfigJson, so a config change delivered here is // applied in place rather than silently discarded. - await ShutdownAsync(cancellationToken).ConfigureAwait(false); - await InitializeAsync(driverConfigJson, cancellationToken).ConfigureAwait(false); + await _lifecycleGate.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + await ShutdownCoreAsync().ConfigureAwait(false); + await InitializeCoreAsync(driverConfigJson, cancellationToken).ConfigureAwait(false); + } + finally + { + _lifecycleGate.Release(); + } } /// public async Task ShutdownAsync(CancellationToken cancellationToken) { - // Signal cancellation to the probe + poll loops first, collect their Task handles, - // 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. + // Bounded wait for the lifecycle gate: a wedged initialize (a connect that never returns, or an + // attempt its caller abandoned) must not turn shutdown into a hang. Past the bound we tear down + // anyway — the generation bump makes the still-running attempt discard instead of publish. + var gated = false; + try { gated = await _lifecycleGate.WaitAsync(LifecycleGateTimeout, cancellationToken).ConfigureAwait(false); } + catch (OperationCanceledException) { /* the caller stopped waiting for the gate — still tear down */ } + if (!gated) + _logger.LogWarning( + "S7 lifecycle gate still held at shutdown; tearing down anyway. Driver={DriverInstanceId}", + _driverInstanceId); + try + { + await ShutdownCoreAsync().ConfigureAwait(false); + } + finally + { + if (gated) _lifecycleGate.Release(); + } + } + + /// + /// The complete teardown: the poll engine (every subscription), the probe loop, the connection, + /// the sticky write fault and the per-generation memos. It is its own generation, so its final + /// health write can never clobber an initialize that started after it. + /// + /// A task that completes once everything is released. + private async Task ShutdownCoreAsync() + { + var gen = BeginGeneration(); // Drain the polling overlay first — the engine cancels each loop and awaits it before - // disposing its CTS (drain-before-dispose), so no poll iteration touches _gate - // after this returns. - await _poll.DisposeAsync().ConfigureAwait(false); - await ReleaseConnectionAsync().ConfigureAwait(false); - _writeFault = null; - _connectBackoff.RecordSuccess(); - Health = new DriverHealth(DriverState.Unknown, Health.LastSuccessfulRead, null); + // disposing its CTS (drain-before-dispose), so no poll iteration touches _gate after this. + try { await _poll.DisposeAsync().ConfigureAwait(false); } + catch (Exception ex) { _logger.LogWarning(ex, "S7 poll engine drain failed. Driver={DriverInstanceId}", _driverInstanceId); } + await ReleaseConnectionAsync(gen).ConfigureAwait(false); + lock (_healthLock) + { + if (_generation != gen) return; // a newer initialize owns the state now + _writeFault = null; + _writeOwnedError = null; + _connectBackoff.RecordSuccess(); + Health = new DriverHealth(DriverState.Unknown, Health.LastSuccessfulRead, null); + } } /// /// Cancels + bounded-joins the probe loop, then disposes its CTS and the PLC connection. The - /// connection part of (everything but the poll engine and health). + /// connection part of a teardown (everything but the poll engine and health). Releases only + /// while is still current — a teardown that ran past the lifecycle gate + /// must not dispose a connection a NEWER initialize already published. /// - private async Task ReleaseConnectionAsync() + /// The generation of the lifecycle operation doing the release. + /// A task that completes once the prior state is released. + private async Task ReleaseConnectionAsync(long gen) { var probeCts = _probeCts; var probeTask = _probeTask; @@ -393,11 +498,6 @@ public sealed class S7Driver catch { /* loop faults are already surfaced via health; teardown continues */ } } - // The probe loop has now observed cancellation and released _gate — safe to dispose the CTS. - probeCts?.Dispose(); - _probeCts = null; - _probeTask = null; - // Swap the connection out under _gate so a Read/Write/poll pass that already holds the gate (they // re-read Plc inside it) can never see a disposed Plc mid-swap. Bounded: a wedged holder must not hang // teardown, in which case we proceed exactly as the un-gated path always did. @@ -409,6 +509,24 @@ public sealed class S7Driver _driverInstanceId, DrainTimeout); try { + if (!IsCurrentGeneration(gen)) return; // superseded: the newer generation released (or owns) this state + + // The probe loop has now observed cancellation and released _gate — safe to dispose the CTS. + if (ReferenceEquals(_probeCts, probeCts)) + { + probeCts?.Dispose(); + } + else + { + // An initialize published (and started a probe loop) after this release took its + // snapshot — a teardown whose generation bump raced the publish block. Cancel that + // loop too; it exits on its next gate wait / delay. Not joined: it needs _gate, which + // we hold. Its CTS is left to the GC rather than disposed under a running loop. + try { _probeCts?.Cancel(); } catch { /* best-effort */ } + probeCts?.Dispose(); + } + _probeCts = null; + _probeTask = null; _initialized = false; _plcDead = false; try { Plc?.Dispose(); } catch { /* best-effort — tearing down anyway */ } @@ -421,6 +539,101 @@ public sealed class S7Driver } } + // ---- lifecycle generation ---- + + /// + /// Guards / / + /// / against each other (R07:H5, the + /// Modbus _lifecycleGate pattern). Distinct from , which serialises I/O. + /// Never disposed: a Dispose racing a still-running ShutdownAsync must not make it throw. + /// + private readonly SemaphoreSlim _lifecycleGate = new(1, 1); + + /// + /// Bounded wait the teardown paths take on before tearing down anyway. + /// Internal so a test can shorten it instead of waiting the production 10 s. + /// + internal TimeSpan LifecycleGateTimeout { get; set; } = TimeSpan.FromSeconds(10); + + /// Serialises generation bumps against generation-stamped health writes. + private readonly object _healthLock = new(); + + /// + /// Lifecycle generation: bumped by every initialize and teardown. Health writes and + /// table/connection publishes carry the generation they were computed under and are dropped + /// once it has moved on. Written under . + /// + private long _generation; + + /// Starts a new lifecycle generation. + /// The new generation. + private long BeginGeneration() + { + lock (_healthLock) return ++_generation; + } + + /// The current lifecycle generation. + private long CurrentGeneration => Interlocked.Read(ref _generation); + + /// True while is still the current lifecycle generation. + /// The generation to test. + /// when nothing has superseded it. + private bool IsCurrentGeneration(long gen) => CurrentGeneration == gen; + + /// Writes only while is current. + /// The generation the health was computed under. + /// The health to publish. + /// when written. + private bool TryWriteHealth(long gen, DriverHealth health) + { + lock (_healthLock) + { + if (_generation != gen) return false; + Health = health; + return true; + } + } + + /// + /// Derives and writes a new health from the current one, atomically, only while + /// is current — for the passes that preserve LastSuccessfulRead. + /// + /// The generation the update was computed under. + /// Maps the current health to the new one; returning null writes nothing. + /// when written. + private bool TryUpdateHealth(long gen, Func update) + { + lock (_healthLock) + { + if (_generation != gen) return false; + if (update(Health) is not { } next) return false; + Health = next; + return true; + } + } + + /// The published RawPath → definition and RawPath → parsed-address tables, swapped as one. + /// RawPath → definition. + /// RawPath → pre-parsed address. + private sealed record TagTables( + IReadOnlyDictionary Tags, + IReadOnlyDictionary Parsed) + { + /// The empty tables a driver starts with. + public static readonly TagTables Empty = new( + new Dictionary(StringComparer.Ordinal), + new Dictionary(StringComparer.Ordinal)); + } + + /// The live tag tables; replaced by reference at init, never mutated in place. + private TagTables _tables = TagTables.Empty; + + /// The live RawPath → definition table. + private IReadOnlyDictionary TagsByRawPath => Volatile.Read(ref _tables).Tags; + + /// The live RawPath → pre-parsed address table. + private IReadOnlyDictionary ParsedByRawPath => Volatile.Read(ref _tables).Parsed; + /// public DriverHealth GetHealth() => Health; @@ -437,13 +650,6 @@ public sealed class S7Driver return trimmed is not "{}" and not "[]"; } - /// - /// S7 data types addressed by START BYTE (the B suffix) rather than by an - /// access-width suffix. Phase 4d decodes these from a contiguous byte buffer - /// (DB{n}.DBB{offset} / MB / IB / QB{offset}), so a wide tag - /// authored against a non-byte suffix (DBW/DBD/…) is a config error — see - /// . - /// /// /// PDU size the CPU negotiated on the most recent successful connect, in bytes; 0 before the first /// connect and after shutdown. Snapshotted at connect so it stays readable while the handle is @@ -474,143 +680,6 @@ public sealed class S7Driver : new Dictionary(); } - private static readonly HashSet ByteAnchoredDataTypes = new() - { - S7DataType.Int64, - S7DataType.UInt64, - S7DataType.Float64, - S7DataType.String, - S7DataType.DateTime, - S7DataType.WString, - S7DataType.Char, - S7DataType.WChar, - S7DataType.Dtl, - S7DataType.Date, - S7DataType.Tod, - S7DataType.Time, - S7DataType.S5Time, - }; - - /// - /// Combined init-time config guard (Phase 4d, supersedes the old Timer/Counter-only - /// RejectUnsupportedTagAddresses). Timer/Counter tags and the wide/structured - /// types (Int64/UInt64/Float64/String/DateTime) are now supported — but only in - /// specific shapes. This guard fails fast on the genuinely-unsupported configurations so - /// a config typo throws here at init rather than as a misleading per-read fault: - /// - /// (a) a wide/structured type declared as an array — wide-type arrays are out of scope this phase. - /// (b) a wide/structured type on a DB/M/I/Q area whose address isn't byte-anchored (DBB/MB/IB/QB). - /// (c) a Timer tag not typed Float64, or a Counter tag not typed Int32. - /// - /// Addresses that don't parse are left to the existing - /// pass in so the parse error isn't double-handled. - /// - private void RejectUnsupportedTagConfigs() - { - foreach (var t in _tagsByRawPath.Values) - { - var isWide = ByteAnchoredDataTypes.Contains(t.DataType); - - // (a) wide-type arrays — out of scope this phase. - if (isWide && t.ArrayCount is >= 1) - { - throw new NotSupportedException( - $"S7 tag '{t.Name}' is a {t.DataType} array, which the S7 driver does not yet " + - "support. Use a scalar wide tag, or an Int16/UInt16/Int32/UInt32/Float32/Byte/Bool " + - "element type for arrays."); - } - - // Parse the address; if it doesn't parse, leave it for the InitializeAsync - // S7AddressParser.Parse pass to throw the FormatException (no double-handling). - if (!S7AddressParser.TryParse(t.Address, out var parsed)) - continue; - - // (b) wide/structured scalar on DB/M/I/Q must be byte-anchored (Size == Byte). The - // codec decodes from a byte buffer at the start byte; a non-byte suffix (DBW/DBD/…) - // would mis-frame the value. - if (isWide - && parsed.Area is S7Area.DataBlock or S7Area.Memory or S7Area.Input or S7Area.Output - && parsed.Size != S7Size.Byte) - { - throw new NotSupportedException( - $"S7 tag '{t.Name}' is a {t.DataType} at '{t.Address}', which is not byte-addressed. " + - "Wide/structured types must be byte-addressed (DBB/MB/IB/QB) — e.g. 'DB1.DBB0', 'MB4'."); - } - - // (c) Timer/Counter type constraints — Timer decodes to a Float64 (seconds), Counter - // to an Int32 (count); both are read-only. - if (parsed.Area is S7Area.Timer && t.DataType != S7DataType.Float64) - { - throw new NotSupportedException( - $"S7 tag '{t.Name}' is a Timer address ('{t.Address}') but is typed {t.DataType}. " + - "Timer tags must be DataType=Float64 (decoded to seconds, read-only)."); - } - if (parsed.Area is S7Area.Counter && t.DataType != S7DataType.Int32) - { - throw new NotSupportedException( - $"S7 tag '{t.Name}' is a Counter address ('{t.Address}') but is typed {t.DataType}. " + - "Counter tags must be DataType=Int32 (decoded to count, read-only)."); - } - - // (d) Timer/Counter declared Writable — writes are read-only this phase; without this - // a node is discovered as Operate-writable but every write returns BadNotSupported. - if (parsed.Area is S7Area.Timer or S7Area.Counter && t.Writable) - { - throw new NotSupportedException( - $"S7 tag '{t.Name}' is a Timer/Counter ('{t.Address}') declared Writable — " + - "Timer/Counter are read-only this phase; set Writable=false."); - } - - // (e) Bool array declared Writable — element writes are byte-granular (one byte per - // element, mirroring the read), so a write would overwrite the 7 neighbouring bits of - // every byte it touches; a safe bit-packed write needs read-modify-write and is not - // implemented. Every other supported element type (Byte/Int16/UInt16/Int32/UInt32/ - // Float32) writes as ONE contiguous block (EncodeArrayBlock). Wide-type arrays - // are already rejected above (guard-a). - if (t.ArrayCount is >= 1 && t.Writable && t.DataType == S7DataType.Bool) - { - throw new NotSupportedException( - $"S7 tag '{t.Name}' is a Bool array (ArrayCount={t.ArrayCount}) declared Writable — " + - "Bool array writes are not supported (a byte-granular write would clobber the neighbouring " + - "bits). Set Writable=false, or write the bits as individual scalar Bool tags."); - } - } - } - - /// - /// Rejects tags configured with an still on the - /// list — types / - /// throw for, which - /// would otherwise create live OPC UA nodes via that return - /// BadNotSupported on every access. - /// The set is now empty (Phase 4d unblocked all five wide types), so this rejects - /// nothing — kept as the seam / grep target should a type ever need re-gating at init. - /// - private void RejectUnsupportedTagDataTypes() - { - foreach (var t in _tagsByRawPath.Values) - { - if (UnimplementedDataTypes.Contains(t.DataType)) - { - throw new NotSupportedException( - $"S7 tag '{t.Name}' uses data type '{t.DataType}' which is not yet " + - "supported by the S7 driver — Read/Write would return BadNotSupported. " + - "Remove the tag or use Bool/Byte/Int16/UInt16/Int32/UInt32/Float32 until " + - $"{t.DataType} is wired through."); - } - } - } - - /// - /// S7DataType members that the read/write helpers throw NotSupportedException for. - /// Now empty — Phase 4d (S7 wide types + Timer/Counter) wired Int64, UInt64, - /// Float64, String, and DateTime through the byte-buffer codec, and Timer/Counter read - /// through it on AREA. Kept here (rather than reflecting over - /// ) as the single grep target / future seam should a - /// data type ever need re-gating at init. - /// - private static readonly HashSet UnimplementedDataTypes = new(); - // ---- IReadable ---- // The batch is served in two passes under ONE gate acquisition. First S7ReadSpanPlanner groups @@ -639,15 +708,22 @@ public sealed class S7Driver // InvalidOperationException from the not-initialized check. ArgumentNullException.ThrowIfNull(fullReferences); RequireInitialized(); - var now = DateTime.UtcNow; + var now = UtcNow(); var results = new DataValueSnapshot[fullReferences.Count]; // Health is decided ONCE, at the end of the pass, from every outcome the pass observed — // never per item. See ReadPassOutcome. var outcome = new ReadPassOutcome(); await _gate.WaitAsync(cancellationToken).ConfigureAwait(false); + // The lifecycle generation this pass runs in — its health write is dropped if a teardown or + // re-initialize has moved on by the time it lands. Captured under _gate, where the init + // publish happens, and re-checked against _initialized: a pass that passed RequireInitialized + // and then waited out a teardown must not reopen a connection on a shut-down driver. + var gen = CurrentGeneration; try { + RequireInitialized(); + // (Re)open the connection under the gate. A failed (re)connect degrades the whole // batch to BadCommunicationError rather than throwing — the poll loop / next read // retries, and EnsureConnectedAsync repairs on the following call. @@ -664,7 +740,7 @@ public sealed class S7Driver { for (var i = 0; i < fullReferences.Count; i++) results[i] = new DataValueSnapshot(null, StatusBadCommunicationError, null, now); - Health = new DriverHealth(DriverState.Degraded, Health.LastSuccessfulRead, ex.Message); + TryUpdateHealth(gen, h => new DriverHealth(DriverState.Degraded, h.LastSuccessfulRead, ex.Message)); return results; } @@ -682,6 +758,15 @@ public sealed class S7Driver // Indexes the per-tag path must serve: everything, when grouping is off; otherwise the // planner's leftovers (Timer/Counter) plus any tag whose address won't parse. var perTag = new List(); + // Indexes that must take the per-tag path WITHOUT the multi-var pass: tags the planner may + // not group (R07:P2) and the members of a span refused this pass. Batching them into a + // multi-var request would just refuse the whole chunk again. + List? forcedPerTag = null; + // Members of each span refused this pass, and which of them were then refused on their own. + List>? refusedSpans = null; + // Members of each multi-var chunk that fell back per tag this pass (R07:P2's multi-var sibling). + List>? multiVarFallbacks = null; + HashSet? refusedAlone = null; if (_options.SpanReadMaxGapBytes >= 0) { @@ -689,6 +774,11 @@ public sealed class S7Driver for (var i = 0; i < fullReferences.Count; i++) { if (resolved[i] is not { } tag) continue; + if (_noGroup.Contains(tag.Name)) + { + (forcedPerTag ??= []).Add(i); + continue; + } if (!TryGetParsedAddress(tag, out var addr)) { // Defensive: every tag pre-parses at init. A tag that somehow didn't goes @@ -712,7 +802,19 @@ public sealed class S7Driver perTag.Add(span.Members[0].OriginalIndex); continue; } - await ReadSpanAsync(plc, span, results, now, outcome, cancellationToken).ConfigureAwait(false); + // Fail fast once an earlier span marked the handle dead (R07:P1): every + // further wire op on it would pay the full deadline under the gate. + if (_plcDead) + { + foreach (var member in span.Members) results[member.OriginalIndex] = DeadHandleSnapshot(now); + continue; + } + if (await ReadSpanAsync(plc, span, results, now, outcome, cancellationToken).ConfigureAwait(false) + is { } refusedMembers) + { + (refusedSpans ??= []).Add(refusedMembers); + (forcedPerTag ??= []).AddRange(refusedMembers); + } } perTag.AddRange(plan.LeftoverIndexes); } @@ -735,11 +837,23 @@ public sealed class S7Driver if (_options.SpanReadMaxGapBytes >= 0) { perTag = await ReadLeftoversByMultiVarAsync( - plc, resolved, perTag, results, now, outcome, cancellationToken).ConfigureAwait(false); + plc, resolved, perTag, results, now, outcome, cancellationToken, + fallback => (multiVarFallbacks ??= []).Add(fallback)).ConfigureAwait(false); + } + if (forcedPerTag is not null) + { + perTag.AddRange(forcedPerTag); + perTag.Sort(); } foreach (var i in perTag) { + // Fail fast once the handle is dead (R07:P1) — see the span loop. + if (_plcDead) + { + results[i] = DeadHandleSnapshot(now); + continue; + } try { var value = await ReadOneAsync(plc, resolved[i]!, cancellationToken).ConfigureAwait(false); @@ -760,7 +874,38 @@ public sealed class S7Driver } catch (Exception ex) { - results[i] = new DataValueSnapshot(null, RecordReadFailure(ex, outcome), null, now); + if ((refusedSpans is not null || multiVarFallbacks is not null) && IsRefusal(ex)) (refusedAlone ??= []).Add(i); + results[i] = new DataValueSnapshot(null, RecordReadFailure(ex, outcome, [resolved[i]!.Name]), null, now); + } + } + + // Remember what must not be grouped again (R07:P2). Skipped when the handle died mid-pass: + // the per-member reads then proved nothing about which member the CPU refuses. + // A multi-var chunk that fell back: memo only the members refused ON THEIR OWN — without one, + // the chunk's failure explains nothing about its members, and they keep batching. + if (multiVarFallbacks is not null && refusedAlone is not null && !_plcDead) + { + foreach (var members in multiVarFallbacks) + { + var bad = members.Where(refusedAlone.Contains).ToList(); + if (bad.Count == 0) continue; + foreach (var m in bad) _noGroup.Add(resolved[m]!.Name); + _logger.LogInformation( + "S7 CPU refused a multi-var read of {MemberCount} tag(s); reading {Tags} individually from now on. Driver={DriverInstanceId}", + members.Count, string.Join(", ", bad.Select(m => resolved[m]!.Name)), _driverInstanceId); + } + } + + if (refusedSpans is not null && !_plcDead) + { + foreach (var members in refusedSpans) + { + var bad = members.Where(m => refusedAlone?.Contains(m) == true).ToList(); + var memo = bad.Count > 0 ? bad : [.. members]; + foreach (var m in memo) _noGroup.Add(resolved[m]!.Name); + _logger.LogInformation( + "S7 CPU refused a grouped read of {MemberCount} tag(s); reading {Tags} individually from now on. Driver={DriverInstanceId}", + members.Count, string.Join(", ", memo.Select(m => resolved[m]!.Name)), _driverInstanceId); } } } @@ -793,10 +938,10 @@ public sealed class S7Driver state = wf.State; error = wf.Message; } - Health = new DriverHealth( + TryUpdateHealth(gen, h => new DriverHealth( state, - outcome.AnySuccess ? now : Health.LastSuccessfulRead, - error); + outcome.AnySuccess ? now : h.LastSuccessfulRead, + error)); } _gate.Release(); @@ -866,6 +1011,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. /// @@ -973,6 +1125,8 @@ public sealed class S7Driver /// The batch timestamp, shared by every tag in the batch. /// The pass's outcome accumulator; every decoded item records into it. /// Cancellation token for the reads. + /// Receives the member indexes of each chunk that fell back per tag, so + /// the caller can memo the members then refused on their own (the multi-var sibling of R07:P2). /// The indexes still owed an individual read, ascending. private async Task> ReadLeftoversByMultiVarAsync( IS7Plc plc, @@ -981,7 +1135,8 @@ public sealed class S7Driver DataValueSnapshot[] results, DateTime now, ReadPassOutcome outcome, - CancellationToken ct) + CancellationToken ct, + Action> onChunkFallback) { var stillPerTag = new List(); List<(int Index, S7TagDefinition Tag, S7ParsedAddress Addr)>? batchable = null; @@ -1012,6 +1167,13 @@ public sealed class S7Driver { var chunk = batchable.GetRange(start, Math.Min(MultiVarReadMaxItems, batchable.Count - start)); + // Fail fast once the handle is dead (R07:P1) — see ReadAsync's span loop. + if (_plcDead) + { + foreach (var (index, _, _) in chunk) results[index] = DeadHandleSnapshot(now); + continue; + } + IReadOnlyList answered; try { @@ -1037,12 +1199,24 @@ public sealed class S7Driver _plcDead = true; throw; } + catch (Exception ex) when (IsS7TransportLoss(ex)) + { + // A transport loss is the connection's fault, not the batch's: falling back would + // send one more doomed per-tag read per member, each paying the full deadline + // (R07:P1). Mark the handle dead and fail the chunk (and, via the guards, the rest + // of the pass) without I/O. + _plcDead = true; + outcome.RecordFailure(DriverState.Degraded, ex.Message); + foreach (var (index, _, _) in chunk) results[index] = DeadHandleSnapshot(now); + continue; + } catch (Exception ex) { _logger.LogDebug(ex, "S7 driver {DriverInstanceId}: multi-var read of {ItemCount} leftover tag(s) failed; " + "falling back to per-tag reads for that chunk", _driverInstanceId, chunk.Count); foreach (var (index, _, _) in chunk) stillPerTag.Add(index); + onChunkFallback([.. chunk.Select(c => c.Index)]); continue; } @@ -1061,7 +1235,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); } } } @@ -1134,8 +1308,12 @@ public sealed class S7Driver /// The batch timestamp, shared by every tag in the batch. /// The pass's outcome accumulator; every member records into it. /// Cancellation token for the read. - /// A task that completes once every member of the span has been stamped. - private async Task ReadSpanAsync( + /// + /// once every member has been stamped; or, when the CPU REFUSED the block + /// of a multi-member span (R07:P2 — one member past the DB's end refuses them all), the members' + /// original indexes, unstamped, for the caller to read one at a time. + /// + private async Task?> ReadSpanAsync( IS7Plc plc, S7ReadSpanPlanner.Span span, DataValueSnapshot[] results, DateTime now, ReadPassOutcome outcome, CancellationToken ct) { @@ -1156,12 +1334,24 @@ public sealed class S7Driver _plcDead = true; throw; } + catch (Exception ex) when (span.Members.Count > 1 && IsRefusal(ex)) + { + // The planner's old invariant ("a grouped read cannot fault where the per-tag reads would + // have succeeded") is false: the block is the UNION of the members' ranges, so one member + // running past the DB's real end, or over-declaring a STRING, makes the CPU refuse every + // neighbour with it. Hand the members back for individual reads. + _logger.LogDebug(ex, + "S7 driver {DriverInstanceId}: CPU refused the block read of {Area} db{Db} @{Start}+{Length}; " + + "re-reading its {Count} member(s) individually", _driverInstanceId, span.Area, span.DbNumber, + span.StartByte, span.LengthBytes, span.Members.Count); + return [.. span.Members.Select(m => m.OriginalIndex)]; + } 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; + return null; } foreach (var member in span.Members) @@ -1177,9 +1367,10 @@ 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); } } + return null; } /// @@ -1226,7 +1417,7 @@ public sealed class S7Driver /// The parsed address, valid only when this returns true. /// true when the address is known or parses. private bool TryGetParsedAddress(S7TagDefinition tag, out S7ParsedAddress addr) - => _parsedByRawPath.TryGetValue(tag.Name, out addr) || S7AddressParser.TryParse(tag.Address, out addr); + => ParsedByRawPath.TryGetValue(tag.Name, out addr) || S7AddressParser.TryParse(tag.Address, out addr); /// /// The read-failure classification ladder, shared by the per-tag path and the span path so @@ -1252,8 +1443,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) { @@ -1262,19 +1455,38 @@ 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. + // It is ALSO an answer from a reachable CPU: LastSuccessfulRead must advance, or a + // factory-default CPU (every read denied) trips the comms-loss watchdog into a + // reconnect + re-subscribe every 90 s that can never fix a PUT/GET setting. + // Faulted still wins the pass (worst state), so the config alert is kept. + outcome.RecordAnswered(); 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 @@ -1295,10 +1507,10 @@ public sealed class S7Driver private async Task ReadOneAsync(IS7Plc plc, S7TagDefinition tag, CancellationToken ct) { - // Every tag pre-parses its address at init (_parsedByRawPath); the on-demand + // Every tag pre-parses its address at init (ParsedByRawPath); the on-demand // S7AddressParser.Parse is a defensive fallback for a tag not in that table. Parse throws // FormatException on a bad address, which the caller's catch maps to BadCommunicationError. - var addr = _parsedByRawPath.TryGetValue(tag.Name, out var parsed) + var addr = ParsedByRawPath.TryGetValue(tag.Name, out var parsed) ? parsed : S7AddressParser.Parse(tag.Address); @@ -1378,7 +1590,7 @@ public sealed class S7Driver /// true if the tag routes through the byte-buffer codec. internal static bool IsBufferType(S7TagDefinition tag, S7ParsedAddress addr) => addr.Area is S7Area.Timer or S7Area.Counter - || ByteAnchoredDataTypes.Contains(tag.DataType); + || S7TagShapeRules.IsByteAnchored(tag.DataType); /// /// Reads a wide/structured scalar as ONE contiguous byte block via S7.Net's @@ -1849,8 +2061,11 @@ public sealed class S7Driver var outcome = new WritePassOutcome(); await _gate.WaitAsync(cancellationToken).ConfigureAwait(false); + var gen = CurrentGeneration; // see ReadAsync try { + RequireInitialized(); + // (Re)open under the gate; a failed (re)connect degrades every write to // BadCommunicationError rather than throwing. IS7Plc plc; @@ -1891,13 +2106,20 @@ public sealed class S7Driver // Timer/Counter, but the mapper doesn't run that guard). // TryParse (not Parse) so a malformed address falls through to the existing // error path rather than throwing here. - if (_parsedByRawPath.TryGetValue(tag.Name, out var tcParsed) + if (ParsedByRawPath.TryGetValue(tag.Name, out var tcParsed) ? tcParsed.Area is S7Area.Timer or S7Area.Counter : (S7AddressParser.TryParse(tag.Address, out tcParsed) && tcParsed.Area is S7Area.Timer or S7Area.Counter)) { results[i] = new WriteResult(StatusBadNotWritable); continue; } + // Fail the rest of the batch fast once a write marked the handle dead (R07:P1): the + // next call reopens; this batch must not serialise one deadline per remaining write. + if (_plcDead) + { + results[i] = new WriteResult(StatusBadCommunicationError); + continue; + } try { await WriteOneAsync(plc, tag, w.Value, cancellationToken).ConfigureAwait(false); @@ -1929,15 +2151,26 @@ 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 + // a config-alert health state, not a transient device failure. results[i] = new WriteResult(StatusBadNotSupported); + // An answer (see the read ladder): advances LastSuccessfulRead. RecordAnswered sets + // AnyAnswered only, so the sticky _writeFault still clears only on a real success. + outcome.RecordAnswered(); 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) { @@ -1964,7 +2197,7 @@ public sealed class S7Driver { // THE single health write of the batch (a finally, so an OCE mid-batch still flushes the // evidence gathered so far — a pass that observed nothing writes nothing). - ApplyWritePassHealth(outcome); + ApplyWritePassHealth(gen, outcome); _gate.Release(); } return results; @@ -1976,10 +2209,11 @@ public sealed class S7Driver /// successful sibling. Maintains , the sticky Faulted a read pass /// must not erase. Caller holds . /// + /// The lifecycle generation the batch ran in; a superseded batch writes nothing. /// The batch's accumulated outcomes. - private void ApplyWritePassHealth(WritePassOutcome outcome) + private void ApplyWritePassHealth(long gen, WritePassOutcome outcome) { - if (!outcome.HasObservations) return; + if (!outcome.HasObservations || !IsCurrentGeneration(gen)) return; var worst = outcome.WorstFailure; var priorWriteFault = _writeFault; @@ -1988,15 +2222,24 @@ public sealed class S7Driver else if (outcome.AnySuccess) _writeFault = null; // a write got through — the protection fault is gone + var answeredAt = outcome.AnyAnswered ? 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 - && Health.State == DriverState.Faulted && Health.LastError == owned.Message) - Health = new DriverHealth(DriverState.Healthy, Health.LastSuccessfulRead, 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; } @@ -2008,7 +2251,8 @@ public sealed class S7Driver state = wf.State; error = wf.Message; } - Health = new DriverHealth(state, Health.LastSuccessfulRead, error); + if (state == DriverState.Degraded) _writeOwnedError = error; + TryUpdateHealth(gen, h => new DriverHealth(state, answeredAt ?? h.LastSuccessfulRead, error)); } /// @@ -2026,10 +2270,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. @@ -2039,10 +2291,10 @@ public sealed class S7Driver private async Task WriteOneAsync(IS7Plc plc, S7TagDefinition tag, object? value, CancellationToken ct) { // Parse the address the same way ReadOneAsync does: every tag pre-parses at init - // (_parsedByRawPath), with an on-demand parse as a defensive fallback. Needed here so the + // (ParsedByRawPath), with an on-demand parse as a defensive fallback. Needed here so the // wide-type write can byte-address the block (the narrow path below addresses by the raw // address string instead). - var addr = _parsedByRawPath.TryGetValue(tag.Name, out var parsed) + var addr = ParsedByRawPath.TryGetValue(tag.Name, out var parsed) ? parsed : S7AddressParser.Parse(tag.Address); @@ -2312,6 +2564,37 @@ public sealed class S7Driver if (IsS7ConnectionFatal(ex)) _plcDead = true; } + /// + /// The result stamped, without any I/O, on an item a pass skipped because an earlier item had + /// already marked the connection dead (R07:P1). The pass's health already carries the failure + /// that marked it. + /// + /// The pass timestamp. + /// A BadCommunicationError snapshot. + private static DataValueSnapshot DeadHandleSnapshot(DateTime now) => new(null, StatusBadCommunicationError, null, now); + + /// + /// The TRANSPORT subset of : a socket / IO / disposed handle / + /// blown deadline / anywhere in the chain. Excludes the + /// framing class, because S7.Net reports a multi-var request with one refused item as + /// PlcException(ReadData, PlcException(WrongNumberReceivedBytes)) (live-probed on the snap7 + /// fixture) over a perfectly healthy stream — the multi-var path falls back per tag for that + /// shape, and a genuine desync still surfaces on the per-tag reads. + /// + /// The exception to classify. + /// for a transport loss. + private static bool IsS7TransportLoss(Exception ex) + { + for (Exception? e = ex; e is not null; e = e.InnerException) + { + if (e is System.Net.Sockets.SocketException or System.IO.IOException or ObjectDisposedException or TimeoutException) + return true; + if (e is PlcException { ErrorCode: ErrorCode.ConnectionError }) + return true; + } + return false; + } + /// /// True when (or any inner exception) is a socket-level / connection /// loss OR an ISO-on-TCP framing/desync fault that a reopen can repair — a @@ -2365,22 +2648,89 @@ 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; + // A transport fault wins over any wording in the chain: a socket error whose text happens to + // contain the phrase must reopen (Degraded), not latch a sticky config alert. + if (IsS7ConnectionFatal(ex) || HasCancellationInChain(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)) + // S7.Net's exact wording for the S7 AccessingObjectNotAllowed status — not the loose "not + // allowed", which any unrelated exception text can contain. + if (e.Message.Contains("Accessing object not allowed", StringComparison.OrdinalIgnoreCase)) { return true; } @@ -2401,7 +2751,7 @@ public sealed class S7Driver { ArgumentNullException.ThrowIfNull(builder); var folder = builder.Folder("S7", "S7"); - foreach (var t in _tagsByRawPath.Values) + foreach (var t in TagsByRawPath.Values) { // A tag carrying a non-null array count (>= 1) surfaces as a 1-D OPC UA array node. // A null count stays scalar. A count of 1 IS a valid 1-element array: the foundation @@ -2465,9 +2815,20 @@ public sealed class S7Driver /// The exception caught by the poll engine. private void HandlePollError(Exception ex) { + // A retained poll that ran into a teardown / re-initialize IN PROGRESS ("not initialized" while a + // lifecycle operation holds _lifecycleGate) belongs to no live generation: that operation owns + // health, and writing Degraded over its Initializing (or its final Unknown) would break the "old + // work never overwrites newer health" rule. A poll against a driver nobody is initializing (a + // subscribe before init, a failed init) still surfaces Degraded (Driver.S7-009). + if (!_initialized && _lifecycleGate.CurrentCount == 0) + { + _logger.LogDebug(ex, "S7 poll reader failed during a lifecycle operation; no health write. Driver={DriverInstanceId}", _driverInstanceId); + return; + } _logger.LogWarning(ex, "S7 poll reader failed. Driver={DriverInstanceId}", _driverInstanceId); - if (Health.State != DriverState.Faulted) - Health = new DriverHealth(DriverState.Degraded, Health.LastSuccessfulRead, ex.Message); + TryUpdateHealth(CurrentGeneration, h => h.State == DriverState.Faulted + ? null + : new DriverHealth(DriverState.Degraded, h.LastSuccessfulRead, ex.Message)); } // ---- IHostConnectivityProbe ---- @@ -2490,66 +2851,96 @@ public sealed class S7Driver { while (!ct.IsCancellationRequested) { - var success = false; + // Running / Stopped, or null for "no verdict this tick" — which leaves the host state alone. + HostState? verdict = null; try { // Probe via S7.Net's low-cost GetCpuStatus — returns the CPU state (Run/Stop) // and is intentionally light on the comms mailbox. Single-word Plc.ReadAsync // would also work but GetCpuStatus doubles as a "PLC actually up" check. - using var gateCts = CancellationTokenSource.CreateLinkedTokenSource(ct); - gateCts.CancelAfter(_options.Probe.Timeout); - - await _gate.WaitAsync(gateCts.Token).ConfigureAwait(false); - try + // + // Gate CONTENTION is not a verdict (R07:H7): a read pass or write batch holding the + // gate past the probe deadline is the connection being USED, and that pass reports + // its own outcome through health. Reporting the host Stopped for it flapped /hosts on + // every long pass against a perfectly healthy PLC. + if (!await _gate.WaitAsync(_options.Probe.Timeout, ct).ConfigureAwait(false)) + { + Interlocked.Increment(ref _probeGateContentions); + } + else { try { - // EnsureConnectedAsync doubles as the reconnect backstop: if a read/write - // marked the handle dead (or the socket dropped between ticks), the probe - // reopens it here so recovery doesn't wait for the next data call. - // It runs on the LOOP token, not the (shorter) probe deadline: the connect is bounded by - // _options.Timeout inside, and a timeout there must count as a failure so the - // backoff window opens — cutting it with the probe deadline looked like a caller cancel, - // recorded nothing, and let a black-holed host cost a connect attempt every probe tick. - var plc = await EnsureConnectedAsync(ct).ConfigureAwait(false); - // The probe deadline bounds only the CPU-status round trip. - using var probeCts = CancellationTokenSource.CreateLinkedTokenSource(ct); - probeCts.CancelAfter(_options.Probe.Timeout); - await plc.ReadStatusAsync(probeCts.Token).ConfigureAwait(false); - success = true; - } - catch (OperationCanceledException) when (ct.IsCancellationRequested) - { - // Loop teardown — rethrow to the outer filter (after finally releases the gate). - throw; - } - catch (OperationCanceledException) - { - // Probe deadline fired mid-PDU: the CPU-status response may still be in - // flight, so the handle can't be reused (half-read ⇒ desync). We hold _gate - // here, keeping _plcDead gate-guarded. - _plcDead = true; - } - catch (Exception ex) - { - // Classify + mark while holding _gate so a wire-dead/desynced - // handle whose TcpClient still claims Connected is reopened by the NEXT - // tick's EnsureConnectedAsync instead of being re-probed forever. - MarkConnectionDeadIfFatal(ex); + // A tick that raced a teardown / re-initialize has nothing to probe and must + // not reopen a connection on a driver that is not initialized. + if (_initialized) + verdict = await ProbeOnceAsync(ct).ConfigureAwait(false); } + finally { _gate.Release(); } } - finally { _gate.Release(); } } catch (OperationCanceledException) when (ct.IsCancellationRequested) { return; } - catch { /* gate-wait timeout / other — treated as Stopped below (gate contention is not a connection fault) */ } + catch { verdict = HostState.Stopped; /* any other probe fault */ } - TransitionTo(success ? HostState.Running : HostState.Stopped); + if (verdict is { } state) TransitionTo(state); try { await Task.Delay(_options.Probe.Interval, ct).ConfigureAwait(false); } catch (OperationCanceledException) { return; } } } + /// + /// One probe round trip. Caller holds . Returns the verdict; rethrows only + /// loop teardown. + /// + /// The probe loop's token. + /// or . + private async Task ProbeOnceAsync(CancellationToken ct) + { + try + { + // EnsureConnectedAsync doubles as the reconnect backstop: if a read/write marked the handle + // dead (or the socket dropped between ticks), the probe reopens it here so recovery doesn't + // wait for the next data call. It runs on the LOOP token, not the (shorter) probe deadline: + // the connect is bounded by _options.Timeout inside, and a timeout there must count as a + // failure so the backoff window opens — cutting it with the probe deadline looked like a + // caller cancel, recorded nothing, and let a black-holed host cost a connect attempt every + // probe tick. + var plc = await EnsureConnectedAsync(ct).ConfigureAwait(false); + // The probe deadline bounds only the CPU-status round trip. + using var probeCts = CancellationTokenSource.CreateLinkedTokenSource(ct); + probeCts.CancelAfter(_options.Probe.Timeout); + await plc.ReadStatusAsync(probeCts.Token).ConfigureAwait(false); + return HostState.Running; + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; // loop teardown — the caller's finally releases the gate + } + catch (OperationCanceledException) + { + // Probe deadline fired mid-PDU: the CPU-status response may still be in flight, so the + // handle can't be reused (half-read ⇒ desync). The caller holds _gate, keeping _plcDead + // gate-guarded. + _plcDead = true; + return HostState.Stopped; + } + catch (Exception ex) + { + // Classify + mark while holding _gate so a wire-dead/desynced handle whose TcpClient still + // claims Connected is reopened by the NEXT tick's EnsureConnectedAsync instead of being + // re-probed forever. + MarkConnectionDeadIfFatal(ex); + return HostState.Stopped; + } + } + + /// Probe ticks that found the gate held past the probe deadline and gave no verdict. + private int _probeGateContentions; + + /// Count of probe ticks skipped for gate contention. Internal so a test can wait on it. + internal int ProbeGateContentions => Volatile.Read(ref _probeGateContentions); + private void TransitionTo(HostState newState) { HostState old; @@ -2576,7 +2967,10 @@ public sealed class S7Driver if (_disposed) return; _disposed = true; SynchronousTeardown(); - _gate.Dispose(); + // _gate / _lifecycleGate are deliberately NOT disposed: a Dispose can race a still-running + // ShutdownAsync (the runtime disposes right after a bounded shutdown), which would then throw + // ObjectDisposedException mid-teardown. A SemaphoreSlim whose wait handle was never requested + // holds no unmanaged resource. } /// Asynchronously disposes the driver and releases resources. @@ -2585,9 +2979,10 @@ public sealed class S7Driver { if (_disposed) return; _disposed = true; + // Safe while a ShutdownAsync is still running: it waits (bounded) for the lifecycle gate, and + // the teardown is idempotent. The semaphores are not disposed (see Dispose). try { await ShutdownAsync(CancellationToken.None).ConfigureAwait(false); } catch { /* disposal is best-effort */ } - _gate.Dispose(); } /// @@ -2597,6 +2992,10 @@ public sealed class S7Driver /// private void SynchronousTeardown() { + // Its own lifecycle generation, like ShutdownCoreAsync: an initialize still running past this + // point discards what it built instead of publishing it over the torn-down driver. + var gen = BeginGeneration(); + // Drain the polling overlay (drain-before-dispose). try { _poll.DisposeAsync().AsTask().Wait(DrainTimeout); } catch { /* timeouts/loop faults are tolerated — teardown continues */ } @@ -2617,10 +3016,11 @@ public sealed class S7Driver _initialized = false; _plcDead = false; _writeFault = null; + _writeOwnedError = null; _connectBackoff.RecordSuccess(); try { Plc?.Dispose(); } catch { /* best-effort — tearing down anyway */ } Plc = null; Volatile.Write(ref _negotiatedPduSize, 0); // a stale post-teardown reading would read as "online" - Health = new DriverHealth(DriverState.Unknown, Health.LastSuccessfulRead, null); + TryUpdateHealth(gen, h => new DriverHealth(DriverState.Unknown, h.LastSuccessfulRead, null)); } } 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..ad53b8b7 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverProbe.cs @@ -42,6 +42,14 @@ public sealed class S7DriverProbe : IDriverProbe if (string.IsNullOrWhiteSpace(host) || port <= 0) return new(false, "Config has no host/port to probe.", null); + // The S7 handshake + test read are bounded by the driver's own per-operation timeout + // (config "timeoutMs", what the deployed driver will wait), capped by the caller's overall + // Test Connect timeout. The options type carries it as a TimeSpan the JSON never names, so it + // used to be ignored and every handshake waited the full Test Connect timeout (R07:H7). + var opTimeout = ReadTimeoutMs(configJson) is { } ms && TimeSpan.FromMilliseconds(ms) < timeout + ? TimeSpan.FromMilliseconds(ms) + : timeout; + // Phase 1: bare TCP preflight — fast rejection for unreachable hosts. var sw = Stopwatch.StartNew(); try @@ -71,9 +79,9 @@ public sealed class S7DriverProbe : IDriverProbe var plc = S7Tsap.Resolve(opts) is { } tsap ? new Plc(host, port, tsap) : new Plc(S7CpuTypeMap.ToS7Net(opts.CpuType), host, port, opts.Rack, opts.Slot); - plc.ReadTimeout = (int)timeout.TotalMilliseconds; + plc.ReadTimeout = (int)opTimeout.TotalMilliseconds; using var handshakeCts = CancellationTokenSource.CreateLinkedTokenSource(ct); - handshakeCts.CancelAfter(timeout); + handshakeCts.CancelAfter(opTimeout); try { await plc.OpenAsync(handshakeCts.Token); @@ -128,15 +136,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) { @@ -146,4 +156,31 @@ public sealed class S7DriverProbe : IDriverProbe private static (string host, int port) ExtractTarget(S7DriverOptions opts) => (opts.Host, opts.Port); + + /// + /// The config's timeoutMs (any casing, as the driver factory binds it), or null when absent + /// or not a positive integer. + /// + /// The driver config. + /// The configured per-operation timeout in milliseconds, or null. + internal static int? ReadTimeoutMs(string configJson) + { + try + { + using var doc = JsonDocument.Parse(configJson); + if (doc.RootElement.ValueKind != JsonValueKind.Object) return null; + foreach (var prop in doc.RootElement.EnumerateObject()) + { + if (!string.Equals(prop.Name, "timeoutMs", StringComparison.OrdinalIgnoreCase)) continue; + return prop.Value.ValueKind == JsonValueKind.Number && prop.Value.TryGetInt32(out var ms) && ms > 0 + ? ms + : null; + } + return null; + } + catch (JsonException) + { + return null; + } + } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7ReadSpanPlanner.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7ReadSpanPlanner.cs index 96f7770a..dfe05b77 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7ReadSpanPlanner.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7ReadSpanPlanner.cs @@ -11,8 +11,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.S7; /// The safety invariant is that a span never extends past its last member's end. A /// span may deliberately read bytes nobody asked for BETWEEN its members (that is what the /// gap budget buys), but every byte it requests lies inside a range some member was going -/// to read anyway. A grouped read therefore cannot fault (area overrun, DB overrun) where -/// the individual per-tag reads would have succeeded. +/// to read anyway. +/// +/// +/// That does NOT make a grouped read as safe as the per-tag reads (R07:P2): the block is +/// the union of its members' ranges, so ONE member the CPU refuses (running past the DB's real +/// end, an over-declared STRING) makes it refuse the whole block, neighbours included. +/// owns that case: a refused multi-member span is re-read one +/// member at a time, and the members that must not be grouped are left out of later plans. /// /// /// Timer and Counter tags are never grouped. Their diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/S7TagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/S7TagConfigModel.cs index 49b4e199..d02db4d9 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/S7TagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/S7TagConfigModel.cs @@ -112,17 +112,15 @@ public sealed class S7TagConfigModel + string.Join(", ", Enum.GetNames()) + "."; } - // Array shape is judged against the driver's INIT guards, which THROW (failing the whole - // driver's init, not just this tag) for a wide-type array and for a writable Bool array. The + // Array shape: the friendlier editor wording for two of the S7TagShapeRules refusals (a wide-type + // array, a writable Bool array). The reader below refuses them too (and the rest of the shape + // rules), so this is wording, not a second authority. The // isArray / arrayLength keys themselves belong to the TagModal's generic array controls // (TagArrayConfig) and are intentionally not fields of this model, so read them off the key bag. if (TagConfigJson.GetBool(_bag, "isArray")) { var dt = DataType; - if (dt is S7DataType.Int64 or S7DataType.UInt64 or S7DataType.Float64 - or S7DataType.String or S7DataType.DateTime or S7DataType.WString or S7DataType.Char - or S7DataType.WChar or S7DataType.Dtl or S7DataType.Date or S7DataType.Tod - or S7DataType.Time or S7DataType.S5Time) + if (S7TagShapeRules.IsByteAnchored(dt)) { return $"S7 does not support {dt} arrays — use Bool (read-only), Byte, Int16, UInt16, Int32, " + "UInt32 or Float32 for an array tag, or a scalar tag."; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/FreezableTcpProxy.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/FreezableTcpProxy.cs new file mode 100644 index 00000000..6c5e48ce --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/FreezableTcpProxy.cs @@ -0,0 +1,104 @@ +using System.Net; +using System.Net.Sockets; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests; + +/// +/// An in-test TCP forwarder in front of the S7 fixture that can be FROZEN: while frozen it keeps +/// every socket open but forwards nothing in either direction — the frozen-but-established peer +/// (cable pull, firewall DROP) that only a wire-op deadline can detect, produced without touching +/// the shared fixture. It counts the ISO-on-TCP (TPKT) request frames the client sends while frozen, +/// so a test can assert how many wire operations went out without timing anything. +/// +internal sealed class FreezableTcpProxy : IAsyncDisposable +{ + private readonly string _targetHost; + private readonly int _targetPort; + private readonly TcpListener _listener = new(IPAddress.Loopback, 0); + private readonly CancellationTokenSource _cts = new(); + private readonly List _sockets = []; + private volatile bool _frozen; + private int _framesWhileFrozen; + + /// Starts a proxy forwarding to :. + /// The fixture host. + /// The fixture port. + public FreezableTcpProxy(string targetHost, int targetPort) + { + _targetHost = targetHost; + _targetPort = targetPort; + _listener.Start(); + _ = AcceptLoopAsync(); + } + + /// The local port clients connect to. + public int Port => ((IPEndPoint)_listener.LocalEndpoint).Port; + + /// TPKT request frames the client sent while the proxy was frozen. + public int FramesWhileFrozen => Volatile.Read(ref _framesWhileFrozen); + + /// Stops forwarding (sockets stay open). + public void Freeze() => _frozen = true; + + /// Resumes forwarding for connections made from now on. + public void Unfreeze() => _frozen = false; + + private async Task AcceptLoopAsync() + { + try + { + while (!_cts.IsCancellationRequested) + { + var client = await _listener.AcceptTcpClientAsync(_cts.Token).ConfigureAwait(false); + var upstream = new TcpClient(); + await upstream.ConnectAsync(_targetHost, _targetPort, _cts.Token).ConfigureAwait(false); + lock (_sockets) { _sockets.Add(client); _sockets.Add(upstream); } + // A connection accepted while frozen stays frozen for its whole life (its peer is "gone"). + var frozenAtBirth = _frozen; + _ = PumpAsync(client.GetStream(), upstream.GetStream(), countFrames: true, frozenAtBirth); + _ = PumpAsync(upstream.GetStream(), client.GetStream(), countFrames: false, frozenAtBirth); + } + } + catch { /* disposed */ } + } + + private async Task PumpAsync(NetworkStream from, NetworkStream to, bool countFrames, bool frozenAtBirth) + { + var buffer = new byte[8192]; + var pending = new List(); + try + { + while (!_cts.IsCancellationRequested) + { + var n = await from.ReadAsync(buffer, _cts.Token).ConfigureAwait(false); + if (n == 0) return; + var frozen = _frozen || frozenAtBirth; + if (countFrames && frozen) + { + // Count complete TPKT frames: 0x03 0x00 then a big-endian total length. + pending.AddRange(buffer.AsSpan(0, n).ToArray()); + while (pending.Count >= 4) + { + var len = (pending[2] << 8) | pending[3]; + if (len < 4 || pending.Count < len) break; + pending.RemoveRange(0, len); + Interlocked.Increment(ref _framesWhileFrozen); + } + } + if (frozen) continue; // swallow — the peer never answers + await to.WriteAsync(buffer.AsMemory(0, n), _cts.Token).ConfigureAwait(false); + } + } + catch { /* socket closed / disposed */ } + } + + /// + public ValueTask DisposeAsync() + { + _cts.Cancel(); + _listener.Stop(); + lock (_sockets) foreach (var s in _sockets) s.Dispose(); + _cts.Dispose(); + return ValueTask.CompletedTask; + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500FrozenPeerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500FrozenPeerTests.cs new file mode 100644 index 00000000..088931de --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500FrozenPeerTests.cs @@ -0,0 +1,71 @@ +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:P1 over real S7comm: a frozen-but-established peer (produced by an in-test +/// , so the shared fixture is never touched). The first wire op of +/// the pass blows its deadline and marks the connection dead; the rest of the pass must NOT go to +/// the wire. The assertion counts request frames, not seconds. +/// +/// +/// What this gate does and does not prove. Verified by mutation (2026-10-22): on the real +/// stack this test also passes with the driver's dead-handle guard removed, because S7.Net 0.20 +/// honours the deadline's cancellation token and closes its own socket, so the next op fails at once +/// with ConnectionError and sends nothing. The driver guard (proven by +/// S7DeadHandleShortCircuitTests against a fake that does not self-close) is what covers an +/// op that ignores its token — the WaitAsync leg of S7OperationDeadline, where the +/// abandoned socket stays open and every further op would pay the full deadline. This test pins the +/// end-to-end property (one request per frozen pass, then recovery) against S7.Net upgrades. +/// +[Collection(Snap7ServerCollection.Name)] +[Trait("Category", "Integration")] +[Trait("Device", "S7_1500")] +public sealed class S7_1500FrozenPeerTests(Snap7ServerFixture sim) +{ + /// A pass against a frozen peer issues ONE request, then fails the rest fast; it recovers after. + [Fact] + public async Task A_frozen_peer_costs_one_wire_op_per_pass_not_one_per_tag() + { + if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + var ct = TestContext.Current.CancellationToken; + + await using var proxy = new FreezableTcpProxy(sim.Host, sim.Port); + var baseOptions = S7_1500Profile.BuildOptions("127.0.0.1", proxy.Port); + var options = new S7DriverOptions + { + Host = baseOptions.Host, Port = baseOptions.Port, CpuType = baseOptions.CpuType, + Rack = baseOptions.Rack, Slot = baseOptions.Slot, + Timeout = TimeSpan.FromSeconds(1), // the per-op deadline that detects the frozen peer + Probe = baseOptions.Probe, + SpanReadMaxGapBytes = -1, // one wire request per tag, so "one per tag" is countable + RawTags = baseOptions.RawTags, + }; + string[] refs = + [ + S7_1500Profile.ProbeTag, S7_1500Profile.SmokeI16Tag, S7_1500Profile.SmokeI32Tag, + S7_1500Profile.SmokeF32Tag, S7_1500Profile.SmokeBoolTag, S7_1500Profile.WriteScratchTag, + ]; + + await using var drv = new S7Driver(options, driverInstanceId: "s7-frozen-peer-live"); + await drv.InitializeAsync("{}", ct); + + // Positive control: through the proxy, every tag reads Good. + (await drv.ReadAsync(refs, ct)).ShouldAllBe(s => s.StatusCode == 0u); + + proxy.Freeze(); + var frozen = await drv.ReadAsync(refs, ct); + frozen.ShouldAllBe(s => s.StatusCode != 0u); + proxy.FramesWhileFrozen.ShouldBe(1, "only the request that discovered the dead connection may go out"); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + + // Recovery: the next pass reopens (a fresh proxy connection) and reads Good again. + proxy.Unfreeze(); + var recovered = await drv.ReadAsync(refs, ct); + recovered.ShouldAllBe(s => s.StatusCode == 0u); + Convert.ToInt32(recovered[0].Value).ShouldBe(S7_1500Profile.ProbeSeedValue); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } +} 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..79015983 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500RefusalTests.cs @@ -0,0 +1,85 @@ +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; + // Let the wall clock move past the init stamp so "advanced" is asserted strictly (review m-1). + while (DateTime.UtcNow <= before) await Task.Yield(); + + // 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.ShouldBeGreaterThan(before, "the refusal is an answer: the stamp advances"); + + // 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.IntegrationTests/S7_1500/S7_1500SpanRefusalTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500SpanRefusalTests.cs new file mode 100644 index 00000000..96dde491 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500SpanRefusalTests.cs @@ -0,0 +1,49 @@ +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:P2 over real S7comm. The fixture's M area is 256 bytes. MW10 is seeded +/// (4097); MD254 runs two bytes past the end of the area. With the gap budget opened wide +/// the planner merges them into ONE block MB10..258, which the CPU refuses — before the fix +/// that made the perfectly readable MW10 Bad (and the driver Degraded) on every poll. +/// +[Collection(Snap7ServerCollection.Name)] +[Trait("Category", "Integration")] +[Trait("Device", "S7_1500")] +public sealed class S7_1500SpanRefusalTests(Snap7ServerFixture sim) +{ + /// The good neighbour of an over-running member keeps reading its seeded value, every pass. + [Fact] + public async Task A_member_past_the_area_end_does_not_spoil_its_span_neighbour() + { + if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + var ct = TestContext.Current.CancellationToken; + + var options = new S7DriverOptions + { + Host = sim.Host, Port = sim.Port, CpuType = S7CpuType.S71500, + Timeout = TimeSpan.FromSeconds(5), + Probe = new S7ProbeOptions { Enabled = false }, + // Wide enough that MW10 and MD254 land in one planned span. + SpanReadMaxGapBytes = 256, + SpanReadMaxSpanBytes = 256, + RawTags = S7RawTags.Entries( + new S7TagDefinition("Mk/Word10", "MW10", S7DataType.UInt16, Writable: false), + new S7TagDefinition("Mk/PastEnd", "MD254", S7DataType.Int32, Writable: false)), + }; + await using var drv = new S7Driver(options, driverInstanceId: "s7-span-refusal-live"); + await drv.InitializeAsync("{}", ct); + + for (var pass = 0; pass < 3; pass++) + { + var r = await drv.ReadAsync(["Mk/Word10", "Mk/PastEnd"], ct); + r[0].StatusCode.ShouldBe(0u, $"pass {pass}: the in-range neighbour must be served"); + Convert.ToInt32(r[0].Value).ShouldBe(4097); + r[1].StatusCode.ShouldNotBe(0u, "the over-running member is refused on its own"); + drv.GetHealth().State.ShouldBe(DriverState.Healthy, "a refusal is not a comms fault"); + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500TagShapeSkipTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500TagShapeSkipTests.cs new file mode 100644 index 00000000..17bc9a61 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500TagShapeSkipTests.cs @@ -0,0 +1,57 @@ +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:H3 — one mis-authored tag must not darken the PLC. Before the fix each of +/// these shapes threw at init, so the driver Faulted and served nothing (and the actor retried the +/// same init every 10 s forever). Now each is skipped on its own and the good tags keep reading +/// their seeded values over real S7comm. +/// +[Collection(Snap7ServerCollection.Name)] +[Trait("Category", "Integration")] +[Trait("Device", "S7_1500")] +public sealed class S7_1500TagShapeSkipTests(Snap7ServerFixture sim) +{ + /// Unservable tags are skipped per tag; the seeded neighbours still read Good. + [Fact] + public async Task Mis_authored_tags_are_skipped_and_the_rest_of_the_PLC_still_reads() + { + if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + var ct = TestContext.Current.CancellationToken; + + var good = S7_1500Profile.BuildOptions(sim.Host, sim.Port); + var bad = new[] + { + new S7TagDefinition("Bad/Typo", "DB1.DBZ0", S7DataType.Int16), + new S7TagDefinition("Bad/WideOnWord", "DB1.DBW0", S7DataType.Float64), + new S7TagDefinition("Bad/TimerAsInt", "T5", S7DataType.Int32, Writable: false), + new S7TagDefinition("Bad/OffsetBoolArray", "DB1.DBX50.3", S7DataType.Bool, Writable: false, ArrayCount: 8), + }; + var options = new S7DriverOptions + { + Host = good.Host, Port = good.Port, CpuType = good.CpuType, Rack = good.Rack, Slot = good.Slot, + Timeout = good.Timeout, Probe = good.Probe, + RawTags = [.. good.RawTags, .. S7RawTags.Entries(bad)], + }; + + await using var drv = new S7Driver(options, driverInstanceId: "s7-shape-skip-live"); + await drv.InitializeAsync("{}", ct); + + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + drv.GetSkippedTags().ShouldNotBeNull().Sample.Select(s => s.RawPath) + .ShouldBe(bad.Select(b => b.Name), ignoreOrder: true); + + var r = await drv.ReadAsync( + [S7_1500Profile.ProbeTag, S7_1500Profile.SmokeI16Tag, S7_1500Profile.SmokeBoolTag, bad[0].Name], ct); + r[0].StatusCode.ShouldBe(0u); + Convert.ToInt32(r[0].Value).ShouldBe(S7_1500Profile.ProbeSeedValue); + r[1].StatusCode.ShouldBe(0u); + Convert.ToInt32(r[1].Value).ShouldBe((int)S7_1500Profile.SmokeI16SeedValue); + r[2].StatusCode.ShouldBe(0u); + Convert.ToBoolean(r[2].Value).ShouldBeTrue(); + r[3].StatusCode.ShouldBe(0x80340000u, "a skipped tag reads BadNodeIdUnknown"); + } +} 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/S7ArrayWriteAndHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ArrayWriteAndHealthTests.cs index fab2b496..27c5e20a 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ArrayWriteAndHealthTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ArrayWriteAndHealthTests.cs @@ -243,7 +243,9 @@ public sealed class S7ArrayWriteAndHealthTests var (drv, factory) = await TwoTagDriver(); using var _ = drv; factory.Created[0].WriteFaults["DB1.DBW0"] = new PlcException(ErrorCode.WriteData, "Accessing object not allowed"); - factory.Created[0].WriteFaults["DB1.DBW2"] = new IOException("socket glitch"); + // A Degraded failure that is NOT connection-fatal: since R07:P1 a fatal one (an IOException) + // would fail the rest of the batch without I/O, and C must reach the wire to be the success. + factory.Created[0].WriteFaults["DB1.DBW2"] = new PlcException(ErrorCode.SendData, "device glitch"); var res = await drv.WriteAsync( [new WriteRequest("A", (ushort)1), new WriteRequest("B", (ushort)2), new WriteRequest("C", (ushort)3)], diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DeadHandleShortCircuitTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DeadHandleShortCircuitTests.cs new file mode 100644 index 00000000..852e9d47 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DeadHandleShortCircuitTests.cs @@ -0,0 +1,167 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// R07:P1 — once a pass has marked the connection dead (a wire op blew its deadline on a frozen +/// peer), the rest of that pass must not go to the wire: each further op would pay the full +/// timeout under the gate, so a 20-span pass took ~100 s and the comms-loss watchdog's clock only +/// started after it. The remaining items are stamped BadCommunicationError without I/O. +/// Every assertion counts wire operations; nothing is timed. +/// +[Trait("Category", "Unit")] +public sealed class S7DeadHandleShortCircuitTests +{ + private const uint BadCommunicationError = 0x80050000u; + + private static S7DriverOptions Options(int maxGap, params S7TagDefinition[] tags) => new() + { + Host = "192.0.2.1", + Timeout = TimeSpan.FromSeconds(5), + Probe = new S7ProbeOptions { Enabled = false }, + SpanReadMaxGapBytes = maxGap, + RawTags = S7RawTags.Entries(tags), + }; + + /// Five isolated words — far enough apart that each is its own span. + private static S7TagDefinition[] FarApartWords() => + [.. Enumerable.Range(0, 5).Select(i => new S7TagDefinition($"W{i}", $"DB{i + 1}.DBW0", S7DataType.UInt16, Writable: true))]; + + private static string[] Refs(int n) => [.. Enumerable.Range(0, n).Select(i => $"W{i}")]; + + private static async Task<(S7Driver, ScriptedS7PlcFactory)> StartAsync(S7DriverOptions options) + { + var factory = new ScriptedS7PlcFactory(); + var drv = new S7Driver(options, "s7-dead-short-circuit", factory); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + return (drv, factory); + } + + /// Per-tag path: the first timeout ends the pass's wire traffic. + [Fact] + public async Task Per_tag_pass_stops_issuing_wire_ops_once_the_handle_is_dead() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory) = await StartAsync(Options(maxGap: -1, FarApartWords())); + using var _ = drv; + factory.Read = (_, _) => throw new TimeoutException("frozen peer"); + + var before = factory.WireOps; + var r = await drv.ReadAsync(Refs(5), ct); + + (factory.WireOps - before).ShouldBe(1, "only the op that discovered the dead handle may go to the wire"); + r.ShouldAllBe(s => s.StatusCode == BadCommunicationError); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + + // Positive control: the next pass reopens and serves every tag. + factory.Read = (_, _) => Task.FromResult((ushort)1); + var recovered = await drv.ReadAsync(Refs(5), ct); + recovered.ShouldAllBe(s => s.StatusCode == 0u); + factory.Created.Count.ShouldBe(2); + } + + /// Span path: arrays are block reads even alone, so five arrays are five spans. + [Fact] + public async Task Span_pass_stops_issuing_wire_ops_once_the_handle_is_dead() + { + var ct = TestContext.Current.CancellationToken; + S7TagDefinition[] arrays = + [.. Enumerable.Range(0, 5).Select(i => new S7TagDefinition($"W{i}", $"DB{i + 1}.DBW0", S7DataType.UInt16, Writable: false, ArrayCount: 2))]; + var (drv, factory) = await StartAsync(Options(maxGap: 16, arrays)); + using var _ = drv; + factory.ReadBytes = (_, _, _, _, _) => throw new TimeoutException("frozen peer"); + + var before = factory.WireOps; + var r = await drv.ReadAsync(Refs(5), ct); + + (factory.WireOps - before).ShouldBe(1); + r.ShouldAllBe(s => s.StatusCode == BadCommunicationError); + } + + /// + /// Multi-var path: a chunk that dies on a transport loss used to fall back to one per-tag read + /// per member — each paying the timeout again. Now it marks the handle dead and the fallback is + /// short-circuited. + /// + [Fact] + public async Task Multi_var_transport_loss_does_not_fall_back_to_per_tag_wire_reads() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory) = await StartAsync(Options(maxGap: 0, FarApartWords())); + using var _ = drv; + factory.ReadMulti = (_, _) => throw new TimeoutException("frozen peer"); + factory.Read = (_, _) => throw new TimeoutException("frozen peer"); + + var before = factory.WireOps; + var r = await drv.ReadAsync(Refs(5), ct); + + (factory.WireOps - before).ShouldBe(1); + r.ShouldAllBe(s => s.StatusCode == BadCommunicationError); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + } + + /// + /// Positive control for the classification: a multi-var REFUSAL (not a transport loss) still + /// falls back per tag, so the members the CPU will serve are served. + /// + [Fact] + public async Task Multi_var_refusal_still_falls_back_per_tag() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory) = await StartAsync(Options(maxGap: 0, FarApartWords())); + using var _ = drv; + // The live snap7 shape of a multi-var with one refused item. + factory.ReadMulti = (_, _) => throw new global::S7.Net.PlcException(global::S7.Net.ErrorCode.ReadData, + new global::S7.Net.PlcException(global::S7.Net.ErrorCode.WrongNumberReceivedBytes)); + factory.Read = (_, _) => Task.FromResult((ushort)9); + + var before = factory.WireOps; + var r = await drv.ReadAsync(Refs(5), ct); + + (factory.WireOps - before).ShouldBe(6, "one multi-var request, then one per-tag read each"); + r.ShouldAllBe(s => s.StatusCode == 0u); + factory.Created.Count.ShouldBe(1, "a refusal is not a reason to reopen"); + } + + /// A refusal does not short-circuit the pass: the CPU answered, the handle is fine. + [Fact] + public async Task A_refusal_does_not_short_circuit_the_pass() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory) = await StartAsync(Options(maxGap: -1, FarApartWords())); + using var _ = drv; + factory.Read = (addr, _) => addr == "DB1.DBW0" + ? throw new Exception("Received error from PLC: Object does not exist.") + : Task.FromResult((ushort)3); + + var before = factory.WireOps; + var r = await drv.ReadAsync(Refs(5), ct); + + (factory.WireOps - before).ShouldBe(5); + r.Skip(1).ShouldAllBe(s => s.StatusCode == 0u); + } + + /// Write batch: the first transport loss fails the rest of the batch without I/O. + [Fact] + public async Task Write_batch_stops_issuing_wire_ops_once_the_handle_is_dead() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory) = await StartAsync(Options(maxGap: -1, FarApartWords())); + using var _ = drv; + factory.Write = (_, _, _) => throw new System.IO.IOException("connection reset"); + + var before = factory.WireOps; + var w = await drv.WriteAsync([.. Refs(5).Select(r => new WriteRequest(r, (ushort)1))], ct); + + (factory.WireOps - before).ShouldBe(1); + w.ShouldAllBe(x => x.StatusCode == BadCommunicationError); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + + // Positive control: the next batch reopens and writes. + factory.Write = (_, _, _) => Task.CompletedTask; + (await drv.WriteAsync([.. Refs(5).Select(r => new WriteRequest(r, (ushort)1))], ct)) + .ShouldAllBe(x => x.StatusCode == 0u); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverCodeReviewFixTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverCodeReviewFixTests.cs index 9c1c3a8a..6f0f12b8 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverCodeReviewFixTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverCodeReviewFixTests.cs @@ -25,26 +25,10 @@ public sealed class S7DriverCodeReviewFixTests [InlineData("C10")] public async Task Initialize_rejects_timer_or_counter_tag_with_NotSupportedException(string address) { - // A Timer/Counter address parses cleanly but the read path has no decode case for it, - // so it must fail fast at init rather than throw a misleading type-mismatch on every - // read. The host is reserved-for-documentation so the TCP connect can never succeed — - // the unsupported-address guard runs before the connect, so the NotSupportedException - // is what surfaces. - var opts = new S7DriverOptions - { - Host = "192.0.2.1", - Timeout = TimeSpan.FromMilliseconds(250), - RawTags = S7RawTags.Entries(new S7TagDefinition("Quirk", address, S7DataType.Int16)), - }; - using var drv = new S7Driver(opts, "s7-tc"); - - var ex = await Should.ThrowAsync(async () => - await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); - ex.Message.ShouldContain(address); - - var health = drv.GetHealth(); - health.State.ShouldBe(DriverState.Faulted, "an unsupported-address config error must Fault the driver"); - health.LastError.ShouldNotBeNull(); + // A Timer/Counter address typed anything but Float64 (Timer) / Int32 (Counter) cannot be decoded. + // It is refused PER TAG at every gate; it used to throw at init and fault every tag on the PLC. + await S7TagSkipAssert.RefusedPerTagAsync( + new S7TagDefinition("Quirk", address, S7DataType.Int16, Writable: false), address); } /// Verifies that initialize accepts DB and MIQ addresses without triggering the unsupported guard. @@ -102,19 +86,21 @@ public sealed class S7DriverCodeReviewFixTests [Fact] public async Task Initialize_rejects_a_timer_tag_supplied_only_through_driverConfigJson() { - // The Timer/Counter guard must run against the *re-parsed* config, not just the - // constructor options — proves Driver.S7-001 and Driver.S7-011 compose correctly. + // The shape gate must run against the *re-parsed* config, not just the constructor options — + // proves the per-tag skip and Driver.S7-011 compose correctly. var ctorOpts = new S7DriverOptions { Host = "192.0.2.1", Timeout = TimeSpan.FromMilliseconds(250) }; - using var drv = new S7Driver(ctorOpts, "s7-cfg-tc"); + using var drv = new S7Driver(ctorOpts, "s7-cfg-tc", new ScriptedS7PlcFactory()); const string json = """ - { "Host": "192.0.2.1", "TimeoutMs": 250, + { "Host": "192.0.2.1", "TimeoutMs": 250, "Probe": { "Enabled": false }, "RawTags": [ { "RawPath": "TimerTag", "TagConfig": "{\"address\":\"T5\",\"dataType\":\"Int16\"}", "WriteIdempotent": false } ] } """; - var ex = await Should.ThrowAsync(async () => - await drv.InitializeAsync(json, TestContext.Current.CancellationToken)); - ex.Message.ShouldContain("T5"); + await drv.InitializeAsync(json, TestContext.Current.CancellationToken); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + var skipped = drv.GetSkippedTags().ShouldNotBeNull().Sample.ShouldHaveSingleItem(); + skipped.RawPath.ShouldBe("TimerTag"); + skipped.Reason.ShouldContain("T5"); } /// Verifies that reinitialize applies a changed driverConfigJson. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverCodeReviewFixTests2.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverCodeReviewFixTests2.cs index 75731de5..59bb0922 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverCodeReviewFixTests2.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverCodeReviewFixTests2.cs @@ -158,23 +158,9 @@ public sealed class S7DriverCodeReviewFixTests2 [InlineData(S7DataType.DateTime)] public async Task Initialize_rejects_wide_type_at_non_byte_address_with_NotSupportedException(S7DataType dt) { - var opts = new S7DriverOptions - { - Host = "192.0.2.1", - Timeout = TimeSpan.FromMilliseconds(250), - // DB1.DBD0 is a DWord (non-byte) address — guard-(b) must fault on the address shape - // for a wide type. (At a byte address like DB1.DBB0 the 8-byte numerics round-trip; - // see S7ScalarBlockTests.) - RawTags = S7RawTags.Entries(new S7TagDefinition("X", "DB1.DBD0", dt)), - }; - using var drv = new S7Driver(opts, $"s7-wide-nonbyte-{dt}"); - - var ex = await Should.ThrowAsync(async () => - await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); - ex.Message.ShouldContain(dt.ToString()); - - var health = drv.GetHealth(); - health.State.ShouldBe(DriverState.Faulted); + // DB1.DBD0 is a DWord (non-byte) address — a wide type there would mis-frame its value. (At a + // byte address like DB1.DBB0 the 8-byte numerics round-trip; see S7ScalarBlockTests.) + await S7TagSkipAssert.RefusedPerTagAsync(new S7TagDefinition("X", "DB1.DBD0", dt), dt.ToString(), "byte-addressed"); } /// Verifies that Initialize accepts implemented data types. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverReadWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverReadWriteTests.cs index 9f59d91c..d5bca500 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverReadWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverReadWriteTests.cs @@ -17,22 +17,11 @@ public sealed class S7DriverReadWriteTests [Fact] public async Task Initialize_rejects_invalid_tag_address_and_fails_fast() { - // Bad address at init time must throw; the alternative (deferring the parse to the - // first read) would surface the config bug as BadInternalError on every subsequent - // Read which is impossible for an operator to diagnose from the OPC UA client. - var opts = new S7DriverOptions - { - Host = "192.0.2.1", // reserved — will never complete TCP handshake - Timeout = TimeSpan.FromMilliseconds(250), - RawTags = S7RawTags.Entries(new S7TagDefinition("BadTag", "NOT-AN-S7-ADDRESS", S7DataType.Int16)), - }; - using var drv = new S7Driver(opts, "s7-bad-tag"); - - // Either the TCP connect fails first (Exception) or the parser fails (FormatException) - // — both are acceptable since both are init-time fail-fast. What matters is that we - // don't return a "healthy" driver with a latent bad tag. - await Should.ThrowAsync(async () => - await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); + // A bad address is refused PER TAG at every gate (mapper/editor, deploy gate, driver init): + // it reads BadNodeIdUnknown while its neighbours keep working. It used to throw at init and + // fault every tag on the PLC, with the actor retrying the same init every 10 s forever. + await S7TagSkipAssert.RefusedPerTagAsync( + new S7TagDefinition("BadTag", "NOT-AN-S7-ADDRESS", S7DataType.Int16), "does not parse"); } /// Verifies that ReadAsync without initialize throws InvalidOperationException. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverScaffoldTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverScaffoldTests.cs index 6c1c6561..a8e4e52e 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverScaffoldTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverScaffoldTests.cs @@ -101,80 +101,32 @@ public sealed class S7DriverScaffoldTests [Fact] public async Task Initialize_rejects_wide_type_array_with_clear_message() { - var opts = new S7DriverOptions - { - Host = "192.0.2.1", - Timeout = TimeSpan.FromMilliseconds(250), - RawTags = S7RawTags.Entries(new S7TagDefinition("Batch64", "DB1.DBB0", S7DataType.Int64, ArrayCount: 4)), - }; - using var drv = new S7Driver(opts, "s7-wide-array"); - - var ex = await Should.ThrowAsync(async () => - await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); - ex.Message.ShouldContain("Batch64"); - ex.Message.ShouldContain("array", Case.Insensitive); - - drv.GetHealth().State.ShouldBe(DriverState.Faulted); + // Refused PER TAG at every gate (it used to throw at init and fault the whole driver). + await S7TagSkipAssert.RefusedPerTagAsync(new S7TagDefinition("Batch64", "DB1.DBB0", S7DataType.Int64, ArrayCount: 4), "Int64", "array"); } /// Verifies that a wide scalar with a non-byte address (Float64 at DB1.DBW0) is rejected with a byte-address message. [Fact] public async Task Initialize_rejects_wide_scalar_with_non_byte_address() { - var opts = new S7DriverOptions - { - Host = "192.0.2.1", - Timeout = TimeSpan.FromMilliseconds(250), - RawTags = S7RawTags.Entries(new S7TagDefinition("WideWord", "DB1.DBW0", S7DataType.Float64)), - }; - using var drv = new S7Driver(opts, "s7-wide-nonbyte"); - - var ex = await Should.ThrowAsync(async () => - await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); - ex.Message.ShouldContain("WideWord"); - ex.Message.ShouldContain("byte", Case.Insensitive); - - drv.GetHealth().State.ShouldBe(DriverState.Faulted); + // Refused PER TAG at every gate (it used to throw at init and fault the whole driver). + await S7TagSkipAssert.RefusedPerTagAsync(new S7TagDefinition("WideWord", "DB1.DBW0", S7DataType.Float64), "Float64", "byte-addressed"); } /// Verifies that a Timer tag with the wrong DataType (Int32, not Float64) is rejected. [Fact] public async Task Initialize_rejects_Timer_tag_with_wrong_data_type() { - var opts = new S7DriverOptions - { - Host = "192.0.2.1", - Timeout = TimeSpan.FromMilliseconds(250), - RawTags = S7RawTags.Entries(new S7TagDefinition("Timer5", "T5", S7DataType.Int32)), - }; - using var drv = new S7Driver(opts, "s7-timer-bad"); - - var ex = await Should.ThrowAsync(async () => - await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); - ex.Message.ShouldContain("Timer5"); - ex.Message.ShouldContain("Float64"); - - drv.GetHealth().State.ShouldBe(DriverState.Faulted); + // Refused PER TAG at every gate (it used to throw at init and fault the whole driver). + await S7TagSkipAssert.RefusedPerTagAsync(new S7TagDefinition("Timer5", "T5", S7DataType.Int32, Writable: false), "T5", "Float64"); } /// Verifies that a Counter tag with the wrong DataType (Float32, not Int32) is rejected. [Fact] public async Task Initialize_rejects_Counter_tag_with_wrong_data_type() { - var opts = new S7DriverOptions - { - Host = "192.0.2.1", - Timeout = TimeSpan.FromMilliseconds(250), - RawTags = S7RawTags.Entries(new S7TagDefinition("Counter3", "C3", S7DataType.Float32)), - }; - using var drv = new S7Driver(opts, "s7-counter-bad"); - - var ex = await Should.ThrowAsync(async () => - await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); - ex.Message.ShouldContain("Counter3"); - ex.Message.ShouldContain("Int32"); - - drv.GetHealth().State.ShouldBe(DriverState.Faulted); + // Refused PER TAG at every gate (it used to throw at init and fault the whole driver). + await S7TagSkipAssert.RefusedPerTagAsync(new S7TagDefinition("Counter3", "C3", S7DataType.Float32, Writable: false), "C3", "Int32"); } // ── Phase 4d bundle-review fix — Writable Timer/Counter guard ─────────────────────── @@ -188,42 +140,16 @@ public sealed class S7DriverScaffoldTests [Fact] public async Task Initialize_rejects_Timer_tag_declared_Writable() { - var opts = new S7DriverOptions - { - Host = "192.0.2.1", - Timeout = TimeSpan.FromMilliseconds(250), - RawTags = S7RawTags.Entries(new S7TagDefinition("Timer5", "T5", S7DataType.Float64, Writable: true)), - }; - using var drv = new S7Driver(opts, "s7-timer-writable"); - - var ex = await Should.ThrowAsync(async () => - await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); - ex.Message.ShouldContain("Timer5"); - ex.Message.ShouldContain("Writable", Case.Insensitive); - ex.Message.ShouldContain("read-only", Case.Insensitive); - - drv.GetHealth().State.ShouldBe(DriverState.Faulted); + // Refused PER TAG at every gate (it used to throw at init and fault the whole driver). + await S7TagSkipAssert.RefusedPerTagAsync(new S7TagDefinition("Timer5", "T5", S7DataType.Float64, Writable: true), "T5", "writable", "read-only"); } /// Verifies that a Counter tag declared Writable=true is rejected at init with a Writable message. [Fact] public async Task Initialize_rejects_Counter_tag_declared_Writable() { - var opts = new S7DriverOptions - { - Host = "192.0.2.1", - Timeout = TimeSpan.FromMilliseconds(250), - RawTags = S7RawTags.Entries(new S7TagDefinition("Counter3", "C3", S7DataType.Int32, Writable: true)), - }; - using var drv = new S7Driver(opts, "s7-counter-writable"); - - var ex = await Should.ThrowAsync(async () => - await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); - ex.Message.ShouldContain("Counter3"); - ex.Message.ShouldContain("Writable", Case.Insensitive); - ex.Message.ShouldContain("read-only", Case.Insensitive); - - drv.GetHealth().State.ShouldBe(DriverState.Faulted); + // Refused PER TAG at every gate (it used to throw at init and fault the whole driver). + await S7TagSkipAssert.RefusedPerTagAsync(new S7TagDefinition("Counter3", "C3", S7DataType.Int32, Writable: true), "C3", "writable", "read-only"); } // ── Driver.S7-015 / S7-016 — writable Bool array must fail fast at init ───────────── @@ -236,20 +162,8 @@ public sealed class S7DriverScaffoldTests [Fact] public async Task Initialize_rejects_writable_bool_array_tag_with_NotSupportedException() { - var opts = new S7DriverOptions - { - Host = "192.0.2.1", - Timeout = TimeSpan.FromMilliseconds(250), - RawTags = S7RawTags.Entries(new S7TagDefinition("ArrTag", "DB1.DBX0.0", S7DataType.Bool, Writable: true, ArrayCount: 4)), - }; - using var drv = new S7Driver(opts, "s7-writable-bool-arr"); - - var ex = await Should.ThrowAsync(async () => - await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); - ex.Message.ShouldContain("ArrTag"); - ex.Message.ShouldContain("Bool array", Case.Insensitive); - - drv.GetHealth().State.ShouldBe(DriverState.Faulted); + // Refused PER TAG at every gate (it used to throw at init and fault the whole driver). + await S7TagSkipAssert.RefusedPerTagAsync(new S7TagDefinition("ArrTag", "DB1.DBX0.0", S7DataType.Bool, Writable: true, ArrayCount: 4), "Bool array", "writable"); } /// Verifies that writable non-bool arrays pass the init guard — the failure diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7LifecycleGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7LifecycleGateTests.cs new file mode 100644 index 00000000..1c99a61e --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7LifecycleGateTests.cs @@ -0,0 +1,206 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// R07:H5 — overlapping lifecycle calls. DriverInstanceActor dispatches initialize on a +/// fire-and-forget task and a reconnect retry can start a second attempt while the first is still in +/// OpenAsync; an abandoned initialize can also still be running when the next shutdown starts. +/// Every wait here is a condition wait on the fake's own signals, never a sleep. +/// +[Trait("Category", "Unit")] +public sealed class S7LifecycleGateTests +{ + private static readonly TimeSpan Bound = TimeSpan.FromSeconds(10); + + private static S7DriverOptions Options(bool probe = false) => new() + { + Host = "192.0.2.1", + Timeout = TimeSpan.FromSeconds(30), // the fake's gated open is released by the test, never timed out + Probe = new S7ProbeOptions { Enabled = probe, Interval = TimeSpan.FromSeconds(30) }, + RawTags = S7RawTags.Entries(new S7TagDefinition("W0", "DB1.DBW0", S7DataType.UInt16, Writable: false)), + }; + + /// A factory whose FIRST open blocks until released; later opens complete immediately. + private sealed class FirstOpenBlocks + { + public readonly ScriptedS7PlcFactory Factory = new(); + public readonly TaskCompletionSource FirstOpenEntered = new(TaskCreationOptions.RunContinuationsAsynchronously); + public readonly TaskCompletionSource ReleaseFirstOpen = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public FirstOpenBlocks() + { + Factory.Open = async (index, ct) => + { + if (index != 0) return; + FirstOpenEntered.TrySetResult(); + await ReleaseFirstOpen.Task.WaitAsync(ct).ConfigureAwait(false); + }; + } + } + + /// + /// A second initialize waits for the first instead of running beside it, then releases the first + /// attempt's connection: exactly one live connection remains. Before the gate, attempt B opened + /// its own connection while A was still in OpenAsync, and A's published connection (and + /// probe loop) was overwritten and never released. + /// + [Fact] + public async Task Overlapping_initializes_are_serialised_and_leave_one_live_connection() + { + var ct = TestContext.Current.CancellationToken; + var f = new FirstOpenBlocks(); + using var drv = new S7Driver(Options(), "s7-overlap", f.Factory); + + var a = drv.InitializeAsync("{}", ct); + await f.FirstOpenEntered.Task.WaitAsync(Bound, ct); + var b = drv.InitializeAsync("{}", ct); + + // Positive control first: B is genuinely pending (not finished, not faulted)... + b.IsCompleted.ShouldBeFalse(); + // ...and while A holds the gate B has not created a connection of its own. B can only reach + // its open after A completes, so this is not a race the test can lose by being slow. + f.Factory.Created.Count.ShouldBe(1, "a second initialize must not open beside the first"); + + f.ReleaseFirstOpen.SetResult(); + await a.WaitAsync(Bound, ct); + await b.WaitAsync(Bound, ct); + + f.Factory.Created.Count.ShouldBe(2); + f.Factory.Created[0].Disposed.ShouldBeTrue("B releases A's published connection before replacing it"); + f.Factory.Created[1].Disposed.ShouldBeFalse(); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + (await drv.ReadAsync(["W0"], ct))[0].StatusCode.ShouldBe(0u); + } + + /// + /// An initialize still running when a shutdown gives up waiting for it must not publish its + /// connection, its health or its probe loop over the shut-down driver; it fails and disposes what + /// it opened. Before the fix the late attempt published Healthy + a live connection after the + /// shutdown returned. + /// + [Fact] + public async Task An_initialize_superseded_by_shutdown_publishes_nothing() + { + var ct = TestContext.Current.CancellationToken; + var f = new FirstOpenBlocks(); + using var drv = new S7Driver(Options(probe: true), "s7-superseded", f.Factory) + { + LifecycleGateTimeout = TimeSpan.FromSeconds(1), + }; + + var init = drv.InitializeAsync("{}", ct); + await f.FirstOpenEntered.Task.WaitAsync(Bound, ct); + + // The shutdown cannot get the lifecycle gate (the attempt holds it), gives up after its bound + // and tears down anyway. + await drv.ShutdownAsync(ct).WaitAsync(Bound, ct); + drv.GetHealth().State.ShouldBe(DriverState.Unknown); + + // The abandoned attempt now completes its open... + f.ReleaseFirstOpen.SetResult(); + var ex = await Should.ThrowAsync(() => init.WaitAsync(Bound, ct)); + ex.Message.ShouldContain("superseded"); + + // ...and published nothing: no connection kept, health untouched, the driver still shut down. + f.Factory.Created.ShouldHaveSingleItem().Disposed.ShouldBeTrue(); + drv.GetHealth().State.ShouldBe(DriverState.Unknown, "a superseded attempt must not write Healthy or Faulted"); + await Should.ThrowAsync(() => drv.ReadAsync(["W0"], ct)); + + // Positive control: the driver is not wedged — a fresh initialize publishes normally. + await drv.InitializeAsync("{}", ct).WaitAsync(Bound, ct); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + (await drv.ReadAsync(["W0"], ct))[0].StatusCode.ShouldBe(0u); + } + + /// + /// Review m-4: a retained poll that runs into a re-initialize in progress ("not initialized") + /// must not write Degraded over the new attempt's Initializing. Waits on the poll's own log line + /// (a counted signal), then asserts; the control is that a successful init then reports Healthy. + /// + [Fact] + public async Task A_retained_poll_does_not_overwrite_a_reinitialize_in_progress() + { + var ct = TestContext.Current.CancellationToken; + var factory = new ScriptedS7PlcFactory(); + var secondOpenEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseSecondOpen = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + factory.Open = async (index, token) => + { + if (index != 1) return; + secondOpenEntered.TrySetResult(); + await releaseSecondOpen.Task.WaitAsync(token); + }; + var log = new CapturingS7Logger(); + using var drv = new S7Driver(Options(), "s7-retained-poll", factory, log); + await drv.InitializeAsync("{}", ct); + await drv.SubscribeAsync(["W0"], TimeSpan.FromMilliseconds(100), ct); + + var reinit = drv.InitializeAsync("{}", ct); + await secondOpenEntered.Task.WaitAsync(Bound, ct); + + using (var cts = CancellationTokenSource.CreateLinkedTokenSource(ct)) + { + cts.CancelAfter(Bound); + while (log.Entries.Count(e => e.Message.Contains("during a lifecycle operation")) < 2) + await Task.Delay(20, cts.Token); // condition wait on the poll's counted log line + } + drv.GetHealth().State.ShouldBe(DriverState.Initializing, "a retained poll must not write over the new attempt"); + + releaseSecondOpen.SetResult(); + await reinit.WaitAsync(Bound, ct); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// Review m-8: the SYNCHRONOUS Dispose() racing a still-running ShutdownAsync is safe too + /// — neither throws, the superseded init publishes nothing, and the driver ends torn down. + /// + [Fact] + public async Task Sync_dispose_while_a_shutdown_is_still_running_is_safe() + { + var ct = TestContext.Current.CancellationToken; + var f = new FirstOpenBlocks(); + var drv = new S7Driver(Options(), "s7-sync-dispose-race", f.Factory) { LifecycleGateTimeout = TimeSpan.FromSeconds(1) }; + + var init = drv.InitializeAsync("{}", ct); + await f.FirstOpenEntered.Task.WaitAsync(Bound, ct); + + var shutdown = drv.ShutdownAsync(ct); // waits on the gate the blocked init holds + Should.NotThrow(() => drv.Dispose()); + await shutdown.WaitAsync(Bound, ct); + + f.ReleaseFirstOpen.SetResult(); + await Should.ThrowAsync(() => init.WaitAsync(Bound, ct)); + f.Factory.Created.ShouldHaveSingleItem().Disposed.ShouldBeTrue(); + drv.GetHealth().State.ShouldBe(DriverState.Unknown); + } + + /// + /// Dispose racing a shutdown that is still running is safe: neither throws, and the driver ends + /// torn down. The semaphores are no longer disposed, so the running shutdown cannot hit an + /// ObjectDisposedException. + /// + [Fact] + public async Task Dispose_while_a_shutdown_is_still_running_is_safe() + { + var ct = TestContext.Current.CancellationToken; + var f = new FirstOpenBlocks(); + var drv = new S7Driver(Options(), "s7-dispose-race", f.Factory) { LifecycleGateTimeout = TimeSpan.FromSeconds(1) }; + + var init = drv.InitializeAsync("{}", ct); + await f.FirstOpenEntered.Task.WaitAsync(Bound, ct); + + var shutdown = drv.ShutdownAsync(ct); // waits on the gate the blocked init holds + var dispose = drv.DisposeAsync().AsTask(); + await shutdown.WaitAsync(Bound, ct); + await dispose.WaitAsync(Bound, ct); + + f.ReleaseFirstOpen.SetResult(); + await Should.ThrowAsync(() => init.WaitAsync(Bound, ct)); + f.Factory.Created.ShouldHaveSingleItem().Disposed.ShouldBeTrue(); + drv.GetHealth().State.ShouldBe(DriverState.Unknown); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ProbeContentionAndTimeoutTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ProbeContentionAndTimeoutTests.cs new file mode 100644 index 00000000..6289a9ea --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ProbeContentionAndTimeoutTests.cs @@ -0,0 +1,121 @@ +using System.Net; +using System.Net.Sockets; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// R07:H7 — (a) the background probe used to report the host Stopped whenever a long read +/// pass held the gate past the probe deadline: contention is the connection being USED, not lost; +/// (b) Test Connect ignored the config's timeoutMs. Waits are condition waits on counters and +/// events; bounds are generous. +/// +[Trait("Category", "Unit")] +public sealed class S7ProbeContentionAndTimeoutTests +{ + private static readonly TimeSpan Bound = TimeSpan.FromSeconds(15); + + private static async Task WaitUntilAsync(Func condition, CancellationToken ct) + { + using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); + cts.CancelAfter(Bound); + while (!condition()) + await Task.Delay(20, cts.Token); // polling a condition, not asserting on elapsed time + } + + /// A read pass holding the gate past the probe deadline does not make the host Stopped. + [Fact] + public async Task Gate_contention_is_no_verdict_but_a_real_probe_failure_still_is() + { + var ct = TestContext.Current.CancellationToken; + var factory = new ScriptedS7PlcFactory(); + var readEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseRead = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var drv = new S7Driver(new S7DriverOptions + { + Host = "192.0.2.1", + Timeout = TimeSpan.FromSeconds(30), + Probe = new S7ProbeOptions { Enabled = true, Interval = TimeSpan.FromMilliseconds(50), Timeout = TimeSpan.FromSeconds(1) }, + RawTags = S7RawTags.Entries(new S7TagDefinition("W0", "DB1.DBW0", S7DataType.UInt16, Writable: false)), + }, "s7-probe-contention", factory); + var transitions = new List(); + drv.OnHostStatusChanged += (_, e) => { lock (transitions) transitions.Add(e.NewState); }; + await drv.InitializeAsync("{}", ct); + + await WaitUntilAsync(() => drv.GetHostStatuses()[0].State == HostState.Running, ct); + + // A read that holds the gate until released. + factory.Read = async (_, token) => + { + readEntered.TrySetResult(); + await releaseRead.Task.WaitAsync(token); + return (ushort)1; + }; + var read = drv.ReadAsync(["W0"], ct); + await readEntered.Task.WaitAsync(Bound, ct); + + // At least two probe ticks give up on the gate... + var contentionsBefore = drv.ProbeGateContentions; + await WaitUntilAsync(() => drv.ProbeGateContentions >= contentionsBefore + 2, ct); + // ...and the host is still Running — no Stopped transition was published for them. + drv.GetHostStatuses()[0].State.ShouldBe(HostState.Running); + lock (transitions) transitions.ShouldNotContain(HostState.Stopped); + + releaseRead.SetResult(); + (await read.WaitAsync(Bound, ct))[0].StatusCode.ShouldBe(0u); + + // Positive control: a genuine probe failure still reports Stopped. + factory.ReadStatus = _ => throw new System.IO.IOException("connection reset"); + await WaitUntilAsync(() => drv.GetHostStatuses()[0].State == HostState.Stopped, ct); + } + + /// Test Connect's handshake is bounded by the config's timeoutMs, not the caller's whole budget. + [Fact] + public async Task Test_connect_honours_the_config_timeoutMs() + { + var ct = TestContext.Current.CancellationToken; + // A TCP endpoint that accepts and then never answers the ISO-on-TCP handshake. + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var held = new List(); + _ = Task.Run(async () => + { + try { while (true) { var c = await listener.AcceptTcpClientAsync(ct); lock (held) held.Add(c); } } + catch { /* stopped */ } + }, ct); + try + { + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + var json = $$"""{"host":"127.0.0.1","port":{{port}},"timeoutMs":1000}"""; + + // The caller's budget (60 s) is far beyond the safety token (20 s): only a probe that honours + // timeoutMs returns its own "handshake failed: timed out"; one that waits the caller's budget + // is cut by the safety token and reports the caller-timeout message instead. + using var safety = CancellationTokenSource.CreateLinkedTokenSource(ct); + safety.CancelAfter(TimeSpan.FromSeconds(20)); + var r = await new S7DriverProbe().ProbeAsync(json, TimeSpan.FromSeconds(60), safety.Token); + + r.Ok.ShouldBeFalse(); + r.Message!.ShouldContain("handshake failed: timed out"); + safety.IsCancellationRequested.ShouldBeFalse(); + } + finally + { + listener.Stop(); + lock (held) foreach (var c in held) c.Dispose(); + } + } + + /// The config reader: any casing, positive integers only. + [Fact] + public void ReadTimeoutMs_reads_the_driver_key() + { + S7DriverProbe.ReadTimeoutMs("""{"TimeoutMs":2500}""").ShouldBe(2500); + S7DriverProbe.ReadTimeoutMs("""{"timeoutms":10}""").ShouldBe(10); + S7DriverProbe.ReadTimeoutMs("""{"timeoutMs":0}""").ShouldBeNull(); + S7DriverProbe.ReadTimeoutMs("""{"host":"x"}""").ShouldBeNull(); + S7DriverProbe.ReadTimeoutMs("not json").ShouldBeNull(); + } +} 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..8c7c5182 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7RefusalHealthTests.cs @@ -0,0 +1,293 @@ +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)), + }; + + /// A stepped pass clock, so "LastSuccessfulRead advanced" is asserted STRICTLY. + private sealed class PassClock + { + public DateTime Now { get; private set; } = new(2026, 10, 22, 12, 0, 0, DateTimeKind.Utc); + + /// Moves the clock one second on and returns the new time. + public DateTime Step() => Now = Now.AddSeconds(1); + } + + private static async Task<(S7Driver Driver, ScriptedS7PlcFactory Factory, CapturingS7Logger Log, PassClock Clock)> StartAsync() + { + var factory = new ScriptedS7PlcFactory(); + var log = new CapturingS7Logger(); + var clock = new PassClock(); + var drv = new S7Driver(Options(), "s7-refusal", factory, log) { UtcNow = () => clock.Now }; + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + drv.GetHealth().LastSuccessfulRead.ShouldBe(clock.Now, "init stamps the pass clock"); + return (drv, factory, log, clock); + } + + /// 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, clock) = await StartAsync(); + using var _ = drv; + factory.Read = (addr, _) => addr == "DB99.DBW0" ? throw ReadRefusal() : Task.FromResult((ushort)7); + + for (var pass = 0; pass < 3; pass++) + { + var at = clock.Step(); + 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.ShouldBe(at, + "the CPU answered, so the all-refused pass advances LastSuccessfulRead to the pass time"); + } + 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, _, clock) = await StartAsync(); + using var __ = drv; + + // Positive control: a genuine comms fault DOES degrade, and leaves LastSuccessfulRead frozen. + var initStamp = clock.Now; + clock.Step(); + factory.Read = (_, _) => throw new TimeoutException("frozen peer"); + await drv.ReadAsync(["Missing"], ct); + var degraded = drv.GetHealth(); + degraded.State.ShouldBe(DriverState.Degraded); + degraded.LastSuccessfulRead.ShouldBe(initStamp); + + var at = clock.Step(); + 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.ShouldBe(at); + } + + /// 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, _, clock) = 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, _, clock) = 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"); + } + + /// + /// Review I-1: an access-denied CPU (PUT/GET left at its factory default — EVERY read denied) is + /// reachable and answering, so each pass advances LastSuccessfulRead while staying Faulted. + /// Before, the pass stayed Faulted with the stamp frozen — the comms-loss watchdog's trip + /// condition, so the actor reconnected + re-subscribed every 90 s, forever, which can never fix + /// a PUT/GET setting. The control: a comms fault afterwards leaves the stamp frozen. + /// + [Fact] + public async Task An_all_access_denied_pass_stays_faulted_but_advances_last_successful_read() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory, _, clock) = await StartAsync(); + using var __ = drv; + factory.Read = (_, _) => throw new Exception("Received error from PLC: Accessing object not allowed."); + + DateTime last = default; + for (var pass = 0; pass < 2; pass++) + { + last = clock.Step(); + await drv.ReadAsync(["Good", "Missing"], ct); + var h = drv.GetHealth(); + h.State.ShouldBe(DriverState.Faulted, "the config alert is kept"); + h.LastSuccessfulRead.ShouldBe(last, "the CPU answered, so the stamp advances every pass"); + } + + // Positive control: a real comms loss does NOT advance it (and the sticky Faulted outranks it). + clock.Step(); + factory.Read = (_, _) => throw new TimeoutException("frozen peer"); + await drv.ReadAsync(["Good"], ct); + drv.GetHealth().LastSuccessfulRead.ShouldBe(last); + } + + /// The write-side access-denied is an answer too, and its sticky Faulted still clears only on a real success. + [Fact] + public async Task A_write_access_denied_advances_last_successful_read_and_stays_sticky() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory, _, clock) = await StartAsync(); + using var __ = drv; + factory.Write = (_, _, _) => throw new PlcException(ErrorCode.WriteData, + new Exception("Received error from PLC: Accessing object not allowed.")); + + var at = clock.Step(); + (await drv.WriteAsync([new WriteRequest("Good", (ushort)1)], ct))[0].StatusCode.ShouldBe(0x803D0000u); + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + drv.GetHealth().LastSuccessfulRead.ShouldBe(at); + + // A clean read leaves the write-side Faulted in place (sticky) … + clock.Step(); + await drv.ReadAsync(["Good"], ct); + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + + // … and a real write success clears it. + factory.Write = (_, _, _) => Task.CompletedTask; + clock.Step(); + await drv.WriteAsync([new WriteRequest("Good", (ushort)1)], ct); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// Review m-7: a transport fault whose text happens to contain "not allowed" reopens (Degraded), + /// and only S7.Net's exact "Accessing object not allowed" is the access-denied alert. + /// + [Fact] + public async Task Only_the_exact_access_denied_wording_without_a_transport_fault_is_access_denied() + { + var ct = TestContext.Current.CancellationToken; + var (drv, factory, _, clock) = await StartAsync(); + using var __ = drv; + + clock.Step(); + factory.Write = (_, _, _) => throw new PlcException(ErrorCode.WriteData, + new System.IO.IOException("Accessing object not allowed: socket closed by peer")); + (await drv.WriteAsync([new WriteRequest("Good", (ushort)1)], ct))[0].StatusCode + .ShouldNotBe(0x803D0000u, "not the access-denied BadNotSupported"); + drv.GetHealth().State.ShouldBe(DriverState.Degraded, "a transport fault is not a config alert"); + + S7Driver.IsAccessDenied(new PlcException(ErrorCode.WriteData, new Exception("operation not allowed"))).ShouldBeFalse(); + S7Driver.IsAccessDenied(new Exception("Received error from PLC: Accessing object not allowed.")).ShouldBeTrue(); + } + + /// 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, _, clock) = 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, clock) = 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 at = clock.Step(); + var w = await drv.WriteAsync([new WriteRequest("Missing", (ushort)1)], ct); + w[0].StatusCode.ShouldBe(BadDeviceFailure); + var h = drv.GetHealth(); + h.LastSuccessfulRead.ShouldBe(at, "a write refusal is an answer: the stamp advances strictly"); + // The Degraded was written by the READ side (its own message), so it is the read pass's to clear. + h.State.ShouldBe(DriverState.Degraded); + } + + var after = drv.GetHealth(); + after.LastSuccessfulRead!.Value.ShouldBeGreaterThan(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, _, clock) = 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/S7SpanRefusalFallbackTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanRefusalFallbackTests.cs new file mode 100644 index 00000000..11b7358d --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanRefusalFallbackTests.cs @@ -0,0 +1,213 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// R07:P2 — a span's block is the union of its members' ranges, so ONE member that runs past the +/// DB's real end (or over-declares a STRING) made the CPU refuse the whole block and every +/// neighbour inside it went Bad on every poll. A refused span now re-reads its members one at a +/// time for that pass and remembers which tags must not be grouped, so later passes group only the +/// members that can be grouped. +/// +[Trait("Category", "Unit")] +public sealed class S7SpanRefusalFallbackTests +{ + private const uint BadDeviceFailure = 0x808B0000u; + + /// DB1 is 4 bytes long on this fake CPU: anything reaching past byte 4 is refused. + private const int DbLength = 4; + + private static Exception OutOfRange() => new("Received error from PLC: Address out of range."); + + private static (S7Driver Driver, ScriptedS7PlcFactory Factory, Func BlockReads, Func TagReads) Start( + params S7TagDefinition[] tags) + { + var factory = new ScriptedS7PlcFactory(); + var blockReads = 0; + var tagReads = 0; + factory.ReadBytes = (_, _, start, count, _) => + { + Interlocked.Increment(ref blockReads); + if (start + count > DbLength) throw OutOfRange(); + var b = new byte[count]; + for (var i = 0; i < count; i++) b[i] = (byte)(start + i + 1); + return Task.FromResult(b); + }; + factory.Read = (addr, _) => + { + Interlocked.Increment(ref tagReads); + var offset = int.Parse(addr[(addr.LastIndexOf('W') + 1)..], System.Globalization.CultureInfo.InvariantCulture); + if (offset + 2 > DbLength) throw OutOfRange(); + return Task.FromResult((ushort)(((offset + 1) << 8) | (offset + 2))); + }; + var drv = new S7Driver(new S7DriverOptions + { + Host = "192.0.2.1", + Probe = new S7ProbeOptions { Enabled = false }, + RawTags = S7RawTags.Entries(tags), + }, "s7-span-fallback", factory); + return (drv, factory, () => Volatile.Read(ref blockReads), () => Volatile.Read(ref tagReads)); + } + + private static S7TagDefinition Word(string name, int offset) => + new(name, $"DB1.DBW{offset}", S7DataType.UInt16, Writable: false); + + /// + /// The over-running member is refused on its own; its neighbours are served from the very first + /// pass, and from the second pass on the neighbours are grouped again WITHOUT the bad member. + /// + [Fact] + public async Task A_refused_span_falls_back_per_member_and_stops_grouping_the_bad_member() + { + var ct = TestContext.Current.CancellationToken; + var (drv, _, blockReads, tagReads) = Start(Word("A", 0), Word("B", 2), Word("C", 4)); + using var _ = drv; + await drv.InitializeAsync("{}", ct); + string[] refs = ["A", "B", "C"]; + + // Pass 1: the 6-byte span is refused, then each member is read on its own. + var r1 = await drv.ReadAsync(refs, ct); + r1[0].StatusCode.ShouldBe(0u); + r1[0].Value.ShouldBe((ushort)0x0102); + r1[1].StatusCode.ShouldBe(0u); + r1[1].Value.ShouldBe((ushort)0x0304); + r1[2].StatusCode.ShouldBe(BadDeviceFailure); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + blockReads().ShouldBe(1); + tagReads().ShouldBe(3); + + // Pass 2: A+B ride ONE block read again (the planner no longer sees C); C reads alone. + var r2 = await drv.ReadAsync(refs, ct); + r2[0].Value.ShouldBe((ushort)0x0102); + r2[1].Value.ShouldBe((ushort)0x0304); + r2[2].StatusCode.ShouldBe(BadDeviceFailure); + blockReads().ShouldBe(2, "A+B grouped again, without the refused member"); + tagReads().ShouldBe(4, "only C took the per-tag path on pass 2"); + } + + /// + /// When the block is refused but every member reads fine alone, the span itself is the problem: + /// all of its members stop being grouped (and stop re-trying the refused block every pass). + /// + [Fact] + public async Task A_refused_span_whose_members_all_read_alone_is_not_grouped_again() + { + var ct = TestContext.Current.CancellationToken; + var factory = new ScriptedS7PlcFactory(); + var blockReads = 0; + factory.ReadBytes = (_, _, _, _, _) => { Interlocked.Increment(ref blockReads); throw OutOfRange(); }; + factory.Read = (_, _) => Task.FromResult((ushort)7); + using var drv = new S7Driver(new S7DriverOptions + { + Host = "192.0.2.1", + Probe = new S7ProbeOptions { Enabled = false }, + RawTags = S7RawTags.Entries(Word("A", 0), Word("B", 2)), + }, "s7-span-fallback-all", factory); + await drv.InitializeAsync("{}", ct); + + (await drv.ReadAsync(["A", "B"], ct)).ShouldAllBe(s => s.StatusCode == 0u); + (await drv.ReadAsync(["A", "B"], ct)).ShouldAllBe(s => s.StatusCode == 0u); + Volatile.Read(ref blockReads).ShouldBe(1, "the refused block is not re-issued on later passes"); + } + + /// + /// Positive control: a span failure that is NOT a refusal (a device-layer fault) does not fall + /// back — re-reading N members one by one against a failing device would only multiply the fault. + /// + [Fact] + public async Task A_non_refusal_span_failure_does_not_fall_back() + { + var ct = TestContext.Current.CancellationToken; + var factory = new ScriptedS7PlcFactory + { + ReadBytes = (_, _, _, _, _) => throw new global::S7.Net.PlcException(global::S7.Net.ErrorCode.SendData, "device fault"), + }; + using var drv = new S7Driver(new S7DriverOptions + { + Host = "192.0.2.1", + Probe = new S7ProbeOptions { Enabled = false }, + RawTags = S7RawTags.Entries(Word("A", 0), Word("B", 2)), + }, "s7-span-nofallback", factory); + await drv.InitializeAsync("{}", ct); + + var before = factory.WireOps; + var r = await drv.ReadAsync(["A", "B"], ct); + r.ShouldAllBe(s => s.StatusCode == BadDeviceFailure); + (factory.WireOps - before).ShouldBe(1); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + } + + /// + /// Review m-5, the multi-var sibling: a leftover chunk holding one refused scalar used to fall back + /// per tag on EVERY poll (1 + N requests). The member refused on its own is now memoised, so the + /// next pass batches the rest and reads only the bad one alone. + /// + [Fact] + public async Task A_refused_multi_var_chunk_memoises_only_the_member_refused_alone() + { + var ct = TestContext.Current.CancellationToken; + var factory = new ScriptedS7PlcFactory(); + var multiCalls = 0; + factory.ReadMulti = (items, _) => + { + Interlocked.Increment(ref multiCalls); + if (items.Any(i => i.DB == 3)) + // The live snap7 shape of a multi-var request with one refused item. + throw new global::S7.Net.PlcException(global::S7.Net.ErrorCode.ReadData, + new global::S7.Net.PlcException(global::S7.Net.ErrorCode.WrongNumberReceivedBytes)); + foreach (var i in items) i.Value = (ushort)5; + return Task.FromResult>(items); + }; + factory.Read = (addr, _) => addr.StartsWith("DB3.", StringComparison.Ordinal) + ? throw new Exception("Received error from PLC: Object does not exist.") + : Task.FromResult((ushort)5); + using var drv = new S7Driver(new S7DriverOptions + { + Host = "192.0.2.1", + Probe = new S7ProbeOptions { Enabled = false }, + RawTags = S7RawTags.Entries( + new S7TagDefinition("A", "DB1.DBW0", S7DataType.UInt16, Writable: false), + new S7TagDefinition("B", "DB2.DBW0", S7DataType.UInt16, Writable: false), + new S7TagDefinition("Bad", "DB3.DBW0", S7DataType.UInt16, Writable: false)), + }, "s7-mv-memo", factory); + await drv.InitializeAsync("{}", ct); + string[] refs = ["A", "B", "Bad"]; + + var before = factory.WireOps; + var r1 = await drv.ReadAsync(refs, ct); + r1[0].StatusCode.ShouldBe(0u); + r1[1].StatusCode.ShouldBe(0u); + r1[2].StatusCode.ShouldBe(BadDeviceFailure); + (factory.WireOps - before).ShouldBe(4, "pass 1: one refused multi-var, then one per-tag read each"); + + before = factory.WireOps; + var r2 = await drv.ReadAsync(refs, ct); + r2[0].StatusCode.ShouldBe(0u); + r2[1].StatusCode.ShouldBe(0u); + r2[2].StatusCode.ShouldBe(BadDeviceFailure); + (factory.WireOps - before).ShouldBe(2, "pass 2: A+B ride one multi-var, Bad reads alone"); + Volatile.Read(ref multiCalls).ShouldBe(2); + } + + /// The memo is per generation: a re-initialize (e.g. a corrected DB) groups everything again. + [Fact] + public async Task Reinitialize_clears_the_do_not_group_memo() + { + var ct = TestContext.Current.CancellationToken; + var (drv, _, blockReads, tagReads) = Start(Word("A", 0), Word("B", 2), Word("C", 4)); + using var _ = drv; + await drv.InitializeAsync("{}", ct); + await drv.ReadAsync(["A", "B", "C"], ct); + blockReads().ShouldBe(1); + tagReads().ShouldBe(3); + + await drv.InitializeAsync("{}", ct); + await drv.ReadAsync(["A", "B", "C"], ct); + // With the memo still in force pass 2 would be one A+B block read plus ONE per-tag read (C). + // Cleared, the full A..C span is tried again, refused again, and all three re-read alone. + blockReads().ShouldBe(2); + tagReads().ShouldBe(6, "after re-init the full span is tried again (and refused again)"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TagShapeGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TagShapeGateTests.cs new file mode 100644 index 00000000..3bc47dca --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TagShapeGateTests.cs @@ -0,0 +1,66 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// R07:H3 / R07:U4 — the tag shapes the driver cannot serve are refused per tag at the mapper, +/// the deploy gate and the driver's init (see ), instead of faulting +/// the whole driver at init or being silently mis-read. +/// +[Trait("Category", "Unit")] +public sealed class S7TagShapeGateTests +{ + /// A typo'd area/size letter is refused per tag (it used to throw FormatException at init). + [Fact] + public Task Unparseable_address_is_refused_per_tag() + => S7TagSkipAssert.RefusedPerTagAsync(new S7TagDefinition("Typo", "DB1.DBZ0", S7DataType.Int16), "DB1.DBZ0", "does not parse"); + + /// + /// R07:U4 — a Bool array reads bit 0 of each byte, so a non-zero start bit was silently ignored + /// (DB1.DBX0.3 returned bit 0 of bytes 0..7). It is refused at authoring now. + /// + [Fact] + public Task Bool_array_starting_at_a_nonzero_bit_is_refused() + => S7TagSkipAssert.RefusedPerTagAsync( + new S7TagDefinition("OffsetBits", "DB1.DBX0.3", S7DataType.Bool, Writable: false, ArrayCount: 8), + "Bool array", "bit 3"); + + /// A narrow type at the wrong access width fails every read with a type mismatch; refused up front. + /// The address. + /// The declared type, which does not match the address's access width. + [Theory] + [InlineData("DB1.DBW0", S7DataType.Int32)] + [InlineData("DB1.DBD0", S7DataType.Int16)] + [InlineData("DB1.DBB0", S7DataType.Bool)] + [InlineData("M0.0", S7DataType.Byte)] + [InlineData("MW0", S7DataType.Float32)] + public Task Narrow_type_at_the_wrong_access_width_is_refused(string address, S7DataType dt) + => S7TagSkipAssert.RefusedPerTagAsync(new S7TagDefinition("Mismatch", address, dt), "access width"); + + /// Positive control: the shapes the driver serves are all still accepted by the mapper and the gate. + /// The address. + /// The declared type. + /// The array count, or 0 for a scalar. + /// Whether the tag is writable. + [Theory] + [InlineData("DB1.DBX0.0", S7DataType.Bool, 8, false)] + [InlineData("DB1.DBX50.3", S7DataType.Bool, 0, true)] + [InlineData("DB1.DBB0", S7DataType.Byte, 4, true)] + [InlineData("DB1.DBW0", S7DataType.Int16, 0, true)] + [InlineData("MW2", S7DataType.UInt16, 3, true)] + [InlineData("DB1.DBD0", S7DataType.Float32, 0, true)] + [InlineData("QD4", S7DataType.Int32, 2, true)] + [InlineData("DB1.DBB0", S7DataType.Float64, 0, true)] + [InlineData("DB1.DBB0", S7DataType.String, 0, true)] + [InlineData("T5", S7DataType.Float64, 0, false)] + [InlineData("C3", S7DataType.Int32, 0, false)] + public void Servable_shapes_are_accepted(string address, S7DataType dt, int arrayCount, bool writable) + { + var def = new S7TagDefinition("Ok", address, dt, Writable: writable, ArrayCount: arrayCount == 0 ? null : arrayCount); + var blob = S7TagDefinitionFactory.ToTagConfig(def); + S7TagDefinitionFactory.FromTagConfig(blob, "Ok", out var mapped, out var reason).ShouldBeTrue(reason); + mapped.Address.ShouldBe(address); + S7TagDefinitionFactory.Inspect(blob).ShouldBeEmpty(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TagSkipAssert.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TagSkipAssert.cs new file mode 100644 index 00000000..47f96de4 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7TagSkipAssert.cs @@ -0,0 +1,60 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// Shared assertion for an S7 tag shape the driver cannot serve. It must be refused PER TAG at +/// every gate, never fault the whole driver: the mapper (and so the AdminUI editor) refuses it with +/// the reason, the deploy gate () warns, and a driver +/// carrying it next to a good tag initializes Healthy, reports the bad one through +/// , and still serves the good one (the positive control). +/// +internal static class S7TagSkipAssert +{ + private const string GoodName = "Good"; + + /// Asserts is refused per tag at the mapper, the deploy gate and the driver. + /// The unservable tag. + /// Fragments the operator-facing rejection reason must contain. + /// A task that completes when every gate has been checked. + public static async Task RefusedPerTagAsync(S7TagDefinition bad, params string[] reasonFragments) + { + var ct = TestContext.Current.CancellationToken; + var blob = S7TagDefinitionFactory.ToTagConfig(bad); + + // Mapper (and therefore the AdminUI editor, which asks it) — refused, with the reason. + S7TagDefinitionFactory.FromTagConfig(blob, bad.Name, out _, out var reason).ShouldBeFalse(); + reason.ShouldNotBeNull(); + foreach (var f in reasonFragments) reason.ShouldContain(f, Case.Insensitive); + + // Deploy gate — warns with the same reason. + var warnings = S7TagDefinitionFactory.Inspect(blob); + warnings.ShouldHaveSingleItem().ShouldContain(reason); + + // Driver — initializes Healthy with the bad tag skipped and the good tag served. + var opts = new S7DriverOptions + { + Host = "192.0.2.1", + Timeout = TimeSpan.FromMilliseconds(250), + Probe = new S7ProbeOptions { Enabled = false }, + RawTags = S7RawTags.Entries(bad, new S7TagDefinition(GoodName, "DB1.DBW0", S7DataType.UInt16, Writable: false)), + }; + using var drv = new S7Driver(opts, "s7-skip-" + bad.Name, new ScriptedS7PlcFactory()); + await drv.InitializeAsync("{}", ct); + + drv.GetHealth().State.ShouldBe(DriverState.Healthy, "one unservable tag must not fault the driver"); + var skipped = drv.GetSkippedTags().ShouldNotBeNull(); + skipped.Sample.ShouldHaveSingleItem().RawPath.ShouldBe(bad.Name); + skipped.Sample[0].Reason.ShouldBe(reason); + + var builder = new RecordingAddressSpaceBuilder(); + await drv.DiscoverAsync(builder, ct); + builder.Variables.Select(v => v.Name).ShouldBe([GoodName]); + + var results = await drv.ReadAsync([bad.Name, GoodName], ct); + results[0].StatusCode.ShouldBe(0x80340000u, "the skipped tag reads BadNodeIdUnknown"); + results[1].StatusCode.ShouldBe(0u, "the good neighbour is still served"); + } +} 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() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ValueCodecTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ValueCodecTests.cs index 506d5d67..95bba0b3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ValueCodecTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ValueCodecTests.cs @@ -318,16 +318,9 @@ public sealed class S7ValueCodecTests [Fact] public async Task New_types_declared_as_arrays_or_off_byte_addresses_fail_init() { - S7DriverOptions With(S7TagDefinition t) => new() - { - Host = "192.0.2.1", - Probe = new S7ProbeOptions { Enabled = false }, - RawTags = S7RawTags.Entries(t), - }; - using var arr = new S7Driver(With(Tag(S7DataType.Dtl) with { ArrayCount = 2 }), "a", new ConnectingFakeS7PlcFactory()); - await Should.ThrowAsync(() => arr.InitializeAsync("{}", TestContext.Current.CancellationToken)); - using var word = new S7Driver(With(Tag(S7DataType.Tod, address: "DB1.DBW0")), "b", new ConnectingFakeS7PlcFactory()); - await Should.ThrowAsync(() => word.InitializeAsync("{}", TestContext.Current.CancellationToken)); + // Refused PER TAG at every gate (they used to throw at init and fault the whole driver). + await S7TagSkipAssert.RefusedPerTagAsync(Tag(S7DataType.Dtl) with { Name = "DtlArr", ArrayCount = 2 }, "Dtl", "array"); + await S7TagSkipAssert.RefusedPerTagAsync(Tag(S7DataType.Tod, address: "DB1.DBW0") with { Name = "TodWord" }, "Tod", "byte-addressed"); } // ---- TagConfig ---- diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/ScriptedS7Plc.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/ScriptedS7Plc.cs new file mode 100644 index 00000000..25d74961 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/ScriptedS7Plc.cs @@ -0,0 +1,142 @@ +using S7DataItem = global::S7.Net.Types.DataItem; +using S7NetDataType = global::S7.Net.DataType; +using VarType = global::S7.Net.VarType; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// A scriptable : every connection it creates delegates its wire +/// operations to the factory's handlers, so a test scripts the device once and observes every +/// connection (including reopened ones). Counts wire operations so a test can prove what did +/// NOT go to the wire without timing anything. +/// +internal sealed class ScriptedS7PlcFactory : IS7PlcFactory +{ + private int _wireOps; + + /// Every connection created, in order. + public List Created { get; } = []; + + /// Wire operations issued (reads, multi-var reads, writes; not opens or status probes). + public int WireOps => Volatile.Read(ref _wireOps); + + /// Hook run inside every ; the default opens immediately. + public Func Open { get; set; } = (_, _) => Task.CompletedTask; + + /// Per-address string read; the default returns (ushort)42. + public Func> Read { get; set; } = (_, _) => Task.FromResult((ushort)42); + + /// Block read; the default returns count zero bytes. + public Func> ReadBytes { get; set; } + = (_, _, _, count, _) => Task.FromResult(new byte[count]); + + /// Multi-var read; the default answers every item with a zero of its var type. + public Func, CancellationToken, Task>> ReadMulti { get; set; } + = (items, _) => + { + foreach (var i in items) + i.Value = i.VarType switch + { + VarType.Bit => false, + VarType.Byte => (byte)0, + VarType.Word => (ushort)0, + _ => (uint)0, + }; + return Task.FromResult>(items); + }; + + /// Per-address string write; the default succeeds. + public Func Write { get; set; } = (_, _, _) => Task.CompletedTask; + + /// Block write; the default succeeds. + public Func WriteBytes { get; set; } + = (_, _, _, _, _) => Task.CompletedTask; + + /// CPU-status probe; the default answers. + public Func ReadStatus { get; set; } = _ => Task.CompletedTask; + + /// + public IS7Plc Create(S7CpuType cpuType, string host, int port, short rack, short slot, TimeSpan timeout) + { + lock (Created) + { + var plc = new ScriptedS7Plc(this, Created.Count); + Created.Add(plc); + return plc; + } + } + + /// Counts one wire operation. + internal void CountWireOp() => Interlocked.Increment(ref _wireOps); +} + +/// One connection created by . +internal sealed class ScriptedS7Plc(ScriptedS7PlcFactory factory, int index) : IS7Plc +{ + /// Zero-based creation index. + public int Index { get; } = index; + + /// + public bool IsConnected { get; private set; } + + /// True once disposed. + public bool Disposed { get; private set; } + + /// + public int MaxPduSize => 240; + + /// + public async Task OpenAsync(CancellationToken cancellationToken) + { + await factory.Open(Index, cancellationToken).ConfigureAwait(false); + IsConnected = true; + } + + /// + public Task ReadAsync(string address, CancellationToken cancellationToken) + { + factory.CountWireOp(); + return factory.Read(address, cancellationToken); + } + + /// + public Task ReadBytesAsync(S7NetDataType area, int db, int startByteAdr, int count, CancellationToken cancellationToken) + { + factory.CountWireOp(); + return factory.ReadBytes(area, db, startByteAdr, count, cancellationToken); + } + + /// + public Task> ReadMultipleVarsAsync(List items, CancellationToken cancellationToken) + { + factory.CountWireOp(); + return factory.ReadMulti(items, cancellationToken); + } + + /// + public Task WriteAsync(string address, object value, CancellationToken cancellationToken) + { + factory.CountWireOp(); + return factory.Write(address, value, cancellationToken); + } + + /// + public Task WriteBytesAsync(S7NetDataType area, int db, int startByteAdr, byte[] value, CancellationToken cancellationToken) + { + factory.CountWireOp(); + return factory.WriteBytes(area, db, startByteAdr, value, cancellationToken); + } + + /// + public Task ReadStatusAsync(CancellationToken cancellationToken) => factory.ReadStatus(cancellationToken); + + /// + public void Close() => IsConnected = false; + + /// + public void Dispose() + { + Disposed = true; + IsConnected = false; + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CsvMergeEvidenceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CsvMergeEvidenceTests.cs index 337c6a34..112f1287 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CsvMergeEvidenceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CsvMergeEvidenceTests.cs @@ -142,7 +142,7 @@ public sealed class CsvMergeEvidenceTests var row = SingleRow( DriverTypeNames.S7, "Name,DataType,Address,S7DataType,TagConfigJson", - "t1,Int16,DB1.DBW0,Int32,\"{\"\"pollHint\"\":\"\"fast\"\"}\""); + "t1,Int16,DB1.DBD0,Int32,\"{\"\"pollHint\"\":\"\"fast\"\"}\""); var blob = Blob(row); blob["dataType"]!.GetValue().ShouldBe("Int32"); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTagConfigGateTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTagConfigGateTests.cs index 65930cf0..17703c6b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTagConfigGateTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTagConfigGateTests.cs @@ -346,7 +346,11 @@ public sealed class RawTagConfigGateTests public void A_browse_committed_config_passes_the_gate(string driverType) { var tagConfig = RawBrowseCommitMapper.BuildTagConfig( - driverType, driverType == DriverTypeNames.AbLegacy ? "N7:0" : "SomeAddress", BrowseFieldsFor(driverType)); + driverType, + // Drivers whose address grammar is validated by their reader need a real address: AbLegacy's + // file grammar, and S7's (S7TagShapeRules — an unparseable or wrong-width address is refused). + driverType == DriverTypeNames.AbLegacy ? "N7:0" : driverType == DriverTypeNames.S7 ? "DB1.DBW0" : "SomeAddress", + BrowseFieldsFor(driverType)); TagConfigValidator.Validate(driverType, tagConfig).ShouldBeNull( $"browse-commit for {driverType} must not author a config the import refuses"); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/S7TagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/S7TagConfigModelTests.cs index dbd8e61b..12cb121c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/S7TagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/S7TagConfigModelTests.cs @@ -98,13 +98,13 @@ public sealed class S7TagConfigModelTests /// driver serves. (An absent key is deliberately not asserted: S7 alone hard-requires the key, an /// asymmetry that predates this change and that its own Inspect does not report either.) [Theory] - [InlineData("""{"address":"DB1.DBW0","dataType":"float32"}""")] - [InlineData("""{"address":"DB1.DBW0","dataType":"FLOAT32"}""")] + [InlineData("""{"address":"DB1.DBD0","dataType":"float32"}""")] + [InlineData("""{"address":"DB1.DBD0","dataType":"FLOAT32"}""")] [InlineData("""{"address":"DB1.DBW0","dataType":7}""")] public void Validate_accepts_every_dataType_the_driver_accepts(string json) => S7TagConfigModel.FromJson(json).Validate().ShouldBeNull(); - // ---- S7-016: array shape vs the driver's init guards (which fail the WHOLE driver init) ---- + // ---- S7-016: array shape vs S7TagShapeRules (refused per tag at every gate) ---- [Theory] [InlineData("Int64")] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigNonEnumParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigNonEnumParityTests.cs index 019498e4..d71a00e0 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigNonEnumParityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigNonEnumParityTests.cs @@ -160,9 +160,9 @@ public sealed class TagConfigNonEnumParityTests /// S7's remaining non-enum refusal: a String tag whose stringLength falls outside /// 0–254. The reader REJECTS it (it does not clamp), so the tag would never resolve. [Theory] - [InlineData("""{"address":"DB1.DBS0","dataType":"String","stringLength":255}""")] - [InlineData("""{"address":"DB1.DBS0","dataType":"String","stringLength":300}""")] - [InlineData("""{"address":"DB1.DBS0","dataType":"String","stringLength":-1}""")] + [InlineData("""{"address":"DB1.DBB0","dataType":"String","stringLength":255}""")] + [InlineData("""{"address":"DB1.DBB0","dataType":"String","stringLength":300}""")] + [InlineData("""{"address":"DB1.DBB0","dataType":"String","stringLength":-1}""")] public void S7_refuses_a_string_length_its_reader_refuses(string blob) { RuntimeAccepts(DriverTypeNames.S7, blob).ShouldBeFalse( @@ -174,9 +174,9 @@ public sealed class TagConfigNonEnumParityTests /// stops this becoming an over-rejection — an out-of-range stringLength on a tag that is not a /// String at all, which the reader ignores entirely. [Theory] - [InlineData("""{"address":"DB1.DBS0","dataType":"String","stringLength":10}""")] - [InlineData("""{"address":"DB1.DBS0","dataType":"String","stringLength":254}""")] - [InlineData("""{"address":"DB1.DBS0","dataType":"String","stringLength":0}""")] + [InlineData("""{"address":"DB1.DBB0","dataType":"String","stringLength":10}""")] + [InlineData("""{"address":"DB1.DBB0","dataType":"String","stringLength":254}""")] + [InlineData("""{"address":"DB1.DBB0","dataType":"String","stringLength":0}""")] [InlineData("""{"address":"DB1.DBW0","dataType":"Int16","stringLength":300}""")] public void S7_accepts_a_string_length_its_reader_accepts(string blob) { @@ -360,8 +360,8 @@ public sealed class TagConfigNonEnumParityTests (DriverTypeNames.S7, """{"address":"DB1.DBW0","dataType":"Int16"}"""), (DriverTypeNames.S7, """{"address":"DB1.DBW0"}"""), (DriverTypeNames.S7, """{"address":" ","dataType":"Int16"}"""), - (DriverTypeNames.S7, """{"address":"DB1.DBS0","dataType":"String","stringLength":300}"""), - (DriverTypeNames.S7, """{"address":"DB1.DBS0","dataType":"String","stringLength":24}"""), + (DriverTypeNames.S7, """{"address":"DB1.DBB0","dataType":"String","stringLength":300}"""), + (DriverTypeNames.S7, """{"address":"DB1.DBB0","dataType":"String","stringLength":24}"""), (DriverTypeNames.AbCip, "{}"), (DriverTypeNames.AbCip, """{"tagPath":"Program:Main.Speed"}"""), @@ -576,7 +576,7 @@ public sealed class TagConfigNonEnumParityTests data.Add(DriverTypeNames.Modbus, new ModbusTagConfigModel { Address = 1 }.ToJson()); data.Add(DriverTypeNames.S7, new S7TagConfigModel { Address = "DB1.DBW0" }.ToJson()); data.Add(DriverTypeNames.S7, - new S7TagConfigModel { Address = "DB1.DBS0", DataType = S7DataType.String, StringLength = 24 }.ToJson()); + new S7TagConfigModel { Address = "DB1.DBB0", DataType = S7DataType.String, StringLength = 24 }.ToJson()); data.Add(DriverTypeNames.AbCip, new AbCipTagConfigModel { TagPath = "Program:Main.Speed" }.ToJson()); data.Add(DriverTypeNames.AbLegacy, new AbLegacyTagConfigModel { Address = "N7:0" }.ToJson()); data.Add(DriverTypeNames.TwinCAT, new TwinCATTagConfigModel { SymbolPath = "MAIN.bStart" }.ToJson()); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/S7AccessDeniedWatchdogTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/S7AccessDeniedWatchdogTests.cs new file mode 100644 index 00000000..b7ace6d4 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/S7AccessDeniedWatchdogTests.cs @@ -0,0 +1,123 @@ +using Akka.Actor; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.S7; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; +using S7DataItem = global::S7.Net.Types.DataItem; +using S7NetDataType = global::S7.Net.DataType; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// Lane S7 review I-1, end to end: the REAL under the REAL +/// comms-loss watchdog. A CPU with PUT/GET disabled denies every +/// read but is reachable and answering; the driver reports Faulted (the config alert) and must +/// still advance LastSuccessfulRead, so the watchdog's "non-Healthy AND silent for the +/// threshold" conjunction is never met and the actor does NOT reconnect + re-subscribe. The +/// positive control is the same rig with a real comms loss, which DOES trip it. +/// +public sealed class S7AccessDeniedWatchdogTests : RuntimeActorTestBase +{ + private static readonly TimeSpan FastPoll = TimeSpan.FromMilliseconds(50); + + /// The watchdog threshold — an order of magnitude above the poll so the window is a decision, not a race. + private static readonly TimeSpan Threshold = TimeSpan.FromMilliseconds(1500); + + private sealed class ScriptedPlcFactory(Func readFault) : IS7PlcFactory + { + public IS7Plc Create(S7CpuType cpuType, string host, int port, short rack, short slot, TimeSpan timeout) + => new ScriptedPlc(readFault); + } + + private sealed class ScriptedPlc(Func readFault) : IS7Plc + { + public bool IsConnected { get; private set; } + public Task OpenAsync(CancellationToken cancellationToken) { IsConnected = true; return Task.CompletedTask; } + public void Close() => IsConnected = false; + public Task ReadAsync(string address, CancellationToken cancellationToken) => Task.FromException(readFault()); + public Task ReadBytesAsync(S7NetDataType area, int db, int startByteAdr, int count, CancellationToken cancellationToken) + => Task.FromException(readFault()); + public Task> ReadMultipleVarsAsync(List items, CancellationToken cancellationToken) + => Task.FromException>(readFault()); + public Task WriteAsync(string address, object value, CancellationToken cancellationToken) => Task.CompletedTask; + public Task WriteBytesAsync(S7NetDataType area, int db, int startByteAdr, byte[] value, CancellationToken cancellationToken) => Task.CompletedTask; + public Task ReadStatusAsync(CancellationToken cancellationToken) => Task.CompletedTask; + public void Dispose() => IsConnected = false; + } + + private (S7Driver Driver, Akka.TestKit.TestProbe Parent) Connect(Func readFault) + { + var driver = new S7Driver(new S7DriverOptions + { + Host = "192.0.2.1", + Probe = new S7ProbeOptions { Enabled = false }, + SpanReadMaxGapBytes = -1, + RawTags = + [ + new RawTagEntry("W0", S7TagDefinitionFactory.ToTagConfig(new S7TagDefinition("W0", "DB1.DBW0", S7DataType.UInt16, Writable: false)), false), + new RawTagEntry("W2", S7TagDefinitionFactory.ToTagConfig(new S7TagDefinition("W2", "DB1.DBW2", S7DataType.UInt16, Writable: false)), false), + ], + }, "s7-watchdog", new ScriptedPlcFactory(readFault)); + + var parent = CreateTestProbe(); + var actor = parent.ChildActorOf(DriverInstanceActor.Props( + driver, + reconnectInterval: TimeSpan.FromSeconds(30), + healthPollInterval: FastPoll, + commsLossTimeout: Threshold)); + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + parent.ExpectMsg(TimeSpan.FromSeconds(5)).Connected.ShouldBeTrue(); + return (driver, parent); + } + + /// Drives read passes (the poll the actor would schedule) until . + private static async Task<(int Passes, int FaultedPasses)> PollUntilAsync(S7Driver driver, DateTime until) + { + var passes = 0; + var faulted = 0; + while (DateTime.UtcNow < until) + { + try + { +#pragma warning disable OTOPCUA0001 // the watchdog's reading of driver health is under test, not resilience wrapping + await driver.ReadAsync(["W0", "W2"], CancellationToken.None); +#pragma warning restore OTOPCUA0001 + passes++; + if (driver.GetHealth().State == DriverState.Faulted) faulted++; + } + catch (InvalidOperationException) { /* the actor tore the driver down (control case) */ } + await Task.Delay(100); // pacing the simulated poll, not an assertion + } + return (passes, faulted); + } + + /// An all-access-denied CPU is left connected across more than twice the watchdog threshold. + [Fact] + public async Task An_access_denied_cpu_is_not_reconnected_by_the_watchdog() + { + var (driver, parent) = Connect(() => new Exception("Received error from PLC: Accessing object not allowed.")); + + var (passes, faulted) = await PollUntilAsync(driver, DateTime.UtcNow + Threshold + Threshold + TimeSpan.FromMilliseconds(500)); + + // Positive controls: the window really ran past the threshold with the driver Faulted throughout, + // so "no reconnect" below cannot pass merely because nothing happened. + passes.ShouldBeGreaterThanOrEqualTo(10); + faulted.ShouldBe(passes, "every pass reports the PUT/GET config alert"); + + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + driver.GetHealth().State.ShouldBe(DriverState.Faulted); + } + + /// The control: a real comms loss (every read times out) on the same rig IS reconnected. + [Fact] + public async Task A_real_comms_loss_is_reconnected_by_the_watchdog() + { + var (driver, parent) = Connect(() => new TimeoutException("frozen peer")); + + var poll = PollUntilAsync(driver, DateTime.UtcNow + Threshold + Threshold + TimeSpan.FromMilliseconds(500)); + parent.ExpectMsg(PresenceBudget).Connected.ShouldBeFalse(); + await poll; + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ZB.MOM.WW.OtOpcUa.Runtime.Tests.csproj b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ZB.MOM.WW.OtOpcUa.Runtime.Tests.csproj index 554d3e15..bc5bdb83 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ZB.MOM.WW.OtOpcUa.Runtime.Tests.csproj +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ZB.MOM.WW.OtOpcUa.Runtime.Tests.csproj @@ -39,6 +39,9 @@ re-statement end-to-end tests prove what reaches the host's /alerts rows and Part 9 conditions. --> + +