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

fix(ablegacy): gate array read on isArray:true; 1-element arrays + assumption comments (review C-2/I-3)

Browse Source
This commit is contained in:
Joseph Doherty
2026-06-16 22:14:36 -04:00
parent 3bbe39c166
commit ce5d46be08
4 changed files with 162 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyEquipmentTagParser.cs
+17 -7
View File
@@ -29,15 +29,21 @@ public static class AbLegacyEquipmentTagParser
if (string.IsNullOrWhiteSpace(address)) return false;
var dataType = ReadEnum(root, "dataType", AbLegacyDataType.Int);
var deviceHostAddress = ReadString(root, "deviceHostAddress");
// Phase 4c #137 — thread the equipment tag's array element count. arrayLength is the
// authoritative count; isArray is the AdminUI's boolean toggle. A positive arrayLength
// (regardless of isArray) makes this an array tag. Clamp to the PCCC file maximum
// Phase 4c #137 — thread the equipment tag's array element count. The canonical
// foundation contract: a tag is an ARRAY ⟺ isArray:true. arrayLength (the element
// count, ≥1) is honoured ONLY when isArray is the JSON literal true, so a stale
// length behind a cleared / absent isArray never produces an orphan array tag that
// mismatches its scalar OPC UA node (review C-2). A 1-element array (isArray:true,
// arrayLength:1) is a valid [1] array. Clamp to the PCCC file maximum
// (AbLegacyArray.MaxElements = 256) so a fat-fingered count can never request a span
// larger than a single data file holds. Absent / non-positive → null (scalar).
// larger than a single data file holds. isArray:false / absent → null (scalar).
int? arrayLength = null;
var rawLength = ReadInt(root, "arrayLength");
if (rawLength > 0)
arrayLength = Math.Min(rawLength, AbLegacyArray.MaxElements);
if (IsArrayFlag(root))
{
var rawLength = ReadInt(root, "arrayLength");
if (rawLength >= 1)
arrayLength = Math.Min(rawLength, AbLegacyArray.MaxElements);
}
def = new AbLegacyTagDefinition(
Name: reference, DeviceHostAddress: deviceHostAddress, Address: address,
DataType: dataType, Writable: true, ArrayLength: arrayLength);
@@ -59,4 +65,8 @@ public static class AbLegacyEquipmentTagParser
private static int ReadInt(JsonElement o, string name)
=> o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.Number
&& e.TryGetInt32(out var v) ? v : 0;
// True only when the `isArray` property is the JSON literal true. Absent or false → scalar.
private static bool IsArrayFlag(JsonElement o)
=> o.TryGetProperty("isArray", out var e) && e.ValueKind == JsonValueKind.True;
}
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs
+27 -16
View File
@@ -267,15 +267,15 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
await runtime.ReadAsync(cancellationToken).ConfigureAwait(false);
status = runtime.GetStatus();
var parsed = AbLegacyAddress.TryParse(def.Address);
// Phase 4c #137 — when the tag addresses a multi-element span (ArrayLength > 1)
// decode the whole contiguous read into a typed CLR array; otherwise decode a
// single scalar value as before. The runtime was created with a matching
// ElementCount in EnsureTagRuntimeAsync so its buffer holds all the elements.
var arrayLen = EffectiveArrayLength(def);
// Phase 4c #137 — an ARRAY tag (non-null ArrayLength, ≥1) decodes the whole
// contiguous read into a typed CLR array of that count, INCLUDING a 1-element
// array (review I-3); a SCALAR tag (null ArrayLength) decodes a single value.
// The runtime was created with a matching ElementCount in EnsureTagRuntimeAsync
// so its buffer holds all the elements.
if (status != 0)
value = null;
else if (arrayLen > 1)
value = runtime.DecodeArray(def.DataType, arrayLen);
else if (IsArrayTag(def))
value = runtime.DecodeArray(def.DataType, EffectiveArrayLength(def));
else
value = runtime.DecodeValue(def.DataType, parsed?.BitIndex);
}
@@ -439,11 +439,14 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
foreach (var tag in tagsForDevice)
{
// Phase 4c #137 — PCCC data files are inherently arrays of elements (a single N7
// file is up to 256 words). A tag whose ArrayLength addresses a multi-element span
// now materialises a 1-D array OPC UA node. ArrayDim is clamped to the PCCC file
// maximum (AbLegacyArray.MaxElements = 256) so the declared dimension can never
// exceed what a single data file holds; an ArrayLength of 1 (or null) stays scalar.
var isArray = tag.ArrayLength is int len && len > 1;
// file is up to 256 words). The canonical contract: a tag is an ARRAY ⟺ its
// ArrayLength is non-null (≥1, set by the parser only when isArray:true). A tag
// with a non-null ArrayLength materialises a 1-D array OPC UA node, INCLUDING a
// 1-element array (ArrayLength:1 → a [1] node — review I-3). ArrayDim is clamped
// to the PCCC file maximum (AbLegacyArray.MaxElements = 256) so the declared
// dimension can never exceed what a single data file holds; ArrayLength == null
// (scalar) stays scalar.
var isArray = tag.ArrayLength is int len && len >= 1;
var arrayDim = isArray
? (uint)Math.Min(tag.ArrayLength!.Value, AbLegacyArray.MaxElements)
: (uint?)null;
@@ -685,15 +688,23 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
}
}
/// <summary>
/// Phase 4c #137 — whether a tag definition is an ARRAY. The canonical contract: a tag is
/// an array ⟺ its <see cref="AbLegacyTagDefinition.ArrayLength"/> is non-null (the parser
/// sets it ≥1 only when isArray:true), so a 1-element array (ArrayLength:1) IS an array
/// (review I-3). <c>null</c> ArrayLength ⇒ scalar.
/// </summary>
private static bool IsArrayTag(AbLegacyTagDefinition def) => def.ArrayLength is int len && len >= 1;
/// <summary>
/// Phase 4c #137 — the effective libplctag element count for a tag definition: the tag's
/// <see cref="AbLegacyTagDefinition.ArrayLength"/> clamped to the PCCC file maximum
/// (<see cref="AbLegacyArray.MaxElements"/> = 256), or <c>1</c> when the tag is scalar
/// (null or non-positive ArrayLength). Used both to size the runtime at create time and to
/// decide whether the read path decodes a scalar or an array.
/// (<see cref="AbLegacyArray.MaxElements"/> = 256) when it is an array (≥1, INCLUDING 1),
/// or <c>1</c> when the tag is scalar (null ArrayLength). Used both to size the runtime at
/// create time and as the element count the read path decodes into an array.
/// </summary>
private static int EffectiveArrayLength(AbLegacyTagDefinition def) =>
def.ArrayLength is int len && len > 1 ? Math.Min(len, AbLegacyArray.MaxElements) : 1;
def.ArrayLength is int len && len >= 1 ? Math.Min(len, AbLegacyArray.MaxElements) : 1;
private async Task<IAbLegacyTagRuntime> EnsureTagRuntimeAsync(
DeviceState device, AbLegacyTagDefinition def, CancellationToken ct)
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs
+24 -6
View File
@@ -69,12 +69,30 @@ internal sealed class LibplctagLegacyTagRuntime : IAbLegacyTagRuntime
/// <inheritdoc />
public object? DecodeArray(AbLegacyDataType type, int count)
{
// Each element is read from its byte offset within the contiguous tag buffer. PCCC word
// files (N/A → Int) are 2 bytes/element; L (Long) and F (Float) are 4 bytes/element. Bit
// (B-file) arrays read individual bits by index — libplctag's ab_pccc layer exposes the
// file's bits via GetBit(bitOffset). These element sizes are the canonical PCCC element
// widths; ASSUMPTION (not live-proven): libplctag packs multi-element reads contiguously
// with no per-element padding, matching the AbCip sibling's offset-decode pattern.
// The 5 libplctag-PCCC array-read ASSUMPTIONS this decode relies on (none live-proven —
// no PCCC fixture on this build host; they match the AbCip sibling's offset-decode pattern
// and the libplctag.NET API surface):
//
// 1. ELEMENT ADDRESSING — each of the `count` elements is read from its own byte offset
// `i * elementSize` within the single contiguous tag buffer that the prior ReadAsync
// filled (the tag was created with ElementCount = count, so one PCCC transaction
// fetched the whole span from the base address, e.g. N7:0 reading N7:0..N7:count-1).
// 2. FILE/ELEMENT vs SUB-ELEMENT — these are whole-element (file/element) reads, NOT
// sub-element reads. The array tag addresses a span of file elements (N7:0, N7:1, …),
// so there is no /N bit suffix or T/C/R sub-element field in play on this path; the
// bit-within-word and structured-element paths are handled elsewhere as scalars.
// 3. ELEMENT WIDTHS — the canonical PCCC element widths drive the offset stride:
// • N / A (integer word files → Int) → 2 bytes/element (GetInt16 at i*2)
// • L (long) and F (float) → 4 bytes/element (GetInt32/GetFloat32 at i*4)
// • B (bit file) → bit-packed: element i is the i-th bit,
// read by index via libplctag's GetBit(i) (NOT a byte offset).
// 4. CONTIGUOUS LAYOUT — libplctag packs the multi-element read contiguously with no
// per-element padding, so the `i * elementSize` stride (and GetBit(i) for B-files)
// lands exactly on element i.
// 5. UNSUPPORTED ELEMENT TYPES — String (ST-file) and Timer / Counter / Control
// sub-element structures do NOT lay out as a flat scalar array (variable-width /
// structured), so array reads of them are rejected in the `default` arm below; the
// driver also filters them before reaching here.
switch (type)
{
case AbLegacyDataType.Int:
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyArrayTests.cs
+94 -9
View File
@@ -138,18 +138,24 @@ public sealed class AbLegacyArrayTests
factory.Tags["N7:0"].CreationParams.ElementCount.ShouldBe(1);
}
/// <summary>An ArrayLength of 1 behaves like a scalar (single value, ElementCount 1).</summary>
/// <summary>
/// A 1-element array (ArrayLength: 1) reads as a 1-element typed array, NOT a scalar.
/// The foundation materialises a <c>[1]</c> OPC UA array node for <c>isArray:true,
/// arrayLength:1</c>, so the driver read must return a single-element array to match
/// (review I-3). ElementCount stays 1.
/// </summary>
[Fact]
public async Task ArrayLength_of_one_behaves_like_scalar()
public async Task ArrayLength_of_one_reads_a_one_element_array()
{
var (drv, factory) = NewDriver(
new AbLegacyTagDefinition("Counter", "ab://10.0.0.5/1,0", "N7:0", AbLegacyDataType.Int, ArrayLength: 1));
await drv.InitializeAsync("{}", CancellationToken.None);
factory.Customise = p => new FakeAbLegacyTag(p) { Value = 7 };
factory.Customise = p => new FakeAbLegacyTag(p) { ArrayValue = new short[] { 7 } };
var snapshots = await drv.ReadAsync(["Counter"], CancellationToken.None);
snapshots.Single().Value.ShouldBe(7);
var arr = snapshots.Single().Value.ShouldBeOfType<short[]>();
arr.ShouldBe(new short[] { 7 });
factory.Tags["N7:0"].CreationParams.ElementCount.ShouldBe(1);
}
@@ -175,6 +181,24 @@ public sealed class AbLegacyArrayTests
captured[0].ArrayDim.ShouldBe(8u);
}
/// <summary>
/// I-3: a 1-element array tag (ArrayLength: 1) discovers as IsArray=true / ArrayDim=1 —
/// a <c>[1]</c> OPC UA array node, not a scalar.
/// </summary>
[Fact]
public async Task Discovery_surfaces_one_element_array_as_IsArray_with_dim_one()
{
var captured = new List<DriverAttributeInfo>();
var (drv, _) = NewDriver(
new AbLegacyTagDefinition("Single", "ab://10.0.0.5/1,0", "N7:0", AbLegacyDataType.Int, ArrayLength: 1));
await drv.InitializeAsync("{}", CancellationToken.None);
await drv.DiscoverAsync(new RecordingBuilder(captured), CancellationToken.None);
captured[0].IsArray.ShouldBeTrue();
captured[0].ArrayDim.ShouldBe(1u);
}
/// <summary>Scalar tag discovery keeps IsArray false / ArrayDim null (regression guard).</summary>
[Fact]
public async Task Discovery_keeps_scalar_tag_non_array()
@@ -216,11 +240,11 @@ public sealed class AbLegacyArrayTests
def!.ArrayLength.ShouldBe(10);
}
/// <summary>The parser caps <c>arrayLength</c> at the PCCC 256-word file maximum.</summary>
/// <summary>The parser caps <c>arrayLength</c> at the PCCC 256-word file maximum (isArray:true).</summary>
[Fact]
public void EquipmentTagParser_caps_arrayLength_at_256()
{
var json = """{"deviceHostAddress":"ab://10.0.0.5/1,0","address":"N7:0","dataType":"Int","arrayLength":99999}""";
var json = """{"deviceHostAddress":"ab://10.0.0.5/1,0","address":"N7:0","dataType":"Int","isArray":true,"arrayLength":99999}""";
AbLegacyEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue();
def!.ArrayLength.ShouldBe(256);
}
@@ -234,15 +258,50 @@ public sealed class AbLegacyArrayTests
def!.ArrayLength.ShouldBeNull();
}
/// <summary>
/// C-2 regression: an <c>arrayLength</c> WITHOUT <c>isArray:true</c> is a SCALAR. The
/// canonical contract gates an array on <c>isArray:true</c>, so a stale length behind a
/// cleared / absent isArray must leave ArrayLength null and never produce an orphan array.
/// </summary>
[Fact]
public void EquipmentTagParser_arrayLength_without_isArray_is_scalar()
{
var json = """{"deviceHostAddress":"ab://10.0.0.5/1,0","address":"N7:0","dataType":"Int","arrayLength":8}""";
AbLegacyEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue();
def!.ArrayLength.ShouldBeNull();
}
/// <summary>C-2 regression: <c>isArray:false</c> with a positive arrayLength is a SCALAR.</summary>
[Fact]
public void EquipmentTagParser_isArray_false_with_arrayLength_is_scalar()
{
var json = """{"deviceHostAddress":"ab://10.0.0.5/1,0","address":"N7:0","dataType":"Int","isArray":false,"arrayLength":8}""";
AbLegacyEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue();
def!.ArrayLength.ShouldBeNull();
}
/// <summary>
/// I-3: <c>isArray:true, arrayLength:1</c> is a valid 1-element array — ArrayLength is 1
/// (not null), so the foundation materialises a <c>[1]</c> node and the driver reads it
/// as a 1-element array.
/// </summary>
[Fact]
public void EquipmentTagParser_isArray_true_arrayLength_one_is_one_element_array()
{
var json = """{"deviceHostAddress":"ab://10.0.0.5/1,0","address":"N7:0","dataType":"Int","isArray":true,"arrayLength":1}""";
AbLegacyEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue();
def!.ArrayLength.ShouldBe(1);
}
/// <summary>
/// End-to-end: an AbLegacy driver with NO authored tags reads an equipment-tag ref whose
/// TagConfig carries <c>arrayLength</c> — the resolver threads the count and the read
/// surfaces a typed array, capping the libplctag element count at 256.
/// TagConfig carries <c>isArray:true</c> + <c>arrayLength</c> — the resolver threads the
/// count and the read surfaces a typed array, capping the libplctag element count at 256.
/// </summary>
[Fact]
public async Task Driver_reads_equipment_array_ref_as_typed_array()
{
var json = """{"deviceHostAddress":"ab://10.0.0.5/1,0","address":"N7:0","dataType":"Int","arrayLength":3}""";
var json = """{"deviceHostAddress":"ab://10.0.0.5/1,0","address":"N7:0","dataType":"Int","isArray":true,"arrayLength":3}""";
var factory = new FakeAbLegacyTagFactory { Customise = p => new FakeAbLegacyTag(p) { ArrayValue = new short[] { 11, 22, 33 } } };
var drv = new AbLegacyDriver(new AbLegacyDriverOptions
{
@@ -260,6 +319,32 @@ public sealed class AbLegacyArrayTests
factory.Tags["N7:0"].CreationParams.ElementCount.ShouldBe(3);
}
/// <summary>
/// C-2 end-to-end: an equipment-tag ref carrying <c>isArray:false, arrayLength:8</c> reads
/// a single SCALAR value (the parser drops the orphan length), so the read never decodes a
/// phantom 8-element array against a scalar node. ElementCount stays 1.
/// </summary>
[Fact]
public async Task Driver_reads_isArray_false_with_arrayLength_as_scalar()
{
var json = """{"deviceHostAddress":"ab://10.0.0.5/1,0","address":"N7:0","dataType":"Int","isArray":false,"arrayLength":8}""";
var factory = new FakeAbLegacyTagFactory { Customise = p => new FakeAbLegacyTag(p) { Value = 99 } };
var drv = new AbLegacyDriver(new AbLegacyDriverOptions
{
Devices = [new AbLegacyDeviceOptions("ab://10.0.0.5/1,0")],
Tags = [],
Probe = new AbLegacyProbeOptions { Enabled = false },
}, "ablegacy-eq-scalar", factory);
await drv.InitializeAsync("{}", CancellationToken.None);
var r = await drv.ReadAsync([json], CancellationToken.None);
r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.Good);
r[0].Value.ShouldBe(99);
r[0].Value.ShouldNotBeOfType<short[]>();
factory.Tags["N7:0"].CreationParams.ElementCount.ShouldBe(1);
}
private sealed class RecordingBuilder(List<DriverAttributeInfo> captured) : IAddressSpaceBuilder
{
public IAddressSpaceBuilder Folder(string browseName, string displayName) => this;