From b06a1ba6077f9bf99d9ee0822d3ebf3327af707d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 19 Apr 2026 17:09:06 -0400 Subject: [PATCH] =?UTF-8?q?AB=20CIP=20PR=206=20=E2=80=94=20UDT=20member-de?= =?UTF-8?q?claration=20support.=20Declaration-driven=20UDT=20member=20fan-?= =?UTF-8?q?out=20=E2=80=94=20users=20declare=20a=20UDT-typed=20tag=20once?= =?UTF-8?q?=20with=20an=20explicit=20Members=20list=20and=20the=20driver?= =?UTF-8?q?=20(1)=20expands=20member-addressable=20tags=20synthetically=20?= =?UTF-8?q?at=20Initialize=20time=20so=20Read/Write/Subscribe=20hit=20indi?= =?UTF-8?q?vidual=20native=20tags=20per=20member,=20(2)=20emits=20a=20fold?= =?UTF-8?q?er=20+=20one=20Variable=20per=20member=20in=20DiscoverAsync=20i?= =?UTF-8?q?nstead=20of=20a=20single=20opaque=20Structure=20Variable.=20Mat?= =?UTF-8?q?ches=20the=20Logix=205000=20addressing=20convention=20where=20m?= =?UTF-8?q?embers=20are=20reached=20via=20dotted=20syntax=20(Motor1.Speed,?= =?UTF-8?q?=20Motor1.Running)=20=E2=80=94=20AbCipTagPath=20already=20parse?= =?UTF-8?q?d=20this=20shape=20in=20PR=202,=20so=20PR=206=20just=20had=20to?= =?UTF-8?q?=20wire=20config=E2=86=92TagPath=20composition.=20New=20AbCipSt?= =?UTF-8?q?ructureMember=20record=20=E2=80=94=20Name=20/=20DataType=20/=20?= =?UTF-8?q?Writable=20/=20WriteIdempotent=20=E2=80=94=20plus=20optional=20?= =?UTF-8?q?Members=20list=20on=20AbCipTagDefinition=20that's=20ignored=20f?= =?UTF-8?q?or=20atomic=20types=20and=20optional=20for=20Structure=20types.?= =?UTF-8?q?=20When=20Structure=20has=20null=20or=20empty=20Members=20the?= =?UTF-8?q?=20driver=20falls=20back=20to=20emitting=20a=20single=20opaque?= =?UTF-8?q?=20Variable=20so=20downstream=20config=20can=20address=20member?= =?UTF-8?q?s=20manually=20(the=20"black=20box"=20path=20documented=20in=20?= =?UTF-8?q?AbCipTagDefinition's=20docstring).=20AbCipDriver.InitializeAsyn?= =?UTF-8?q?c=20now=20iterates=20tags=20+=20for=20every=20Structure=20tag?= =?UTF-8?q?=20with=20non-empty=20Members=20synthesises=20a=20child=20AbCip?= =?UTF-8?q?TagDefinition=20per=20member=20(composed=20full-reference=20Par?= =?UTF-8?q?ent.Member=20+=20composed=20TagPath=20parent.member=20passed=20?= =?UTF-8?q?through=20to=20libplctag=20as=20a=20normal=20symbolic=20read).?= =?UTF-8?q?=20Per-member=20Writable/WriteIdempotent=20metadata=20propagate?= =?UTF-8?q?s=20so=20IWritable=20correctly=20rejects=20writes=20to=20member?= =?UTF-8?q?s=20flagged=20non-writable=20even=20when=20the=20parent=20tag?= =?UTF-8?q?=20is=20writable=20=E2=80=94=20each=20member=20stands=20alone?= =?UTF-8?q?=20from=20the=20resilience=20+=20authz=20perspective.=20Discove?= =?UTF-8?q?rAsync=20gains=20a=20matching=20branch=20=E2=80=94=20Structure?= =?UTF-8?q?=20with=20Members=20emits=20an=20intermediate=20folder=20named?= =?UTF-8?q?=20after=20the=20parent=20tag=20+=20one=20Variable=20per=20memb?= =?UTF-8?q?er=20under=20it=20(browse=20name=20=3D=20member.Name,=20FullNam?= =?UTF-8?q?e=20=3D=20Parent.Member).=20Members=20with=20Writable=3Dfalse?= =?UTF-8?q?=20surface=20SecurityClassification.ViewOnly,=20WriteIdempotent?= =?UTF-8?q?=20flag=20passes=20through=20to=20the=20DriverAttributeInfo.=20?= =?UTF-8?q?Structure=20without=20Members=20falls=20through=20to=20the=20no?= =?UTF-8?q?rmal=20single-Variable=20path.=20Whole-UDT=20read=20optimizatio?= =?UTF-8?q?n=20(one=20libplctag=20call=20returns=20the=20packed=20buffer?= =?UTF-8?q?=20+=20client-side=20member=20decode)=20is=20deferred=20?= =?UTF-8?q?=E2=80=94=20needs=20the=20CIP=20Template=20Object=20class=200x6?= =?UTF-8?q?C=20reader=20which=20is=20blocked=20on=20the=20same=20libplctag?= =?UTF-8?q?=201.5.2=20TagInfoPlcMapper=20gap=20that=20deferred=20the=20rea?= =?UTF-8?q?l=20@tags=20walker=20in=20PR=205.=20AbCipTemplateCache=20shippe?= =?UTF-8?q?d=20in=20PR=205=20is=20the=20drop-in=20point=20when=20that=20re?= =?UTF-8?q?ader=20lands.=20Per-member=20reads=20today=20are=20N=20native?= =?UTF-8?q?=20round-trips;=20whole-UDT=20optimisation=20is=20a=20perf=20wi?= =?UTF-8?q?n,=20not=20a=20correctness=20gap.=207=20new=20unit=20tests=20in?= =?UTF-8?q?=20AbCipUdtMemberTests=20=E2=80=94=20UDT=20fan-out=20to=20Varia?= =?UTF-8?q?ble=20children=20under=20folder=20with=20correct=20SecurityClas?= =?UTF-8?q?sification=20+=20WriteIdempotent=20propagation,=20member=20read?= =?UTF-8?q?s=20via=20synthesised=20full-reference=20with=20correct=20per-m?= =?UTF-8?q?ember=20values,=20member=20writes=20routing=20to=20correct=20Ta?= =?UTF-8?q?gPath,=20member=20Writable=3Dfalse=20flag=20correctly=20blockin?= =?UTF-8?q?g=20IWritable,=20Structure=20without=20Members=20falls=20back?= =?UTF-8?q?=20to=20single=20Variable,=20empty=20Members=20list=20treated?= =?UTF-8?q?=20identically=20to=20null,=20UDT=20tags=20coexist=20with=20fla?= =?UTF-8?q?t=20tags=20in=20the=20discovery=20output.=20Total=20AbCip=20uni?= =?UTF-8?q?t=20tests=20now=20130/130=20passing=20(+7=20from=20PR=205's=201?= =?UTF-8?q?23).=20Modbus=20+=20other=20drivers=20untouched;=20full=20solut?= =?UTF-8?q?ion=20builds=200=20errors.=20Unblocks=20PR=207=20(ISubscribable?= =?UTF-8?q?)=20=E2=80=94=20the=20poll=20engine=20already=20works=20with=20?= =?UTF-8?q?member-level=20full=20references.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.7 (1M context) --- .../AbCipDriver.cs | 49 +++- .../AbCipDriverOptions.cs | 18 ++ .../AbCipUdtMemberTests.cs | 217 ++++++++++++++++++ 3 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index be3e8c0..6b33eff 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -61,7 +61,27 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, var profile = AbCipPlcFamilyProfile.ForFamily(device.PlcFamily); _devices[device.HostAddress] = new DeviceState(addr, device, profile); } - foreach (var tag in _options.Tags) _tagsByName[tag.Name] = tag; + foreach (var tag in _options.Tags) + { + // UDT tags with declared Members fan out into synthetic member-tag entries addressable + // by composed full-reference. Parent structure tag also stored so discovery can emit a + // folder for it. + _tagsByName[tag.Name] = tag; + if (tag.DataType == AbCipDataType.Structure && tag.Members is { Count: > 0 }) + { + foreach (var member in tag.Members) + { + var memberTag = new AbCipTagDefinition( + Name: $"{tag.Name}.{member.Name}", + DeviceHostAddress: tag.DeviceHostAddress, + TagPath: $"{tag.TagPath}.{member.Name}", + DataType: member.DataType, + Writable: member.Writable, + WriteIdempotent: member.WriteIdempotent); + _tagsByName[memberTag.Name] = memberTag; + } + } + } _health = new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null); } catch (Exception ex) @@ -304,12 +324,37 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, var deviceLabel = device.DeviceName ?? device.HostAddress; var deviceFolder = root.Folder(device.HostAddress, deviceLabel); - // Pre-declared tags — always emitted; the primary config path. + // Pre-declared tags — always emitted; the primary config path. UDT tags with declared + // Members fan out into a sub-folder + one Variable per member instead of a single + // Structure Variable (Structure has no useful scalar value + member-addressable paths + // are what downstream consumers actually want). var preDeclared = _options.Tags.Where(t => string.Equals(t.DeviceHostAddress, device.HostAddress, StringComparison.OrdinalIgnoreCase)); foreach (var tag in preDeclared) { if (AbCipSystemTagFilter.IsSystemTag(tag.Name)) continue; + + if (tag.DataType == AbCipDataType.Structure && tag.Members is { Count: > 0 }) + { + var udtFolder = deviceFolder.Folder(tag.Name, tag.Name); + foreach (var member in tag.Members) + { + var memberFullName = $"{tag.Name}.{member.Name}"; + udtFolder.Variable(member.Name, member.Name, new DriverAttributeInfo( + FullName: memberFullName, + DriverDataType: member.DataType.ToDriverDataType(), + IsArray: false, + ArrayDim: null, + SecurityClass: member.Writable + ? SecurityClassification.Operate + : SecurityClassification.ViewOnly, + IsHistorized: false, + IsAlarm: false, + WriteIdempotent: member.WriteIdempotent)); + } + continue; + } + deviceFolder.Variable(tag.Name, tag.Name, ToAttributeInfo(tag)); } diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverOptions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverOptions.cs index 2d99471..552660b 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverOptions.cs @@ -54,12 +54,30 @@ public sealed record AbCipDeviceOptions( /// Logix atomic type, or for UDT-typed tags. /// When true and the tag's ExternalAccess permits writes, IWritable routes writes here. /// Per plan decisions #44–#45, #143 — safe to replay on write timeout. Default false. +/// For -typed tags, the declared UDT +/// member layout. When supplied, discovery fans out the UDT into a folder + one Variable per +/// member (member TagPath = {tag.TagPath}.{member.Name}). When null on a Structure +/// tag, the driver treats it as a black-box and relies on downstream configuration to address +/// members individually via dotted syntax. Ignored for atomic types. public sealed record AbCipTagDefinition( string Name, string DeviceHostAddress, string TagPath, AbCipDataType DataType, bool Writable = true, + bool WriteIdempotent = false, + IReadOnlyList? Members = null); + +/// +/// One declared member of a UDT tag. Name is the member identifier on the PLC (e.g. Speed, +/// Status), DataType is the atomic Logix type, Writable/WriteIdempotent mirror +/// . Declaration-driven — the real CIP Template Object reader +/// (class 0x6C) that would auto-discover member layouts lands as a follow-up PR. +/// +public sealed record AbCipStructureMember( + string Name, + AbCipDataType DataType, + bool Writable = true, bool WriteIdempotent = false); /// Which AB PLC family the device is — selects the profile applied to connection params. diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberTests.cs new file mode 100644 index 0000000..0b369c2 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberTests.cs @@ -0,0 +1,217 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +[Trait("Category", "Unit")] +public sealed class AbCipUdtMemberTests +{ + [Fact] + public async Task UDT_with_declared_members_fans_out_to_member_variables() + { + var builder = new RecordingBuilder(); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Tags = + [ + new AbCipTagDefinition( + Name: "Motor1", + DeviceHostAddress: "ab://10.0.0.5/1,0", + TagPath: "Motor1", + DataType: AbCipDataType.Structure, + Members: + [ + new AbCipStructureMember("Speed", AbCipDataType.DInt), + new AbCipStructureMember("Running", AbCipDataType.Bool, Writable: false), + new AbCipStructureMember("SetPoint", AbCipDataType.Real, WriteIdempotent: true), + ]), + ], + }, "drv-1"); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.DiscoverAsync(builder, CancellationToken.None); + + builder.Folders.ShouldContain(f => f.BrowseName == "Motor1"); + var variables = builder.Variables.Select(v => (v.BrowseName, v.Info.FullName)).ToList(); + variables.ShouldContain(("Speed", "Motor1.Speed")); + variables.ShouldContain(("Running", "Motor1.Running")); + variables.ShouldContain(("SetPoint", "Motor1.SetPoint")); + + builder.Variables.Single(v => v.BrowseName == "Running").Info.SecurityClass + .ShouldBe(SecurityClassification.ViewOnly); + builder.Variables.Single(v => v.BrowseName == "SetPoint").Info.WriteIdempotent + .ShouldBeTrue(); + } + + [Fact] + public async Task UDT_members_resolvable_for_read_via_synthesised_full_reference() + { + var factory = new FakeAbCipTagFactory + { + Customise = p => p.TagName switch + { + "Motor1.Speed" => new FakeAbCipTag(p) { Value = 1800 }, + "Motor1.Running" => new FakeAbCipTag(p) { Value = true }, + _ => new FakeAbCipTag(p), + }, + }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Tags = + [ + new AbCipTagDefinition("Motor1", "ab://10.0.0.5/1,0", "Motor1", AbCipDataType.Structure, + Members: + [ + new AbCipStructureMember("Speed", AbCipDataType.DInt), + new AbCipStructureMember("Running", AbCipDataType.Bool), + ]), + ], + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var snapshots = await drv.ReadAsync(["Motor1.Speed", "Motor1.Running"], CancellationToken.None); + + snapshots[0].Value.ShouldBe(1800); + snapshots[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + snapshots[1].Value.ShouldBe(true); + snapshots[1].StatusCode.ShouldBe(AbCipStatusMapper.Good); + } + + [Fact] + public async Task UDT_member_write_routes_through_synthesised_tagpath() + { + var factory = new FakeAbCipTagFactory(); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Tags = + [ + new AbCipTagDefinition("Motor1", "ab://10.0.0.5/1,0", "Motor1", AbCipDataType.Structure, + Members: + [ + new AbCipStructureMember("SetPoint", AbCipDataType.Real), + ]), + ], + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("Motor1.SetPoint", 42.5f)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(AbCipStatusMapper.Good); + factory.Tags["Motor1.SetPoint"].Value.ShouldBe(42.5f); + } + + [Fact] + public async Task UDT_member_read_write_honours_member_Writable_flag() + { + var factory = new FakeAbCipTagFactory(); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Tags = + [ + new AbCipTagDefinition("Motor1", "ab://10.0.0.5/1,0", "Motor1", AbCipDataType.Structure, + Members: + [ + new AbCipStructureMember("Status", AbCipDataType.DInt, Writable: false), + ]), + ], + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("Motor1.Status", 1)], CancellationToken.None); + + results.Single().StatusCode.ShouldBe(AbCipStatusMapper.BadNotWritable); + } + + [Fact] + public async Task Structure_tag_without_members_is_emitted_as_single_variable() + { + // Fallback path: a Structure tag with no declared Members still appears as a Variable so + // downstream configuration can address it manually. This matches the "black box" note in + // AbCipTagDefinition's docstring. + var builder = new RecordingBuilder(); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Tags = [new AbCipTagDefinition("OpaqueUdt", "ab://10.0.0.5/1,0", "OpaqueUdt", AbCipDataType.Structure)], + }, "drv-1"); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.DiscoverAsync(builder, CancellationToken.None); + + builder.Variables.ShouldContain(v => v.BrowseName == "OpaqueUdt"); + builder.Folders.ShouldNotContain(f => f.BrowseName == "OpaqueUdt"); + } + + [Fact] + public async Task Empty_Members_list_is_treated_like_null() + { + var builder = new RecordingBuilder(); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Tags = [new AbCipTagDefinition("EmptyUdt", "ab://10.0.0.5/1,0", "E", AbCipDataType.Structure, Members: [])], + }, "drv-1"); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.DiscoverAsync(builder, CancellationToken.None); + + builder.Folders.ShouldNotContain(f => f.BrowseName == "EmptyUdt"); + builder.Variables.ShouldContain(v => v.BrowseName == "EmptyUdt"); + } + + [Fact] + public async Task UDT_members_mixed_with_flat_tags_coexist() + { + var builder = new RecordingBuilder(); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Tags = + [ + new AbCipTagDefinition("FlatA", "ab://10.0.0.5/1,0", "A", AbCipDataType.DInt), + new AbCipTagDefinition("Motor1", "ab://10.0.0.5/1,0", "Motor1", AbCipDataType.Structure, + Members: + [ + new AbCipStructureMember("Speed", AbCipDataType.DInt), + ]), + new AbCipTagDefinition("FlatB", "ab://10.0.0.5/1,0", "B", AbCipDataType.Real), + ], + }, "drv-1"); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.DiscoverAsync(builder, CancellationToken.None); + + builder.Variables.Select(v => v.BrowseName).ShouldBe(["FlatA", "Speed", "FlatB"], ignoreOrder: true); + } + + // ---- helpers ---- + + private sealed class RecordingBuilder : IAddressSpaceBuilder + { + public List<(string BrowseName, string DisplayName)> Folders { get; } = new(); + public List<(string BrowseName, DriverAttributeInfo Info)> Variables { get; } = new(); + + public IAddressSpaceBuilder Folder(string browseName, string displayName) + { Folders.Add((browseName, displayName)); return this; } + + public IVariableHandle Variable(string browseName, string displayName, DriverAttributeInfo info) + { Variables.Add((browseName, info)); return new Handle(info.FullName); } + + public void AddProperty(string _, DriverDataType __, object? ___) { } + + private sealed class Handle(string fullRef) : IVariableHandle + { + public string FullReference => fullRef; + public IAlarmConditionSink MarkAsAlarmCondition(AlarmConditionInfo info) => new NullSink(); + } + private sealed class NullSink : IAlarmConditionSink { public void OnTransition(AlarmEventArgs args) { } } + } +} -- 2.49.1