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

fix(abcip): native runtime write guards, same-width integer exemption, deferred path encoding (Task 12 review r1)

Browse Source
This commit is contained in:
Joseph Doherty
2026-10-07 16:56:30 -04:00
parent 25cc836865
commit e2e441d45e
2 changed files with 159 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagRuntime.cs
+62 -10
View File
@@ -28,10 +28,13 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
private readonly AbCipTagCreateParams _p;
private readonly DeviceSessionPool? _pool;
private readonly int? _implicitBit;
private readonly byte[] _path;
private readonly ushort _elements;
// Encoded in InitializeAsync, never in the constructor: a malformed name must surface from
// Initialize (inside the driver's / probe's try), not from IAbCipTagFactory.Create — as libplctag.
private int? _implicitBit;
private byte[]? _path;
private ConnectionLease? _lease;
private RequestScheduler? _privateScheduler; // ForTest only — production always uses the lease's
private LogixTagClient? _client;
@@ -57,9 +60,6 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
private NativeAbCipTagRuntime(AbCipTagCreateParams p)
{
_p = p ?? throw new ArgumentNullException(nameof(p));
var (name, bit) = CipPath.SplitBitSelector(p.TagName);
_implicitBit = bit;
_path = CipPath.EncodeSymbolic(name);
_elements = (ushort)Math.Clamp(p.ElementCount, 1, ushort.MaxValue);
}
@@ -97,6 +97,14 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
public async Task InitializeAsync(CancellationToken cancellationToken)
{
ObjectDisposedException.ThrowIf(_disposed, this);
if (_path is null)
{
// Before any lease: a malformed name costs no connection.
var (name, bit) = CipPath.SplitBitSelector(_p.TagName);
var path = CipPath.EncodeSymbolic(name);
_implicitBit = bit;
_path = path;
}
if (_client is null)
{
var lease = _pool!.Acquire(NativeAbCipDevice.Key(_p), NativeAbCipDevice.Options(_p));
@@ -111,9 +119,10 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
public async Task ReadAsync(CancellationToken cancellationToken)
{
var client = Client();
var path = Path();
try
{
var (type, handle, data) = await client.ReadAsync(_path, _elements, cancellationToken).ConfigureAwait(false);
var (type, handle, data) = await client.ReadAsync(path,_elements, cancellationToken).ConfigureAwait(false);
_data = data;
_typeCode = type;
_structHandle = handle;
@@ -131,6 +140,15 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
public async Task WriteAsync(CancellationToken cancellationToken)
{
var client = Client();
var path = Path();
// A bit tag ("Flags.3") names one bit of its host word. Writing any encoded value to it would
// write the WHOLE host and wipe its other bits on the controller; BOOL bit writes are an RMW the
// driver performs on a parent-DINT runtime (WriteBitInDIntAsync), so anything reaching here is refused.
if (_implicitBit is int implicitBit)
throw new NotSupportedException(
$"Write to bit tag '{_p.TagName}' (bit {implicitBit}) is not supported here — bit writes go through AbCipDriver.WriteBitInDIntAsync.");
try
{
if (_pendingString is { } text)
@@ -144,7 +162,7 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
throw new AbTransportException(AbStatus.Unsupported,
$"STRING write to '{_p.TagName}': the controller returned no structure handle.");
var payload = EncodeStringPayload(text);
await client.WriteAsync(_path, _typeCode!.Value, _structHandle, 1, payload, cancellationToken)
await client.WriteAsync(path, _typeCode!.Value, _structHandle, 1, payload, cancellationToken)
.ConfigureAwait(false);
}
else if (_writeBuffer is { } buffer)
@@ -152,11 +170,25 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
var type = _typeCode ?? AtomicCode(_p.AuthoredType)
?? throw new AbTransportException(AbStatus.Unsupported,
$"Write to '{_p.TagName}': type unknown — no prior read and no authored atomic type.");
// The read rule decides first: a same-width mismatch (REAL under a DINT-authored tag)
// would otherwise send the int bit pattern as a REAL and report Good.
if (_typeCode is { } returned && IsTypeMismatch(returned))
throw new AbTransportException(AbStatus.TypeMismatch,
$"Write to '{_p.TagName}': the controller's type 0x{returned:X4} does not match the authored {_p.AuthoredType}.");
// Width is checked only for codes with a known width: DATE / DATE_AND_TIME (0xCD/0xCF) and
// Micro800 BYTE/WORD/LWORD (0xD1/0xD2/0xD4) are outside LogixTypeCode's table and write as today.
var width = LogixTypeCode.ByteWidth(type);
if (width != buffer.Length)
if (width > 0 && width != buffer.Length)
{
// An authored BOOL over a packed BOOL-array DWORD: writing the DWORD would clobber the
// 31 sibling bools (libplctag does exactly that), so refuse it as a config error.
if (type == LogixTypeCode.BoolArray)
throw new NotSupportedException(
$"Write to '{_p.TagName}': a BOOL-array element (type 0xD3 DWORD) cannot be written as a single BOOL.");
throw new AbTransportException(AbStatus.TypeMismatch,
$"Write to '{_p.TagName}': encoded {buffer.Length} byte(s) but the controller's type 0x{type:X4} is {width} byte(s) wide.");
await client.WriteAsync(_path, type, null, 1, buffer, cancellationToken).ConfigureAwait(false);
}
await client.WriteAsync(path, type, null, 1, buffer, cancellationToken).ConfigureAwait(false);
}
else
{
@@ -324,6 +356,9 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
_privateScheduler = null;
}
private byte[] Path() =>
_path ?? throw new InvalidOperationException("InitializeAsync has not been called.");
private LogixTagClient Client()
{
ObjectDisposedException.ThrowIf(_disposed, this);
@@ -334,7 +369,10 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
/// True when the controller returned an atomic type other than the authored one. Exempt:
/// no authored type, String/Structure, a non-atomic return, a BOOL bit selector against an integer
/// host (<c>Flags.3</c> on a DINT), and an authored BOOL answered as 0xD3 (a packed BOOL-array
/// DWORD, which is how Logix returns a BOOL-array element).
/// DWORD, which is how Logix returns a BOOL-array element), and same-width integers that differ only
/// in signedness or are the Micro800 bit-string type of that width (SINT/USINT/BYTE, INT/UINT/WORD,
/// DINT/UDINT/DWORD, LINT/ULINT/LWORD): libplctag reads those Good today and a transport cutover
/// must not turn them Bad. Different widths and integer↔real still mismatch.
/// </summary>
private bool IsTypeMismatch(ushort returned)
{
@@ -345,9 +383,23 @@ internal sealed class NativeAbCipTagRuntime : IAbCipTagRuntime
if (returned == LogixTypeCode.BoolArray) return false;
if (_implicitBit is not null && IsIntegerCode(returned)) return false;
}
if (IntegerWidth(expected) is int width && IntegerWidth(returned) == width) return false;
return true;
}
/// <summary>Micro800 bit-string type codes (DWORD is <see cref="LogixTypeCode.BoolArray"/>, 0xD3).</summary>
private const ushort ByteCode = 0xD1, WordCode = 0xD2, LWordCode = 0xD4;
/// <summary>Width of an integer or bit-string code; null for anything else (BOOL, REAL, ...).</summary>
private static int? IntegerWidth(ushort code) => code switch
{
LogixTypeCode.SInt or LogixTypeCode.USInt or ByteCode => 1,
LogixTypeCode.Int or LogixTypeCode.UInt or WordCode => 2,
LogixTypeCode.DInt or LogixTypeCode.UDInt or LogixTypeCode.BoolArray => 4,
LogixTypeCode.LInt or LogixTypeCode.ULInt or LWordCode => 8,
_ => null,
};
/// <summary>The Logix type code an authored type expects, or null when it is exempt from checking.</summary>
private static ushort? AtomicCode(AbCipDataType? type) => type switch
{
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagRuntimeTests.cs
+97
View File
@@ -248,6 +248,103 @@ public sealed class NativeAbCipTagRuntimeTests
ch.SentCount.ShouldBe(sentBefore);
}
[Fact]
public async Task A_Dt_write_under_DATE_type_0xCD_sends_four_bytes()
{
// 0xCD is outside LogixTypeCode's width table: no width check, the write goes out as today.
var ch = Replying("CD00", "00000000");
using var rt = await ReadAsync(ch, Params("D", AbCipDataType.Dt));
rt.GetStatus().ShouldBe((int)AbStatus.Ok);
ch.Respond = req => Task.FromResult(FakeCipChannel.OkReply(req));
rt.EncodeValue(AbCipDataType.Dt, null, 123456);
await rt.WriteAsync(CancellationToken.None);
ch.Sent[^1].ShouldBe(Convert.FromHexString("4D02" + "91014400" + "CD00" + "0100" + "40E20100"));
}
[Fact]
public async Task A_BOOL_write_over_a_packed_BOOL_array_DWORD_is_NotSupported_and_sends_nothing()
{
var ch = Replying("D300", "01000000");
using var rt = await ReadAsync(ch, Params("Bits[0]", AbCipDataType.Bool));
var sentBefore = ch.SentCount;
rt.EncodeValue(AbCipDataType.Bool, null, false);
await Should.ThrowAsync<NotSupportedException>(() => rt.WriteAsync(CancellationToken.None));
ch.SentCount.ShouldBe(sentBefore);
}
[Theory]
[InlineData(AbCipDataType.DInt)]
[InlineData(AbCipDataType.Int)]
[InlineData(AbCipDataType.Bool)]
public async Task Any_write_to_a_bit_tag_is_NotSupported_and_sends_nothing(AbCipDataType authored)
{
// "Flags.3" authored as DInt/Int would otherwise write the value over the WHOLE host DINT.
var ch = Replying("C400", "08000000");
using var rt = await ReadAsync(ch, Params("Flags.3", authored));
var sentBefore = ch.SentCount;
rt.EncodeValue(authored, null, 1);
await Should.ThrowAsync<NotSupportedException>(() => rt.WriteAsync(CancellationToken.None));
ch.SentCount.ShouldBe(sentBefore);
}
[Fact]
public async Task A_write_after_a_same_width_type_mismatch_is_TypeMismatch_and_sends_nothing()
{
// REAL returned for a DINT-authored tag: same width, so only the read rule can catch it.
var ch = Replying("CA00", "0000803F");
using var rt = await ReadAsync(ch, Params("Speed", AbCipDataType.DInt));
var sentBefore = ch.SentCount;
rt.EncodeValue(AbCipDataType.DInt, null, 42);
(await Should.ThrowAsync<AbTransportException>(() => rt.WriteAsync(CancellationToken.None)))
.Status.ShouldBe(AbStatus.TypeMismatch);
ch.SentCount.ShouldBe(sentBefore);
rt.GetStatus().ShouldBe((int)AbStatus.TypeMismatch);
}
[Theory]
[InlineData(AbCipDataType.DInt, "C800", "2A000000", 42)] // DINT ← UDINT
[InlineData(AbCipDataType.UDInt, "C400", "2A000000", 42u)] // UDINT ← DINT
[InlineData(AbCipDataType.Int, "C700", "2A00", 42)] // INT ← UINT
[InlineData(AbCipDataType.UInt, "C300", "2A00", 42)] // UINT ← INT
[InlineData(AbCipDataType.SInt, "C600", "2A", 42)] // SINT ← USINT
[InlineData(AbCipDataType.USInt, "C200", "2A", 42)] // USINT ← SINT
[InlineData(AbCipDataType.LInt, "C900", "2A00000000000000", 42L)] // LINT ← ULINT
[InlineData(AbCipDataType.ULInt, "C500", "2A00000000000000", 42UL)] // ULINT ← LINT
[InlineData(AbCipDataType.DInt, "D300", "2A000000", 42)] // DINT ← Micro800 DWORD
[InlineData(AbCipDataType.UDInt, "D300", "2A000000", 42u)] // UDINT ← Micro800 DWORD
[InlineData(AbCipDataType.Int, "D200", "2A00", 42)] // INT ← Micro800 WORD
[InlineData(AbCipDataType.USInt, "D100", "2A", 42)] // USINT ← Micro800 BYTE
[InlineData(AbCipDataType.LInt, "D400", "2A00000000000000", 42L)] // LINT ← Micro800 LWORD
public async Task Same_width_integer_pairs_are_not_a_type_mismatch(
AbCipDataType authored, string typeHex, string dataHex, object expected)
{
using var rt = await ReadAsync(Replying(typeHex, dataHex), Params("T", authored));
rt.GetStatus().ShouldBe((int)AbStatus.Ok);
rt.DecodeValue(authored, null).ShouldBe(expected);
}
[Theory]
[InlineData(AbCipDataType.Int, "C400", "2A000000")] // different width
[InlineData(AbCipDataType.Real, "D300", "2A000000")] // integer ↔ real, same width
[InlineData(AbCipDataType.UDInt, "CA00", "0000803F")] // real ↔ integer, same width
public async Task Different_widths_and_integer_real_pairs_still_mismatch(
AbCipDataType authored, string typeHex, string dataHex)
{
using var rt = await ReadAsync(Replying(typeHex, dataHex), Params("T", authored));
rt.GetStatus().ShouldBe((int)AbStatus.TypeMismatch);
}
[Fact]
public async Task A_malformed_tag_name_never_throws_from_Create_and_surfaces_on_Initialize_without_IO()
{
var ch = new FakeCipChannel();
var p = Params("Arr[abc]", AbCipDataType.DInt);
using var created = new NativeAbCipTagFactory().Create(p); // must not throw
using var rt = NativeAbCipTagRuntime.ForTest(ch, p);
await Should.ThrowAsync<Exception>(() => rt.InitializeAsync(CancellationToken.None));
ch.SentCount.ShouldBe(0);
}
[Fact]
public async Task A_refused_write_stores_the_status_and_rethrows()
{