fix(modbus): gate array read on isArray:true; 1-element arrays (review C-1)

This commit is contained in:
Joseph Doherty
2026-06-16 22:12:18 -04:00
parent 3e74239532
commit 49ac1392a8
2 changed files with 84 additions and 2 deletions
@@ -35,9 +35,13 @@ public static class ModbusEquipmentTagParser
var bitIndex = (byte)ReadInt(root, "bitIndex");
var stringLength = (ushort)ReadInt(root, "stringLength");
// isArray / arrayLength — optional keys authored by the typed Modbus tag editor.
// When arrayLength > 0 we expose an array tag of that count; otherwise scalar.
// Canonical rule: a tag is an array iff isArray:true AND arrayLength >= 1.
// isArray:false (with any arrayLength) is scalar — the foundation materialises a
// scalar OPC UA node, so returning an array value would cause a shape mismatch.
// A 1-element array (isArray:true, arrayLength:1) IS valid and reads as short[1].
var isArray = ReadBool(root, "isArray");
var arrayLength = ReadInt(root, "arrayLength");
int? arrayCount = arrayLength > 0 ? arrayLength : null;
int? arrayCount = (isArray && arrayLength >= 1) ? arrayLength : null;
def = new ModbusTagDefinition(
Name: reference, Region: region, Address: (ushort)address, DataType: dataType,
Writable: true, ByteOrder: byteOrder, BitIndex: bitIndex, StringLength: stringLength,
@@ -56,4 +60,7 @@ public static class ModbusEquipmentTagParser
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;
private static bool ReadBool(JsonElement o, string name)
=> o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.True;
}
@@ -332,6 +332,81 @@ public sealed class ModbusArrayTests
def!.ArrayCount.ShouldBeNull();
}
// ---- C-1 regression tests: isArray gate ----
/// <summary>
/// C-1 regression: <c>isArray:false</c> with a non-zero <c>arrayLength</c> must produce
/// <c>ArrayCount == null</c> (scalar), NOT an array. The foundation materialises a scalar
/// node when <c>isArray</c> is false; if the driver returned an array the value-shape
/// would mismatch and the OPC UA write would be rejected.
/// </summary>
[Fact]
public void Parser_IsArrayFalse_With_ArrayLength_Gives_Null_ArrayCount()
{
// isArray:false, arrayLength:8 → must be scalar (ArrayCount == null).
var json = """{"region":"HoldingRegisters","address":0,"dataType":"Int16","byteOrder":"BigEndian","bitIndex":0,"stringLength":0,"isArray":false,"arrayLength":8}""";
ModbusEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue();
def!.ArrayCount.ShouldBeNull();
}
/// <summary>
/// C-1 regression: <c>isArray:false</c> with <c>arrayLength:8</c> must read as a SCALAR,
/// not as an 8-element array.
/// </summary>
[Fact]
public async Task Equipment_Tag_IsArrayFalse_With_ArrayLength_Reads_As_Scalar()
{
// isArray:false, arrayLength:8 → scalar read; only register[40] is consumed.
var json = """{"region":"HoldingRegisters","address":40,"dataType":"Int16","byteOrder":"BigEndian","bitIndex":0,"stringLength":0,"isArray":false,"arrayLength":8}""";
var fake = new ModbusDriverTests.FakeTransport();
var opts = new ModbusDriverOptions { Host = "fake", Tags = [] };
var drv = new ModbusDriver(opts, "modbus-c1-scalar", _ => fake);
await drv.InitializeAsync("{}", CancellationToken.None);
fake.HoldingRegisters[40] = 9999;
var r = await drv.ReadAsync([json], CancellationToken.None);
r[0].StatusCode.ShouldBe(0u);
// Must be a scalar short, NOT a short[].
r[0].Value.ShouldBeOfType<short>();
r[0].Value.ShouldBe((short)9999);
}
/// <summary>
/// Verifies that <c>isArray:true, arrayLength:1</c> produces <c>ArrayCount == 1</c>
/// (a valid 1-element array — the canonical rule says arrayLength &gt;= 1 is valid).
/// </summary>
[Fact]
public void Parser_IsArrayTrue_ArrayLength1_Gives_ArrayCount_1()
{
var json = """{"region":"HoldingRegisters","address":0,"dataType":"Int16","byteOrder":"BigEndian","bitIndex":0,"stringLength":0,"isArray":true,"arrayLength":1}""";
ModbusEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue();
def!.ArrayCount.ShouldBe(1);
}
/// <summary>
/// Verifies that <c>isArray:true, arrayLength:1</c> reads as a 1-ELEMENT array (not a
/// scalar). The foundation materialises a <c>[1]</c> OPC UA array node for this tag.
/// </summary>
[Fact]
public async Task Equipment_Tag_IsArrayTrue_ArrayLength1_Reads_As_One_Element_Array()
{
var json = """{"region":"HoldingRegisters","address":50,"dataType":"Int16","byteOrder":"BigEndian","bitIndex":0,"stringLength":0,"isArray":true,"arrayLength":1}""";
var fake = new ModbusDriverTests.FakeTransport();
var opts = new ModbusDriverOptions { Host = "fake", Tags = [] };
var drv = new ModbusDriver(opts, "modbus-c1-arr1", _ => fake);
await drv.InitializeAsync("{}", CancellationToken.None);
fake.HoldingRegisters[50] = 777;
var r = await drv.ReadAsync([json], CancellationToken.None);
r[0].StatusCode.ShouldBe(0u);
// Must be a short[] of length 1, NOT a scalar short.
var arr = r[0].Value.ShouldBeOfType<short[]>();
arr.Length.ShouldBe(1);
arr[0].ShouldBe((short)777);
}
/// <summary>Recording address space builder for capturing discovered attributes.</summary>
/// <param name="captured">List to capture discovered attributes into.</param>
private sealed class RecordingBuilder(List<DriverAttributeInfo> captured) : IAddressSpaceBuilder