fix(ablegacy): an Unsupported the device replied with is an answered refusal, not a local one

Review I2: Driver.Ab.Transport raises statuses that came off the wire in an
Execute PCCC reply (envelope CIP status, PCCC STS) as AbDeviceReplyException.
AbLegacy's IsLocalRefusal now excludes it, and the new IsAnsweredRefusal (used
by the read, write, probe and indirect-index paths) counts a wire Unsupported -
CIP 0x08/0x1C/0x20 on the envelope, STS 0x10 - as answered: per-tag
BadNotSupported, Healthy, LastSuccessfulRead advanced. Before, they landed in
the 'refused before the wire' neutral class. Pre-wire refusals are unchanged
(still neutral). A socket test covers all four codes (red before); transport
tests pin the marker and a pre-wire builder refusal as the positive control.
The AbCip Forward Open BadConnection path is untouched.

Refs #747
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 17:21:42 -04:00
parent 0c9e4f4e3d
commit 004c09fa13
5 changed files with 93 additions and 9 deletions
@@ -0,0 +1,12 @@
namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport;
/// <summary>
/// An <see cref="AbTransportException"/> 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 <see cref="AbStatus"/> raised locally before anything was sent — most importantly
/// <see cref="AbStatus.Unsupported"/>, which a native runtime also throws for operations it refuses pre-wire
/// (review lane K I2).
/// </summary>
/// <param name="status">The status mapped from the reply.</param>
/// <param name="message">The exception message.</param>
public sealed class AbDeviceReplyException(AbStatus status, string message) : AbTransportException(status, message);
@@ -113,7 +113,7 @@ public static class PcccServices
public static ReadOnlyMemory<byte> 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..];
}
@@ -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.
/// </summary>
/// <remarks>
/// An Unsupported that came OFF THE WIRE (<see cref="AbDeviceReplyException"/>: CIP 0x08 / 0x1C / 0x20 on the
/// Execute PCCC envelope, PCCC STS 0x10) is NOT local — the device answered — and is classified by
/// <see cref="IsAnsweredRefusal"/> instead (review lane K I2).
/// </remarks>
/// <param name="ex">The exception to classify.</param>
/// <returns><see langword="true"/> if the exception is a local refusal that never reached the wire.</returns>
internal static bool IsLocalRefusal(Exception ex) =>
ex is AbTransportException { Status: AbStatus.Unsupported };
ex is AbTransportException { Status: AbStatus.Unsupported } and not AbDeviceReplyException;
/// <summary>
/// True for an exception proving the device ANSWERED and refused this operation: a device-refusal status
/// (<see cref="IsDeviceRefusal"/>), or an <see cref="AbStatus.Unsupported"/> that came off the wire
/// (<see cref="AbDeviceReplyException"/>). Per-tag Bad, keeps the handle, advances <c>LastSuccessfulRead</c>.
/// </summary>
/// <param name="ex">The exception to classify.</param>
/// <returns><see langword="true"/> for an answered refusal.</returns>
internal static bool IsAnsweredRefusal(Exception ex) =>
ex is AbTransportException ate
&& (IsDeviceRefusal(ate.Status) || ate is AbDeviceReplyException { Status: AbStatus.Unsupported });
/// <summary>
/// Initializes a freshly created runtime. A NATIVE runtime whose init (one read) is refused locally is
@@ -148,7 +148,23 @@ public sealed class PcccServicesTests
[Fact]
public void ParseReply_cip_error_status_throws() =>
Should.Throw<AbTransportException>(() => PcccServices.ParseReply(Reply("", 0x08), 1)).Status.ShouldBe(AbStatus.Unsupported);
Should.Throw<AbDeviceReplyException>(() => PcccServices.ParseReply(Reply("", 0x08), 1)).Status.ShouldBe(AbStatus.Unsupported);
/// <summary>Review I2: a status the device REPLIED with is marked as such (envelope CIP status and PCCC STS).</summary>
[Fact]
public void ParseReply_marks_device_statuses_as_device_replies()
{
Should.Throw<AbDeviceReplyException>(() => PcccServices.ParseReply(Reply("", 0x20), 1)).Status.ShouldBe(AbStatus.Unsupported);
Should.Throw<AbDeviceReplyException>(() => PcccServices.ParseReply(Reply(Id + "4F10 9987"), 0x8799)).Status.ShouldBe(AbStatus.Unsupported);
}
/// <summary>Positive control: a refusal raised by a builder before anything is sent is NOT a device reply.</summary>
[Fact]
public void A_pre_wire_builder_refusal_is_not_a_device_reply()
{
var ex = Should.Throw<AbTransportException>(() => PcccServices.SlcTypedRead(1, A("N7:0"), 999));
ex.ShouldNotBeOfType<AbDeviceReplyException>();
}
[Fact]
public void ParseReply_wrong_command_byte_is_BadReply() =>
@@ -20,6 +20,16 @@ public sealed class NativePcccBridgeStatusHealthTests
{
private static readonly byte[] FixedId = [0x07, 0x3D, 0xF3, 0x45, 0x43, 0x50, 0x21];
/// <summary>A fake whose Execute PCCC envelope itself is refused with CIP general status <paramref name="general"/>.</summary>
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;
}
/// <summary>A PCCC fake that answers every request with one fixed STS.</summary>
private static FakeEipServer StsServer(byte sts)
{
@@ -87,5 +97,35 @@ public sealed class NativePcccBridgeStatusHealthTests
await driver.ShutdownAsync(ct);
server.HandlerErrors.ShouldBeEmpty();
}
/// <summary>
/// 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, <c>LastSuccessfulRead</c> advanced. It must not land in the "refused before the wire" neutral class
/// (which advances nothing). The local-refusal counterpart stays neutral (NativeAbLegacyRefusedWriteTests).
/// </summary>
/// <param name="general">The CIP general status of the envelope reply, or 0 for a PCCC STS 0x10 reply.</param>
[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
}