diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 36907a15..8afb0a38 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -799,6 +799,21 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, var tagPath = AbCipTagPath.TryParse(def.TagPath); var bitIndex = tagPath?.BitIndex; + + // libplctag never checks the CIP type the controller returned; it decodes the bytes as + // the authored type. A scalar whose buffer is not that type's width is mis-typed config + // (seen live: a REAL authored as INT read Good -17346), so refuse it rather than publish + // garbage as Good. Same-width mismatches (DINT vs REAL) cannot be seen this way. + if (!IsArrayTag(def) && bitIndex is null + && ExpectedScalarWidth(def.DataType) is { } width + && runtime.GetSize() is var size && size >= 0 && size != width) + { + results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadTypeMismatch, null, now); + _logger.LogWarning( + "AbCip read of {Tag} on device {Device} returned {Size} bytes but its authored type {DataType} is {Width} bytes wide; check the tag's dataType", + reference, def.DeviceHostAddress, size, def.DataType, width); + return ReadOutcome.Degraded($"type mismatch reading {reference}: {size} bytes for {def.DataType}"); + } // Review I-1 — an array tag (the EXPLICIT IsArray flag) decodes the whole buffer into an // element-typed CLR array (int[]/float[]/bool[]/string[]…), INCLUDING a 1-element array // (ElementCount 1). Scalar tags keep the single-value path. @@ -1367,6 +1382,21 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, memberName.StartsWith("ZZZZZZZZZZ", StringComparison.Ordinal) || memberName.StartsWith("__", StringComparison.Ordinal); + /// + /// Wire width of a scalar atomic Logix type, or null where the read buffer's size is not + /// a reliable type check (BOOL, STRING, DT, structures). + /// + /// The authored type. + /// The width in bytes, or null. + internal static int? ExpectedScalarWidth(AbCipDataType type) => type switch + { + AbCipDataType.SInt or AbCipDataType.USInt => 1, + AbCipDataType.Int or AbCipDataType.UInt => 2, + AbCipDataType.DInt or AbCipDataType.UDInt or AbCipDataType.Real => 4, + AbCipDataType.LInt or AbCipDataType.ULInt or AbCipDataType.LReal => 8, + _ => null, + }; + /// /// Recursively fan a controller-discovered UDT shape out into Variable nodes — one per /// ATOMIC leaf — under the folder produced by . Mirrors the diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs index 87a630be..d3cbafa8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs @@ -31,6 +31,13 @@ public interface IAbCipTagRuntime : IDisposable /// The raw libplctag status code (zero on success, negative on error). int GetStatus(); + /// + /// Bytes in the last read's buffer, or -1 when the runtime cannot say. Used to catch an + /// authored type whose width disagrees with what the controller actually returned. + /// + /// The buffer size in bytes, or -1 if unknown. + int GetSize() => -1; + /// /// Decode the local buffer into a boxed .NET value per the tag's configured type. /// is non-null only for BOOL-within-DINT tags captured in diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs index b11cbd49..b503dc03 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs @@ -46,6 +46,9 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime /// public int GetStatus() => (int)_tag.GetStatus(); + /// + public int GetSize() => _tag.GetSize(); + /// public object? DecodeValue(AbCipDataType type, int? bitIndex) => DecodeValueAt(type, 0, bitIndex); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipReadWidthMismatchTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipReadWidthMismatchTests.cs new file mode 100644 index 00000000..0e23496a --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipReadWidthMismatchTests.cs @@ -0,0 +1,72 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// libplctag hands back whatever bytes the controller sends and decodes them as the authored type +/// — it never checks the CIP type code. Seen live on a production press: WW_ActualVacuum is a +/// REAL (the L5X and the controller's symbol table agree), the legacy Kepware export types it as +/// Short, and reading it authored as INT returned Good -17346 (the low half of a float) +/// where the real value was 5.37. A scalar atomic read whose buffer is not the authored type's +/// width is now BadTypeMismatch. Same-width mismatches (DINT vs REAL) remain undetectable +/// without the type code. +/// +[Trait("Category", "Unit")] +public sealed class AbCipReadWidthMismatchTests +{ + private const string Device = "ab://10.0.0.5/1,0"; + + private static async Task ReadStatusAsync(AbCipDataType authored, int bufferSize, string path = "WW_ActualVacuum") + { + var factory = new FakeAbCipTagFactory + { + Customise = p => new FakeAbCipTag(p) { Value = 1, Size = bufferSize }, + }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("T", Device, path, authored)), + Probe = new AbCipProbeOptions { Enabled = false }, + }, "abcip-1", factory); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + var r = await drv.ReadAsync(["T"], TestContext.Current.CancellationToken); + return r[0].StatusCode; + } + + /// An INT read that comes back 4 bytes wide is a type mismatch, not a Good value. + [Fact] + public async Task INT_authored_over_a_4_byte_REAL_is_BadTypeMismatch() + { + (await ReadStatusAsync(AbCipDataType.Int, bufferSize: 4)).ShouldBe(AbCipStatusMapper.BadTypeMismatch); + } + + /// Matching widths still read Good — including the live-observed sizes per type. + /// Authored type. + /// Buffer size the controller returned for that type (observed live). + [Theory] + [InlineData(AbCipDataType.Int, 2)] + [InlineData(AbCipDataType.DInt, 4)] + [InlineData(AbCipDataType.Real, 4)] + [InlineData(AbCipDataType.LInt, 8)] + [InlineData(AbCipDataType.SInt, 1)] + public async Task Matching_width_reads_Good(AbCipDataType type, int size) + { + (await ReadStatusAsync(type, size)).ShouldBe(AbCipStatusMapper.Good); + } + + /// An unknown size (a runtime that cannot report it) never blocks a read. + [Fact] + public async Task Unknown_size_is_not_checked() + { + (await ReadStatusAsync(AbCipDataType.Int, bufferSize: -1)).ShouldBe(AbCipStatusMapper.Good); + } + + /// A bit-in-DINT read decodes from the parent's 4-byte buffer and is not width-checked. + [Fact] + public async Task Bit_index_read_is_not_width_checked() + { + (await ReadStatusAsync(AbCipDataType.Bool, bufferSize: 4, path: "Flags.3")).ShouldBe(AbCipStatusMapper.Good); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs index 7eb01fc5..3d8be6b3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs @@ -64,6 +64,12 @@ internal class FakeAbCipTag : IAbCipTagRuntime /// Returns the simulated status code. public virtual int GetStatus() => Status; + /// Gets or sets the simulated read-buffer size in bytes (-1 = unknown, the default). + public int Size { get; set; } = -1; + + /// Returns the simulated read-buffer size. + public virtual int GetSize() => Size; + /// Returns the mock tag value. /// The data type being decoded. /// The optional bit index for bit operations.