fix(ablegacy): refuse native status-bit and PLC-5 writes before creating the runtime (#593)
A first-touch native write to a T/C/R status-bit sub-element (T4:0.DN) created and initialised the tag's runtime before the runtime's write refusal could fire; the init is one READ, so against a device lacking the file (the ab_server fixture has no T/C/R) it surfaced as BadCommunicationError + Degraded health instead of BadNotSupported. The bit-RMW path was not involved: AbLegacyAddress rejects sub-element + /N, so a status bit's BitIndex is always null. NativeAbLegacyTagRuntime.WriteRefusal is now the single predicate (status bit by address whatever the authored type; any PLC-5 write), asked by the driver via IAbLegacyTagFactory.WriteRefusal (default null, so libplctag is unchanged) before EnsureTagRuntimeAsync and ahead of the bit-RMW branch. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
@@ -428,11 +428,23 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
|
||||
{
|
||||
var parsed = AbLegacyAddress.TryParse(def.Address);
|
||||
|
||||
// A write the transport refuses outright (native: a T/C/R status-bit sub-element such as T4:0.DN,
|
||||
// or any PLC-5 write) is answered BEFORE a runtime is created. Creating one first costs an init
|
||||
// READ, and against a device lacking that file the read failure surfaced as a comms fault —
|
||||
// BadCommunicationError, evicted handle, Degraded health — instead of the refusal (#593 follow-up).
|
||||
// Ahead of the bit-RMW branch too, which would otherwise read the parent word first.
|
||||
if (parsed is not null && _tagFactory.WriteRefusal(CreateParamsFor(device, def, parsed)) is not null)
|
||||
{
|
||||
results[i] = new WriteResult(AbLegacyStatusMapper.BadNotSupported);
|
||||
continue;
|
||||
}
|
||||
|
||||
// PCCC bit-within-word writes — RMW against a parallel parent-word runtime (strip the /N
|
||||
// bit suffix). The per-parent-word lock serialises concurrent bit writers. Applies to every
|
||||
// bit-addressable PCCC file: N-file (N7:0/3), B-file (B3:0/0), and the I/O image files
|
||||
// (I:0/0, O:1/2); L-file bits RMW a 32-bit parent, the rest a 16-bit word. T/C/R sub-elements
|
||||
// don't reach this path because they're not Bit typed. NOTE: an Input-image (I) write is sent
|
||||
// (T4:0.DN, even when authored Bit-typed) never reach this path: AbLegacyAddress.TryParse rejects a
|
||||
// sub-element combined with a /N bit index, so their BitIndex is always null. NOTE: an Input-image (I) write is sent
|
||||
// to the PLC like any other write — the device drives the input image from physical inputs and
|
||||
// may reject it; we surface that real PCCC status rather than pre-rejecting at the driver.
|
||||
if (def.DataType == AbLegacyDataType.Bit && parsed?.BitIndex is int bit)
|
||||
@@ -817,6 +829,17 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
|
||||
keep();
|
||||
}
|
||||
|
||||
/// <summary>The create parameters for a tag's own runtime — shared by creation and the write-refusal check, so
|
||||
/// the refusal is asked about exactly the runtime a write would otherwise create.</summary>
|
||||
private AbLegacyTagCreateParams CreateParamsFor(DeviceState device, AbLegacyTagDefinition def, AbLegacyAddress parsed) =>
|
||||
new(Gateway: device.ParsedAddress.Gateway,
|
||||
Port: device.ParsedAddress.Port,
|
||||
CipPath: device.EffectiveCipPath,
|
||||
LibplctagPlcAttribute: device.Profile.LibplctagPlcAttribute,
|
||||
TagName: parsed.ToLibplctagName(),
|
||||
Timeout: _options.Timeout,
|
||||
ElementCount: EffectiveArrayLength(def));
|
||||
|
||||
private async Task<IAbLegacyTagRuntime> EnsureTagRuntimeAsync(
|
||||
DeviceState device, AbLegacyTagDefinition def, CancellationToken ct)
|
||||
{
|
||||
@@ -835,14 +858,7 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
|
||||
?? throw new InvalidOperationException(
|
||||
$"AbLegacy tag '{def.Name}' has malformed Address '{def.Address}'.");
|
||||
|
||||
var runtime = _tagFactory.Create(new AbLegacyTagCreateParams(
|
||||
Gateway: device.ParsedAddress.Gateway,
|
||||
Port: device.ParsedAddress.Port,
|
||||
CipPath: device.EffectiveCipPath,
|
||||
LibplctagPlcAttribute: device.Profile.LibplctagPlcAttribute,
|
||||
TagName: parsed.ToLibplctagName(),
|
||||
Timeout: _options.Timeout,
|
||||
ElementCount: EffectiveArrayLength(def)));
|
||||
var runtime = _tagFactory.Create(CreateParamsFor(device, def, parsed));
|
||||
await InitializeOrKeepRefusedAsync(runtime, () => device.Runtimes[def.Name] = runtime, ct)
|
||||
.ConfigureAwait(false);
|
||||
return runtime;
|
||||
|
||||
@@ -58,6 +58,16 @@ public interface IAbLegacyTagFactory
|
||||
/// <param name="createParams">The tag creation parameters.</param>
|
||||
/// <returns>The created tag runtime instance.</returns>
|
||||
IAbLegacyTagRuntime Create(AbLegacyTagCreateParams createParams);
|
||||
|
||||
/// <summary>
|
||||
/// Why a runtime from this factory would refuse a WRITE to <paramref name="createParams"/> before anything
|
||||
/// reached the wire, or <see langword="null"/> when it would attempt it. The driver asks this BEFORE it
|
||||
/// creates or initialises a runtime for a write, so a refused write never costs the init read either —
|
||||
/// which against a device lacking the file would otherwise surface as a comms fault.
|
||||
/// </summary>
|
||||
/// <param name="createParams">The parameters the write's runtime would be created with.</param>
|
||||
/// <returns>The refusal reason, or <see langword="null"/> when the write is attempted.</returns>
|
||||
string? WriteRefusal(AbLegacyTagCreateParams createParams) => null;
|
||||
}
|
||||
|
||||
/// <param name="Gateway">The PLC gateway host (IP or DNS).</param>
|
||||
|
||||
@@ -11,4 +11,7 @@ internal sealed class NativeAbLegacyTagFactory(DeviceSessionPool? pool = null) :
|
||||
/// <inheritdoc />
|
||||
public IAbLegacyTagRuntime Create(AbLegacyTagCreateParams createParams) =>
|
||||
new NativeAbLegacyTagRuntime(createParams, _pool);
|
||||
|
||||
/// <inheritdoc />
|
||||
public string? WriteRefusal(AbLegacyTagCreateParams createParams) => NativeAbLegacyTagRuntime.WriteRefusal(createParams);
|
||||
}
|
||||
|
||||
@@ -31,6 +31,7 @@ internal sealed class NativeAbLegacyTagRuntime : IAbLegacyTagRuntime
|
||||
private readonly int _elementSize;
|
||||
private readonly int _count;
|
||||
private readonly byte[] _buffer;
|
||||
private readonly string? _writeRefusal;
|
||||
private int _status;
|
||||
|
||||
/// <summary>Creates a runtime that leases the shared connection for its device from <paramref name="pool"/>.</summary>
|
||||
@@ -64,6 +65,7 @@ internal sealed class NativeAbLegacyTagRuntime : IAbLegacyTagRuntime
|
||||
BinaryPrimitives.WriteUInt16LittleEndian(_requestorId.AsSpan(1), vendor);
|
||||
BinaryPrimitives.WriteUInt32LittleEndian(_requestorId.AsSpan(3), serial);
|
||||
_count = Math.Max(1, p.ElementCount);
|
||||
_writeRefusal = WriteRefusal(p);
|
||||
|
||||
try
|
||||
{
|
||||
@@ -113,6 +115,45 @@ internal sealed class NativeAbLegacyTagRuntime : IAbLegacyTagRuntime
|
||||
};
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The single authority for the writes this runtime refuses before the wire, or <see langword="null"/>
|
||||
/// when it attempts the write. <see cref="WriteCoreAsync"/> throws on it, and the driver asks it (via
|
||||
/// <see cref="NativeAbLegacyTagFactory.WriteRefusal"/>) before creating a runtime, so a refused write never
|
||||
/// costs the init read either (Gitea #593 follow-up).
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para><b>PLC-5:</b> every write — no capture of the PLC-5 write frames exists.</para>
|
||||
/// <para><b>Status-bit sub-elements</b> (<c>T4:0.DN</c>, <c>C5:0.CU</c>, <c>R6:0.EN</c>…, whatever the tag's
|
||||
/// authored type): libplctag does a masked bit write (0xAB) here; there is no capture of it, and a
|
||||
/// read-modify-write would race the PLC updating TT/DN. Writing the word instead would clobber EN/TT/DN.</para>
|
||||
/// <para>An address this runtime cannot parse is NOT a write refusal: the runtime's own address check refuses
|
||||
/// every operation on it, read included, with the same <see cref="AbStatus.Unsupported"/>.</para>
|
||||
/// </remarks>
|
||||
/// <param name="p">The tag's create parameters (family + address).</param>
|
||||
/// <returns>The refusal reason, or <see langword="null"/>.</returns>
|
||||
internal static string? WriteRefusal(AbLegacyTagCreateParams p)
|
||||
{
|
||||
if (p.LibplctagPlcAttribute == "plc5") return "PLC-5 writes are not capture-verified.";
|
||||
return StatusBitOf(p.TagName) is not null
|
||||
? "Writes to a status-bit sub-element (e.g. T4:0.DN) are not supported by the native transport."
|
||||
: null;
|
||||
}
|
||||
|
||||
/// <summary>The control-word bit a status-bit sub-element address names, else null (word sub-elements,
|
||||
/// plain addresses and unparseable ones all yield null).</summary>
|
||||
private static int? StatusBitOf(string tagName)
|
||||
{
|
||||
if (AbLegacyAddress.TryParse(tagName) is not { SubElement: { } sub } a) return null;
|
||||
try
|
||||
{
|
||||
return PcccAddress.WithSubElementName(a.FileLetter, a.FileNumber, a.WordNumber, sub).Bit;
|
||||
}
|
||||
catch (AbTransportException)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
private static PcccAddress ToPcccAddress(string tagName)
|
||||
{
|
||||
var a = AbLegacyAddress.TryParse(tagName)
|
||||
@@ -218,14 +259,9 @@ internal sealed class NativeAbLegacyTagRuntime : IAbLegacyTagRuntime
|
||||
|
||||
private async Task WriteCoreAsync(CancellationToken ct)
|
||||
{
|
||||
if (_plc == "plc5")
|
||||
throw new AbTransportException(AbStatus.Unsupported, "PLC-5 writes are not capture-verified.");
|
||||
// libplctag does a masked bit write (0xAB) here; there is no capture of it, and a read-modify-write
|
||||
// would race the PLC updating TT/DN. Writing the word instead would clobber EN/TT/DN, so refuse
|
||||
// before anything reaches the wire.
|
||||
if (StatusBit is not null)
|
||||
throw new AbTransportException(AbStatus.Unsupported,
|
||||
"Writes to a status-bit sub-element (e.g. T4:0.DN) are not supported by the native transport.");
|
||||
// Refuse before anything reaches the wire (see WriteRefusal for why each case is refused).
|
||||
if (_writeRefusal is not null)
|
||||
throw new AbTransportException(AbStatus.Unsupported, _writeRefusal);
|
||||
var (chunk, address) = Plan();
|
||||
for (var done = 0; done < _count; done += chunk)
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user