diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.Coalescing.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.Coalescing.cs index c1c6fac1..401ce098 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.Coalescing.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.Coalescing.cs @@ -19,8 +19,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; /// The span is safe to read. A block runs from its lowest to its highest member, so every element /// it reads lies between two authored elements of the same file — inside the file whenever the authored tags /// are. A device REFUSAL of a block (an authored element past the end of the file, a PLC-5 native non-N file) -/// falls back to single reads for that block's members, so one bad tag never blackens its neighbours, and the -/// block shape is remembered so it is not retried every poll. A comms fault fails the members together — they +/// — or ANY other status the device returned, incl. libplctag's remote error — falls back to single reads for that +/// block's members in the same pass, so one bad tag never blackens its neighbours; a shape refused twice in a row +/// is remembered so it is not retried every poll. Only a NO-REPLY failure fails the members together — they /// share the connection — exactly as their single reads would have. /// Turned off with the driver-config key coalesceReads: false. /// @@ -133,10 +134,19 @@ public sealed partial class AbLegacyDriver (BlockOutcome, OpVerdict, string?) Refused(string why) { - device.RefusedBlocks.TryAdd(block.Key, 0); - _logger.LogInformation( - "AbLegacy coalesced read {Block} refused ({Reason}); its {Count} tags read singly from now on. Driver={DriverInstanceId} Device={DeviceHostAddress}", - block.StartName + "+" + block.Count, why, block.Members.Count, _driverInstanceId, device.Options.HostAddress); + // The handle is never kept for a refused shape (review 07 r0 M5): it may never be planned again. + EvictRuntime(device, block.Key); + // Every refusal falls back to per-member reads THIS pass, so each member gets its own answer and a bad + // member can never black out its neighbours. The shape is only REMEMBERED after a repeat — one refusal can + // be transient (program mode, a download in progress), two in a row are deterministic. + var refusals = device.BlockRefusals.AddOrUpdate(block.Key, 1, (_, n) => n + 1); + if (refusals >= 2 && device.RefusedBlocks.TryAdd(block.Key, 0)) + { + device.BlockRefusals.TryRemove(block.Key, out _); + _logger.LogInformation( + "AbLegacy coalesced read {Block} refused twice ({Reason}); its {Count} tags read singly from now on. Driver={DriverInstanceId} Device={DeviceHostAddress}", + block.StartName + "+" + block.Count, why, block.Members.Count, _driverInstanceId, device.Options.HostAddress); + } return (BlockOutcome.Fallback, OpVerdict.Neutral, null); } @@ -170,11 +180,10 @@ public sealed partial class AbLegacyDriver if (status != 0) { - if (IsDeviceRefusal((AbStatus)status) || (AbStatus)status == AbStatus.Unsupported) - { - EvictRuntime(device, block.Key); + // ANY status the device returned (a refusal, a remote error, an unmapped reply, Unsupported) falls back + // to per-member reads; only a no-reply status fails the block's members together (review 07 r0 I1). + if (!IsNoReply((AbStatus)status)) return Refused($"device status {(AbStatus)status}"); - } EvictRuntime(device, block.Key); FailAll(AbLegacyStatusMapper.MapStatus(status)); return (BlockOutcome.Done, OpVerdict.Fault, $"transport status {status} reading {block.StartName}+{block.Count}"); @@ -184,6 +193,7 @@ public sealed partial class AbLegacyDriver foreach (var m in block.Members) results[m.Index] = new DataValueSnapshot(DecodeMember(block, m, data), AbLegacyStatusMapper.Good, now, now); + device.BlockRefusals.TryRemove(block.Key, out _); device.FirstNonZeroStatusLogged = false; return (BlockOutcome.Done, OpVerdict.Answered, null); } @@ -193,14 +203,12 @@ public sealed partial class AbLegacyDriver FailAll(AbLegacyStatusMapper.BadCommunicationError); return (BlockOutcome.Done, OpVerdict.Throttled, tex.Message); } - catch (AbTransportException ex) when (IsLocalRefusal(ex) || IsDeviceRefusal(ex.Status)) + catch (AbTransportException ex) when (!IsNoReply(ex.Status)) { - EvictRuntime(device, block.Key); return Refused(ex.Message); } catch (NotSupportedException nse) { - EvictRuntime(device, block.Key); return Refused(nse.Message); } catch (Exception ex) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs index 773acc64..2f303ee8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -677,12 +677,18 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag results[item.Index] = new DataValueSnapshot(null, AbLegacyStatusMapper.MapStatus(status), null, now); if (IsDeviceRefusal((AbStatus)status)) { - // The device ANSWERED with a deterministic refusal (file/element missing, out of bounds…): per-tag - // Bad, the handle is kept (re-creating it would only re-pay the init read), warned once per tag, - // and it is evidence of reachability rather than a comms fault. - WarnRefusalOnce(device, def, "read", $"device status {(AbStatus)status}"); + // The device ANSWERED with a refusal (file/element missing, out of bounds, a remote error…): + // per-tag Bad, the handle is kept (re-creating it would only re-pay the init read), warned once + // per tag, and it is evidence of reachability rather than a comms fault. + WarnRefusalOnce(device, def, "read", $"device status {(AbStatus)status}", results[item.Index].StatusCode); return (OpVerdict.Answered, null); } + if ((AbStatus)status == AbStatus.Unsupported) + { + // Refused locally / not supported: says nothing about reachability. Keep the handle. + WarnRefusalOnce(device, def, "read", "status Unsupported", AbLegacyStatusMapper.BadNotSupported); + return (OpVerdict.Neutral, null); + } // Evict the stale handle so the next call re-creates it (mirrors AbCip). EvictRuntime(device, def.Name); if (!device.FirstNonZeroStatusLogged) @@ -711,14 +717,14 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag { // The runtime refused the operation before the wire (e.g. a native PLC-5 non-N read). // Not a comms fault: BadNotSupported, keep the handle, leave health alone. - WarnRefusalOnce(device, def, "read", ex.Message); + WarnRefusalOnce(device, def, "read", ex.Message, AbLegacyStatusMapper.BadNotSupported); results[item.Index] = new DataValueSnapshot(null, AbLegacyStatusMapper.BadNotSupported, null, now); return (OpVerdict.Neutral, null); } catch (AbTransportException ex) when (IsDeviceRefusal(ex.Status)) { // The native runtime raises a device's refusal (e.g. STS 0x20 "file not found") as an exception. - WarnRefusalOnce(device, def, "read", ex.Message); + WarnRefusalOnce(device, def, "read", ex.Message, AbLegacyStatusMapper.MapStatus(ex.Status)); results[item.Index] = new DataValueSnapshot(null, AbLegacyStatusMapper.MapStatus(ex.Status), null, now); return (OpVerdict.Answered, null); } @@ -726,7 +732,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag { // A deterministic decode refusal (review 07 H16) — the same answer every poll. BadNotSupported, keep // the handle (re-creating it only re-paid the init read), warn once, and NO health write. - WarnRefusalOnce(device, def, "read", nse.Message); + WarnRefusalOnce(device, def, "read", nse.Message, AbLegacyStatusMapper.BadNotSupported); results[item.Index] = new DataValueSnapshot(null, AbLegacyStatusMapper.BadNotSupported, null, now); return (OpVerdict.Neutral, null); } @@ -740,17 +746,34 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag } /// - /// True for a status that proves the DEVICE answered and refused this one operation deterministically — the - /// file or element does not exist, the access is not allowed, the data type or size is wrong. Such a status - /// is per-tag, keeps the handle, and is not a comms fault. Everything else non-zero (timeouts, dropped or - /// refused connections, malformed or partial replies, unknown codes) is treated as a comms fault. - /// is classified separately (). + /// True for a status meaning NO REPLY arrived: a timeout, a dropped / refused connection, an aborted request, a + /// request still pending, or a code this build does not know. Only these are comms faults — they evict the + /// handle, degrade health and count toward (review 07 r0 I1: fail-fast + /// must trip only when nothing came back). /// /// The transport status. - /// for a deterministic device refusal. - internal static bool IsDeviceRefusal(AbStatus status) => status is - AbStatus.NotFound or AbStatus.NotAllowed or AbStatus.OutOfBounds or AbStatus.TooLarge or AbStatus.TooSmall - or AbStatus.TypeMismatch or AbStatus.DeviceStateConflict; + /// when no reply arrived. + internal static bool IsNoReply(AbStatus status) => + !Enum.IsDefined(status) || status is AbStatus.Timeout or AbStatus.BadConnection or AbStatus.Abort or AbStatus.Pending; + + /// + /// True for a non-zero status that proves the DEVICE answered and refused this one operation — the file or + /// element does not exist, the access is not allowed, a size / type is wrong, a remote error (libplctag + /// ERR_REMOTE_ERR, translated to ), or a reply the transport + /// could not map (, ). Such a status is per-tag Bad, + /// keeps the handle, and is evidence of reachability. is classified + /// separately (: it says nothing about reachability). + /// + /// + /// ⚠️ Depends on the transport's PCCC STS table (AbStatusCodes.FromPccc, Lane K): today it maps STS 0x30 + /// ("remote node missing" — a dead DH+/DH-485 node behind a bridge) and 0x20 to answered values, so on the + /// NATIVE transport such a dead node reads as answered and never trips the backoff or the comms-loss watchdog. + /// The remap is follow-up F8 (review 07 r0 I4); this classification needs no change once it lands. + /// + /// The transport status. + /// for a device refusal. + internal static bool IsDeviceRefusal(AbStatus status) => + status is not (AbStatus.Ok or AbStatus.Unsupported) && !IsNoReply(status); // ---- IWritable ---- @@ -1029,7 +1052,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag // The probe's verdict feeds the data path's backoff (review 07 P7): an answer resets the window so a // recovered device is read again at once; a comms fault counts toward marking it down. if (success || IsDeviceRefusal((AbStatus)status)) state.RecordAnswered(); - else state.RecordCommsFault(DateTime.UtcNow); + else if ((AbStatus)status != AbStatus.Unsupported) state.RecordCommsFault(DateTime.UtcNow); } catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; } catch (Exception ex) @@ -1456,12 +1479,14 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag /// A deterministic refusal (BadNotSupported) is answered the same way every time, so it is logged ONCE per /// tag and operation — a polled tag must not flood the log, and the refusal never touches driver health. /// - private void WarnRefusalOnce(DeviceState device, AbLegacyTagDefinition def, string operation, string reason) + private void WarnRefusalOnce(DeviceState device, AbLegacyTagDefinition def, string operation, string reason, + uint status = AbLegacyStatusMapper.BadNotSupported) { if (!device.RefusalLogged.TryAdd($"{operation}:{def.Name}", 0)) return; + // The status the operation is actually answered with (review 07 r0 M8) — not always BadNotSupported. _logger.LogWarning( - "AbLegacy {Operation} refused (BadNotSupported). Driver={DriverInstanceId} Device={DeviceHostAddress} Reference={Reference} Address={Address} Reason={Reason}", - operation, _driverInstanceId, def.DeviceHostAddress, def.Name, def.Address, reason); + "AbLegacy {Operation} refused (0x{Status:X8}). Driver={DriverInstanceId} Device={DeviceHostAddress} Reference={Reference} Address={Address} Reason={Reason}", + operation, status, _driverInstanceId, def.DeviceHostAddress, def.Name, def.Address, reason); } /// The create parameters for a tag's own runtime — shared by creation and the write-refusal check, so @@ -1718,6 +1743,10 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag /// distinct subscription shapes, released with the state). public System.Collections.Concurrent.ConcurrentDictionary RefusedBlocks { get; } = new(StringComparer.Ordinal); + /// Consecutive refusals per coalesced block shape not yet remembered in + /// (a success clears the count). + public System.Collections.Concurrent.ConcurrentDictionary BlockRefusals { get; } = new(StringComparer.Ordinal); + /// Tag operations already warned about for a deterministic refusal (once-per-tag-and-operation log). public System.Collections.Concurrent.ConcurrentDictionary RefusalLogged { get; } = new(StringComparer.Ordinal); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyStatusTranslation.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyStatusTranslation.cs index 3d762e84..281c4ebd 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyStatusTranslation.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyStatusTranslation.cs @@ -23,6 +23,14 @@ internal static class LibplctagLegacyStatusTranslation Status.ErrorPartial => AbStatus.Partial, Status.ErrorAbort => AbStatus.Abort, Status.ErrorBadReply => AbStatus.BadReply, + // The PLC (or a CIP hop) ANSWERED with an error: libplctag's PCCC layer turns a non-zero STS — a file or + // element that does not exist, a write the processor refuses — into PLCTAG_ERR_REMOTE_ERR (and a non-zero + // CIP general status into ERR_REMOTE_ERR / ERR_BAD_STATUS). AbStatus has no dedicated "remote error" value and + // Driver.Ab.Transport is not this driver's to extend (Lane K), so this maps to the existing value whose meaning + // is closest — "the device is reachable but refused the operation" — DeviceStateConflict (OPC UA + // BadDeviceFailure). It MUST NOT fall into the BadConnection default: that is a no-reply comms fault, which + // evicts handles, degrades health and counts toward marking the device down (review 07 r0 I1). + Status.ErrorRemoteErr or Status.ErrorBadStatus => AbStatus.DeviceStateConflict, _ => AbStatus.BadConnection, }; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadCoalescingIntegrationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadCoalescingIntegrationTests.cs index 2558adc3..e3aa58d2 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadCoalescingIntegrationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadCoalescingIntegrationTests.cs @@ -66,4 +66,44 @@ public sealed class AbLegacyReadCoalescingIntegrationTests(AbLegacyServerFixture b.ShouldAllBe(s => s.StatusCode == AbLegacyStatusMapper.Good); b.Select(s => s.Value).ShouldBe(a.Select(s => s.Value)); } + + /// + /// Review 07 r0 I1, on the wire: one coalesced member past the END of the file (the fixture's N7 holds 10 + /// words) makes the PLC refuse the block with a real transport status (libplctag: ERR_REMOTE_ERR). Its + /// neighbours must still read Good on every poll, and the device must never be failed fast or degraded. + /// + /// The simulator profile. + /// The transport stack the driver runs on. + [AbLegacyTheory] + [MemberData(nameof(TransportMatrix.Profiles), MemberType = typeof(TransportMatrix))] + public async Task A_member_past_the_end_of_the_file_never_blacks_out_its_neighbours( + AbLegacyServerProfile profile, AbLegacyTransport transport) + { + if (sim.SkipReasonFor(profile) is { } skip) Assert.Skip(skip); + var ct = TestContext.Current.CancellationToken; + var uri = $"ab://{sim.Host}:{sim.PortFor(profile)}/{sim.CipPath}"; + await using var drv = new AbLegacyDriver(new AbLegacyDriverOptions + { + Devices = [new AbLegacyDeviceOptions(uri, profile.Family)], + RawTags = AbLegacyRawTags.Entries( + new AbLegacyTagDefinition("A", uri, "N7:7", AbLegacyDataType.Int), + new AbLegacyTagDefinition("B", uri, "N7:8", AbLegacyDataType.Int), + new AbLegacyTagDefinition("PastEnd", uri, "N7:12", AbLegacyDataType.Int)), + Timeout = TimeSpan.FromSeconds(5), + Probe = new AbLegacyProbeOptions { Enabled = false }, + Transport = transport, + }, driverInstanceId: $"ablegacy-pastend-{profile.Family}-{transport}"); + await drv.InitializeAsync("{}", ct); + + for (var poll = 0; poll < 3; poll++) + { + var r = await drv.ReadAsync(["A", "B", "PastEnd"], ct); + r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: neighbour A"); + r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: neighbour B"); + r[2].StatusCode.ShouldNotBe(AbLegacyStatusMapper.Good, $"poll {poll}: the past-the-end tag"); + r[2].StatusCode.ShouldNotBe(AbLegacyStatusMapper.BadCommunicationError, + $"poll {poll}: the PLC answered — not a comms fault (status 0x{r[2].StatusCode:X8})"); + } + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyAnsweredStatusTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyAnsweredStatusTests.cs new file mode 100644 index 00000000..8cbaba2a --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyAnsweredStatusTests.cs @@ -0,0 +1,115 @@ +using libplctag; +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.AbLegacy.Tests; + +/// +/// Review 07 r0 I1: on the default libplctag transport a PLC's own refusal (a non-zero PCCC STS — e.g. an element +/// past the end of the file) arrives as ERR_REMOTE_ERR, which used to translate to BadConnection: a +/// no-reply comms fault. A coalesced block holding one such member then failed as a group every poll, and two +/// refused tags in a row tripped the device's fail-fast window. Statuses the device RETURNED are now answers; +/// only no-reply statuses are comms faults. +/// +[Trait("Category", "Unit")] +public sealed class AbLegacyAnsweredStatusTests +{ + private const string Host = "ab://10.0.0.5/1,0"; + + /// What the libplctag runtime's GetStatus() reports for a PLC refusal (through the real translation). + private static readonly int RemoteErr = (int)LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorRemoteErr); + + private static AbLegacyDriver NewDriver(FakeAbLegacyTagFactory factory, bool coalesce, params AbLegacyTagDefinition[] tags) => + new(new AbLegacyDriverOptions + { + Devices = [new AbLegacyDeviceOptions(Host)], + RawTags = AbLegacyRawTags.Entries(tags), + Probe = new AbLegacyProbeOptions { Enabled = false }, + CoalesceReads = coalesce, + }, "drv-i1", factory); + + [Fact] + public void Remote_error_translates_to_an_answered_status_not_a_comms_fault() + { + var s = LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorRemoteErr); + AbLegacyDriver.IsNoReply(s).ShouldBeFalse(); + AbLegacyDriver.IsDeviceRefusal(s).ShouldBeTrue(); + AbLegacyDriver.IsDeviceRefusal(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorBadStatus)).ShouldBeTrue(); + // Positive control: real no-reply statuses still are. + AbLegacyDriver.IsNoReply(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorTimeout)).ShouldBeTrue(); + AbLegacyDriver.IsNoReply(LibplctagLegacyStatusTranslation.ToAbStatus(Status.ErrorBadConnection)).ShouldBeTrue(); + } + + [Theory] + [InlineData(AbStatus.BadReply)] // native: an STS the transport does not map + [InlineData(AbStatus.Partial)] + [InlineData(AbStatus.NotFound)] + [InlineData(AbStatus.DeviceStateConflict)] + public void Statuses_the_device_returned_are_answers(AbStatus status) => + AbLegacyDriver.IsDeviceRefusal(status).ShouldBeTrue(); + + [Theory] + [InlineData(AbStatus.Timeout)] + [InlineData(AbStatus.BadConnection)] + [InlineData(AbStatus.Abort)] + [InlineData((AbStatus)(-99))] // a code this build does not know: conservatively no reply + public void Only_no_reply_statuses_are_comms_faults(AbStatus status) + { + AbLegacyDriver.IsNoReply(status).ShouldBeTrue(); + AbLegacyDriver.IsDeviceRefusal(status).ShouldBeFalse(); + } + + [Fact] + public async Task A_remote_error_member_never_blacks_out_its_coalesced_neighbours() + { + // N7:52 is past the end of the file: the block N7:48+5 and the single N7:52 both draw ERR_REMOTE_ERR. + var factory = new FakeAbLegacyTagFactory + { + Customise = p => p.ElementCount > 1 || p.TagName == "N7:52" + ? new FakeAbLegacyTag(p) { Status = RemoteErr } + : new FakeAbLegacyTag(p) { Value = 48 }, + }; + var drv = NewDriver(factory, coalesce: true, + new AbLegacyTagDefinition("A", Host, "N7:48", AbLegacyDataType.Int), + new AbLegacyTagDefinition("B", Host, "N7:49", AbLegacyDataType.Int), + new AbLegacyTagDefinition("C", Host, "N7:52", AbLegacyDataType.Int)); + await drv.InitializeAsync("{}", CancellationToken.None); + + for (var poll = 0; poll < 4; poll++) + { + var r = await drv.ReadAsync(["A", "B", "C"], CancellationToken.None); + r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}"); + r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}"); + r[2].StatusCode.ShouldBe(AbLegacyStatusMapper.BadDeviceFailure, $"poll {poll}"); + } + drv.GetHealth().State.ShouldBe(DriverState.Healthy); + drv.GetDeviceState(Host)!.IsThrottled(DateTime.UtcNow).ShouldBeFalse(); + await drv.ShutdownAsync(CancellationToken.None); + } + + [Fact] + public async Task Consecutive_refused_tags_do_not_trip_fail_fast_on_an_answering_device() + { + var factory = new FakeAbLegacyTagFactory + { + Customise = p => p.TagName == "N7:9" ? new FakeAbLegacyTag(p) { Value = 9 } : new FakeAbLegacyTag(p) { Status = RemoteErr }, + }; + var drv = NewDriver(factory, coalesce: false, + new AbLegacyTagDefinition("R1", Host, "N7:0", AbLegacyDataType.Int), + new AbLegacyTagDefinition("R2", Host, "N7:3", AbLegacyDataType.Int), + new AbLegacyTagDefinition("R3", Host, "N7:6", AbLegacyDataType.Int), + new AbLegacyTagDefinition("Good", Host, "N7:9", AbLegacyDataType.Int)); + await drv.InitializeAsync("{}", CancellationToken.None); + + for (var poll = 0; poll < 3; poll++) + { + var r = await drv.ReadAsync(["R1", "R2", "R3", "Good"], CancellationToken.None); + r[3].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}: an answering device is never failed fast"); + } + factory.Tags["N7:9"].ReadCount.ShouldBe(3); + factory.Tags["N7:0"].InitializeCount.ShouldBe(1); // refused handles are kept, not churned + await drv.ShutdownAsync(CancellationToken.None); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHostAndStatusTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHostAndStatusTests.cs index 8338dc3b..79cd5d80 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHostAndStatusTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyHostAndStatusTests.cs @@ -89,6 +89,9 @@ public sealed class AbLegacyHostAndStatusTests Status.ErrorNotAllowed => AbLegacyStatusMapper.BadNotWritable, Status.ErrorOutOfBounds or Status.ErrorTooLarge or Status.ErrorTooSmall => AbLegacyStatusMapper.BadOutOfRange, Status.ErrorUnsupported or Status.ErrorNotImplemented => AbLegacyStatusMapper.BadNotSupported, + // DELIBERATE change (review 07 r0 I1): a PLC's own refusal (PCCC STS / CIP status → ERR_REMOTE_ERR / + // ERR_BAD_STATUS) is "device answered", not a comms error. + Status.ErrorRemoteErr or Status.ErrorBadStatus => AbLegacyStatusMapper.BadDeviceFailure, _ => AbLegacyStatusMapper.BadCommunicationError, }; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReadCoalescingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReadCoalescingTests.cs index 59fddba0..0dc8cf89 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReadCoalescingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyReadCoalescingTests.cs @@ -113,7 +113,7 @@ public sealed class AbLegacyReadCoalescingTests } [Fact] - public async Task A_refused_block_falls_back_to_single_reads_and_is_not_retried() + public async Task A_refused_block_falls_back_to_single_reads_and_is_remembered_after_a_repeat() { // N7:9 is past the end of the file: the BLOCK is refused; singly, only that tag is. var (drv, _, created) = NewDriver( @@ -133,7 +133,9 @@ public sealed class AbLegacyReadCoalescingTests r[1].StatusCode.ShouldBe(AbLegacyStatusMapper.Good, $"poll {poll}"); r[2].StatusCode.ShouldBe(AbLegacyStatusMapper.MapStatus(AbStatus.OutOfBounds)); } - created.Count(c => c.ElementCount > 1).ShouldBe(1); // the refused shape is remembered + // Refused on polls 0 and 1 (each falling back to singles in the same pass); remembered after the repeat, + // so poll 2 does not try the block again. + created.Count(c => c.ElementCount > 1).ShouldBe(2); drv.GetHealth().State.ShouldBe(DriverState.Healthy); // refusals are not comms faults await drv.ShutdownAsync(CancellationToken.None); }