Merge hw4-udtwrite (#592/#611)

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe

# Conflicts:
#	docs/drivers/AbCip.md
This commit is contained in:
Joseph Doherty
2026-10-09 04:32:03 -04:00
12 changed files with 970 additions and 25 deletions
@@ -226,6 +226,17 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
_logger.LogWarning(message, args);
}
// Tags whose write-refusal Warning has already been logged (Gitea #611) — a client may retry forever.
private readonly System.Collections.Concurrent.ConcurrentDictionary<string, byte> _writeRefusalWarned =
new(StringComparer.OrdinalIgnoreCase);
/// <summary>Logs a write-refusal Warning the first time <paramref name="tagName"/> reports one on this instance.</summary>
private void WarnWriteRefusedOnce(string tagName, string message, params object?[] args)
{
if (_writeRefusalWarned.TryAdd(tagName, 0))
_logger.LogWarning(message, args);
}
// Tags whose malformed-name Warning has already been logged (Gitea #570) — polled forever, warned once.
private readonly System.Collections.Concurrent.ConcurrentDictionary<string, byte> _configErrorWarned =
new(StringComparer.OrdinalIgnoreCase);
@@ -1145,6 +1156,19 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
continue;
}
// Gitea #611 item 3 — whole-UDT writes are refused by DECISION, not deferred: a structure write
// overwrites every member of the controller's instance, hidden BOOL hosts and padding included,
// with whatever the client sent. Address the members instead (each is its own raw tag). Refused
// before a runtime exists, so nothing is read or sent; a refusal is not a fault — no health change.
if (def.DataType == AbCipDataType.Structure)
{
results[i] = new WriteResult(AbCipStatusMapper.BadNotSupported);
WarnWriteRefusedOnce(def.Name,
"AbCip write to {Tag} on device {Device} refused: whole-UDT writes are not supported — write its members (logged once per tag)",
w.FullReference, def.DeviceHostAddress);
continue;
}
try
{
var parsedPath = AbCipTagPath.TryParse(def.TagPath);
@@ -1170,18 +1194,43 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
var opLock = device.GetRuntimeLock(def.Name);
await opLock.WaitAsync(cancellationToken).ConfigureAwait(false);
int status;
uint? refusal;
int size;
try
{
runtime.EncodeValue(def.DataType, parsedPath?.BitIndex, w.Value);
await runtime.WriteAsync(cancellationToken).ConfigureAwait(false);
status = runtime.GetStatus();
// Gitea #611: the bytes the controller holds for this path must be what the authored type
// encodes, BEFORE anything is sent — on BOTH transports. libplctag never checks: it encodes
// into the buffer the last read sized and sends the whole buffer, so an INT authored over a
// DINT patched two bytes and wrote four (stale high word), and a BOOL over a packed
// BOOL-array DWORD wrote all 32 bits. Discovered UDT members are where a mis-authored type
// is most likely (browse-commit fills dataType from the template, an operator may retype).
size = runtime.GetSize();
refusal = WriteWidthRefusal(def, parsedPath?.BitIndex, size, runtimeTypesBool: runtime is NativeAbCipTagRuntime);
if (refusal is null)
{
runtime.EncodeValue(def.DataType, parsedPath?.BitIndex, w.Value);
await runtime.WriteAsync(cancellationToken).ConfigureAwait(false);
status = runtime.GetStatus();
}
else
{
status = 0;
}
}
finally
{
opLock.Release();
}
if (status == (int)AbStatus.TypeMismatch)
if (refusal is { } refused)
{
// Deterministic, nothing sent: keep the handle, leave health alone, warn once per tag.
results[i] = new WriteResult(refused);
WarnTypeMismatchOnce(def.Name,
"AbCip write to {Tag} on device {Device} refused before sending: the controller holds {Size} byte(s) there but the authored {DataType} encodes {Width} (logged once per tag)",
w.FullReference, def.DeviceHostAddress, size, def.DataType, ExpectedScalarWriteWidth(def.DataType));
}
else if (status == (int)AbStatus.TypeMismatch)
{
// Final review I4: deterministic — keep the handle, leave health, warn once per tag.
results[i] = new WriteResult(AbCipStatusMapper.BadTypeMismatch);
@@ -1210,12 +1259,14 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
}
catch (NotSupportedException nse)
{
// Type/protocol error — not a transport fault; don't evict the handle.
// A deterministic local refusal (a runtime that cannot encode this write without clobbering
// something — e.g. a BOOL-array element as a single BOOL) — not a transport fault: keep the
// handle and leave driver health alone (Gitea #611; same rule as the Unsupported refusal
// below). It used to latch Degraded, which a refusal the device never saw must not do.
results[i] = new WriteResult(AbCipStatusMapper.BadNotSupported);
_health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, nse.Message);
_logger.LogWarning(nse,
"AbCip write not supported for tag {Tag} on device {Device}",
w.FullReference, def.DeviceHostAddress);
WarnWriteRefusedOnce(def.Name,
"AbCip write not supported for tag {Tag} on device {Device}: {Reason} (logged once per tag)",
w.FullReference, def.DeviceHostAddress, nse.Message);
}
catch (FormatException fe)
{
@@ -1650,6 +1701,46 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
_ => null,
};
/// <summary>
/// Bytes one scalar write of <paramref name="type"/> encodes: the read-side width, plus BOOL (one byte —
/// a Logix BOOL tag or UDT BIT member reads back as one byte, type 0xC1). <c>null</c> where the encoded
/// size is not fixed (STRING, DT, structures).
/// </summary>
/// <param name="type">The authored type.</param>
/// <returns>The width in bytes, or <c>null</c>.</returns>
internal static int? ExpectedScalarWriteWidth(AbCipDataType type) =>
type == AbCipDataType.Bool ? 1 : ExpectedScalarWidth(type);
/// <summary>
/// Gitea #611 — the refusal status for a scalar write whose authored type does not fit what the controller
/// holds at that path (<paramref name="size"/> bytes, from the runtime's last read), or <c>null</c> to
/// write. A BOOL over a wider value (a packed BOOL-array DWORD) is <c>BadNotSupported</c> — writing it
/// would clobber the neighbouring bits, the same refusal the native runtime raises; any other width
/// mismatch is <c>BadTypeMismatch</c>. Arrays, bit selectors (<c>Flags.3</c>, served by the
/// read-modify-write path) and an unknown size (<c>&lt;= 0</c>) are not judged here.
/// <para>
/// A BOOL over a wider value is left to the runtime when it can see the CIP type
/// (<paramref name="runtimeTypesBool"/> — the native transport): it tells a packed BOOL-array DWORD
/// (<c>0xD3</c>, <c>BadNotSupported</c>) from a mistyped integer member (<c>BadTypeMismatch</c>, the
/// same verdict as its read), and refuses both before sending. libplctag exposes only the size, so the
/// two are indistinguishable there and both get the safe <c>BadNotSupported</c>.
/// </para>
/// </summary>
/// <param name="def">The tag being written.</param>
/// <param name="bitIndex">The parsed bit selector, if any.</param>
/// <param name="size">The runtime's read-buffer size in bytes.</param>
/// <param name="runtimeTypesBool">The runtime checks a BOOL write against the controller's CIP type itself.</param>
/// <returns>The refusal status, or <c>null</c>.</returns>
internal static uint? WriteWidthRefusal(AbCipTagDefinition def, int? bitIndex, int size, bool runtimeTypesBool = false)
{
if (IsArrayTag(def) || bitIndex is not null || size <= 0) return null;
if (ExpectedScalarWriteWidth(def.DataType) is not { } width || size == width) return null;
if (def.DataType == AbCipDataType.Bool && runtimeTypesBool) return null;
return def.DataType == AbCipDataType.Bool
? AbCipStatusMapper.BadNotSupported
: AbCipStatusMapper.BadTypeMismatch;
}
/// <summary>
/// Recursively fan a controller-discovered UDT shape out into Variable nodes — one per
/// ATOMIC leaf — under the folder produced by <paramref name="folder"/>. Mirrors the
@@ -277,9 +277,10 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime
_tag.SetInt32(0, Convert.ToInt32(value));
break;
case AbCipDataType.Structure:
// Whole-UDT writes are not implemented — operators address individual UDT
// members via dotted tag paths, which route per-member through the atomic
// encode cases above. A whole-UDT writer is a deferred follow-up.
// Whole-UDT writes are refused by decision (Gitea #611), not deferred: a structure
// write overwrites every member, hidden BOOL hosts and padding included. Members are
// written via their own dotted tag paths through the atomic cases above. The driver
// refuses a Structure tag before it ever reaches here; this is the backstop.
throw new NotSupportedException(
"Whole-UDT writes are not supported — address individual member paths instead.");
default: