From 459cbb444ee2f5387bc24595239f78c79cf82b5f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 14:53:06 -0400 Subject: [PATCH] fix(abcip): review fix round 1 - BOOL-array fallback, local refusals, minors I1: the U11 Bits[n] -> Bits[n/32] rewrite was unconditional. The native runtime now keeps the original element path until the first reply settles the layout: 0xD3 confirms DWORD addressing (bit n%32); any other reply type (e.g. a 0xC1 per-element BOOL array) or a refusal of the DWORD index falls back to Bits[n] and re-reads, and a write settles the layout by a read first, so it can never decode or write element n/32 of an element-addressed array. I2: libplctag ErrorNoMatch/ErrorBadDevice no longer fold into NotFound (they now read as BadConnection: comms, Degraded, LastSuccessfulRead preserved). ErrorNotImplemented - and the native runtime's own pre-send refusals - raise AbCipLocalRefusalException: BadNotSupported, no health, LastSuccessfulRead NOT advanced, no backoff record, not reachable for the probe or Test Connect. M1: the bit RMW's parent read/write status words follow the health contract (refusal: per-tag Bad + Warning; comms: evict the parent + Degraded). M2: device Unsupported read refusals return Refused (advance LastSuccessfulRead); device write refusals call RecordWriteAnswered (not the pre-send native type gate). M3: a good write clears a Degraded a write comms fault recorded (a read-side Degraded is still left for the read path). M4: a superseded init throws AbCipInitializationSupersededException (not an OCE); ShutdownAsync stamps Unknown only if no newer init published meanwhile. M5: the sync-Dispose timing assert is 2 s against a 4 s probe read; the unencodable-array test asserts health. Refs #737 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../AbCipDriver.cs | 126 ++++++++++++++---- .../AbCipDriverProbe.cs | 3 +- .../AbCipInitializationSupersededException.cs | 11 ++ .../AbCipLocalRefusalException.cs | 15 +++ .../AbCipRefusals.cs | 4 +- .../LibplctagStatusTranslation.cs | 28 +++- .../NativeAbCipTagRuntime.cs | 43 +++++- .../AbCipArrayWriteTests.cs | 1 + .../AbCipBitInWordWidthTests.cs | 34 +++++ .../AbCipDeviceRefusalTests.cs | 70 ++++++++++ .../AbCipLifecycleConcurrencyTests.cs | 37 ++++- .../AbCipLocalRefusalTests.cs | 7 +- .../AbCipStatusMapperTests.cs | 2 +- .../AbCipWriteHealthTests.cs | 18 +++ .../LibplctagStatusTranslationTests.cs | 24 +++- .../NativeAbCipTagRuntimeTests.cs | 24 ++++ 16 files changed, 395 insertions(+), 52 deletions(-) create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipInitializationSupersededException.cs create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipLocalRefusalException.cs 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 8e19daed..10b7a813 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -515,7 +515,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, lock (_lifecycleLock) { if (generation != _lifecycleGeneration) - throw new OperationCanceledException( + throw new AbCipInitializationSupersededException( $"AbCip driver {_driverInstanceId} initialization was superseded by a shutdown or a newer initialization."); _options = options; @@ -562,7 +562,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // 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 OperationCanceledException) + if (ex is AbCipInitializationSupersededException) { _logger.LogInformation("AbCip driver {DriverInstanceId} initialization abandoned: {Reason}", _driverInstanceId, ex.Message); @@ -652,9 +652,15 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, public async Task ShutdownAsync(CancellationToken cancellationToken) { await _poll.DisposeAsync().ConfigureAwait(false); - await ReleaseDeviceStatesAsync().ConfigureAwait(false); + var generation = await ReleaseDeviceStatesAsync().ConfigureAwait(false); _resolver.Clear(); // no-op in v3 (the resolver reads the live table by closure); kept for symmetry - UpdateHealth(h => new DriverHealth(DriverState.Unknown, h.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) ---- @@ -764,10 +770,10 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, probeRuntime = null; } } - catch (AbCipTagConfigurationException ace) + catch (Exception ace) when (ace is AbCipTagConfigurationException or AbCipLocalRefusalException) { - // The probe tag's own name cannot become a request — it never reached the device, so it is no evidence - // either way: no backoff signal, no host transition, one Warning. + // 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; @@ -1145,13 +1151,21 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, { 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); WarnRefusalOnce(def.Name, "read", reference, def.DeviceHostAddress, ate.Message); - return ReadOutcome.None; + return ReadOutcome.Refused; } catch (AbCipTagConfigurationException ace) { @@ -1169,7 +1183,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, reference, def.DeviceHostAddress, ate.Message); return ReadOutcome.Refused; } - catch (AbTransportException ate) when (AbCipRefusals.IsDeviceRefusal(ate.Status)) + 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, @@ -1284,10 +1298,17 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, { 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) { @@ -1485,14 +1506,17 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, { // Final review I4: deterministic — keep the handle, leave health, warn once per tag. 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 health. + // 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) @@ -1550,14 +1574,20 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, "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) { @@ -1570,15 +1600,18 @@ 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.Status)) + catch (AbTransportException ate) when (AbCipRefusals.IsDeviceRefusal(ate)) { - // H1: the controller answered "no" to this write — keep the handle, leave health alone, warn once. + // 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) @@ -1607,14 +1640,29 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// 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) => - UpdateHealth(h => h with { LastSuccessfulRead = now }); + UpdateHealth(h => h.State == DriverState.Degraded && h.LastError is { } err + && ReferenceEquals(err, Volatile.Read(ref _writeFaultError)) + ? new DriverHealth(DriverState.Healthy, now, null) + : h with { LastSuccessfulRead = now }); + + // The LastError string the most recent write-side comms fault wrote (reference identity marks "write-owned"). + private string? _writeFaultError; /// A write-side comms fault: Degraded (never downgrading Faulted), LastSuccessfulRead preserved. - private void RecordWriteCommsFault(string error) => + private void RecordWriteCommsFault(string error) + { + var owned = new string(error.AsSpan()); // a distinct instance, so reference identity cannot alias a read error + Volatile.Write(ref _writeFaultError, owned); UpdateHealth(h => h.State == DriverState.Faulted ? h - : new DriverHealth(DriverState.Degraded, h.LastSuccessfulRead, error)); + : new DriverHealth(DriverState.Degraded, h.LastSuccessfulRead, owned)); + } /// /// Read-modify-write one bit within an integer parent (SINT / INT / DINT / LINT — Logix allows .n on @@ -1636,7 +1684,7 @@ 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"); // 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). @@ -1685,7 +1733,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, var writeStatus = parentRuntime.GetStatus(); return writeStatus == 0 ? AbCipStatusMapper.Good - : AbCipStatusMapper.MapStatus(writeStatus); + : BitWriteStatusWord(device, parentName, writeStatus, "write"); } finally { @@ -1693,6 +1741,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. @@ -1802,10 +1872,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; } 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 52bb0f46..b927d758 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs @@ -206,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..eeb1afef --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipInitializationSupersededException.cs @@ -0,0 +1,11 @@ +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. +/// +/// 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 index fa82e31b..334568a1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipRefusals.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipRefusals.cs @@ -36,5 +36,7 @@ internal static class AbCipRefusals /// 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); + /// 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/LibplctagStatusTranslation.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagStatusTranslation.cs index 37614e2b..002e8c21 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,17 @@ 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). Status.ErrorUnsupported or Status.ErrorNotImplemented => AbStatus.Unsupported, Status.ErrorPartial => AbStatus.Partial, Status.ErrorAbort => AbStatus.Abort, @@ -70,9 +76,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 +98,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 +108,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 +116,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/NativeAbCipTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs index a77179f9..33ce02d8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs @@ -42,9 +42,14 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime private byte[]? _path; // U11: a scalar BOOL authored over a BOOL-array element "Bits[n]" — the bit (n % 32) of the DWORD the request - // addresses (n / 32). Null for every other tag. + // addresses (n / 32). Null for every other tag, and cleared (with _path restored to _elementPath) the moment the + // controller shows its BOOL array is element-addressed (#737 I1: a 0xC1 reply, or a refusal, to the DWORD request). private int? _boolElementBit; + // The ORIGINAL element-addressed path ("Bits[n]") while a DWORD-addressed _path is still unconfirmed; null once + // the layout is settled either way. + private byte[]? _elementPath; + private ConnectionLease? _lease; private RequestScheduler? _privateScheduler; // ForTest only — production always uses the lease's private LogixTagClient? _client; @@ -124,6 +129,7 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime if (split.Bit is null && !_p.IsArray && _p.AuthoredType == AbCipDataType.Bool && TrySplitBoolArrayElement(name, out var dwordName, out var bit)) { + _elementPath = CipPath.EncodeSymbolic(name); name = dwordName; _boolElementBit = bit; } @@ -148,7 +154,7 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime { await ReadAsync(cancellationToken).ConfigureAwait(false); } - catch (AbTransportException ex) when (AbCipRefusals.IsDeviceRefusal(ex.Status)) + catch (AbTransportException ex) when (AbCipRefusals.IsDeviceRefusal(ex)) { // The controller answered. _status already holds the refusal. } @@ -162,11 +168,25 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime try { var (type, handle, data) = await client.ReadAsync(path, _elements, cancellationToken, ExpectedElementBytes(_p.AuthoredType)).ConfigureAwait(false); + if (_elementPath is not null) + { + // First answer to the DWORD-addressed BOOL-array request settles the layout: 0xD3 confirms it; anything + // else means this family addresses BOOL-array ELEMENTS (e.g. a 0xC1 per-element array) — decoding or + // writing element n / 32 there would hit the wrong element, so go back to "Bits[n]" and read again. + if (type != LogixTypeCode.BoolArray) { FallBackToElementPath(); await ReadAsync(cancellationToken).ConfigureAwait(false); return; } + _elementPath = null; + } _data = data; _typeCode = type; _structHandle = handle; _status = IsTypeMismatch(type) ? AbStatus.TypeMismatch : AbStatus.Ok; } + catch (AbTransportException ex) when (_elementPath is not null && AbCipRefusals.IsDeviceRefusal(ex)) + { + // The DWORD index was refused (e.g. out of bounds on an element-addressed array): retry as "Bits[n]". + FallBackToElementPath(); + await ReadAsync(cancellationToken).ConfigureAwait(false); + } catch (AbTransportException ex) { // libplctag reports a failed operation by exception AND leaves the status readable. @@ -175,6 +195,14 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime } } + /// Abandons the DWORD addressing of a BOOL-array element: the original "Bits[n]" path, no bit extraction. + private void FallBackToElementPath() + { + _path = _elementPath; + _elementPath = null; + _boolElementBit = null; + } + /// /// 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. @@ -207,6 +235,13 @@ 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). + if (_elementPath is not null) + { + 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 @@ -215,7 +250,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) @@ -224,7 +259,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. 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 index ea908d12..5ebd5908 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipArrayWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipArrayWriteTests.cs @@ -123,6 +123,7 @@ public sealed class AbCipArrayWriteTests 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. 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 index 11f7b736..dce0c47d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipBitInWordWidthTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipBitInWordWidthTests.cs @@ -124,4 +124,38 @@ public sealed class AbCipBitInWordWidthTests 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 index 6803d3ab..e4e23d7c 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDeviceRefusalTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDeviceRefusalTests.cs @@ -225,4 +225,74 @@ public sealed class AbCipDeviceRefusalTests { 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 index 2b009a7f..6811afcc 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLifecycleConcurrencyTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLifecycleConcurrencyTests.cs @@ -56,9 +56,9 @@ public sealed class AbCipLifecycleConcurrencyTests return (drv, factory); } - private static async Task WaitUntilAsync(Func condition, string what) + private static async Task WaitUntilAsync(Func condition, string what, int seconds = 5) { - var deadline = DateTime.UtcNow.AddSeconds(5); + var deadline = DateTime.UtcNow.AddSeconds(seconds); while (!condition()) { if (DateTime.UtcNow > deadline) throw new TimeoutException(what); @@ -106,7 +106,8 @@ public sealed class AbCipLifecycleConcurrencyTests var init = drv.InitializeAsync("{}", CancellationToken.None); await Task.Delay(50); await drv.ShutdownAsync(CancellationToken.None); - await Should.ThrowAsync(() => init); + var superseded = await Should.ThrowAsync(() => init); + superseded.ShouldNotBeAssignableTo(); // M4: distinct from a caller cancellation drv.GetDeviceState(Host).ShouldBeNull(); drv.GetHostStatuses().ShouldBeEmpty(); @@ -199,18 +200,40 @@ public sealed class AbCipLifecycleConcurrencyTests [Fact] public async Task Sync_Dispose_does_not_block_on_the_teardown_but_still_completes_it() { - var (drv, factory) = Build(probeReadDelay: TimeSpan.FromMilliseconds(500)); + 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(20); // the probe is inside its slow, cancellation-deaf read + 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.FromMilliseconds(250)); // did not wait out the 500 ms probe read - await WaitUntilAsync(() => loop.IsCompleted && factory.Tags["ProbeTag"].Disposed, "teardown never completed"); + 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/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/AbCipWriteHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipWriteHealthTests.cs index 4821a43a..9a1458c4 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipWriteHealthTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipWriteHealthTests.cs @@ -110,4 +110,22 @@ public sealed class AbCipWriteHealthTests 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(); + } } 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 9d013613..1d514c91 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 @@ -135,6 +135,30 @@ public sealed class NativeAbCipTagRuntimeTests 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] + } + /// 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()