diff --git a/docs/DriverLifecycle.md b/docs/DriverLifecycle.md index ef029efb..fdfc8cf6 100644 --- a/docs/DriverLifecycle.md +++ b/docs/DriverLifecycle.md @@ -512,16 +512,16 @@ same pass (a UNS reference variable, say) are visible through the index with no ⚠️ **The value fan is now a single message.** It sends one `DriverInstanceActor.AttributeValueBatchPublished` carrying one item per RawPath, where it previously sent -one `AttributeValuePublished` per ref — 5 000 messages onto the mux that also carries the node's steady +one per-value message per ref — 5 000 messages onto the mux that also carries the node's steady value ingress, every time one driver flapped. Delivery is unchanged: the mux filters the batch per item against the same registered-interest map the per-value receive consulted, so only the framing moved. The **alarm** fan is indexed but deliberately not batched — one driver ref can back several condition NodeIds, so it still emits one `AlarmQualityUpdate` per condition. -**Consequence worth knowing: `AttributeValuePublished` now has no production sender at all.** This fan was -its last one — everything on the subscription path was already wrapped in `AttributeValueBatchPublished`. -`DependencyMuxActor` still registers the per-value receive, but in production it is dead code; it stays -for tests and direct senders. +**Consequence: the per-value `AttributeValuePublished` record is deleted.** This fan was its last +production sender (everything on the subscription path was already wrapped in +`AttributeValueBatchPublished`), so the record and `DependencyMuxActor`'s per-value receive were removed +(2026-10-22). A single value is a one-item batch; tests build one with `ValuePublishes.Single`. #### The third fan: the value nodes (archreview 03/S15) diff --git a/docs/ReadWriteOperations.md b/docs/ReadWriteOperations.md index c1f8b03a..e519f5e4 100644 --- a/docs/ReadWriteOperations.md +++ b/docs/ReadWriteOperations.md @@ -39,8 +39,8 @@ This was not always true. Until #519 the publish path projected the driver's `ui `Good`/`Uncertain`/`Bad` enum in `DriverInstanceActor`, and `OtOpcUaNodeManager` re-expanded `Bad` to a bare `StatusCodes.Bad` (`0x80000000`). Every driver's diagnostic intent survived in logs only — including the 16 status constants #497 had just corrected across six drivers. The value path now -carries the raw `uint` end to end (`AttributeValuePublished` → `AttributeValueUpdate` → -`IOpcUaAddressSpaceSink.WriteValue`), and severity classification is a helper +carries the raw `uint` end to end (`AttributeValueBatchPublished` → `AttributeValueBatchUpdate` → +`IOpcUaAddressSpaceSink.WriteValues`), and severity classification is a helper (`OpcUaStatus.IsGood`/`IsUncertain`/`IsBad`) rather than a lossy projection. **Writing a status code in engine code:** use a named constant on diff --git a/docs/ScriptEditor.md b/docs/ScriptEditor.md index eb6985e2..e039653c 100644 --- a/docs/ScriptEditor.md +++ b/docs/ScriptEditor.md @@ -198,10 +198,32 @@ scripted-alarm predicates) runs scripts through the shared `ScriptRunGuard`: 2 s default) quarantines immediately, through the same path (Error log, metric `reason=stuck`, Bad). A short overrun (a young orphan) stays a plain refusal and the script resumes once the orphan completes. +- **Cooperative cancellation (`ctx.CancellationToken`).** The guard cancels the run + token when a run times out, and scripts see it as `ctx.CancellationToken`. A + long-running loop that polls `ctx.CancellationToken.IsCancellationRequested` (or calls + `ctx.CancellationToken.ThrowIfCancellationRequested()`) ends its own abandoned run + and frees the slot, instead of holding a pool thread until it returns. A script that + ignores the token is still an orphan. The editor completes the member, and + `CancellationToken` is on the sandbox allow-list. - **Allocation budget.** A single run allocating more than `Scripting:MaxAllocBytesPerRun` (64 MB) is quarantined. This is sampled after the run returns, so a run that OOMs the process first is not stopped — that needs an out-of-process runner. +- **Configuration keys and their ranges.** All three knobs are node-wide (the + `Scripting` appsettings section, shared by virtual tags, Calculation tags and + scripted alarms) and are clamped when bound, with one startup Warning per + clamped value: + + | Key | Default | Range after the clamp | + |---|---|---| + | `Scripting:MaxAllocBytesPerRun` | `67108864` (64 MB) | `0` (or negative) disables the budget; a positive value below `1048576` (1 MB) becomes 1 MB | + | `Scripting:QuarantineAfterConsecutiveTimeouts` | `3` | `1`–`1000` | + | `Scripting:StuckOrphanQuarantineFactor` | `10` | `0` (or negative) disables the rule; above `1000` becomes `1000`; `NaN` becomes the default | + + A Calculation driver may override `maxAllocBytesPerRun` and + `quarantineAfterConsecutiveTimeouts` in its own driver config. Those keys are + layered on the node values and clamped to the same ranges, and an out-of-range + value logs a Warning naming the driver. The stuck factor has no per-driver key. - **Lifting.** A quarantine lifts when the script source changes or on redeploy, never by itself. - **Script-log rate limit.** The cluster-wide script-log fan-out admits 20 entries @@ -243,7 +265,7 @@ prefix. The concrete implementation `ScriptTagCatalog` reads `Tag` + | Virtual tag | Leaf `Name` (best-effort) | **Why only resolvable keys?** The live runtime resolves a `ctx.GetTag("X")` -literal against `DriverInstanceActor.AttributeValuePublished.FullReference`, +literal against `DriverInstanceActor.AttributeValueItem.FullReference`, which is the `FullName` field from `Tag.TagConfig` (see `ScriptTagCatalog.ExtractFullNameFromTagConfig` + `EquipmentNodeWalker.ExtractFullName`). The former UNS-path cascade engine (`Core.VirtualTags.VirtualTagEngine`, keyed by the diff --git a/docs/VirtualTags.md b/docs/VirtualTags.md index 183fcef0..2b7dc28a 100644 --- a/docs/VirtualTags.md +++ b/docs/VirtualTags.md @@ -93,7 +93,7 @@ Virtual-tag nodes are materialised by `AddressSpaceApplier` and fed by `VirtualT `ITagUpstreamSource` and `IHistoryWriter` are the two ports value consumers require from their host. Both live in `Core.VirtualTags`. In the v2 actor system: -- **Upstream-tag feed.** `DependencyMuxActor` (`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs`) routes `DriverInstanceActor.AttributeValuePublished` events to the `VirtualTagActor` instances that declared interest in those tag refs. Each `VirtualTagActor` holds the in-memory per-tag dependency map; the `IVirtualTagEvaluator` (`RoslynVirtualTagEvaluator`) receives the dependency snapshot synchronously on the actor message thread. Reads of never-pushed dependency refs return `null` values in the dependency snapshot. +- **Upstream-tag feed.** `DependencyMuxActor` (`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs`) routes the items of each `DriverInstanceActor.AttributeValueBatchPublished` to the `VirtualTagActor` instances that declared interest in those tag refs. Each `VirtualTagActor` holds the in-memory per-tag dependency map; the `IVirtualTagEvaluator` (`RoslynVirtualTagEvaluator`) receives the dependency snapshot synchronously on the actor message thread. Reads of never-pushed dependency refs return `null` values in the dependency snapshot. - **`IHistoryWriter`** — the equipment-namespace path threads `Historize` end-to-end and `VirtualTagHostActor` invokes the injected writer on historized results (H5); the writer is resolved through `DriverHostActor` DI with a `NullHistoryWriter` default. No *durable* writer ships on this seam — durable continuous historization now flows through the separate `ContinuousHistorizationRecorder` → HistorianGateway `WriteLiveValues` path (see the `IHistoryWriter` section above and [Historian.md](Historian.md)). ## Composition @@ -104,7 +104,7 @@ In the v2 actor system, virtual-tag engine composition is owned by the driver-ro - `AddressSpaceComposer.Compose` emits `DriverInstancePlan` / `ScriptedAlarmPlan` records; the driver-role `DriverHostActor` spawns one `VirtualTagActor` per virtual-tag expression and one `ScriptedAlarmActor` per scripted alarm. - `RoslynVirtualTagEvaluator` (`src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs`) is injected into each `VirtualTagActor` as its `IVirtualTagEvaluator`. It holds a per-source `CompiledScriptCache` keyed by script source and compiles on first use. -- `DependencyMuxActor` (`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs`) receives every `DriverInstanceActor.AttributeValuePublished` event and routes it to the `VirtualTagActor` instances that registered interest in that tag ref. +- `DependencyMuxActor` (`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs`) receives every `DriverInstanceActor.AttributeValueBatchPublished` and routes each item to the `VirtualTagActor` instances that registered interest in that tag ref. `ITagUpstreamSource` and `IHistoryWriter` remain available as `Core.VirtualTags` ports. @@ -125,7 +125,7 @@ In the v2 actor system, virtual-tag engine composition is owned by the driver-ro - `src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/ITagUpstreamSource.cs` — driver-tag read + subscribe port - `src/Core/ZB.MOM.WW.OtOpcUa.Core.VirtualTags/IHistoryWriter.cs` — historize sink port + `NullHistoryWriter` - `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs` — actor that receives `DependencyValueChanged` from the mux and invokes `IVirtualTagEvaluator` per expression -- `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs` — routes `DriverInstanceActor.AttributeValuePublished` to interested `VirtualTagActor` subscribers +- `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs` — routes `DriverInstanceActor.AttributeValueBatchPublished` items to interested `VirtualTagActor` subscribers - `src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs` — production `IVirtualTagEvaluator` binding; holds a per-source `CompiledScriptCache` - `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs` — pure data composer: config-DB entities → `AddressSpaceComposition` (UNS topology + raw subtree + driver/alarm plans) - `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs` — applies the composed plan into the SDK node manager diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDependencyConsumer.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDependencyConsumer.cs index e3261ceb..bbe5adba 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDependencyConsumer.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDependencyConsumer.cs @@ -11,7 +11,7 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// /// The mechanism mirrors how a VirtualTagActor registers interest on the /// DependencyMuxActor: the mux already receives every driver's -/// AttributeValuePublished keyed by wire-ref (under v3, RawPath), so a driver that +/// published values (batched, one item per value) keyed by wire-ref (under v3, RawPath), so a driver that /// implements this interface simply piggy-backs on that fan-out. A calc tag's computed output /// re-enters the mux via the ordinary driver publish path, so calc-of-calc chains work with no /// extra machinery — which is exactly why the deploy-time cycle gate is mandatory. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ScriptContext.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ScriptContext.cs index 6a73c622..b4fdf3cd 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ScriptContext.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ScriptContext.cs @@ -80,6 +80,50 @@ public abstract class ScriptContext /// public abstract ILogger Logger { get; } + /// + /// The current run's cancellation token. The run guard cancels it when the run exceeds its wall-clock + /// budget, and it is linked to the caller's token, so it is also cancelled when the caller cancels (host + /// shutdown, an actor stopping mid-run). A long-running script can poll ctx.CancellationToken.IsCancellationRequested (or call + /// ThrowIfCancellationRequested()) and end its own abandoned run instead of holding a pool thread + /// until it returns. Cooperative only: a script that never looks at it still runs to completion as an + /// orphan. outside a run. + /// + /// + /// Scoped per run through an , not stored on the context, because a context + /// object can be reused across runs (the scripted-alarm engine keeps one per alarm). An abandoned orphan + /// therefore keeps seeing its own cancelled token even after a newer run starts on the same context. + /// + public CancellationToken CancellationToken => s_runToken.Value; + + private static readonly AsyncLocal s_runToken = new(); + + /// + /// Makes the script-visible on the calling + /// execution context until the returned scope is disposed. Called by ScriptEvaluator.RunAsync + /// around the compiled delegate. + /// + /// The run token. + /// A scope that restores the previous token. + internal static RunTokenScope EnterRun(CancellationToken token) + { + var previous = s_runToken.Value; + s_runToken.Value = token; + return new RunTokenScope(previous); + } + + /// Restores the token that was current before . + internal readonly struct RunTokenScope : IDisposable + { + private readonly CancellationToken _previous; + + /// Captures the token to restore. + /// The token current before the run. + public RunTokenScope(CancellationToken previous) => _previous = previous; + + /// + public void Dispose() => s_runToken.Value = _previous; + } + /// /// Deadband helper — returns true when differs /// from by more than . diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions.csproj b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions.csproj index f9305356..0f07e95c 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions.csproj +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions/ZB.MOM.WW.OtOpcUa.Core.Scripting.Abstractions.csproj @@ -16,4 +16,10 @@ + + + + + diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptEvaluator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptEvaluator.cs index c7ac8280..a33fba6f 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptEvaluator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptEvaluator.cs @@ -185,7 +185,13 @@ public sealed class ScriptEvaluator : IDisposable // TResult directly). We surface a Task only to keep the existing // RunAsync contract consumers depend on. TimedScriptEvaluator wraps this in // Task.Run so a long-running script still honours its wall-clock budget. - var result = _func(globals); + // The run token is the script-visible ctx.CancellationToken for exactly this call. ScriptRunGuard passes + // the token it cancels on timeout, so a polling script can end its own abandoned run. + TResult result; + using (ScriptContext.EnterRun(ct)) + { + result = _func(globals); + } return Task.FromResult(result); } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptRunGuard.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptRunGuard.cs index de1afee2..7efa4026 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptRunGuard.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptRunGuard.cs @@ -96,9 +96,11 @@ public sealed class ScriptRunGuard /// Where the quarantine Error line goes; null ⇒ Serilog's global logger. public ScriptRunGuard(ScriptingOptions? options = null, ILogger? logger = null) { - options ??= new ScriptingOptions(); + // Clamped here as well as at the bind site, because options also arrive from a Calculation driver's own + // config and from tests. An unclamped factor made the refusal path's timeout * factor overflow. + options = (options ?? new ScriptingOptions()).Sanitized(out _); _maxAllocBytesPerRun = options.MaxAllocBytesPerRun; - _quarantineThreshold = Math.Max(1, options.QuarantineAfterConsecutiveTimeouts); + _quarantineThreshold = options.QuarantineAfterConsecutiveTimeouts; _stuckFactor = options.StuckOrphanQuarantineFactor; _logger = logger ?? Log.Logger; } @@ -197,7 +199,11 @@ public sealed class ScriptRunGuard if (_stuckFactor > 0 && Volatile.Read(ref state.RunStartedTimestamp) is var startedAt and not 0) { var running = Stopwatch.GetElapsedTime(startedAt); - var threshold = timeout * _stuckFactor; + // Saturating: even the clamped maximum factor times a huge timeout must not overflow TimeSpan. + var thresholdTicks = timeout.Ticks * _stuckFactor; + var threshold = thresholdTicks >= TimeSpan.MaxValue.Ticks + ? TimeSpan.MaxValue + : TimeSpan.FromTicks((long)thresholdTicks); if (running > threshold) { Quarantine(key, state, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptingOptions.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptingOptions.cs index 296d862c..167e124f 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptingOptions.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptingOptions.cs @@ -53,4 +53,85 @@ public sealed class ScriptingOptions /// refusal. Zero or negative disables the rule. Default 10. /// public double StuckOrphanQuarantineFactor { get; set; } = DefaultStuckOrphanQuarantineFactor; + + /// Upper bound applies to . + public const double MaxStuckOrphanQuarantineFactor = 1000; + + /// Upper bound applies to . + public const int MaxQuarantineAfterConsecutiveTimeouts = 1000; + + /// + /// Lower bound applies to a positive : 1 MB. A + /// smaller budget would quarantine practically every script on its first run. + /// + public const long MinMaxAllocBytesPerRun = 1024L * 1024; + + /// + /// Returns a copy with every numeric knob pulled into a sane range. The bind site calls it and logs + /// . calls it again, so options that reach the + /// guard by another route (a Calculation driver's own config, a test) are clamped too. + /// + /// : NaN becomes the default, a negative value becomes 0 + /// (disabled), and a value above becomes that maximum. + /// Unclamped, an absurd factor made timeout * factor throw on + /// the first refused run. + /// : into [1, ]. + /// : a negative value becomes 0 (disabled), and a positive value + /// below becomes that minimum. + /// + /// + /// One line per value that was out of range, naming its key. A negative value that + /// already meant "disabled" is normalised without a line. + /// A new, clamped instance. This instance is not modified. + public ScriptingOptions Sanitized(out IReadOnlyList adjustments) + { + var notes = new List(); + + var factor = StuckOrphanQuarantineFactor; + if (double.IsNaN(factor)) + { + notes.Add($"{SectionName}:{nameof(StuckOrphanQuarantineFactor)} is NaN; using the default {DefaultStuckOrphanQuarantineFactor}."); + factor = DefaultStuckOrphanQuarantineFactor; + } + else if (factor < 0) + { + factor = 0; + } + else if (factor > MaxStuckOrphanQuarantineFactor) + { + notes.Add($"{SectionName}:{nameof(StuckOrphanQuarantineFactor)} {factor} is above the maximum; clamped to {MaxStuckOrphanQuarantineFactor}."); + factor = MaxStuckOrphanQuarantineFactor; + } + + var threshold = QuarantineAfterConsecutiveTimeouts; + if (threshold < 1) + { + notes.Add($"{SectionName}:{nameof(QuarantineAfterConsecutiveTimeouts)} {threshold} is below 1; clamped to 1."); + threshold = 1; + } + else if (threshold > MaxQuarantineAfterConsecutiveTimeouts) + { + notes.Add($"{SectionName}:{nameof(QuarantineAfterConsecutiveTimeouts)} {threshold} is above the maximum; clamped to {MaxQuarantineAfterConsecutiveTimeouts}."); + threshold = MaxQuarantineAfterConsecutiveTimeouts; + } + + var alloc = MaxAllocBytesPerRun; + if (alloc < 0) + { + alloc = 0; + } + else if (alloc > 0 && alloc < MinMaxAllocBytesPerRun) + { + notes.Add($"{SectionName}:{nameof(MaxAllocBytesPerRun)} {alloc} is below the minimum; clamped to {MinMaxAllocBytesPerRun}."); + alloc = MinMaxAllocBytesPerRun; + } + + adjustments = notes; + return new ScriptingOptions + { + MaxAllocBytesPerRun = alloc, + QuarantineAfterConsecutiveTimeouts = threshold, + StuckOrphanQuarantineFactor = factor, + }; + } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs index 9fc101c3..9656a70e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs @@ -117,6 +117,12 @@ public sealed class AbCipDriverOptions /// device round trips (the whole point: a strictly sequential batch pays one full round /// trip per tag); lowering it to 1 restores strictly sequential reads. Values below /// 1 are clamped to 1 at the use site. + /// + /// On the transport the effective window is at least 64: there the + /// window caps how many services one Multiple Service Packet coalesces (the device's shared scheduler packs + /// what is queued, bounded by the connection's request/reply byte budget), so a small value only multiplies + /// round trips. A value above 64 is honoured; a value below it is raised. + /// /// public int MaxConcurrentReads { get; init; } = 8; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipArrayCodec.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipArrayCodec.cs new file mode 100644 index 00000000..bb6455ab --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipArrayCodec.cs @@ -0,0 +1,80 @@ +using System.Buffers.Binary; +using System.Collections; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +/// +/// Encodes a whole-array write (review 07 U10): count elements at the element type's wire width, +/// little-endian, exactly the layout a Logix atomic array holds and the read path decodes. Shared by both +/// runtimes so the bytes on the wire are transport-independent. +/// +/// +/// Element values CONVERT, they are not exact-matched. ScadaBridge (and any OPC UA client that +/// round-trips what it read) writes CLR lists such as int[] to an INT array — the read path itself +/// publishes INT/SINT elements as int — so each element goes through the same the +/// scalar path uses. A value that does not fit is an (BadOutOfRange) and one that +/// cannot convert a / (BadTypeMismatch), all +/// thrown before any byte is sent. +/// BOOL arrays (bit-packed DWORDs — a byte-per-element encode would clobber neighbour bits), STRING arrays +/// (structures) and structure arrays are not encodable here; the driver refuses them BadNotSupported +/// before a runtime exists. +/// +internal static class AbCipArrayCodec +{ + /// Wire width of one element of an encodable array type, or null when the type is not encodable. + /// The element type. + /// The element width in bytes, or null. + public static int? ElementWidth(AbCipDataType type) => type switch + { + AbCipDataType.SInt or AbCipDataType.USInt => 1, + AbCipDataType.Int or AbCipDataType.UInt => 2, + AbCipDataType.DInt or AbCipDataType.UDInt or AbCipDataType.Real or AbCipDataType.Dt => 4, + AbCipDataType.LInt or AbCipDataType.ULInt or AbCipDataType.LReal => 8, + _ => null, + }; + + /// + /// The elements of an array write value, or null when the value is not a list (a scalar, a string, null). + /// Any non-string is accepted — int[], short[], List<object> … + /// + /// The client's write value. + /// The elements, or null. + public static object?[]? Elements(object? value) => value switch + { + null or string => null, + IEnumerable list => [.. list.Cast()], + _ => null, + }; + + /// Encodes as elements. + /// The element type (must have an ). + /// The element values; their count is the element count. + /// The little-endian wire bytes, values.Count × ElementWidth(type) long. + /// The type is not encodable. + public static byte[] Encode(AbCipDataType type, IReadOnlyList values) + { + var width = ElementWidth(type) + ?? throw new NotSupportedException($"Array writes of {type} elements are not supported."); + var buffer = new byte[values.Count * width]; + for (var i = 0; i < values.Count; i++) + { + var v = values[i]; + var span = buffer.AsSpan(i * width, width); + switch (type) + { + case AbCipDataType.SInt: span[0] = unchecked((byte)Convert.ToSByte(v)); break; + case AbCipDataType.USInt: span[0] = Convert.ToByte(v); break; + case AbCipDataType.Int: BinaryPrimitives.WriteInt16LittleEndian(span, Convert.ToInt16(v)); break; + case AbCipDataType.UInt: BinaryPrimitives.WriteUInt16LittleEndian(span, Convert.ToUInt16(v)); break; + case AbCipDataType.DInt: + case AbCipDataType.Dt: BinaryPrimitives.WriteInt32LittleEndian(span, Convert.ToInt32(v)); break; + case AbCipDataType.UDInt: BinaryPrimitives.WriteUInt32LittleEndian(span, Convert.ToUInt32(v)); break; + case AbCipDataType.LInt: BinaryPrimitives.WriteInt64LittleEndian(span, Convert.ToInt64(v)); break; + case AbCipDataType.ULInt: BinaryPrimitives.WriteUInt64LittleEndian(span, Convert.ToUInt64(v)); break; + case AbCipDataType.Real: BinaryPrimitives.WriteSingleLittleEndian(span, Convert.ToSingle(v)); break; + case AbCipDataType.LReal: BinaryPrimitives.WriteDoubleLittleEndian(span, Convert.ToDouble(v)); break; + } + } + return buffer; + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 9f1d4fec..6be5b374 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -27,7 +27,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// handles GC-collected tags. /// public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, IBatchSubscribable, - IHostConnectivityProbe, IPerCallHostResolver, IAlarmSource, ISkippedTagReporter, IDisposable, IAsyncDisposable + IHostConnectivityProbe, IPerCallHostResolver, IAlarmSource, ISkippedTagReporter, IAsyncDisposable, IDisposable { /// Authored tags this driver could not build into its poll table. private readonly SkippedTagLedger _skippedTags = new(); @@ -98,12 +98,21 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, private readonly PollGroupEngine _poll; // Devices keyed by host address — the primary registry (GetDeviceState, discovery, probe, diagnostics). - private readonly Dictionary _devices = new(StringComparer.OrdinalIgnoreCase); + // BUILD-AND-SWAP, never mutated after publish: the retained poll engine, the alarm projection, discovery and + // the actor's GetHostStatuses pull all read it while a re-init / shutdown runs. Each teardown takes the table + // it replaces, so concurrent teardowns (ShutdownAsync racing DisposeAsync) own disjoint device states. + private volatile Dictionary _devices = new(StringComparer.OrdinalIgnoreCase); // Tag-routing index: a tag's device routing key (DeviceName ?? HostAddress, plus the HostAddress alias) // → its DeviceState. A tag resolves to its device by DeviceName-or-host so multi-device routing keys on // the RawPath's device segment. The HostAddress alias is deliberate (Client CLI + tests route by host); - // _options.Devices is already the merged per-device DeviceConfig. - private readonly Dictionary _devicesByRoutingKey = new(StringComparer.OrdinalIgnoreCase); + // _options.Devices is already the merged per-device DeviceConfig. Swapped with _devices, same rule. + private volatile Dictionary _devicesByRoutingKey = new(StringComparer.OrdinalIgnoreCase); + + // Serialises table publish/retire. _lifecycleGeneration is bumped by every retire (an init's own release, a + // shutdown, a dispose); an init publishes only if no retire happened after its own, so an abandoned init that a + // shutdown overtook can never resurrect devices or probe loops after that shutdown returned. + private readonly object _lifecycleLock = new(); + private int _lifecycleGeneration; // v3 authored RawPath → definition table (parents + fanned-out UDT members), keyed case-sensitive // ordinal. Built in InitializeAsync from _options.RawTags via the pure mapper. private volatile Dictionary _tagsByRawPath = new(StringComparer.Ordinal); @@ -133,7 +142,25 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// when a definition exists for . internal bool TryGetTagDefinition(string rawPath, out AbCipTagDefinition definition) => _tagsByRawPath.TryGetValue(rawPath, out definition!); - private DriverHealth _health = new(DriverState.Unknown, null, null); + // volatile: written from the read batch, the write path, the poll engine's error hook and the lifecycle, and + // read from the actor thread (GetHealth). Read-modify-write updates go through UpdateHealth (CAS) so a + // concurrent update is never lost. + private volatile DriverHealth _health = new(DriverState.Unknown, null, null); + + /// Atomically replaces with applied to its current value. + /// Computes the new health from the current one. + /// The health instance this call installed. + private DriverHealth UpdateHealth(Func update) + { + while (true) + { + var current = _health; + var next = update(current); +#pragma warning disable CS0420 // a volatile field passed by ref to Interlocked keeps its full-fence semantics + if (ReferenceEquals(Interlocked.CompareExchange(ref _health, next, current), current)) return next; +#pragma warning restore CS0420 + } + } /// Occurs when a subscribed tag's value changes. public event EventHandler? OnDataChange; @@ -240,6 +267,21 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, _logger.LogWarning(message, args); } + // Tags whose device-refusal Warning has already been logged, per operation ("read"/"write"/…): a refused tag is + // polled (or retried) forever, so the Warning is once per tag per operation per driver instance. + private readonly System.Collections.Concurrent.ConcurrentDictionary _refusalWarned = + new(StringComparer.OrdinalIgnoreCase); + + /// Logs a device-refusal Warning the first time is refused for . + private void WarnRefusalOnce(string tagName, string operation, string reference, string device, string reason) + { + if (_refusalWarned.TryAdd($"{operation}\0{tagName}", 0)) + _logger.LogWarning( + "AbCip {Operation} of {Tag} on device {Device} refused by the controller: {Reason}; reporting it Bad " + + "for this tag only — the device and its other tags are unaffected (logged once per tag)", + operation, reference, device, reason); + } + // Tags whose malformed-name Warning has already been logged — polled forever, warned once. private readonly System.Collections.Concurrent.ConcurrentDictionary _configErrorWarned = new(StringComparer.OrdinalIgnoreCase); @@ -270,9 +312,9 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, internal void HandlePollError(Exception ex) { _logger.LogWarning(ex, "AbCip poll reader failed. Driver={DriverInstanceId}", _driverInstanceId); - var current = _health; - if (current.State != DriverState.Faulted) - _health = new DriverHealth(DriverState.Degraded, current.LastSuccessfulRead, ex.Message); + UpdateHealth(current => current.State == DriverState.Faulted + ? current + : new DriverHealth(DriverState.Degraded, current.LastSuccessfulRead, ex.Message)); } /// @@ -364,48 +406,58 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, 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). Release the previous device states' probe loops + handles before - // the overwrite below. Deliberately NOT ShutdownAsync: that also drops _poll and the subscriptions. - await ReleaseDeviceStatesAsync(cancellationToken).ConfigureAwait(false); - _health = new DriverHealth(DriverState.Initializing, null, null); + // allows it, and a reconnect retry can overlap a running attempt). Retire the previous device states' probe + // loops + handles before building new ones. Deliberately NOT ShutdownAsync: that also drops _poll and the + // subscriptions. The generation this release produced is what the publish below is checked against. + var generation = await ReleaseDeviceStatesAsync().ConfigureAwait(false); + lock (_lifecycleLock) + { + if (generation == _lifecycleGeneration) + _health = new DriverHealth(DriverState.Initializing, null, null); + } _logger.LogInformation( "AbCip driver {DriverInstanceId} initializing: transport={Transport} tagFactory={TagFactory}", _driverInstanceId, _transportDescription, _tagFactory.GetType().Name); + Dictionary? builtDevices = null; try { + var options = _options; + var adoptedConfig = false; if (!string.IsNullOrWhiteSpace(driverConfigJson)) { var parsed = AbCipDriverFactoryExtensions.ParseOptions(_driverInstanceId, driverConfigJson); if (parsed.Devices.Count > 0 || parsed.RawTags.Count > 0) { - _options = parsed; - _alarmProjection = new AbCipAlarmProjection(this, _options.AlarmPollInterval, _logger); + options = parsed; + adoptedConfig = true; } } - foreach (var device in _options.Devices) + builtDevices = new Dictionary(StringComparer.OrdinalIgnoreCase); + var builtRoutes = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var device in options.Devices) { var addr = AbCipHostAddress.TryParse(device.HostAddress) ?? throw new InvalidOperationException( $"AbCip device has invalid HostAddress '{device.HostAddress}' — expected 'ab://gateway[:port]/cip-path'."); var profile = AbCipPlcFamilyProfile.ForFamily(device.PlcFamily); var state = new DeviceState(addr, device, profile); - _devices[device.HostAddress] = state; + builtDevices[device.HostAddress] = state; // Route index: a tag reaches this device by matching its DeviceName routing key OR its // host address. Devices with no explicit DeviceName are identified purely by host today. - _devicesByRoutingKey[device.HostAddress] = state; + builtRoutes[device.HostAddress] = state; if (!string.IsNullOrEmpty(device.DeviceName)) - _devicesByRoutingKey[device.DeviceName] = state; + builtRoutes[device.DeviceName] = state; } // Build the RawPath → definition table from the authored raw tags. Each entry's TagConfig blob // is mapped by the pure factory; the entry's DeviceName (routing key) + WriteIdempotent flag are // threaded onto the def (they live on the RawTagEntry, not inside the blob). A mapper miss is // skipped (logged), never thrown — a bad tag must not fail the whole driver init. - _skippedTags.BeginRebuild(_options.RawTags.Count); + _skippedTags.BeginRebuild(options.RawTags.Count); var builtTagsByRawPath = new Dictionary(StringComparer.Ordinal); var builtDeclaredTags = new List(); - foreach (var entry in _options.RawTags) + foreach (var entry in options.RawTags) { if (!AbCipTagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var mapped)) { @@ -457,24 +509,43 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } } } - // Publish the finished tables by reference swap — readers see the old or the new, never a half-built one. - _tagsByRawPath = builtTagsByRawPath; - _declaredTags = builtDeclaredTags; - _skippedTags.Commit(); - // Probe loops — one per device when enabled + a ProbeTagPath is configured. - if (_options.Probe.Enabled && !string.IsNullOrWhiteSpace(_options.Probe.ProbeTagPath)) + // Publish everything under the lifecycle lock, and only if no shutdown / other init retired the tables + // after this init's own release — readers see the old tables or the new ones, never a half-built one, + // and an overtaken init leaves nothing running behind the shutdown that overtook it. + lock (_lifecycleLock) { - foreach (var state in _devices.Values) + if (generation != _lifecycleGeneration) + throw new AbCipInitializationSupersededException( + $"AbCip driver {_driverInstanceId} initialization was superseded by a shutdown or a newer initialization."); + + _options = options; + if (adoptedConfig) + _alarmProjection = new AbCipAlarmProjection(this, options.AlarmPollInterval, _logger); + _devices = builtDevices; + _devicesByRoutingKey = builtRoutes; + _tagsByRawPath = builtTagsByRawPath; + _declaredTags = builtDeclaredTags; + _skippedTags.Commit(); + + // Probe loops — one per device when enabled + a ProbeTagPath is configured. Started inside the lock so + // a retire that follows this publish always sees (and joins) them. + if (options.Probe.Enabled && !string.IsNullOrWhiteSpace(options.Probe.ProbeTagPath)) { - state.ProbeCts = new CancellationTokenSource(); - var ct = state.ProbeCts.Token; - // Keep the loop Task so ShutdownAsync can await its clean exit before - // disposing the CTS / handles the loop is still using. - state.ProbeTask = Task.Run(() => ProbeLoopAsync(state, ct), ct); + foreach (var state in builtDevices.Values) + { + state.ProbeCts = new CancellationTokenSource(); + var ct = state.ProbeCts.Token; + // Keep the loop Task so a teardown can await its clean exit before + // disposing the CTS / handles the loop is still using. + state.ProbeTask = Task.Run(() => ProbeLoopAsync(state, ct), ct); + } } + _health = new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null); } - else if (_options.Probe.Enabled && _devices.Count > 0) + builtDevices = null; // published: owned by the next retire now + + if (options.Probe.Enabled && string.IsNullOrWhiteSpace(options.Probe.ProbeTagPath) && options.Devices.Count > 0) { // Probe is Enabled but no ProbeTagPath is configured. Without a // tag path the loop has nothing to read, so HostState would stay Unknown forever @@ -486,51 +557,84 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, "as Unknown until a ProbeTagPath is set or Probe.Enabled is set to false.", _driverInstanceId); } - _health = new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null); } catch (Exception ex) { - _health = new DriverHealth(DriverState.Faulted, null, ex.Message); + // Never-published states hold no probe loop and no runtime yet; dispose for symmetry. + if (builtDevices is not null) + foreach (var state in builtDevices.Values) state.Retire(); + if (ex is AbCipInitializationSupersededException) + { + _logger.LogInformation("AbCip driver {DriverInstanceId} initialization abandoned: {Reason}", + _driverInstanceId, ex.Message); + throw; + } + lock (_lifecycleLock) + { + // A superseded init must not overwrite the health of whatever superseded it. + if (generation == _lifecycleGeneration) + _health = new DriverHealth(DriverState.Faulted, null, ex.Message); + } _logger.LogError(ex, "AbCip driver {DriverInstanceId} failed to initialize", _driverInstanceId); throw; } } + /// Bound on joining the retired probe loops — matches the fleet's "≤ 5 s join" teardown rule. + private static readonly TimeSpan ProbeJoinTimeout = TimeSpan.FromSeconds(5); + /// - /// Cancels, joins (bounded) and disposes every existing device state's probe loop + handles, then - /// clears the device/route tables. Device part of only. + /// Retires the published device states: swaps empty device/route/tag tables in under the lifecycle lock + /// (bumping the generation), then cancels, joins (bounded, independent of any caller token — a cancelled + /// caller must not dispose handles under a probe loop that is still running) and disposes the states it took. + /// Concurrent callers each take a disjoint table, so every state is torn down exactly once. The device part of + /// and the first step of . /// - private async Task ReleaseDeviceStatesAsync(CancellationToken cancellationToken) + /// The lifecycle generation this retire produced. + private async Task ReleaseDeviceStatesAsync() { - // The projection is replaced (not reused) when a config body is adopted; dispose the old one so its - // poll loop does not outlive it and raise duplicate alarm events through this driver. - await _alarmProjection.DisposeAsync().ConfigureAwait(false); - foreach (var state in _devices.Values) + Dictionary retired; + AbCipAlarmProjection projection; + int generation; + lock (_lifecycleLock) + { + generation = ++_lifecycleGeneration; + retired = _devices; + _devices = new Dictionary(StringComparer.OrdinalIgnoreCase); + _devicesByRoutingKey = new Dictionary(StringComparer.OrdinalIgnoreCase); + // Rebuilt wholesale by Initialize; swap, never Clear(): the retained poll engine / discovery may be enumerating. + _tagsByRawPath = new(StringComparer.Ordinal); + _declaredTags = new(); + projection = _alarmProjection; + } + + // The projection is replaced (not reused) when a config body is adopted; dispose it so its poll loop does + // not outlive it and raise duplicate alarm events through this driver. Idempotent, and reusable afterwards. + await projection.DisposeAsync().ConfigureAwait(false); + + foreach (var state in retired.Values) { try { state.ProbeCts?.Cancel(); } catch (ObjectDisposedException) { } } - foreach (var state in _devices.Values) + var loops = retired.Values.Select(s => s.ProbeTask).OfType().ToArray(); + if (loops.Length > 0) { - var probeTask = state.ProbeTask; - if (probeTask is null) continue; - // Own bounded timeout, independent of the caller token: cancelling init must not abort the join - // and dispose handles under a probe loop that is still running. - try { await probeTask.WaitAsync(TimeSpan.FromSeconds(5)).ConfigureAwait(false); } - catch (TimeoutException) { } - catch (OperationCanceledException) { } + try { await Task.WhenAll(loops).WaitAsync(ProbeJoinTimeout).ConfigureAwait(false); } + catch (TimeoutException) + { + _logger.LogWarning("AbCip driver {DriverInstanceId}: a probe loop did not exit within {Timeout}; disposing anyway", + _driverInstanceId, ProbeJoinTimeout); + } + catch (Exception) { /* a loop never faults by design; nothing to add here */ } } - foreach (var state in _devices.Values) + foreach (var state in retired.Values) { state.ProbeCts?.Dispose(); state.ProbeCts = null; state.ProbeTask = null; - state.DisposeHandles(); + state.Retire(); } - _devices.Clear(); - _devicesByRoutingKey.Clear(); - // Rebuilt wholesale by Initialize; stale entries would trip its member fan-out collision check. - _tagsByRawPath = new(StringComparer.Ordinal); // swap, never Clear(): the retained poll engine / discovery may be enumerating - _declaredTags = new(); + return generation; } /// @@ -541,46 +645,23 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } /// + /// + /// A complete teardown: the poll engine (and every subscription), the alarm projection, every probe loop + /// (joined ≤ 5 s) and every runtime handle. Safe to run concurrently with and with + /// an it overtakes (that init then publishes nothing). + /// public async Task ShutdownAsync(CancellationToken cancellationToken) { - await _alarmProjection.DisposeAsync().ConfigureAwait(false); await _poll.DisposeAsync().ConfigureAwait(false); - - // Phase 1: signal every probe loop to stop. - foreach (var state in _devices.Values) - { - try { state.ProbeCts?.Cancel(); } catch (ObjectDisposedException) { } - } - - // Phase 2: wait for each probe loop to observe cancellation and exit. The loop never - // throws on cancellation (it catches OperationCanceledException internally), but guard - // anyway so one slow device can't wedge the whole shutdown. - foreach (var state in _devices.Values) - { - var probeTask = state.ProbeTask; - if (probeTask is null) continue; - try - { - await probeTask.WaitAsync(TimeSpan.FromSeconds(10), cancellationToken).ConfigureAwait(false); - } - catch (TimeoutException) { } - catch (OperationCanceledException) { } - } - - // Phase 3: now the loops are gone, dispose the CTS + native handles with no live reader. - foreach (var state in _devices.Values) - { - state.ProbeCts?.Dispose(); - state.ProbeCts = null; - state.ProbeTask = null; - state.DisposeHandles(); - } - _devices.Clear(); - _devicesByRoutingKey.Clear(); - _tagsByRawPath = new(StringComparer.Ordinal); - _declaredTags = new(); + var generation = await ReleaseDeviceStatesAsync().ConfigureAwait(false); _resolver.Clear(); // no-op in v3 (the resolver reads the live table by closure); kept for symmetry - _health = new DriverHealth(DriverState.Unknown, _health.LastSuccessfulRead, null); + lock (_lifecycleLock) + { + // M4: a newer InitializeAsync that published while this shutdown was still joining its probe loops owns the + // health now — stamping Unknown over its Healthy would misreport a live driver. + if (generation == _lifecycleGeneration) + UpdateHealth(h => new DriverHealth(DriverState.Unknown, h.LastSuccessfulRead, null)); + } } // ---- ISubscribable (polling overlay via shared engine) ---- @@ -638,12 +719,12 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, IAbCipTagRuntime? probeRuntime = null; while (!ct.IsCancellationRequested) { - var success = false; - // The probe's backoff verdict for this tick: true = record success, false = record - // failure, null = no signal (a read that returned a non-zero status without throwing - // never recorded either, and still doesn't). Recorded AFTER the try/catch so the - // per-device connect gate is taken outside a catch block — see below. + // The probe's verdicts for this tick, decided SEPARATELY (review 07 H10): + // backoffVerdict — true = record success, false = record failure, null = no signal; + // hostState — the host transition, null = no evidence either way (leave it). + // Recorded AFTER the try/catch so the per-device connect gate is taken outside a catch block. bool? backoffVerdict = null; + HostState? hostState = HostState.Stopped; try { probeRuntime ??= _tagFactory.Create(probeParams); @@ -655,14 +736,51 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, state.ProbeInitialized = true; } await probeRuntime.ReadAsync(ct).ConfigureAwait(false); - success = probeRuntime.GetStatus() == 0; - if (success) + var status = probeRuntime.GetStatus(); + if (status == 0) + { backoffVerdict = true; // Device reachable — reset the data-path window + hostState = HostState.Running; + } + else if (AbCipRefusals.IsDeviceRefusal(status)) + { + // The controller answered, refusing the probe tag: reachable. + backoffVerdict = true; + hostState = HostState.Running; + WarnProbeRefusedOnce(state, ((AbStatus)status).ToString()); + } } catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; } + catch (Exception ex) when (AbCipRefusals.IsDeviceRefusal(ex)) + { + // H10: a missing / typo'd / download-deleted probe tag. The controller ANSWERED over a working session, + // so this is a config error on the probe tag — not a Forward-Open failure. Recording a failure here + // every tick saturated the backoff (5 s ticks vs a 30 s cap ⇒ ShouldAttempt never true) and blacked out + // every cold / evicted data tag on a reachable device, indefinitely. + backoffVerdict = true; + hostState = HostState.Running; + WarnProbeRefusedOnce(state, ex.Message); + // Keep an initialised runtime (its lease stays up — no Forward Open/Close per tick); one whose + // initialisation threw is re-created next tick, as before. + if (!state.ProbeInitialized) + { + try { probeRuntime?.Dispose(); } catch { } + probeRuntime = null; + } + } + catch (Exception ace) when (ace is AbCipTagConfigurationException or AbCipLocalRefusalException) + { + // The probe tag's own name cannot become a request (or the library refused it locally) — it never + // reached the device, so it is no evidence either way: no backoff signal, no host transition, one Warning. + try { probeRuntime?.Dispose(); } catch { } + probeRuntime = null; + state.ProbeInitialized = false; + hostState = null; + WarnProbeRefusedOnce(state, ace.Message); + } catch (Exception ex) { // Wire / init error — tear down the probe runtime so the next tick re-creates it. @@ -674,9 +792,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { probeRuntime?.Dispose(); } catch { } probeRuntime = null; state.ProbeInitialized = false; - // Keep the data-path window open — unless the probe tag's own name is malformed - // — it never reached the device, so it is no evidence either way. - backoffVerdict = ex is AbCipTagConfigurationException ? null : false; + backoffVerdict = false; // the device did not answer — keep the data-path window open } // ConnectionBackoff is caller-serialized, and the data path now records under the @@ -706,7 +822,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } } - TransitionDeviceState(state, success ? HostState.Running : HostState.Stopped); + if (hostState is { } host) TransitionDeviceState(state, host); try { await Task.Delay(_options.Probe.Interval, ct).ConfigureAwait(false); } catch (OperationCanceledException) { break; } @@ -715,6 +831,16 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { probeRuntime?.Dispose(); } catch { } } + /// Logs, once per device state, that the controller answered but refused the configured probe tag. + private void WarnProbeRefusedOnce(DeviceState state, string reason) + { + if (Interlocked.Exchange(ref state.ProbeRefusalWarned, 1) != 0) return; + _logger.LogWarning( + "AbCip probe tag {ProbeTag} on device {Device} cannot be read: {Reason}. The controller answered, so the device " + + "is treated as reachable; fix Probe.ProbeTagPath for driver {DriverInstanceId} (logged once per device)", + _options.Probe.ProbeTagPath, state.Options.HostAddress, reason, _driverInstanceId); + } + private void TransitionDeviceState(DeviceState state, HostState newState) { HostState old; @@ -776,7 +902,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // round trips overlap, which a strictly sequential await chain made impossible. Results are // index-addressed, so completion order is irrelevant. Per-item health verdicts are COLLECTED // and written once after the batch: an inline write to the shared _health field would race. - var maxConcurrent = Math.Max(1, _options.MaxConcurrentReads); + var maxConcurrent = ReadWindow; var outcomes = new ReadOutcome[itemCount]; using var window = new SemaphoreSlim(maxConcurrent, maxConcurrent); var tasks = new Task[itemCount]; @@ -791,32 +917,44 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, ApplyBatchHealth(outcomes, now); return results; + // The window slot is taken INSIDE each item, after its runtime exists (P7): a cold create queues on its + // device's single-flight connect gate, and holding a slot while queued there let one dead device's + // backoff-expiry attempt occupy the whole window for a full Timeout, stalling every other device's reads. async Task RunItemAsync(int slot) { - // Acquire OUTSIDE the try: a wait that throws never acquired, so it must not release. - await window.WaitAsync(cancellationToken).ConfigureAwait(false); - try + if (slot < groups.Count) { - if (slot < groups.Count) - { - outcomes[slot] = await ReadGroupAsync(groups[slot], results, now, cancellationToken) - .ConfigureAwait(false); - } - else - { - var fb = fallbacks[slot - groups.Count]; - outcomes[slot] = await ReadSingleAsync( - fb, fullReferences[fb.OriginalIndex], results, now, cancellationToken) - .ConfigureAwait(false); - } + outcomes[slot] = await ReadGroupAsync(groups[slot], results, now, window, cancellationToken) + .ConfigureAwait(false); } - finally + else { - window.Release(); + var fb = fallbacks[slot - groups.Count]; + outcomes[slot] = await ReadSingleAsync( + fb, fullReferences[fb.OriginalIndex], results, now, window, cancellationToken) + .ConfigureAwait(false); } } } + /// + /// Floor on the read window when the native transport serves this driver (P8). Its runtimes share one + /// request scheduler per device that packs whatever is queued when the pump wakes into one Multiple Service + /// Packet — bounded by the connection's request AND reply byte budget, so a wide window cannot over-pack. The + /// window is therefore the cap on services per round trip: at 8, a 1000-tag controller paid ~125 round trips + /// per poll instead of ~10. + /// + internal const int NativeMinReadWindow = 64; + + /// + /// The effective bounded-parallel read window: (clamped + /// to ≥ 1), raised to at least when the native tag factory serves the reads. + /// libplctag overlaps per-handle round trips only, so it (and any injected factory) keeps the option as is. + /// + internal int ReadWindow => _tagFactory is NativeAbCipTagFactory + ? Math.Max(NativeMinReadWindow, _options.MaxConcurrentReads) + : Math.Max(1, _options.MaxConcurrentReads); + /// /// One read work item's health verdict. Returned by / /// instead of written to the shared health field inline, because @@ -825,11 +963,17 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// A null means "no health signal" — a config-level miss such as an /// unknown reference or unknown device, which never touched the wire and never touched health. /// - private readonly record struct ReadOutcome(DriverState? State, string? Error) + private readonly record struct ReadOutcome(DriverState? State, string? Error, bool Answered = false) { /// The item produced no health signal at all. public static ReadOutcome None => new(null, null); + /// + /// The controller answered with a deterministic refusal: no health STATE signal (a refusal is not a comms + /// fault), but proof the device is reachable, so LastSuccessfulRead advances. + /// + public static ReadOutcome Refused => new(null, null, Answered: true); + /// The item read successfully. public static ReadOutcome Success => new(DriverState.Healthy, null); @@ -858,10 +1002,12 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, private void ApplyBatchHealth(ReadOnlySpan outcomes, DateTime now) { var anySuccess = false; + var anyAnswered = false; DriverState? worst = null; string? worstError = null; foreach (var outcome in outcomes) { + if (outcome.Answered) anyAnswered = true; if (outcome.State is not { } state) continue; if (state == DriverState.Healthy) anySuccess = true; if (worst is null || HealthRank(state) > HealthRank(worst.Value)) @@ -871,18 +1017,25 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } } - // Nothing reached the wire (every reference was unknown / unroutable) — leave health alone, - // exactly as the per-item writes did before. - if (worst is not { } worstState) return; + if (worst is not { } worstState) + { + // No state signal. If a controller answered (a refusal), that still proves it reachable: advance + // LastSuccessfulRead and leave the state alone. Otherwise nothing reached the wire (every reference was + // unknown / unroutable / misconfigured) — leave health alone entirely. + if (anyAnswered) + UpdateHealth(current => current with { LastSuccessfulRead = now }); + return; + } - _health = new DriverHealth( + UpdateHealth(current => new DriverHealth( worstState, - anySuccess ? now : _health.LastSuccessfulRead, - worstState == DriverState.Healthy ? null : worstError); + anySuccess || anyAnswered ? now : current.LastSuccessfulRead, + worstState == DriverState.Healthy ? null : worstError)); } private async Task ReadSingleAsync( - AbCipUdtReadFallback fb, string reference, DataValueSnapshot[] results, DateTime now, CancellationToken ct) + AbCipUdtReadFallback fb, string reference, DataValueSnapshot[] results, DateTime now, + SemaphoreSlim window, CancellationToken ct) { if (!_resolver.TryResolve(reference, out var def)) { @@ -908,87 +1061,112 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, { var runtime = await EnsureTagRuntimeAsync(device, def, ct).ConfigureAwait(false); + // Window slot only now that the runtime exists (see ReadAsync); a wait that throws never acquired. + await window.WaitAsync(ct).ConfigureAwait(false); // Serialise Read → GetStatus → Decode against the shared runtime — the cached libplctag // Tag handle is not concurrency-safe and GetStatus returns the LAST op's status. var opLock = device.GetRuntimeLock(def.Name); - await opLock.WaitAsync(ct).ConfigureAwait(false); try { - await runtime.ReadAsync(ct).ConfigureAwait(false); - - var status = runtime.GetStatus(); - if (status == (int)AbStatus.TypeMismatch) + await opLock.WaitAsync(ct).ConfigureAwait(false); + try { - // Final review I4: the controller ANSWERED with a type that disagrees with the authored - // one — a deterministic config error, not a comms fault. Keep the handle (evicting would - // re-create it — and, for a sole lease holder, re-Forward-Open — on every poll), leave - // health alone, warn once per tag. - results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadTypeMismatch, null, now); - WarnTypeMismatchOnce(def.Name, - "AbCip read of {Tag} on device {Device}: the controller's type does not match the authored " + - "{DataType}; reporting BadTypeMismatch until the tag's dataType is corrected (logged once per tag)", - reference, def.DeviceHostAddress, def.DataType); - return ReadOutcome.None; - } - if (status != 0) - { - // Evict the stale handle so the next call re-creates it. - // A non-zero status can mean the controller dropped the connection or the tag - // handle became permanently invalid (e.g. after a PLC download). Evicting - // mirrors the probe loop's recreate-on-failure behaviour. - EvictRuntime(device, def.Name); - results[fb.OriginalIndex] = new DataValueSnapshot(null, - AbCipStatusMapper.MapStatus(status), null, now); - _logger.LogWarning( - "AbCip read returned non-zero transport status {Status} for tag {Tag} on device {Device}; " + - "evicting cached runtime so next call re-creates it", - status, reference, def.DeviceHostAddress); - return ReadOutcome.Degraded($"transport status {status} reading {reference}"); - } + await runtime.ReadAsync(ct).ConfigureAwait(false); - var tagPath = AbCipTagPath.TryParse(def.TagPath); - var bitIndex = tagPath?.BitIndex; + var status = runtime.GetStatus(); + if (status == (int)AbStatus.TypeMismatch) + { + // Final review I4: the controller ANSWERED with a type that disagrees with the authored + // one — a deterministic config error, not a comms fault. Keep the handle (evicting would + // re-create it — and, for a sole lease holder, re-Forward-Open — on every poll), leave + // health alone, warn once per tag. + results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadTypeMismatch, null, now); + WarnTypeMismatchOnce(def.Name, + "AbCip read of {Tag} on device {Device}: the controller's type does not match the authored " + + "{DataType}; reporting BadTypeMismatch until the tag's dataType is corrected (logged once per tag)", + reference, def.DeviceHostAddress, def.DataType); + return ReadOutcome.Refused; + } + if (AbCipRefusals.IsDeviceRefusal(status)) + { + // The controller answered with a deterministic refusal: per-tag Bad, keep the handle. + results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.MapReadStatus((AbStatus)status), null, now); + WarnRefusalOnce(def.Name, "read", reference, def.DeviceHostAddress, ((AbStatus)status).ToString()); + return ReadOutcome.Refused; + } + if (status != 0) + { + // Evict the stale handle so the next call re-creates it. + // A non-zero status can mean the controller dropped the connection or the tag + // handle became permanently invalid (e.g. after a PLC download). Evicting + // mirrors the probe loop's recreate-on-failure behaviour. + EvictRuntime(device, def.Name); + results[fb.OriginalIndex] = new DataValueSnapshot(null, + AbCipStatusMapper.MapStatus(status), null, now); + _logger.LogWarning( + "AbCip read returned non-zero transport status {Status} for tag {Tag} on device {Device}; " + + "evicting cached runtime so next call re-creates it", + status, reference, def.DeviceHostAddress); + return ReadOutcome.Degraded($"transport status {status} reading {reference}"); + } - // libplctag never checks the CIP type the controller returned; it decodes the bytes as - // the authored type. A scalar whose buffer is not that type's width is mis-typed config - // (seen live: a REAL authored as INT read Good -17346), so refuse it rather than publish - // garbage as Good. Same-width mismatches (DINT vs REAL) cannot be seen this way. - if (!IsArrayTag(def) && bitIndex is null - && ExpectedScalarWidth(def.DataType) is { } width - && runtime.GetSize() is var size && size >= 0 && size != width) - { - // Same class as the native TypeMismatch above (final review I4): deterministic, device - // answered — no Degraded, warn once per tag. - results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadTypeMismatch, null, now); - WarnTypeMismatchOnce(def.Name, - "AbCip read of {Tag} on device {Device} returned {Size} bytes but its authored type {DataType} is {Width} bytes wide; check the tag's dataType (logged once per tag)", - reference, def.DeviceHostAddress, size, def.DataType, width); - return ReadOutcome.None; + var tagPath = AbCipTagPath.TryParse(def.TagPath); + var bitIndex = tagPath?.BitIndex; + + // libplctag never checks the CIP type the controller returned; it decodes the bytes as + // the authored type. A scalar whose buffer is not that type's width is mis-typed config + // (seen live: a REAL authored as INT read Good -17346), so refuse it rather than publish + // garbage as Good. Same-width mismatches (DINT vs REAL) cannot be seen this way. + if (!IsArrayTag(def) && bitIndex is null + && ExpectedScalarWidth(def.DataType) is { } width + && runtime.GetSize() is var size && size >= 0 && size != width) + { + // Same class as the native TypeMismatch above (final review I4): deterministic, device + // answered — no Degraded, warn once per tag. + results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadTypeMismatch, null, now); + WarnTypeMismatchOnce(def.Name, + "AbCip read of {Tag} on device {Device} returned {Size} bytes but its authored type {DataType} is {Width} bytes wide; check the tag's dataType (logged once per tag)", + reference, def.DeviceHostAddress, size, def.DataType, width); + return ReadOutcome.Refused; + } + // Review I-1 — an array tag (the EXPLICIT IsArray flag) decodes the whole buffer into an + // element-typed CLR array (int[]/float[]/bool[]/string[]…), INCLUDING a 1-element array + // (ElementCount 1). Scalar tags keep the single-value path. + var value = IsArrayTag(def) + ? runtime.DecodeArray(def.DataType, Math.Max(1, def.ElementCount)) + : runtime.DecodeValue(def.DataType, bitIndex); + results[fb.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); + return ReadOutcome.Success; + } + finally + { + opLock.Release(); } - // Review I-1 — an array tag (the EXPLICIT IsArray flag) decodes the whole buffer into an - // element-typed CLR array (int[]/float[]/bool[]/string[]…), INCLUDING a 1-element array - // (ElementCount 1). Scalar tags keep the single-value path. - var value = IsArrayTag(def) - ? runtime.DecodeArray(def.DataType, Math.Max(1, def.ElementCount)) - : runtime.DecodeValue(def.DataType, bitIndex); - results[fb.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); - return ReadOutcome.Success; } finally { - opLock.Release(); + window.Release(); } } catch (OperationCanceledException) { throw; } + catch (AbCipLocalRefusalException local) + { + // #737 I2: refused by the library / driver itself — no I/O, so no proof the device answered: BadNotSupported, + // keep the handle, no health signal, LastSuccessfulRead untouched. + results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadNotSupported, null, now); + WarnRefusalOnce(def.Name, "read", reference, def.DeviceHostAddress, local.Message); + return ReadOutcome.None; + } catch (AbTransportException ate) when (ate.Status == AbStatus.Unsupported) { - // A deterministic local refusal — not a comms fault: BadNotSupported, keep the handle, no - // health signal. + // The DEVICE refused (CIP 0x08, or a decode refusal of data it returned) — not a comms fault: + // BadNotSupported, keep the handle, no state change; it answered, so LastSuccessfulRead advances (M2). results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadNotSupported, null, now); - return ReadOutcome.None; + WarnRefusalOnce(def.Name, "read", reference, def.DeviceHostAddress, ate.Message); + return ReadOutcome.Refused; } catch (AbCipTagConfigurationException ace) { @@ -1004,6 +1182,22 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadTypeMismatch, null, now); WarnTypeMismatchOnce(def.Name, "AbCip read of {Tag} on device {Device} refused: {Reason} (logged once per tag)", reference, def.DeviceHostAddress, ate.Message); + return ReadOutcome.Refused; + } + catch (AbTransportException ate) when (AbCipRefusals.IsDeviceRefusal(ate)) + { + // H1/L4: the controller answered "no" (renamed tag, Arr[999], read-protected …) — a per-tag config + // error, not a comms fault. Keep the handle (no evict → no lease / Forward-Open churn), no Degraded, + // no backoff (EnsureTagRuntimeAsync already recorded the answer as reachable), warn once per tag. + results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.MapReadStatus(ate.Status), null, now); + WarnRefusalOnce(def.Name, "read", reference, def.DeviceHostAddress, ate.Message); + return ReadOutcome.Refused; + } + catch (Exception) when (device.Retired) + { + // A re-init / shutdown retired this device under the read: its handles were disposed by the teardown, not + // lost on the wire — no evict (the teardown owns them), no health signal. + results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadCommunicationError, null, now); return ReadOutcome.None; } catch (Exception ex) @@ -1029,7 +1223,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// per-tag fallback list are unaffected. /// private async Task ReadGroupAsync( - AbCipUdtReadGroup group, DataValueSnapshot[] results, DateTime now, CancellationToken ct) + AbCipUdtReadGroup group, DataValueSnapshot[] results, DateTime now, SemaphoreSlim window, CancellationToken ct) { var parent = group.ParentDefinition; @@ -1043,57 +1237,79 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, { var runtime = await EnsureTagRuntimeAsync(device, parent, ct).ConfigureAwait(false); + // Window slot only now that the runtime exists (see ReadAsync); a wait that throws never acquired. + await window.WaitAsync(ct).ConfigureAwait(false); // Serialise the whole-UDT Read → GetStatus → per-member DecodeValueAt against the shared // parent runtime — the members decode from that one handle's buffer. var opLock = device.GetRuntimeLock(parent.Name); - await opLock.WaitAsync(ct).ConfigureAwait(false); try { - await runtime.ReadAsync(ct).ConfigureAwait(false); + await opLock.WaitAsync(ct).ConfigureAwait(false); + try + { + await runtime.ReadAsync(ct).ConfigureAwait(false); - var status = runtime.GetStatus(); - if (status == (int)AbStatus.TypeMismatch) - { - // Final review I4: deterministic — keep the handle, no health signal, warn once. - StampGroupStatus(group, results, now, AbCipStatusMapper.BadTypeMismatch); - WarnTypeMismatchOnce(parent.Name, - "AbCip whole-UDT read of {Parent} on device {Device}: the controller's type does not match " + - "the authored type; {MemberCount} members report BadTypeMismatch (logged once per tag)", - group.ParentName, parent.DeviceHostAddress, group.Members.Count); - return ReadOutcome.None; - } - if (status != 0) - { - EvictRuntime(device, parent.Name); - var mapped = AbCipStatusMapper.MapStatus(status); - StampGroupStatus(group, results, now, mapped); - _logger.LogWarning( - "AbCip whole-UDT read returned non-zero transport status {Status} for parent {Parent} " + - "on device {Device}; {MemberCount} member values stamped with mapped status", - status, group.ParentName, parent.DeviceHostAddress, group.Members.Count); - return ReadOutcome.Degraded($"transport status {status} reading UDT {group.ParentName}"); - } + var status = runtime.GetStatus(); + if (status == (int)AbStatus.TypeMismatch) + { + // Final review I4: deterministic — keep the handle, no health signal, warn once. + StampGroupStatus(group, results, now, AbCipStatusMapper.BadTypeMismatch); + WarnTypeMismatchOnce(parent.Name, + "AbCip whole-UDT read of {Parent} on device {Device}: the controller's type does not match " + + "the authored type; {MemberCount} members report BadTypeMismatch (logged once per tag)", + group.ParentName, parent.DeviceHostAddress, group.Members.Count); + return ReadOutcome.Refused; + } + if (AbCipRefusals.IsDeviceRefusal(status)) + { + StampGroupStatus(group, results, now, AbCipStatusMapper.MapReadStatus((AbStatus)status)); + WarnRefusalOnce(parent.Name, "whole-UDT read", group.ParentName, parent.DeviceHostAddress, ((AbStatus)status).ToString()); + return ReadOutcome.Refused; + } + if (status != 0) + { + EvictRuntime(device, parent.Name); + var mapped = AbCipStatusMapper.MapStatus(status); + StampGroupStatus(group, results, now, mapped); + _logger.LogWarning( + "AbCip whole-UDT read returned non-zero transport status {Status} for parent {Parent} " + + "on device {Device}; {MemberCount} member values stamped with mapped status", + status, group.ParentName, parent.DeviceHostAddress, group.Members.Count); + return ReadOutcome.Degraded($"transport status {status} reading UDT {group.ParentName}"); + } - foreach (var member in group.Members) - { - var value = runtime.DecodeValueAt(member.Definition.DataType, member.Offset, bitIndex: null); - results[member.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); + foreach (var member in group.Members) + { + var value = runtime.DecodeValueAt(member.Definition.DataType, member.Offset, bitIndex: null); + results[member.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); + } + return ReadOutcome.Success; + } + finally + { + opLock.Release(); } - return ReadOutcome.Success; } finally { - opLock.Release(); + window.Release(); } } catch (OperationCanceledException) { throw; } + catch (AbCipLocalRefusalException local) + { + StampGroupStatus(group, results, now, AbCipStatusMapper.BadNotSupported); + WarnRefusalOnce(parent.Name, "whole-UDT read", group.ParentName, parent.DeviceHostAddress, local.Message); + return ReadOutcome.None; + } catch (AbTransportException ate) when (ate.Status == AbStatus.Unsupported) { StampGroupStatus(group, results, now, AbCipStatusMapper.BadNotSupported); - return ReadOutcome.None; + WarnRefusalOnce(parent.Name, "whole-UDT read", group.ParentName, parent.DeviceHostAddress, ate.Message); + return ReadOutcome.Refused; } catch (AbCipTagConfigurationException ace) { @@ -1108,6 +1324,17 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, WarnTypeMismatchOnce(parent.Name, "AbCip whole-UDT read of {Parent} on device {Device} refused: {Reason} (logged once per tag)", group.ParentName, parent.DeviceHostAddress, ate.Message); + return ReadOutcome.Refused; + } + catch (AbTransportException ate) when (AbCipRefusals.IsDeviceRefusal(ate.Status)) + { + StampGroupStatus(group, results, now, AbCipStatusMapper.MapReadStatus(ate.Status)); + WarnRefusalOnce(parent.Name, "whole-UDT read", group.ParentName, parent.DeviceHostAddress, ate.Message); + return ReadOutcome.Refused; + } + catch (Exception) when (device.Retired) + { + StampGroupStatus(group, results, now, AbCipStatusMapper.BadCommunicationError); return ReadOutcome.None; } catch (Exception ex) @@ -1170,6 +1397,44 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, continue; } + string? rmwParent = null; + // U10: an array tag writes the WHOLE array. Shape is checked before any runtime exists, so a refused + // write creates, reads and sends nothing; a refusal is not a fault — no health change. + object?[]? arrayElements = null; + if (IsArrayTag(def)) + { + var count = Math.Max(1, def.ElementCount); + uint? shapeRefusal = null; + string reason = ""; + if (AbCipArrayCodec.ElementWidth(def.DataType) is null) + { + shapeRefusal = AbCipStatusMapper.BadNotSupported; + reason = $"{def.DataType} arrays cannot be written element-wise (BOOL arrays are bit-packed DWORDs, STRING/structure elements are structures)"; + } + else if (AbCipArrayCodec.Elements(w.Value) is not { } elements) + { + shapeRefusal = AbCipStatusMapper.BadTypeMismatch; + reason = $"the value is not an array ({w.Value?.GetType().Name ?? "null"})"; + } + else if (elements.Length != count) + { + shapeRefusal = AbCipStatusMapper.BadOutOfRange; + reason = $"{elements.Length} element(s) written to a {count}-element array"; + } + else + { + arrayElements = elements; + } + if (shapeRefusal is { } refusedShape) + { + results[i] = new WriteResult(refusedShape); + WarnWriteRefusedOnce(def.Name, + "AbCip array write to {Tag} on device {Device} refused: {Reason} (logged once per tag)", + w.FullReference, def.DeviceHostAddress, reason); + continue; + } + } + try { var parsedPath = AbCipTagPath.TryParse(def.TagPath); @@ -1180,11 +1445,12 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // losing one another's update. if (def.DataType == AbCipDataType.Bool && parsedPath?.BitIndex is int bit) { + rmwParent = (parsedPath with { BitIndex = null }).ToLibplctagName(); results[i] = new WriteResult( await WriteBitInDIntAsync(device, parsedPath, bit, w.Value, cancellationToken) .ConfigureAwait(false)); if (results[i].StatusCode == AbCipStatusMapper.Good) - _health = new DriverHealth(DriverState.Healthy, now, null); + RecordWriteAnswered(now); continue; } @@ -1206,10 +1472,13 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // BOOL-array DWORD wrote all 32 bits. Discovered UDT members are where a mis-authored type // is most likely (browse-commit fills dataType from the template, an operator may retype). size = runtime.GetSize(); - refusal = WriteWidthRefusal(def, parsedPath?.BitIndex, size, runtimeTypesBool: runtime is NativeAbCipTagRuntime); + refusal = arrayElements is not null + ? ArrayWriteWidthRefusal(def, size) + : WriteWidthRefusal(def, parsedPath?.BitIndex, size, runtimeTypesBool: runtime is NativeAbCipTagRuntime); if (refusal is null) { - runtime.EncodeValue(def.DataType, parsedPath?.BitIndex, w.Value); + if (arrayElements is not null) runtime.EncodeArray(def.DataType, arrayElements); + else runtime.EncodeValue(def.DataType, parsedPath?.BitIndex, w.Value); await runtime.WriteAsync(cancellationToken).ConfigureAwait(false); status = runtime.GetStatus(); } @@ -1229,20 +1498,36 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, results[i] = new WriteResult(refused); WarnTypeMismatchOnce(def.Name, "AbCip write to {Tag} on device {Device} refused before sending: the controller holds {Size} byte(s) there but the authored {DataType} encodes {Width} (logged once per tag)", - w.FullReference, def.DeviceHostAddress, size, def.DataType, ExpectedScalarWriteWidth(def.DataType)); + w.FullReference, def.DeviceHostAddress, size, def.DataType, + arrayElements is not null + ? AbCipArrayCodec.ElementWidth(def.DataType) * arrayElements.Length + : ExpectedScalarWriteWidth(def.DataType)); } else if (status == (int)AbStatus.TypeMismatch) { // Final review I4: deterministic — keep the handle, leave health, warn once per tag. + // Only libplctag reaches this status-word branch, where it is a real controller answer: the native + // runtime's pre-send type gate THROWS (caught below, no LastSuccessfulRead advance) and never + // returns here with the status merely set (m5). results[i] = new WriteResult(AbCipStatusMapper.BadTypeMismatch); + RecordWriteAnswered(now); WarnTypeMismatchOnce(def.Name, "AbCip write to {Tag} on device {Device} refused: the controller's type does not match the authored {DataType} (logged once per tag)", w.FullReference, def.DeviceHostAddress, def.DataType); } + else if (AbCipRefusals.IsDeviceRefusal(status)) + { + // The controller refused this write (read-only, out of bounds …): keep the handle, no state change; + // it answered, so LastSuccessfulRead advances (M2). + results[i] = new WriteResult(AbCipStatusMapper.MapStatus(status)); + RecordWriteAnswered(now); + WarnRefusalOnce(def.Name, "write", w.FullReference, def.DeviceHostAddress, ((AbStatus)status).ToString()); + } else if (status != 0) { EvictRuntime(device, def.Name); results[i] = new WriteResult(AbCipStatusMapper.MapStatus(status)); + RecordWriteCommsFault($"transport status {status} writing {w.FullReference}"); _logger.LogWarning( "AbCip write returned non-zero transport status {Status} for tag {Tag} on device {Device}; " + "evicting cached runtime so next call re-creates it", @@ -1251,7 +1536,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, else { results[i] = new WriteResult(AbCipStatusMapper.Good); - _health = new DriverHealth(DriverState.Healthy, now, null); + RecordWriteAnswered(now); } } catch (OperationCanceledException) @@ -1269,39 +1554,44 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, "AbCip write not supported for tag {Tag} on device {Device}: {Reason} (logged once per tag)", w.FullReference, def.DeviceHostAddress, nse.Message); } + // H19: the three client-value conversion failures below never touched the wire — they say nothing about + // the device, so they write no health (a Degraded here latched until the next good read, and AbLegacy + // already maps the same three with no health write). Not a transport fault either: no evict. catch (FormatException fe) { - // Value conversion error — not a transport fault; don't evict. results[i] = new WriteResult(AbCipStatusMapper.BadTypeMismatch); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, fe.Message); - _logger.LogWarning(fe, - "AbCip write value-conversion error for tag {Tag} on device {Device}", - w.FullReference, def.DeviceHostAddress); + WarnWriteRefusedOnce(def.Name, + "AbCip write to {Tag} on device {Device}: the value cannot be converted to {DataType}: {Reason} (logged once per tag)", + w.FullReference, def.DeviceHostAddress, def.DataType, fe.Message); } catch (InvalidCastException ice) { results[i] = new WriteResult(AbCipStatusMapper.BadTypeMismatch); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ice.Message); - _logger.LogWarning(ice, - "AbCip write type-cast error for tag {Tag} on device {Device}", - w.FullReference, def.DeviceHostAddress); + WarnWriteRefusedOnce(def.Name, + "AbCip write to {Tag} on device {Device}: the value cannot be converted to {DataType}: {Reason} (logged once per tag)", + w.FullReference, def.DeviceHostAddress, def.DataType, ice.Message); } catch (OverflowException oe) { results[i] = new WriteResult(AbCipStatusMapper.BadOutOfRange); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, oe.Message); - _logger.LogWarning(oe, - "AbCip write value out of range for tag {Tag} on device {Device}", - w.FullReference, def.DeviceHostAddress); + WarnWriteRefusedOnce(def.Name, + "AbCip write to {Tag} on device {Device}: the value is out of range for {DataType}: {Reason} (logged once per tag)", + w.FullReference, def.DeviceHostAddress, def.DataType, oe.Message); + } + catch (AbCipLocalRefusalException local) + { + // Refused locally (e.g. the native runtime cannot type the write; libplctag NotImplemented) — nothing + // sent: BadNotSupported, keep the handle, leave health alone. + results[i] = new WriteResult(AbCipStatusMapper.BadNotSupported); + WarnRefusalOnce(def.Name, "write", w.FullReference, def.DeviceHostAddress, local.Message); } catch (AbTransportException ate) when (ate.Status == AbStatus.Unsupported) { - // A deterministic local refusal (e.g. the native runtime cannot type the write) — not a comms - // fault: BadNotSupported, keep the handle, leave health alone. + // The device refused the write as unsupported — BadNotSupported, keep the handle, no state change; + // it answered, so LastSuccessfulRead advances (M2). results[i] = new WriteResult(AbCipStatusMapper.BadNotSupported); - _logger.LogWarning(ate, - "AbCip write refused (not supported) for tag {Tag} on device {Device}", - w.FullReference, def.DeviceHostAddress); + RecordWriteAnswered(now); + WarnRefusalOnce(def.Name, "write", w.FullReference, def.DeviceHostAddress, ate.Message); } catch (AbCipTagConfigurationException ace) { @@ -1314,17 +1604,33 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // Final review I4: the controller's type disagrees with the authored one (native type gate, or // CIP 0xFF/0x2107 from the controller) — deterministic: BadTypeMismatch, keep the handle, leave // health alone, warn once per tag. Retrying cannot succeed until the config is corrected. + // No LastSuccessfulRead advance: the native type gate throws this BEFORE sending. results[i] = new WriteResult(AbCipStatusMapper.BadTypeMismatch); WarnTypeMismatchOnce(def.Name, "AbCip write to {Tag} on device {Device} refused: {Reason} (logged once per tag)", w.FullReference, def.DeviceHostAddress, ate.Message); } + catch (AbTransportException ate) when (AbCipRefusals.IsDeviceRefusal(ate)) + { + // H1: the controller answered "no" to this write — keep the handle, no state change, warn once; it + // answered, so LastSuccessfulRead advances (M2). + results[i] = new WriteResult(AbCipStatusMapper.MapStatus(ate.Status)); + RecordWriteAnswered(now); + WarnRefusalOnce(def.Name, "write", w.FullReference, def.DeviceHostAddress, ate.Message); + } + catch (Exception) when (device.Retired) + { + // The device was retired under this write by a re-init / shutdown — not a comms fault. + results[i] = new WriteResult(AbCipStatusMapper.BadCommunicationError); + } catch (Exception ex) { - // Transport / wire error — evict so the next write creates a fresh handle. - EvictRuntime(device, def.Name); + // Transport / wire error — evict so the next write creates a fresh handle (for a bit write, the + // parent runtime the read-modify-write used; the bit tag itself holds none). + if (rmwParent is not null) EvictParentRuntime(device, rmwParent); + else EvictRuntime(device, def.Name); results[i] = new WriteResult(AbCipStatusMapper.BadCommunicationError); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); + RecordWriteCommsFault(ex.Message); _logger.LogWarning(ex, "AbCip write transport exception for tag {Tag} on device {Device}", w.FullReference, def.DeviceHostAddress); @@ -1335,10 +1641,51 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } /// - /// Read-modify-write one bit within a DINT parent. Creates / reuses a parallel - /// parent-DINT runtime (distinct from the bit-selector handle) + serialises concurrent - /// writers against the same parent via a per-parent . - /// Matches the Modbus BitInRegister + FOCAS PMC Bit pattern. + /// A good write proves the device answered: advance LastSuccessfulRead but leave the STATE alone. Writing + /// Healthy here (H19) overwrote a Degraded the poll loop had just recorded for other tags. + /// + /// + /// M3: a Degraded that a WRITE comms fault recorded (still the current health — same error, no read has + /// replaced it since) is cleared back to Healthy: the device just accepted a write. A Degraded the read + /// path recorded is left for the read path to clear. + /// + private void RecordWriteAnswered(DateTime now) + { + lock (_writeHealthLock) + { + UpdateHealth(h => h.State == DriverState.Degraded && h.LastError is { } err && ReferenceEquals(err, _writeOwnedError) + ? new DriverHealth(DriverState.Healthy, now, null) + : h with { LastSuccessfulRead = now }); + } + } + + // The exact LastError string INSTANCE the most recent write-side comms fault installed (a fresh copy, so reference + // identity cannot alias a read-side error). A health still carrying it is write-owned: a refusal-only read batch + // keeps it (`with` copies the reference), while any read / poll fault installs a new string and takes ownership. + // Set only under _writeHealthLock TOGETHER with the CAS that installs it, so two concurrent failing writes can never + // leave the marker naming a different instance than the installed health (#737 m1). + private string? _writeOwnedError; + private readonly object _writeHealthLock = new(); + + /// A write-side comms fault: Degraded (never downgrading Faulted), LastSuccessfulRead preserved. + private void RecordWriteCommsFault(string error) + { + var owned = new string(error.AsSpan()); + lock (_writeHealthLock) + { + var installed = UpdateHealth(h => h.State == DriverState.Faulted + ? h + : new DriverHealth(DriverState.Degraded, h.LastSuccessfulRead, owned)); + if (ReferenceEquals(installed.LastError, owned)) _writeOwnedError = owned; + } + } + + /// + /// Read-modify-write one bit within an integer parent (SINT / INT / DINT / LINT — Logix allows .n on + /// each). Creates / reuses a parallel parent runtime (distinct from the bit-selector handle) + serialises + /// concurrent writers against the same parent via a per-parent . The parent's + /// width comes from the size its read returned (H17 — a hard-coded DINT decode threw OutOfBounds on an INT + /// host and was reported as a comms fault). Matches the Modbus BitInRegister + FOCAS PMC Bit pattern. /// private async Task WriteBitInDIntAsync( DeviceState device, AbCipTagPath bitPath, int bit, object? value, CancellationToken ct) @@ -1353,19 +1700,56 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, var parentRuntime = await EnsureParentRuntimeAsync(device, parentName, ct).ConfigureAwait(false); await parentRuntime.ReadAsync(ct).ConfigureAwait(false); var readStatus = parentRuntime.GetStatus(); - if (readStatus != 0) return AbCipStatusMapper.MapStatus(readStatus); + if (readStatus != 0) return BitWriteStatusWord(device, parentName, readStatus, "read"); - var current = Convert.ToInt32(parentRuntime.DecodeValue(AbCipDataType.DInt, bitIndex: null) ?? 0); - var updated = Convert.ToBoolean(value) - ? current | (1 << bit) - : current & ~(1 << bit); + // Unsigned at the narrow widths so a set top bit (SINT bit 7, INT bit 15) encodes without overflowing the + // signed type; DINT keeps its signed int arithmetic (1 << 31 is int.MinValue, which a DINT encodes as is). + var size = parentRuntime.GetSize(); + AbCipDataType? hostType = size switch + { + 1 => AbCipDataType.USInt, + 2 => AbCipDataType.UInt, + 8 => AbCipDataType.LInt, + 4 or <= 0 => AbCipDataType.DInt, // <= 0: the runtime cannot say — the historical DINT assumption + _ => null, + }; + if (hostType is not { } type) + { + WarnTypeMismatchOnce(parentName, + "AbCip bit write to {Parent}.{Bit} on device {Device} refused: the parent is {Size} byte(s), not an integer host (logged once per tag)", + parentName, bit, device.Options.HostAddress, size); + return AbCipStatusMapper.BadTypeMismatch; + } + var widthBits = type switch { AbCipDataType.USInt => 8, AbCipDataType.UInt => 16, AbCipDataType.LInt => 64, _ => 32 }; + if (bit >= widthBits) + { + WarnTypeMismatchOnce(parentName, + "AbCip bit write to {Parent}.{Bit} on device {Device} refused: the parent holds only {Bits} bits (logged once per tag)", + parentName, bit, device.Options.HostAddress, widthBits); + return AbCipStatusMapper.BadOutOfRange; + } - parentRuntime.EncodeValue(AbCipDataType.DInt, bitIndex: null, updated); + var set = Convert.ToBoolean(value); + var decoded = parentRuntime.DecodeValue(type, bitIndex: null) ?? 0; + object updated; + if (type == AbCipDataType.LInt) + { + var current = Convert.ToInt64(decoded); + updated = set ? current | (1L << bit) : current & ~(1L << bit); + } + else + { + var mask = type switch { AbCipDataType.USInt => 0xFF, AbCipDataType.UInt => 0xFFFF, _ => -1 }; + var current = Convert.ToInt32(decoded) & mask; + updated = (set ? current | (1 << bit) : current & ~(1 << bit)) & mask; + } + + parentRuntime.EncodeValue(type, bitIndex: null, updated); await parentRuntime.WriteAsync(ct).ConfigureAwait(false); var writeStatus = parentRuntime.GetStatus(); return writeStatus == 0 ? AbCipStatusMapper.Good - : AbCipStatusMapper.MapStatus(writeStatus); + : BitWriteStatusWord(device, parentName, writeStatus, "write"); } finally { @@ -1373,6 +1757,28 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } } + /// + /// M1: a non-zero status word on the bit write's parent read / write follows the health contract — a device + /// refusal is per-tag Bad with one Warning and no state change (it answered: LastSuccessfulRead advances); any + /// other status is a comms fault: evict the PARENT runtime and write Degraded (LastSuccessfulRead preserved). + /// + private uint BitWriteStatusWord(DeviceState device, string parentName, int status, string phase) + { + var mapped = AbCipStatusMapper.MapStatus(status); + if (AbCipRefusals.IsDeviceRefusal(status)) + { + RecordWriteAnswered(DateTime.UtcNow); + WarnRefusalOnce(parentName, $"bit-write parent {phase}", parentName, device.Options.HostAddress, ((AbStatus)status).ToString()); + return mapped; + } + EvictParentRuntime(device, parentName); + RecordWriteCommsFault($"transport status {status} on the bit-write parent {phase} of {parentName}"); + _logger.LogWarning( + "AbCip bit-write parent {Phase} of {Parent} on device {Device} returned transport status {Status}; evicting the parent runtime", + phase, parentName, device.Options.HostAddress, status); + return mapped; + } + /// /// Get or lazily create a parent-DINT runtime for a parent tag path, cached per-device /// so repeated bit writes against the same DINT share one handle. @@ -1381,6 +1787,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, DeviceState device, string parentTagName, CancellationToken ct) { if (device.ParentRuntimes.TryGetValue(parentTagName, out var existing)) return existing; + ThrowIfRetired(device); var runtime = _tagFactory.Create(device.BuildCreateParams(parentTagName, _options.Timeout)); try @@ -1394,10 +1801,25 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } // Two concurrent callers can both miss the cache + both initialize a runtime; only the // first TryAdd wins. Dispose the loser so it doesn't leak a native tag handle. - if (device.ParentRuntimes.TryAdd(parentTagName, runtime)) - return runtime; - runtime.Dispose(); - return device.ParentRuntimes[parentTagName]; + if (!device.ParentRuntimes.TryAdd(parentTagName, runtime)) + { + runtime.Dispose(); + return device.ParentRuntimes[parentTagName]; + } + Interlocked.MemoryBarrier(); // pairs with DeviceState.Retire + if (device.Retired) + { + if (device.ParentRuntimes.TryRemove(parentTagName, out var orphan)) orphan.Dispose(); + ThrowIfRetired(device); + } + return runtime; + } + + /// Throws when a teardown has retired (a reader raced a re-init / shutdown). + private static void ThrowIfRetired(DeviceState device) + { + if (device.Retired) + throw new ObjectDisposedException(nameof(DeviceState), $"AbCip device {device.Options.HostAddress} was retired by a re-initialization or shutdown."); } /// @@ -1434,6 +1856,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // Re-check under the gate: a sibling cold tag may have connected (or a concurrent write // may have materialised this very handle) while this caller waited. if (device.Runtimes.TryGetValue(def.Name, out var raced)) return raced; + ThrowIfRetired(device); // Inside an open backoff window a data-path caller fails fast — no runtime // create, no Forward-Open. The malformed-path config error above still surfaces (a permanent @@ -1465,10 +1888,10 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, if (!device.Runtimes.TryAdd(def.Name, runtime)) runtime.Dispose(); throw; } - catch (AbCipTagConfigurationException) + catch (Exception ex) when (ex is AbCipTagConfigurationException or AbCipLocalRefusalException) { - // The tag's own name cannot become a request — no I/O happened, so it says nothing - // about the device: leave the backoff window exactly as it was. + // The tag's own name cannot become a request, or the library refused the tag locally — no I/O happened, + // so it says nothing about the device: leave the backoff window exactly as it was. runtime.Dispose(); throw; } @@ -1479,9 +1902,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // not answer. "Tag not found" means the controller replied over a working session; it // is a config error on this one tag, and counting it as a failed Forward-Open throttled // every sibling tag on the device (seen live: one typo blacked out the whole device). - if (ex is AbTransportException ate - && ate.Status != AbStatus.Ok - && AbCipDriverProbe.IsReachableStatus(ate.Status)) + if (AbCipRefusals.IsDeviceRefusal(ex)) device.Backoff.RecordSuccess(); else device.Backoff.RecordFailure(DateTime.UtcNow); @@ -1491,10 +1912,20 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // The gate makes a cold create single-flight, but a concurrent EvictRuntime (a failed read // on this handle) can still land between the re-check and the add; only the first TryAdd // wins. Dispose the loser so it doesn't leak a native tag handle. - if (device.Runtimes.TryAdd(def.Name, runtime)) - return runtime; - runtime.Dispose(); - return device.Runtimes[def.Name]; + if (!device.Runtimes.TryAdd(def.Name, runtime)) + { + runtime.Dispose(); + return device.Runtimes[def.Name]; + } + // A teardown that retired this state while we created would never see this handle: take it back out + // (TryRemove — exactly one of us disposes it) rather than leak it into a dead table. + Interlocked.MemoryBarrier(); // pairs with DeviceState.Retire + if (device.Retired) + { + if (device.Runtimes.TryRemove(def.Name, out var orphan)) orphan.Dispose(); + ThrowIfRetired(device); + } + return runtime; } finally { @@ -1517,6 +1948,15 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } } + /// Evicts + disposes a bit-write parent runtime so the next read-modify-write re-creates it. + private static void EvictParentRuntime(DeviceState device, string parentName) + { + if (device.ParentRuntimes.TryRemove(parentName, out var stale)) + { + try { stale.Dispose(); } catch { } + } + } + /// public DriverHealth GetHealth() => _health; @@ -1744,6 +2184,20 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, : AbCipStatusMapper.BadTypeMismatch; } + /// + /// The refusal for a whole-array write whose authored shape does not fit what the controller holds: the last + /// read returned bytes, which must be the element width × the element count. Unknown + /// size (<= 0) is not judged here (the native runtime re-checks against the controller's type code). + /// + /// The array tag being written. + /// The runtime's read-buffer size in bytes. + /// BadTypeMismatch, or null to write. + internal static uint? ArrayWriteWidthRefusal(AbCipTagDefinition def, int size) + { + if (size <= 0 || AbCipArrayCodec.ElementWidth(def.DataType) is not { } width) return null; + return size == width * Math.Max(1, def.ElementCount) ? null : AbCipStatusMapper.BadTypeMismatch; + } + /// /// Recursively fan a controller-discovered UDT shape out into Variable nodes — one per /// ATOMIC leaf — under the folder produced by . Mirrors the @@ -1896,16 +2350,35 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, internal DeviceState? GetDeviceState(string hostAddress) => _devices.TryGetValue(hostAddress, out var s) ? s : null; - /// Releases all resources used by the driver. - public void Dispose() => DisposeAsync().AsTask().GetAwaiter().GetResult(); - - /// Asynchronously releases all resources used by the driver. + /// + /// Asynchronously releases all resources used by the driver — the same complete teardown as + /// . Idempotent, and safe CONCURRENTLY with a still-running + /// (the runtime's final stop abandons a shutdown that overruns its budget and + /// disposes anyway): each teardown retires a disjoint device table. This is the dispose the runtime calls + /// (it prefers ). + /// /// A value task representing the asynchronous disposal. public async ValueTask DisposeAsync() { await ShutdownAsync(CancellationToken.None).ConfigureAwait(false); } + /// + /// Starts the same teardown as WITHOUT blocking the calling thread on it (review + /// 07 F10: the old DisposeAsync().AsTask().GetAwaiter().GetResult() parked the caller — a dispatcher + /// thread — for up to the probe join, with the usual sync-over-async deadlock exposure). The teardown runs to + /// completion in the background and any fault is logged. Callers that need completion use + /// ; kept only for synchronous using / Dispose() call sites. + /// + public void Dispose() + { + var teardown = DisposeAsync().AsTask(); + if (teardown.IsCompleted) { teardown.GetAwaiter().GetResult(); return; } + _ = teardown.ContinueWith( + t => _logger.LogWarning(t.Exception, "AbCip driver {DriverInstanceId}: background dispose failed", _driverInstanceId), + CancellationToken.None, TaskContinuationOptions.OnlyOnFaulted, TaskScheduler.Default); + } + /// /// Per-device runtime state. Holds the parsed host address, family profile, and the /// live libplctag.NET instances keyed by tag name. @@ -1967,6 +2440,9 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// Gets or sets whether the probe has been initialized. public bool ProbeInitialized { get; set; } + /// 1 once the probe-tag refusal Warning has been logged for this device state. + public int ProbeRefusalWarned; + /// /// The fire-and-forget probe loop's . Stored so /// can await the loop's clean exit after @@ -2053,19 +2529,38 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, AuthoredType: authoredType, UseConnectedMessaging: Profile.SupportsConnectedMessaging); - /// Disposes all runtime tag handles and clears the caches. + private volatile bool _retired; + + /// + /// True once a teardown retired this state. A reader that resolved the device just before the swap can + /// still be running against it: its failures are the teardown's doing, not the device's (no Degraded), and a + /// runtime it cold-creates afterwards is disposed by the creator instead of leaking into a dead table. + /// + public bool Retired => _retired; + + /// Marks the state retired, then disposes every runtime handle (see ). + public void Retire() + { + _retired = true; + // Dekker pairing with the creator's post-add check: the flag is globally visible before the tables are + // enumerated, and the creator fences between its add and its read of the flag — so at least one of the two + // sees the other's write and the handle is disposed. + Interlocked.MemoryBarrier(); + DisposeHandles(); + } + + /// + /// Disposes every runtime tag handle, each exactly once (removed before it is disposed, so a racing + /// creator's own cleanup cannot dispose it a second time). The per-runtime / RMW gates are deliberately NOT + /// disposed, for the same reason as : an in-flight reader of a retired state may + /// still release one, and a only awaited asynchronously holds no wait handle. + /// public void DisposeHandles() { - foreach (var r in Runtimes.Values) r.Dispose(); - Runtimes.Clear(); - foreach (var r in ParentRuntimes.Values) r.Dispose(); - ParentRuntimes.Clear(); - // Dispose + clear the per-runtime/RMW gates so ReinitializeAsync cycles don't orphan - // their SemaphoreSlim instances (each leaks a wait handle once contended). - foreach (var sem in _runtimeLocks.Values) sem.Dispose(); - _runtimeLocks.Clear(); - foreach (var sem in _rmwLocks.Values) sem.Dispose(); - _rmwLocks.Clear(); + foreach (var key in Runtimes.Keys) + if (Runtimes.TryRemove(key, out var r)) r.Dispose(); + foreach (var key in ParentRuntimes.Keys) + if (ParentRuntimes.TryRemove(key, out var r)) r.Dispose(); } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs index e9bedb8f..b927d758 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs @@ -18,10 +18,11 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// non-CIP server) returns Ok = false with a "handshake failed" message instead of /// a false-positive green tick. /// -/// Status-code discrimination: AbStatus.Ok and AbStatus.NotFound mean the controller -/// answered the CIP session (a non-existent probe tag name still proves CIP connectivity) and are -/// treated as "reachable - Ok = true". Every other status (timeout, bad connection, bad reply, -/// partial, abort, ...) is a transport or session failure: "handshake failed - Ok = false". +/// Status-code discrimination: AbStatus.Ok and every device refusal (not found, out of bounds, +/// not allowed, too large/small, type mismatch, unsupported) mean the controller answered the CIP session +/// (a non-existent probe tag name still proves CIP connectivity) and are treated as "reachable - Ok = true". +/// Every other status (timeout, bad connection, bad reply, partial, abort, ...) is a transport or session +/// failure: "handshake failed - Ok = false". /// /// public sealed class AbCipDriverProbe : IDriverProbe @@ -147,7 +148,7 @@ public sealed class AbCipDriverProbe : IDriverProbe { var msg = statusCode == AbStatus.Ok ? "CIP session OK" - : "CIP session OK (controller reachable; probe tag not found)"; + : RefusedProbeMessage(statusCode); return new(true, msg, sw.Elapsed); } @@ -164,7 +165,7 @@ public sealed class AbCipDriverProbe : IDriverProbe if (IsReachableException(ex)) { sw.Stop(); - return new(true, "CIP session OK (controller reachable; probe tag not found)", sw.Elapsed); + return new(true, RefusedProbeMessage(ex.Status), sw.Elapsed); } return new(false, $"Reachable at {host}:{port} but CIP handshake failed: {ex.Message}", null); @@ -180,13 +181,20 @@ public sealed class AbCipDriverProbe : IDriverProbe } /// - /// Returns true for transport statuses that indicate the CIP controller answered - /// but the probe tag name was not found or did not match — sufficient to confirm CIP - /// reachability. Returns true for Ok (tag was found). + /// Returns true for transport statuses that indicate the CIP controller answered — Ok, or any + /// device refusal (: not found, out of bounds, not + /// allowed, too large/small, type mismatch, unsupported) — sufficient to confirm CIP reachability. /// /// The transport status to classify. /// if the status confirms CIP reachability. - internal static bool IsReachableStatus(AbStatus s) => s is AbStatus.Ok or AbStatus.NotFound; + internal static bool IsReachableStatus(AbStatus s) => s == AbStatus.Ok || AbCipRefusals.IsDeviceRefusal(s); + + /// The Test-Connect message for a controller that answered but refused the probe tag. + /// The refusal. + /// The message. + private static string RefusedProbeMessage(AbStatus status) => status == AbStatus.NotFound + ? "CIP session OK (controller reachable; probe tag not found)" + : $"CIP session OK (controller reachable; probe tag refused: {status})"; /// /// Decides whether an is a "controller answered CIP but the @@ -198,7 +206,8 @@ public sealed class AbCipDriverProbe : IDriverProbe internal static bool IsReachableException(AbTransportException ex) { ArgumentNullException.ThrowIfNull(ex); - return IsReachableStatus(ex.Status); + // A local refusal (the library / driver refused without any I/O) proves nothing about the controller. + return ex is not AbCipLocalRefusalException && IsReachableStatus(ex.Status); } private static (AbCipHostAddress? parsed, AbCipDeviceOptions? device) ExtractTarget(AbCipDriverOptions opts) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipInitializationSupersededException.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipInitializationSupersededException.cs new file mode 100644 index 00000000..ffa3c0b1 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipInitializationSupersededException.cs @@ -0,0 +1,18 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +/// +/// Thrown by when a shutdown, dispose or newer initialization retired the +/// driver's tables while this initialization was building, so it published nothing. Deliberately NOT an +/// : nobody cancelled this call, and a caller that treats cancellation as +/// "my own token fired" must not mistake a superseded attempt for one. The superseding operation owns the driver's +/// state; the caller (normally the runtime, which has already abandoned this attempt) can ignore the result. +/// +/// +/// Resilience accounting (#737 m2): not being an OCE, it passes the Initialize pipeline's breaker +/// ShouldHandle (DriverResiliencePipelineBuilder) and, if its generation still matched, the actor's +/// RecordFault. That is practically unreachable — DriverInstanceActor runs initialisation single-flight +/// and drops generation-mismatched results — and a superseded attempt is at worst one counted failure. Recognising +/// it there is a runtime-side (Lane A) change if it is ever wanted. +/// +/// The detail. +public sealed class AbCipInitializationSupersededException(string message) : InvalidOperationException(message); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipLocalRefusalException.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipLocalRefusalException.cs new file mode 100644 index 00000000..baa350e1 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipLocalRefusalException.cs @@ -0,0 +1,15 @@ +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +/// +/// A refusal raised LOCALLY — by the driver's own runtime or by libplctag (ErrorNotImplemented) — before or +/// without any reply from the controller. It carries (so it maps to +/// BadNotSupported) but, unlike a device refusal, proves nothing about the device: it keeps the runtime and +/// writes no health, does NOT advance LastSuccessfulRead, records nothing in the connect backoff, and is not +/// "reachable" for the probe or Test Connect (review 07 / Gitea #737 I2). +/// +/// The refusal detail. +/// The underlying exception, if any. +public sealed class AbCipLocalRefusalException(string message, Exception? inner = null) + : AbTransportException(AbStatus.Unsupported, message, inner); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipRefusals.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipRefusals.cs new file mode 100644 index 00000000..334568a1 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipRefusals.cs @@ -0,0 +1,42 @@ +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +/// +/// The ONE classification of a transport status into "the controller answered" vs "the wire failed", shared by +/// the data path, the probe loop, the Test-Connect probe and the native runtime's initialisation. +/// +/// +/// A device refusal is deterministic: the session works and the controller said no to this one tag (a renamed +/// tag, Arr[999], a read-protected tag, a type the authored config does not match). It is per-tag Bad, +/// keeps the tag runtime (evicting re-creates it — and, for a sole lease holder, re-Forward-Opens — on every poll), +/// never writes Degraded, never opens the device-wide connect backoff, and warns once per tag. Everything +/// else (timeout, bad connection, a reply that failed to parse, partial, abort, …) is a comms fault. +/// +/// The native transport currently folds CIP general statuses 0x1C, 0x20 and 0xFF/0x2104 into +/// (AbStatusCodes.FromCip, owned by Driver.Ab.Transport), which +/// this classification cannot tell from a garbled reply; until the transport maps them explicitly they stay +/// comms faults. +/// +/// +internal static class AbCipRefusals +{ + /// True when proves the controller answered but refused this tag. + /// The transport status. + /// for a deterministic device refusal. + public static bool IsDeviceRefusal(AbStatus status) => status is AbStatus.NotFound or AbStatus.OutOfBounds + or AbStatus.NotAllowed or AbStatus.TooLarge or AbStatus.TooSmall or AbStatus.TypeMismatch + or AbStatus.Unsupported; + + /// True when (an value) is a device refusal. + /// The runtime status word. + /// for a deterministic device refusal. + public static bool IsDeviceRefusal(int status) => IsDeviceRefusal((AbStatus)status); + + /// True when is an carrying a device refusal. + /// The exception. + /// for a deterministic device refusal. + /// An is never a device refusal: nothing reached the device. + public static bool IsDeviceRefusal(Exception ex) => + ex is AbTransportException ate and not AbCipLocalRefusalException && IsDeviceRefusal(ate.Status); +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipStatusMapper.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipStatusMapper.cs index 18296c88..2988a685 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipStatusMapper.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipStatusMapper.cs @@ -15,6 +15,7 @@ public static class AbCipStatusMapper public const uint GoodMoreData = 0x00A60000u; public const uint BadInternalError = 0x80020000u; public const uint BadNodeIdUnknown = 0x80340000u; + public const uint BadNotReadable = 0x803A0000u; public const uint BadNotWritable = 0x803B0000u; public const uint BadOutOfRange = 0x803C0000u; public const uint BadNotSupported = 0x803D0000u; @@ -51,4 +52,14 @@ public static class AbCipStatusMapper /// The OPC UA status code. public static uint MapException(Exception ex) => AbOpcUaStatusMap.MapException(ex); + + /// + /// Status for a READ the controller refused: as , except that a + /// answer to a read is (the shared table's + /// BadNotWritable describes a refused write). + /// + /// The refusal status. + /// The OPC UA status code. + public static uint MapReadStatus(AbStatus status) => + status == AbStatus.NotAllowed ? BadNotReadable : MapStatus(status); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtMemberLayout.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtMemberLayout.cs index b4b271f3..3c44f80f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtMemberLayout.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtMemberLayout.cs @@ -25,6 +25,30 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// public static class AbCipUdtMemberLayout { + /// Layouts already computed, keyed by the parent definition's member list (reference identity). + private static readonly System.Runtime.CompilerServices.ConditionalWeakTable, LayoutBox> Cache = new(); + + /// Holds a computed layout, including a "not groupable" (null) verdict. + private sealed class LayoutBox(IReadOnlyDictionary? offsets) + { + public IReadOnlyDictionary? Offsets { get; } = offsets; + } + + /// + /// , computed once per member list (review 07 P11: the read planner rebuilt the layout — + /// a dictionary allocation and an O(members) walk — for every member reference on every poll, O(members²) per + /// parent per tick). A parent definition's member list is immutable for the life of the tag table that holds + /// it, so caching by list identity is exact; a re-init builds new lists and the old entries are collected with + /// them (). + /// + /// The parent definition's declared members. + /// The member offsets, or null when the members cannot be laid out by declaration. + public static IReadOnlyDictionary? GetOrBuild(IReadOnlyList members) + { + ArgumentNullException.ThrowIfNull(members); + return Cache.GetValue(members, m => new LayoutBox(TryBuild(m))).Offsets; + } + /// /// Try to compute member offsets for the supplied declared members. Returns null /// if any member type is unsupported for declaration-only layout. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtReadPlanner.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtReadPlanner.cs index 28bc181f..338d905e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtReadPlanner.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtReadPlanner.cs @@ -67,7 +67,7 @@ public static class AbCipUdtReadPlanner continue; } - var offsets = AbCipUdtMemberLayout.TryBuild(parent.Members); + var offsets = AbCipUdtMemberLayout.GetOrBuild(parent.Members); if (offsets is null || !offsets.TryGetValue(memberName, out var offset)) { fallback.Add(new AbCipUdtReadFallback(i, name)); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs index 1cf3c2e5..ad8b00a2 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs @@ -85,6 +85,17 @@ public interface IAbCipTagRuntime : IDisposable /// Bit index for BOOL-within-DINT insertion, or null. /// Value to encode. void EncodeValue(AbCipDataType type, int? bitIndex, object? value); + + /// + /// Encode a WHOLE-array write of (values.Count elements of + /// , at the element width — see ) into the local buffer. + /// Callers pair this with . A runtime that cannot write arrays throws + /// . + /// + /// The array element type. + /// The element values; already length-checked against the tag's element count. + void EncodeArray(AbCipDataType type, IReadOnlyList values) => + throw new NotSupportedException($"{GetType().Name} does not support array writes."); } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagStatusTranslation.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagStatusTranslation.cs index 37614e2b..7be295a6 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagStatusTranslation.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagStatusTranslation.cs @@ -14,11 +14,21 @@ internal static class LibplctagStatusTranslation Status.Ok => AbStatus.Ok, Status.Pending => AbStatus.Pending, Status.ErrorTimeout => AbStatus.Timeout, - Status.ErrorNotFound or Status.ErrorNoMatch or Status.ErrorBadDevice => AbStatus.NotFound, + // #737 I2: only ErrorNotFound is the controller saying "no such tag". ErrorNoMatch / ErrorBadDevice describe a + // session or device-level failure (no proof the controller answered this tag), so they are comms-class: with + // the widened refusal classification, folding them into NotFound made a broken session read as "reachable". + Status.ErrorNotFound => AbStatus.NotFound, + Status.ErrorNoMatch or Status.ErrorBadDevice => AbStatus.BadConnection, Status.ErrorNotAllowed => AbStatus.NotAllowed, Status.ErrorOutOfBounds => AbStatus.OutOfBounds, Status.ErrorTooLarge => AbStatus.TooLarge, Status.ErrorTooSmall => AbStatus.TooSmall, + // ErrorNotImplemented is the LIBRARY refusing (no I/O); the exception paths raise it as an + // AbCipLocalRefusalException so it never counts as "the device answered" (see FromException). + // ASSUMPTION (#737 m3): libplctag reports a failed operation by THROWING, so a NotImplemented reaches the driver + // only through FromException. AbStatus has no "local" member (it is Driver.Ab.Transport's, Lane K), so a + // NotImplemented observed only as a STATUS WORD (GetStatus after a non-throwing op) would still classify as a + // device Unsupported refusal; that combination is not produced by libplctag.NET 1.5.2. Status.ErrorUnsupported or Status.ErrorNotImplemented => AbStatus.Unsupported, Status.ErrorPartial => AbStatus.Partial, Status.ErrorAbort => AbStatus.Abort, @@ -70,9 +80,21 @@ internal static class LibplctagStatusTranslation public static async Task Wrap(Func op) { try { await op().ConfigureAwait(false); } - catch (LibPlcTagException ex) { throw new AbTransportException(ToAbStatus(ex), ex.Message, ex); } + catch (LibPlcTagException ex) { throw FromException(ex); } } + /// + /// The transport exception for a libplctag failure: ErrorNotImplemented becomes an + /// (a library refusal, no I/O); everything else an + /// carrying the translated status. + /// + /// The libplctag exception. + /// The exception to throw. + public static AbTransportException FromException(LibPlcTagException ex) => + Enum.TryParse((ex.Message ?? string.Empty).Trim(), ignoreCase: true, out var s) && s == Status.ErrorNotImplemented + ? new AbCipLocalRefusalException($"libplctag does not implement this operation: {ex.Message}", ex) + : new AbTransportException(ToAbStatus(ex), ex.Message ?? string.Empty, ex); + /// Runs , rethrowing any as . /// The result type. /// The libplctag operation. @@ -80,7 +102,7 @@ internal static class LibplctagStatusTranslation public static async Task Wrap(Func> op) { try { return await op().ConfigureAwait(false); } - catch (LibPlcTagException ex) { throw new AbTransportException(ToAbStatus(ex), ex.Message, ex); } + catch (LibPlcTagException ex) { throw FromException(ex); } } /// Runs a synchronous libplctag call, rethrowing as . @@ -90,7 +112,7 @@ internal static class LibplctagStatusTranslation public static T WrapSync(Func op) { try { return op(); } - catch (LibPlcTagException ex) { throw new AbTransportException(ToAbStatus(ex), ex.Message, ex); } + catch (LibPlcTagException ex) { throw FromException(ex); } } /// Runs a synchronous libplctag call, rethrowing as . @@ -98,6 +120,6 @@ internal static class LibplctagStatusTranslation public static void WrapSync(Action op) { try { op(); } - catch (LibPlcTagException ex) { throw new AbTransportException(ToAbStatus(ex), ex.Message, ex); } + catch (LibPlcTagException ex) { throw FromException(ex); } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs index e2ff94c9..07ac008e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs @@ -29,8 +29,19 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime public LibplctagTagRuntime(AbCipTagCreateParams p) { _tag = BuildTag(p); + _boolArrayElement = !p.IsArray && p.AuthoredType == AbCipDataType.Bool + && NativeAbCipTagRuntime.TrySplitBoolArrayElement(p.TagName, out _, out _); } + /// + /// A scalar BOOL authored over a BOOL-array element (Bits[n]). libplctag reads it as whatever the + /// controller returns for that element segment — a packed DWORD — and exposes no type code, so which DWORD + /// came back (and so which bit is element n) cannot be checked here. Review 07 U11: such a read used to decode + /// "low byte != 0" as Good; it is refused (BadNotSupported) when a DWORD came back. The native transport + /// (the default) addresses the right DWORD and decodes the bit. + /// + private readonly bool _boolArrayElement; + /// Builds the (uninitialized) libplctag tag for . /// The tag creation parameters. /// The new, uninitialised libplctag tag. @@ -120,7 +131,10 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime { AbCipDataType.Bool => bitIndex is int bit ? _tag.GetBit(bit) - : _tag.GetInt8(offset) != 0, + : _boolArrayElement && offset == 0 && _tag.GetSize() == 4 + ? throw new AbTransportException(AbStatus.Unsupported, + $"'{_tag.Name}' is a BOOL-array element read as a packed DWORD; libplctag cannot say which DWORD it is — use the native transport.") + : _tag.GetInt8(offset) != 0, AbCipDataType.SInt => (int)(sbyte)_tag.GetInt8(offset), AbCipDataType.USInt => (int)_tag.GetUInt8(offset), AbCipDataType.Int => (int)_tag.GetInt16(offset), @@ -221,6 +235,23 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime _ => 4, }; + /// + /// + /// The codec's bytes are copied into the buffer the last read sized (elem_count elements), so the write + /// sends exactly the new array — never a stale tail. A buffer smaller than the encoded array is refused + /// () before anything is changed. + /// + public void EncodeArray(AbCipDataType type, IReadOnlyList values) => + LibplctagStatusTranslation.WrapSync(() => + { + var bytes = AbCipArrayCodec.Encode(type, values); + var size = _tag.GetSize(); + if (size < bytes.Length) + throw new AbTransportException(AbStatus.TooSmall, + $"Array write to '{_tag.Name}': {bytes.Length} byte(s) encoded but the tag buffer holds {size}."); + for (var i = 0; i < bytes.Length; i++) _tag.SetUInt8(i, bytes[i]); + }); + /// public void EncodeValue(AbCipDataType type, int? bitIndex, object? value) => LibplctagStatusTranslation.WrapSync(() => EncodeValueCore(type, bitIndex, value)); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs index af1ce7e6..a666239c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs @@ -41,6 +41,34 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime private int? _implicitBit; private byte[]? _path; + // U11: a scalar BOOL authored over a BOOL-array element "Bits[n]" — the bit (n % 32) of DWORD n / 32 when the + // array is packed. Null for every other tag. + private int? _boolElementBit; + + // Both addressings of such a tag: "Bits[n / 32]" (packed: a Logix BOOL array is stored as 0xD3 DWORDs) and the + // original "Bits[n]" (per-element: a family that answers 0xC1). _path is whichever _boolLayout selects. + private byte[]? _dwordPath; + private byte[]? _elementPath; + + /// How a BOOL-array element tag's array is laid out, as the controller's REPLY TYPE last showed it. + private enum BoolLayout + { + /// No reply type seen yet (or only refusals / comms faults): request DWORD-addressed, keep asking. + Unsettled, + + /// The controller answered 0xD3: address DWORD n / 32, decode bit n % 32. + Packed, + + /// The controller answered 0xC1: address element n, decode its byte. + PerElement, + } + + // #737 fix round 2 (N1): ONLY a reply type settles (or re-settles) the layout — 0xD3 ⇒ Packed, 0xC1 ⇒ PerElement. + // A refusal or comms fault says nothing about the layout and leaves it as it was, so a tag deployed before the + // online edit that creates its array starts reading correctly once the array exists. A reply whose type disagrees + // with the addressing it was asked under is DISCARDED (never decoded) and re-read under the right addressing. + private BoolLayout _boolLayout; + private ConnectionLease? _lease; private RequestScheduler? _privateScheduler; // ForTest only — production always uses the lease's private LogixTagClient? _client; @@ -51,6 +79,7 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime private AbStatus _status = AbStatus.Pending; private byte[]? _writeBuffer; + private ushort _writeElements = 1; private string? _pendingString; private bool _disposed; @@ -94,16 +123,15 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime /// The trailing bit selector split off the tag name (Tag.5), if any. internal int? ImplicitBit => _implicitBit; - // Performs one read to prove the tag exists: libplctag's InitializeAsync errors for an unknown - // tag, and the driver's backoff classification depends on create throwing AbStatus.NotFound. + // Performs one read to prove the session works and learn the tag's type. // // A name the encoder refuses throws AbCipTagConfigurationException before any lease: it is this // tag's configuration, not the device, and must not black out the device's other tags. A read the - // controller answers with a deterministic refusal (AbStatus.TypeMismatch / AbStatus.Unsupported) - // proves the tag exists, so it completes the initialization with that status rather than throwing: - // a throw here makes the driver dispose the runtime and re-create it - and, for a sole lease - // holder, re-Forward-Open - on every poll (mistype churn). The driver's own read reports the - // refusal. + // controller answers with a deterministic refusal (AbCipRefusals.IsDeviceRefusal: NotFound, OutOfBounds, + // NotAllowed, TooLarge/TooSmall, TypeMismatch, Unsupported) proves the session works, so it completes the + // initialization with that status rather than throwing: a throw here makes the driver dispose the runtime + // and re-create it - and, for a sole lease holder, re-Forward-Open - on every poll (a renamed tag or a + // mistype churned the lease). The driver's own read reports the refusal, per tag, on the kept runtime. /// public async Task InitializeAsync(CancellationToken cancellationToken) { @@ -116,7 +144,20 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime try { split = CipPath.SplitBitSelector(_p.TagName); - path = CipPath.EncodeSymbolic(split.Name); + var name = split.Name; + if (split.Bit is null && !_p.IsArray && _p.AuthoredType == AbCipDataType.Bool + && TrySplitBoolArrayElement(name, out var dwordName, out var bit)) + { + _elementPath = CipPath.EncodeSymbolic(name); + _dwordPath = CipPath.EncodeSymbolic(dwordName); + _boolElementBit = bit; + _boolLayout = BoolLayout.Unsettled; + path = _dwordPath; + } + else + { + path = CipPath.EncodeSymbolic(name); + } } catch (Exception ex) when (ex is ArgumentException or FormatException or OverflowException) { @@ -137,20 +178,45 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime { await ReadAsync(cancellationToken).ConfigureAwait(false); } - catch (AbTransportException ex) when (ex.Status is AbStatus.TypeMismatch or AbStatus.Unsupported) + catch (AbTransportException ex) when (AbCipRefusals.IsDeviceRefusal(ex)) { - // The controller answered — the tag exists. _status already holds the refusal. + // The controller answered. _status already holds the refusal. } } /// - public async Task ReadAsync(CancellationToken cancellationToken) + public Task ReadAsync(CancellationToken cancellationToken) => ReadCoreAsync(cancellationToken, mayRelayout: true); + + private async Task ReadCoreAsync(CancellationToken cancellationToken, bool mayRelayout) { var client = Client(); var path = Path(); try { var (type, handle, data) = await client.ReadAsync(path, _elements, cancellationToken, ExpectedElementBytes(_p.AuthoredType)).ConfigureAwait(false); + if (_boolElementBit is not null) + { + BoolLayout? shown = type == LogixTypeCode.BoolArray ? BoolLayout.Packed + : type == LogixTypeCode.Bool ? BoolLayout.PerElement + : null; // any other type (a mistyped tag): no layout evidence; the type check below reports it + if (shown is { } layout) + { + var askedPacked = ReferenceEquals(path, _dwordPath); + SetBoolLayout(layout); + if ((layout == BoolLayout.Packed) != askedPacked) + { + // The reply is for the OTHER addressing (element n / 32 of a per-element array, or DWORD n of a + // packed one) — not this element. Discard it and ask again under the layout it just showed. + if (mayRelayout) + { + await ReadCoreAsync(cancellationToken, mayRelayout: false).ConfigureAwait(false); + return; + } + throw new AbTransportException(AbStatus.TypeMismatch, + $"'{_p.TagName}': the controller's BOOL-array layout changed between two reads; retrying next read."); + } + } + } _data = data; _typeCode = type; _structHandle = handle; @@ -164,6 +230,13 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime } } + /// Settles a BOOL-array element tag's layout and selects the matching request path. + private void SetBoolLayout(BoolLayout layout) + { + _boolLayout = layout; + _path = layout == BoolLayout.PerElement ? _elementPath : _dwordPath; + } + /// /// Upper bound on one element's wire bytes for the scheduler's reply budget; 0 when the /// authored type is unknown or a structure, which sends the read alone rather than guess. @@ -196,6 +269,14 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime try { + // An unsettled BOOL-array element layout is settled by a read first, so a write never goes to the + // DWORD-addressed element of an element-addressed array (#737 I1). A packed layout's write is refused below + // (0xD3 DWORD — it would clobber 31 neighbours); a per-element one writes "Bits[n]" as a BOOL. + if (_boolElementBit is not null && _boolLayout == BoolLayout.Unsettled) + { + await ReadAsync(cancellationToken).ConfigureAwait(false); + path = Path(); + } if (_pendingString is { } text) { // A STRING writes as the type the controller reported — a Logix STRING is a structure @@ -204,7 +285,7 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime if (_typeCode is null || (_typeCode == LogixTypeCode.Struct && _structHandle is null)) await ReadAsync(cancellationToken).ConfigureAwait(false); if (_typeCode == LogixTypeCode.Struct && _structHandle is null) - throw new AbTransportException(AbStatus.Unsupported, + throw new AbCipLocalRefusalException( $"STRING write to '{_p.TagName}': the controller returned no structure handle."); var payload = EncodeStringPayload(text); await client.WriteAsync(path, _typeCode!.Value, _structHandle, 1, payload, cancellationToken) @@ -213,7 +294,7 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime else if (_writeBuffer is { } buffer) { var type = _typeCode ?? AtomicCode(_p.AuthoredType) - ?? throw new AbTransportException(AbStatus.Unsupported, + ?? throw new AbCipLocalRefusalException( $"Write to '{_p.TagName}': type unknown — no prior read and no authored atomic type."); // The read rule decides first: a same-width mismatch (REAL under a DINT-authored tag) // would otherwise send the int bit pattern as a REAL and report Good. @@ -223,17 +304,18 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime // Width is checked only for codes with a known width: DATE / DATE_AND_TIME (0xCD/0xCF) and // Micro800 BYTE/WORD/LWORD (0xD1/0xD2/0xD4) are outside LogixTypeCode's table and write as today. var width = LogixTypeCode.ByteWidth(type); - if (width > 0 && width != buffer.Length) + if (width > 0 && width * _writeElements != buffer.Length) { // An authored BOOL over a packed BOOL-array DWORD: writing the DWORD would clobber the // 31 sibling bools (libplctag does exactly that), so refuse it as a config error. - if (type == LogixTypeCode.BoolArray) + if (type == LogixTypeCode.BoolArray && _writeElements == 1) throw new NotSupportedException( $"Write to '{_p.TagName}': a BOOL-array element (type 0xD3 DWORD) cannot be written as a single BOOL."); throw new AbTransportException(AbStatus.TypeMismatch, - $"Write to '{_p.TagName}': encoded {buffer.Length} byte(s) but the controller's type 0x{type:X4} is {width} byte(s) wide."); + $"Write to '{_p.TagName}': encoded {buffer.Length} byte(s) for {_writeElements} element(s) but the controller's type 0x{type:X4} is {width} byte(s) wide."); } - await client.WriteAsync(path, type, null, 1, buffer, cancellationToken).ConfigureAwait(false); + // U10: an array write is ONE Write Tag of _writeElements elements (the client fragments it if large). + await client.WriteAsync(path, type, null, _writeElements, buffer, cancellationToken).ConfigureAwait(false); } else { @@ -276,7 +358,7 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime return type switch { - AbCipDataType.Bool => bitIndex is int bit ? GetBit(bit) : Span(offset, 1)[0] != 0, + AbCipDataType.Bool => bitIndex is int bit ? GetBit(bit) : DecodeScalarBool(offset), AbCipDataType.SInt => (int)(sbyte)Span(offset, 1)[0], AbCipDataType.USInt => (int)Span(offset, 1)[0], AbCipDataType.Int => (int)BinaryPrimitives.ReadInt16LittleEndian(Span(offset, 2)), @@ -315,10 +397,22 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime }; } + /// + public void EncodeArray(AbCipDataType type, IReadOnlyList values) + { + ArgumentNullException.ThrowIfNull(values); + _writeBuffer = null; + _pendingString = null; + var buffer = AbCipArrayCodec.Encode(type, values); // throws (nothing staged) before any state changes + _writeElements = (ushort)Math.Clamp(values.Count, 1, ushort.MaxValue); + _writeBuffer = buffer; + } + /// public void EncodeValue(AbCipDataType type, int? bitIndex, object? value) { _writeBuffer = null; + _writeElements = 1; _pendingString = null; switch (type) { @@ -410,6 +504,47 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime catch (Exception) { /* ForTest only; Dispose must not throw */ } } + /// + /// A scalar BOOL. A packed (0xD3) reply is decoded ONLY as the element's own bit of the DWORD the packed layout + /// addressed; any other 0xD3 reply (no element bit, or not settled packed) is refused rather than decoded as + /// "byte 0 != 0" — that value would be some other bit, published as Good (#737 N1). + /// + private bool DecodeScalarBool(int offset) + { + if (offset == 0 && _typeCode == LogixTypeCode.BoolArray) + { + if (_boolElementBit is int element && _boolLayout == BoolLayout.Packed) return GetBit(element); + throw new AbTransportException(AbStatus.TypeMismatch, + $"'{_p.TagName}' is a scalar BOOL but the controller answered a packed BOOL-array DWORD (0xD3) with no element bit to select."); + } + return Span(offset, 1)[0] != 0; + } + + /// + /// Splits a BOOL-array element reference Name[n] (one non-negative subscript, last segment) into the + /// DWORD it lives in (Name[n / 32]) and its bit within that DWORD (n % 32). A Logix BOOL array + /// is stored as DWORDs (type 0xD3) and its element segment addresses those DWORDs. + /// + /// The tag name, bit selector already split off. + /// The name addressing the containing DWORD. + /// The element's bit within that DWORD. + /// when ends in a single subscript. + internal static bool TrySplitBoolArrayElement(string name, out string dwordName, out int bit) + { + dwordName = name; + bit = 0; + if (!name.EndsWith(']')) return false; + var open = name.LastIndexOf('['); + if (open <= 0) return false; + var inner = name[(open + 1)..^1]; + if (inner.Length == 0 || !inner.All(char.IsAsciiDigit) + || !int.TryParse(inner, System.Globalization.NumberStyles.None, System.Globalization.CultureInfo.InvariantCulture, out var n)) + return false; + dwordName = $"{name[..open]}[{n / 32}]"; + bit = n % 32; + return true; + } + private byte[] Path() => _path ?? throw new InvalidOperationException("InitializeAsync has not been called."); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriverFactoryExtensions.cs index 1a589d65..2c417de1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriverFactoryExtensions.cs @@ -38,13 +38,16 @@ public static class CalculationDriverFactoryExtensions /// The driver factory registry to register with. /// Optional logger factory for per-instance diagnostics loggers. /// Optional root script logger (fans failure entries onto the script-logs topic). + /// The node-wide Scripting options every Calculation driver inherits; a key + /// authored in a driver's own config overrides that one knob. Null ⇒ the compiled defaults. public static void Register( DriverFactoryRegistry registry, ILoggerFactory? loggerFactory = null, - ScriptRootLogger? scriptRoot = null) + ScriptRootLogger? scriptRoot = null, + ScriptingOptions? scripting = null) { ArgumentNullException.ThrowIfNull(registry); - registry.Register(DriverTypeName, (id, json) => CreateInstance(id, json, loggerFactory, scriptRoot)); + registry.Register(DriverTypeName, (id, json) => CreateInstance(id, json, loggerFactory, scriptRoot, scripting)); } /// Public overload for the Server-side bootstrapper + test consumers. @@ -63,6 +66,18 @@ public static class CalculationDriverFactoryExtensions public static CalculationDriver CreateInstance( string driverInstanceId, string driverConfigJson, ILoggerFactory? loggerFactory, ScriptRootLogger? scriptRoot) + => CreateInstance(driverInstanceId, driverConfigJson, loggerFactory, scriptRoot, nodeScripting: null); + + /// The full overload: also takes the node-wide Scripting options the driver inherits. + /// The unique identifier for the driver instance. + /// The merged driver config JSON (carries the RawTags array). + /// Optional logger factory for per-instance diagnostics loggers. + /// Optional root script logger for failure fan-out. + /// The node-wide run-guard options; null ⇒ the compiled defaults. + /// The constructed . + public static CalculationDriver CreateInstance( + string driverInstanceId, string driverConfigJson, + ILoggerFactory? loggerFactory, ScriptRootLogger? scriptRoot, ScriptingOptions? nodeScripting) { ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); ArgumentException.ThrowIfNullOrWhiteSpace(driverConfigJson); @@ -71,15 +86,14 @@ public static class CalculationDriverFactoryExtensions ?? throw new InvalidOperationException( $"Calculation driver config for '{driverInstanceId}' deserialised to null"); - // Run-guard knobs are optional and default to the node-wide ScriptingOptions defaults; only an - // explicitly authored key overrides one (so a config carrying neither binds Scripting = null). - ScriptingOptions? scripting = null; - if (dto.MaxAllocBytesPerRun is not null || dto.QuarantineAfterConsecutiveTimeouts is not null) - { - scripting = new ScriptingOptions(); - if (dto.MaxAllocBytesPerRun is { } alloc) scripting.MaxAllocBytesPerRun = alloc; - if (dto.QuarantineAfterConsecutiveTimeouts is { } n) scripting.QuarantineAfterConsecutiveTimeouts = n; - } + var scripting = ResolveScripting( + dto.MaxAllocBytesPerRun, dto.QuarantineAfterConsecutiveTimeouts, nodeScripting, out var adjustments); + var logger = loggerFactory?.CreateLogger(); + // An authored value is never changed silently: the run guard would clamp it anyway, without a word. + foreach (var adjustment in adjustments) + logger?.LogWarning( + "Calculation driver {DriverInstanceId}: its run-guard config was out of range and is clamped: {Adjustment}", + driverInstanceId, adjustment); var options = new CalculationDriverOptions { @@ -89,9 +103,37 @@ public static class CalculationDriverFactoryExtensions Scripting = scripting, }; - return new CalculationDriver( - options, driverInstanceId, scriptRoot, - logger: loggerFactory?.CreateLogger()); + return new CalculationDriver(options, driverInstanceId, scriptRoot, logger: logger); + } + + /// + /// The run-guard options for one Calculation driver. The node-wide options are the base. A key authored + /// in the driver's config overrides only its own knob, on a copy, so the shared node object is never + /// written. With neither, the result is null and the guard uses its compiled defaults. + /// + /// The driver config's maxAllocBytesPerRun, or null. + /// The driver config's quarantineAfterConsecutiveTimeouts, or null. + /// The node-wide options, or null. + /// The clamps applied to the driver's OWN keys (the node options were already + /// clamped and logged at bind), for the caller to log. Empty when nothing was out of range. + /// The options to hand the driver, or null. + internal static ScriptingOptions? ResolveScripting( + long? maxAllocBytesPerRun, int? quarantineAfterConsecutiveTimeouts, ScriptingOptions? nodeScripting, + out IReadOnlyList adjustments) + { + adjustments = []; + if (maxAllocBytesPerRun is null && quarantineAfterConsecutiveTimeouts is null) return nodeScripting; + + var resolved = new ScriptingOptions(); + if (nodeScripting is not null) + { + resolved.MaxAllocBytesPerRun = nodeScripting.MaxAllocBytesPerRun; + resolved.QuarantineAfterConsecutiveTimeouts = nodeScripting.QuarantineAfterConsecutiveTimeouts; + resolved.StuckOrphanQuarantineFactor = nodeScripting.StuckOrphanQuarantineFactor; + } + if (maxAllocBytesPerRun is { } alloc) resolved.MaxAllocBytesPerRun = alloc; + if (quarantineAfterConsecutiveTimeouts is { } n) resolved.QuarantineAfterConsecutiveTimeouts = n; + return resolved.Sanitized(out adjustments); } /// The merged-config DTO the calc factory binds from. The Calculation driver has no diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor index 49ecfd06..496a2bed 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor @@ -21,7 +21,7 @@
-
Read work items in flight at once. Default 8; 1 = sequential.
+
Read work items in flight at once. Default 8; 1 = sequential (libplctag). The native transport always uses at least 64 — it packs concurrent reads into one request.
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/IScriptTagCatalog.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/IScriptTagCatalog.cs index 6088edb3..f4b652e2 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/IScriptTagCatalog.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/IScriptTagCatalog.cs @@ -50,7 +50,7 @@ public sealed record ScriptTagInfo(string Path, string Kind, string DataType, st /// else. Verified chain: AddressSpaceComposer harvests the ctx.GetTag("…") literals /// (via EquipmentScriptPaths.ExtractDependencyRefs) into DependencyRefs; those become /// VirtualTagActor._dependencyRefs, registered with DependencyMuxActor, whose -/// _byRef map is keyed (Ordinal) by DriverInstanceActor.AttributeValuePublished.FullReference +/// _byRef map is keyed (Ordinal) by DriverInstanceActor.AttributeValueItem.FullReference /// — and in v3 a driver's wire reference is the RawPath /// (DriverHostActor._nodeIdByDriverRef is keyed (DriverInstanceId, RawPath)). /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs index ba69b0da..23d610b7 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs @@ -268,7 +268,16 @@ public sealed class ScriptAnalysisService var typeInfo = model.GetTypeInfo(memberAccess.Expression); var type = typeInfo.Type ?? typeInfo.ConvertedType; if (type == null) return null; - return type.GetMembers() + // The type's own members, then the INSTANCE members it inherits (stopping before System.Object, whose + // ToString/GetHashCode noise was never offered). Without the base chain, a member declared only on + // ScriptContext — ctx.CancellationToken — compiled and published but never completed. Inherited statics + // (ScriptContext.Deadband) are left out: they are not reachable through an instance receiver. This applies to + // EVERY receiver, not only ctx: a struct or enum receiver now also offers the System.ValueType / + // System.Enum instance members it inherits (CompareTo, HasFlag, ...), all of which compile. + var inherited = new List(); + for (var b = type.BaseType; b is not null && b.SpecialType != SpecialType.System_Object; b = b.BaseType) + inherited.AddRange(b.GetMembers().Where(m => !m.IsStatic)); + return type.GetMembers().Concat(inherited) .Where(m => m.CanBeReferencedByName && !m.IsImplicitlyDeclared) // NotApplicable covers members (e.g. some BCL/special members) that carry no explicit accessibility but are referenceable. .Where(m => m.DeclaredAccessibility == Accessibility.Public || m.DeclaredAccessibility == Accessibility.NotApplicable) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/ScriptingOptionsBinding.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/ScriptingOptionsBinding.cs new file mode 100644 index 00000000..1155748b --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/ScriptingOptionsBinding.cs @@ -0,0 +1,23 @@ +using Microsoft.Extensions.Configuration; +using ZB.MOM.WW.OtOpcUa.Core.Scripting; + +namespace ZB.MOM.WW.OtOpcUa.Host.Configuration; + +/// +/// Binds the node-wide Scripting section into and clamps it +/// (). One place, so Program.cs and its unit test bind the same +/// way and a misspelled key cannot silently fall back to the compiled defaults unnoticed. +/// +public static class ScriptingOptionsBinding +{ + /// Binds and clamps the Scripting section. + /// The host configuration. + /// One line per clamped value, for the caller to log as a Warning. + /// The clamped options; the compiled defaults when the section is absent. + public static ScriptingOptions Bind(IConfiguration configuration, out IReadOnlyList adjustments) + { + ArgumentNullException.ThrowIfNull(configuration); + var bound = configuration.GetSection(ScriptingOptions.SectionName).Get() ?? new ScriptingOptions(); + return bound.Sanitized(out adjustments); + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs index 45ee0286..7cb8d869 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs @@ -68,6 +68,8 @@ public static class DriverFactoryBootstrap // The calc driver needs the root script logger so a script failure fans out onto the // script-logs topic; resolve it here (null on nodes without the script pipeline wired). var scriptRoot = sp.GetService(); + // The node-wide Scripting section (bound + clamped in Program.cs); null in test graphs ⇒ defaults. + var scripting = sp.GetService(); // The Galaxy driver resolves its Gateway.ApiKeySecretRef secret: arm through the // shared ISecretResolver (registered unconditionally by AddZbSecrets on every node). var secretResolver = sp.GetRequiredService(); @@ -84,7 +86,7 @@ public static class DriverFactoryBootstrap // failures; without this it writes to NullLogger in production. ConfigureAbTransportLogging(sp); Register(registry, loggerFactory, secretResolver, primaryRoleSource, ResolveAbCipDefaultTransport(sp), - scriptRoot); + scriptRoot, scripting); return registry; }); services.AddSingleton(sp => @@ -209,11 +211,12 @@ public static class DriverFactoryBootstrap ISecretResolver secretResolver, IDriverPrimaryRoleSource primaryRoleSource, Driver.AbCip.AbCipTransport abCipDefaultTransport, - ZB.MOM.WW.OtOpcUa.Core.Scripting.ScriptRootLogger? scriptRoot = null) + ZB.MOM.WW.OtOpcUa.Core.Scripting.ScriptRootLogger? scriptRoot = null, + ZB.MOM.WW.OtOpcUa.Core.Scripting.ScriptingOptions? scripting = null) { Driver.AbCip.AbCipDriverFactoryExtensions.Register(registry, loggerFactory, abCipDefaultTransport); Driver.AbLegacy.AbLegacyDriverFactoryExtensions.Register(registry, loggerFactory); - Driver.Calculation.CalculationDriverFactoryExtensions.Register(registry, loggerFactory, scriptRoot); + Driver.Calculation.CalculationDriverFactoryExtensions.Register(registry, loggerFactory, scriptRoot, scripting); Driver.FOCAS.FocasDriverFactoryExtensions.Register(registry, loggerFactory); Driver.Galaxy.GalaxyDriverFactoryExtensions.Register(registry, secretResolver, loggerFactory); Driver.Modbus.ModbusDriverFactoryExtensions.Register(registry, loggerFactory); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs index b2300ab6..c3c105ab 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs @@ -365,10 +365,22 @@ if (hasDriver) // F8b — production virtual-tag evaluator (Roslyn-compiled scripts cached per expression). // Replaces the F8-default NullVirtualTagEvaluator so VirtualTagActor evaluates real user // scripts at runtime. + // Node-wide script run-guard knobs (Scripting:MaxAllocBytesPerRun / QuarantineAfterConsecutiveTimeouts / + // StuckOrphanQuarantineFactor). Bound once and clamped into sane ranges here, so an absurd value is a startup + // Warning rather than an OverflowException inside a script run. The one instance is shared by every script + // seam on the node: the virtual-tag evaluator below, the scripted-alarm engine (DriverHostActor, via + // WithOtOpcUaRuntimeActors) and every Calculation driver (DriverFactoryBootstrap). + var scriptingOptions = ZB.MOM.WW.OtOpcUa.Host.Configuration.ScriptingOptionsBinding.Bind( + builder.Configuration, out var scriptingAdjustments); + foreach (var adjustment in scriptingAdjustments) + Log.Warning("Scripting misconfiguration detected at startup: {Adjustment}", adjustment); + builder.Services.AddSingleton(scriptingOptions); + builder.Services.AddSingleton(sp => new RoslynVirtualTagEvaluator( sp.GetRequiredService().CreateLogger(), - sp.GetRequiredService())); + sp.GetRequiredService(), + scripting: sp.GetRequiredService())); builder.Services.AddSingleton(sp => sp.GetRequiredService()); // Script-log fan-out (Layer 0). The DPS publisher resolves the ActorSystem lazily so it never diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index 76c82ebf..fd29c0b8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -54,7 +54,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Drivers; /// and the ONLY value ingress from driver children ( wraps even a /// single data change in an AttributeValueBatchPublished). /// fabricates a batch of the same shape straight to the dependency mux, so the host neither receives nor -/// sends the per-value AttributeValuePublished any more. +/// sends a per-value message any more (the per-value record is deleted). ///
public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedStash { @@ -202,6 +202,20 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS private readonly IHistoryWriter _historyWriter; private readonly IActorRef? _virtualTagHostOverride; private readonly ScriptRootLogger? _scriptRootLogger; + + /// Node-wide script run-guard options for the scripted-alarm engine; null ⇒ compiled defaults. + private readonly ScriptingOptions? _scripting; + + /// + /// Driver children this host has stopped (, an operator Restart) and is watching + /// until they terminate. A value burst from one of them is DROPPED in : + /// Context.Stop is asynchronous, so the old child keeps publishing for the rest of the apply + /// handler and its own stop, and forwarding those bursts after would + /// re-retain a removed driver's last Good in the dependency mux indefinitely. Akka delivers + /// after the last message the watched child sent here, so removing the entry + /// there bounds the set without ever dropping a live incarnation's values. + /// + private readonly HashSet _retiredChildren = new(); private readonly IActorRef? _scriptedAlarmHostOverride; private readonly ILoggingAdapter _log = Context.GetLogger(); @@ -264,6 +278,11 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// from . private IActorRef? _scriptedAlarmHost; + /// The scripted-alarm plans last sent to , re-sent when it reports a + /// supervision restart (). Null until the + /// first apply. + private IReadOnlyList? _lastScriptedAlarmPlans; + private RevisionHash? _currentRevision; /// @@ -641,8 +660,6 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// VirtualTag host instead of spawning a real child, so tests /// can intercept the message. Null in /// production (the real host is spawned). - /// Retained for constructor-signature stability; unused since the - /// EfAlarmConditionStateStore fallback that consumed it was retired. Defaults to null. /// Optional root script logger required to spawn the ScriptedAlarm /// host (the engine + its script logging hang off it). When null the ScriptedAlarm host is left /// unspawned — the graceful dev/None-deployment path. @@ -694,6 +711,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// capture to a mesh-partner Primary the cluster failure detector reports unreachable. /// Where the OPC UA HistoryUpdate verdict is published for the node manager's /// HistoryUpdate Primary gate. Null on nodes that wire no redundancy manager. + /// The node-wide Scripting run-guard options handed to the scripted-alarm + /// engine. Null ⇒ the compiled defaults. /// The Akka.NET used to spawn this actor. public static Props Props( IDbContextFactory? dbFactory, @@ -707,7 +726,6 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS IVirtualTagEvaluator? virtualTagEvaluator = null, IHistoryWriter? historyWriter = null, IActorRef? virtualTagHostOverride = null, - ILoggerFactory? loggerFactory = null, ScriptRootLogger? scriptRootLogger = null, IActorRef? scriptedAlarmHostOverride = null, IDriverCapabilityInvokerFactory? invokerFactory = null, @@ -728,7 +746,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS Func? sharesAlarmQueueWithLocal = null, TimeSpan? applyOutcomeTimeout = null, bool captureWhilePrimaryUnreachable = true, - IHistoryUpdateRoleView? historyUpdateRoleView = null) => + IHistoryUpdateRoleView? historyUpdateRoleView = null, + ScriptingOptions? scripting = null) => // WARNING: this forwarding list is POSITIONAL, and Props.Create compiles it into an // expression tree. Six IActorRef? parameters and several interface-typed ones mean a // mis-ordered argument is usually type-compatible and therefore compiles clean, then binds @@ -737,11 +756,12 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS Akka.Actor.Props.Create(() => new DriverHostActor( dbFactory, localNode, ackRouter, driverFactory, localRoles, dependencyMux, opcUaPublishActor, healthPublisher, virtualTagEvaluator, historyWriter, virtualTagHostOverride, - loggerFactory, scriptRootLogger, scriptedAlarmHostOverride, invokerFactory, driverMemberCountProvider, + scriptRootLogger, scriptedAlarmHostOverride, invokerFactory, driverMemberCountProvider, deploymentArtifactCache, redundancyRoleView, replicationPeerHost, fetchAndCacheMode, artifactFetcher, alarmStateStore, telemetryHub, primaryRoleSource, historizationRoleView, sharesHistorizationScopeWithLocal, applyWedgeTimeout, reprovisionBackoff, commsRecoveryAuditDelay, - sharesAlarmQueueWithLocal, applyOutcomeTimeout, captureWhilePrimaryUnreachable, historyUpdateRoleView)); + sharesAlarmQueueWithLocal, applyOutcomeTimeout, captureWhilePrimaryUnreachable, historyUpdateRoleView, + scripting)); /// Initializes a new DriverHostActor with the specified dependencies. /// Database context factory for configuration database access. @@ -760,8 +780,6 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// for historized VirtualTag results; defaults to . /// Test seam: when supplied, used as the VirtualTag host /// instead of spawning a real child. - /// Retained for constructor-signature stability; unused since the - /// EfAlarmConditionStateStore fallback that consumed it was retired. Defaults to null. /// Optional root script logger required to spawn the ScriptedAlarm /// host; when null the host is left unspawned. /// Test seam: when supplied, used as the ScriptedAlarm host @@ -830,6 +848,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// ( — the data-plane verdict, but closed until the first /// redundancy snapshot) is published for the node manager's HistoryUpdate Primary gate. Null on nodes that wire no /// redundancy manager. + /// The node-wide Scripting run-guard options handed to the scripted-alarm + /// engine. Null ⇒ the compiled defaults. public DriverHostActor( IDbContextFactory? dbFactory, CommonsNodeId localNode, @@ -842,7 +862,6 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS IVirtualTagEvaluator? virtualTagEvaluator = null, IHistoryWriter? historyWriter = null, IActorRef? virtualTagHostOverride = null, - ILoggerFactory? loggerFactory = null, ScriptRootLogger? scriptRootLogger = null, IActorRef? scriptedAlarmHostOverride = null, IDriverCapabilityInvokerFactory? invokerFactory = null, @@ -863,8 +882,10 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS Func? sharesAlarmQueueWithLocal = null, TimeSpan? applyOutcomeTimeout = null, bool captureWhilePrimaryUnreachable = true, - IHistoryUpdateRoleView? historyUpdateRoleView = null) + IHistoryUpdateRoleView? historyUpdateRoleView = null, + ScriptingOptions? scripting = null) { + _scripting = scripting; _captureWhilePrimaryUnreachable = captureWhilePrimaryUnreachable; _historyUpdateRoleView = historyUpdateRoleView; _applyOutcomeTimeout = applyOutcomeTimeout ?? DefaultApplyOutcomeTimeout; @@ -1146,8 +1167,9 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// null and ApplyScriptedAlarms becomes a no-op. The engine is built around a fresh /// + the wired (the /// replicated LocalDbAlarmConditionStateStore on a driver-role node); when no store was - /// wired the host is skipped. The host (spawned as a child) owns + disposes the engine in its - /// PostStop, so it stops with the driver host. + /// wired the host is skipped. The host is spawned through the factory Props, so EACH + /// incarnation (a supervision restart included) builds, owns and disposes its own engine; it stops + /// with the driver host. /// private void SpawnScriptedAlarmHost() { @@ -1178,13 +1200,23 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS return; } + // Factory Props: every incarnation of the host (a supervision restart included) builds and owns a fresh + // (upstream, engine) pair, so a restart never reuses an engine the previous incarnation disposed or left + // half-loaded. The closure captures locals only; it runs in the child's constructor. var store = _alarmStateStore; - var upstream = new DependencyMuxTagUpstreamSource(); - var engine = new ScriptedAlarmEngine( - upstream, store, new ScriptLoggerFactory(_scriptRootLogger.Logger), _scriptRootLogger.Logger); + var scriptLogger = _scriptRootLogger.Logger; + var scripting = _scripting; _scriptedAlarmHost = Context.ActorOf( ScriptedAlarmHostActor.Props( - _opcUaPublishActor, _dependencyMux, upstream, engine, _localNode, telemetryHub: _telemetryHub), + _opcUaPublishActor, _dependencyMux, + () => + { + var upstream = new DependencyMuxTagUpstreamSource(); + var engine = new ScriptedAlarmEngine( + upstream, store, new ScriptLoggerFactory(scriptLogger), scriptLogger, scripting: scripting); + return (upstream, engine); + }, + _localNode, telemetryHub: _telemetryHub), "scripted-alarm-host"); } @@ -1500,6 +1532,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS Receive(HandleApplyWedged); Receive(HandleGetDiagnostics); Receive(ForwardBatchToMux); + Receive(OnRetiredChildTerminated); + Receive(_ => HandleScriptedAlarmHostRestarted()); Receive(ForwardNativeAlarm); Receive(OnDriverConnectivityChanged); Receive(HandleCommsRecoveryAudit); @@ -1562,6 +1596,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS Receive(HandleApplyWedged); Receive(HandleGetDiagnostics); Receive(ForwardBatchToMux); + Receive(OnRetiredChildTerminated); + Receive(_ => HandleScriptedAlarmHostRestarted()); Receive(ForwardNativeAlarm); Receive(OnDriverConnectivityChanged); Receive(HandleCommsRecoveryAudit); @@ -1606,6 +1642,16 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// private void ForwardBatchToMux(DriverInstanceActor.AttributeValueBatchPublished msg) { + // A burst from an incarnation this host already stopped: forwarding it would undo the SourceRemoved + // eviction (see _retiredChildren). Its ConnectivityChanged is NOT filtered — the respawn path relies on + // that Bad fan to cover the gap. + if (_retiredChildren.Contains(Sender)) + { + _log.Debug("DriverHost {Node}: dropped a {Count}-item value burst from a stopped incarnation of {Id}", + _localNode, msg.Items.Count, msg.DriverInstanceId); + return; + } + // One envelope per burst; the mux filters per item (it registered a batch receive). _dependencyMux?.Tell(msg); @@ -2501,6 +2547,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS Receive(m => _log.Debug("DriverHost {Node}: dropping a {Count}-item value burst from {Id} while Stale (no address space)", _localNode, m.Items.Count, m.DriverInstanceId)); + Receive(OnRetiredChildTerminated); + Receive(_ => HandleScriptedAlarmHostRestarted()); Receive(m => _log.Debug("DriverHost {Node}: dropping a native alarm transition ({Cond}) from {Id} while Stale", _localNode, m.Args.ConditionId, m.DriverInstanceId)); @@ -3652,6 +3700,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS // (re)loads its engine + re-registers mux interest for the union of dependency refs. Covers // both the fresh-apply and bootstrap-restore paths (both call this method); the Stale-recovery // path deliberately does not, matching driver + VirtualTag recovery. + _lastScriptedAlarmPlans = composition.EquipmentScriptedAlarms; _scriptedAlarmHost?.Tell(new ScriptedAlarmHostActor.ApplyScriptedAlarms(composition.EquipmentScriptedAlarms)); if (composition.EquipmentScriptedAlarms.Count > 0) { @@ -3816,8 +3865,10 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS { if (!_children.TryGetValue(driverInstanceId, out var entry)) return; StopDependencyAdapter(driverInstanceId); + RetireChild(entry.Actor); Context.Stop(entry.Actor); _children.Remove(driverInstanceId); + EvictRetainedValues(driverInstanceId); // The instance left the plan (StopChild is only reached via plan.ToStop), so its cached // Health/Resilience snapshots must not replay to new telemetry subscribers. Release the shared // resilience state FIRST: the resilience publisher re-emits the tracker's snapshots every tick, so @@ -3836,6 +3887,56 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS _log.Info("DriverHost {Node}: stopped driver child {Id}", _localNode, driverInstanceId); } + /// + /// Tells the dependency mux to drop every retained value this driver instance produced, so a stopped + /// driver's last Good is never replayed to a later replay-opting subscriber as if it were current. Sent + /// wherever a driver OBJECT ends: a deploy removal, the stop half of a config-change respawn + /// () and an operator Restart (a new driver object). Deliberately NOT sent on a + /// supervision restart (): that re-uses the same driver, whose last value is + /// still its current one. Sent before any respawn, on the same host→mux pair the new incarnation's + /// values travel, so the eviction is ordered ahead of them. + /// + /// + /// The stopped child keeps publishing until it actually terminates, and for a REMOVED driver nothing + /// would ever overwrite a value re-retained after this eviction (it never publishes again, and its + /// PostStop Bad fan resolves no refs once the apply has rebuilt the index without it). So the caller + /// also s it, and drops every burst from a + /// retired child. + /// + /// The driver instance that ended. + private void EvictRetainedValues(string driverInstanceId) => + _dependencyMux?.Tell(new DependencyMuxActor.SourceRemoved(driverInstanceId)); + + /// Marks a child this host is about to stop as retired and watches it until it terminates. + /// The child being stopped. + private void RetireChild(IActorRef child) + { + _retiredChildren.Add(child); + Context.Watch(child); + } + + /// + /// The scripted-alarm host was restarted by supervision and has an empty engine: re-send the last plans so + /// its inputs and operator commands work again without waiting for the next deploy. Only the CURRENT host's + /// signal is honoured. + /// + private void HandleScriptedAlarmHostRestarted() + { + if (_scriptedAlarmHost is null || !Sender.Equals(_scriptedAlarmHost)) return; + if (_lastScriptedAlarmPlans is null) + { + _log.Debug("DriverHost {Node}: ScriptedAlarm host restarted before any apply; nothing to re-send", _localNode); + return; + } + _log.Warning("DriverHost {Node}: ScriptedAlarm host was restarted by supervision; re-applying {Count} scripted alarm(s)", + _localNode, _lastScriptedAlarmPlans.Count); + _scriptedAlarmHost.Tell(new ScriptedAlarmHostActor.ApplyScriptedAlarms(_lastScriptedAlarmPlans)); + } + + /// A retired child has terminated; nothing more can arrive from it. + /// The termination notice. + private void OnRetiredChildTerminated(Terminated msg) => _retiredChildren.Remove(msg.ActorRef); + private static string ActorNameFor(string driverInstanceId, long generation) { // Akka actor names cannot contain '/', ':', or whitespace. Mangle defensively. The monotonic @@ -3881,8 +3982,12 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS // Stop the existing child actor — DriverInstanceActor.PostStop calls ShutdownAsync. StopDependencyAdapter(msg.DriverInstanceId); + RetireChild(entry.Actor); Context.Stop(entry.Actor); _children.Remove(msg.DriverInstanceId); + // An operator Restart builds a NEW driver object, so the old one's retained values are evicted like a + // removal's; the respawned driver republishes its own. + EvictRetainedValues(msg.DriverInstanceId); // The old incarnation's health must not read as the new one's until it publishes its own. _healthPublisher.Forget(msg.DriverInstanceId); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs index c1ad189b..287e0763 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -108,21 +108,16 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// state — a no-op when already Reconnecting or Connecting. /// public sealed record ForceReconnect; - /// One driver-published value. No production sender remains — the subscription path - /// batches every data change into an , and the comms-loss fan - /// (DriverHostActor.FanCommsLossToDependencyMux), which was the last sender to fabricate this - /// message, now fabricates a batch too. The dependency mux still accepts it for tests and direct senders. + /// One value within an . /// is the driver's full OPC UA status code including its sub-code /// — not a severity bucket. Classify it with and friends rather - /// than comparing it to . - public sealed record AttributeValuePublished(string DriverInstanceId, string FullReference, object? Value, uint StatusCode, DateTime TimestampUtc); - /// One value within an . + /// than comparing it to . /// A struct deliberately: an AttributeValueItem[] then holds its items inline, so a burst of /// N values costs one array allocation instead of one array plus N boxes. Indexed access through /// does not box. public readonly record struct AttributeValueItem(string FullReference, object? Value, uint StatusCode, DateTime TimestampUtc); - /// Batched form of — every changed attribute of ONE - /// driver production burst in one message. The parent fans the whole batch to the mux and the OPC UA + /// The ONLY value message a driver child sends: every changed attribute of ONE driver production + /// burst in one message (a single data change is a one-item batch). The parent fans the whole batch to the mux and the OPC UA /// publish path in one hop each. Status codes ride verbatim. public sealed record AttributeValueBatchPublished(string DriverInstanceId, IReadOnlyList Items); private sealed record DataChangeForward(string FullReference, DataValueSnapshot Snapshot); @@ -132,7 +127,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers private sealed record DataChangeBatchForward(IReadOnlyList Items); /// Published to the parent whenever the subscribed driver (an ) fires /// . The parent () projects + routes it - /// to the materialised Part 9 condition. Parallels . + /// to the materialised Part 9 condition. Parallels . public sealed record AttributeAlarmPublished(string DriverInstanceId, AlarmEventArgs Args); private sealed record NativeAlarmRaised(AlarmEventArgs Args); /// Self-sent on Connected entry / when alarm refs are (re)pushed, to establish the native-alarm diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs index 210a84ca..60f05729 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs @@ -86,6 +86,21 @@ public sealed class ScriptedAlarmHostActor : ReceiveActor /// The desired Equipment-namespace scripted-alarm plans. public sealed record ApplyScriptedAlarms(IReadOnlyList Plans); + /// + /// Told to the parent from . A supervision restart is otherwise invisible to the + /// parent, and the new incarnation has an EMPTY engine (factory Props) and no mux interest, so until the + /// next its inputs stop and every operator command is dropped as not + /// owned. The parent (DriverHostActor) answers by re-sending the last plans it applied, mirroring + /// how a driver child's ReprovisionRequested is answered. + /// + public sealed class ScriptedAlarmHostRestarted + { + /// The single instance. + public static readonly ScriptedAlarmHostRestarted Instance = new(); + + private ScriptedAlarmHostRestarted() { } + } + /// Marshals an engine emission off the engine's worker thread onto the actor thread. /// Carries the the engine raised on OnEvent. private sealed record EngineEmission(ScriptedAlarmEvent Event); @@ -384,9 +399,10 @@ public sealed class ScriptedAlarmHostActor : ReceiveActor // subscriptions exist, so any DependencyValueChanged the mux delivers will be observed by the // engine. RegisterInterest replaces a subscriber's prior interest set (see DependencyMuxActor), // so a re-Apply with a fresh union simply supersedes the old one — no explicit unregister needed. - // ReplayRetained: the mux replays the last value it saw for each ref, so a fresh engine (a - // supervision restart builds one around an EMPTY upstream) converges immediately instead of - // waiting for every input to change again. + // ReplayRetained: the mux replays the last value it saw for each ref, so a freshly loaded engine + // converges immediately instead of waiting for every input to change again. That includes the engine + // a supervision restart builds around an EMPTY upstream, but only once the parent has re-sent + // ApplyScriptedAlarms in answer to ScriptedAlarmHostRestarted (PostRestart); nothing reloads it before. _mux?.Tell(new DependencyMuxActor.RegisterInterest(msg.DepRefs, Self, ReplayRetained: true)); // Prune the upstream's retained snapshots to the loaded set (R02:L3): a path no alarm references // any more will never be pushed again, so its last value is dead weight for the process lifetime. @@ -791,6 +807,15 @@ public sealed class ScriptedAlarmHostActor : ReceiveActor base.PreRestart(reason, message); } + /// + /// Tells the parent so it re-sends the last + /// : the new incarnation has nothing loaded and no mux interest. + protected override void PostRestart(Exception reason) + { + base.PostRestart(reason); + Context.Parent.Tell(ScriptedAlarmHostRestarted.Instance); + } + /// protected override void PostStop() { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs index e6e58419..ea327b42 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs @@ -609,7 +609,6 @@ public static class ServiceCollectionExtensions healthPublisher: healthPublisher, virtualTagEvaluator: virtualTagEvaluator, historyWriter: historyWriter, - loggerFactory: loggerFactory, scriptRootLogger: scriptRootLogger, invokerFactory: invokerFactory, deploymentArtifactCache: deploymentArtifactCache, @@ -638,7 +637,10 @@ public static class ServiceCollectionExtensions sharesAlarmQueueWithLocal: BuildAlarmQueuePeerPredicate( resolver.GetService(), roleInfo, replicationPeerHost), // The HistoryUpdate verdict (closed until the first redundancy snapshot). - historyUpdateRoleView: resolver.GetService()), + historyUpdateRoleView: resolver.GetService(), + // The node-wide Scripting section (bound + clamped in Program.cs) for the scripted-alarm + // engine. Unbound in test graphs ⇒ null ⇒ the compiled defaults. + scripting: resolver.GetService()), DriverHostActorName); registry.Register(driverHost); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs index 40e9bf19..51174b9e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs @@ -15,11 +15,9 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.VirtualTags; /// DriverHostActor forwards driver-child value publishes to this mux (one mux per driver-role /// node). The ONLY driver ingress is the batched /// — one envelope per driver -/// production burst, which the mux filters per item. The per-value -/// receive has no production sender left -/// at all: the comms-loss fan (DriverHostActor.FanCommsLossToDependencyMux) was its last -/// one and now fabricates an AttributeValueBatchPublished instead. The receive is kept for -/// tests and direct senders — treat it as dead in production. +/// production burst, which the mux filters per item. The comms-loss fan +/// (DriverHostActor.FanCommsLossToDependencyMux) fabricates one too. There is no per-value +/// ingress: a single value is a one-item batch (tests build one with ValuePublishes.Single). /// /// The mux emits ONLY batches, so a subscriber's /// batch receive is its complete hot path. Every subscriber keeps a per-value @@ -52,11 +50,9 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.VirtualTags; /// Source removal. The mux records which driver instance last published each retained ref; /// evicts every value that driver produced, so a removed driver's last /// Good is never replayed as current. A comms-loss fan (a Bad batch) already overwrites the retained -/// value with Bad. ⚠️ As of fix round 1 no production actor sends yet — -/// DriverHostActor (outside the scripting lane) must Tell it when it stops / removes a driver -/// child. Until then a removed driver's last value can still be replayed to a replay-opting -/// subscriber that re-registers for that ref while another replay-opting subscriber holds it, or -/// within the grace window. +/// value with Bad. DriverHostActor sends wherever a driver object +/// ends (a deploy removal, the stop half of a config-change respawn, an operator Restart), and never +/// on a supervision restart, which re-uses the same driver. /// /// public sealed class DependencyMuxActor : ReceiveActor, IWithTimers @@ -154,7 +150,6 @@ public sealed class DependencyMuxActor : ReceiveActor, IWithTimers Receive(msg => RemoveSubscriber(msg.Subscriber)); Receive(OnSourceRemoved); Receive(_ => OnSweep()); - Receive(OnAttributeValuePublished); Receive(OnAttributeValueBatchPublished); Receive(msg => RemoveSubscriber(msg.ActorRef)); } @@ -265,30 +260,6 @@ public sealed class DependencyMuxActor : ReceiveActor, IWithTimers _log.Debug("DependencyMux: source {Driver} removed — evicted {Count} retained value(s)", msg.DriverInstanceId, evicted); } - private void OnAttributeValuePublished(DriverInstanceActor.AttributeValuePublished msg) - { - if (!_byRef.TryGetValue(msg.FullReference, out var entry)) - { - // Nothing is registered for (or retaining) this ref — drop. Common in normal operation; the address - // space carries thousands of tags and only a fraction feed virtual-tag expressions. - return; - } - var dep = new VirtualTagActor.DependencyValueChanged(msg.FullReference, msg.Value, msg.TimestampUtc, msg.StatusCode); - if (entry.Retains) - { - entry.Last = dep; - entry.LastSource = msg.DriverInstanceId; - } - foreach (var sub in entry.Subscribers) - { - // A single-item batch, so the mux emits exactly ONE message shape and every subscriber needs - // only the batch receive on its hot path. One list per subscriber: batch ownership transfers - // to the receiver, so a shared list must never cross mailboxes. Nothing in production reaches - // this path (see the class doc) — the extra allocation is noise on a test-only seam. - sub.Tell(new VirtualTagActor.DependencyValueBatch(new[] { dep })); - } - } - private void OnAttributeValueBatchPublished(DriverInstanceActor.AttributeValueBatchPublished msg) { // Nothing registered and nothing retained — skip the whole burst rather than probing an empty map per item. diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptCancellationTokenTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptCancellationTokenTests.cs new file mode 100644 index 00000000..a465d2d4 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptCancellationTokenTests.cs @@ -0,0 +1,67 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests; + +/// +/// ctx.CancellationToken — the run token, visible to scripts. already +/// cancels the token it hands when a run times out; +/// a script that polls it can now end its own abandoned run instead of burning a pool thread until it +/// returns. Cooperative only: a script that ignores it is still an orphan. +/// +[Trait("Category", "Unit")] +public sealed class ScriptCancellationTokenTests +{ + /// Spins until cancelled, but ends by itself after 10 s so a broken implementation cannot pin a + /// pool thread past the test. + private const string CooperativeSpin = + """ + var end = System.DateTime.UtcNow.AddSeconds(10); + while (!ctx.CancellationToken.IsCancellationRequested && System.DateTime.UtcNow < end) { } + return ctx.CancellationToken.IsCancellationRequested ? -1 : 1; + """; + + [Fact] + public async Task A_script_sees_an_uncancelled_token_on_a_normal_run() + { + using var eval = ScriptEvaluator.Compile( + "return ctx.CancellationToken.CanBeCanceled && !ctx.CancellationToken.IsCancellationRequested;"); + using var cts = new CancellationTokenSource(); + + (await eval.RunAsync(new FakeScriptContext(), cts.Token)).ShouldBeTrue(); + } + + [Fact] + public async Task Outside_a_run_the_token_is_None() + { + using var eval = ScriptEvaluator.Compile("return 1;"); + var ctx = new FakeScriptContext(); + (await eval.RunAsync(ctx)).ShouldBe(1); + + ctx.CancellationToken.ShouldBe(CancellationToken.None, "the token is scoped to the run, not left on the context"); + } + + [Fact] + public async Task A_timed_out_run_whose_script_polls_the_token_ends_its_own_orphan() + { + var guard = new ScriptRunGuard(); + using var eval = ScriptEvaluator.Compile(CooperativeSpin); + var ct = TestContext.Current.CancellationToken; + + await Should.ThrowAsync( + () => guard.RunAsync("k", CooperativeSpin, eval, new FakeScriptContext(), TimeSpan.FromMilliseconds(100), ct)); + + // The guard cancelled the run token on timeout; the script saw it and returned, which frees the slot well + // inside its own 10 s fallback. Without ctx.CancellationToken the orphan would hold it for the full 10 s. + var deadline = DateTime.UtcNow.AddSeconds(3); + while (guard.InFlightCount != 0 && DateTime.UtcNow < deadline) + await Task.Delay(20, ct); + guard.InFlightCount.ShouldBe(0, "the cooperative script ended its abandoned run"); + + // Positive control: the slot is free, so the next run is dispatched rather than refused. Same source + // identity on purpose: a different one would reset the key and dispatch even behind a live orphan. + using var fast = ScriptEvaluator.Compile("return 5;"); + (await guard.RunAsync("k", CooperativeSpin, fast, new FakeScriptContext(), TimeSpan.FromSeconds(5), ct)) + .ShouldBe(5); + } +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptingOptionsTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptingOptionsTests.cs new file mode 100644 index 00000000..c702854a --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/ScriptingOptionsTests.cs @@ -0,0 +1,109 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests; + +/// +/// — the bind-time clamp of the Scripting section's numeric +/// knobs. An absurd value must be pulled into a sane range at bind time rather than surfacing later as an +/// inside a script run (timeout * StuckOrphanQuarantineFactor). +/// +[Trait("Category", "Unit")] +public sealed class ScriptingOptionsTests +{ + [Fact] + public void Defaults_pass_through_unchanged_with_no_adjustments() + { + var sanitized = new ScriptingOptions().Sanitized(out var adjustments); + + adjustments.ShouldBeEmpty(); + sanitized.MaxAllocBytesPerRun.ShouldBe(ScriptingOptions.DefaultMaxAllocBytesPerRun); + sanitized.QuarantineAfterConsecutiveTimeouts.ShouldBe(ScriptingOptions.DefaultQuarantineAfterConsecutiveTimeouts); + sanitized.StuckOrphanQuarantineFactor.ShouldBe(ScriptingOptions.DefaultStuckOrphanQuarantineFactor); + } + + [Theory] + [InlineData(double.MaxValue, ScriptingOptions.MaxStuckOrphanQuarantineFactor)] + [InlineData(double.PositiveInfinity, ScriptingOptions.MaxStuckOrphanQuarantineFactor)] + [InlineData(1e300, ScriptingOptions.MaxStuckOrphanQuarantineFactor)] + [InlineData(double.NaN, ScriptingOptions.DefaultStuckOrphanQuarantineFactor)] + [InlineData(-5, 0)] + [InlineData(double.NegativeInfinity, 0)] + [InlineData(0, 0)] + [InlineData(2.5, 2.5)] + public void StuckOrphanQuarantineFactor_is_clamped(double configured, double expected) + { + var sanitized = new ScriptingOptions { StuckOrphanQuarantineFactor = configured }.Sanitized(out var adjustments); + + sanitized.StuckOrphanQuarantineFactor.ShouldBe(expected); + // Negative already means "disabled" and is documented as such; normalising it to 0 is not worth a warning. + if (!double.IsNaN(configured) && configured >= 0 && configured <= ScriptingOptions.MaxStuckOrphanQuarantineFactor) + adjustments.ShouldBeEmpty(); + } + + [Theory] + [InlineData(0, 1)] + [InlineData(-3, 1)] + [InlineData(int.MaxValue, ScriptingOptions.MaxQuarantineAfterConsecutiveTimeouts)] + [InlineData(5, 5)] + public void QuarantineAfterConsecutiveTimeouts_is_clamped(int configured, int expected) => + new ScriptingOptions { QuarantineAfterConsecutiveTimeouts = configured }.Sanitized(out _) + .QuarantineAfterConsecutiveTimeouts.ShouldBe(expected); + + [Theory] + [InlineData(-1L, 0L)] + [InlineData(0L, 0L)] + [InlineData(1024L, ScriptingOptions.MinMaxAllocBytesPerRun)] + [InlineData(long.MaxValue, long.MaxValue)] + public void MaxAllocBytesPerRun_is_clamped(long configured, long expected) => + new ScriptingOptions { MaxAllocBytesPerRun = configured }.Sanitized(out _) + .MaxAllocBytesPerRun.ShouldBe(expected); + + [Fact] + public void An_out_of_range_value_is_reported_as_an_adjustment_naming_the_key() + { + _ = new ScriptingOptions { StuckOrphanQuarantineFactor = 1e300 }.Sanitized(out var adjustments); + + adjustments.ShouldHaveSingleItem().ShouldContain("Scripting:StuckOrphanQuarantineFactor"); + } + + [Fact] + public void Sanitized_returns_a_copy_and_leaves_the_source_untouched() + { + var source = new ScriptingOptions { StuckOrphanQuarantineFactor = 1e300 }; + + var sanitized = source.Sanitized(out _); + + sanitized.ShouldNotBeSameAs(source); + source.StuckOrphanQuarantineFactor.ShouldBe(1e300); + } + + /// + /// The guard takes options from more places than the bind (the Calculation driver's per-driver DTO, tests), + /// so it must not overflow either: an absurd factor reaching the guard directly is clamped, and a refusal + /// behind a young orphan is still a plain refusal rather than an . + /// + [Fact] + public async Task An_absurd_factor_reaching_the_guard_directly_does_not_overflow_on_a_refusal() + { + var guard = new ScriptRunGuard(new ScriptingOptions { StuckOrphanQuarantineFactor = double.MaxValue }); + const string spin = + """ + var end = System.DateTime.UtcNow.AddSeconds(3); + while (System.DateTime.UtcNow < end) { } + return 1; + """; + using var eval = ScriptEvaluator.Compile(spin); + var ct = TestContext.Current.CancellationToken; + var timeout = TimeSpan.FromMilliseconds(60); + + await Should.ThrowAsync( + () => guard.RunAsync("k", spin, eval, new FakeScriptContext(), timeout, ct)); + // The refusal path evaluates timeout * factor. Unclamped, TimeSpan * double.MaxValue throws OverflowException. + await Should.ThrowAsync( + () => guard.RunAsync("k", spin, eval, new FakeScriptContext(), timeout, ct)); + // A huge timeout times the clamped maximum factor must not overflow either. + await Should.ThrowAsync( + () => guard.RunAsync("k", spin, eval, new FakeScriptContext(), TimeSpan.FromDays(365 * 1000), ct)); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbCipRefusalLiveTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbCipRefusalLiveTests.cs new file mode 100644 index 00000000..c5aebf8d --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbCipRefusalLiveTests.cs @@ -0,0 +1,172 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests; + +/// +/// Review 07 (Gitea #737) against a running ab_server on both transports: a controller refusal is a per-tag +/// config error, not a comms fault (H10 probe, H1/L4 data path), and a whole-array write round-trips (U10). +/// Skipped when the fixture is unreachable (); the array write additionally +/// needs AB_SERVER_ALLOW_WRITES=1 (). +/// +[Trait("Category", "Integration")] +[Trait("Requires", "AbServer")] +public sealed class AbCipRefusalLiveTests +{ + private static string Device(AbServerFixture fixture) => $"ab://{fixture.Host}:{fixture.Port}/1,0"; + + private static async Task WaitUntilAsync(Func condition, TimeSpan timeout, string what) + { + var deadline = DateTime.UtcNow + timeout; + while (!condition()) + { + if (DateTime.UtcNow > deadline) throw new TimeoutException(what); + await Task.Delay(50); + } + } + + /// + /// H10: a typo'd probe tag on a reachable controller leaves the host Running, and a COLD data tag created + /// after many probe ticks still connects (the backoff window was never opened by the probe's NotFound). + /// + /// The transport stack the driver runs on. + [AbServerTheory] + [MemberData(nameof(TransportMatrix.Transports), MemberType = typeof(TransportMatrix))] + public async Task A_typod_probe_tag_does_not_black_out_a_reachable_device(AbCipTransport transport) + { + var device = Device(new AbServerFixture()); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(device, AbCipPlcFamily.ControlLogix)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Counter", device, "TestDINT", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions + { + Enabled = true, + ProbeTagPath = "NoSuchProbeTag_xyz", + Interval = TimeSpan.FromMilliseconds(200), + Timeout = TimeSpan.FromSeconds(2), + }, + Timeout = TimeSpan.FromSeconds(5), + Transport = transport, + }, $"drv-h10-{transport}"); + await drv.InitializeAsync("{}", CancellationToken.None); + try + { + await WaitUntilAsync(() => drv.GetHostStatuses().Single().State == HostState.Running, + TimeSpan.FromSeconds(10), "the host never went Running on a refused probe tag"); + await Task.Delay(1_000); // several probe ticks — each one used to record a Forward-Open failure + + var r = await drv.ReadAsync(["Counter"], CancellationToken.None); // a cold create + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + drv.GetHostStatuses().Single().State.ShouldBe(HostState.Running); + } + finally + { + await drv.DisposeAsync(); + } + } + + /// + /// H1/L4: a tag the controller does not have reads BadNodeIdUnknown every poll while a sibling reads Good, and + /// the driver stays Healthy with LastSuccessfulRead advancing (it used to latch Degraded and, with no other + /// tag, trip the comms-loss watchdog against a reachable controller). + /// + /// The transport stack the driver runs on. + [AbServerTheory] + [MemberData(nameof(TransportMatrix.Transports), MemberType = typeof(TransportMatrix))] + public async Task A_missing_tag_is_per_tag_Bad_and_never_degrades_the_driver(AbCipTransport transport) + { + var device = Device(new AbServerFixture()); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(device, AbCipPlcFamily.ControlLogix)], + RawTags = AbCipRawTags.From( + new AbCipTagDefinition("Typo", device, "NoSuchTag_xyz", AbCipDataType.DInt), + new AbCipTagDefinition("Counter", device, "TestDINT", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions { Enabled = false }, + Timeout = TimeSpan.FromSeconds(5), + Transport = transport, + }, $"drv-h1-{transport}"); + await drv.InitializeAsync("{}", CancellationToken.None); + try + { + DateTime? last = null; + for (var i = 0; i < 3; i++) + { + var r = await drv.ReadAsync(["Typo", "Counter"], CancellationToken.None); + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadNodeIdUnknown); + r[1].StatusCode.ShouldBe(AbCipStatusMapper.Good); + var health = drv.GetHealth(); + health.State.ShouldBe(DriverState.Healthy); + if (last is { } previous) health.LastSuccessfulRead!.Value.ShouldBeGreaterThanOrEqualTo(previous); + last = health.LastSuccessfulRead; + } + + // Only the refused tag: still Healthy, still answering (the watchdog conjunction can never trip). + var before = drv.GetHealth().LastSuccessfulRead!.Value; + await Task.Delay(20); + (await drv.ReadAsync(["Typo"], CancellationToken.None))[0].StatusCode.ShouldBe(AbCipStatusMapper.BadNodeIdUnknown); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + drv.GetHealth().LastSuccessfulRead!.Value.ShouldBeGreaterThan(before); + } + finally + { + await drv.DisposeAsync(); + } + } + + /// + /// U10: a whole int[] written to the 16-element TestArray (DINT[16]) reads back element for element + /// through a fresh driver; a wrong-length write is refused BadOutOfRange and changes nothing. + /// + /// The transport stack the driver runs on. + [AbServerTheory] + [MemberData(nameof(TransportMatrix.Transports), MemberType = typeof(TransportMatrix))] + public async Task A_whole_array_write_round_trips(AbCipTransport transport) + { + IntegrationWriteGate.SkipUnlessWritesAllowed(); + var device = Device(new AbServerFixture()); + var seed = Random.Shared.Next(1_000, 1_000_000); + var values = Enumerable.Range(0, 16).Select(i => seed + i * 7).ToArray(); + + AbCipDriver Make(string id) => new(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(device, AbCipPlcFamily.ControlLogix)], + RawTags = AbCipRawTags.From( + new AbCipTagDefinition("Arr", device, "TestArray", AbCipDataType.DInt, ElementCount: 16, IsArray: true)), + Probe = new AbCipProbeOptions { Enabled = false }, + Timeout = TimeSpan.FromSeconds(5), + Transport = transport, + }, $"drv-u10-{id}-{transport}"); + + var writer = Make("writer"); + await writer.InitializeAsync("{}", CancellationToken.None); + try + { + var w = await writer.WriteAsync([new WriteRequest("Arr", values)], CancellationToken.None); + w[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + var bad = await writer.WriteAsync([new WriteRequest("Arr", values.Take(15).ToArray())], CancellationToken.None); + bad[0].StatusCode.ShouldBe(AbCipStatusMapper.BadOutOfRange); + writer.GetHealth().State.ShouldBe(DriverState.Healthy); + } + finally + { + await writer.DisposeAsync(); + } + + var reader = Make("reader"); + await reader.InitializeAsync("{}", CancellationToken.None); + try + { + var r = await reader.ReadAsync(["Arr"], CancellationToken.None); + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + r[0].Value.ShouldBe(values); + } + finally + { + await reader.DisposeAsync(); + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlmaProjectionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlmaProjectionTests.cs index c6012a88..d93616e8 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlmaProjectionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlmaProjectionTests.cs @@ -311,8 +311,9 @@ public sealed class AbCipAlmaProjectionTests // A non-zero libplctag status ⇒ Bad on every member read; the driver evicts the handle and re-creates it, so // every re-created runtime must fail too (a fresh fake would read as "all clear"). var recreations = 0; - r.Factory.Customise = p => { Interlocked.Increment(ref recreations); return new FakeAbCipTag(p) { Status = (int)AbStatus.NotFound }; }; - foreach (var t in r.Factory.Tags.Values.ToList()) t.Status = (int)AbStatus.NotFound; + // A comms status (not a device refusal, which keeps the handle since review 07 H1) so the handle is evicted. + r.Factory.Customise = p => { Interlocked.Increment(ref recreations); return new FakeAbCipTag(p) { Status = (int)AbStatus.BadConnection }; }; + foreach (var t in r.Factory.Tags.Values.ToList()) t.Status = (int)AbStatus.BadConnection; await WaitFor(() => Volatile.Read(ref recreations) >= 6); await r.StopAsync(); Snapshot(r).Any(x => x.Kind == AlarmTransitionKind.Clear).ShouldBeFalse(); @@ -334,9 +335,9 @@ public sealed class AbCipAlmaProjectionTests { if (!p.TagName.EndsWith("HAcked", StringComparison.Ordinal)) return null!; Interlocked.Increment(ref bad); - return new FakeAbCipTag(p) { Status = (int)AbStatus.NotFound }; + return new FakeAbCipTag(p) { Status = (int)AbStatus.BadConnection }; // comms ⇒ evicted + re-created }; - r.Factory.Tags[$"{Tag}.HAcked"].Status = (int)AbStatus.NotFound; + r.Factory.Tags[$"{Tag}.HAcked"].Status = (int)AbStatus.BadConnection; await WaitFor(() => Volatile.Read(ref bad) >= 4); // The link recovers and HAcked reads true again. r.Factory.Customise = p => new FakeAbCipTag(p) { Value = true }; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipArrayWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipArrayWriteTests.cs new file mode 100644 index 00000000..5ebd5908 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipArrayWriteTests.cs @@ -0,0 +1,179 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Fakes; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Review 07 U10: array writes. Config and the AdminUI allowed them and discovery advertised array nodes +/// Operate-writable, but both runtimes encoded a SCALAR: an int[] threw InvalidCastException (BadTypeMismatch +/// AND Degraded), a scalar wrote element 0 natively, and libplctag re-sent the whole last-read buffer with only +/// element 0 patched. The driver now encodes count elements at the element width, refuses a length mismatch +/// with BadOutOfRange, and converts element values (ScadaBridge writes int[] to INT arrays). +/// +[Trait("Category", "Unit")] +public sealed class AbCipArrayWriteTests +{ + private const string Device = "ab://10.0.0.5/1,0"; + + private static async Task<(AbCipDriver Driver, FakeAbCipTagFactory Factory)> BuildAsync( + AbCipDataType type, int count, int size = -1) + { + var factory = new FakeAbCipTagFactory { Customise = p => new FakeAbCipTag(p) { Size = size } }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Arr", Device, "Arr", type, ElementCount: count, IsArray: true)), + Probe = new AbCipProbeOptions { Enabled = false }, + }, "abcip-array-write", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + return (drv, factory); + } + + // ---- the codec ---- + + /// Each element is converted and laid out little-endian at the element width. + [Fact] + public void The_codec_converts_elements_and_lays_them_out_at_the_element_width() + { + AbCipArrayCodec.Encode(AbCipDataType.Int, [1, -2, 300]).ShouldBe(Convert.FromHexString("0100FEFF2C01")); + AbCipArrayCodec.Encode(AbCipDataType.SInt, [(short)-1, 5L]).ShouldBe(Convert.FromHexString("FF05")); + AbCipArrayCodec.Encode(AbCipDataType.DInt, [1.0, 2]).ShouldBe(Convert.FromHexString("0100000002000000")); + AbCipArrayCodec.Encode(AbCipDataType.Real, [1, 2.5]).ShouldBe(Convert.FromHexString("0000803F00002040")); + AbCipArrayCodec.Encode(AbCipDataType.ULInt, [ulong.MaxValue]).ShouldBe(Convert.FromHexString("FFFFFFFFFFFFFFFF")); + Should.Throw(() => AbCipArrayCodec.Encode(AbCipDataType.Int, [40000])); + Should.Throw(() => AbCipArrayCodec.Encode(AbCipDataType.Bool, [true])); + } + + // ---- the driver ---- + + /// ScadaBridge's wire shape: an int[] to an INT[3] encodes 3 two-byte elements and writes once, Good. + [Fact] + public async Task An_int_array_to_an_INT_array_encodes_every_element() + { + var (drv, factory) = await BuildAsync(AbCipDataType.Int, 3, size: 6); + + var r = await drv.WriteAsync([new WriteRequest("Arr", new[] { 1, 2, 3 })], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + var tag = factory.Tags["Arr"]; + tag.EncodedArrayType.ShouldBe(AbCipDataType.Int); + tag.EncodedArrayCount.ShouldBe(3); + tag.EncodedArrayBytes.ShouldBe(Convert.FromHexString("010002000300")); + tag.WriteCount.ShouldBe(1); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// A list of a different length is BadOutOfRange; nothing is created, encoded or written. + /// The client's element count. + [Theory] + [InlineData(2)] + [InlineData(4)] + public async Task A_length_mismatch_is_BadOutOfRange_and_nothing_is_written(int length) + { + var (drv, factory) = await BuildAsync(AbCipDataType.DInt, 3); + + var r = await drv.WriteAsync([new WriteRequest("Arr", Enumerable.Range(0, length).ToArray())], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadOutOfRange); + factory.Tags.ShouldNotContainKey("Arr"); // refused before any runtime exists + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// A scalar (or a string) to an array tag is BadTypeMismatch — it used to write element 0 only. + [Fact] + public async Task A_scalar_to_an_array_tag_is_BadTypeMismatch() + { + var (drv, factory) = await BuildAsync(AbCipDataType.DInt, 3); + + var r = await drv.WriteAsync( + [new WriteRequest("Arr", 5), new WriteRequest("Arr", "1,2,3")], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadTypeMismatch); + r[1].StatusCode.ShouldBe(AbCipStatusMapper.BadTypeMismatch); + factory.Tags.ShouldNotContainKey("Arr"); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// An element that does not fit is BadOutOfRange with no health write and nothing sent. + [Fact] + public async Task An_element_overflow_is_BadOutOfRange_and_nothing_is_sent() + { + var (drv, factory) = await BuildAsync(AbCipDataType.Int, 2, size: 4); + + var r = await drv.WriteAsync([new WriteRequest("Arr", new[] { 1, 70000 })], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadOutOfRange); + factory.Tags["Arr"].WriteCount.ShouldBe(0); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// BOOL / STRING arrays are refused BadNotSupported before any runtime exists. + /// The array element type. + [Theory] + [InlineData(AbCipDataType.Bool)] + [InlineData(AbCipDataType.String)] + public async Task Unencodable_element_types_are_BadNotSupported(AbCipDataType type) + { + var (drv, factory) = await BuildAsync(type, 2); + + var r = await drv.WriteAsync([new WriteRequest("Arr", new object[] { true, false })], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadNotSupported); + factory.Tags.ShouldNotContainKey("Arr"); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); // M5: a refusal writes no health + } + + /// The controller holding a different shape than authored (size ≠ width × count) is BadTypeMismatch, nothing sent. + [Fact] + public async Task A_controller_shape_mismatch_is_BadTypeMismatch_and_nothing_is_sent() + { + var (drv, factory) = await BuildAsync(AbCipDataType.Int, 3, size: 12); // DINT[3] on the controller + + var r = await drv.WriteAsync([new WriteRequest("Arr", new[] { 1, 2, 3 })], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadTypeMismatch); + factory.Tags["Arr"].WriteCount.ShouldBe(0); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// End to end over the REAL native runtime: an int[] to an INT[3] goes out as ONE Write Tag of type + /// 0xC3 with element count 3 and the three 2-byte elements. + /// + [Fact] + public async Task A_native_array_write_is_one_write_of_count_elements() + { + await using var target = new FakeCipTarget(); + target.ConnectedHandler = req => Task.FromResult(req[0] switch + { + 0x52 => [0xD2, 0x00, 0x00, 0x00, 0xC3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00], // INT[3] = 0,0,0 + 0x4D => [0xCD, 0x00, 0x00, 0x00], + _ => target.ServiceReply(req), + }); + var host = $"ab://127.0.0.1:{target.Port}/1,0"; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(host, AbCipPlcFamily.ControlLogix)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Arr", host, "Arr", AbCipDataType.Int, ElementCount: 3, IsArray: true)), + Probe = new AbCipProbeOptions { Enabled = false }, + Timeout = TimeSpan.FromSeconds(5), + }, "abcip-array-native", new NativeAbCipTagFactory(new DeviceSessionPool())); + await drv.InitializeAsync("{}", CancellationToken.None); + try + { + var r = await drv.WriteAsync([new WriteRequest("Arr", new[] { 1, -2, 300 })], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + var write = target.ConnectedCips.Single(c => c[0] == 0x4D); + var typeAt = 2 + write[1] * 2; + write.AsSpan(typeAt).ToArray().ShouldBe(Convert.FromHexString("C300" + "0300" + "0100FEFF2C01")); + } + finally + { + await drv.DisposeAsync(); + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipBitInWordWidthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipBitInWordWidthTests.cs new file mode 100644 index 00000000..dce0c47d --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipBitInWordWidthTests.cs @@ -0,0 +1,161 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Fakes; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Review 07 H17: Logix allows a .n bit selector on SINT / INT / DINT (and LINT) hosts, and reads of +/// Flags.3 on an INT worked — but the bit write's read-modify-write hard-coded a DINT parent, so its +/// DecodeValue(DInt) on a 2-byte buffer threw OutOfBounds and the write was reported as a comms fault +/// (BadCommunicationError + Degraded). The parent width now comes from the parent read's size. +/// +[Trait("Category", "Unit")] +public sealed class AbCipBitInWordWidthTests +{ + private const string Device = "ab://10.0.0.5/1,0"; + + private static async Task<(AbCipDriver Driver, FakeAbCipTagFactory Factory)> BuildAsync(int parentSize, object parentValue, string bitPath) + { + var factory = new FakeAbCipTagFactory + { + Customise = p => new FakeAbCipTag(p) { Value = parentValue, Size = parentSize }, + }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Bit", Device, bitPath, AbCipDataType.Bool)), + Probe = new AbCipProbeOptions { Enabled = false }, + }, "abcip-bit", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + return (drv, factory); + } + + /// SINT / INT / DINT hosts: the bit is set, every other bit is preserved, nothing degrades. + /// The parent's wire width. + /// The parent's value before the write. + /// The bit written true. + /// The value the parent must be written back with. + [Theory] + [InlineData(1, 0x81, 3, 0x89)] // SINT 0x81 (-127): bit 7 set stays set, written as its unsigned byte + [InlineData(1, 0x00, 7, 0x80)] // SINT top bit: 128 would overflow an sbyte encode + [InlineData(2, 0x8001, 3, 0x8009)] // INT top bit preserved + [InlineData(2, 0x0000, 15, 0x8000)] // INT top bit: 32768 would overflow a short encode + [InlineData(4, 0x00000001, 3, 0x00000009)] + public async Task A_bit_write_uses_the_parent_width(int size, int before, int bit, int after) + { + var (drv, factory) = await BuildAsync(size, before, $"Motor.Flags.{bit}"); + + var r = await drv.WriteAsync([new WriteRequest("Bit", true)], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + Convert.ToInt64(factory.Tags["Motor.Flags"].Value).ShouldBe(after); + factory.Tags["Motor.Flags"].WriteCount.ShouldBe(1); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// A bit past the host's width is refused before anything is written, with no health write. + [Fact] + public async Task A_bit_past_the_host_width_is_BadOutOfRange_and_nothing_is_written() + { + var (drv, factory) = await BuildAsync(2, 0, "Motor.Flags.20"); + + var r = await drv.WriteAsync([new WriteRequest("Bit", true)], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadOutOfRange); + factory.Tags["Motor.Flags"].WriteCount.ShouldBe(0); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// A host whose width is no integer's is a type mismatch, refused before writing, no health write. + [Fact] + public async Task A_host_of_no_integer_width_is_BadTypeMismatch() + { + var (drv, factory) = await BuildAsync(3, 0, "Motor.Flags.1"); + + var r = await drv.WriteAsync([new WriteRequest("Bit", true)], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadTypeMismatch); + factory.Tags["Motor.Flags"].WriteCount.ShouldBe(0); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// End to end over the REAL native runtime against a fake CIP target whose Flags is an INT: the write + /// goes out as ONE INT (type 0xC3, two data bytes) with the bit set and the other bits preserved. + /// + [Fact] + public async Task A_native_bit_write_on_an_INT_host_writes_an_INT() + { + await using var target = new FakeCipTarget(); + // Read Tag Fragmented (0x52) → INT (0x00C3) 0x8001; Write Tag (0x4D) → success. + target.ConnectedHandler = req => Task.FromResult(req[0] switch + { + 0x52 => [0xD2, 0x00, 0x00, 0x00, 0xC3, 0x00, 0x01, 0x80], + 0x4D => [0xCD, 0x00, 0x00, 0x00], + _ => target.ServiceReply(req), + }); + var host = $"ab://127.0.0.1:{target.Port}/1,0"; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(host, AbCipPlcFamily.ControlLogix)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Bit", host, "Flags.3", AbCipDataType.Bool)), + Probe = new AbCipProbeOptions { Enabled = false }, + Timeout = TimeSpan.FromSeconds(5), + }, "abcip-bit-native", new NativeAbCipTagFactory(new DeviceSessionPool())); + await drv.InitializeAsync("{}", CancellationToken.None); + try + { + var r = await drv.WriteAsync([new WriteRequest("Bit", true)], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + var write = target.ConnectedCips.Single(c => c[0] == 0x4D); + var typeAt = 2 + write[1] * 2; + write[typeAt].ShouldBe((byte)0xC3); // INT, not DINT + write.Length.ShouldBe(typeAt + 2 + 2 + 2); // type, element count, ONE 2-byte element + write[^2].ShouldBe((byte)0x09); // 0x8001 | bit 3 + write[^1].ShouldBe((byte)0x80); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + finally + { + await drv.DisposeAsync(); + } + } + + /// + /// M1: a parent read that fails with a COMMS status evicts the parent runtime and degrades (LastSuccessfulRead + /// preserved); one the controller REFUSES is per-tag Bad, keeps the runtime and writes no health. + /// + [Fact] + public async Task A_failed_parent_read_follows_the_health_contract() + { + var (drv, factory) = await BuildAsync(4, 0, "Motor.Flags.3"); + factory.Customise = p => new FakeAbCipTag(p) { Status = (int)AbStatus.BadConnection }; + var before = drv.GetHealth(); + + var r = await drv.WriteAsync([new WriteRequest("Bit", true)], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadCommunicationError); + factory.Tags["Motor.Flags"].Disposed.ShouldBeTrue(); + factory.Tags["Motor.Flags"].WriteCount.ShouldBe(0); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + drv.GetHealth().LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + } + + /// M1 positive control: a refused parent read keeps the runtime, no health write. + [Fact] + public async Task A_refused_parent_read_keeps_the_runtime_and_health() + { + var (drv, factory) = await BuildAsync(4, 0, "Motor.Flags.3"); + factory.Customise = p => new FakeAbCipTag(p) { Status = (int)AbStatus.NotAllowed }; + + var r = await drv.WriteAsync([new WriteRequest("Bit", true)], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadNotWritable); + factory.Tags["Motor.Flags"].Disposed.ShouldBeFalse(); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDeviceRefusalTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDeviceRefusalTests.cs new file mode 100644 index 00000000..e4e23d7c --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDeviceRefusalTests.cs @@ -0,0 +1,298 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Fakes; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Review 07 H1 / L4: a status that proves the controller ANSWERED (NotFound, OutOfBounds, NotAllowed, +/// TooLarge, TooSmall, TypeMismatch, Unsupported) is a deterministic per-tag refusal, not a comms fault. It +/// keeps the tag runtime (no evict → no lease / Forward-Open churn), writes no Degraded, opens no device-wide +/// backoff window for siblings, warns once, and — since the device answered — advances LastSuccessfulRead. +/// +[Trait("Category", "Unit")] +public sealed class AbCipDeviceRefusalTests +{ + private const string Device = "ab://10.0.0.5/1,0"; + + /// A fake whose reads/writes throw a chosen transport status (init succeeds unless told otherwise). + private sealed class RefusingTag(AbCipTagCreateParams p) : FakeAbCipTag(p) + { + public AbStatus? ReadThrows { get; set; } + public AbStatus? WriteThrows { get; set; } + + public override async Task ReadAsync(CancellationToken cancellationToken) + { + await base.ReadAsync(cancellationToken); + if (ReadThrows is { } s) throw new AbTransportException(s, s.ToString()); + } + + public override async Task WriteAsync(CancellationToken cancellationToken) + { + await base.WriteAsync(cancellationToken); + if (WriteThrows is { } s) throw new AbTransportException(s, s.ToString()); + } + } + + private static (AbCipDriver Driver, FakeAbCipTagFactory Factory) Build( + Func make) + { + var factory = new FakeAbCipTagFactory { Customise = make }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device)], + RawTags = AbCipRawTags.From( + new AbCipTagDefinition("Bad", Device, "Arr[999]", AbCipDataType.DInt), + new AbCipTagDefinition("Good", Device, "Real1", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions { Enabled = false }, + MaxConcurrentReads = 1, + }, "abcip-refusal", factory); + return (drv, factory); + } + + /// The classification itself: every answered refusal is reachable; transport failures are not. + /// The transport status. + /// Whether it is a device refusal. + [Theory] + [InlineData(AbStatus.NotFound, true)] + [InlineData(AbStatus.OutOfBounds, true)] + [InlineData(AbStatus.NotAllowed, true)] + [InlineData(AbStatus.TooLarge, true)] + [InlineData(AbStatus.TooSmall, true)] + [InlineData(AbStatus.TypeMismatch, true)] + [InlineData(AbStatus.Unsupported, true)] + [InlineData(AbStatus.Ok, false)] + [InlineData(AbStatus.Timeout, false)] + [InlineData(AbStatus.BadConnection, false)] + [InlineData(AbStatus.BadReply, false)] + [InlineData(AbStatus.Partial, false)] + [InlineData(AbStatus.Abort, false)] + public void Answered_statuses_are_device_refusals(AbStatus status, bool refusal) + { + AbCipRefusals.IsDeviceRefusal(status).ShouldBe(refusal); + AbCipDriverProbe.IsReachableStatus(status).ShouldBe(refusal || status == AbStatus.Ok); + } + + /// A read the controller refuses keeps its runtime, stays Healthy, and advances LastSuccessfulRead. + /// The refusal the controller answers with. + /// The OPC UA status the tag reads. + [Theory] + [InlineData(AbStatus.NotFound, AbCipStatusMapper.BadNodeIdUnknown)] + [InlineData(AbStatus.OutOfBounds, AbCipStatusMapper.BadOutOfRange)] + [InlineData(AbStatus.TooLarge, AbCipStatusMapper.BadOutOfRange)] + [InlineData(AbStatus.TooSmall, AbCipStatusMapper.BadOutOfRange)] + [InlineData(AbStatus.NotAllowed, AbCipStatusMapper.BadNotReadable)] + public async Task A_refused_read_keeps_the_runtime_and_writes_no_Degraded(AbStatus status, uint expected) + { + var (drv, factory) = Build(p => new RefusingTag(p) { ReadThrows = p.TagName == "Arr[999]" ? status : null, Value = 1 }); + await drv.InitializeAsync("{}", CancellationToken.None); + var before = drv.GetHealth().LastSuccessfulRead!.Value; + await Task.Delay(20); + + for (var i = 0; i < 3; i++) + { + var r = await drv.ReadAsync(["Bad"], CancellationToken.None); + r[0].StatusCode.ShouldBe(expected); + } + + var bad = factory.Tags["Arr[999]"]; + bad.InitializeCount.ShouldBe(1); // never evicted → never re-created + bad.Disposed.ShouldBeFalse(); + bad.ReadCount.ShouldBe(3); + var health = drv.GetHealth(); + health.State.ShouldBe(DriverState.Healthy); + health.LastSuccessfulRead!.Value.ShouldBeGreaterThan(before); // the device answered + } + + /// Positive control: a genuine transport failure on the same tag still evicts and degrades. + [Fact] + public async Task A_transport_failure_still_evicts_and_degrades() + { + var (drv, factory) = Build(p => new RefusingTag(p) { ReadThrows = p.TagName == "Arr[999]" ? AbStatus.Timeout : null }); + await drv.InitializeAsync("{}", CancellationToken.None); + + var r = await drv.ReadAsync(["Bad"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadTimeout); + factory.Tags["Arr[999]"].Disposed.ShouldBeTrue(); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + } + + /// A refusal returned as a status word (no throw) is classified the same way. + [Fact] + public async Task A_refusal_status_word_keeps_the_runtime() + { + var (drv, factory) = Build(p => new FakeAbCipTag(p) { Status = p.TagName == "Arr[999]" ? (int)AbStatus.OutOfBounds : 0, Value = 1 }); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.ReadAsync(["Bad"], CancellationToken.None); + var r = await drv.ReadAsync(["Bad", "Good"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadOutOfRange); + r[1].StatusCode.ShouldBe(AbCipStatusMapper.Good); + factory.Tags["Arr[999]"].InitializeCount.ShouldBe(1); + factory.Tags["Arr[999]"].Disposed.ShouldBeFalse(); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// A cold create the controller refuses with OutOfBounds proves the session works: it must not open the + /// device-wide backoff window, so a sibling cold tag still connects and reads Good. + /// + [Fact] + public async Task A_refused_create_does_not_throttle_sibling_cold_creates() + { + var (drv, _) = Build(p => p.TagName == "Arr[999]" + ? new FakeAbCipTag(p) { ThrowOnInitialize = true, Exception = new AbTransportException(AbStatus.OutOfBounds, "OutOfBounds") } + : new FakeAbCipTag(p) { Value = 42 }); + await drv.InitializeAsync("{}", CancellationToken.None); + + var r = await drv.ReadAsync(["Bad", "Good"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadOutOfRange); + r[1].StatusCode.ShouldBe(AbCipStatusMapper.Good); + r[1].Value.ShouldBe(42); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// A write the controller refuses keeps the runtime and writes no health. + [Fact] + public async Task A_refused_write_keeps_the_runtime_and_writes_no_Degraded() + { + var factory = new FakeAbCipTagFactory { Customise = p => new RefusingTag(p) { WriteThrows = AbStatus.NotAllowed } }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Ro", Device, "ReadOnlyTag", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions { Enabled = false }, + }, "abcip-refusal", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var w1 = await drv.WriteAsync([new WriteRequest("Ro", 5)], CancellationToken.None); + var w2 = await drv.WriteAsync([new WriteRequest("Ro", 6)], CancellationToken.None); + + w1[0].StatusCode.ShouldBe(AbCipStatusMapper.BadNotWritable); + w2[0].StatusCode.ShouldBe(AbCipStatusMapper.BadNotWritable); + factory.Tags["ReadOnlyTag"].InitializeCount.ShouldBe(1); + factory.Tags["ReadOnlyTag"].Disposed.ShouldBeFalse(); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// L4 end to end over the REAL native runtime + a private pool: a renamed tag (CIP 0x04) that is the device's + /// ONLY lease holder used to be evicted every poll — the last release, so Forward Close, then a throttled Forward + /// Open on the next poll. One runtime and one Forward Open across N polls and no Forward Close proves no churn. + /// + [Fact] + public async Task A_native_NotFound_tag_causes_no_runtime_or_connection_churn() + { + await using var target = new FakeCipTarget(); + target.ConnectedHandler = req => Task.FromResult( + req[0] == 0x52 ? [0xD2, 0x00, 0x04, 0x00] : target.ServiceReply(req)); + var host = $"ab://127.0.0.1:{target.Port}/1,0"; + var creates = 0; + var native = new NativeAbCipTagFactory(new DeviceSessionPool()); + var factory = new CountingFactory(p => { Interlocked.Increment(ref creates); return native.Create(p); }); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(host, AbCipPlcFamily.ControlLogix)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Renamed", host, "Renamed", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions { Enabled = false }, + Timeout = TimeSpan.FromSeconds(5), + }, "abcip-l4-native", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + try + { + for (var n = 0; n < 5; n++) + (await drv.ReadAsync(["Renamed"], CancellationToken.None))[0].StatusCode + .ShouldBe(AbCipStatusMapper.BadNodeIdUnknown); + + Volatile.Read(ref creates).ShouldBe(1); + target.ForwardOpens.ShouldBe(1); + target.ForwardCloses.ShouldBe(0); + target.ConnectedRequests.ShouldBeGreaterThanOrEqualTo(5); // positive control: every poll hit the wire + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + finally + { + await drv.DisposeAsync(); + } + } + + private sealed class CountingFactory(Func create) : IAbCipTagFactory + { + public IAbCipTagRuntime Create(AbCipTagCreateParams createParams) => create(createParams); + } + + /// + /// #737 I2: a LOCAL refusal (libplctag ErrorNotImplemented — no I/O) is per-tag BadNotSupported, keeps health + /// state, and does NOT advance LastSuccessfulRead (nothing proves the device answered). Positive control: the + /// device's own Unsupported refusal does advance it (M2). + /// + [Fact] + public async Task A_local_refusal_does_not_advance_LastSuccessfulRead_but_a_device_one_does() + { + var (drv, _) = Build(p => p.TagName == "Arr[999]" + ? new LocalRefusingTag(p) + : new RefusingTag(p) { ReadThrows = AbStatus.Unsupported }); + await drv.InitializeAsync("{}", CancellationToken.None); + var before = drv.GetHealth().LastSuccessfulRead!.Value; + await Task.Delay(20); + + (await drv.ReadAsync(["Bad"], CancellationToken.None))[0].StatusCode.ShouldBe(AbCipStatusMapper.BadNotSupported); + drv.GetHealth().LastSuccessfulRead.ShouldBe(before); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + + (await drv.ReadAsync(["Good"], CancellationToken.None))[0].StatusCode.ShouldBe(AbCipStatusMapper.BadNotSupported); + drv.GetHealth().LastSuccessfulRead!.Value.ShouldBeGreaterThan(before); + } + + /// #737 I2: a local refusal at create opens no backoff — a sibling cold create still connects. + [Fact] + public async Task A_local_refusal_at_create_does_not_throttle_or_count_as_answered() + { + var (drv, _) = Build(p => p.TagName == "Arr[999]" + ? new FakeAbCipTag(p) { ThrowOnInitialize = true, Exception = new AbCipLocalRefusalException("ErrorNotImplemented") } + : new FakeAbCipTag(p) { Value = 3 }); + await drv.InitializeAsync("{}", CancellationToken.None); + + var r = await drv.ReadAsync(["Bad", "Good"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadNotSupported); + r[1].StatusCode.ShouldBe(AbCipStatusMapper.Good); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// M2: a write the device refuses proves it answered — LastSuccessfulRead advances. + [Fact] + public async Task A_refused_write_advances_LastSuccessfulRead() + { + var factory = new FakeAbCipTagFactory { Customise = p => new RefusingTag(p) { WriteThrows = AbStatus.NotAllowed } }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Ro", Device, "ReadOnlyTag", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions { Enabled = false }, + }, "abcip-refusal", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + var before = drv.GetHealth().LastSuccessfulRead!.Value; + await Task.Delay(20); + + (await drv.WriteAsync([new WriteRequest("Ro", 5)], CancellationToken.None))[0].StatusCode + .ShouldBe(AbCipStatusMapper.BadNotWritable); + + drv.GetHealth().LastSuccessfulRead!.Value.ShouldBeGreaterThan(before); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + private sealed class LocalRefusingTag(AbCipTagCreateParams p) : FakeAbCipTag(p) + { + public override async Task ReadAsync(CancellationToken cancellationToken) + { + await base.ReadAsync(cancellationToken); + throw new AbCipLocalRefusalException("ErrorNotImplemented"); + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLifecycleConcurrencyTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLifecycleConcurrencyTests.cs new file mode 100644 index 00000000..6811afcc --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLifecycleConcurrencyTests.cs @@ -0,0 +1,239 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Lifecycle overlap the runtime can produce (review 07 H13/F10, Lane A): the final stop runs +/// ShutdownAsync then DisposeAsync under one bounded budget, so a slow shutdown is abandoned and +/// the dispose runs CONCURRENTLY with it; an abandoned InitializeAsync can overlap the next attempt's +/// shutdown. Device tables are build-and-swapped, so each teardown owns a disjoint set of device states. +/// +[Trait("Category", "Unit")] +public sealed class AbCipLifecycleConcurrencyTests +{ + private const string Host = "ab://10.0.0.5/1,0"; + + /// A fake whose reads ignore cancellation for a while, so a teardown waits on the probe loop. + private sealed class SlowTag(AbCipTagCreateParams p, TimeSpan readDelay) : FakeAbCipTag(p) + { + private int _disposeCount; + + public int DisposeCount => Volatile.Read(ref _disposeCount); + + public override async Task ReadAsync(CancellationToken cancellationToken) + { + await base.ReadAsync(cancellationToken); + if (readDelay > TimeSpan.Zero) await Task.Delay(readDelay, CancellationToken.None); + } + + public override void Dispose() + { + Interlocked.Increment(ref _disposeCount); + base.Dispose(); + } + } + + private static (AbCipDriver Driver, FakeAbCipTagFactory Factory) Build(TimeSpan probeReadDelay) + { + var factory = new FakeAbCipTagFactory + { + Customise = p => new SlowTag(p, p.TagName == "ProbeTag" ? probeReadDelay : TimeSpan.Zero) { Value = 1 }, + }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Host)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Speed", Host, "Motor1.Speed", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions + { + Enabled = true, + ProbeTagPath = "ProbeTag", + Interval = TimeSpan.FromMilliseconds(10), + Timeout = TimeSpan.FromMilliseconds(200), + }, + }, "abcip-lifecycle", factory); + return (drv, factory); + } + + private static async Task WaitUntilAsync(Func condition, string what, int seconds = 5) + { + var deadline = DateTime.UtcNow.AddSeconds(seconds); + while (!condition()) + { + if (DateTime.UtcNow > deadline) throw new TimeoutException(what); + await Task.Delay(5); + } + } + + /// + /// Shutdown and Dispose racing each other tear the device down exactly once: the data runtime is + /// disposed once (not once per teardown), neither call throws, and the probe loop has exited. + /// + [Fact] + public async Task Dispose_concurrent_with_a_running_Shutdown_tears_each_state_down_exactly_once() + { + var (drv, factory) = Build(probeReadDelay: TimeSpan.FromMilliseconds(300)); + await drv.InitializeAsync("{}", CancellationToken.None); + (await drv.ReadAsync(["Speed"], CancellationToken.None))[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + await WaitUntilAsync(() => factory.Tags.ContainsKey("ProbeTag"), "probe never started"); + var state = drv.GetDeviceState(Host)!; + var probeLoop = state.ProbeTask!; + var data = (SlowTag)factory.Tags["Motor1.Speed"]; + + var shutdown = drv.ShutdownAsync(CancellationToken.None); + var dispose = drv.DisposeAsync().AsTask(); + await Task.WhenAll(shutdown, dispose); + + data.DisposeCount.ShouldBe(1); + probeLoop.IsCompleted.ShouldBeTrue(); + drv.GetDeviceState(Host).ShouldBeNull(); + } + + /// + /// An InitializeAsync that a ShutdownAsync overtakes publishes nothing: no device state and no probe + /// loop survive the shutdown. Positive control: an init that nothing overtakes still publishes. + /// + [Fact] + public async Task An_init_overtaken_by_Shutdown_publishes_no_device_and_starts_no_probe() + { + var (drv, factory) = Build(probeReadDelay: TimeSpan.FromMilliseconds(300)); + await drv.InitializeAsync("{}", CancellationToken.None); + await WaitUntilAsync(() => factory.Tags.ContainsKey("ProbeTag"), "probe never started"); + var firstProbe = factory.Tags["ProbeTag"]; + + // The second init's own release waits on the slow first probe loop; the shutdown overtakes it there. + var init = drv.InitializeAsync("{}", CancellationToken.None); + await Task.Delay(50); + await drv.ShutdownAsync(CancellationToken.None); + var superseded = await Should.ThrowAsync(() => init); + superseded.ShouldNotBeAssignableTo(); // M4: distinct from a caller cancellation + + drv.GetDeviceState(Host).ShouldBeNull(); + drv.GetHostStatuses().ShouldBeEmpty(); + await Task.Delay(100); + factory.Tags["ProbeTag"].ShouldBeSameAs(firstProbe); // no second probe loop ever created a runtime + drv.GetHealth().State.ShouldNotBe(DriverState.Healthy); + + // Positive control: a later init that nothing overtakes publishes the device again. + await drv.InitializeAsync("{}", CancellationToken.None); + drv.GetDeviceState(Host).ShouldNotBeNull(); + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + await drv.DisposeAsync(); + } + + /// + /// Readers of the device/routing tables (the retained poll engine, the actor's host-status pull) never + /// see a table mid-mutation while the driver re-initialises underneath them, never trip over a per-tag lock a + /// retire disposed, and never leak a runtime they cold-created into a device state a retire already tore down. + /// + [Fact] + public async Task Readers_survive_repeated_reinitialisation_and_leak_no_runtime() + { + var created = new System.Collections.Concurrent.ConcurrentBag(); + var factory = new FakeAbCipTagFactory + { + Customise = p => + { + var t = new FakeAbCipTag(p) { Value = 1 }; + created.Add(t); + return t; + }, + }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Host)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Speed", Host, "Motor1.Speed", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions { Enabled = false }, + }, "abcip-lifecycle", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + using var stop = new CancellationTokenSource(); + var started = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var badHealth = 0; + var reader = Task.Run(async () => + { + var reads = 0; + while (!stop.IsCancellationRequested) + { + _ = drv.GetHostStatuses(); + _ = drv.ResolveHost("Speed"); + await drv.ReadAsync(["Speed"], CancellationToken.None); + if (drv.GetHealth().State == DriverState.Degraded) Interlocked.Increment(ref badHealth); + reads++; + started.TrySetResult(); + } + return reads; + }); + await started.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + for (var i = 0; i < 200; i++) + await drv.InitializeAsync("{}", CancellationToken.None); + stop.Cancel(); + + (await reader).ShouldBeGreaterThan(0); // positive control: the reader actually ran + Volatile.Read(ref badHealth).ShouldBe(0, "a re-init is not a comms fault"); + await drv.DisposeAsync(); + created.Where(t => !t.Disposed).ShouldBeEmpty("every runtime a reader created must be disposed by some teardown"); + } + + /// F10: DisposeAsync is the complete, idempotent teardown (the runtime prefers it). + [Fact] + public async Task DisposeAsync_is_a_complete_idempotent_teardown() + { + var (drv, factory) = Build(probeReadDelay: TimeSpan.Zero); + await drv.InitializeAsync("{}", CancellationToken.None); + await WaitUntilAsync(() => factory.Tags.ContainsKey("ProbeTag"), "probe never started"); + var loop = drv.GetDeviceState(Host)!.ProbeTask!; + + await drv.DisposeAsync(); + await drv.DisposeAsync(); // idempotent + + loop.IsCompleted.ShouldBeTrue(); + factory.Tags["ProbeTag"].Disposed.ShouldBeTrue(); + } + + /// + /// F10: the sync Dispose() no longer blocks the caller's thread on the async teardown (sync-over-async — + /// a dispatcher thread for up to the probe join). It starts the same teardown and returns; the teardown still + /// completes. + /// + [Fact] + public async Task Sync_Dispose_does_not_block_on_the_teardown_but_still_completes_it() + { + var (drv, factory) = Build(probeReadDelay: TimeSpan.FromSeconds(4)); + await drv.InitializeAsync("{}", CancellationToken.None); + await WaitUntilAsync(() => factory.Tags.ContainsKey("ProbeTag"), "probe never started"); + var loop = drv.GetDeviceState(Host)!.ProbeTask!; + await Task.Delay(50); // the probe is inside its slow, cancellation-deaf read + + var sw = System.Diagnostics.Stopwatch.StartNew(); + ((IDisposable)drv).Dispose(); + sw.Stop(); + + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(2)); // did not wait out the 4 s probe read (CI-tolerant margin) + await WaitUntilAsync(() => loop.IsCompleted && factory.Tags["ProbeTag"].Disposed, "teardown never completed", seconds: 15); + drv.GetDeviceState(Host).ShouldBeNull(); + } + + /// + /// M4: a ShutdownAsync overtaken by a NEWER InitializeAsync (its probe join is slow) must not stamp Unknown over + /// the newer init's Healthy when it finally finishes. + /// + [Fact] + public async Task A_slow_shutdown_does_not_overwrite_a_newer_inits_health() + { + var (drv, factory) = Build(probeReadDelay: TimeSpan.FromMilliseconds(400)); + await drv.InitializeAsync("{}", CancellationToken.None); + await WaitUntilAsync(() => factory.Tags.ContainsKey("ProbeTag"), "probe never started"); + await Task.Delay(30); // inside the slow probe read + + var shutdown = drv.ShutdownAsync(CancellationToken.None); + await Task.Delay(30); + await drv.InitializeAsync("{}", CancellationToken.None); // newer: publishes while the shutdown still joins + await shutdown; + + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + drv.GetDeviceState(Host).ShouldNotBeNull(); + await drv.DisposeAsync(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLocalRefusalTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLocalRefusalTests.cs index f51bfe18..cb99093e 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLocalRefusalTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLocalRefusalTests.cs @@ -7,7 +7,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; /// /// Task 22 review r1: a runtime that refuses an operation with (the native -/// transport does this before the wire, e.g. a write it cannot type) reaches the client as BadNotSupported — +/// transport does this before the wire, e.g. a write it cannot type — an +/// since #737 I2) reaches the client as BadNotSupported — /// NOT as a comms fault. The handle is kept (one create, never disposed) and driver health is untouched. /// [Trait("Category", "Unit")] @@ -19,12 +20,12 @@ public sealed class AbCipLocalRefusalTests { public override Task ReadAsync(CancellationToken cancellationToken) => refuseRead - ? Task.FromException(new AbTransportException(AbStatus.Unsupported, "refused read")) + ? Task.FromException(new AbCipLocalRefusalException("refused read")) : base.ReadAsync(cancellationToken); public override Task WriteAsync(CancellationToken cancellationToken) => refuseWrite - ? Task.FromException(new AbTransportException(AbStatus.Unsupported, "refused write")) + ? Task.FromException(new AbCipLocalRefusalException("refused write")) : base.WriteAsync(cancellationToken); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipProbeRefusalTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipProbeRefusalTests.cs new file mode 100644 index 00000000..c0693b8b --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipProbeRefusalTests.cs @@ -0,0 +1,86 @@ +using Microsoft.Extensions.Logging; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; +using CapturingLogger = ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests.AbCipLoggingTests.CapturingLogger; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Review 07 H10: a typo'd (or download-deleted) Probe.ProbeTagPath used to record a Forward-Open +/// FAILURE every probe tick, so the device's backoff window never closed and every cold / evicted data tag read +/// BadCommunicationError forever — a permanent device blackout that a restart did not cure. The controller +/// ANSWERED, so the probe now counts it as reachable: backoff success, host Running, one config Warning. +/// +[Trait("Category", "Unit")] +public sealed class AbCipProbeRefusalTests +{ + private const string Device = "ab://10.0.0.5/1,0"; + + private static (AbCipDriver Driver, CapturingLogger Logger) Build(Exception probeFault) + { + var logger = new CapturingLogger(); + var factory = new FakeAbCipTagFactory + { + Customise = p => p.TagName == "ProbeTagTypo" + ? new FakeAbCipTag(p) { ThrowOnInitialize = true, Exception = probeFault } + : new FakeAbCipTag(p) { Value = 7 }, + }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Speed", Device, "Motor1.Speed", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions + { + Enabled = true, + ProbeTagPath = "ProbeTagTypo", + Interval = TimeSpan.FromMilliseconds(10), + Timeout = TimeSpan.FromMilliseconds(100), + }, + }, "abcip-probe-refusal", factory, logger: logger); + return (drv, logger); + } + + private static async Task WaitUntilAsync(Func condition) + { + var deadline = DateTime.UtcNow.AddSeconds(5); + while (!condition()) + { + if (DateTime.UtcNow > deadline) throw new TimeoutException("condition never met"); + await Task.Delay(5); + } + } + + /// A NotFound probe tag leaves the device reachable: data tags still connect, host is Running, warned once. + [Fact] + public async Task A_missing_probe_tag_does_not_black_out_the_device() + { + var (drv, logger) = Build(new AbTransportException(AbStatus.NotFound, "NotFound")); + await drv.InitializeAsync("{}", CancellationToken.None); + await WaitUntilAsync(() => drv.GetHostStatuses().Single().State == HostState.Running); + await Task.Delay(100); // many probe ticks + + var r = await drv.ReadAsync(["Speed"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); // a cold create, NOT throttled + r[0].Value.ShouldBe(7); + drv.GetHostStatuses().Single().State.ShouldBe(HostState.Running); + logger.Entries.Count(e => e.Level == LogLevel.Warning && e.Message.Contains("ProbeTagTypo")).ShouldBe(1); + await drv.DisposeAsync(); + } + + /// Positive control: a probe that genuinely cannot connect still stops the host and throttles cold creates. + [Fact] + public async Task An_unreachable_device_still_stops_the_host_and_throttles() + { + var (drv, _) = Build(new AbTransportException(AbStatus.Timeout, "Timeout")); + await drv.InitializeAsync("{}", CancellationToken.None); + await WaitUntilAsync(() => drv.GetHostStatuses().Single().State == HostState.Stopped); + + var r = await drv.ReadAsync(["Speed"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadCommunicationError); // throttled by the open window + await drv.DisposeAsync(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipReadWindowTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipReadWindowTests.cs new file mode 100644 index 00000000..45cd3489 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipReadWindowTests.cs @@ -0,0 +1,96 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Review 07 P8 / P7: the bounded-parallel read window. On the native transport every concurrent tag read of a +/// device rides ONE shared request scheduler that packs whatever is queued into a Multiple Service Packet, so a +/// window of 8 capped MSP coalescing at 8 services per round trip; it is widened to at least 64 there (the +/// scheduler's reply budget already prevents over-packing). And a cold create waiting on a device's connect gate +/// must not occupy a window slot, or one dead device's backoff-expiry attempt stalls every other device's reads. +/// +[Trait("Category", "Unit")] +public sealed class AbCipReadWindowTests +{ + private static AbCipDriverOptions Options(int maxConcurrentReads, AbCipTransport? transport = null) => new() + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Probe = new AbCipProbeOptions { Enabled = false }, + MaxConcurrentReads = maxConcurrentReads, + Transport = transport, + }; + + /// The window per transport: native ≥ 64; libplctag and injected factories honour the option exactly. + [Fact] + public void The_native_window_is_at_least_64_and_other_transports_keep_the_option() + { + new AbCipDriver(Options(8, AbCipTransport.Native), "a").ReadWindow.ShouldBe(AbCipDriver.NativeMinReadWindow); + AbCipDriver.NativeMinReadWindow.ShouldBeGreaterThanOrEqualTo(64); + new AbCipDriver(Options(1, AbCipTransport.Native), "a").ReadWindow.ShouldBe(AbCipDriver.NativeMinReadWindow); + new AbCipDriver(Options(200, AbCipTransport.Native), "a").ReadWindow.ShouldBe(200); + new AbCipDriver(Options(8), "a", defaultTransport: AbCipTransport.Native).ReadWindow.ShouldBe(AbCipDriver.NativeMinReadWindow); + + // Positive controls: libplctag (per-handle concurrency) and an injected test factory keep the option. + new AbCipDriver(Options(8, AbCipTransport.Libplctag), "a").ReadWindow.ShouldBe(8); + new AbCipDriver(Options(0, AbCipTransport.Libplctag), "a").ReadWindow.ShouldBe(1); + new AbCipDriver(Options(3, AbCipTransport.Native), "a", new FakeAbCipTagFactory()).ReadWindow.ShouldBe(3); + } + + /// A tag whose initialisation blocks until released (a cold connect to a slow / dead device). + private sealed class BlockingInitTag(AbCipTagCreateParams p, Task release) : FakeAbCipTag(p) + { + public override async Task InitializeAsync(CancellationToken cancellationToken) + { + await base.InitializeAsync(cancellationToken); + await release.WaitAsync(cancellationToken); + } + } + + /// + /// With a window of ONE, device B's blocked cold create (and its queued sibling) must not stop device A's tag + /// from being read: cold waiters sit on B's connect gate, not in the window. + /// + [Fact] + public async Task A_blocked_cold_create_on_one_device_does_not_hold_the_window_against_another() + { + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var factory = new FakeAbCipTagFactory + { + Customise = p => p.Gateway == "10.0.0.6" + ? new BlockingInitTag(p, release.Task) { Value = 2 } + : new FakeAbCipTag(p) { Value = 1 }, + }; + const string a = "ab://10.0.0.5/1,0", b = "ab://10.0.0.6/1,0"; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(a), new AbCipDeviceOptions(b)], + RawTags = AbCipRawTags.From( + new AbCipTagDefinition("B1", b, "B1", AbCipDataType.DInt), + new AbCipTagDefinition("B2", b, "B2", AbCipDataType.DInt), + new AbCipTagDefinition("A1", a, "A1", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions { Enabled = false }, + MaxConcurrentReads = 1, + }, "abcip-window", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var read = drv.ReadAsync(["B1", "B2", "A1"], CancellationToken.None); + var deadline = DateTime.UtcNow.AddSeconds(5); + while (!(factory.Tags.TryGetValue("A1", out var tag) && tag.ReadCount > 0)) + { + if (DateTime.UtcNow > deadline) + { + release.TrySetResult(); + await read; + throw new ShouldAssertException("device A's read waited behind device B's blocked cold create"); + } + await Task.Delay(5); + } + read.IsCompleted.ShouldBeFalse(); // B is still blocked — A was read regardless + + release.SetResult(); + var r = await read; + r.Select(x => x.Value).ShouldBe([2, 2, 1]); // positive control: B's reads complete once it connects + await drv.DisposeAsync(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipStatusMapperTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipStatusMapperTests.cs index 7c3c63df..778f2c42 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipStatusMapperTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipStatusMapperTests.cs @@ -77,7 +77,7 @@ public sealed class AbCipStatusMapperTests Status.Ok => AbCipStatusMapper.Good, Status.Pending => AbCipStatusMapper.GoodMoreData, Status.ErrorTimeout => AbCipStatusMapper.BadTimeout, - Status.ErrorNotFound or Status.ErrorNoMatch or Status.ErrorBadDevice => AbCipStatusMapper.BadNodeIdUnknown, + Status.ErrorNotFound => AbCipStatusMapper.BadNodeIdUnknown, // #737 I2: NoMatch/BadDevice are comms Status.ErrorNotAllowed => AbCipStatusMapper.BadNotWritable, Status.ErrorOutOfBounds or Status.ErrorTooLarge or Status.ErrorTooSmall => AbCipStatusMapper.BadOutOfRange, Status.ErrorUnsupported or Status.ErrorNotImplemented => AbCipStatusMapper.BadNotSupported, diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTypeMismatchTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTypeMismatchTests.cs index 1bd4cbe5..65171947 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTypeMismatchTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTypeMismatchTests.cs @@ -73,7 +73,7 @@ public sealed class AbCipTypeMismatchTests creates().ShouldBe(1); factory.Tags["T"].Disposed.ShouldBeFalse(); factory.Tags["T"].ReadCount.ShouldBe(Polls); // positive control: every poll did reach the runtime - drv.GetHealth().ShouldBe(health); + AssertStateUnchanged(drv.GetHealth(), health); MismatchWarnings(log).ShouldBe(1); } @@ -104,7 +104,7 @@ public sealed class AbCipTypeMismatchTests Volatile.Read(ref creates).ShouldBe(1); factory.Tags["T"].Disposed.ShouldBeFalse(); factory.Tags["T"].ReadCount.ShouldBe(Polls); // positive control: every poll did reach the runtime - drv.GetHealth().ShouldBe(health); + AssertStateUnchanged(drv.GetHealth(), health); drv.GetHealth().State.ShouldNotBe(DriverState.Degraded); MismatchWarnings(log).ShouldBe(1); } @@ -121,7 +121,7 @@ public sealed class AbCipTypeMismatchTests creates().ShouldBe(1); factory.Tags["T"].Disposed.ShouldBeFalse(); - drv.GetHealth().ShouldBe(health); + AssertStateUnchanged(drv.GetHealth(), health); MismatchWarnings(log).ShouldBe(1); } @@ -179,7 +179,7 @@ public sealed class AbCipTypeMismatchTests target.ForwardOpens.ShouldBe(1); target.ForwardCloses.ShouldBe(0); target.ConnectedRequests.ShouldBeGreaterThanOrEqualTo(Polls); // positive control: polls hit the wire - drv.GetHealth().ShouldBe(health); + AssertStateUnchanged(drv.GetHealth(), health); MismatchWarnings(log).ShouldBe(1); } finally @@ -226,7 +226,7 @@ public sealed class AbCipTypeMismatchTests target.ForwardOpens.ShouldBe(1); target.ForwardCloses.ShouldBe(0); target.ConnectedRequests.ShouldBeGreaterThanOrEqualTo(Polls); // positive control: polls hit the wire - drv.GetHealth().ShouldBe(health); + AssertStateUnchanged(drv.GetHealth(), health); } finally { @@ -267,12 +267,23 @@ public sealed class AbCipTypeMismatchTests Volatile.Read(ref creates).ShouldBe(1); factory.Tags["T"].Disposed.ShouldBeFalse(); factory.Tags["T"].ReadCount.ShouldBe(Polls - 1); // positive control: later polls reached the kept runtime - drv.GetHealth().ShouldBe(health); + AssertStateUnchanged(drv.GetHealth(), health); } private sealed class CountingFactory(Func create) : IAbCipTagFactory { public IAbCipTagRuntime Create(AbCipTagCreateParams createParams) => create(createParams); } + /// + /// A refusal leaves the health STATE and error alone; since the controller answered, LastSuccessfulRead may + /// only move forward (review 07 H1 — any data proving the device answered advances it). + /// + private static void AssertStateUnchanged(DriverHealth actual, DriverHealth before) + { + actual.State.ShouldBe(before.State); + actual.LastError.ShouldBe(before.LastError); + (actual.LastSuccessfulRead ?? DateTime.MinValue).ShouldBeGreaterThanOrEqualTo(before.LastSuccessfulRead ?? DateTime.MinValue); + } + #pragma warning restore OTOPCUA0001 } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberLayoutTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberLayoutTests.cs index b895b2cc..4841c4f6 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberLayoutTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberLayoutTests.cs @@ -99,4 +99,26 @@ public sealed class AbCipUdtMemberLayoutTests new AbCipStructureMember("Setpoints", AbCipDataType.Real, ElementCount: 4), }).ShouldBeNull(); } + + /// Review 07 P11: the layout is computed once per member list, not once per member per poll. + [Fact] + public void GetOrBuild_computes_a_layout_once_per_member_list() + { + IReadOnlyList members = + [ + new AbCipStructureMember("A", AbCipDataType.DInt), + new AbCipStructureMember("B", AbCipDataType.Real), + ]; + var first = AbCipUdtMemberLayout.GetOrBuild(members); + first.ShouldNotBeNull(); + first["B"].ShouldBe(4); + AbCipUdtMemberLayout.GetOrBuild(members).ShouldBeSameAs(first); + + // Positive control: a different (equal-content) list is its own entry, and a not-groupable verdict caches too. + IReadOnlyList copy = [.. members]; + AbCipUdtMemberLayout.GetOrBuild(copy).ShouldNotBeSameAs(first); + IReadOnlyList withBool = [new AbCipStructureMember("F", AbCipDataType.Bool)]; + AbCipUdtMemberLayout.GetOrBuild(withBool).ShouldBeNull(); + AbCipUdtMemberLayout.GetOrBuild(withBool).ShouldBeNull(); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipWriteHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipWriteHealthTests.cs new file mode 100644 index 00000000..db68d1f5 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipWriteHealthTests.cs @@ -0,0 +1,175 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Review 07 H19: write-side health. A client value that cannot be converted never touched the wire, so it must +/// not latch Degraded; and a good write must not overwrite a Degraded the read path just recorded +/// with Healthy (the batch read path is worst-wins for exactly this reason). A good write proves the device +/// answered, so it advances LastSuccessfulRead without touching the state. +/// +[Trait("Category", "Unit")] +public sealed class AbCipWriteHealthTests +{ + private const string Device = "ab://10.0.0.5/1,0"; + + private sealed class ThrowingWriteTag(AbCipTagCreateParams p) : FakeAbCipTag(p) + { + public override Task WriteAsync(CancellationToken cancellationToken) => + throw new InvalidOperationException("socket reset"); + } + + private static async Task<(AbCipDriver Driver, FakeAbCipTagFactory Factory)> BuildAsync( + Func? make = null) + { + var factory = new FakeAbCipTagFactory + { + Customise = make ?? (p => p.TagName == "Broken" + ? new FakeAbCipTag(p) { ThrowOnRead = true } + : new RealEncodingTag(p) { Value = 0 }), + }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device)], + RawTags = AbCipRawTags.From( + new AbCipTagDefinition("D", Device, "D", AbCipDataType.DInt), + new AbCipTagDefinition("I", Device, "I", AbCipDataType.Int), + new AbCipTagDefinition("Broken", Device, "Broken", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions { Enabled = false }, + }, "abcip-write-health", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + return (drv, factory); + } + + /// A fake whose encode converts like the real runtimes do (Convert.ToInt32 / ToInt16). + private sealed class RealEncodingTag(AbCipTagCreateParams p) : FakeAbCipTag(p) + { + public override void EncodeValue(AbCipDataType type, int? bitIndex, object? value) => + Value = type switch + { + AbCipDataType.Int => Convert.ToInt16(value), + _ => Convert.ToInt32(value), + }; + } + + /// Unconvertible / out-of-range client values: refused per tag, health untouched, nothing sent. + [Fact] + public async Task A_client_conversion_error_writes_no_health() + { + var (drv, factory) = await BuildAsync(); + var before = drv.GetHealth(); + + var r = await drv.WriteAsync( + [ + new WriteRequest("D", "not a number"), // FormatException + new WriteRequest("D", new object()), // InvalidCastException + new WriteRequest("I", 1_000_000), // OverflowException + ], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadTypeMismatch); + r[1].StatusCode.ShouldBe(AbCipStatusMapper.BadTypeMismatch); + r[2].StatusCode.ShouldBe(AbCipStatusMapper.BadOutOfRange); + drv.GetHealth().ShouldBe(before); + factory.Tags["D"].WriteCount.ShouldBe(0); + factory.Tags["I"].WriteCount.ShouldBe(0); + } + + /// A good write keeps a Degraded the read path recorded, and advances LastSuccessfulRead. + [Fact] + public async Task A_good_write_does_not_overwrite_Degraded_with_Healthy() + { + var (drv, _) = await BuildAsync(); + await drv.ReadAsync(["Broken"], CancellationToken.None); + var degraded = drv.GetHealth(); + degraded.State.ShouldBe(DriverState.Degraded); + await Task.Delay(20); + + var r = await drv.WriteAsync([new WriteRequest("D", 5)], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + var after = drv.GetHealth(); + after.State.ShouldBe(DriverState.Degraded); + after.LastError.ShouldBe(degraded.LastError); + after.LastSuccessfulRead!.Value.ShouldBeGreaterThan(degraded.LastSuccessfulRead!.Value); // the device answered + } + + /// Positive control: a genuine write transport fault still degrades and preserves LastSuccessfulRead. + [Fact] + public async Task A_write_transport_fault_still_degrades() + { + var (drv, factory) = await BuildAsync(p => new ThrowingWriteTag(p)); + var before = drv.GetHealth(); + + var r = await drv.WriteAsync([new WriteRequest("D", 5)], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadCommunicationError); + var after = drv.GetHealth(); + after.State.ShouldBe(DriverState.Degraded); + after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + factory.Tags["D"].Disposed.ShouldBeTrue(); + } + + /// M3: a good write clears a Degraded that a WRITE comms fault recorded. + [Fact] + public async Task A_good_write_clears_a_write_side_Degraded() + { + var fail = true; + var (drv, _) = await BuildAsync(p => fail ? new ThrowingWriteTag(p) : new RealEncodingTag(p)); + (await drv.WriteAsync([new WriteRequest("D", 5)], CancellationToken.None))[0].StatusCode + .ShouldBe(AbCipStatusMapper.BadCommunicationError); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + + fail = false; // the evicted runtime is re-created healthy + (await drv.WriteAsync([new WriteRequest("D", 6)], CancellationToken.None))[0].StatusCode + .ShouldBe(AbCipStatusMapper.Good); + + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + drv.GetHealth().LastError.ShouldBeNull(); + } + + /// + /// m1: many concurrent failing writes leave the write-owned marker naming the installed Degraded, so one good + /// write still clears it. + /// + [Fact] + public async Task Concurrent_failing_writes_then_a_good_write_clear_the_write_side_Degraded() + { + var fail = true; + var (drv, _) = await BuildAsync(p => fail ? new ThrowingWriteTag(p) : new RealEncodingTag(p)); + for (var round = 0; round < 20; round++) + { + fail = true; + await Task.WhenAll(Enumerable.Range(0, 8).Select(_ => Task.Run(() => + drv.WriteAsync([new WriteRequest("D", 1), new WriteRequest("I", 2)], CancellationToken.None)))); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + + fail = false; + (await drv.WriteAsync([new WriteRequest("D", 3)], CancellationToken.None))[0].StatusCode + .ShouldBe(AbCipStatusMapper.Good); + drv.GetHealth().State.ShouldBe(DriverState.Healthy, $"round {round}"); + } + } + + /// + /// M3: a refusal-only read batch in between (no read comms fault happened) keeps the Degraded write-owned, so a + /// good write still clears it. + /// + [Fact] + public async Task A_refusal_only_read_between_keeps_the_Degraded_write_owned() + { + var fail = true; + var (drv, _) = await BuildAsync(p => p.TagName == "Broken" + ? new FakeAbCipTag(p) { Status = (int)ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.AbStatus.NotFound } + : fail ? new ThrowingWriteTag(p) : new RealEncodingTag(p)); + await drv.WriteAsync([new WriteRequest("D", 5)], CancellationToken.None); + drv.GetHealth().State.ShouldBe(DriverState.Degraded); + (await drv.ReadAsync(["Broken"], CancellationToken.None))[0].StatusCode.ShouldBe(AbCipStatusMapper.BadNodeIdUnknown); + + fail = false; + await drv.WriteAsync([new WriteRequest("D", 6)], CancellationToken.None); + + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs index 740e0985..a95b8822 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs @@ -115,6 +115,25 @@ internal class FakeAbCipTag : IAbCipTagRuntime /// The value to encode. public virtual void EncodeValue(AbCipDataType type, int? bitIndex, object? value) => Value = value; + /// The element type of the last , if any. + public AbCipDataType? EncodedArrayType { get; private set; } + + /// The bytes the last produced (the shared codec, so conversions throw as in production). + public byte[]? EncodedArrayBytes { get; private set; } + + /// The element count of the last . + public int EncodedArrayCount { get; private set; } + + /// Encodes an array write through the production codec and records it. + /// The element type. + /// The element values. + public virtual void EncodeArray(AbCipDataType type, IReadOnlyList values) + { + EncodedArrayBytes = AbCipArrayCodec.Encode(type, values); + EncodedArrayType = type; + EncodedArrayCount = values.Count; + } + /// Marks the tag as disposed. public virtual void Dispose() => Disposed = true; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/LibplctagStatusTranslationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/LibplctagStatusTranslationTests.cs index fba8f7ea..9e1fd14d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/LibplctagStatusTranslationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/LibplctagStatusTranslationTests.cs @@ -16,8 +16,8 @@ public sealed class LibplctagStatusTranslationTests [InlineData(Status.Pending, AbStatus.Pending)] [InlineData(Status.ErrorTimeout, AbStatus.Timeout)] [InlineData(Status.ErrorNotFound, AbStatus.NotFound)] - [InlineData(Status.ErrorNoMatch, AbStatus.NotFound)] - [InlineData(Status.ErrorBadDevice, AbStatus.NotFound)] + [InlineData(Status.ErrorNoMatch, AbStatus.BadConnection)] // #737 I2: not proof the controller answered + [InlineData(Status.ErrorBadDevice, AbStatus.BadConnection)] [InlineData(Status.ErrorNotAllowed, AbStatus.NotAllowed)] [InlineData(Status.ErrorOutOfBounds, AbStatus.OutOfBounds)] [InlineData(Status.ErrorTooLarge, AbStatus.TooLarge)] @@ -148,4 +148,24 @@ public sealed class LibplctagStatusTranslationTests LibplctagStatusTranslation.Wrap(() => throw new LibPlcTagException(Status.ErrorBadParam))); thrown.Status.ShouldBe(AbStatus.BadConnection); } + + /// + /// #737 I2: libplctag's ErrorNotImplemented is the LIBRARY refusing (no I/O): a local refusal, not a device + /// refusal — it must not count as "the controller answered". + /// + [Fact] + public async Task NotImplemented_is_a_local_refusal_not_a_device_refusal() + { + var ex = await Should.ThrowAsync(() => + LibplctagStatusTranslation.Wrap(() => throw new LibPlcTagException("ErrorNotImplemented"))); + ex.ShouldBeOfType(); + ex.Status.ShouldBe(AbStatus.Unsupported); + AbCipRefusals.IsDeviceRefusal(ex).ShouldBeFalse(); + AbCipDriverProbe.IsReachableException(ex).ShouldBeFalse(); + + // Positive control: the device's own Unsupported refusal still is one. + var device = await Should.ThrowAsync(() => + LibplctagStatusTranslation.Wrap(() => throw new LibPlcTagException("ErrorUnsupported"))); + AbCipRefusals.IsDeviceRefusal(device).ShouldBeTrue(); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagRuntimeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagRuntimeTests.cs index 91a1a428..028d6e27 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagRuntimeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagRuntimeTests.cs @@ -112,6 +112,163 @@ public sealed class NativeAbCipTagRuntimeTests rt.DecodeValue(AbCipDataType.Bool, null).ShouldBe(true); } + /// + /// Review 07 U11: a scalar BOOL authored over a BOOL-array element Bits[n] reads DWORD n / 32 + /// (a Logix BOOL array is stored as DWORDs and its CIP element index addresses them) and decodes bit + /// n % 32 of it — it used to request element n and report "any of bits 0-7 of the reply set". + /// + /// The authored BOOL-array element. + /// The DWORD the request must address. + /// The DWORD the controller returns (little-endian hex). + /// The element's value. + [Theory] + [InlineData(37, 1, "20000000", true)] // bit 5 of DWORD 1 + [InlineData(36, 1, "20000000", false)] // bit 4 of the same DWORD — the old "low byte != 0" read true + [InlineData(5, 0, "20000000", true)] + [InlineData(1, 0, "01000000", false)] // bit 1, not bit 0 + [InlineData(31, 0, "00000080", true)] // top bit of DWORD 0 — outside the low byte entirely + public async Task A_BOOL_array_element_decodes_its_bit_of_its_DWORD(int n, int dword, string reply, bool expected) + { + var ch = Replying("D300", reply); + using var rt = await ReadAsync(ch, Params($"Bits[{n}]", AbCipDataType.Bool)); + rt.DecodeValue(AbCipDataType.Bool, null).ShouldBe(expected); + ch.Sent[0].ShouldBe(Convert.FromHexString("5204" + "910442697473" + "28" + dword.ToString("X2") + "0100" + "00000000")); + } + + /// + /// #737 I1: a family whose BOOL array is element-addressed (the controller answers the DWORD-index request with a + /// plain BOOL, 0xC1) falls back to the ORIGINAL element path Bits[n] — it must neither decode nor write + /// element n / 32. Positive control: the 0xD3 theory above still decodes bit n % 32. + /// + [Fact] + public async Task A_per_element_BOOL_array_falls_back_to_the_element_path_for_read_and_write() + { + var ch = new FakeCipChannel + { + Respond = req => Task.FromResult(Convert.FromHexString(req[0] == 0x4D ? "CD000000" : "D2000000C10001")), + }; + using var rt = NativeAbCipTagRuntime.ForTest(ch, Params("Bits[37]", AbCipDataType.Bool)); + await rt.InitializeAsync(CancellationToken.None); + await rt.ReadAsync(CancellationToken.None); + + rt.DecodeValue(AbCipDataType.Bool, null).ShouldBe(true); + ch.Sent[^1].ShouldBe(Convert.FromHexString("5204" + "910442697473" + "2825" + "0100" + "00000000")); // Bits[37] + + rt.EncodeValue(AbCipDataType.Bool, null, false); + await rt.WriteAsync(CancellationToken.None); + ch.Sent[^1].ShouldBe(Convert.FromHexString("4D04" + "910442697473" + "2825" + "C100" + "0100" + "00")); // Bits[37] + } + + /// A channel whose reply can be switched mid-test (read replies; writes always succeed). + private static FakeCipChannel Switchable(Func readReply) => new() + { + Respond = req => Task.FromResult(Convert.FromHexString(req[0] == 0x4D ? "CD000000" : readReply())), + }; + + private const string Dword1 = "5204" + "910442697473" + "2801" + "0100" + "00000000"; // Bits[1] (DWORD 37/32) + private const string Element37 = "5204" + "910442697473" + "2825" + "0100" + "00000000"; // Bits[37] (element) + + /// + /// #737 fix round 2 (N1): a REFUSAL of the DWORD request says nothing about the layout — it must not settle it. + /// Tag deployed before the online edit that creates Bits : BOOL[2048]: the init read is refused NotFound; + /// once the array exists the controller answers 0xD3, and the read must address DWORD n/32 and decode bit n%32 + /// (the round-1 code had permanently fallen back to Bits[37] = DWORD 37 and decoded byte 0 as Good). + /// + /// The refusal reply the DWORD request first gets. + [Theory] + [InlineData("D2000400")] // NotFound + [InlineData("D2000F00")] // NotAllowed + [InlineData("D2000A00")] // OutOfBounds + public async Task A_refusal_leaves_the_BOOL_layout_unsettled_and_a_later_DWORD_reply_decodes_the_right_bit(string refusal) + { + var reply = refusal; + var ch = Switchable(() => reply); + using var rt = NativeAbCipTagRuntime.ForTest(ch, Params("Bits[37]", AbCipDataType.Bool)); + await rt.InitializeAsync(CancellationToken.None); // refused: completes with the refusal status + rt.GetStatus().ShouldNotBe((int)AbStatus.Ok); + ch.Sent[^1].ShouldBe(Convert.FromHexString(Dword1)); // never fell back to the element path + + reply = "D2000000D300" + "20000000"; // bit 5 of DWORD 1 = element 37 + await rt.ReadAsync(CancellationToken.None); + + ch.Sent[^1].ShouldBe(Convert.FromHexString(Dword1)); + rt.DecodeValue(AbCipDataType.Bool, null).ShouldBe(true); + } + + /// + /// N1 positive control: a refusal, then a 0xC1 reply, settles PER-ELEMENT — the n/32 element that came back is + /// discarded and Bits[n] is re-read and decoded. + /// + [Fact] + public async Task A_refusal_then_a_per_element_reply_settles_the_element_path() + { + var reply = "D2000A00"; + var ch = Switchable(() => reply); + using var rt = NativeAbCipTagRuntime.ForTest(ch, Params("Bits[37]", AbCipDataType.Bool)); + await rt.InitializeAsync(CancellationToken.None); + + reply = "D2000000C10001"; + await rt.ReadAsync(CancellationToken.None); + + ch.Sent[^2].ShouldBe(Convert.FromHexString(Dword1)); // asked DWORD-addressed, got a plain BOOL… + ch.Sent[^1].ShouldBe(Convert.FromHexString(Element37)); // …so element 37 is re-read and decoded + rt.DecodeValue(AbCipDataType.Bool, null).ShouldBe(true); + } + + /// + /// N1: a layout that changes after it was settled (an online edit re-types the array) re-settles from the reply + /// type — it never decodes a DWORD reply to the ELEMENT request (DWORD n, byte 0) as the element's value. + /// + [Fact] + public async Task A_settled_per_element_layout_resettles_when_the_reply_becomes_a_DWORD() + { + var reply = "D2000000C10000"; + var ch = Switchable(() => reply); + using var rt = NativeAbCipTagRuntime.ForTest(ch, Params("Bits[36]", AbCipDataType.Bool)); + await rt.InitializeAsync(CancellationToken.None); // settles per-element + + reply = "D2000000D300" + "01000000"; // byte 0 non-zero, bit 4 (element 36) clear + await rt.ReadAsync(CancellationToken.None); + + ch.Sent[^1].ShouldBe(Convert.FromHexString("5204" + "910442697473" + "2801" + "0100" + "00000000")); + rt.DecodeValue(AbCipDataType.Bool, null).ShouldBe(false); // bit 4 of DWORD 1, not "byte 0 != 0" + } + + /// N1 minimum guard: a scalar BOOL with no element bit answered as a packed DWORD is refused, not byte 0. + [Fact] + public async Task A_scalar_BOOL_answered_as_a_packed_DWORD_without_an_element_bit_is_refused() + { + using var rt = await ReadAsync(Replying("D300", "01000000"), Params("Flag", AbCipDataType.Bool)); + (Should.Throw(() => rt.DecodeValue(AbCipDataType.Bool, null))).Status.ShouldBe(AbStatus.TypeMismatch); + } + + /// + /// m4: a runtime whose init read failed on the wire (layout still unsettled) settles the layout by a read BEFORE + /// it writes, so the write goes to the right element of a per-element array. + /// + [Fact] + public async Task A_write_on_an_unsettled_layout_reads_first_and_writes_the_element() + { + var reply = "D2000100"; // 0x01: not a refusal — init throws, layout unsettled + var ch = Switchable(() => reply); + using var rt = NativeAbCipTagRuntime.ForTest(ch, Params("Bits[37]", AbCipDataType.Bool)); + await Should.ThrowAsync(() => rt.InitializeAsync(CancellationToken.None)); + + reply = "D2000000C10000"; + rt.EncodeValue(AbCipDataType.Bool, null, true); + await rt.WriteAsync(CancellationToken.None); + + ch.Sent[^1].ShouldBe(Convert.FromHexString("4D04" + "910442697473" + "2825" + "C100" + "0100" + "01")); // Bits[37] + } + + /// Positive control: a BOOL answered as a plain BOOL (0xC1) still decodes its byte. + [Fact] + public async Task A_BOOL_element_answered_as_BOOL_decodes_the_byte() + { + using var rt = await ReadAsync(Replying("C100", "01"), Params("Flag", AbCipDataType.Bool)); + rt.DecodeValue(AbCipDataType.Bool, null).ShouldBe(true); + } + [Fact] public async Task Real_returned_for_a_DInt_authored_tag_is_TypeMismatch() { @@ -129,14 +286,35 @@ public sealed class NativeAbCipTagRuntimeTests rt.GetStatus().ShouldBe((int)AbStatus.Ok); } - [Fact] - public async Task Unknown_tag_fails_initialize_with_NotFound() + /// + /// Review 07 L4: a controller refusal on the existence read completes the initialization (the session works — + /// throwing made the driver dispose + re-create the runtime, re-leasing the connection, every poll); the + /// refusal stays readable and every later read reports it again. + /// + [Theory] + [InlineData("D2000400", AbStatus.NotFound)] + [InlineData("D2000500", AbStatus.NotFound)] + [InlineData("D2000A00", AbStatus.OutOfBounds)] + [InlineData("D2000F00", AbStatus.NotAllowed)] + [InlineData("D200FF010521", AbStatus.OutOfBounds)] + public async Task A_refused_tag_completes_initialize_and_reports_the_refusal_on_read(string reply, AbStatus expected) { - var ch = new FakeCipChannel { Respond = _ => Task.FromResult(Convert.FromHexString("D2000400")) }; + var ch = new FakeCipChannel { Respond = _ => Task.FromResult(Convert.FromHexString(reply)) }; + using var rt = NativeAbCipTagRuntime.ForTest(ch, Params("Nope", AbCipDataType.DInt)); + await rt.InitializeAsync(CancellationToken.None); + rt.GetStatus().ShouldBe((int)expected); + (await Should.ThrowAsync(() => rt.ReadAsync(CancellationToken.None))) + .Status.ShouldBe(expected); + } + + /// Positive control: a reply the transport cannot classify as a refusal still fails initialize. + [Fact] + public async Task An_unclassified_reply_still_fails_initialize() + { + var ch = new FakeCipChannel { Respond = _ => Task.FromResult(Convert.FromHexString("D2000100")) }; using var rt = NativeAbCipTagRuntime.ForTest(ch, Params("Nope", AbCipDataType.DInt)); (await Should.ThrowAsync(() => rt.InitializeAsync(CancellationToken.None))) - .Status.ShouldBe(AbStatus.NotFound); - rt.GetStatus().ShouldBe((int)AbStatus.NotFound); + .Status.ShouldBe(AbStatus.BadReply); } [Fact] diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationScriptingOptionsResolutionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationScriptingOptionsResolutionTests.cs new file mode 100644 index 00000000..5d2a86b7 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationScriptingOptionsResolutionTests.cs @@ -0,0 +1,69 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Scripting; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests; + +/// +/// The Calculation driver's run-guard options: the node-wide Scripting section is the base, and a key +/// authored in the driver's own config overrides only that knob. Before the section was bound, a Calculation +/// driver always ran on the compiled defaults whatever the node configured. +/// +[Trait("Category", "Unit")] +public sealed class CalculationScriptingOptionsResolutionTests +{ + private static readonly ScriptingOptions Node = new() + { + MaxAllocBytesPerRun = 8L * 1024 * 1024, + QuarantineAfterConsecutiveTimeouts = 7, + StuckOrphanQuarantineFactor = 4, + }; + + [Fact] + public void No_driver_keys_and_no_node_options_resolves_null_so_the_guard_uses_its_defaults() => + CalculationDriverFactoryExtensions.ResolveScripting(null, null, null, out _).ShouldBeNull(); + + [Fact] + public void No_driver_keys_inherits_every_node_knob() + { + var resolved = CalculationDriverFactoryExtensions.ResolveScripting(null, null, Node, out _).ShouldNotBeNull(); + + resolved.MaxAllocBytesPerRun.ShouldBe(Node.MaxAllocBytesPerRun); + resolved.QuarantineAfterConsecutiveTimeouts.ShouldBe(7); + resolved.StuckOrphanQuarantineFactor.ShouldBe(4); + } + + [Fact] + public void A_driver_key_overrides_only_its_own_knob_and_the_node_object_is_not_mutated() + { + var resolved = CalculationDriverFactoryExtensions.ResolveScripting(null, 2, Node, out var adjustments).ShouldNotBeNull(); + + resolved.QuarantineAfterConsecutiveTimeouts.ShouldBe(2); + resolved.MaxAllocBytesPerRun.ShouldBe(Node.MaxAllocBytesPerRun, "the unauthored key keeps the node value"); + resolved.StuckOrphanQuarantineFactor.ShouldBe(4, "the stuck factor has no driver key; it is node-wide"); + resolved.ShouldNotBeSameAs(Node); + Node.QuarantineAfterConsecutiveTimeouts.ShouldBe(7); + adjustments.ShouldBeEmpty(); + } + + /// Review m-2: an out-of-range value authored in the driver's own config is clamped AND reported, so + /// the factory can log it rather than the guard changing it silently. + [Fact] + public void An_out_of_range_driver_key_is_clamped_and_reported() + { + var resolved = CalculationDriverFactoryExtensions.ResolveScripting(1024L, 0, Node, out var adjustments).ShouldNotBeNull(); + + resolved.MaxAllocBytesPerRun.ShouldBe(ScriptingOptions.MinMaxAllocBytesPerRun); + resolved.QuarantineAfterConsecutiveTimeouts.ShouldBe(1); + adjustments.Count.ShouldBe(2); + } + + [Fact] + public void A_driver_key_with_no_node_options_layers_on_the_compiled_defaults() + { + var resolved = CalculationDriverFactoryExtensions.ResolveScripting(0L, null, null, out _).ShouldNotBeNull(); + + resolved.MaxAllocBytesPerRun.ShouldBe(0); + resolved.QuarantineAfterConsecutiveTimeouts.ShouldBe(ScriptingOptions.DefaultQuarantineAfterConsecutiveTimeouts); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Authorization/PageAuthorizationGuardTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Authorization/PageAuthorizationGuardTests.cs index b58c9500..231c61cc 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Authorization/PageAuthorizationGuardTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Authorization/PageAuthorizationGuardTests.cs @@ -57,10 +57,11 @@ public class PageAuthorizationGuardTests // AddSecretsAuthorization(). ── [typeof(SecretsAdmin)] = global::ZB.MOM.WW.Secrets.Ui.SecretsAuthorization.ManagePolicy, - // ── AuthenticatedRead (16): dashboards, lists, live tails (read-only) + the dev ContextMenu demo ── + // ── AuthenticatedRead (17): dashboards, lists, live tails (read-only) + the dev ContextMenu demo ── [typeof(Home)] = AdminUiPolicies.AuthenticatedRead, [typeof(global::ZB.MOM.WW.OtOpcUa.AdminUI.Components.Pages.Dev.ContextMenuDemo)] = AdminUiPolicies.AuthenticatedRead, [typeof(Fleet)] = AdminUiPolicies.AuthenticatedRead, + [typeof(global::ZB.MOM.WW.OtOpcUa.AdminUI.Components.Pages.FleetDiagnostics)] = AdminUiPolicies.AuthenticatedRead, [typeof(global::ZB.MOM.WW.OtOpcUa.AdminUI.Components.Pages.Hosts)] = AdminUiPolicies.AuthenticatedRead, [typeof(Alerts)] = AdminUiPolicies.AuthenticatedRead, [typeof(ScriptLog)] = AdminUiPolicies.AuthenticatedRead, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/CompletionTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/CompletionTests.cs index 63b319da..be424a2a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/CompletionTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/CompletionTests.cs @@ -18,6 +18,7 @@ public sealed class CompletionTests labels.ShouldContain("SetVirtualTag"); labels.ShouldContain("Now"); labels.ShouldContain("Logger"); + labels.ShouldContain("CancellationToken"); } [Fact] public async Task Scope_completion_includes_the_ctx_local() diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/DiagnoseTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/DiagnoseTests.cs index 94784a45..27c8679e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/DiagnoseTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/DiagnoseTests.cs @@ -48,6 +48,7 @@ public sealed class DiagnoseTests [InlineData("""var x = 41; System.Threading.Interlocked.Increment(ref x); return x;""")] [InlineData("""var x = 42; return System.Threading.Volatile.Read(ref x);""")] [InlineData("""object o = 42; return o.GetType().IsValueType ? 1 : 0;""")] + [InlineData("""ctx.CancellationToken.ThrowIfCancellationRequested(); return ctx.CancellationToken.IsCancellationRequested ? 1 : 0;""")] public void Allow_listed_forms_are_not_flagged_in_the_editor(string code) => Diag(code).ShouldBeEmpty(); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/ScriptingOptionsBindingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/ScriptingOptionsBindingTests.cs new file mode 100644 index 00000000..f717d6c5 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/ScriptingOptionsBindingTests.cs @@ -0,0 +1,64 @@ +using Microsoft.Extensions.Configuration; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Scripting; +using ZB.MOM.WW.OtOpcUa.Host.Configuration; + +namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Configuration; + +/// +/// Review m-1: the Scripting section binds by the documented key names and is clamped at bind. Each +/// configured value differs from the compiled default, so a misspelled key (which the binder would silently +/// ignore, falling back to the default) goes red. +/// +public sealed class ScriptingOptionsBindingTests +{ + private static IConfiguration Config(Dictionary values) => + new ConfigurationBuilder().AddInMemoryCollection(values).Build(); + + [Fact] + public void Every_documented_key_binds_to_its_property() + { + var bound = ScriptingOptionsBinding.Bind(Config(new() + { + ["Scripting:MaxAllocBytesPerRun"] = "8388608", + ["Scripting:QuarantineAfterConsecutiveTimeouts"] = "7", + ["Scripting:StuckOrphanQuarantineFactor"] = "4.5", + // Sibling keys owned by the script-log pipeline must not disturb the bind. + ["Scripting:LogFilePath"] = "logs/x-.log", + ["Scripting:LogTopicMinLevel"] = "Warning", + }), out var adjustments); + + adjustments.ShouldBeEmpty(); + bound.MaxAllocBytesPerRun.ShouldBe(8L * 1024 * 1024); + bound.QuarantineAfterConsecutiveTimeouts.ShouldBe(7); + bound.StuckOrphanQuarantineFactor.ShouldBe(4.5); + } + + [Fact] + public void Out_of_range_values_are_clamped_at_bind_and_reported() + { + var bound = ScriptingOptionsBinding.Bind(Config(new() + { + ["Scripting:StuckOrphanQuarantineFactor"] = "1e300", + ["Scripting:MaxAllocBytesPerRun"] = "1024", + ["Scripting:QuarantineAfterConsecutiveTimeouts"] = "0", + }), out var adjustments); + + bound.StuckOrphanQuarantineFactor.ShouldBe(ScriptingOptions.MaxStuckOrphanQuarantineFactor); + bound.MaxAllocBytesPerRun.ShouldBe(ScriptingOptions.MinMaxAllocBytesPerRun); + bound.QuarantineAfterConsecutiveTimeouts.ShouldBe(1); + adjustments.Count.ShouldBe(3); + } + + [Fact] + public void An_absent_section_yields_the_compiled_defaults() + { + var bound = ScriptingOptionsBinding.Bind(Config(new()), out var adjustments); + + adjustments.ShouldBeEmpty(); + bound.MaxAllocBytesPerRun.ShouldBe(ScriptingOptions.DefaultMaxAllocBytesPerRun); + bound.QuarantineAfterConsecutiveTimeouts.ShouldBe(ScriptingOptions.DefaultQuarantineAfterConsecutiveTimeouts); + bound.StuckOrphanQuarantineFactor.ShouldBe(ScriptingOptions.DefaultStuckOrphanQuarantineFactor); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorScriptedAlarmRestartTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorScriptedAlarmRestartTests.cs new file mode 100644 index 00000000..2ff682f2 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorScriptedAlarmRestartTests.cs @@ -0,0 +1,103 @@ +using System.Text.Json; +using Akka.Actor; +using Serilog; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.Engines; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Commons.Types; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms; +using ZB.MOM.WW.OtOpcUa.Core.Scripting; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.OpcUa; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; +using ZB.MOM.WW.OtOpcUa.Runtime.VirtualTags; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// Review I-2: under the factory Props a supervision restart of the scripted-alarm host builds a fresh, EMPTY +/// engine. Nothing re-sent ApplyScriptedAlarms, so until the next deploy the host had no mux interest +/// (inputs stopped) and its ownership filter dropped every operator ack/shelve as "not owned". The host now +/// tells its parent from PostRestart, +/// and re-sends the last plans it applied. This test never re-applies by hand. +/// +public sealed class DriverHostActorScriptedAlarmRestartTests : RuntimeActorTestBase +{ + private static readonly NodeId TestNode = NodeId.Parse("driver-alarmhost-restart-test"); + private static readonly RevisionHash RevA = RevisionHash.Parse(new string('d', 64)); + private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(30); + + private static DeploymentId SeedAlarmDeployment(Microsoft.EntityFrameworkCore.IDbContextFactory db) + { + var artifact = JsonSerializer.SerializeToUtf8Bytes(new + { + ScriptedAlarms = new[] + { + new + { + ScriptedAlarmId = "alm-1", EquipmentId = "eq-1", Name = "HighTemp", AlarmType = "AlarmCondition", + Severity = 800, MessageTemplate = "hot", PredicateScriptId = "s-1", + HistorizeToAveva = false, Retain = true, Enabled = true, + }, + }, + Scripts = new[] { new { ScriptId = "s-1", SourceCode = "return (int)ctx.GetTag(\"M.T\").Value > 90;" } }, + }); + var id = DeploymentId.NewId(); + using var ctx = db.CreateDbContext(); + ctx.Deployments.Add(new Deployment + { + DeploymentId = id.Value, RevisionHash = RevA.Value, Status = DeploymentStatus.Sealed, + CreatedBy = "test", SealedAtUtc = DateTime.UtcNow, ArtifactBlob = artifact, + }); + ctx.SaveChanges(); + return id; + } + + [Fact] + public void After_a_supervision_restart_the_alarm_host_is_re_applied_and_inputs_and_acks_work_again() + { + var db = NewInMemoryDbFactory(); + var deploymentId = SeedAlarmDeployment(db); + var publish = this.CreateCleanOutcomePublishProbe(); + var mux = CreateTestProbe(); + var coordinator = CreateTestProbe(); + var host = Sys.ActorOf(DriverHostActor.Props( + db, TestNode, coordinator.Ref, + localRoles: new HashSet { "driver" }, + dependencyMux: mux.Ref, + opcUaPublishActor: publish.Ref, + virtualTagEvaluator: NullVirtualTagEvaluator.Instance, + virtualTagHostOverride: CreateTestProbe().Ref, + scriptRootLogger: new ScriptRootLogger(new LoggerConfiguration().CreateLogger()), + alarmStateStore: new InMemoryAlarmStateStore())); + V3ArtifactSeeder.Dispatch(host, coordinator, deploymentId, RevA); + + var first = mux.FishForMessage(r => r.TagRefs.Contains("M.T"), Timeout); + var alarmHost = first.Subscriber; + + // The crash: a batch item with a null TagId makes the upstream Push throw (the documented unguarded + // path), so supervision restarts the alarm host. NO manual re-apply follows. + alarmHost.Tell(new VirtualTagActor.DependencyValueBatch(new[] + { + new VirtualTagActor.DependencyValueChanged(null!, 1, DateTime.UtcNow), + })); + mux.FishForMessage(u => u.Subscriber.Equals(alarmHost), Timeout); + + // The parent re-applies on the restart signal: interest is registered again by the new incarnation. + mux.FishForMessage(r => r.TagRefs.Contains("M.T"), Timeout); + + // Inputs work: a dependency push activates the alarm. + alarmHost.Tell(new VirtualTagActor.DependencyValueChanged("M.T", 99, DateTime.UtcNow)); + publish.FishForMessage( + m => m.AlarmNodeId == "alm-1" && m.State.Active && !m.State.Acknowledged, Timeout); + + // Acks work: the restarted engine owns alm-1 again, so the command is not filtered out as unowned. + alarmHost.Tell(new AlarmCommand(AlarmId: "alm-1", Operation: "Acknowledge", User: "alice", Comment: null, UnshelveAtUtc: null)); + publish.FishForMessage( + m => m.AlarmNodeId == "alm-1" && m.State.Acknowledged, Timeout) + .State.Acknowledged.ShouldBeTrue(); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorSourceRemovedTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorSourceRemovedTests.cs new file mode 100644 index 00000000..0e7e48ac --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorSourceRemovedTests.cs @@ -0,0 +1,224 @@ +using Akka.Actor; +using Akka.TestKit; +using Microsoft.EntityFrameworkCore; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.Engines; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Admin; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Commons.Types; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; +using ZB.MOM.WW.OtOpcUa.Runtime.VirtualTags; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// The dependency mux retains a ref's last value for replay-opting subscribers and evicts a driver's values on +/// . Before this, nothing in production sent that message, so a +/// driver removed by a deploy kept its last Good in the mux and replayed it to the next replay-opting +/// subscriber as if it were current. The host now sends it wherever a driver OBJECT ends (deploy removal, +/// config-change respawn, operator Restart), never on a supervision restart, and drops value bursts from the +/// child incarnations it has already stopped so a late burst cannot re-retain what was evicted. All through a +/// REAL mux and real driver children. +/// +public sealed class DriverHostActorSourceRemovedTests : RuntimeActorTestBase +{ + private static readonly NodeId TestNode = NodeId.Parse("driver-sourceremoved-test"); + private static readonly RevisionHash RevA = RevisionHash.Parse(new string('a', 64)); + private static readonly RevisionHash RevB = RevisionHash.Parse(new string('b', 64)); + private const string Path1 = "Plant/Modbus/dev1/speed"; + private const string Path2 = "Plant/Modbus2/dev2/flow"; + private static readonly (string, string, string, string, string?)[] NoRefs = []; + + private static readonly (string Driver, string DriverName, string Device, string Tag, string DataType, bool Writable)[] BothDrivers = + [ + ("DI-1", "Modbus", "dev1", "speed", "Double", false), + ("DI-2", "Modbus2", "dev2", "flow", "Double", false), + ]; + + private IActorRef _host = ActorRefs.Nobody; + private IActorRef _mux = ActorRefs.Nobody; + private TestProbe _coordinator = null!; + private TestProbe _holder = null!; + private RecordingSubscribableDriverFactory _factory = null!; + private IDbContextFactory _db = null!; + + /// Spawns a host over a real mux with two enabled drivers, waits for both children to subscribe, + /// registers a replay-opting holder (so the mux retains both refs) and publishes one value per driver. + private void ArrangeTwoDriversWithRetainedValues() + { + _db = NewInMemoryDbFactory(); + var revA = V3ArtifactSeeder.SeedV3Deployment(_db, RevA, BothDrivers, NoRefs, enabled: true); + _factory = new RecordingSubscribableDriverFactory("Modbus"); + _mux = Sys.ActorOf(DependencyMuxActor.Props()); + _coordinator = CreateTestProbe(); + _holder = CreateTestProbe(); + _host = Sys.ActorOf(DriverHostActor.Props( + _db, TestNode, _coordinator.Ref, + driverFactory: _factory, + localRoles: new HashSet { "driver" }, + dependencyMux: _mux, + virtualTagEvaluator: NullVirtualTagEvaluator.Instance, + virtualTagHostOverride: CreateTestProbe().Ref)); + V3ArtifactSeeder.Dispatch(_host, _coordinator, revA, RevA); + AwaitAssert(() => _factory.Created.Count.ShouldBe(2), PresenceBudget); + // The spawn hands a child the PREVIOUS apply's set (empty on the first deploy) and the bulk push then + // re-subscribes with the real one; a value fired before that lands is lost, so wait for the real set. + AwaitSubscribed("DI-1", Path1); + AwaitSubscribed("DI-2", Path2); + + _mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { Path1, Path2 }, _holder.Ref, ReplayRetained: true)); + Driver("DI-1").FireDataChange(Path1, 11.0); + Driver("DI-2").FireDataChange(Path2, 22.0); + // One fish for both: the two drivers' bursts arrive in either order, and fishing for one at a time would + // discard the other. + var pending = new HashSet(StringComparer.Ordinal) { Path1, Path2 }; + _holder.FishForMessage(b => + { + foreach (var i in b.Items) + if ((i.TagId == Path1 && Equals(i.Value, 11.0)) || (i.TagId == Path2 && Equals(i.Value, 22.0))) + pending.Remove(i.TagId); + return pending.Count == 0; + }, PresenceBudget); + } + + /// Waits until the holder has been delivered for . + private void AwaitHolder(string path, object value) => + _holder.FishForMessage( + b => b.Items.Any(i => i.TagId == path && Equals(i.Value, value)), PresenceBudget); + + private void AwaitSubscribed(string driverId, string path) => + AwaitAssert(() => (Driver(driverId).LastSubscribedRefs ?? []).ShouldContain(path), PresenceBudget); + + private RecordingSubscribableDriver Driver(string id) => _factory.Created.Last(d => d.DriverInstanceId == id); + + /// A fresh replay-opting subscriber for both refs; returns what it was replayed, keyed by ref. + private Dictionary Replayed() + { + var probe = CreateTestProbe(); + _mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { Path1, Path2 }, probe.Ref, ReplayRetained: true)); + return probe.ExpectMsg(PresenceBudget) + .Items.ToDictionary(i => i.TagId, StringComparer.Ordinal); + } + + /// The live child actor for (named drv-{id}-g{generation}). + private IActorRef ChildOf(string driverId) + { + var probe = CreateTestProbe(); + Sys.ActorSelection(_host.Path.ToStringWithoutAddress() + "/drv-" + driverId + "-g*").Tell(new Identify(driverId), probe.Ref); + return probe.ExpectMsg(PresenceBudget).Subject.ShouldNotBeNull(); + } + + private static void ShouldNotBeTheOldGood(Dictionary replay, string path, object oldValue) + { + if (!replay.TryGetValue(path, out var item)) return; + (Equals(item.Value, oldValue) && OpcUaStatus.IsGood(item.StatusCode)) + .ShouldBeFalse($"the stopped incarnation's last Good for {path} was replayed as current"); + } + + [Fact] + public void A_driver_removed_by_a_deploy_is_no_longer_replayed_while_a_still_present_driver_is() + { + ArrangeTwoDriversWithRetainedValues(); + + // Positive control before the removal: both values replay. + Replayed().Keys.ShouldBe(new[] { Path1, Path2 }, ignoreOrder: true); + + // Deploy B drops DI-1 (StopChild). + var revB = V3ArtifactSeeder.SeedV3Deployment(_db, RevB, [("DI-2", "Modbus2", "dev2", "flow", "Double", false)], NoRefs, enabled: true); + V3ArtifactSeeder.Dispatch(_host, _coordinator, revB, RevB); + + AwaitAssert( + () => Replayed().Keys.ToArray().ShouldBe(new[] { Path2 }, "DI-1 was removed: its last Good must not replay; DI-2's must"), + PresenceBudget); + } + + /// Review I-1: a burst the removed child sent after the eviction (it stops asynchronously, so it keeps + /// publishing through the rest of the apply) must be dropped by the host, not forwarded to re-retain the value. + [Fact] + public void A_late_burst_from_a_removed_child_does_not_re_retain_its_value() + { + ArrangeTwoDriversWithRetainedValues(); + var oldChild = ChildOf("DI-1"); + // Hold the removed child in its PostStop so it has NOT terminated when the stale burst lands; a terminated + // actor cannot send, and the host forgets a retired child on Terminated. + var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + Driver("DI-1").ShutdownGate = gate; + + var revB = V3ArtifactSeeder.SeedV3Deployment(_db, RevB, [("DI-2", "Modbus2", "dev2", "flow", "Double", false)], NoRefs, enabled: true); + V3ArtifactSeeder.Dispatch(_host, _coordinator, revB, RevB); + + // The stale burst, sent AS the stopped child. + _host.Tell(ValuePublishes.Single("DI-1", Path1, 11.0, OpcUaStatus.Good, DateTime.UtcNow), oldChild); + // Positive control / ordering fence: a live driver's value sent after it is delivered, so the stale burst + // has been processed by the host by the time we look. + Driver("DI-2").FireDataChange(Path2, 23.0); + AwaitHolder(Path2, 23.0); + + Replayed().Keys.ToArray().ShouldBe(new[] { Path2 }, "the removed child's late burst re-retained its last Good"); + gate.TrySetResult(); + } + + /// Review m-3: a config edit respawns the driver (stop + spawn); the OLD object's value is evicted, and + /// the NEW driver's value replays (positive control). + [Fact] + public void A_config_change_respawn_evicts_the_old_value_and_the_new_drivers_value_replays() + { + ArrangeTwoDriversWithRetainedValues(); + + // Adding a tag to DI-1 changes its merged DriverConfig, which respawns it. + var revB = V3ArtifactSeeder.SeedV3Deployment(_db, RevB, + [.. BothDrivers, ("DI-1", "Modbus", "dev1", "temp", "Double", false)], NoRefs, enabled: true); + V3ArtifactSeeder.Dispatch(_host, _coordinator, revB, RevB); + AwaitAssert(() => _factory.Created.Count.ShouldBe(3), PresenceBudget); + var respawned = Driver("DI-1"); + // The spawn hands the child the PREVIOUS apply's set, then the bulk push re-subscribes with the new one + // (which names the added tag); a value fired between the two can be lost, so wait for the final set. + AwaitAssert(() => (respawned.LastSubscribedRefs ?? []).ShouldContain("Plant/Modbus/dev1/temp"), PresenceBudget); + + ShouldNotBeTheOldGood(Replayed(), Path1, 11.0); + + respawned.FireDataChange(Path1, 33.0); + AwaitHolder(Path1, 33.0); + var replay = Replayed(); + replay[Path1].Value.ShouldBe(33.0, "the respawned driver's own value replays"); + replay[Path2].Value.ShouldBe(22.0, "the untouched driver is unaffected"); + } + + /// Review m-3: an operator Restart builds a new driver object; the old one's value is evicted and the + /// new one's replays. + [Fact] + public void An_operator_restart_evicts_the_old_value_and_the_new_drivers_value_replays() + { + ArrangeTwoDriversWithRetainedValues(); + + _host.Tell(new RestartDriver("c1", "DI-1", "tester", Guid.NewGuid())); + AwaitAssert(() => _factory.Created.Count.ShouldBe(3), PresenceBudget); + var restarted = Driver("DI-1"); + AwaitSubscribed("DI-1", Path1); + + ShouldNotBeTheOldGood(Replayed(), Path1, 11.0); + + restarted.FireDataChange(Path1, 44.0); + AwaitHolder(Path1, 44.0); + Replayed()[Path1].Value.ShouldBe(44.0); + } + + [Fact] + public void A_supervision_restart_does_not_evict_the_drivers_retained_value() + { + ArrangeTwoDriversWithRetainedValues(); + var di1 = Driver("DI-1"); + + // Crash DI-1's child: Akka restarts it around the SAME driver object, and the host reprovisions it. + di1.FireMalformedDataChange(Path1); + AwaitAssert(() => di1.InitializeCount.ShouldBe(2), PresenceBudget); + _factory.Created.Count.ShouldBe(2, "a supervision restart re-uses the driver; no new one is created"); + + var replay = Replayed(); + replay.Keys.ShouldBe(new[] { Path1, Path2 }, ignoreOrder: true, + "a restart is the same driver instance: its last value stays replayable"); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorBatchForwardTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorBatchForwardTests.cs index 1c470992..1efaed5e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorBatchForwardTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorBatchForwardTests.cs @@ -110,7 +110,7 @@ public sealed class DriverInstanceActorBatchForwardTests : RuntimeActorTestBase batch.Items.Count.ShouldBe(2); // The per-tag raises for the SAME burst produced nothing extra — no second batch, no - // AttributeValuePublished. Anything here means both handlers were attached. + // single-item batch. Anything here means both handlers were attached. parent.ExpectNoMsg(TimeSpan.FromMilliseconds(400)); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/StubDrivers.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/StubDrivers.cs index 627a1c38..ea0cfaef 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/StubDrivers.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/StubDrivers.cs @@ -279,11 +279,18 @@ internal sealed class RecordingSubscribableDriver : IDriver, ISubscribable, IAla /// public Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask; + /// + /// When set, awaits it (the actor bounds the wait), which keeps a stopping child + /// ALIVE in its PostStop for as long as the test wants — the window in which a stopped child can still + /// have messages reaching its parent. + /// + public TaskCompletionSource? ShutdownGate { get; set; } + /// public Task ShutdownAsync(CancellationToken cancellationToken) { Interlocked.Increment(ref _shutdownCount); - return Task.CompletedTask; + return ShutdownGate?.Task ?? Task.CompletedTask; } /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/ValuePublishes.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/ValuePublishes.cs index d0582eff..4831643c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/ValuePublishes.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/ValuePublishes.cs @@ -5,8 +5,8 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; /// /// Builds the ONE message shape a driver child sends for its values — a /// — for tests that push a single value into the -/// dependency mux by hand. The per-value has no -/// production sender (02/U4); tests should not keep one alive either. +/// dependency mux by hand. There is no per-value message: the former per-value record had no production +/// sender and was deleted (02/U4). /// public static class ValuePublishes { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs index 10bce23b..7cccf27d 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs @@ -19,7 +19,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Historian; /// /// Adapted from the plan's Task 17: the recorder handles the REAL fan-out message the mux emits — /// (TagId/Value/TimestampUtc/StatusCode) — -/// not DriverInstanceActor.AttributeValuePublished. The typed-value / real-status behaviour is +/// not the driver-side DriverInstanceActor.AttributeValueBatchPublished. The typed-value / real-status behaviour is /// pinned in . /// public sealed class ContinuousHistorizationRecorderTests : TestKit diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/DependencyMuxActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/DependencyMuxActorTests.cs index 76ad462f..79237192 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/DependencyMuxActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/DependencyMuxActorTests.cs @@ -10,9 +10,9 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.VirtualTags; public sealed class DependencyMuxActorTests : RuntimeActorTestBase { - /// Verifies that AttributeValuePublished messages are forwarded only to subscribers interested in the specific reference. + /// Verifies that a published value is forwarded only to subscribers interested in the specific reference. [Fact] - public void AttributeValuePublished_is_forwarded_only_to_subscribers_interested_in_that_ref() + public void A_published_value_is_forwarded_only_to_subscribers_interested_in_that_ref() { var mux = Sys.ActorOf(DependencyMuxActor.Props()); var subA = CreateTestProbe(); @@ -21,9 +21,9 @@ public sealed class DependencyMuxActorTests : RuntimeActorTestBase mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "tag-1", "tag-2" }, subA.Ref)); mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "tag-2", "tag-3" }, subB.Ref)); - mux.Tell(new DriverInstanceActor.AttributeValuePublished("driver-1", "tag-1", 10, OpcUaStatus.Good, DateTime.UtcNow)); - mux.Tell(new DriverInstanceActor.AttributeValuePublished("driver-1", "tag-3", 30, OpcUaStatus.Good, DateTime.UtcNow)); - mux.Tell(new DriverInstanceActor.AttributeValuePublished("driver-1", "tag-2", 20, OpcUaStatus.Good, DateTime.UtcNow)); + mux.Tell(ValuePublishes.Single("driver-1", "tag-1", 10, OpcUaStatus.Good, DateTime.UtcNow)); + mux.Tell(ValuePublishes.Single("driver-1", "tag-3", 30, OpcUaStatus.Good, DateTime.UtcNow)); + mux.Tell(ValuePublishes.Single("driver-1", "tag-2", 20, OpcUaStatus.Good, DateTime.UtcNow)); // The mux emits ONLY batches — the per-value comms-loss ingress fans out as a single-item batch. // subA hears tag-1 + tag-2. @@ -54,7 +54,7 @@ public sealed class DependencyMuxActorTests : RuntimeActorTestBase var sub = CreateTestProbe(); mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "tag-1" }, sub.Ref)); - mux.Tell(new DriverInstanceActor.AttributeValuePublished("driver-1", "nobody-cares", 99, OpcUaStatus.Good, DateTime.UtcNow)); + mux.Tell(ValuePublishes.Single("driver-1", "nobody-cares", 99, OpcUaStatus.Good, DateTime.UtcNow)); sub.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); } @@ -67,11 +67,11 @@ public sealed class DependencyMuxActorTests : RuntimeActorTestBase var sub = CreateTestProbe(); mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "tag-1" }, sub.Ref)); - mux.Tell(new DriverInstanceActor.AttributeValuePublished("driver-1", "tag-1", 10, OpcUaStatus.Good, DateTime.UtcNow)); + mux.Tell(ValuePublishes.Single("driver-1", "tag-1", 10, OpcUaStatus.Good, DateTime.UtcNow)); sub.ExpectMsg(); mux.Tell(new DependencyMuxActor.UnregisterInterest(sub.Ref)); - mux.Tell(new DriverInstanceActor.AttributeValuePublished("driver-1", "tag-1", 20, OpcUaStatus.Good, DateTime.UtcNow)); + mux.Tell(ValuePublishes.Single("driver-1", "tag-1", 20, OpcUaStatus.Good, DateTime.UtcNow)); sub.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); } @@ -85,10 +85,10 @@ public sealed class DependencyMuxActorTests : RuntimeActorTestBase mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "tag-1" }, sub.Ref)); mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "tag-2" }, sub.Ref)); // replaces tag-1 - mux.Tell(new DriverInstanceActor.AttributeValuePublished("driver-1", "tag-1", 10, OpcUaStatus.Good, DateTime.UtcNow)); + mux.Tell(ValuePublishes.Single("driver-1", "tag-1", 10, OpcUaStatus.Good, DateTime.UtcNow)); sub.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); - mux.Tell(new DriverInstanceActor.AttributeValuePublished("driver-1", "tag-2", 20, OpcUaStatus.Good, DateTime.UtcNow)); + mux.Tell(ValuePublishes.Single("driver-1", "tag-2", 20, OpcUaStatus.Good, DateTime.UtcNow)); sub.ExpectMsg().Items.ShouldHaveSingleItem().TagId.ShouldBe("tag-2"); } @@ -110,18 +110,18 @@ public sealed class DependencyMuxActorTests : RuntimeActorTestBase // gets dropped (no subscriber for "ref-a" yet), so we re-publish each pass. AwaitAssert(() => { - mux.Tell(new DriverInstanceActor.AttributeValuePublished( + mux.Tell(ValuePublishes.Single( "driver-1", "ref-a", 10, OpcUaStatus.Good, DateTime.UtcNow)); parent.ExpectMsg(TimeSpan.FromMilliseconds(200)) .Value.ShouldBe(10); }, duration: PresenceBudget); // From here the actor is wired — second publish drives the sum. - mux.Tell(new DriverInstanceActor.AttributeValuePublished("driver-1", "ref-b", 32, OpcUaStatus.Good, DateTime.UtcNow)); + mux.Tell(ValuePublishes.Single("driver-1", "ref-b", 32, OpcUaStatus.Good, DateTime.UtcNow)); parent.ExpectMsg(PresenceBudget).Value.ShouldBe(42); // Unrelated ref shouldn't fire eval. - mux.Tell(new DriverInstanceActor.AttributeValuePublished("driver-1", "ref-unrelated", 99, OpcUaStatus.Good, DateTime.UtcNow)); + mux.Tell(ValuePublishes.Single("driver-1", "ref-unrelated", 99, OpcUaStatus.Good, DateTime.UtcNow)); parent.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); } @@ -221,7 +221,7 @@ public sealed class DependencyMuxActorTests : RuntimeActorTestBase var sub = CreateTestProbe(); mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "tag-1" }, sub.Ref)); - mux.Tell(new DriverInstanceActor.AttributeValuePublished( + mux.Tell(ValuePublishes.Single( "driver-1", "tag-1", 10, OpcUaStatus.Bad, DateTime.UtcNow)); sub.ExpectMsg()