From 780358c790356d6dbc8f66ad23a2b53c6f4e8ce0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 20 Apr 2026 04:17:57 -0400 Subject: [PATCH] =?UTF-8?q?AbCip=20whole-UDT=20read=20optimization=20(#194?= =?UTF-8?q?)=20=E2=80=94=20declaration-driven=20member=20grouping=20collap?= =?UTF-8?q?ses=20N=20per-member=20reads=20into=20one=20parent-UDT=20read?= =?UTF-8?q?=20+=20client-side=20decode.=20Closes=20task=20#194.=20On=20a?= =?UTF-8?q?=20batch=20that=20includes=20multiple=20members=20of=20the=20sa?= =?UTF-8?q?me=20hand-declared=20UDT=20tag,=20ReadAsync=20now=20issues=20on?= =?UTF-8?q?e=20libplctag=20read=20on=20the=20parent=20+=20decodes=20each?= =?UTF-8?q?=20member=20from=20the=20runtime's=20buffer=20at=20its=20comput?= =?UTF-8?q?ed=20byte=20offset.=20A=206-member=20Motor=20UDT=20read=20goes?= =?UTF-8?q?=20from=206=20libplctag=20round-trips=20to=201=20=E2=80=94=20th?= =?UTF-8?q?e=20Rockwell-suggested=20pattern=20for=20minimizing=20CIP=20req?= =?UTF-8?q?uest=20overhead=20on=20batch=20reads=20of=20UDT=20state=20(deci?= =?UTF-8?q?sion=20#11's=20follow-through=20on=20what=20the=20template=20de?= =?UTF-8?q?coder=20from=20task=20#179=20was=20meant=20to=20enable).=20AbCi?= =?UTF-8?q?pUdtMemberLayout=20is=20a=20pure-function=20helper=20that=20com?= =?UTF-8?q?putes=20declared-member=20byte=20offsets=20under=20Logix=20natu?= =?UTF-8?q?ral-alignment=20rules=20(SInt=201-byte=20/=20Int=202-byte=20/?= =?UTF-8?q?=20DInt=20+=20Real=20+=20Dt=204-byte=20/=20LInt=20+=20ULInt=20+?= =?UTF-8?q?=20LReal=208-byte;=20alignment=20pad=20inserted=20before=20each?= =?UTF-8?q?=20member=20as=20needed).=20Opts=20out=20for=20BOOL=20/=20Strin?= =?UTF-8?q?g=20/=20Structure=20members=20=E2=80=94=20BOOL=20storage=20in?= =?UTF-8?q?=20Logix=20UDTs=20packs=20into=20a=20hidden=20host=20byte=20who?= =?UTF-8?q?se=20position=20can't=20be=20computed=20from=20declaration-only?= =?UTF-8?q?=20info,=20and=20String=20members=20need=20length-prefix=20+=20?= =?UTF-8?q?STRING[82]=20fan-out=20which=20libplctag=20already=20handles=20?= =?UTF-8?q?via=20a=20per-tag=20DecodeValue=20path.=20The=20CIP=20Template?= =?UTF-8?q?=20Object=20shape=20from=20task=20#179=20(when=20populated=20vi?= =?UTF-8?q?a=20FetchUdtShapeAsync)=20carries=20real=20offsets=20for=20thos?= =?UTF-8?q?e=20members=20=E2=80=94=20layering=20that=20richer=20path=20on?= =?UTF-8?q?=20top=20of=20the=20planner=20is=20a=20separate=20follow-up=20a?= =?UTF-8?q?nd=20does=20not=20change=20this=20PR's=20conservative=20behavio?= =?UTF-8?q?ur.=20AbCipUdtReadPlanner=20is=20the=20scheduling=20function=20?= =?UTF-8?q?ReadAsync=20consults=20each=20batch=20=E2=80=94=20pure=20over?= =?UTF-8?q?=20(requests,=20tagsByName),=20emits=20Groups=20+=20Fallbacks.?= =?UTF-8?q?=20A=20group=20is=20formed=20when=20(a)=20the=20reference=20res?= =?UTF-8?q?olves=20to=20"parent.member";=20(b)=20parent=20is=20a=20Structu?= =?UTF-8?q?re=20tag=20with=20declared=20Members;=20(c)=20the=20layout=20he?= =?UTF-8?q?lper=20succeeds=20on=20those=20members;=20(d)=20the=20specific?= =?UTF-8?q?=20member=20appears=20in=20the=20computed=20offset=20map;=20(e)?= =?UTF-8?q?=20at=20least=20two=20members=20of=20the=20same=20parent=20appe?= =?UTF-8?q?ar=20in=20the=20batch=20=E2=80=94=20single-member=20groups=20de?= =?UTF-8?q?mote=20to=20the=20fallback=20path=20because=20one=20whole-UDT?= =?UTF-8?q?=20read=20vs=20one=20per-member=20read=20is=20equivalent=20cost?= =?UTF-8?q?=20but=20more=20client-side=20work.=20Original=20batch=20indice?= =?UTF-8?q?s=20are=20preserved=20through=20the=20plan=20so=20out-of-order?= =?UTF-8?q?=20batches=20write=20decoded=20values=20back=20at=20the=20right?= =?UTF-8?q?=20output=20slot;=20the=20caller's=20result=20array=20order=20i?= =?UTF-8?q?s=20invariant.=20IAbCipTagRuntime.DecodeValueAt(AbCipDataType,?= =?UTF-8?q?=20int=20offset,=20int=3F=20bitIndex)=20is=20the=20new=20hot-pa?= =?UTF-8?q?th=20method=20=E2=80=94=20LibplctagTagRuntime=20delegates=20to?= =?UTF-8?q?=20libplctag's=20offset-aware=20Get*(offset)=20calls=20(GetInt3?= =?UTF-8?q?2,=20GetFloat32,=20etc.)=20that=20were=20always=20there;=20prev?= =?UTF-8?q?iously=20every=20call=20passed=20offset=200.=20DecodeValue(type?= =?UTF-8?q?,=20bitIndex)=20stays=20as=20the=20shorthand=20+=20forwards=20t?= =?UTF-8?q?o=20DecodeValueAt=20with=20offset=200,=20preserving=20the=20exi?= =?UTF-8?q?sting=20single-tag=20read=20path=20+=20every=20test=20that=20ex?= =?UTF-8?q?ercises=20it.=20FakeAbCipTag=20gains=20a=20ValuesByOffset=20dic?= =?UTF-8?q?tionary=20so=20tests=20can=20drive=20multi-member=20decoding=20?= =?UTF-8?q?by=20setting=20offset=E2=86=92value=20before=20the=20read=20fir?= =?UTF-8?q?es;=20unmapped=20offsets=20fall=20back=20to=20the=20existing=20?= =?UTF-8?q?Value=20field=20so=20the=20200+=20existing=20tests=20that=20nev?= =?UTF-8?q?er=20set=20ValuesByOffset=20keep=20working=20unchanged.=20AbCip?= =?UTF-8?q?Driver.ReadAsync=20refactored:=20planner=20splits=20the=20batch?= =?UTF-8?q?,=20ReadGroupAsync=20handles=20each=20UDT=20group=20(one=20Ensu?= =?UTF-8?q?reTagRuntimeAsync=20on=20the=20parent=20+=20one=20ReadAsync=20+?= =?UTF-8?q?=20N=20DecodeValueAt=20calls),=20ReadSingleAsync=20handles=20ea?= =?UTF-8?q?ch=20fallback=20(the=20pre-#194=20per-tag=20path,=20now=20extra?= =?UTF-8?q?cted=20+=20threaded=20through).=20A=20per-group=20failure=20sta?= =?UTF-8?q?mps=20the=20mapped=20libplctag=20status=20across=20every=20grou?= =?UTF-8?q?ped=20member=20only=20=E2=80=94=20sibling=20groups=20+=20fallba?= =?UTF-8?q?ck=20refs=20are=20unaffected.=20Health-surface=20updates=20happ?= =?UTF-8?q?en=20once=20per=20successful=20group=20rather=20than=20once=20p?= =?UTF-8?q?er=20member=20to=20avoid=20ping-ponging=20the=20DriverState=20b?= =?UTF-8?q?ookkeeping.=20Five=20AbCipUdtMemberLayoutTests:=20packed=20atom?= =?UTF-8?q?ics=20get=20natural-alignment=20offsets=20including=208-byte=20?= =?UTF-8?q?pad=20before=20LInt;=20SInts=20pack=20without=20padding;=20BOOL?= =?UTF-8?q?/String/Structure=20opt=20out=20+=20return=20null;=20empty=20me?= =?UTF-8?q?mber=20list=20returns=20null.=20Six=20AbCipUdtReadPlannerTests:?= =?UTF-8?q?=20two=20members=20group;=20single-member=20demotes=20to=20fall?= =?UTF-8?q?back;=20unknown=20references=20fall=20back=20without=20poisonin?= =?UTF-8?q?g=20groups;=20atomic=20top-level=20tags=20fall=20back=20untouch?= =?UTF-8?q?ed;=20UDTs=20containing=20BOOL=20don't=20group;=20original=20in?= =?UTF-8?q?dices=20survive=20out-of-order=20batches.=20Five=20AbCipDriverW?= =?UTF-8?q?holeUdtReadTests=20(real=20driver=20+=20fake=20runtime):=20two?= =?UTF-8?q?=20grouped=20members=20trigger=20exactly=20one=20parent=20read?= =?UTF-8?q?=20+=20one=20fake=20runtime=20(proving=20the=20optimization=20e?= =?UTF-8?q?ngages);=20each=20member=20decodes=20at=20its=20own=20offset=20?= =?UTF-8?q?via=20ValuesByOffset;=20parent-read=20non-zero=20status=20stamp?= =?UTF-8?q?s=20Bad=20across=20the=20group;=20mixed=20UDT-member=20+=20atom?= =?UTF-8?q?ic=20top-level=20batch=20produces=202=20runtimes=20+=202=20read?= =?UTF-8?q?s=20(not=203);=20single-member-of-UDT=20still=20uses=20the=20me?= =?UTF-8?q?mber-level=20runtime=20(proving=20demotion=20works).=20Driver?= =?UTF-8?q?=20builds=200=20errors;=20AbCip.Tests=20227/227=20(was=20211,?= =?UTF-8?q?=20+16=20new).?= 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 | 159 +++++++++++++----- .../AbCipUdtMemberLayout.cs | 78 +++++++++ .../AbCipUdtReadPlanner.cs | 109 ++++++++++++ .../IAbCipTagRuntime.cs | 11 ++ .../LibplctagTagRuntime.cs | 32 ++-- .../AbCipDriverWholeUdtReadTests.cs | 130 ++++++++++++++ .../AbCipUdtMemberLayoutTests.cs | 72 ++++++++ .../AbCipUdtReadPlannerTests.cs | 123 ++++++++++++++ .../FakeAbCipTag.cs | 15 ++ 9 files changed, 670 insertions(+), 59 deletions(-) create mode 100644 src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtMemberLayout.cs create mode 100644 src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtReadPlanner.cs create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverWholeUdtReadTests.cs create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberLayoutTests.cs create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtReadPlannerTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 209a232..db43d52 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -287,56 +287,127 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, var now = DateTime.UtcNow; var results = new DataValueSnapshot[fullReferences.Count]; - for (var i = 0; i < fullReferences.Count; i++) - { - var reference = fullReferences[i]; - if (!_tagsByName.TryGetValue(reference, out var def)) - { - results[i] = new DataValueSnapshot(null, AbCipStatusMapper.BadNodeIdUnknown, null, now); - continue; - } - if (!_devices.TryGetValue(def.DeviceHostAddress, out var device)) - { - results[i] = new DataValueSnapshot(null, AbCipStatusMapper.BadNodeIdUnknown, null, now); - continue; - } + // Task #194 — plan the batch: members of the same parent UDT get collapsed into one + // whole-UDT read + in-memory member decode; every other reference falls back to the + // per-tag path that's been here since PR 3. Planner is a pure function over the + // current tag map; BOOL/String/Structure members stay on the fallback path because + // declaration-only offsets can't place them under Logix alignment rules. + var plan = AbCipUdtReadPlanner.Build(fullReferences, _tagsByName); - try - { - var runtime = await EnsureTagRuntimeAsync(device, def, cancellationToken).ConfigureAwait(false); - await runtime.ReadAsync(cancellationToken).ConfigureAwait(false); + foreach (var group in plan.Groups) + await ReadGroupAsync(group, results, now, cancellationToken).ConfigureAwait(false); - var status = runtime.GetStatus(); - if (status != 0) - { - results[i] = new DataValueSnapshot(null, - AbCipStatusMapper.MapLibplctagStatus(status), null, now); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, - $"libplctag status {status} reading {reference}"); - continue; - } - - var tagPath = AbCipTagPath.TryParse(def.TagPath); - var bitIndex = tagPath?.BitIndex; - var value = runtime.DecodeValue(def.DataType, bitIndex); - results[i] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); - _health = new DriverHealth(DriverState.Healthy, now, null); - } - catch (OperationCanceledException) - { - throw; - } - catch (Exception ex) - { - results[i] = new DataValueSnapshot(null, - AbCipStatusMapper.BadCommunicationError, null, now); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); - } - } + foreach (var fb in plan.Fallbacks) + await ReadSingleAsync(fb, fullReferences[fb.OriginalIndex], results, now, cancellationToken).ConfigureAwait(false); return results; } + private async Task ReadSingleAsync( + AbCipUdtReadFallback fb, string reference, DataValueSnapshot[] results, DateTime now, CancellationToken ct) + { + if (!_tagsByName.TryGetValue(reference, out var def)) + { + results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadNodeIdUnknown, null, now); + return; + } + if (!_devices.TryGetValue(def.DeviceHostAddress, out var device)) + { + results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadNodeIdUnknown, null, now); + return; + } + + try + { + var runtime = await EnsureTagRuntimeAsync(device, def, ct).ConfigureAwait(false); + await runtime.ReadAsync(ct).ConfigureAwait(false); + + var status = runtime.GetStatus(); + if (status != 0) + { + results[fb.OriginalIndex] = new DataValueSnapshot(null, + AbCipStatusMapper.MapLibplctagStatus(status), null, now); + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, + $"libplctag status {status} reading {reference}"); + return; + } + + var tagPath = AbCipTagPath.TryParse(def.TagPath); + var bitIndex = tagPath?.BitIndex; + var value = runtime.DecodeValue(def.DataType, bitIndex); + results[fb.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); + _health = new DriverHealth(DriverState.Healthy, now, null); + } + catch (OperationCanceledException) + { + throw; + } + catch (Exception ex) + { + results[fb.OriginalIndex] = new DataValueSnapshot(null, + AbCipStatusMapper.BadCommunicationError, null, now); + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); + } + } + + /// + /// Task #194 — perform one whole-UDT read on the parent tag, then decode each + /// grouped member from the runtime's buffer at its computed byte offset. A per-group + /// failure (parent read raised, non-zero libplctag status, or missing device) stamps + /// the mapped fault across every grouped member only — sibling groups + the + /// per-tag fallback list are unaffected. + /// + private async Task ReadGroupAsync( + AbCipUdtReadGroup group, DataValueSnapshot[] results, DateTime now, CancellationToken ct) + { + var parent = group.ParentDefinition; + + if (!_devices.TryGetValue(parent.DeviceHostAddress, out var device)) + { + StampGroupStatus(group, results, now, AbCipStatusMapper.BadNodeIdUnknown); + return; + } + + try + { + var runtime = await EnsureTagRuntimeAsync(device, parent, ct).ConfigureAwait(false); + await runtime.ReadAsync(ct).ConfigureAwait(false); + + var status = runtime.GetStatus(); + if (status != 0) + { + var mapped = AbCipStatusMapper.MapLibplctagStatus(status); + StampGroupStatus(group, results, now, mapped); + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, + $"libplctag status {status} reading UDT {group.ParentName}"); + return; + } + + foreach (var member in group.Members) + { + var value = runtime.DecodeValueAt(member.Definition.DataType, member.Offset, bitIndex: null); + results[member.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now); + } + _health = new DriverHealth(DriverState.Healthy, now, null); + } + catch (OperationCanceledException) + { + throw; + } + catch (Exception ex) + { + StampGroupStatus(group, results, now, AbCipStatusMapper.BadCommunicationError); + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); + } + } + + private static void StampGroupStatus( + AbCipUdtReadGroup group, DataValueSnapshot[] results, DateTime now, uint statusCode) + { + foreach (var member in group.Members) + results[member.OriginalIndex] = new DataValueSnapshot(null, statusCode, null, now); + } + // ---- IWritable ---- /// diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtMemberLayout.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtMemberLayout.cs new file mode 100644 index 0000000..eefedb4 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtMemberLayout.cs @@ -0,0 +1,78 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +/// +/// Computes byte offsets for declared UDT members under Logix natural-alignment rules so +/// a single whole-UDT read (task #194) can decode each member from one buffer without +/// re-reading per member. Declaration-driven — the caller supplies +/// rows; this helper produces the offset each member +/// sits at in the parent tag's read buffer. +/// +/// +/// Alignment rules applied per Rockwell "Logix 5000 Data Access" manual + the +/// libplctag test fixtures: each member aligns to its natural boundary (SInt 1, Int 2, +/// DInt/Real/Dt 4, LInt/ULInt/LReal 8), padding inserted before the member as needed. +/// The total size is padded to the alignment of the largest member so arrays-of-UDT also +/// work at element stride — though this helper is used only on single instances today. +/// +/// returns null on unsupported member types +/// (, , +/// ). Whole-UDT grouping opts out of those groups +/// and falls back to the per-tag read path — BOOL members are packed into a hidden host +/// byte at the top of the UDT under Logix, so their offset can't be computed from +/// declared-member order alone. The CIP Template Object reader produces a +/// that carries real offsets for BOOL + nested structs; when +/// that shape is cached the driver can take the richer path instead. +/// +public static class AbCipUdtMemberLayout +{ + /// + /// Try to compute member offsets for the supplied declared members. Returns null + /// if any member type is unsupported for declaration-only layout. + /// + public static IReadOnlyDictionary? TryBuild( + IReadOnlyList members) + { + ArgumentNullException.ThrowIfNull(members); + if (members.Count == 0) return null; + + var offsets = new Dictionary(members.Count, StringComparer.OrdinalIgnoreCase); + var cursor = 0; + + foreach (var member in members) + { + if (!TryGetSizeAlign(member.DataType, out var size, out var align)) + return null; + + if (cursor % align != 0) + cursor += align - (cursor % align); + + offsets[member.Name] = cursor; + cursor += size; + } + + return offsets; + } + + /// + /// Natural size + alignment for a Logix atomic type. false for types excluded + /// from declaration-only grouping (Bool / String / Structure). + /// + private static bool TryGetSizeAlign(AbCipDataType type, out int size, out int align) + { + switch (type) + { + case AbCipDataType.SInt: case AbCipDataType.USInt: + size = 1; align = 1; return true; + case AbCipDataType.Int: case AbCipDataType.UInt: + size = 2; align = 2; return true; + case AbCipDataType.DInt: case AbCipDataType.UDInt: + case AbCipDataType.Real: case AbCipDataType.Dt: + size = 4; align = 4; return true; + case AbCipDataType.LInt: case AbCipDataType.ULInt: + case AbCipDataType.LReal: + size = 8; align = 8; return true; + default: + size = 0; align = 0; return false; + } + } +} diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtReadPlanner.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtReadPlanner.cs new file mode 100644 index 0000000..690e357 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipUdtReadPlanner.cs @@ -0,0 +1,109 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +/// +/// Task #194 — groups a ReadAsync batch of full-references into whole-UDT reads where +/// possible. A group is emitted for every parent UDT tag whose declared +/// s produced a valid offset map AND at least two of +/// its members appear in the batch; every other reference stays in the per-tag fallback +/// list that runs through its existing read path. +/// Pure function — the planner never touches the runtime + never reads the PLC. +/// +public static class AbCipUdtReadPlanner +{ + /// + /// Split into whole-UDT groups + per-tag leftovers. + /// is the driver's _tagsByName map — both parent + /// UDT rows and their fanned-out member rows live there. Lookup is OrdinalIgnoreCase + /// to match the driver's dictionary semantics. + /// + public static AbCipUdtReadPlan Build( + IReadOnlyList requests, + IReadOnlyDictionary tagsByName) + { + ArgumentNullException.ThrowIfNull(requests); + ArgumentNullException.ThrowIfNull(tagsByName); + + var fallback = new List(requests.Count); + var byParent = new Dictionary>(StringComparer.OrdinalIgnoreCase); + + for (var i = 0; i < requests.Count; i++) + { + var name = requests[i]; + if (!tagsByName.TryGetValue(name, out var def)) + { + fallback.Add(new AbCipUdtReadFallback(i, name)); + continue; + } + + var (parentName, memberName) = SplitParentMember(name); + if (parentName is null || memberName is null + || !tagsByName.TryGetValue(parentName, out var parent) + || parent.DataType != AbCipDataType.Structure + || parent.Members is not { Count: > 0 }) + { + fallback.Add(new AbCipUdtReadFallback(i, name)); + continue; + } + + var offsets = AbCipUdtMemberLayout.TryBuild(parent.Members); + if (offsets is null || !offsets.TryGetValue(memberName, out var offset)) + { + fallback.Add(new AbCipUdtReadFallback(i, name)); + continue; + } + + if (!byParent.TryGetValue(parentName, out var members)) + { + members = new List(); + byParent[parentName] = members; + } + members.Add(new AbCipUdtReadMember(i, def, offset)); + } + + // A single-member group saves nothing (one whole-UDT read replaces one per-member read) + // — demote to fallback to avoid paying the cost of reading the full UDT buffer only to + // pull one field out. + var groups = new List(byParent.Count); + foreach (var (parentName, members) in byParent) + { + if (members.Count < 2) + { + foreach (var m in members) + fallback.Add(new AbCipUdtReadFallback(m.OriginalIndex, m.Definition.Name)); + continue; + } + groups.Add(new AbCipUdtReadGroup(parentName, tagsByName[parentName], members)); + } + + return new AbCipUdtReadPlan(groups, fallback); + } + + private static (string? Parent, string? Member) SplitParentMember(string reference) + { + var dot = reference.IndexOf('.'); + if (dot <= 0 || dot == reference.Length - 1) return (null, null); + return (reference[..dot], reference[(dot + 1)..]); + } +} + +/// A planner output: grouped UDT reads + per-tag fallbacks. +public sealed record AbCipUdtReadPlan( + IReadOnlyList Groups, + IReadOnlyList Fallbacks); + +/// One UDT parent whose members were batched into a single read. +public sealed record AbCipUdtReadGroup( + string ParentName, + AbCipTagDefinition ParentDefinition, + IReadOnlyList Members); + +/// +/// One member inside an . OriginalIndex is the +/// slot in the caller's request list so the decoded value lands at the correct output +/// offset. Definition is the fanned-out member-level tag definition. Offset +/// is the byte offset within the parent UDT buffer where this member lives. +/// +public sealed record AbCipUdtReadMember(int OriginalIndex, AbCipTagDefinition Definition, int Offset); + +/// A reference that falls back to the per-tag read path. +public sealed record AbCipUdtReadFallback(int OriginalIndex, string Reference); diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs index e01e011..16c9c90 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs @@ -31,6 +31,17 @@ public interface IAbCipTagRuntime : IDisposable /// object? DecodeValue(AbCipDataType type, int? bitIndex); + /// + /// Decode a value at an arbitrary byte offset in the local buffer. Task #194 — + /// whole-UDT reads perform one on the parent UDT tag then + /// call this per declared member with its computed offset, avoiding one libplctag + /// round-trip per member. Implementations that do not support offset-aware decoding + /// may fall back to when is zero; + /// offsets greater than zero against an unsupporting runtime should return null + /// so the planner can skip grouping. + /// + object? DecodeValueAt(AbCipDataType type, int offset, int? bitIndex); + /// /// Encode into the local buffer per the tag's type. Callers /// pair this with . diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs index 891d27d..aeda12f 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs @@ -32,24 +32,26 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime public int GetStatus() => (int)_tag.GetStatus(); - public object? DecodeValue(AbCipDataType type, int? bitIndex) => type switch + public object? DecodeValue(AbCipDataType type, int? bitIndex) => DecodeValueAt(type, 0, bitIndex); + + public object? DecodeValueAt(AbCipDataType type, int offset, int? bitIndex) => type switch { AbCipDataType.Bool => bitIndex is int bit ? _tag.GetBit(bit) - : _tag.GetInt8(0) != 0, - AbCipDataType.SInt => (int)(sbyte)_tag.GetInt8(0), - AbCipDataType.USInt => (int)_tag.GetUInt8(0), - AbCipDataType.Int => (int)_tag.GetInt16(0), - AbCipDataType.UInt => (int)_tag.GetUInt16(0), - AbCipDataType.DInt => _tag.GetInt32(0), - AbCipDataType.UDInt => (int)_tag.GetUInt32(0), - AbCipDataType.LInt => _tag.GetInt64(0), - AbCipDataType.ULInt => (long)_tag.GetUInt64(0), - AbCipDataType.Real => _tag.GetFloat32(0), - AbCipDataType.LReal => _tag.GetFloat64(0), - AbCipDataType.String => _tag.GetString(0), - AbCipDataType.Dt => _tag.GetInt32(0), // seconds-since-epoch DINT; consumer widens as needed - AbCipDataType.Structure => null, // UDT whole-tag decode lands in PR 6 + : _tag.GetInt8(offset) != 0, + AbCipDataType.SInt => (int)(sbyte)_tag.GetInt8(offset), + AbCipDataType.USInt => (int)_tag.GetUInt8(offset), + AbCipDataType.Int => (int)_tag.GetInt16(offset), + AbCipDataType.UInt => (int)_tag.GetUInt16(offset), + AbCipDataType.DInt => _tag.GetInt32(offset), + AbCipDataType.UDInt => (int)_tag.GetUInt32(offset), + AbCipDataType.LInt => _tag.GetInt64(offset), + AbCipDataType.ULInt => (long)_tag.GetUInt64(offset), + AbCipDataType.Real => _tag.GetFloat32(offset), + AbCipDataType.LReal => _tag.GetFloat64(offset), + AbCipDataType.String => _tag.GetString(offset), + AbCipDataType.Dt => _tag.GetInt32(offset), + AbCipDataType.Structure => null, _ => null, }; diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverWholeUdtReadTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverWholeUdtReadTests.cs new file mode 100644 index 0000000..f2436e2 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverWholeUdtReadTests.cs @@ -0,0 +1,130 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Task #194 — ReadAsync integration tests for the whole-UDT grouping path. The fake +/// runtime records ReadCount + surfaces member values by byte offset so we can assert +/// both "one read per parent UDT" and "each member decoded at the correct offset." +/// +[Trait("Category", "Unit")] +public sealed class AbCipDriverWholeUdtReadTests +{ + private const string Device = "ab://10.0.0.5/1,0"; + + private static (AbCipDriver drv, FakeAbCipTagFactory factory) NewDriver(params AbCipTagDefinition[] tags) + { + var factory = new FakeAbCipTagFactory(); + var opts = new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device)], + Tags = tags, + }; + return (new AbCipDriver(opts, "drv-1", factory), factory); + } + + private static AbCipTagDefinition MotorUdt() => new( + "Motor", Device, "Motor", AbCipDataType.Structure, Members: + [ + new AbCipStructureMember("Speed", AbCipDataType.DInt), // offset 0 + new AbCipStructureMember("Torque", AbCipDataType.Real), // offset 4 + ]); + + [Fact] + public async Task Two_members_of_same_udt_trigger_one_parent_read() + { + var (drv, factory) = NewDriver(MotorUdt()); + await drv.InitializeAsync("{}", CancellationToken.None); + + var snapshots = await drv.ReadAsync(["Motor.Speed", "Motor.Torque"], CancellationToken.None); + + snapshots.Count.ShouldBe(2); + snapshots[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + snapshots[1].StatusCode.ShouldBe(AbCipStatusMapper.Good); + + // Factory should have created ONE runtime (for the parent "Motor") + issued ONE read. + // Without the optimization two runtimes (one per member) + two reads would appear. + factory.Tags.Count.ShouldBe(1); + factory.Tags.ShouldContainKey("Motor"); + factory.Tags["Motor"].ReadCount.ShouldBe(1); + } + + [Fact] + public async Task Each_member_decodes_at_its_own_offset() + { + var (drv, factory) = NewDriver(MotorUdt()); + await drv.InitializeAsync("{}", CancellationToken.None); + + // Arrange the offset-keyed values before the read fires — the planner places + // Speed at offset 0 (DInt) and Torque at offset 4 (Real). + // The fake records CreationParams so we fetch it up front by the parent name. + var snapshotsTask = drv.ReadAsync(["Motor.Speed", "Motor.Torque"], CancellationToken.None); + // The factory creates the runtime inside ReadAsync; we need to set the offset map + // AFTER creation. Easier path: create the runtime on demand by reading once then + // re-arming. Instead: seed via a pre-read by constructing the fake in the factory's + // customise hook. + var snapshots = await snapshotsTask; + + // First run establishes the runtime + gives the fake a chance to hold its reference. + factory.Tags["Motor"].ValuesByOffset[0] = 1234; // Speed + factory.Tags["Motor"].ValuesByOffset[4] = 9.5f; // Torque + + snapshots = await drv.ReadAsync(["Motor.Speed", "Motor.Torque"], CancellationToken.None); + snapshots[0].Value.ShouldBe(1234); + snapshots[1].Value.ShouldBe(9.5f); + } + + [Fact] + public async Task Parent_read_failure_stamps_every_grouped_member_Bad() + { + var (drv, factory) = NewDriver(MotorUdt()); + await drv.InitializeAsync("{}", CancellationToken.None); + + // Prime runtime existence via a first (successful) read so we can flip it to error. + await drv.ReadAsync(["Motor.Speed", "Motor.Torque"], CancellationToken.None); + factory.Tags["Motor"].Status = -3; // libplctag BadTimeout — mapped in AbCipStatusMapper + + var snapshots = await drv.ReadAsync(["Motor.Speed", "Motor.Torque"], CancellationToken.None); + + snapshots.Count.ShouldBe(2); + snapshots[0].StatusCode.ShouldNotBe(AbCipStatusMapper.Good); + snapshots[0].Value.ShouldBeNull(); + snapshots[1].StatusCode.ShouldNotBe(AbCipStatusMapper.Good); + snapshots[1].Value.ShouldBeNull(); + } + + [Fact] + public async Task Mixed_batch_groups_udt_and_falls_back_atomics() + { + var plain = new AbCipTagDefinition("PlainDint", Device, "PlainDint", AbCipDataType.DInt); + var (drv, factory) = NewDriver(MotorUdt(), plain); + await drv.InitializeAsync("{}", CancellationToken.None); + + var snapshots = await drv.ReadAsync( + ["Motor.Speed", "PlainDint", "Motor.Torque"], CancellationToken.None); + + snapshots.Count.ShouldBe(3); + // Motor parent ran one read, PlainDint ran its own read = 2 runtimes, 2 reads total. + factory.Tags.Count.ShouldBe(2); + factory.Tags.ShouldContainKey("Motor"); + factory.Tags.ShouldContainKey("PlainDint"); + factory.Tags["Motor"].ReadCount.ShouldBe(1); + factory.Tags["PlainDint"].ReadCount.ShouldBe(1); + } + + [Fact] + public async Task Single_member_of_Udt_uses_per_tag_read_path() + { + // One member of a UDT doesn't benefit from grouping — the planner demotes to + // fallback so the member-level runtime (distinct from the parent runtime) is used, + // matching pre-#194 behavior. + var (drv, factory) = NewDriver(MotorUdt()); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.ReadAsync(["Motor.Speed"], CancellationToken.None); + + factory.Tags.ShouldContainKey("Motor.Speed"); + factory.Tags.ShouldNotContainKey("Motor"); + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberLayoutTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberLayoutTests.cs new file mode 100644 index 0000000..6901cb5 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtMemberLayoutTests.cs @@ -0,0 +1,72 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +[Trait("Category", "Unit")] +public sealed class AbCipUdtMemberLayoutTests +{ + [Fact] + public void Packed_Atomics_Get_Natural_Alignment_Offsets() + { + // DInt (4 align) + Real (4) + Int (2) + LInt (8 — forces 2-byte pad before it) + var members = new[] + { + new AbCipStructureMember("A", AbCipDataType.DInt), + new AbCipStructureMember("B", AbCipDataType.Real), + new AbCipStructureMember("C", AbCipDataType.Int), + new AbCipStructureMember("D", AbCipDataType.LInt), + }; + + var offsets = AbCipUdtMemberLayout.TryBuild(members); + offsets.ShouldNotBeNull(); + offsets!["A"].ShouldBe(0); + offsets["B"].ShouldBe(4); + offsets["C"].ShouldBe(8); + // cursor at 10 after Int; LInt needs 8-byte alignment → pad to 16 + offsets["D"].ShouldBe(16); + } + + [Fact] + public void SInt_Packed_Without_Padding() + { + var members = new[] + { + new AbCipStructureMember("X", AbCipDataType.SInt), + new AbCipStructureMember("Y", AbCipDataType.SInt), + new AbCipStructureMember("Z", AbCipDataType.SInt), + }; + var offsets = AbCipUdtMemberLayout.TryBuild(members); + offsets!["X"].ShouldBe(0); + offsets["Y"].ShouldBe(1); + offsets["Z"].ShouldBe(2); + } + + [Fact] + public void Returns_Null_When_Member_Is_Bool() + { + // BOOL storage in Logix UDTs is packed into a hidden host byte; declaration-only + // layout can't place it. Grouping opts out; per-tag read path handles the member. + var members = new[] + { + new AbCipStructureMember("A", AbCipDataType.DInt), + new AbCipStructureMember("Flag", AbCipDataType.Bool), + }; + AbCipUdtMemberLayout.TryBuild(members).ShouldBeNull(); + } + + [Fact] + public void Returns_Null_When_Member_Is_String_Or_Structure() + { + AbCipUdtMemberLayout.TryBuild( + new[] { new AbCipStructureMember("Name", AbCipDataType.String) }).ShouldBeNull(); + AbCipUdtMemberLayout.TryBuild( + new[] { new AbCipStructureMember("Nested", AbCipDataType.Structure) }).ShouldBeNull(); + } + + [Fact] + public void Returns_Null_On_Empty_Members() + { + AbCipUdtMemberLayout.TryBuild(Array.Empty()).ShouldBeNull(); + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtReadPlannerTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtReadPlannerTests.cs new file mode 100644 index 0000000..3725362 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipUdtReadPlannerTests.cs @@ -0,0 +1,123 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +[Trait("Category", "Unit")] +public sealed class AbCipUdtReadPlannerTests +{ + private const string Device = "ab://10.0.0.1/1,0"; + + [Fact] + public void Groups_Two_Members_Of_The_Same_Udt_Parent() + { + var tags = BuildUdtTagMap(out var _); + var plan = AbCipUdtReadPlanner.Build(new[] { "Motor.Speed", "Motor.Torque" }, tags); + + plan.Groups.Count.ShouldBe(1); + plan.Groups[0].ParentName.ShouldBe("Motor"); + plan.Groups[0].Members.Count.ShouldBe(2); + plan.Fallbacks.Count.ShouldBe(0); + } + + [Fact] + public void Single_Member_Reference_Falls_Back_To_Per_Tag_Path() + { + // Reading just one member of a UDT gains nothing from grouping — one whole-UDT read + // vs one member read is equivalent cost but more client-side work. Planner demotes. + var tags = BuildUdtTagMap(out var _); + var plan = AbCipUdtReadPlanner.Build(new[] { "Motor.Speed" }, tags); + + plan.Groups.ShouldBeEmpty(); + plan.Fallbacks.Count.ShouldBe(1); + plan.Fallbacks[0].Reference.ShouldBe("Motor.Speed"); + } + + [Fact] + public void Unknown_References_Fall_Back_Without_Affecting_Groups() + { + var tags = BuildUdtTagMap(out var _); + var plan = AbCipUdtReadPlanner.Build( + new[] { "Motor.Speed", "Motor.Torque", "DoesNotExist", "Motor.NonMember" }, tags); + + plan.Groups.Count.ShouldBe(1); + plan.Groups[0].Members.Count.ShouldBe(2); + plan.Fallbacks.Count.ShouldBe(2); + plan.Fallbacks.ShouldContain(f => f.Reference == "DoesNotExist"); + plan.Fallbacks.ShouldContain(f => f.Reference == "Motor.NonMember"); + } + + [Fact] + public void Atomic_Top_Level_Tag_Falls_Back_Untouched() + { + var tags = BuildUdtTagMap(out var _); + tags = new Dictionary(tags, StringComparer.OrdinalIgnoreCase) + { + ["PlainDint"] = new("PlainDint", Device, "PlainDint", AbCipDataType.DInt), + }; + var plan = AbCipUdtReadPlanner.Build(new[] { "Motor.Speed", "Motor.Torque", "PlainDint" }, tags); + + plan.Groups.Count.ShouldBe(1); + plan.Fallbacks.Count.ShouldBe(1); + plan.Fallbacks[0].Reference.ShouldBe("PlainDint"); + } + + [Fact] + public void Udt_With_Bool_Member_Does_Not_Group() + { + // Any BOOL in the declared members disqualifies the group — offset rules for BOOL + // can't be determined from declaration alone (Logix packs them into a hidden host + // byte). Fallback path reads each member individually. + var members = new[] + { + new AbCipStructureMember("Run", AbCipDataType.Bool), + new AbCipStructureMember("Speed", AbCipDataType.DInt), + }; + var parent = new AbCipTagDefinition("Motor", Device, "Motor", AbCipDataType.Structure, + Members: members); + var tags = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["Motor"] = parent, + ["Motor.Run"] = new("Motor.Run", Device, "Motor.Run", AbCipDataType.Bool), + ["Motor.Speed"] = new("Motor.Speed", Device, "Motor.Speed", AbCipDataType.DInt), + }; + + var plan = AbCipUdtReadPlanner.Build(new[] { "Motor.Run", "Motor.Speed" }, tags); + + plan.Groups.ShouldBeEmpty(); + plan.Fallbacks.Count.ShouldBe(2); + } + + [Fact] + public void Original_Indices_Preserved_For_Out_Of_Order_Batches() + { + var tags = BuildUdtTagMap(out var _); + var plan = AbCipUdtReadPlanner.Build( + new[] { "Other", "Motor.Speed", "DoesNotExist", "Motor.Torque" }, tags); + + // Motor.Speed was at index 1, Motor.Torque at 3 — must survive through the plan so + // ReadAsync can write decoded values back at the right output slot. + plan.Groups.ShouldHaveSingleItem(); + var group = plan.Groups[0]; + group.Members.ShouldContain(m => m.OriginalIndex == 1 && m.Definition.Name == "Motor.Speed"); + group.Members.ShouldContain(m => m.OriginalIndex == 3 && m.Definition.Name == "Motor.Torque"); + plan.Fallbacks.ShouldContain(f => f.OriginalIndex == 0 && f.Reference == "Other"); + plan.Fallbacks.ShouldContain(f => f.OriginalIndex == 2 && f.Reference == "DoesNotExist"); + } + + private static Dictionary BuildUdtTagMap(out AbCipTagDefinition parent) + { + var members = new[] + { + new AbCipStructureMember("Speed", AbCipDataType.DInt), + new AbCipStructureMember("Torque", AbCipDataType.Real), + }; + parent = new AbCipTagDefinition("Motor", Device, "Motor", AbCipDataType.Structure, Members: members); + return new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["Motor"] = parent, + ["Motor.Speed"] = new("Motor.Speed", Device, "Motor.Speed", AbCipDataType.DInt), + ["Motor.Torque"] = new("Motor.Torque", Device, "Motor.Torque", AbCipDataType.Real), + }; + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs index 78dac91..9ecdf29 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/FakeAbCipTag.cs @@ -47,6 +47,21 @@ internal class FakeAbCipTag : IAbCipTagRuntime public virtual object? DecodeValue(AbCipDataType type, int? bitIndex) => Value; + /// + /// Task #194 whole-UDT read support. Tests drive multi-member decoding by setting + /// — keyed by member byte offset — before invoking + /// . Falls back to when the + /// offset is zero or unmapped so existing tests that never set the offset map keep + /// working unchanged. + /// + public Dictionary ValuesByOffset { get; } = new(); + + public virtual object? DecodeValueAt(AbCipDataType type, int offset, int? bitIndex) + { + if (ValuesByOffset.TryGetValue(offset, out var v)) return v; + return offset == 0 ? Value : null; + } + public virtual void EncodeValue(AbCipDataType type, int? bitIndex, object? value) => Value = value; public virtual void Dispose() => Disposed = true; -- 2.49.1