From 3b48e05e9a1cc19ce8612fcda3233717e9518944 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 7 Oct 2026 14:04:11 -0400 Subject: [PATCH] fix(abcip): decode real @udt template buffers; browse STRINGs as String, hide BOOL hosts Live read-only check against production CompactLogix 5069-L320ERS2 rev 34.11 presses (Z-35162 / 10.209.70.160): every @udt/ template the controller returned decoded to null, so controller-discovered UDTs never fanned out. The decoder assumed a 12-byte header led by the member count and [info][array][offset] blocks with ';'-terminated names; libplctag actually returns a 14-byte header (u16 id, u32 def words, u32 instance size, u16 member count, u16 handle), [info][type][offset] blocks, and NUL-terminated names after a 'Name;' type name. All three test builders restated the old assumption, so the unit suite agreed with itself (the #468 trap again); they now share a TemplateBufferBuilder pinned by real captures (CipTemplateObjectWireCaptureTests). Discovery follow-ons that the working decoder exposed: - STRING (and user string types, LEN DINT + DATA SINT[n]) browse as one String variable, top-level and nested, not as a LEN/DATA folder. - Controller-generated members (ZZZZZZZZZZ* BOOL hosts, __* AOI backing) are not browsed. - A UDT array is not fanned out into element-free Tag.Member paths. --- .../AbCipDriver.cs | 94 +++++++---- .../AbCipTemplateCache.cs | 14 +- .../CipTemplateObjectDecoder.cs | 153 ++++++++---------- .../AbCipDriverDiscoveryTests.cs | 135 ++++++++++++---- .../AbCipFetchUdtShapeTests.cs | 32 +--- .../CipTemplateObjectDecoderTests.cs | 81 +++------- .../CipTemplateObjectWireCaptureTests.cs | 98 +++++++++++ .../TemplateBufferBuilder.cs | 45 ++++++ 8 files changed, 428 insertions(+), 224 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/CipTemplateObjectWireCaptureTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/TemplateBufferBuilder.cs 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 931a7069..5c780907 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -1285,6 +1285,11 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, // bogus single Structure → String placeholder Variable. When the shape can't be // resolved (or yields no emittable leaf) we fall through to the legacy single // Variable so nothing regresses. + // Review I-1 — a discovered array of length 1 is still an array; honour the + // explicit IsArray flag (legacy ElementCount > 1 still surfaces as an array). + var discoveredIsArray = discovered.IsArray || discovered.ElementCount > 1; + var discoveredType = discovered.DataType; + if (discovered.DataType == AbCipDataType.Structure) { // Pass the discovered tag's CIP template instance id (surfaced by the Symbol @@ -1294,7 +1299,14 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, var shape = await ResolveDiscoveredUdtShapeAsync( device.HostAddress, discovered.Name, discovered.TemplateInstanceId, cancellationToken) .ConfigureAwait(false); - if (shape is not null) + if (shape is { IsStringFamily: true }) + { + // A controller reports every STRING (built-in or a user string type such as + // STR0016) as a LEN + DATA struct. It is one String value — the read path reads + // it as such — so it becomes one String Variable, never a LEN/DATA folder. + discoveredType = AbCipDataType.String; + } + else if (shape is not null && !discoveredIsArray) { // Materialise the member sub-folder LAZILY — only once a leaf is actually // about to be emitted into it. A shape that yields no usable atomic leaf @@ -1313,12 +1325,9 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } if (!emitted.Add(fullName)) continue; - // Review I-1 — a discovered array of length 1 is still an array; honour the - // explicit IsArray flag (legacy ElementCount > 1 still surfaces as an array). - var discoveredIsArray = discovered.IsArray || discovered.ElementCount > 1; discoveredFolder.Variable(fullName, discovered.Name, new DriverAttributeInfo( FullName: fullName, - DriverDataType: discovered.DataType.ToDriverDataType(), + DriverDataType: discoveredType.ToDriverDataType(), IsArray: discoveredIsArray, ArrayDim: discoveredIsArray ? (uint)Math.Max(1, discovered.ElementCount) : null, SecurityClass: discovered.ReadOnly @@ -1327,12 +1336,24 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, IsHistorized: false, IsAlarm: false, WriteIdempotent: false, - NativeDataType: discovered.DataType.ToString())); + NativeDataType: discoveredType.ToString())); } } } } + /// + /// true for a UDT member the controller generates rather than the user declares: the + /// hidden SINT/DINT hosts Logix packs BOOL members into (ZZZZZZZZZZAlarms0) and AOI + /// backing members (__BitHost00). A user-declared Logix name can never begin with two + /// underscores, so neither rule can hide a real member. + /// + /// The member name from the template. + /// true when the member is controller-generated. + internal static bool IsHiddenUdtMember(string memberName) => + memberName.StartsWith("ZZZZZZZZZZ", StringComparison.Ordinal) + || memberName.StartsWith("__", StringComparison.Ordinal); + /// /// Recursively fan a controller-discovered UDT shape out into Variable nodes — one per /// ATOMIC leaf — under the folder produced by . Mirrors the @@ -1381,37 +1402,50 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, var anyEmitted = false; foreach (var member in shape.Members) { + if (IsHiddenUdtMember(member.Name)) continue; var memberFullName = $"{parentFullName}.{member.Name}"; + var leafType = member.DataType; if (member.DataType == AbCipDataType.Structure) { - // Nested struct: recurse into its sub-shape if it resolves, the depth cap allows it, - // and we're not re-entering a struct already on this path (cycle guard). The nested - // members live a level deeper, so the cap is checked against depth + 1. - if (depth + 1 > MaxUdtDepth) continue; - if (!visited.Add(member.Name)) continue; // cyclic UDT reference — stop here - // The decoded parent shape carries the nested UDT's template instance id (low 12 bits of the - // member info, captured by CipTemplateObjectDecoder) — thread it so the nested shape is fetched - // off the controller via FetchUdtShapeAsync, making nested-struct members addressable in - // production (name-keyed seeded shapes still win first, for the test seam). + // Nested struct: resolve its sub-shape first. The decoded parent shape carries the nested + // UDT's template instance id (low 12 bits of the member type word, captured by + // CipTemplateObjectDecoder) — thread it so the nested shape is fetched off the controller + // via FetchUdtShapeAsync (name-keyed seeded shapes still win first, for the test seam). var nested = await ResolveDiscoveredUdtShapeAsync( deviceHostAddress, member.Name, templateInstanceId: member.NestedTemplateId, cancellationToken) .ConfigureAwait(false); - if (nested is not null) + + if (nested is { IsStringFamily: true }) { - // Materialise the nested sub-folder lazily under the parent folder (itself lazy), - // so a nested struct that yields no leaf leaves no empty sub-folder behind. - var parentFolder = folder; - var nestedFolder = new Lazy( - () => parentFolder.Value.Folder(memberFullName, member.Name)); - var nestedEmitted = await FanOutDiscoveredUdtMembersAsync( - nestedFolder, deviceHostAddress, memberFullName, nested, parentReadOnly, - depth + 1, emitted, visited, cancellationToken) - .ConfigureAwait(false); - anyEmitted |= nestedEmitted; + // A nested STRING (or user string type) member is one String leaf, not LEN + DATA. + leafType = AbCipDataType.String; + } + else + { + // An array of a UDT has no element-free member path (Parent.Arr.X names no element), + // so it is not recursed into. Otherwise recurse if the depth cap allows it and we're + // not re-entering a struct already on this path (cycle guard). The nested members + // live a level deeper, so the cap is checked against depth + 1. + if (member.ArrayLength > 1) continue; + if (depth + 1 > MaxUdtDepth) continue; + if (!visited.Add(member.Name)) continue; // cyclic UDT reference — stop here + if (nested is not null) + { + // Materialise the nested sub-folder lazily under the parent folder (itself lazy), + // so a nested struct that yields no leaf leaves no empty sub-folder behind. + var parentFolder = folder; + var nestedFolder = new Lazy( + () => parentFolder.Value.Folder(memberFullName, member.Name)); + var nestedEmitted = await FanOutDiscoveredUdtMembersAsync( + nestedFolder, deviceHostAddress, memberFullName, nested, parentReadOnly, + depth + 1, emitted, visited, cancellationToken) + .ConfigureAwait(false); + anyEmitted |= nestedEmitted; + } + visited.Remove(member.Name); + continue; } - visited.Remove(member.Name); - continue; } // Atomic leaf — emit one Variable. Dedup by full-name within the Discovered folder. @@ -1420,7 +1454,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, var memberIsArray = member.ArrayLength > 1; folder.Value.Variable(member.Name, member.Name, new DriverAttributeInfo( FullName: memberFullName, - DriverDataType: member.DataType.ToDriverDataType(), + DriverDataType: leafType.ToDriverDataType(), IsArray: memberIsArray, ArrayDim: memberIsArray ? (uint)Math.Max(1, member.ArrayLength) : null, // The Symbol Object carries no per-member ExternalAccess bits, so members inherit the @@ -1431,7 +1465,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, IsHistorized: false, IsAlarm: false, WriteIdempotent: false, - NativeDataType: member.DataType.ToString())); + NativeDataType: leafType.ToString())); anyEmitted = true; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTemplateCache.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTemplateCache.cs index 81cda88d..a6cb34ee 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTemplateCache.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTemplateCache.cs @@ -57,7 +57,19 @@ public sealed class AbCipTemplateCache public sealed record AbCipUdtShape( string TypeName, int TotalSize, - IReadOnlyList Members); + IReadOnlyList Members) +{ + /// + /// true when the shape is a Logix string-family type — the built-in STRING or + /// a user string type such as STR0016: exactly a LEN DINT at offset 0 and a + /// DATA SINT array at offset 4. A controller reports every string tag as a struct of + /// this shape, but it is read and presented as one String value, never as LEN + DATA leaves. + /// + public bool IsStringFamily => + Members.Count == 2 + && Members[0] is { Name: "LEN", DataType: AbCipDataType.DInt, Offset: 0 } + && Members[1] is { Name: "DATA", DataType: AbCipDataType.SInt, Offset: 4, ArrayLength: > 1 }; +} /// One member of a Logix UDT. /// Member name as reported by the Template Object. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/CipTemplateObjectDecoder.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/CipTemplateObjectDecoder.cs index 1bce8e91..7649ff48 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/CipTemplateObjectDecoder.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/CipTemplateObjectDecoder.cs @@ -4,103 +4,101 @@ using System.Text; namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// -/// Decoder for the CIP Template Object (class 0x6C) blob returned by a Read Template -/// service. Produces an describing the UDT's name, total size, -/// + ordered member list with per-member offset + type + array length. +/// Decoder for the UDT template buffer libplctag returns for an @udt/<id> tag (CIP +/// Template Object, class 0x6C). Produces an describing the UDT's +/// name, total size, + ordered member list with per-member offset + type + array length. /// /// -/// Wire format per Rockwell CIP Vol 1 §5A + Logix 5000 CIP Programming Manual -/// 1756-PM019 §"Template Object", cross-checked against libplctag's ab/cip.c -/// handle_read_template_reply: +/// The layout below is what libplctag hands back from Tag.GetBuffer() — its own +/// header followed by the controller's Read Template reply — matching libplctag's +/// list_tags_logix example and pinned by real captures in +/// CipTemplateObjectWireCaptureTests (CompactLogix 5069-L320ERS2 rev 34.11). An earlier +/// version assumed a 12-byte header led by the member count and [info][array][offset] +/// blocks; every real buffer decoded to null under that assumption. /// -/// Header (fixed-size, little-endian): +/// Header (14 bytes, little-endian): /// -/// u16Member count. -/// u16Struct handle (opaque id). +/// u16Template instance id. +/// u32Member-definition size in 32-bit words — not used here. /// u32Instance size — bytes per structure instance. -/// u32Member-definition total size — not used here. +/// u16Member count. +/// u16Struct handle (opaque). /// /// /// Then member_count member blocks (8 bytes each): /// -/// u16Member info — type code + flags (same encoding -/// as Symbol Object: bit 15 = struct, lower 12 = CIP type code). -/// u16Array size — 0 for scalar members. -/// u32Struct offset — byte offset from struct start. +/// u16Info — the array length for an array member, the bit +/// number for a BOOL packed into a host integer, otherwise 0. +/// u16Type — bit 15 = struct, bit 13 = array, low 12 bits = +/// CIP atomic type code, or for a struct the nested template instance id. +/// u32Byte offset from the struct start. /// /// -/// Then strings: UDT name followed by each member name, each terminated by a -/// semicolon ; followed by a null \0. The UDT name may itself contain the -/// sequence UDTName;0\0 where 0 after the semicolon is an ASCII flag byte. -/// Decoder trims to the first semicolon. +/// Then the strings: the UDT name terminated by ; (followed by an encoded tail that +/// runs to a NUL — discarded), then each member name terminated by a NUL. Anything after the +/// last member name is ignored. /// public static class CipTemplateObjectDecoder { - private const int HeaderSize = 12; // u16 + u16 + u32 + u32 - private const int MemberBlockSize = 8; // u16 + u16 + u32 + private const int HeaderSize = 14; // u16 id + u32 def size + u32 instance size + u16 count + u16 handle + private const int MemberBlockSize = 8; // u16 info + u16 type + u32 offset - private const ushort MemberInfoStructFlag = 0x8000; - private const ushort MemberInfoTypeCodeMask = 0x0FFF; + private const ushort TypeStructFlag = 0x8000; + private const ushort TypeArrayFlag = 0x2000; + private const ushort TypeCodeMask = 0x0FFF; /// - /// Decode the raw Template Object blob. Returns null when the header indicates - /// zero members or the buffer is too short to hold the fixed header. + /// Decode the raw template buffer. Returns null when the header indicates zero + /// members or the buffer is too short to hold the header + member blocks + type name. /// - /// The raw Template Object buffer to decode. + /// The raw template buffer to decode. /// An AbCipUdtShape describing the structure, or null if decoding fails. public static AbCipUdtShape? Decode(byte[] buffer) { ArgumentNullException.ThrowIfNull(buffer); if (buffer.Length < HeaderSize) return null; - var memberCount = BinaryPrimitives.ReadUInt16LittleEndian(buffer.AsSpan(0)); - // bytes 2-3: struct handle — opaque, not needed for the shape record - var instanceSize = BinaryPrimitives.ReadUInt32LittleEndian(buffer.AsSpan(4)); - // bytes 8-11: member-definition total size — inferred from names list instead + // bytes 0-1: template instance id; bytes 2-5: definition size in words — neither needed. + var instanceSize = BinaryPrimitives.ReadUInt32LittleEndian(buffer.AsSpan(6)); + var memberCount = BinaryPrimitives.ReadUInt16LittleEndian(buffer.AsSpan(10)); + // bytes 12-13: struct handle — opaque. if (memberCount == 0) return null; - var memberBlocksOffset = HeaderSize; - var namesOffset = memberBlocksOffset + MemberBlockSize * memberCount; + var namesOffset = HeaderSize + (MemberBlockSize * memberCount); if (namesOffset > buffer.Length) return null; - var stringsSpan = buffer.AsSpan(namesOffset); - var names = ParseSemicolonTerminatedStrings(stringsSpan); - if (names.Count == 0) return null; - - // Strings layout: UDT name first, then one per member (in the same order as the - // member-info blocks). Always consume the first entry as the UDT name; missing - // trailing member names get placeholders below. - var udtName = names[0]; - var memberNames = names.Skip(1).ToArray(); + var strings = ParseTemplateStrings(buffer.AsSpan(namesOffset), memberCount); + if (strings.Count == 0) return null; + var udtName = strings[0]; var members = new List(memberCount); for (var i = 0; i < memberCount; i++) { - var blockOffset = memberBlocksOffset + (i * MemberBlockSize); + var blockOffset = HeaderSize + (i * MemberBlockSize); var info = BinaryPrimitives.ReadUInt16LittleEndian(buffer.AsSpan(blockOffset)); - var arraySize = BinaryPrimitives.ReadUInt16LittleEndian(buffer.AsSpan(blockOffset + 2)); + var type = BinaryPrimitives.ReadUInt16LittleEndian(buffer.AsSpan(blockOffset + 2)); var offset = (int)BinaryPrimitives.ReadUInt32LittleEndian(buffer.AsSpan(blockOffset + 4)); - var isStruct = (info & MemberInfoStructFlag) != 0; - // Scalar path only — Rockwell primitive type codes (0xC1-0xD0) fit a byte. Do NOT use for a - // struct member: a nested template id can exceed a byte (see nestedTemplateId below). - var typeCode = (byte)(info & MemberInfoTypeCodeMask); + var isStruct = (type & TypeStructFlag) != 0; + var isArray = (type & TypeArrayFlag) != 0; + // Atomic CIP type codes (0xC1-0xD0) fit a byte; for a struct the low 12 bits are the nested + // template id, which can exceed a byte, so it is taken from the full 12-bit mask below. var dataType = isStruct ? AbCipDataType.Structure - : (CipSymbolObjectDecoder.MapTypeCode(typeCode) ?? AbCipDataType.Structure); + : (CipSymbolObjectDecoder.MapTypeCode((byte)(type & TypeCodeMask)) ?? AbCipDataType.Structure); + var nestedTemplateId = isStruct ? (uint?)(type & TypeCodeMask) : null; - // For a struct member the low 12 bits are the nested UDT's template instance id (same encoding as the - // Symbol Object), NOT a primitive type code — capture it so the nested shape can be fetched. Use the - // FULL 12-bit mask (not the byte-cast typeCode, which truncates a 12-bit id). - var nestedTemplateId = isStruct ? (uint?)(info & MemberInfoTypeCodeMask) : null; + // info is the element count only for an array member; for a BOOL it is the bit number, + // which must never be read as a length. + var arrayLength = isArray && info > 0 ? info : 1; - var memberName = i < memberNames.Length ? memberNames[i] : $""; + var memberName = i + 1 < strings.Count ? strings[i + 1] : $""; members.Add(new AbCipUdtMember( Name: memberName, Offset: offset, DataType: dataType, - ArrayLength: arraySize == 0 ? 1 : arraySize, + ArrayLength: arrayLength, NestedTemplateId: nestedTemplateId)); } @@ -111,41 +109,30 @@ public static class CipTemplateObjectDecoder } /// - /// Walk a span of NAME;\0NAME;\0… byte sequences. Splits at each semicolon — - /// the null byte after each semicolon is optional padding per Rockwell's string - /// encoding convention. Stops at a trailing null / end of buffer. + /// Split the template string section into the UDT name (cut at its first ;) followed + /// by up to NUL-terminated member names. Stops at the end of + /// the buffer; trailing bytes past the last member name are ignored. /// - /// The byte span containing semicolon-terminated strings. - /// List of parsed strings, one per name in the span. - internal static List ParseSemicolonTerminatedStrings(ReadOnlySpan span) + /// The string section, starting right after the last member block. + /// Member names to read after the UDT name. + /// The UDT name followed by the member names found. + internal static List ParseTemplateStrings(ReadOnlySpan span, int memberCount) { - var result = new List(); - var start = 0; - for (var i = 0; i < span.Length; i++) + var result = new List(memberCount + 1); + var pos = 0; + while (pos < span.Length && result.Count < memberCount + 1) { - var b = span[i]; - if (b == ';') + var nul = span[pos..].IndexOf((byte)0); + var end = nul < 0 ? span.Length : pos + nul; + var raw = span[pos..end]; + if (result.Count == 0) { - if (i > start) - result.Add(Encoding.ASCII.GetString(span[start..i])); - // Skip the optional null/space padding following the semicolon. - while (i + 1 < span.Length && (span[i + 1] == '\0' || span[i + 1] == ' ')) - i++; - start = i + 1; + // The type name carries an encoded tail after ';' ("STRING;nEAIA"). + var semi = raw.IndexOf((byte)';'); + if (semi >= 0) raw = raw[..semi]; } - else if (b == 0 && start == i) - { - // Trailing null at a string boundary — done. - break; - } - } - // Trailing name without a semicolon (unlikely but observed on some firmwares). - if (start < span.Length) - { - var zeroAt = span[start..].IndexOf((byte)0); - var end = zeroAt < 0 ? span.Length : start + zeroAt; - if (end > start) - result.Add(Encoding.ASCII.GetString(span[start..end])); + result.Add(Encoding.ASCII.GetString(raw)); + pos = end + 1; } return result; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs index 439a5bda..404ef00c 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs @@ -603,6 +603,110 @@ public sealed class AbCipDriverDiscoveryTests cache.Count.ShouldBe(0); } + // ---- real-controller shapes (captures from a CompactLogix 5069-L320ERS2 rev 34.11) ---- + + private static async Task DiscoverWithTemplatesAsync( + Dictionary templates, params AbCipDiscoveredTag[] tags) + { + var builder = new RecordingBuilder(); + var readers = new IdKeyedTemplateReaderFactory(); + foreach (var (id, blob) in templates) readers.Templates[id] = blob; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + EnableControllerBrowse = true, + }, "drv-1", enumeratorFactory: new FakeEnumeratorFactory(tags), templateReaderFactory: readers); + await drv.InitializeAsync("{}", CancellationToken.None); + await drv.DiscoverAsync(builder, CancellationToken.None); + return builder; + } + + /// + /// A controller reports every STRING tag as a struct (symbol type 0x8FCE → template 0x0FCE, + /// LEN + DATA). Browse must present it as the one String value it is — the read path already + /// reads it as a String — not as a folder holding a DINT and an SINT[82]. + /// + [Fact] + public async Task Discovered_STRING_struct_emits_one_String_variable_not_LEN_and_DATA() + { + var builder = await DiscoverWithTemplatesAsync( + new() { [0x0FCE] = Convert.FromHexString(CipTemplateObjectWireCaptureTests.StringTemplate) }, + new AbCipDiscoveredTag("WW_WorkOrderNum", null, AbCipDataType.Structure, ReadOnly: false, + TemplateInstanceId: 0x0FCE)); + + var v = builder.Variables.ShouldHaveSingleItem(); + v.Info.FullName.ShouldBe("WW_WorkOrderNum"); + v.Info.DriverDataType.ShouldBe(DriverDataType.String); + v.Info.IsArray.ShouldBeFalse(); + builder.Folders.ShouldNotContain(f => f.BrowseName == "WW_WorkOrderNum"); + } + + /// A nested STRING member of a UDT is one String leaf, addressable as Parent.Member. + [Fact] + public async Task Nested_STRING_member_emits_a_String_leaf() + { + var builder = await DiscoverWithTemplatesAsync( + new() + { + [3386] = Convert.FromHexString(CipTemplateObjectWireCaptureTests.LookupMemberTemplate), + [0x0FCE] = Convert.FromHexString(CipTemplateObjectWireCaptureTests.StringTemplate), + }, + new AbCipDiscoveredTag("Lookup", null, AbCipDataType.Structure, ReadOnly: false, + TemplateInstanceId: 3386)); + + var byName = builder.Variables.ToDictionary(v => v.Info.FullName, v => v.Info); + byName.Keys.ShouldBe(["Lookup.Code", "Lookup.Desc", "Lookup.DataType", "Lookup.Lock"], ignoreOrder: true); + byName["Lookup.Desc"].DriverDataType.ShouldBe(DriverDataType.String); + byName["Lookup.Code"].DriverDataType.ShouldBe(DriverDataType.Int32); + } + + /// + /// Logix packs BOOL members into hidden host integers (ZZZZZZZZZZAlarms0) and AOIs carry + /// __BitHost00-style backing members. Neither is user data, and a Logix name can never + /// contain a double underscore, so both are dropped from the browse. + /// + [Fact] + public async Task Hidden_BOOL_host_members_are_not_browsed() + { + var alarms = TemplateBufferBuilder.Build("Alarms", 4, + ("ZZZZZZZZZZAlarms0", 0xC2, 0, 0), + ("_00", 0xC1, 0, 0), + ("__BitHost00", 0xC4, 0, 0)); + + var builder = await DiscoverWithTemplatesAsync( + new() { [3425] = alarms }, + new AbCipDiscoveredTag("Alarms", null, AbCipDataType.Structure, ReadOnly: false, + TemplateInstanceId: 3425)); + + builder.Variables.Select(v => v.Info.FullName).ShouldBe(["Alarms._00"]); + } + + /// + /// An ARRAY of a UDT is not fanned out into Tag.Member leaves — those paths name no + /// element and every read of them would fail. An array of STRING is a String array. + /// + [Fact] + public async Task Discovered_struct_arrays_do_not_fan_out_and_STRING_arrays_are_String_arrays() + { + var udt = TemplateBufferBuilder.Build("Rec", 8, ("A", 0xC4, 0, 0), ("B", 0xCA, 0, 4)); + var builder = await DiscoverWithTemplatesAsync( + new() + { + [0x0FCE] = Convert.FromHexString(CipTemplateObjectWireCaptureTests.StringTemplate), + [0x55] = udt, + }, + new AbCipDiscoveredTag("arrMessages", null, AbCipDataType.Structure, ReadOnly: false, + ElementCount: 50, IsArray: true, TemplateInstanceId: 0x0FCE), + new AbCipDiscoveredTag("Stored", null, AbCipDataType.Structure, ReadOnly: false, + ElementCount: 255, IsArray: true, TemplateInstanceId: 0x55)); + + builder.Variables.ShouldNotContain(v => v.Info.FullName.StartsWith("Stored.")); + var messages = builder.Variables.Single(v => v.Info.FullName == "arrMessages").Info; + messages.DriverDataType.ShouldBe(DriverDataType.String); + messages.IsArray.ShouldBeTrue(); + messages.ArrayDim.ShouldBe(50u); + } + // ---- helpers ---- /// Test implementation of IAddressSpaceBuilder that records calls. @@ -714,34 +818,11 @@ public sealed class AbCipDriverDiscoveryTests } /// - /// Build a minimal CIP Template Object blob can decode - /// into an . Mirrors the helper in - /// AbCipFetchUdtShapeTests: 12-byte header + 8-byte member blocks + semicolon/NUL - /// terminated UDT-then-member name strings. + /// Build a template buffer in libplctag's real @udt/id layout via + /// . info is the member's CIP type word; arr + /// > 0 marks an array member. /// private static byte[] BuildSimpleTemplate( string name, uint instanceSize, params (string n, ushort info, ushort arr, uint off)[] members) - { - const int headerSize = 12; - const int blockSize = 8; - var strings = new MemoryStream(); - void Add(string s) { var b = Encoding.ASCII.GetBytes(s + ";\0"); strings.Write(b, 0, b.Length); } - Add(name); - foreach (var m in members) Add(m.n); - var stringsArr = strings.ToArray(); - - var buf = new byte[headerSize + blockSize * members.Length + stringsArr.Length]; - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(0), (ushort)members.Length); - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(2), 0x1234); - BinaryPrimitives.WriteUInt32LittleEndian(buf.AsSpan(4), instanceSize); - for (var i = 0; i < members.Length; i++) - { - var o = headerSize + i * blockSize; - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(o), members[i].info); - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(o + 2), members[i].arr); - BinaryPrimitives.WriteUInt32LittleEndian(buf.AsSpan(o + 4), members[i].off); - } - Buffer.BlockCopy(stringsArr, 0, buf, headerSize + blockSize * members.Length, stringsArr.Length); - return buf; - } + => TemplateBufferBuilder.Build(name, instanceSize, [.. members.Select(m => (m.n, m.info, m.arr, m.off))]); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipFetchUdtShapeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipFetchUdtShapeTests.cs index f56e5893..c2a2c642 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipFetchUdtShapeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipFetchUdtShapeTests.cs @@ -60,30 +60,14 @@ public sealed class AbCipFetchUdtShapeTests } } - private static byte[] BuildSimpleTemplate(string name, uint instanceSize, params (string n, ushort info, ushort arr, uint off)[] members) - { - var headerSize = 12; - var blockSize = 8; - var strings = new MemoryStream(); - void Add(string s) { var b = Encoding.ASCII.GetBytes(s + ";\0"); strings.Write(b, 0, b.Length); } - Add(name); - foreach (var m in members) Add(m.n); - var stringsArr = strings.ToArray(); - - var buf = new byte[headerSize + blockSize * members.Length + stringsArr.Length]; - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(0), (ushort)members.Length); - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(2), 0x1234); - BinaryPrimitives.WriteUInt32LittleEndian(buf.AsSpan(4), instanceSize); - for (var i = 0; i < members.Length; i++) - { - var o = headerSize + i * blockSize; - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(o), members[i].info); - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(o + 2), members[i].arr); - BinaryPrimitives.WriteUInt32LittleEndian(buf.AsSpan(o + 4), members[i].off); - } - Buffer.BlockCopy(stringsArr, 0, buf, headerSize + blockSize * members.Length, stringsArr.Length); - return buf; - } + /// + /// Build a template buffer in libplctag's real @udt/id layout via + /// . info is the member's CIP type word; arr + /// > 0 marks an array member. + /// + private static byte[] BuildSimpleTemplate( + string name, uint instanceSize, params (string n, ushort info, ushort arr, uint off)[] members) + => TemplateBufferBuilder.Build(name, instanceSize, [.. members.Select(m => (m.n, m.info, m.arr, m.off))]); private static Task InvokeFetch(AbCipDriver drv, string deviceHostAddress, uint templateId) { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/CipTemplateObjectDecoderTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/CipTemplateObjectDecoderTests.cs index aeb5a987..39600b54 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/CipTemplateObjectDecoderTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/CipTemplateObjectDecoderTests.cs @@ -10,45 +10,15 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; public sealed class CipTemplateObjectDecoderTests { /// - /// Construct a Template Object blob — header + member blocks + semicolon-delimited - /// strings (UDT name first, then member names). + /// Construct a template buffer in libplctag's real layout (see ). + /// info here is the member's CIP type word; arraySize > 0 marks an array member. /// private static byte[] BuildTemplate( string udtName, uint instanceSize, params (string name, ushort info, ushort arraySize, uint offset)[] members) - { - var memberCount = (ushort)members.Length; - var headerSize = 12; - var memberBlockSize = 8; - var blocksSize = memberBlockSize * members.Length; - - var stringsBuf = new MemoryStream(); - void AppendString(string s) - { - var bytes = Encoding.ASCII.GetBytes(s + ";\0"); - stringsBuf.Write(bytes, 0, bytes.Length); - } - AppendString(udtName); - foreach (var m in members) AppendString(m.name); - var strings = stringsBuf.ToArray(); - - var buf = new byte[headerSize + blocksSize + strings.Length]; - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(0), memberCount); - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(2), 0x1234); - BinaryPrimitives.WriteUInt32LittleEndian(buf.AsSpan(4), instanceSize); - BinaryPrimitives.WriteUInt32LittleEndian(buf.AsSpan(8), 0); - - for (var i = 0; i < members.Length; i++) - { - var o = headerSize + (i * memberBlockSize); - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(o), members[i].info); - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(o + 2), members[i].arraySize); - BinaryPrimitives.WriteUInt32LittleEndian(buf.AsSpan(o + 4), members[i].offset); - } - Buffer.BlockCopy(strings, 0, buf, headerSize + blocksSize, strings.Length); - return buf; - } + => TemplateBufferBuilder.Build(udtName, instanceSize, + [.. members.Select(m => (m.name, m.info, m.arraySize, m.offset))]); /// Verifies that a simple UDT with two members decodes correctly. [Fact] @@ -161,8 +131,9 @@ public sealed class CipTemplateObjectDecoderTests [Fact] public void Zero_member_count_returns_null() { - var buf = new byte[12]; - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(0), 0); + var buf = new byte[14]; + BinaryPrimitives.WriteUInt32LittleEndian(buf.AsSpan(6), 8); // instance size + BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(10), 0); // member count CipTemplateObjectDecoder.Decode(buf).ShouldBeNull(); } @@ -178,19 +149,10 @@ public sealed class CipTemplateObjectDecoderTests public void Missing_member_name_surfaces_placeholder() { // Header says 3 members but strings list has only UDT name + 2 member names. - var memberCount = (ushort)3; - var buf = new byte[12 + 8 * 3]; - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(0), memberCount); - BinaryPrimitives.WriteUInt32LittleEndian(buf.AsSpan(4), 12); - for (var i = 0; i < 3; i++) - { - var o = 12 + i * 8; - BinaryPrimitives.WriteUInt16LittleEndian(buf.AsSpan(o), 0xC4); - BinaryPrimitives.WriteUInt32LittleEndian(buf.AsSpan(o + 4), (uint)(i * 4)); - } - // strings: only UDT + 2 members, missing the third. - var strings = Encoding.ASCII.GetBytes("MyUdt;\0A;\0B;\0"); - var combined = buf.Concat(strings).ToArray(); + var full = TemplateBufferBuilder.Build("MyUdt", 12, + ("A", 0xC4, 0, 0), ("B", 0xC4, 0, 4), ("C", 0xC4, 0, 8)); + // Drop the trailing "C\0" so only two member names remain. + var combined = full[..^2]; var shape = CipTemplateObjectDecoder.Decode(combined); shape.ShouldNotBeNull(); @@ -198,19 +160,20 @@ public sealed class CipTemplateObjectDecoderTests shape.Members[2].Name.ShouldBe(""); } - /// Verifies that semicolon-terminated string parsing handles various input shapes. - /// The raw semicolon-terminated string input. - /// The expected array of parsed strings. + /// Verifies the template string section splits into the type name + NUL-terminated member names. + /// The raw string section. + /// Member names to read after the type name. + /// The expected type name followed by member names. [Theory] - [InlineData("Foo;\0Bar;\0", new[] { "Foo", "Bar" })] - [InlineData("Foo;Bar;", new[] { "Foo", "Bar" })] // no nulls - [InlineData("Only;\0", new[] { "Only" })] - [InlineData(";\0", new string[] { })] // empty - [InlineData("", new string[] { })] - public void ParseSemicolonTerminatedStrings_handles_shapes(string input, string[] expected) + [InlineData("STRING;nEAIA\0LEN\0DATA\0", 2, new[] { "STRING", "LEN", "DATA" })] + [InlineData("Udt;n\0A\0B\0\0\u0001junk", 2, new[] { "Udt", "A", "B" })] // trailing bytes ignored + [InlineData("Udt\0A\0", 1, new[] { "Udt", "A" })] // no ';' tail + [InlineData("Udt;nEA\0A\0", 3, new[] { "Udt", "A" })] // fewer names than members + [InlineData("", 1, new string[] { })] + public void ParseTemplateStrings_handles_shapes(string input, int memberCount, string[] expected) { var bytes = Encoding.ASCII.GetBytes(input); - var result = CipTemplateObjectDecoder.ParseSemicolonTerminatedStrings(bytes); + var result = CipTemplateObjectDecoder.ParseTemplateStrings(bytes, memberCount); result.ShouldBe(expected); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/CipTemplateObjectWireCaptureTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/CipTemplateObjectWireCaptureTests.cs new file mode 100644 index 00000000..8ebd336b --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/CipTemplateObjectWireCaptureTests.cs @@ -0,0 +1,98 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Decodes @udt/<id> template buffers captured byte for byte off a real controller — +/// a CompactLogix 5069-L320ERS2 rev 34.11, read through libplctag 1.5.2's Tag.GetBuffer() +/// on 2026-10-07 (read-only; one template read per id). +/// +/// +/// +/// Every pre-existing template test built its own input in the layout the decoder assumed +/// (u16 member count first, a 12-byte header, [info][array][offset] blocks, names +/// each ending ;\0). A real buffer agrees with none of that, so the decoder returned +/// null for every UDT a controller has and discovered UDTs never fanned out. The +/// same builder-shares-the-parser's-assumption trap as #468. +/// +/// +/// What libplctag actually hands back for @udt/id (cross-checked against its own +/// list_tags_logix example): a 14-byte header — u16 template id, u32 member-definition +/// size in 32-bit words, u32 instance size in bytes, u16 member count, u16 struct handle — +/// then one 8-byte block per member: u16 info (array length, or the bit number of a +/// BOOL), u16 type (bit 15 = struct, bit 13 = array, low 12 bits = CIP type code or +/// the nested template id), u32 byte offset. Then the strings: the type name ending at +/// ; (followed by an encoded tail up to a NUL), then each member name NUL-terminated. +/// +/// +[Trait("Category", "Unit")] +public sealed class CipTemplateObjectWireCaptureTests +{ + /// The built-in STRING template (id 0x0FCE): LEN DINT + DATA SINT[82], 88 bytes. + internal const string StringTemplate = + "CE0F10000000580000000200CE0F0000C400000000005200C22004000000" + + "535452494E473B6E45414941004C454E004441544100000001000200"; + + /// + /// Rockwell library UDT raC_UDT_LookupMember_STR0082 (id 3386): DINT, a nested + /// STRING member (struct type 0x8FCE), SINT, BOOL. Trailing bytes after the last name are + /// part of the capture and must be ignored. + /// + internal const string LookupMemberTemplate = + "3A0D1F000000600000000400B3810000C400000000000000CE8F040000000000C2005C0000000000C1005C000000" + + "7261435F5544545F4C6F6F6B75704D656D6265725F535452303038323B6E45414141454245430043" + + "6F64650044657363004461746154797065004C6F636B00008F92361DA1314013"; + + private static AbCipUdtShape DecodeHex(string hex) + { + var shape = CipTemplateObjectDecoder.Decode(Convert.FromHexString(hex)); + shape.ShouldNotBeNull(); + return shape; + } + + /// The STRING template decodes to its two members with the real array length. + [Fact] + public void Captured_STRING_template_decodes_LEN_and_DATA() + { + var shape = DecodeHex(StringTemplate); + + shape.TypeName.ShouldBe("STRING"); + shape.TotalSize.ShouldBe(88); + shape.Members.Select(m => m.Name).ShouldBe(["LEN", "DATA"]); + shape.Members[0].DataType.ShouldBe(AbCipDataType.DInt); + shape.Members[0].Offset.ShouldBe(0); + shape.Members[0].ArrayLength.ShouldBe(1); + shape.Members[1].DataType.ShouldBe(AbCipDataType.SInt); + shape.Members[1].Offset.ShouldBe(4); + shape.Members[1].ArrayLength.ShouldBe(82); + } + + /// A nested struct member carries its template id; trailing bytes are ignored. + [Fact] + public void Captured_library_UDT_decodes_nested_STRING_member_and_ignores_trailing_bytes() + { + var shape = DecodeHex(LookupMemberTemplate); + + shape.TypeName.ShouldBe("raC_UDT_LookupMember_STR0082"); + shape.TotalSize.ShouldBe(96); + shape.Members.Select(m => m.Name).ShouldBe(["Code", "Desc", "DataType", "Lock"]); + shape.Members[0].DataType.ShouldBe(AbCipDataType.DInt); + shape.Members[1].DataType.ShouldBe(AbCipDataType.Structure); + shape.Members[1].NestedTemplateId.ShouldBe(0x0FCEu); + shape.Members[1].Offset.ShouldBe(4); + shape.Members[2].DataType.ShouldBe(AbCipDataType.SInt); + shape.Members[2].Offset.ShouldBe(92); + shape.Members[3].DataType.ShouldBe(AbCipDataType.Bool); + shape.Members.ShouldAllBe(m => m.ArrayLength == 1); + } + + /// A shape is recognised as a Logix string family (LEN DINT + DATA SINT[n]). + [Fact] + public void Captured_STRING_template_is_a_string_family_shape_and_a_plain_UDT_is_not() + { + DecodeHex(StringTemplate).IsStringFamily.ShouldBeTrue(); + DecodeHex(LookupMemberTemplate).IsStringFamily.ShouldBeFalse(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/TemplateBufferBuilder.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/TemplateBufferBuilder.cs new file mode 100644 index 00000000..f699267e --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/TemplateBufferBuilder.cs @@ -0,0 +1,45 @@ +using System.Buffers.Binary; +using System.Text; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Builds an @udt/<id> template buffer in the layout libplctag really returns — +/// the layout pinned by the real-controller captures in CipTemplateObjectWireCaptureTests. +/// Keep it in step with those captures, never with the decoder: a builder that restates the +/// decoder's assumption proves nothing (#468, and the template decoder after it). +/// +internal static class TemplateBufferBuilder +{ + /// Build a template buffer. + /// UDT type name (written as name;n + an encoded tail, as a controller does). + /// Bytes per instance. + /// Per member: name, CIP type word (atomic code, or 0x8000 | nestedTemplateId), + /// array length (0 = scalar; > 0 sets the array flag and the info word), byte offset. + /// The buffer. + public static byte[] Build( + string udtName, uint instanceSize, params (string name, ushort type, ushort arraySize, uint offset)[] members) + { + const int header = 14; + var buf = new MemoryStream(); + var head = new byte[header + (8 * members.Length)]; + BinaryPrimitives.WriteUInt16LittleEndian(head.AsSpan(0), 0x0123); // template id + BinaryPrimitives.WriteUInt32LittleEndian(head.AsSpan(2), 0); // def size (words) + BinaryPrimitives.WriteUInt32LittleEndian(head.AsSpan(6), instanceSize); + BinaryPrimitives.WriteUInt16LittleEndian(head.AsSpan(10), (ushort)members.Length); + BinaryPrimitives.WriteUInt16LittleEndian(head.AsSpan(12), 0xABCD); // struct handle + for (var i = 0; i < members.Length; i++) + { + var o = header + (i * 8); + var (_, type, arraySize, offset) = members[i]; + var typeWord = arraySize > 0 ? (ushort)(type | 0x2000) : type; + BinaryPrimitives.WriteUInt16LittleEndian(head.AsSpan(o), arraySize); + BinaryPrimitives.WriteUInt16LittleEndian(head.AsSpan(o + 2), typeWord); + BinaryPrimitives.WriteUInt32LittleEndian(head.AsSpan(o + 4), offset); + } + buf.Write(head); + buf.Write(Encoding.ASCII.GetBytes(udtName + ";nEAIA\0")); + foreach (var m in members) buf.Write(Encoding.ASCII.GetBytes(m.name + "\0")); + return buf.ToArray(); + } +}