diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyEquipmentTagParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyEquipmentTagParser.cs index ad9beea0..2c31de77 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyEquipmentTagParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Contracts/AbLegacyEquipmentTagParser.cs @@ -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; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs index d6fce5db..b219d86a 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -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 } } + /// + /// Phase 4c #137 — whether a tag definition is an ARRAY. The canonical contract: a tag is + /// an array ⟺ its 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). null ArrayLength ⇒ scalar. + /// + private static bool IsArrayTag(AbLegacyTagDefinition def) => def.ArrayLength is int len && len >= 1; + /// /// Phase 4c #137 — the effective libplctag element count for a tag definition: the tag's /// clamped to the PCCC file maximum - /// ( = 256), or 1 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. + /// ( = 256) when it is an array (≥1, INCLUDING 1), + /// or 1 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. /// 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 EnsureTagRuntimeAsync( DeviceState device, AbLegacyTagDefinition def, CancellationToken ct) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs index 548013f7..46fd45de 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs @@ -69,12 +69,30 @@ internal sealed class LibplctagLegacyTagRuntime : IAbLegacyTagRuntime /// 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: diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyArrayTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyArrayTests.cs index f41908f8..3123f85f 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyArrayTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyArrayTests.cs @@ -138,18 +138,24 @@ public sealed class AbLegacyArrayTests factory.Tags["N7:0"].CreationParams.ElementCount.ShouldBe(1); } - /// An ArrayLength of 1 behaves like a scalar (single value, ElementCount 1). + /// + /// A 1-element array (ArrayLength: 1) reads as a 1-element typed array, NOT a scalar. + /// The foundation materialises a [1] OPC UA array node for isArray:true, + /// arrayLength:1, so the driver read must return a single-element array to match + /// (review I-3). ElementCount stays 1. + /// [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(); + 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); } + /// + /// I-3: a 1-element array tag (ArrayLength: 1) discovers as IsArray=true / ArrayDim=1 — + /// a [1] OPC UA array node, not a scalar. + /// + [Fact] + public async Task Discovery_surfaces_one_element_array_as_IsArray_with_dim_one() + { + var captured = new List(); + 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); + } + /// Scalar tag discovery keeps IsArray false / ArrayDim null (regression guard). [Fact] public async Task Discovery_keeps_scalar_tag_non_array() @@ -216,11 +240,11 @@ public sealed class AbLegacyArrayTests def!.ArrayLength.ShouldBe(10); } - /// The parser caps arrayLength at the PCCC 256-word file maximum. + /// The parser caps arrayLength at the PCCC 256-word file maximum (isArray:true). [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(); } + /// + /// C-2 regression: an arrayLength WITHOUT isArray:true is a SCALAR. The + /// canonical contract gates an array on isArray:true, so a stale length behind a + /// cleared / absent isArray must leave ArrayLength null and never produce an orphan array. + /// + [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(); + } + + /// C-2 regression: isArray:false with a positive arrayLength is a SCALAR. + [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(); + } + + /// + /// I-3: isArray:true, arrayLength:1 is a valid 1-element array — ArrayLength is 1 + /// (not null), so the foundation materialises a [1] node and the driver reads it + /// as a 1-element array. + /// + [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); + } + /// /// End-to-end: an AbLegacy driver with NO authored tags reads an equipment-tag ref whose - /// TagConfig carries arrayLength — the resolver threads the count and the read - /// surfaces a typed array, capping the libplctag element count at 256. + /// TagConfig carries isArray:true + arrayLength — the resolver threads the + /// count and the read surfaces a typed array, capping the libplctag element count at 256. /// [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); } + /// + /// C-2 end-to-end: an equipment-tag ref carrying isArray:false, arrayLength:8 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. + /// + [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(); + factory.Tags["N7:0"].CreationParams.ElementCount.ShouldBe(1); + } + private sealed class RecordingBuilder(List captured) : IAddressSpaceBuilder { public IAddressSpaceBuilder Folder(string browseName, string displayName) => this;