fix(abcip): only a reply type settles the BOOL-array layout (review r2)
N1: a refusal of the DWORD-addressed request permanently settled the layout as per-element, so a tag deployed before the online edit that creates its BOOL array later read DWORD n and published byte 0 as Good. The native runtime now keeps an explicit layout (Unsettled / Packed / PerElement) that ONLY a reply type changes: 0xD3 settles packed (Bits[n/32], bit n%32), 0xC1 settles per-element (Bits[n]). Refusals and comms faults leave it unsettled, so the next read asks again. A reply whose type disagrees with the addressing it was asked under is discarded and re-read once under the shown layout (it would be some other element), also when an online edit re-types a settled array. A scalar BOOL answered 0xD3 with no element bit, or not settled packed, is refused BadTypeMismatch instead of decoding byte 0. m1: the write-owned Degraded marker is set together with the CAS that installs it, under one lock, so concurrent failing writes cannot leave it naming a different instance (a good write then clears it); a refusal-only read batch still keeps it write-owned. m2/m3/m5: documented (breaker accounting of a superseded init; libplctag throw-only NotImplemented assumption; the TypeMismatch write status word is libplctag-only). m4: test for a write on an unsettled layout (settles by a read first). Refs #737 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
@@ -149,14 +149,15 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
||||
|
||||
/// <summary>Atomically replaces <see cref="_health"/> with <paramref name="update"/> applied to its current value.</summary>
|
||||
/// <param name="update">Computes the new health from the current one.</param>
|
||||
private void UpdateHealth(Func<DriverHealth, DriverHealth> update)
|
||||
/// <returns>The health instance this call installed.</returns>
|
||||
private DriverHealth UpdateHealth(Func<DriverHealth, DriverHealth> update)
|
||||
{
|
||||
while (true)
|
||||
{
|
||||
var current = _health;
|
||||
var next = update(current);
|
||||
#pragma warning disable CS0420 // a volatile field passed by ref to Interlocked keeps its full-fence semantics
|
||||
if (ReferenceEquals(Interlocked.CompareExchange(ref _health, next, current), current)) return;
|
||||
if (ReferenceEquals(Interlocked.CompareExchange(ref _health, next, current), current)) return next;
|
||||
#pragma warning restore CS0420
|
||||
}
|
||||
}
|
||||
@@ -1505,6 +1506,9 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
||||
else if (status == (int)AbStatus.TypeMismatch)
|
||||
{
|
||||
// Final review I4: deterministic — keep the handle, leave health, warn once per tag.
|
||||
// Only libplctag reaches this status-word branch, where it is a real controller answer: the native
|
||||
// runtime's pre-send type gate THROWS (caught below, no LastSuccessfulRead advance) and never
|
||||
// returns here with the status merely set (m5).
|
||||
results[i] = new WriteResult(AbCipStatusMapper.BadTypeMismatch);
|
||||
RecordWriteAnswered(now);
|
||||
WarnTypeMismatchOnce(def.Name,
|
||||
@@ -1645,23 +1649,35 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
||||
/// replaced it since) is cleared back to <c>Healthy</c>: the device just accepted a write. A Degraded the read
|
||||
/// path recorded is left for the read path to clear.
|
||||
/// </remarks>
|
||||
private void RecordWriteAnswered(DateTime 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 });
|
||||
private void RecordWriteAnswered(DateTime now)
|
||||
{
|
||||
lock (_writeHealthLock)
|
||||
{
|
||||
UpdateHealth(h => h.State == DriverState.Degraded && h.LastError is { } err && ReferenceEquals(err, _writeOwnedError)
|
||||
? 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;
|
||||
// The exact LastError string INSTANCE the most recent write-side comms fault installed (a fresh copy, so reference
|
||||
// identity cannot alias a read-side error). A health still carrying it is write-owned: a refusal-only read batch
|
||||
// keeps it (`with` copies the reference), while any read / poll fault installs a new string and takes ownership.
|
||||
// Set only under _writeHealthLock TOGETHER with the CAS that installs it, so two concurrent failing writes can never
|
||||
// leave the marker naming a different instance than the installed health (#737 m1).
|
||||
private string? _writeOwnedError;
|
||||
private readonly object _writeHealthLock = new();
|
||||
|
||||
/// <summary>A write-side comms fault: <c>Degraded</c> (never downgrading <c>Faulted</c>), <c>LastSuccessfulRead</c> preserved.</summary>
|
||||
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, owned));
|
||||
var owned = new string(error.AsSpan());
|
||||
lock (_writeHealthLock)
|
||||
{
|
||||
var installed = UpdateHealth(h => h.State == DriverState.Faulted
|
||||
? h
|
||||
: new DriverHealth(DriverState.Degraded, h.LastSuccessfulRead, owned));
|
||||
if (ReferenceEquals(installed.LastError, owned)) _writeOwnedError = owned;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -7,5 +7,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip;
|
||||
/// "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.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Resilience accounting (#737 m2): not being an OCE, it passes the Initialize pipeline's breaker
|
||||
/// <c>ShouldHandle</c> (<c>DriverResiliencePipelineBuilder</c>) and, if its generation still matched, the actor's
|
||||
/// <c>RecordFault</c>. That is practically unreachable — <c>DriverInstanceActor</c> runs initialisation single-flight
|
||||
/// and drops generation-mismatched results — and a superseded attempt is at worst one counted failure. Recognising
|
||||
/// it there is a runtime-side (Lane A) change if it is ever wanted.
|
||||
/// </remarks>
|
||||
/// <param name="message">The detail.</param>
|
||||
public sealed class AbCipInitializationSupersededException(string message) : InvalidOperationException(message);
|
||||
|
||||
@@ -25,6 +25,10 @@ internal static class LibplctagStatusTranslation
|
||||
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).
|
||||
// ASSUMPTION (#737 m3): libplctag reports a failed operation by THROWING, so a NotImplemented reaches the driver
|
||||
// only through FromException. AbStatus has no "local" member (it is Driver.Ab.Transport's, Lane K), so a
|
||||
// NotImplemented observed only as a STATUS WORD (GetStatus after a non-throwing op) would still classify as a
|
||||
// device Unsupported refusal; that combination is not produced by libplctag.NET 1.5.2.
|
||||
Status.ErrorUnsupported or Status.ErrorNotImplemented => AbStatus.Unsupported,
|
||||
Status.ErrorPartial => AbStatus.Partial,
|
||||
Status.ErrorAbort => AbStatus.Abort,
|
||||
|
||||
@@ -41,15 +41,34 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
|
||||
private int? _implicitBit;
|
||||
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, 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).
|
||||
// U11: a scalar BOOL authored over a BOOL-array element "Bits[n]" — the bit (n % 32) of DWORD n / 32 when the
|
||||
// array is packed. Null for every other tag.
|
||||
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.
|
||||
// Both addressings of such a tag: "Bits[n / 32]" (packed: a Logix BOOL array is stored as 0xD3 DWORDs) and the
|
||||
// original "Bits[n]" (per-element: a family that answers 0xC1). _path is whichever _boolLayout selects.
|
||||
private byte[]? _dwordPath;
|
||||
private byte[]? _elementPath;
|
||||
|
||||
/// <summary>How a BOOL-array element tag's array is laid out, as the controller's REPLY TYPE last showed it.</summary>
|
||||
private enum BoolLayout
|
||||
{
|
||||
/// <summary>No reply type seen yet (or only refusals / comms faults): request DWORD-addressed, keep asking.</summary>
|
||||
Unsettled,
|
||||
|
||||
/// <summary>The controller answered 0xD3: address DWORD n / 32, decode bit n % 32.</summary>
|
||||
Packed,
|
||||
|
||||
/// <summary>The controller answered 0xC1: address element n, decode its byte.</summary>
|
||||
PerElement,
|
||||
}
|
||||
|
||||
// #737 fix round 2 (N1): ONLY a reply type settles (or re-settles) the layout — 0xD3 ⇒ Packed, 0xC1 ⇒ PerElement.
|
||||
// A refusal or comms fault says nothing about the layout and leaves it as it was, so a tag deployed before the
|
||||
// online edit that creates its array starts reading correctly once the array exists. A reply whose type disagrees
|
||||
// with the addressing it was asked under is DISCARDED (never decoded) and re-read under the right addressing.
|
||||
private BoolLayout _boolLayout;
|
||||
|
||||
private ConnectionLease? _lease;
|
||||
private RequestScheduler? _privateScheduler; // ForTest only — production always uses the lease's
|
||||
private LogixTagClient? _client;
|
||||
@@ -130,10 +149,15 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
|
||||
&& TrySplitBoolArrayElement(name, out var dwordName, out var bit))
|
||||
{
|
||||
_elementPath = CipPath.EncodeSymbolic(name);
|
||||
name = dwordName;
|
||||
_dwordPath = CipPath.EncodeSymbolic(dwordName);
|
||||
_boolElementBit = bit;
|
||||
_boolLayout = BoolLayout.Unsettled;
|
||||
path = _dwordPath;
|
||||
}
|
||||
else
|
||||
{
|
||||
path = CipPath.EncodeSymbolic(name);
|
||||
}
|
||||
path = CipPath.EncodeSymbolic(name);
|
||||
}
|
||||
catch (Exception ex) when (ex is ArgumentException or FormatException or OverflowException)
|
||||
{
|
||||
@@ -161,32 +185,43 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public async Task ReadAsync(CancellationToken cancellationToken)
|
||||
public Task ReadAsync(CancellationToken cancellationToken) => ReadCoreAsync(cancellationToken, mayRelayout: true);
|
||||
|
||||
private async Task ReadCoreAsync(CancellationToken cancellationToken, bool mayRelayout)
|
||||
{
|
||||
var client = Client();
|
||||
var path = Path();
|
||||
try
|
||||
{
|
||||
var (type, handle, data) = await client.ReadAsync(path, _elements, cancellationToken, ExpectedElementBytes(_p.AuthoredType)).ConfigureAwait(false);
|
||||
if (_elementPath is not null)
|
||||
if (_boolElementBit 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;
|
||||
BoolLayout? shown = type == LogixTypeCode.BoolArray ? BoolLayout.Packed
|
||||
: type == LogixTypeCode.Bool ? BoolLayout.PerElement
|
||||
: null; // any other type (a mistyped tag): no layout evidence; the type check below reports it
|
||||
if (shown is { } layout)
|
||||
{
|
||||
var askedPacked = ReferenceEquals(path, _dwordPath);
|
||||
SetBoolLayout(layout);
|
||||
if ((layout == BoolLayout.Packed) != askedPacked)
|
||||
{
|
||||
// The reply is for the OTHER addressing (element n / 32 of a per-element array, or DWORD n of a
|
||||
// packed one) — not this element. Discard it and ask again under the layout it just showed.
|
||||
if (mayRelayout)
|
||||
{
|
||||
await ReadCoreAsync(cancellationToken, mayRelayout: false).ConfigureAwait(false);
|
||||
return;
|
||||
}
|
||||
throw new AbTransportException(AbStatus.TypeMismatch,
|
||||
$"'{_p.TagName}': the controller's BOOL-array layout changed between two reads; retrying next read.");
|
||||
}
|
||||
}
|
||||
}
|
||||
_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.
|
||||
@@ -195,12 +230,11 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Abandons the DWORD addressing of a BOOL-array element: the original "Bits[n]" path, no bit extraction.</summary>
|
||||
private void FallBackToElementPath()
|
||||
/// <summary>Settles a BOOL-array element tag's layout and selects the matching request path.</summary>
|
||||
private void SetBoolLayout(BoolLayout layout)
|
||||
{
|
||||
_path = _elementPath;
|
||||
_elementPath = null;
|
||||
_boolElementBit = null;
|
||||
_boolLayout = layout;
|
||||
_path = layout == BoolLayout.PerElement ? _elementPath : _dwordPath;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -236,8 +270,9 @@ 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)
|
||||
// DWORD-addressed element of an element-addressed array (#737 I1). A packed layout's write is refused below
|
||||
// (0xD3 DWORD — it would clobber 31 neighbours); a per-element one writes "Bits[n]" as a BOOL.
|
||||
if (_boolElementBit is not null && _boolLayout == BoolLayout.Unsettled)
|
||||
{
|
||||
await ReadAsync(cancellationToken).ConfigureAwait(false);
|
||||
path = Path();
|
||||
@@ -323,9 +358,7 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
|
||||
|
||||
return type switch
|
||||
{
|
||||
AbCipDataType.Bool => bitIndex is int bit ? GetBit(bit)
|
||||
: offset == 0 && _boolElementBit is int element && _typeCode == LogixTypeCode.BoolArray ? GetBit(element)
|
||||
: Span(offset, 1)[0] != 0,
|
||||
AbCipDataType.Bool => bitIndex is int bit ? GetBit(bit) : DecodeScalarBool(offset),
|
||||
AbCipDataType.SInt => (int)(sbyte)Span(offset, 1)[0],
|
||||
AbCipDataType.USInt => (int)Span(offset, 1)[0],
|
||||
AbCipDataType.Int => (int)BinaryPrimitives.ReadInt16LittleEndian(Span(offset, 2)),
|
||||
@@ -471,6 +504,22 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
|
||||
catch (Exception) { /* ForTest only; Dispose must not throw */ }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A scalar BOOL. A packed (0xD3) reply is decoded ONLY as the element's own bit of the DWORD the packed layout
|
||||
/// addressed; any other 0xD3 reply (no element bit, or not settled packed) is refused rather than decoded as
|
||||
/// "byte 0 != 0" — that value would be some other bit, published as Good (#737 N1).
|
||||
/// </summary>
|
||||
private bool DecodeScalarBool(int offset)
|
||||
{
|
||||
if (offset == 0 && _typeCode == LogixTypeCode.BoolArray)
|
||||
{
|
||||
if (_boolElementBit is int element && _boolLayout == BoolLayout.Packed) return GetBit(element);
|
||||
throw new AbTransportException(AbStatus.TypeMismatch,
|
||||
$"'{_p.TagName}' is a scalar BOOL but the controller answered a packed BOOL-array DWORD (0xD3) with no element bit to select.");
|
||||
}
|
||||
return Span(offset, 1)[0] != 0;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Splits a BOOL-array element reference <c>Name[n]</c> (one non-negative subscript, last segment) into the
|
||||
/// DWORD it lives in (<c>Name[n / 32]</c>) and its bit within that DWORD (<c>n % 32</c>). A Logix BOOL array
|
||||
|
||||
Reference in New Issue
Block a user