feat(abcip): #611 discovered UDT member writes + whole-UDT refusal by decision
Item 2 — controller-discovered UDT members are writable on both transports once browse-committed and authored ReadWrite (writability is the operator's: the commit lands AccessLevel Read with no writable key). The write names the member (one CIP symbolic segment per member), its own type, its own bytes. BIT members are supported: Z-35162 (read-only probe) exposes a UDT BIT member as a 1-byte 0xC1 BOOL, so the controller applies the bit and the hidden host's other bits are never in the request. - AbCipDriver: pre-write width check on BOTH transports (libplctag patched part of its read buffer and sent it all): BadTypeMismatch, Bool over a wider value BadNotSupported; nothing sent, handle kept, health untouched, warn once. - A runtime NotSupportedException is a refusal: no longer latches Degraded. Item 3 — whole-UDT writes refused BadNotSupported before any runtime exists, by decision (a structure write overwrites hidden hosts + padding), documented. ab_server patch 0003: serve member paths from dotted flat tags (UdtSeg.*), and accept libplctag's pad byte on 1-byte writes (stock ab_server refused every libplctag BOOL/SINT write with 0x03). New AbCipUdtMemberWriteTests (both transports), AbCipDiscoveredMemberWriteTests, native request-byte theory, and a browse-commit mapper pin. Docs: AbCip.md Member writes / Whole-UDT writes / Known gaps, fixture docs, CLAUDE.md AB section. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
+222
@@ -0,0 +1,222 @@
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests;
|
||||
|
||||
/// <summary>
|
||||
/// Gitea #611 item 2 — writes to controller-discovered UDT members, end to end against ab_server on both
|
||||
/// transports. A discovered member reaches the driver the way browse-commit authors it: an ordinary raw tag
|
||||
/// whose <c>TagConfig</c> carries the member path as <c>tagPath</c> and the browsed atomic type as
|
||||
/// <c>dataType</c> (<c>RawBrowseCommitMapper</c>), with no <c>writable</c> key. ab_server has no structures,
|
||||
/// so the <c>UdtSeg.*</c> members are flat tags served on their member path by Docker patch 0003 — what is
|
||||
/// proved here is the wire: one symbolic segment per member (plus the element index for
|
||||
/// <c>UdtSeg.Points[2]</c>), the member's own CIP type, and a value that reads back through a fresh handle.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Writes: skipped unless <c>AB_SERVER_ALLOW_WRITES=1</c> (<see cref="IntegrationWriteGate"/>), so pointing
|
||||
/// <c>AB_SERVER_ENDPOINT</c> at a real controller never writes to it. BIT-member neighbour preservation is
|
||||
/// NOT claimed here: ab_server's <c>UdtSeg.Enable</c> / <c>UdtSeg.Ready</c> are separate tags, not bits of
|
||||
/// one host SINT, so such a test would pass whatever the driver did. The BIT-member argument rests on the
|
||||
/// driver addressing the member (never its hidden host) — pinned at the request-byte level by
|
||||
/// <c>NativeAbCipTagRuntimeTests</c> — see docs/drivers/AbCip.md §"Member writes".
|
||||
/// </remarks>
|
||||
[Trait("Category", "Integration")]
|
||||
public sealed class AbCipUdtMemberWriteTests
|
||||
{
|
||||
/// <summary>A discovered member authored writable writes, and the value reads back through a fresh driver.</summary>
|
||||
/// <param name="transport">The transport stack the driver runs on.</param>
|
||||
[AbServerTheory]
|
||||
[MemberData(nameof(TransportMatrix.Transports), MemberType = typeof(TransportMatrix))]
|
||||
public async Task A_discovered_member_write_reaches_the_member_and_reads_back(AbCipTransport transport)
|
||||
{
|
||||
IntegrationWriteGate.SkipUnlessWritesAllowed();
|
||||
var fixture = new AbServerFixture();
|
||||
var device = Device(fixture);
|
||||
// A per-run value so a stale server value from an earlier run cannot satisfy the read-back.
|
||||
var seed = Random.Shared.Next(1_000, 30_000);
|
||||
|
||||
var writer = await StartAsync(device, transport, "writer",
|
||||
Member("RampTime", "UdtSeg.RampTime", "Real"),
|
||||
Member("Id", "UdtSeg.ID", "DInt"),
|
||||
Member("Code", "UdtSeg.Sub.Code", "Int"),
|
||||
Member("Point2", "UdtSeg.Points[2]", "DInt"),
|
||||
Member("Enable", "UdtSeg.Enable", "Bool"));
|
||||
await RequireMemberTagsAsync(writer);
|
||||
try
|
||||
{
|
||||
var results = await writer.WriteAsync(
|
||||
[
|
||||
new WriteRequest("RampTime", seed + 0.5f),
|
||||
new WriteRequest("Id", seed),
|
||||
new WriteRequest("Code", (short)(seed % 30_000)),
|
||||
new WriteRequest("Point2", -seed),
|
||||
new WriteRequest("Enable", seed % 2 == 0),
|
||||
], CancellationToken.None);
|
||||
string[] names = ["RampTime", "Id", "Code", "Point2", "Enable"];
|
||||
results.Select((r, i) => $"{names[i]}=0x{r.StatusCode:X8}")
|
||||
.ShouldBe(names.Select(n => $"{n}=0x{AbCipStatusMapper.Good:X8}"));
|
||||
writer.GetHealth().State.ShouldBe(DriverState.Healthy);
|
||||
}
|
||||
finally
|
||||
{
|
||||
await writer.ShutdownAsync(CancellationToken.None);
|
||||
}
|
||||
|
||||
// A FRESH driver — new handles, nothing cached from the writer — proves the server holds the values.
|
||||
var reader = await StartAsync(device, transport, "reader",
|
||||
Member("RampTime", "UdtSeg.RampTime", "Real"),
|
||||
Member("Id", "UdtSeg.ID", "DInt"),
|
||||
Member("Code", "UdtSeg.Sub.Code", "Int"),
|
||||
Member("Point2", "UdtSeg.Points[2]", "DInt"),
|
||||
Member("Enable", "UdtSeg.Enable", "Bool"));
|
||||
try
|
||||
{
|
||||
var read = await reader.ReadAsync(["RampTime", "Id", "Code", "Point2", "Enable"], CancellationToken.None);
|
||||
read.Select(r => r.StatusCode).ShouldAllBe(s => s == AbCipStatusMapper.Good);
|
||||
Convert.ToSingle(read[0].Value).ShouldBe(seed + 0.5f);
|
||||
Convert.ToInt32(read[1].Value).ShouldBe(seed);
|
||||
Convert.ToInt32(read[2].Value).ShouldBe(seed % 30_000);
|
||||
Convert.ToInt32(read[3].Value).ShouldBe(-seed);
|
||||
Convert.ToBoolean(read[4].Value).ShouldBe(seed % 2 == 0);
|
||||
}
|
||||
finally
|
||||
{
|
||||
await reader.ShutdownAsync(CancellationToken.None);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// An element write to a member array touches that element only: the neighbours read back unchanged.
|
||||
/// </summary>
|
||||
/// <param name="transport">The transport stack the driver runs on.</param>
|
||||
[AbServerTheory]
|
||||
[MemberData(nameof(TransportMatrix.Transports), MemberType = typeof(TransportMatrix))]
|
||||
public async Task A_member_array_element_write_leaves_the_other_elements_alone(AbCipTransport transport)
|
||||
{
|
||||
IntegrationWriteGate.SkipUnlessWritesAllowed();
|
||||
var device = Device(new AbServerFixture());
|
||||
var seed = Random.Shared.Next(1_000, 30_000);
|
||||
var drv = await StartAsync(device, transport, "elements",
|
||||
Member("P0", "UdtSeg.Points[0]", "DInt"),
|
||||
Member("P1", "UdtSeg.Points[1]", "DInt"),
|
||||
Member("P2", "UdtSeg.Points[2]", "DInt"),
|
||||
Member("P3", "UdtSeg.Points[3]", "DInt"));
|
||||
await RequireMemberTagsAsync(drv, "P0");
|
||||
try
|
||||
{
|
||||
(await drv.WriteAsync(
|
||||
[new WriteRequest("P0", seed), new WriteRequest("P1", seed + 1),
|
||||
new WriteRequest("P2", seed + 2), new WriteRequest("P3", seed + 3)], CancellationToken.None))
|
||||
.Select(r => r.StatusCode).ShouldAllBe(s => s == AbCipStatusMapper.Good);
|
||||
|
||||
(await drv.WriteAsync([new WriteRequest("P2", -7)], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.Good);
|
||||
|
||||
var read = await drv.ReadAsync(["P0", "P1", "P2", "P3"], CancellationToken.None);
|
||||
read.Select(r => Convert.ToInt32(r.Value)).ShouldBe([seed, seed + 1, -7, seed + 3]);
|
||||
}
|
||||
finally
|
||||
{
|
||||
await drv.ShutdownAsync(CancellationToken.None);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A member authored narrower than the controller's type (INT over the DINT <c>UdtSeg.ID</c>) is refused
|
||||
/// <c>BadTypeMismatch</c> with nothing written, on BOTH transports — libplctag included, which would
|
||||
/// otherwise patch the low two bytes of the 4-byte buffer and send the DINT back.
|
||||
/// </summary>
|
||||
/// <param name="transport">The transport stack the driver runs on.</param>
|
||||
[AbServerTheory]
|
||||
[MemberData(nameof(TransportMatrix.Transports), MemberType = typeof(TransportMatrix))]
|
||||
public async Task A_width_mistyped_member_write_is_refused_and_writes_nothing(AbCipTransport transport)
|
||||
{
|
||||
IntegrationWriteGate.SkipUnlessWritesAllowed();
|
||||
var device = Device(new AbServerFixture());
|
||||
var seed = Random.Shared.Next(100_000, 1_000_000); // needs the high word, so a 2-byte patch would show
|
||||
var drv = await StartAsync(device, transport, "mistyped",
|
||||
Member("IdRight", "UdtSeg.ID", "DInt"),
|
||||
Member("IdWrong", "UdtSeg.ID", "Int"));
|
||||
await RequireMemberTagsAsync(drv, "IdRight");
|
||||
try
|
||||
{
|
||||
(await drv.WriteAsync([new WriteRequest("IdRight", seed)], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.Good);
|
||||
|
||||
(await drv.WriteAsync([new WriteRequest("IdWrong", (short)5)], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.BadTypeMismatch);
|
||||
|
||||
drv.GetHealth().State.ShouldBe(DriverState.Healthy);
|
||||
Convert.ToInt32((await drv.ReadAsync(["IdRight"], CancellationToken.None)).Single().Value).ShouldBe(seed);
|
||||
}
|
||||
finally
|
||||
{
|
||||
await drv.ShutdownAsync(CancellationToken.None);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>A discovered member the operator authored read-only (<c>writable:false</c>) is never written.</summary>
|
||||
/// <param name="transport">The transport stack the driver runs on.</param>
|
||||
[AbServerTheory]
|
||||
[MemberData(nameof(TransportMatrix.Transports), MemberType = typeof(TransportMatrix))]
|
||||
public async Task A_member_authored_read_only_is_not_written(AbCipTransport transport)
|
||||
{
|
||||
IntegrationWriteGate.SkipUnlessWritesAllowed();
|
||||
var device = Device(new AbServerFixture());
|
||||
var seed = Random.Shared.Next(1_000, 30_000);
|
||||
var drv = await StartAsync(device, transport, "readonly",
|
||||
Member("Id", "UdtSeg.ID", "DInt"),
|
||||
Member("IdRo", "UdtSeg.ID", "DInt", writable: false));
|
||||
await RequireMemberTagsAsync(drv);
|
||||
try
|
||||
{
|
||||
(await drv.WriteAsync([new WriteRequest("Id", seed)], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.Good);
|
||||
(await drv.WriteAsync([new WriteRequest("IdRo", seed + 1)], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.BadNotWritable);
|
||||
Convert.ToInt32((await drv.ReadAsync(["Id"], CancellationToken.None)).Single().Value).ShouldBe(seed);
|
||||
}
|
||||
finally
|
||||
{
|
||||
await drv.ShutdownAsync(CancellationToken.None);
|
||||
}
|
||||
}
|
||||
|
||||
private static string Device(AbServerFixture fixture) => $"ab://{fixture.Host}:{fixture.Port}/1,0";
|
||||
|
||||
/// <summary>A raw tag exactly as browse-commit authors a discovered member: tagPath + dataType, no writable key.</summary>
|
||||
private static RawTagEntry Member(string rawPath, string tagPath, string dataType, bool? writable = null) =>
|
||||
new(rawPath,
|
||||
writable is { } w
|
||||
? $$"""{"tagPath":"{{tagPath}}","dataType":"{{dataType}}","writable":{{(w ? "true" : "false")}}}"""
|
||||
: $$"""{"tagPath":"{{tagPath}}","dataType":"{{dataType}}"}""",
|
||||
WriteIdempotent: false);
|
||||
|
||||
private static async Task<AbCipDriver> StartAsync(
|
||||
string device, AbCipTransport transport, string id, params RawTagEntry[] tags)
|
||||
{
|
||||
var drv = new AbCipDriver(new AbCipDriverOptions
|
||||
{
|
||||
Devices = [new AbCipDeviceOptions(device, AbCipPlcFamily.ControlLogix)],
|
||||
RawTags = [.. tags.Select(t => t with { DeviceName = device })],
|
||||
Timeout = TimeSpan.FromSeconds(5),
|
||||
Transport = transport,
|
||||
Probe = new AbCipProbeOptions { Enabled = false },
|
||||
}, $"drv-udtwrite-{id}-{transport}");
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
return drv;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Fails (not skips) with a rebuild hint when the running ab_server predates patch 0003: write tests are
|
||||
/// opt-in, so whoever opted in is pointing at a simulator that should carry the member tags.
|
||||
/// </summary>
|
||||
private static async Task RequireMemberTagsAsync(AbCipDriver drv, string probe = "Id")
|
||||
{
|
||||
var status = (await drv.ReadAsync([probe], CancellationToken.None)).Single().StatusCode;
|
||||
if (status == AbCipStatusMapper.BadNodeIdUnknown)
|
||||
Assert.Fail("ab_server does not serve the UdtSeg.* member tags — rebuild the image from Docker/ " +
|
||||
"(patch 0003) and run the controllogix profile.");
|
||||
}
|
||||
}
|
||||
@@ -72,7 +72,25 @@ all). `AbCip_WithNoAuthoredTags_ReportsTheControllerReachable` in
|
||||
`Host.IntegrationTests` is the pin, and it needs the fixture **rebuilt** from this
|
||||
patch, not merely running.
|
||||
|
||||
**Bumping `LIBPLCTAG_TAG` means re-checking both patches against the new tag.** It
|
||||
### 0003 — member paths, and libplctag's pad byte
|
||||
|
||||
`patches/0003-ab_server-resolve-member-paths-against-dotted-flat-tags.patch` does two
|
||||
things for the discovered-UDT-member write suite (`AbCipUdtMemberWriteTests`, Gitea #611):
|
||||
|
||||
- **Member paths.** A Logix member path (`Seg.Sub.Code`) arrives as one CIP symbolic
|
||||
segment per member. ab_server has no structures, so the patch joins consecutive symbolic
|
||||
segments with `.` and looks up a flat tag whose `--tag` name is that dotted path (the name
|
||||
charset now admits `.`); element segments after the last member work as before
|
||||
(`UdtSeg.Points[2]`). This proves the wire of a member read/write — not template offsets
|
||||
and not BIT-member semantics (each `UdtSeg.*` BOOL is its own byte, not a bit of a hidden
|
||||
host). The `controllogix` profile seeds `UdtSeg.ID` / `.Enable` / `.Ready` / `.RampTime` /
|
||||
`.Sub.Code` / `.Points[4]`, modelled on the Z-35162 press's `UDT_TemperatureProfile_Segment`.
|
||||
- **libplctag's pad byte.** libplctag pads an odd-length write payload (a 1-byte SINT / BOOL)
|
||||
with one byte; stock ab_server counted it as data and refused every such write with CIP
|
||||
`0x03` (libplctag `ErrorBadParam`), so no libplctag BOOL/SINT write could pass here. The
|
||||
patch drops exactly one trailing pad byte when it alone would overrun an odd-sized target.
|
||||
|
||||
**Bumping `LIBPLCTAG_TAG` means re-checking every patch against the new tag.** It
|
||||
is a concrete tag rather than the `release` branch precisely so that this stays
|
||||
a deliberate act — the branch moves, and the Dockerfile previously called it
|
||||
"pinned" while tracking it.
|
||||
@@ -141,7 +159,7 @@ place means updating both.
|
||||
|
||||
| Family | Seeded tags | Notes |
|
||||
|---|---|---|
|
||||
| ControlLogix | `TestDINT` `TestREAL` `TestBOOL` `TestSINT` `TestString` `TestArray` `Pump_1_Speed` `Line1_Count` `Motor_Run` | Widest-coverage; PR 9 baseline. UDT emulation missing from ab_server. The last three serve the controller-browse gate and are additive — the read/write suites key off the `Test*` tags. `Motor_Run` (9), `Line1_Count` (11) and `TestArray` (9) have **odd-length names on purpose**: entries are unpadded, so an odd name puts every following entry on an odd offset, and a decoder that assumes even alignment loses everything behind it |
|
||||
| ControlLogix | `TestDINT` `TestREAL` `TestBOOL` `TestSINT` `TestString` `TestArray` `Pump_1_Speed` `Line1_Count` `Motor_Run` `UdtSeg.*` | Widest-coverage; PR 9 baseline. UDT emulation missing from ab_server; the `UdtSeg.*` member tags (patch 0003) serve member-path writes only. The last three serve the controller-browse gate and are additive — the read/write suites key off the `Test*` tags. `Motor_Run` (9), `Line1_Count` (11) and `TestArray` (9) have **odd-length names on purpose**: entries are unpadded, so an odd name puts every following entry on an odd offset, and a decoder that assumes even alignment loses everything behind it |
|
||||
| CompactLogix | `TestDINT` `TestREAL` `TestBOOL` | Narrow ConnectionSize cap enforced driver-side; ab_server accepts any size |
|
||||
| Micro800 | `TestDINT` `TestREAL` | ab_server has no `micro800` mode; falls back to `controllogix` emulation |
|
||||
| GuardLogix | `TestDINT` `SafetyDINT_S` | ab_server has no safety subsystem; `_S` suffix triggers driver-side classification only |
|
||||
|
||||
+13
-1
@@ -22,6 +22,12 @@ services:
|
||||
- "44818:44818"
|
||||
# The last three are here for the controller-browse gate (Gitea #468) and
|
||||
# are additive — the read/write suites key off the Test* tags above.
|
||||
# The UdtSeg.* tags (Gitea #611) stand in for the members of a UDT tag `UdtSeg`
|
||||
# (modelled on the Z-35162 press's UDT_TemperatureProfile_Segment): ab_server has
|
||||
# no structures, so patch 0003 serves a member path (one CIP symbolic segment per
|
||||
# member) from the flat tag whose --tag name is that dotted path. They exist for
|
||||
# the discovered-member write suite (AbCipUdtMemberWriteTests); a bare `UdtSeg`
|
||||
# read stays unknown, as does any member not listed here.
|
||||
# Odd-length names are deliberate: Symbol Object entries are packed with no
|
||||
# padding, so a name with an odd byte count puts every following entry on an
|
||||
# odd offset. A decoder that assumes even alignment reads all six correctly
|
||||
@@ -39,7 +45,13 @@ services:
|
||||
"--tag=TestArray:DINT[16]",
|
||||
"--tag=Pump_1_Speed:REAL[1]",
|
||||
"--tag=Line1_Count:DINT[1]",
|
||||
"--tag=Motor_Run:BOOL[1]"
|
||||
"--tag=Motor_Run:BOOL[1]",
|
||||
"--tag=UdtSeg.ID:DINT[1]",
|
||||
"--tag=UdtSeg.Enable:BOOL[1]",
|
||||
"--tag=UdtSeg.Ready:BOOL[1]",
|
||||
"--tag=UdtSeg.RampTime:REAL[1]",
|
||||
"--tag=UdtSeg.Sub.Code:INT[1]",
|
||||
"--tag=UdtSeg.Points:DINT[4]"
|
||||
]
|
||||
|
||||
compactlogix:
|
||||
|
||||
+93
@@ -0,0 +1,93 @@
|
||||
diff --git a/src/tools/ab_server/cip.c b/src/tools/ab_server/cip.c
|
||||
index db44932..31a23b5 100644
|
||||
--- a/src/tools/ab_server/cip.c
|
||||
+++ b/src/tools/ab_server/cip.c
|
||||
@@ -1149,6 +1149,16 @@ slice_s handle_write_request(uint8_t cip_service, slice_s cip_service_path, slic
|
||||
/* what is the actual starting byte offset for this specific request which might be a fragment. */
|
||||
request_start_byte_offset += request_fragment_start_byte_offset;
|
||||
|
||||
+ /* OtOpcUa patch 0003: libplctag pads an odd-length write payload (a 1-byte SINT / BOOL) with one
|
||||
+ * byte to keep the CIP message 16-bit aligned. A Logix controller ignores that pad byte; stock
|
||||
+ * ab_server counted it as data and refused every 1-byte write from libplctag with 0x03. Drop a
|
||||
+ * single trailing pad byte when exactly one would overrun an odd-sized target. */
|
||||
+ if((slice_len(write_request_payload_slice) + request_start_byte_offset) == request_end_byte_offset + 1
|
||||
+ && ((request_end_byte_offset - request_start_byte_offset) % 2) == 1) {
|
||||
+ write_request_payload_slice =
|
||||
+ slice_from_slice(write_request_payload_slice, 0, slice_len(write_request_payload_slice) - 1);
|
||||
+ }
|
||||
+
|
||||
/* Make sure we are not trying to write too much. */
|
||||
if((slice_len(write_request_payload_slice) + request_start_byte_offset) > request_end_byte_offset) {
|
||||
log_info("Too much data in the write request!");
|
||||
@@ -1266,11 +1276,40 @@ bool parse_tag_path(slice_s tag_path, plc_s *plc, tag_def_s **tag, uint32_t *num
|
||||
/* Align to 16-bit boundary if necessary */
|
||||
if(offset % 2 != 0) { offset++; }
|
||||
|
||||
+ /* OtOpcUa patch 0003: a Logix member path ("Seg.ID", "Seg.Sub.Code") arrives as one symbolic
|
||||
+ * segment per member. ab_server has no structures, so a member is served by a flat tag whose
|
||||
+ * --tag name is the dotted path; join consecutive symbolic segments with '.' and look that up.
|
||||
+ * A single-segment path takes exactly the original lookup below. */
|
||||
+ char joined_name[256] = {0};
|
||||
+ size_t joined_len = 0;
|
||||
+ bool has_members = false;
|
||||
+ if(offset < slice_len(tag_path) && slice_get_uint8(tag_path, offset) == CIP_SYMBOLIC_SEGMENT_MARKER) {
|
||||
+ has_members = true;
|
||||
+ if(name_len >= sizeof(joined_name)) { return false; }
|
||||
+ memcpy(joined_name, tag_name_slice.data, name_len);
|
||||
+ joined_len = name_len;
|
||||
+ while(offset + 2 <= slice_len(tag_path) && slice_get_uint8(tag_path, offset) == CIP_SYMBOLIC_SEGMENT_MARKER) {
|
||||
+ uint8_t member_len = slice_get_uint8(tag_path, offset + 1);
|
||||
+ offset += 2;
|
||||
+ if(offset + member_len > slice_len(tag_path) || joined_len + 1 + member_len >= sizeof(joined_name)) {
|
||||
+ log_info("Member segment overruns the tag path!");
|
||||
+ return false;
|
||||
+ }
|
||||
+ joined_name[joined_len++] = '.';
|
||||
+ memcpy(joined_name + joined_len, tag_path.data + offset, member_len);
|
||||
+ joined_len += member_len;
|
||||
+ offset += member_len;
|
||||
+ if(offset % 2 != 0) { offset++; }
|
||||
+ }
|
||||
+ joined_name[joined_len] = 0;
|
||||
+ }
|
||||
+
|
||||
/* find the tag */
|
||||
*tag = plc->tags;
|
||||
|
||||
while(*tag) {
|
||||
- if(slice_match_string_exact(tag_name_slice, (*tag)->name)) {
|
||||
+ if(has_members ? (strcmp(joined_name, (*tag)->name) == 0)
|
||||
+ : slice_match_string_exact(tag_name_slice, (*tag)->name)) {
|
||||
log_info("Found tag %s", (*tag)->name);
|
||||
break;
|
||||
}
|
||||
@@ -1279,7 +1318,11 @@ bool parse_tag_path(slice_s tag_path, plc_s *plc, tag_def_s **tag, uint32_t *num
|
||||
}
|
||||
|
||||
if(!*tag) {
|
||||
- log_info("Tag %.*s not found!", slice_len(tag_name_slice), (const char *)(tag_name_slice.data));
|
||||
+ if(has_members) {
|
||||
+ log_info("Tag %s not found!", joined_name);
|
||||
+ } else {
|
||||
+ log_info("Tag %.*s not found!", slice_len(tag_name_slice), (const char *)(tag_name_slice.data));
|
||||
+ }
|
||||
return false;
|
||||
}
|
||||
|
||||
diff --git a/src/tools/ab_server/main.c b/src/tools/ab_server/main.c
|
||||
index f8f8593..29cd157 100644
|
||||
--- a/src/tools/ab_server/main.c
|
||||
+++ b/src/tools/ab_server/main.c
|
||||
@@ -603,9 +603,10 @@ void parse_cip_tag(const char *tag_str, plc_s *plc) {
|
||||
|
||||
/* try to match the three parts of a tag definition string. */
|
||||
|
||||
- /* first match the name. */
|
||||
+ /* first match the name. OtOpcUa patch 0003: '.' is accepted so a flat tag can stand in for a
|
||||
+ * UDT member path ("Seg.ID") — see parse_tag_path in cip.c. */
|
||||
start = 0;
|
||||
- len = strspn(tag_str + start, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_");
|
||||
+ len = strspn(tag_str + start, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.");
|
||||
if(!len) {
|
||||
// NOLINTNEXTLINE
|
||||
fprintf(stderr, "Unable to parse tag definition string, cannot find tag name in \"%s\"!\n", tag_str);
|
||||
+259
@@ -0,0 +1,259 @@
|
||||
using System.Runtime.CompilerServices;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Gitea #611 — writes to controller-discovered UDT members (item 2) and the whole-UDT refusal (item 3).
|
||||
/// A discovered member becomes writable the way every v3 tag does: browse-commit authors it as a raw tag
|
||||
/// (<c>tagPath</c> = the member path, <c>dataType</c> = the template's atomic type, AccessLevel Read — see
|
||||
/// <c>RawBrowseCommitMapperTests.A_discovered_AbCip_UDT_member_commits_read_only_with_its_member_path_and_type</c>)
|
||||
/// and the operator flips it to ReadWrite. These tests start from what discovery emits and assert what the
|
||||
/// driver does with that tag.
|
||||
/// </summary>
|
||||
[Trait("Category", "Unit")]
|
||||
public sealed class AbCipDiscoveredMemberWriteTests
|
||||
{
|
||||
private const string Device = "ab://10.0.0.5/1,0";
|
||||
|
||||
/// <summary>
|
||||
/// Discovery → commit → write, end to end at the driver seam: every atomic leaf of a browsed UDT
|
||||
/// (top-level, nested, BIT) committed as a raw tag writes through a runtime named by its member path,
|
||||
/// encoded as its browsed type.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task Every_browsed_member_leaf_committed_as_a_raw_tag_writes_to_its_own_member_path()
|
||||
{
|
||||
var leaves = await DiscoverLeavesAsync();
|
||||
leaves.Select(l => l.FullName).ShouldBe(
|
||||
["Seg.ID", "Seg.Enable", "Seg.RampTime", "Seg.Sub.Code"], ignoreOrder: true);
|
||||
|
||||
var factory = new FakeAbCipTagFactory();
|
||||
var drv = await StartAsync(factory, [.. leaves.Select(l => Committed(l.FullName, l.NativeDataType!))]);
|
||||
var values = new Dictionary<string, object>
|
||||
{
|
||||
["Seg.ID"] = 42, ["Seg.Enable"] = true, ["Seg.RampTime"] = 1.5f, ["Seg.Sub.Code"] = (short)7,
|
||||
};
|
||||
|
||||
var results = await drv.WriteAsync(
|
||||
[.. values.Select(kv => new WriteRequest(kv.Key, kv.Value))], CancellationToken.None);
|
||||
|
||||
results.Select(r => r.StatusCode).ShouldAllBe(s => s == AbCipStatusMapper.Good);
|
||||
foreach (var (path, value) in values)
|
||||
{
|
||||
var tag = factory.Tags[path];
|
||||
tag.CreationParams.TagName.ShouldBe(path); // the member path itself — never its UDT, never a hidden host
|
||||
tag.Value.ShouldBe(value);
|
||||
tag.WriteCount.ShouldBe(1);
|
||||
}
|
||||
factory.Tags["Seg.Enable"].CreationParams.AuthoredType.ShouldBe(AbCipDataType.Bool);
|
||||
factory.Tags["Seg.Sub.Code"].CreationParams.AuthoredType.ShouldBe(AbCipDataType.Int);
|
||||
}
|
||||
|
||||
/// <summary>The operator's <c>writable:false</c> on a committed member is honoured — nothing is created or sent.</summary>
|
||||
[Fact]
|
||||
public async Task A_member_authored_not_writable_is_BadNotWritable()
|
||||
{
|
||||
var factory = new FakeAbCipTagFactory();
|
||||
var drv = await StartAsync(factory, Committed("Seg.ID", "DInt", writable: false));
|
||||
|
||||
(await drv.WriteAsync([new WriteRequest("Seg.ID", 1)], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.BadNotWritable);
|
||||
factory.Tags.ShouldNotContainKey("Seg.ID");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// An INT authored over a member the controller holds as 4 bytes: libplctag would patch the low two bytes
|
||||
/// of its read buffer and send all four. Refused BadTypeMismatch before anything is sent, handle kept,
|
||||
/// health untouched.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task A_member_authored_narrower_than_the_controller_value_is_refused_before_sending()
|
||||
{
|
||||
var factory = new FakeAbCipTagFactory { Customise = p => new FakeAbCipTag(p) { Size = 4, Value = 0 } };
|
||||
// The mistyped tag cannot prime health itself — its READ is refused for the same width mismatch.
|
||||
var drv = await StartAsync(factory, Committed("Seg.ID", "Int"), Committed("Seg.RampTime", "Real"));
|
||||
await PrimeHealthAsync(drv, "Seg.RampTime");
|
||||
var health = drv.GetHealth();
|
||||
|
||||
(await drv.WriteAsync([new WriteRequest("Seg.ID", (short)5)], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.BadTypeMismatch);
|
||||
|
||||
factory.Tags["Seg.ID"].WriteCount.ShouldBe(0);
|
||||
factory.Tags["Seg.ID"].Disposed.ShouldBeFalse();
|
||||
drv.GetHealth().ShouldBe(health);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A BOOL authored over a 4-byte value (a packed BOOL-array DWORD, or a mis-typed DINT member): writing it
|
||||
/// would rewrite all 32 bits. Refused BadNotSupported — the native runtime's refusal — before sending.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task A_BOOL_over_a_wider_value_is_refused_NotSupported_before_sending()
|
||||
{
|
||||
var factory = new FakeAbCipTagFactory { Customise = p => new FakeAbCipTag(p) { Size = 4, Value = 0 } };
|
||||
var drv = await StartAsync(factory, Committed("Seg.Flags", "Bool"));
|
||||
await PrimeHealthAsync(drv, "Seg.Flags");
|
||||
var health = drv.GetHealth();
|
||||
|
||||
(await drv.WriteAsync([new WriteRequest("Seg.Flags", true)], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.BadNotSupported);
|
||||
|
||||
factory.Tags["Seg.Flags"].WriteCount.ShouldBe(0);
|
||||
drv.GetHealth().ShouldBe(health);
|
||||
}
|
||||
|
||||
/// <summary>A BIT member reads back as one byte (type 0xC1), so a BOOL write to it goes out.</summary>
|
||||
[Fact]
|
||||
public async Task A_BOOL_member_held_as_one_byte_writes()
|
||||
{
|
||||
var factory = new FakeAbCipTagFactory { Customise = p => new FakeAbCipTag(p) { Size = 1, Value = false } };
|
||||
var drv = await StartAsync(factory, Committed("Seg.Enable", "Bool"));
|
||||
|
||||
(await drv.WriteAsync([new WriteRequest("Seg.Enable", true)], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.Good);
|
||||
factory.Tags["Seg.Enable"].WriteCount.ShouldBe(1);
|
||||
}
|
||||
|
||||
/// <summary>Gitea #611 item 3 — a whole-UDT write is refused by decision: no runtime, nothing sent, no health change.</summary>
|
||||
[Fact]
|
||||
public async Task A_whole_UDT_write_is_refused_NotSupported_without_a_runtime_or_a_health_change()
|
||||
{
|
||||
var factory = new FakeAbCipTagFactory();
|
||||
var drv = await StartAsync(factory, Committed("Seg", "Structure"), Committed("Seg.ID", "DInt"));
|
||||
await PrimeHealthAsync(drv, "Seg.ID");
|
||||
var health = drv.GetHealth();
|
||||
|
||||
(await drv.WriteAsync([new WriteRequest("Seg", new byte[8])], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.BadNotSupported);
|
||||
|
||||
factory.Tags.ShouldNotContainKey("Seg");
|
||||
drv.GetHealth().ShouldBe(health);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// A runtime's NotSupportedException is a local refusal the device never saw — BadNotSupported with the
|
||||
/// handle kept and health untouched (it used to latch Degraded).
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task A_runtime_NotSupported_refusal_leaves_health_alone_and_keeps_the_handle()
|
||||
{
|
||||
var factory = new FakeAbCipTagFactory { Customise = p => new RefusingEncodeTag(p) { Value = 0 } };
|
||||
var drv = await StartAsync(factory, Committed("Seg.ID", "DInt"));
|
||||
await PrimeHealthAsync(drv, "Seg.ID");
|
||||
var health = drv.GetHealth();
|
||||
|
||||
(await drv.WriteAsync([new WriteRequest("Seg.ID", 1)], CancellationToken.None))
|
||||
.Single().StatusCode.ShouldBe(AbCipStatusMapper.BadNotSupported);
|
||||
|
||||
drv.GetHealth().ShouldBe(health);
|
||||
factory.Tags["Seg.ID"].Disposed.ShouldBeFalse();
|
||||
factory.Tags["Seg.ID"].WriteCount.ShouldBe(0);
|
||||
}
|
||||
|
||||
// ---- helpers ---------------------------------------------------------------------------------------
|
||||
|
||||
/// <summary>Browses a UDT shaped like the Z-35162 press's UDT_TemperatureProfile_Segment (+ a nested struct).</summary>
|
||||
private static async Task<List<DriverAttributeInfo>> DiscoverLeavesAsync()
|
||||
{
|
||||
var drv = new AbCipDriver(new AbCipDriverOptions
|
||||
{
|
||||
Devices = [new AbCipDeviceOptions(Device)],
|
||||
EnableControllerBrowse = true,
|
||||
Probe = new AbCipProbeOptions { Enabled = false },
|
||||
}, "drv-browse", enumeratorFactory: new OneTagEnumeratorFactory(
|
||||
new AbCipDiscoveredTag("Seg", null, AbCipDataType.Structure, ReadOnly: false)));
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
drv.SeedDiscoveredUdtShapeForTest(Device, "Seg", new AbCipUdtShape("UDT_Segment", 24,
|
||||
[
|
||||
new AbCipUdtMember("ID", 0, AbCipDataType.DInt, ArrayLength: 1),
|
||||
new AbCipUdtMember("ZZZZZZZZZZUDT_Segm1", 4, AbCipDataType.SInt, ArrayLength: 1), // hidden BIT host
|
||||
new AbCipUdtMember("Enable", 4, AbCipDataType.Bool, ArrayLength: 1),
|
||||
new AbCipUdtMember("RampTime", 8, AbCipDataType.Real, ArrayLength: 1),
|
||||
new AbCipUdtMember("Sub", 12, AbCipDataType.Structure, ArrayLength: 1),
|
||||
]));
|
||||
drv.SeedDiscoveredUdtShapeForTest(Device, "Sub", new AbCipUdtShape("UDT_Sub", 4,
|
||||
[new AbCipUdtMember("Code", 0, AbCipDataType.Int, ArrayLength: 1)]));
|
||||
|
||||
var builder = new LeafRecorder();
|
||||
await drv.DiscoverAsync(builder, CancellationToken.None);
|
||||
await drv.ShutdownAsync(CancellationToken.None);
|
||||
return builder.Leaves;
|
||||
}
|
||||
|
||||
/// <summary>A raw tag as <c>RawBrowseCommitMapper</c> commits a discovered leaf (plus an optional writable key).</summary>
|
||||
private static RawTagEntry Committed(string tagPath, string dataType, bool? writable = null) =>
|
||||
new(tagPath,
|
||||
writable is { } w
|
||||
? $$"""{"tagPath":"{{tagPath}}","dataType":"{{dataType}}","writable":{{(w ? "true" : "false")}}}"""
|
||||
: $$"""{"tagPath":"{{tagPath}}","dataType":"{{dataType}}"}""",
|
||||
WriteIdempotent: false,
|
||||
DeviceName: Device);
|
||||
|
||||
private static async Task<AbCipDriver> StartAsync(FakeAbCipTagFactory factory, params RawTagEntry[] tags)
|
||||
{
|
||||
var drv = new AbCipDriver(new AbCipDriverOptions
|
||||
{
|
||||
Devices = [new AbCipDeviceOptions(Device)],
|
||||
RawTags = tags,
|
||||
Probe = new AbCipProbeOptions { Enabled = false },
|
||||
}, "drv-1", factory);
|
||||
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||
return drv;
|
||||
}
|
||||
|
||||
/// <summary>One good read so health holds a concrete Healthy snapshot the refusal must leave untouched.</summary>
|
||||
private static async Task PrimeHealthAsync(AbCipDriver drv, string reference) =>
|
||||
(await drv.ReadAsync([reference], CancellationToken.None)).Single().StatusCode.ShouldBe(AbCipStatusMapper.Good);
|
||||
|
||||
private sealed class RefusingEncodeTag(AbCipTagCreateParams p) : FakeAbCipTag(p)
|
||||
{
|
||||
public override void EncodeValue(AbCipDataType type, int? bitIndex, object? value) =>
|
||||
throw new NotSupportedException("refused locally");
|
||||
}
|
||||
|
||||
private sealed class OneTagEnumeratorFactory(params AbCipDiscoveredTag[] tags) : IAbCipTagEnumeratorFactory
|
||||
{
|
||||
public IAbCipTagEnumerator Create() => new Enumerator(tags);
|
||||
|
||||
private sealed class Enumerator(AbCipDiscoveredTag[] tags) : IAbCipTagEnumerator
|
||||
{
|
||||
public async IAsyncEnumerable<AbCipDiscoveredTag> EnumerateAsync(
|
||||
AbCipTagCreateParams deviceParams, [EnumeratorCancellation] CancellationToken cancellationToken)
|
||||
{
|
||||
await Task.CompletedTask;
|
||||
foreach (var t in tags) yield return t;
|
||||
}
|
||||
|
||||
public void Dispose() { }
|
||||
}
|
||||
}
|
||||
|
||||
private sealed class LeafRecorder : IAddressSpaceBuilder
|
||||
{
|
||||
public List<DriverAttributeInfo> Leaves { get; } = [];
|
||||
|
||||
public IAddressSpaceBuilder Folder(string browseName, string displayName) => this;
|
||||
|
||||
public IVariableHandle Variable(string browseName, string displayName, DriverAttributeInfo info)
|
||||
{
|
||||
Leaves.Add(info);
|
||||
return new Handle(info.FullName);
|
||||
}
|
||||
|
||||
public void AddProperty(string browseName, DriverDataType dataType, object? value) { }
|
||||
|
||||
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) { }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -196,6 +196,31 @@ public sealed class NativeAbCipTagRuntimeTests
|
||||
ch.Sent[^1].ShouldBe(Convert.FromHexString("4D02" + "91015400" + "C400" + "0100" + "2A000000"));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Gitea #611 — a discovered UDT member writes to the MEMBER: one symbolic segment per member (plus an
|
||||
/// element segment), the member's own CIP type, and only its own bytes. For a BIT member (<c>Seg.Enable</c>,
|
||||
/// a BOOL packed into a hidden SINT host) that is what keeps the neighbour bits safe: the request names
|
||||
/// the member and carries one BOOL byte, never the host, so the controller applies the bit.
|
||||
/// </summary>
|
||||
[Theory]
|
||||
[InlineData("Seg.Enable", AbCipDataType.Bool, "C100", "00", true,
|
||||
"4D07" + "9103536567009106456E61626C65" + "C100" + "0100" + "01")]
|
||||
[InlineData("Seg.Sub.Code", AbCipDataType.Int, "C300", "0000", (short)-2,
|
||||
"4D09" + "910353656700" + "910353756200" + "9104436F6465" + "C300" + "0100" + "FEFF")]
|
||||
[InlineData("Seg.Points[2]", AbCipDataType.DInt, "C400", "00000000", 42,
|
||||
"4D08" + "910353656700" + "9106506F696E7473" + "2802" + "C400" + "0100" + "2A000000")]
|
||||
public async Task A_member_path_write_names_the_member_and_sends_only_its_own_bytes(
|
||||
string tag, AbCipDataType type, string typeHex, string readHex, object value, string expectedRequestHex)
|
||||
{
|
||||
var ch = Replying(typeHex, readHex);
|
||||
using var rt = await ReadAsync(ch, Params(tag, type));
|
||||
ch.Respond = req => Task.FromResult(FakeCipChannel.OkReply(req));
|
||||
rt.EncodeValue(type, null, value);
|
||||
await rt.WriteAsync(CancellationToken.None);
|
||||
rt.GetStatus().ShouldBe((int)AbStatus.Ok);
|
||||
ch.Sent[^1].ShouldBe(Convert.FromHexString(expectedRequestHex));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task A_STRING_write_sends_Struct_and_the_handle_learned_from_the_prior_read()
|
||||
{
|
||||
|
||||
@@ -985,4 +985,41 @@ public sealed class RawBrowseCommitMapperTests
|
||||
refused.ShouldBe(expectRefused);
|
||||
blobRejected.ShouldBe(expectRefused);
|
||||
}
|
||||
|
||||
// ---- AB CIP discovered UDT members (Gitea #611) ----------------------------------------------
|
||||
|
||||
/// <summary>
|
||||
/// Gitea #611 — a controller-discovered UDT member leaf commits as an ordinary raw tag: its member path
|
||||
/// is the <c>tagPath</c>, the template's atomic type is the <c>dataType</c>, and it lands READ-ONLY with
|
||||
/// no <c>writable</c> key. That is the whole writability rule for discovered members: the driver writes a
|
||||
/// member only once the operator authors the raw tag <c>ReadWrite</c> (the node-manager gate), and the
|
||||
/// commit never pre-authors that — the browse's <c>SecurityClass</c> is not consulted.
|
||||
/// </summary>
|
||||
/// <param name="memberPath">The discovered member's full name (its Logix member path).</param>
|
||||
/// <param name="browseName">The member's browse name.</param>
|
||||
/// <param name="nativeType">The atomic type the template stated.</param>
|
||||
[Theory]
|
||||
[InlineData("_EmptyTemperatureSegment.RampTime", "RampTime", "Real")]
|
||||
[InlineData("_EmptyTemperatureSegment.Enable", "Enable", "Bool")]
|
||||
[InlineData("Program:Main.Motor1.Status.Code", "Code", "Int")]
|
||||
public void A_discovered_AbCip_UDT_member_commits_read_only_with_its_member_path_and_type(
|
||||
string memberPath, string browseName, string nativeType)
|
||||
{
|
||||
var row = RawBrowseCommitMapper.MapLeaf(
|
||||
driverType: DriverTypeNames.AbCip,
|
||||
fullName: memberPath,
|
||||
browseName: browseName,
|
||||
driverDataType: null,
|
||||
defaultDataType: "Int32",
|
||||
groupPrefix: null,
|
||||
folderPath: null,
|
||||
createGroups: false,
|
||||
addressFields: NativeTypeFields(nativeType));
|
||||
|
||||
row.Tag.AccessLevel.ShouldBe(TagAccessLevel.Read);
|
||||
var o = JsonNode.Parse(row.Tag.TagConfig)!.AsObject();
|
||||
o["tagPath"]!.GetValue<string>().ShouldBe(memberPath);
|
||||
o["dataType"]!.GetValue<string>().ShouldBe(nativeType);
|
||||
o.ContainsKey("writable").ShouldBeFalse();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user