fix(abcip): thread CIP template-instance-id so discovered-UDT expansion works in production (review)

This commit is contained in:
Joseph Doherty
2026-06-17 20:30:12 -04:00
parent fdd6b332fe
commit 4a7b0fde7b
6 changed files with 239 additions and 20 deletions
@@ -78,12 +78,19 @@ public static class CipSymbolObjectDecoder
var (programScope, simpleName) = SplitProgramScope(name);
var dataType = isStruct ? AbCipDataType.Structure : MapTypeCode((byte)typeCode);
// For a struct symbol the lower 12 bits (typeCode) are NOT a primitive CIP type code —
// they are the CIP template instance id (per the bit-15 struct-flag remarks above). Surface
// it so the driver can fetch the UDT's Template Object during discovery. Atomic symbols
// carry no template id.
var templateInstanceId = isStruct ? (uint?)typeCode : null;
yield return new AbCipDiscoveredTag(
Name: simpleName,
ProgramScope: programScope,
DataType: dataType ?? AbCipDataType.Structure, // unknown type code → treat as opaque
ReadOnly: false, // Symbol Object doesn't carry write-protection bits; lift via AccessControl Object later
IsSystemTag: isSystem);
IsSystemTag: isSystem,
TemplateInstanceId: templateInstanceId);
_ = instanceId; // retained in the wire format for diagnostics; not surfaced to the driver today
}
@@ -45,6 +45,9 @@ public interface IAbCipTagEnumeratorFactory
/// reported a 1-D array (even of length 1). Surfaces the tag as an OPC UA array node at
/// discovery; <see cref="ElementCount"/> alone can't distinguish a scalar from a 1-element
/// array.</param>
/// <param name="TemplateInstanceId">The CIP template instance id for a discovered Structure tag
/// (<c>null</c> for non-structs / when unknown). Used to fetch the UDT's Template Object shape
/// during discovery so the struct can be fanned out into addressable member variables.</param>
public sealed record AbCipDiscoveredTag(
string Name,
string? ProgramScope,
@@ -52,7 +55,8 @@ public sealed record AbCipDiscoveredTag(
bool ReadOnly,
bool IsSystemTag = false,
int ElementCount = 1,
bool IsArray = false);
bool IsArray = false,
uint? TemplateInstanceId = null);
/// <summary>
/// No-op enumerator returning an empty sequence. Useful for tests + strict-config
@@ -45,6 +45,9 @@ internal sealed class LibplctagTagEnumerator : IAbCipTagEnumerator
await _tag.ReadAsync(cancellationToken).ConfigureAwait(false);
var buffer = _tag.GetBuffer();
// The decoder already surfaces each struct tag's CIP template instance id on
// AbCipDiscoveredTag.TemplateInstanceId (from the lower 12 bits of the symbol type), so
// re-yielding the decoded records as-is carries it through to DiscoverAsync's UDT fan-out.
foreach (var tag in CipSymbolObjectDecoder.Decode(buffer))
yield return tag;
}
@@ -1,4 +1,6 @@
using System.Buffers.Binary;
using System.Runtime.CompilerServices;
using System.Text;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
@@ -166,6 +168,65 @@ public sealed class AbCipDriverDiscoveryTests
byName["Motor1.Status.Code"].DriverDataType.ShouldBe(DriverDataType.Int32);
}
/// <summary>
/// PRODUCTION PATH (no test seam): a controller-discovered Structure tag carries its CIP
/// template instance id (surfaced by the Symbol Object decoder onto
/// <see cref="AbCipDiscoveredTag.TemplateInstanceId"/>). DiscoverAsync threads that id into
/// <c>FetchUdtShapeAsync</c>, which reads the Template Object off the controller via the
/// injected <see cref="IAbCipTemplateReader"/> and fans the top-level UDT out into atomic
/// member Variables — WITHOUT any call to <c>SeedDiscoveredUdtShapeForTest</c>. This proves the
/// production top-level expansion is functional, not inert.
/// </summary>
[Fact]
public async Task Controller_discovered_UDT_top_level_expands_via_template_id_fetch_no_seam()
{
const string device = "ab://10.0.0.5/1,0";
const uint motorTemplateId = 0x2A; // lower 12 bits of the struct symbol type
var builder = new RecordingBuilder();
// Discovered tag carries the template instance id — exactly what the real Symbol Object
// decoder produces for a struct symbol.
var enumeratorFactory = new FakeEnumeratorFactory(
new AbCipDiscoveredTag("Motor1", null, AbCipDataType.Structure, ReadOnly: false,
TemplateInstanceId: motorTemplateId));
// Fake template reader keyed by id: a Read Template for motorTemplateId returns a real
// Template Object blob with two atomic members (Speed/Real, Running/Bool). No seed seam.
var templateReaderFactory = new IdKeyedTemplateReaderFactory
{
Templates =
{
[motorTemplateId] = BuildSimpleTemplate("MotorUdt", 8,
("Speed", 0xCA, 0, 0), // 0xCA = Real
("Running", 0xC1, 0, 4)), // 0xC1 = Bool
},
};
var drv = new AbCipDriver(new AbCipDriverOptions
{
Devices = [new AbCipDeviceOptions(device)],
EnableControllerBrowse = true,
}, "drv-1", enumeratorFactory: enumeratorFactory, templateReaderFactory: templateReaderFactory);
await drv.InitializeAsync("{}", CancellationToken.None);
await drv.DiscoverAsync(builder, CancellationToken.None);
// The reader was actually consulted for the discovered tag's template id — the production
// fetch path ran (not the seam).
templateReaderFactory.LastTemplateId.ShouldBe(motorTemplateId);
// The Motor1 sub-folder + atomic member leaves are emitted; the bogus Structure placeholder
// Variable is NOT.
builder.Folders.ShouldContain(f => f.BrowseName == "Motor1");
builder.Variables.ShouldNotContain(v => v.Info.FullName == "Motor1");
var byName = builder.Variables.ToDictionary(v => v.Info.FullName, v => v.Info);
byName.ShouldContainKey("Motor1.Speed");
byName["Motor1.Speed"].DriverDataType.ShouldBe(DriverDataType.Float32);
byName.ShouldContainKey("Motor1.Running");
byName["Motor1.Running"].DriverDataType.ShouldBe(DriverDataType.Boolean);
}
/// <summary>
/// A discovered Structure whose shape cannot be resolved degrades to the prior single
/// Variable behavior (no regression): one Variable under the Discovered folder keyed by
@@ -194,6 +255,51 @@ public sealed class AbCipDriverDiscoveryTests
builder.Folders.ShouldNotContain(f => f.BrowseName == "MysteryUdt");
}
/// <summary>
/// A nested struct member whose sub-shape resolves but yields NO emittable atomic leaf must
/// not leave an empty nested sub-folder in the browse tree — the folder is materialised lazily,
/// only once a leaf is actually emitted into it. Here <c>Empty</c> (a nested struct) has only
/// a further nested struct child whose own shape is unresolvable, so the whole <c>Empty</c>
/// branch produces no leaf and must produce no folder either; the sibling atomic leaf is kept.
/// </summary>
[Fact]
public async Task Controller_discovered_UDT_does_not_leave_empty_nested_subfolder()
{
const string device = "ab://10.0.0.5/1,0";
var builder = new RecordingBuilder();
var enumeratorFactory = new FakeEnumeratorFactory(
new AbCipDiscoveredTag("Top", null, AbCipDataType.Structure, ReadOnly: false));
var drv = new AbCipDriver(new AbCipDriverOptions
{
Devices = [new AbCipDeviceOptions(device)],
EnableControllerBrowse = true,
}, "drv-1", enumeratorFactory: enumeratorFactory);
await drv.InitializeAsync("{}", CancellationToken.None);
// Top has one atomic leaf (Ok) + a nested struct (Empty). Empty's shape resolves, but its
// only member is a further nested struct (Ghost) whose shape is NOT seeded → unresolvable →
// no leaf anywhere under Empty.
drv.SeedDiscoveredUdtShapeForTest(device, "Top", new AbCipUdtShape("TopUdt", 8,
[
new AbCipUdtMember("Ok", 0, AbCipDataType.DInt, ArrayLength: 1),
new AbCipUdtMember("Empty", 4, AbCipDataType.Structure, ArrayLength: 1),
]));
drv.SeedDiscoveredUdtShapeForTest(device, "Empty", new AbCipUdtShape("EmptyUdt", 4,
[
new AbCipUdtMember("Ghost", 0, AbCipDataType.Structure, ArrayLength: 1),
]));
await drv.DiscoverAsync(builder, CancellationToken.None);
// The Top folder + its atomic leaf are emitted.
builder.Folders.ShouldContain(f => f.BrowseName == "Top");
builder.Variables.Select(v => v.Info.FullName).ShouldContain("Top.Ok");
// The Empty nested struct yields no leaf → no empty sub-folder is created for it.
builder.Folders.ShouldNotContain(f => f.BrowseName == "Empty");
builder.Variables.Select(v => v.Info.FullName).ShouldNotContain(n => n.StartsWith("Top.Empty", StringComparison.Ordinal));
}
/// <summary>
/// Nested-struct recursion is bounded by <c>MaxUdtDepth</c>: a member at the cap depth
/// is dropped rather than emitted. Here the chain L0.L1.L2.… is seeded deeper than the
@@ -252,27 +358,42 @@ public sealed class AbCipDriverDiscoveryTests
}
/// <summary>
/// A discovered member-path read resolves to the member's atomic type via the
/// equipment-tag resolver over the authored TagConfig (FullName = <c>Parent.Member</c>),
/// so no extra registration is needed for discovered members to be readable.
/// A discovered member-path read goes end-to-end through the driver: with NO pre-declared
/// tag for the member, <see cref="AbCipDriver.ReadAsync"/> resolves the authored TagConfig
/// JSON blob (FullName = <c>Parent.Member</c>) via the equipment-tag resolver, materialises a
/// runtime on the device, reads it, and decodes the member's atomic value — proving discovered
/// members are readable with no extra registration. The libplctag tag name the driver builds
/// for the member equals the dotted member path.
/// </summary>
[Fact]
public void Discovered_member_path_resolves_to_member_atomic_type()
public async Task Discovered_member_path_reads_through_driver_as_member_atomic_type()
{
var drv = new AbCipDriver(new AbCipDriverOptions
{
Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")],
}, "drv-1");
// The wire reference handed to ReadAsync for a discovered member is the authored
// TagConfig JSON blob (FullName = Motor1.Status.Code, atomic type Real here).
const string device = "ab://10.0.0.5/1,0";
// The wire reference handed to ReadAsync for a discovered member is the authored TagConfig
// JSON blob (FullName = Motor1.Status.Code, atomic type Real here). No tag is pre-declared.
const string tagConfig =
"{\"tagPath\":\"Motor1.Status.Code\",\"dataType\":\"Real\",\"deviceHostAddress\":\"ab://10.0.0.5/1,0\"}";
AbCipEquipmentTagParser.TryParse(tagConfig, out var def).ShouldBeTrue();
def.TagPath.ShouldBe("Motor1.Status.Code");
def.DataType.ShouldBe(AbCipDataType.Real);
def.DataType.ToDriverDataType().ShouldBe(DriverDataType.Float32);
var tagFactory = new FakeAbCipTagFactory
{
// The driver creates the runtime keyed by the libplctag tag name = the dotted member
// path; seed a Real value the read should surface.
Customise = p => new FakeAbCipTag(p) { Value = p.TagName == "Motor1.Status.Code" ? 12.5f : null },
};
var drv = new AbCipDriver(new AbCipDriverOptions
{
Devices = [new AbCipDeviceOptions(device)],
}, "drv-1", tagFactory: tagFactory);
await drv.InitializeAsync("{}", CancellationToken.None);
var results = await drv.ReadAsync([tagConfig], CancellationToken.None);
results.Count.ShouldBe(1);
results[0].StatusCode.ShouldBe(AbCipStatusMapper.Good);
results[0].Value.ShouldBe(12.5f);
// The driver built a runtime under the dotted member tag name — confirming the member-path
// read addresses the member directly (no parent-UDT registration needed).
tagFactory.Tags.ShouldContainKey("Motor1.Status.Code");
}
/// <summary>Verifies that controller enumeration honours system tag hint and filter.</summary>
@@ -486,4 +607,65 @@ public sealed class AbCipDriverDiscoveryTests
public void Dispose() { }
}
}
/// <summary>
/// Fake <see cref="IAbCipTemplateReaderFactory"/> whose readers return a Template Object blob
/// keyed by template instance id — the production fetch path (FetchUdtShapeAsync) keyed by the
/// id the Symbol Object decoder surfaces, with NO use of the seed seam.
/// </summary>
private sealed class IdKeyedTemplateReaderFactory : IAbCipTemplateReaderFactory
{
/// <summary>Gets the template blobs keyed by template instance id.</summary>
public Dictionary<uint, byte[]> Templates { get; } = new();
/// <summary>Gets the last template id any reader was asked for.</summary>
public uint? LastTemplateId { get; private set; }
/// <summary>Creates a reader over the shared template map.</summary>
public IAbCipTemplateReader Create() => new Reader(this);
private sealed class Reader(IdKeyedTemplateReaderFactory outer) : IAbCipTemplateReader
{
public Task<byte[]> ReadAsync(
AbCipTagCreateParams deviceParams, uint templateInstanceId, CancellationToken cancellationToken)
{
outer.LastTemplateId = templateInstanceId;
return Task.FromResult(
outer.Templates.TryGetValue(templateInstanceId, out var blob) ? blob : []);
}
public void Dispose() { }
}
}
/// <summary>
/// Build a minimal CIP Template Object blob <see cref="CipTemplateObjectDecoder"/> can decode
/// into an <see cref="AbCipUdtShape"/>. Mirrors the helper in
/// <c>AbCipFetchUdtShapeTests</c>: 12-byte header + 8-byte member blocks + semicolon/NUL
/// terminated UDT-then-member name strings.
/// </summary>
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;
}
}
@@ -98,11 +98,15 @@ public sealed class CipSymbolObjectDecoderTests
CipSymbolObjectDecoder.MapTypeCode(0xFF).ShouldBeNull();
}
/// <summary>Verifies that struct flag overrides type code.</summary>
/// <summary>
/// Verifies that the struct flag overrides the type code AND that the lower 12 bits are
/// surfaced as the CIP template instance id (used by discovery to fetch the UDT's Template
/// Object). This is the plumbing that makes top-level discovered-UDT expansion functional.
/// </summary>
[Fact]
public void Struct_flag_overrides_type_code_and_yields_Structure()
public void Struct_flag_yields_Structure_and_surfaces_template_instance_id()
{
// 0x8000 (struct) + 0x1234 (template instance id in lower 12 bits; uses 0x234)
// 0x8000 (struct flag) | 0x0234 (template instance id in the lower 12 bits)
var bytes = BuildEntry(
instanceId: 5,
symbolType: 0x8000 | 0x0234,
@@ -112,6 +116,25 @@ public sealed class CipSymbolObjectDecoderTests
var tag = CipSymbolObjectDecoder.Decode(bytes).Single();
tag.DataType.ShouldBe(AbCipDataType.Structure);
tag.TemplateInstanceId.ShouldBe(0x0234u);
}
/// <summary>
/// Verifies that an atomic (non-struct) symbol carries no template instance id — only struct
/// symbols repurpose the lower 12 bits as a template id.
/// </summary>
[Fact]
public void Atomic_symbol_has_no_template_instance_id()
{
var bytes = BuildEntry(
instanceId: 7,
symbolType: 0xC4, // DINT — atomic, not a struct
elementLength: 4,
arrayDims: (0, 0, 0),
name: "Counter");
var tag = CipSymbolObjectDecoder.Decode(bytes).Single();
tag.TemplateInstanceId.ShouldBeNull();
}
/// <summary>Verifies that system flag surfaces as IsSystemTag true.</summary>