diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/AbDeviceReplyException.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/AbDeviceReplyException.cs new file mode 100644 index 00000000..e68d4daf --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/AbDeviceReplyException.cs @@ -0,0 +1,12 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; + +/// +/// An whose status came OFF THE WIRE: the device (or the bridge's Execute PCCC +/// object) replied with a non-zero CIP envelope status or PCCC STS. It lets a driver tell a refusal the device +/// ANSWERED from the same raised locally before anything was sent — most importantly +/// , which a native runtime also throws for operations it refuses pre-wire +/// (review lane K I2). +/// +/// The status mapped from the reply. +/// The exception message. +public sealed class AbDeviceReplyException(AbStatus status, string message) : AbTransportException(status, message); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Pccc/PcccServices.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Pccc/PcccServices.cs index edd193ee..326dca03 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Pccc/PcccServices.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Pccc/PcccServices.cs @@ -113,7 +113,7 @@ public static class PcccServices public static ReadOnlyMemory ParseReply(CipReply reply, ushort expectedTns, byte[]? expectedRequestorId = null) { if (reply.GeneralStatus != 0) - throw new AbTransportException(reply.Status, $"Execute PCCC CIP status 0x{reply.GeneralStatus:X2}."); + throw new AbDeviceReplyException(reply.Status, $"Execute PCCC CIP status 0x{reply.GeneralStatus:X2}."); var r = new CipReader(reply.Data.Span); var idLen = r.U8(); if (idLen < 1) throw new AbTransportException(AbStatus.BadReply, "Empty requestor id in PCCC reply."); @@ -131,7 +131,7 @@ public static class PcccServices throw new AbTransportException(AbStatus.BadReply, $"PCCC TNS {tns} != {expectedTns}."); byte? ext = sts == 0xF0 ? r.U8() : null; if (sts != 0) - throw new AbTransportException(AbStatusCodes.FromPccc(sts, ext), + throw new AbDeviceReplyException(AbStatusCodes.FromPccc(sts, ext), $"PCCC STS 0x{sts:X2}{(ext is { } e ? $" EXT 0x{e:X2}" : "")}."); return reply.Data[r.Position..]; } 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 4d7657d7..9fdcddf9 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -789,9 +789,9 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag results[item.Index] = new DataValueSnapshot(null, AbLegacyStatusMapper.BadNotSupported, null, now); return (OpVerdict.Neutral, null); } - catch (AbTransportException ex) when (IsDeviceRefusal(ex.Status)) + catch (AbTransportException ex) when (IsAnsweredRefusal(ex)) { - // The native runtime raises a device's refusal (e.g. STS 0x20 "file not found") as an exception. + // The native runtime raises a device's refusal (e.g. STS 0x50 "addressing problem") as an exception. 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); @@ -1035,7 +1035,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag results[i] = new WriteResult(AbLegacyStatusMapper.BadCommunicationError); outcome.FaultMessage ??= tex.Message; } - catch (AbTransportException ex) when (IsDeviceRefusal(ex.Status)) + catch (AbTransportException ex) when (IsAnsweredRefusal(ex)) { device.RecordAnswered(); outcome.AnyAnswered = true; @@ -1210,7 +1210,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag try { probeRuntime?.Dispose(); } catch { } probeRuntime = null; state.ProbeInitialized = false; - if (ex is AbTransportException ate && IsDeviceRefusal(ate.Status)) state.RecordAnswered(); + if (IsAnsweredRefusal(ex)) state.RecordAnswered(); else if (ex is not AbTransportException { Status: var st } || IsNoReply(st)) state.RecordCommsFault(Clock()); } @@ -1478,7 +1478,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag { return (null, AbLegacyStatusMapper.BadNotSupported, false, false); } - catch (AbTransportException ex) when (IsDeviceRefusal(ex.Status)) + catch (AbTransportException ex) when (IsAnsweredRefusal(ex)) { // The device answered: the index file/element does not exist. Per-tag, not a comms fault. return (null, AbLegacyStatusMapper.MapStatus(ex.Status), false, false); @@ -1617,10 +1617,26 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag /// all PLC-5 writes), and libplctag's ErrorUnsupported maps to it too. It says nothing about the /// device's reachability, so it must not evict the handle or degrade driver health. /// + /// + /// An Unsupported that came OFF THE WIRE (: CIP 0x08 / 0x1C / 0x20 on the + /// Execute PCCC envelope, PCCC STS 0x10) is NOT local — the device answered — and is classified by + /// instead (review lane K I2). + /// /// The exception to classify. /// if the exception is a local refusal that never reached the wire. internal static bool IsLocalRefusal(Exception ex) => - ex is AbTransportException { Status: AbStatus.Unsupported }; + ex is AbTransportException { Status: AbStatus.Unsupported } and not AbDeviceReplyException; + + /// + /// True for an exception proving the device ANSWERED and refused this operation: a device-refusal status + /// (), or an that came off the wire + /// (). Per-tag Bad, keeps the handle, advances LastSuccessfulRead. + /// + /// The exception to classify. + /// for an answered refusal. + internal static bool IsAnsweredRefusal(Exception ex) => + ex is AbTransportException ate + && (IsDeviceRefusal(ate.Status) || ate is AbDeviceReplyException { Status: AbStatus.Unsupported }); /// /// Initializes a freshly created runtime. A NATIVE runtime whose init (one read) is refused locally is diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Pccc/PcccServicesTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Pccc/PcccServicesTests.cs index d8c6edb7..fbac7f56 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Pccc/PcccServicesTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Pccc/PcccServicesTests.cs @@ -148,7 +148,23 @@ public sealed class PcccServicesTests [Fact] public void ParseReply_cip_error_status_throws() => - Should.Throw(() => PcccServices.ParseReply(Reply("", 0x08), 1)).Status.ShouldBe(AbStatus.Unsupported); + Should.Throw(() => PcccServices.ParseReply(Reply("", 0x08), 1)).Status.ShouldBe(AbStatus.Unsupported); + + /// Review I2: a status the device REPLIED with is marked as such (envelope CIP status and PCCC STS). + [Fact] + public void ParseReply_marks_device_statuses_as_device_replies() + { + Should.Throw(() => PcccServices.ParseReply(Reply("", 0x20), 1)).Status.ShouldBe(AbStatus.Unsupported); + Should.Throw(() => PcccServices.ParseReply(Reply(Id + "4F10 9987"), 0x8799)).Status.ShouldBe(AbStatus.Unsupported); + } + + /// Positive control: a refusal raised by a builder before anything is sent is NOT a device reply. + [Fact] + public void A_pre_wire_builder_refusal_is_not_a_device_reply() + { + var ex = Should.Throw(() => PcccServices.SlcTypedRead(1, A("N7:0"), 999)); + ex.ShouldNotBeOfType(); + } [Fact] public void ParseReply_wrong_command_byte_is_BadReply() => diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/NativePcccBridgeStatusHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/NativePcccBridgeStatusHealthTests.cs index 4829c654..58eecee7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/NativePcccBridgeStatusHealthTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/NativePcccBridgeStatusHealthTests.cs @@ -20,6 +20,16 @@ public sealed class NativePcccBridgeStatusHealthTests { private static readonly byte[] FixedId = [0x07, 0x3D, 0xF3, 0x45, 0x43, 0x50, 0x21]; + /// A fake whose Execute PCCC envelope itself is refused with CIP general status . + private static FakeEipServer CipEnvelopeServer(byte general) + { + var server = new FakeEipServer { ZeroReplyContext = true }; + server.Handler = req => FakeEipServer.Command(req) == Ab.Transport.Eip.EncapCommand.SendRRData + ? FakeEipServer.RRReply(req, [0xCB, 0x00, general, 0x00]) + : null; + return server; + } + /// A PCCC fake that answers every request with one fixed STS. private static FakeEipServer StsServer(byte sts) { @@ -87,5 +97,35 @@ public sealed class NativePcccBridgeStatusHealthTests await driver.ShutdownAsync(ct); server.HandlerErrors.ShouldBeEmpty(); } + + /// + /// Review I2: a refusal that came OFF THE WIRE — the device (or the bridge's Execute PCCC object) answered with + /// CIP 0x08 / 0x1C / 0x20 on the envelope, or PCCC STS 0x10 — is an answered refusal: per-tag BadNotSupported, + /// Healthy, LastSuccessfulRead advanced. It must not land in the "refused before the wire" neutral class + /// (which advances nothing). The local-refusal counterpart stays neutral (NativeAbLegacyRefusedWriteTests). + /// + /// The CIP general status of the envelope reply, or 0 for a PCCC STS 0x10 reply. + [Theory] + [InlineData((byte)0x08)] + [InlineData((byte)0x1C)] + [InlineData((byte)0x20)] + [InlineData((byte)0x00)] + public async Task A_wire_Unsupported_reply_is_an_answered_refusal(byte general) + { + var ct = TestContext.Current.CancellationToken; + await using var server = general == 0 ? StsServer(0x10) : CipEnvelopeServer(general); + var driver = await DriverAsync(server); + var before = driver.GetHealth().LastSuccessfulRead; + + var result = (await driver.ReadAsync(["T"], ct)).Single(); + + result.StatusCode.ShouldBe(AbLegacyStatusMapper.BadNotSupported); + var health = driver.GetHealth(); + health.State.ShouldBe(DriverState.Healthy); + health.LastSuccessfulRead.ShouldNotBeNull(); + health.LastSuccessfulRead.ShouldNotBe(before); + await driver.ShutdownAsync(ct); + server.HandlerErrors.ShouldBeEmpty(); + } #pragma warning restore OTOPCUA0001 }