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 02c5f055..3ddd7e82 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -266,6 +266,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); @@ -884,11 +899,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); @@ -917,10 +938,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)) @@ -930,13 +953,19 @@ 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; + } UpdateHealth(current => new DriverHealth( worstState, - anySuccess ? now : current.LastSuccessfulRead, + anySuccess || anyAnswered ? now : current.LastSuccessfulRead, worstState == DriverState.Healthy ? null : worstError)); } @@ -987,7 +1016,14 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, "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; + 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) { @@ -1022,7 +1058,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, 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; + 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 @@ -1063,7 +1099,16 @@ 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.None; + return ReadOutcome.Refused; + } + catch (AbTransportException ate) when (AbCipRefusals.IsDeviceRefusal(ate.Status)) + { + // 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 ex) { @@ -1119,7 +1164,13 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, "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; + 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) { @@ -1167,7 +1218,13 @@ 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.None; + 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 ex) { @@ -1298,6 +1355,12 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, "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 health. + results[i] = new WriteResult(AbCipStatusMapper.MapStatus(status)); + WarnRefusalOnce(def.Name, "write", w.FullReference, def.DeviceHostAddress, ((AbStatus)status).ToString()); + } else if (status != 0) { EvictRuntime(device, def.Name); @@ -1378,6 +1441,12 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, "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.Status)) + { + // H1: the controller answered "no" to this write — keep the handle, leave health alone, warn once. + results[i] = new WriteResult(AbCipStatusMapper.MapStatus(ate.Status)); + WarnRefusalOnce(def.Name, "write", w.FullReference, def.DeviceHostAddress, ate.Message); + } catch (Exception ex) { // Transport / wire error — evict so the next write creates a fresh handle. @@ -1538,9 +1607,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); 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..52bb0f46 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 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..fa82e31b --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipRefusals.cs @@ -0,0 +1,40 @@ +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. + public static bool IsDeviceRefusal(Exception ex) => ex is AbTransportException ate && 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/NativeAbCipTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs index af1ce7e6..8bf65cca 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs @@ -94,16 +94,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) { @@ -137,9 +136,9 @@ 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.Status)) { - // The controller answered — the tag exists. _status already holds the refusal. + // The controller answered. _status already holds the refusal. } } 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/AbCipDeviceRefusalTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDeviceRefusalTests.cs new file mode 100644 index 00000000..2cb690f5 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDeviceRefusalTests.cs @@ -0,0 +1,181 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; + +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); + } +} 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/NativeAbCipTagRuntimeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagRuntimeTests.cs index 91a1a428..35f42bd3 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 @@ -129,14 +129,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]