dohertj2/lmxopcua
1
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(ab): a refused operation is BadNotSupported, not a comms fault (Task 22 review r1; closes #571)

Browse Source 
Native is the default, so a locally refused write must not look like a comms fault.

- AbLegacy: an AbTransportException with AbStatus.Unsupported (status-bit sub-element writes, PLC-5
  non-N reads, all PLC-5 writes; libplctag's ErrorUnsupported maps here too) now returns
  BadNotSupported on both the read and the write path (incl. the bit-RMW path). The tag handle is
  not evicted and driver health is not touched. A native runtime whose init read is refused locally
  is CACHED rather than disposed, so a poll of a refused tag does not re-create (and re-lease the
  pooled connection) every cycle.
- AbCip: the same mapping on the write path and both read paths (single + whole-UDT).
- Uses the existing named BadNotSupported constants (StatusCodeParityTests guard).
- Tests (falsified by disabling the mapping, and separately the keep-on-refusal): status-bit
  writes and PLC-5 read/write through the driver over the native runtime — BadNotSupported, zero
  wire requests, one create, health unchanged; AbCip refused read/write via a refusing fake plus a
  control that a non-refusal failure still evicts and degrades.
- Docs/CLAUDE.md: BadNotSupported; #571 known-gap wording removed.

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-07 21:12:08 -04:00
parent 1806af54f0
commit 39ae286cd7
7 changed files with 279 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs
+21
View File
@@ -854,6 +854,13 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
{
throw;
}
catch (AbTransportException ate) when (ate.Status == AbStatus.Unsupported)
{
// A deterministic local refusal — not a comms fault: BadNotSupported, keep the handle, no
// health signal (Task 22 review r1).
results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadNotSupported, null, now);
return ReadOutcome.None;
}
catch (Exception ex)
{
// Transport exception — evict so the next read creates a fresh handle. An AbTransportException
@@ -928,6 +935,11 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
{
throw;
}
catch (AbTransportException ate) when (ate.Status == AbStatus.Unsupported)
{
StampGroupStatus(group, results, now, AbCipStatusMapper.BadNotSupported);
return ReadOutcome.None;
}
catch (Exception ex)
{
EvictRuntime(device, parent.Name);
@@ -1064,6 +1076,15 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
"AbCip write value out of range for tag {Tag} on device {Device}",
w.FullReference, def.DeviceHostAddress);
}
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 (Task 22 review r1).
results[i] = new WriteResult(AbCipStatusMapper.BadNotSupported);
_logger.LogWarning(ate,
"AbCip write refused (not supported) for tag {Tag} on device {Device}",
w.FullReference, def.DeviceHostAddress);
}
catch (Exception ex)
{
// Transport / wire error — evict so the next write creates a fresh handle.
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs
+50 -20
View File
@@ -2,6 +2,7 @@ using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using ZB.MOM.WW.OtOpcUa.Commons.Observability;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport;
using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies;
namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy;
@@ -376,6 +377,12 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
_health = new DriverHealth(DriverState.Healthy, now, null);
}
catch (OperationCanceledException) { throw; }
catch (AbTransportException ex) when (IsLocalRefusal(ex))
{
// The runtime refused the operation before the wire (e.g. a native PLC-5 non-N read, #571).
// Not a comms fault: BadNotSupported, keep the handle, leave health alone.
results[i] = new DataValueSnapshot(null, AbLegacyStatusMapper.BadNotSupported, null, now);
}
catch (Exception ex)
{
// Transport exception — evict so the next read creates a fresh handle (mirrors AbCip).
@@ -478,6 +485,12 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
{
results[i] = new WriteResult(AbLegacyStatusMapper.BadOutOfRange);
}
catch (AbTransportException ex) when (IsLocalRefusal(ex))
{
// The runtime refused the write before the wire — a native status-bit sub-element write or a
// PLC-5 write (#571). Not a comms fault: BadNotSupported, keep the handle, leave health alone.
results[i] = new WriteResult(AbLegacyStatusMapper.BadNotSupported);
}
catch (Exception ex)
{
// Transport exception — evict so the next write/read creates a fresh handle (mirrors AbCip).
@@ -744,16 +757,8 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
LibplctagPlcAttribute: device.Profile.LibplctagPlcAttribute,
TagName: parentName,
Timeout: _options.Timeout));
try
{
await runtime.InitializeAsync(ct).ConfigureAwait(false);
}
catch
{
runtime.Dispose();
throw;
}
device.ParentRuntimes[parentName] = runtime;
await InitializeOrKeepRefusedAsync(runtime, () => device.ParentRuntimes[parentName] = runtime, ct)
.ConfigureAwait(false);
return runtime;
}
finally
@@ -780,6 +785,39 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
private static int EffectiveArrayLength(AbLegacyTagDefinition def) =>
def.ArrayLength is int len && len >= 1 ? Math.Min(len, AbLegacyArray.MaxElements) : 1;
/// <summary>
/// True for a deterministic refusal of the operation itself (<see cref="AbStatus.Unsupported"/>) — the
/// native runtime raises it BEFORE anything reaches the wire (status-bit writes, PLC-5 non-N reads and
/// 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>
internal static bool IsLocalRefusal(Exception ex) =>
ex is AbTransportException { Status: AbStatus.Unsupported };
/// <summary>
/// Initializes a freshly created runtime. A NATIVE runtime whose init (one read) is refused locally is
/// still KEPT: the refusal is deterministic and per-operation, so caching it avoids re-creating — and
/// re-leasing the pooled device connection — on every poll. Any other failure disposes it, as before.
/// </summary>
private static async Task InitializeOrKeepRefusedAsync(IAbLegacyTagRuntime runtime, Action keep, CancellationToken ct)
{
try
{
await runtime.InitializeAsync(ct).ConfigureAwait(false);
}
catch (Exception ex) when (IsLocalRefusal(ex) && runtime is NativeAbLegacyTagRuntime)
{
keep();
throw;
}
catch
{
runtime.Dispose();
throw;
}
keep();
}
private async Task<IAbLegacyTagRuntime> EnsureTagRuntimeAsync(
DeviceState device, AbLegacyTagDefinition def, CancellationToken ct)
{
@@ -806,16 +844,8 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
TagName: parsed.ToLibplctagName(),
Timeout: _options.Timeout,
ElementCount: EffectiveArrayLength(def)));
try
{
await runtime.InitializeAsync(ct).ConfigureAwait(false);
}
catch
{
runtime.Dispose();
throw;
}
device.Runtimes[def.Name] = runtime;
await InitializeOrKeepRefusedAsync(runtime, () => device.Runtimes[def.Name] = runtime, ct)
.ConfigureAwait(false);
return runtime;
}
finally