diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireClient.cs index 53a23d2c..fd76bf66 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireClient.cs @@ -1137,15 +1137,88 @@ public sealed class FocasWireClient : IAsyncDisposable, IDisposable /// The requested record count. /// The decoded alarms. /// The payload length matches neither record layout. + /// + /// Choosing the layout. The response block carries no record count, so a length that is a + /// multiple of BOTH strides (1,520 B = 20 × 76 = 19 × 80, and its multiples) is ambiguous; picking the + /// vendor stride on Length % 76 == 0 alone decoded 19 legacy records as 20 garbage ones. The choice + /// now uses what IS known: (1) the record count the REQUEST asked for — a reply never holds more, so a + /// layout implying more records than is ruled out; (2) each layout's own field + /// invariants, checked on every record ( / ). + /// Only when both readings remain plausible does the vendor layout win, as before. + /// internal static IReadOnlyList ParseAlarms(byte[] payload, short count) { - if (payload.Length % 76 == 0) return ParseVendorAlarms(payload, count); - if (payload.Length % 80 == 0) return ParseLegacyWireAlarms(payload, count); + if (payload.Length == 0) return []; + var vendor = payload.Length % VendorAlarmStride == 0 && payload.Length / VendorAlarmStride <= Math.Max((int)count, 1); + var legacy = payload.Length % LegacyAlarmStride == 0 && payload.Length / LegacyAlarmStride <= Math.Max((int)count, 1); + if (vendor && legacy) + { + // Ambiguous length: a legacy record's int32 type/axis/length fields must each fit the short they are + // read into and its length must fit the 64-byte message — a vendor payload read at the legacy stride + // fails that on its first non-empty message. Prefer the reading whose every record is self-consistent. + var legacyOk = LegacyRecordsValid(payload); + var vendorOk = VendorRecordsValid(payload); + if (legacyOk && !vendorOk) vendor = false; + else if (legacyOk && vendorOk && LengthsMatchText(payload, LegacyAlarmStride, 12, 4, 16) + && !LengthsMatchText(payload, VendorAlarmStride, 10, 2, 12)) vendor = false; + } + if (vendor) return ParseVendorAlarms(payload, count); + if (legacy) return ParseLegacyWireAlarms(payload, count); + if (payload.Length % VendorAlarmStride == 0 || payload.Length % LegacyAlarmStride == 0) + { + // Fits a stride but holds more records than were requested: fall back to the stride it fits, capped + // at the count, exactly as before this check existed. + return payload.Length % VendorAlarmStride == 0 + ? ParseVendorAlarms(payload, count) + : ParseLegacyWireAlarms(payload, count); + } throw new FocasWireException( $"cnc_rdalmmsg2 returned a {payload.Length}-byte payload, which fits neither the 76- nor the 80-byte " + "record layout; refusing to read it as an empty alarm list.", rc: null); } + private const int VendorAlarmStride = 76; + private const int LegacyAlarmStride = 80; + private const int AlarmMessageBytes = 64; + + /// Every 80-byte record's int32 type / axis / length fit a short, and the length fits the message. + private static bool LegacyRecordsValid(byte[] payload) + { + for (var o = 0; o + LegacyAlarmStride <= payload.Length; o += LegacyAlarmStride) + { + var type = ReadInt32(payload, o + 4); + var axis = ReadInt32(payload, o + 8); + var length = ReadInt32(payload, o + 12); + if (type is < short.MinValue or > short.MaxValue) return false; + if (axis is < short.MinValue or > short.MaxValue) return false; + if (length is < 0 or > AlarmMessageBytes) return false; + } + return true; + } + + /// Every 76-byte record's i16 message length fits the 64-byte message. + private static bool VendorRecordsValid(byte[] payload) + { + for (var o = 0; o + VendorAlarmStride <= payload.Length; o += VendorAlarmStride) + { + var length = ReadInt16(payload, o + 10); + if (length is < 0 or > AlarmMessageBytes) return false; + } + return true; + } + + /// Every record's declared message length equals the length of the text actually in its message field. + private static bool LengthsMatchText(byte[] payload, int stride, int lengthOffset, int lengthWidth, int messageOffset) + { + for (var o = 0; o + stride <= payload.Length; o += stride) + { + var length = lengthWidth == 4 ? ReadInt32(payload, o + lengthOffset) : ReadInt16(payload, o + lengthOffset); + var text = FocasWireProtocol.ReadAscii(payload.AsSpan(o + messageOffset, AlarmMessageBytes)); + if (length != text.Length) return false; + } + return true; + } + private static IReadOnlyList ParseVendorAlarms(byte[] payload, short count) { var alarms = new List(); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAlarmLayoutTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAlarmLayoutTests.cs new file mode 100644 index 00000000..bc1bc1b2 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAlarmLayoutTests.cs @@ -0,0 +1,96 @@ +using System.Buffers.Binary; +using System.Text; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Wire; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// #740 / R07:H20 — ParseAlarms picked the 76-byte vendor layout whenever Length % 76 == 0, so a +/// legacy 80-byte reply of exactly 19 records (1,520 B = 20 × 76) decoded as 20 garbage vendor records. The +/// response block carries no record count, so the layout is now chosen from the two things that are known: the +/// count the REQUEST asked for (a reply never holds more) and each layout's own field invariants. +/// +[Trait("Category", "Unit")] +public sealed class FocasAlarmLayoutTests +{ + private static byte[] Legacy(params (int no, int type, int axis, string msg)[] records) + { + var buf = new byte[records.Length * 80]; + for (var i = 0; i < records.Length; i++) + { + var o = i * 80; + var (no, type, axis, msg) = records[i]; + BinaryPrimitives.WriteInt32BigEndian(buf.AsSpan(o, 4), no); + BinaryPrimitives.WriteInt32BigEndian(buf.AsSpan(o + 4, 4), type); + BinaryPrimitives.WriteInt32BigEndian(buf.AsSpan(o + 8, 4), axis); + BinaryPrimitives.WriteInt32BigEndian(buf.AsSpan(o + 12, 4), msg.Length); + Encoding.ASCII.GetBytes(msg).CopyTo(buf.AsSpan(o + 16)); + } + return buf; + } + + private static byte[] Vendor(params (int no, short type, short axis, string msg)[] records) + { + var buf = new byte[records.Length * 76]; + for (var i = 0; i < records.Length; i++) + { + var o = i * 76; + var (no, type, axis, msg) = records[i]; + BinaryPrimitives.WriteInt32BigEndian(buf.AsSpan(o, 4), no); + BinaryPrimitives.WriteInt16BigEndian(buf.AsSpan(o + 4, 2), type); + BinaryPrimitives.WriteInt16BigEndian(buf.AsSpan(o + 6, 2), axis); + BinaryPrimitives.WriteInt16BigEndian(buf.AsSpan(o + 10, 2), (short)msg.Length); + Encoding.ASCII.GetBytes(msg).CopyTo(buf.AsSpan(o + 12)); + } + return buf; + } + + private static (int, int, int, string)[] LegacySet(int n) => + [.. Enumerable.Range(0, n).Select(i => (400 + i, 2 + (i % 3), 1 + (i % 4), $"SERVO ALARM {400 + i}"))]; + + private static (int, short, short, string)[] VendorSet(int n) => + [.. Enumerable.Range(0, n).Select(i => (1000 + i, (short)(4 + (i % 3)), (short)(i % 2), $"OVERTRAVEL +{i}"))]; + + /// The ambiguous length: 19 legacy records decode as 19 legacy records, not 20 vendor ones. + [Fact] + public void Nineteen_legacy_records_decode_as_legacy() + { + var payload = Legacy(LegacySet(19)); + payload.Length.ShouldBe(1520); + (payload.Length % 76).ShouldBe(0, "the length really is ambiguous"); + + var alarms = FocasWireClient.ParseAlarms(payload, 32); + + alarms.Count.ShouldBe(19); + alarms[0].AlarmNumber.ShouldBe(400); + alarms[0].Message.ShouldBe("SERVO ALARM 400"); + alarms[18].AlarmNumber.ShouldBe(418); + alarms[18].Axis.ShouldBe((short)(1 + (18 % 4))); + } + + /// Positive control at the same length: 20 vendor records still decode as vendor. + [Fact] + public void Twenty_vendor_records_decode_as_vendor() + { + var payload = Vendor(VendorSet(20)); + payload.Length.ShouldBe(1520); + + var alarms = FocasWireClient.ParseAlarms(payload, 32); + + alarms.Count.ShouldBe(20); + alarms[0].AlarmNumber.ShouldBe(1000); + alarms[19].Message.ShouldBe("OVERTRAVEL +19"); + } + + /// The unambiguous lengths are unchanged. + [Fact] + public void Unambiguous_lengths_are_unchanged() + { + FocasWireClient.ParseAlarms(Vendor(VendorSet(3)), 32).Count.ShouldBe(3); + FocasWireClient.ParseAlarms(Legacy(LegacySet(3)), 32).Count.ShouldBe(3); + FocasWireClient.ParseAlarms([], 32).ShouldBeEmpty(); + Should.Throw(() => FocasWireClient.ParseAlarms(new byte[77], 32)); + } +}