AbCip whole-UDT read optimization (#194) — declaration-driven member grouping collapses N per-member reads into one parent-UDT read + client-side decode. Closes task #194. On a batch that includes multiple members of the same hand-declared UDT tag, ReadAsync now issues one libplctag read on the parent + decodes each member from the runtime's buffer at its computed byte offset. A 6-member Motor UDT read goes from 6 libplctag round-trips to 1 — the Rockwell-suggested pattern for minimizing CIP request overhead on batch reads of UDT state (decision #11's follow-through on what the template decoder from task #179 was meant to enable). AbCipUdtMemberLayout is a pure-function helper that computes declared-member byte offsets under Logix natural-alignment rules (SInt 1-byte / Int 2-byte / DInt + Real + Dt 4-byte / LInt + ULInt + LReal 8-byte; alignment pad inserted before each member as needed). Opts out for BOOL / String / Structure members — BOOL storage in Logix UDTs packs into a hidden host byte whose position can't be computed from declaration-only info, and String members need length-prefix + STRING[82] fan-out which libplctag already handles via a per-tag DecodeValue path. The CIP Template Object shape from task #179 (when populated via FetchUdtShapeAsync) carries real offsets for those members — layering that richer path on top of the planner is a separate follow-up and does not change this PR's conservative behaviour. AbCipUdtReadPlanner is the scheduling function ReadAsync consults each batch — pure over (requests, tagsByName), emits Groups + Fallbacks. A group is formed when (a) the reference resolves to "parent.member"; (b) parent is a Structure tag with declared Members; (c) the layout helper succeeds on those members; (d) the specific member appears in the computed offset map; (e) at least two members of the same parent appear in the batch — single-member groups demote to the fallback path because one whole-UDT read vs one per-member read is equivalent cost but more client-side work. Original batch indices are preserved through the plan so out-of-order batches write decoded values back at the right output slot; the caller's result array order is invariant. IAbCipTagRuntime.DecodeValueAt(AbCipDataType, int offset, int? bitIndex) is the new hot-path method — LibplctagTagRuntime delegates to libplctag's offset-aware Get*(offset) calls (GetInt32, GetFloat32, etc.) that were always there; previously every call passed offset 0. DecodeValue(type, bitIndex) stays as the shorthand + forwards to DecodeValueAt with offset 0, preserving the existing single-tag read path + every test that exercises it. FakeAbCipTag gains a ValuesByOffset dictionary so tests can drive multi-member decoding by setting offset→value before the read fires; unmapped offsets fall back to the existing Value field so the 200+ existing tests that never set ValuesByOffset keep working unchanged. AbCipDriver.ReadAsync refactored: planner splits the batch, ReadGroupAsync handles each UDT group (one EnsureTagRuntimeAsync on the parent + one ReadAsync + N DecodeValueAt calls), ReadSingleAsync handles each fallback (the pre-#194 per-tag path, now extracted + threaded through). A per-group failure stamps the mapped libplctag status across every grouped member only — sibling groups + fallback refs are unaffected. Health-surface updates happen once per successful group rather than once per member to avoid ping-ponging the DriverState bookkeeping. Five AbCipUdtMemberLayoutTests: packed atomics get natural-alignment offsets including 8-byte pad before LInt; SInts pack without padding; BOOL/String/Structure opt out + return null; empty member list returns null. Six AbCipUdtReadPlannerTests: two members group; single-member demotes to fallback; unknown references fall back without poisoning groups; atomic top-level tags fall back untouched; UDTs containing BOOL don't group; original indices survive out-of-order batches. Five AbCipDriverWholeUdtReadTests (real driver + fake runtime): two grouped members trigger exactly one parent read + one fake runtime (proving the optimization engages); each member decodes at its own offset via ValuesByOffset; parent-read non-zero status stamps Bad across the group; mixed UDT-member + atomic top-level batch produces 2 runtimes + 2 reads (not 3); single-member-of-UDT still uses the member-level runtime (proving demotion works). Driver builds 0 errors; AbCip.Tests 227/227 (was 211, +16 new).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,130 @@
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// 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."
|
||||
/// </summary>
|
||||
[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");
|
||||
}
|
||||
}
|
||||
@@ -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<AbCipStructureMember>()).ShouldBeNull();
|
||||
}
|
||||
}
|
||||
@@ -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<string, AbCipTagDefinition>(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<string, AbCipTagDefinition>(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<string, AbCipTagDefinition> 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<string, AbCipTagDefinition>(StringComparer.OrdinalIgnoreCase)
|
||||
{
|
||||
["Motor"] = parent,
|
||||
["Motor.Speed"] = new("Motor.Speed", Device, "Motor.Speed", AbCipDataType.DInt),
|
||||
["Motor.Torque"] = new("Motor.Torque", Device, "Motor.Torque", AbCipDataType.Real),
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -47,6 +47,21 @@ internal class FakeAbCipTag : IAbCipTagRuntime
|
||||
|
||||
public virtual object? DecodeValue(AbCipDataType type, int? bitIndex) => Value;
|
||||
|
||||
/// <summary>
|
||||
/// Task #194 whole-UDT read support. Tests drive multi-member decoding by setting
|
||||
/// <see cref="ValuesByOffset"/> — keyed by member byte offset — before invoking
|
||||
/// <see cref="AbCipDriver.ReadAsync"/>. Falls back to <see cref="Value"/> when the
|
||||
/// offset is zero or unmapped so existing tests that never set the offset map keep
|
||||
/// working unchanged.
|
||||
/// </summary>
|
||||
public Dictionary<int, object?> 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;
|
||||
|
||||
Reference in New Issue
Block a user