fix(driver-abcip): correct Driver.AbCip-005 approach and fix 014 tests

Finding 005 revised approach: keep the parent Structure tag in
`_tagsByName` so the whole-UDT grouping planner can find it (required
for Driver.AbCip-003 opt-in path + alarm projection). Instead, detect a
direct read of a Structure-with-Members in `ReadSingleAsync` and return
`BadNotSupported` rather than Good/null — explicitly documenting the
contract that callers must address member paths. Duplicate-key checks
(scalar and member fan-out) remain.

Finding 014 test corrections: `Structure_parent_tag_read_returns_BadNotSupported`
now asserts the new contract. `Read_UDInt_tag_returns_uint_value_not_negative_wrapped_int`
assertion fixed to use `ShouldBeOfType<uint>()` instead of
`ShouldNotBe(-1)` (Shouldly overflows comparing uint.MaxValue with int).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-22 09:30:54 -04:00
parent cec7ab6ec4
commit 17432bb1a4
2 changed files with 34 additions and 29 deletions

View File

@@ -146,10 +146,16 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
}
foreach (var tag in _options.Tags)
{
// Driver.AbCip-005: Structure tags whose Members are declared fan out to per-member
// entries only. The bare parent name is deliberately NOT registered as a readable
// tag — reading it would return Good/null (DecodeValue for Structure returns null),
// which is misleading. Clients read the individual member paths instead.
// Duplicate-key check: a collision means two configured tags have the same name.
// Fail fast at init time with a diagnostic rather than silently clobbering.
// (Driver.AbCip-005)
if (_tagsByName.TryGetValue(tag.Name, out var existingTag))
throw new InvalidOperationException(
$"AbCip tag name collision: '{tag.Name}' is declared more than once. " +
$"Existing entry DeviceHostAddress='{existingTag.DeviceHostAddress}', " +
$"TagPath='{existingTag.TagPath}'. Rename or remove the duplicate.");
_tagsByName[tag.Name] = tag;
if (tag.DataType == AbCipDataType.Structure && tag.Members is { Count: > 0 })
{
foreach (var member in tag.Members)
@@ -161,29 +167,17 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
DataType: member.DataType,
Writable: member.Writable,
WriteIdempotent: member.WriteIdempotent);
// Duplicate-key check: a collision means two configured tags produce the
// same readable path (e.g. two structure parents with the same member name,
// or a member name that matches an independently-declared tag). Fail fast
// at init time with a diagnostic rather than silently clobbering.
if (_tagsByName.TryGetValue(memberTag.Name, out var existing))
// Member fan-out duplicate check: a member-path collision means two
// configured structure tags produce the same member path, or a member
// name collides with an independently-declared tag.
if (_tagsByName.TryGetValue(memberTag.Name, out var existingMember))
throw new InvalidOperationException(
$"AbCip tag name collision: '{memberTag.Name}' is produced by both " +
$"'{tag.Name}.{member.Name}' (member fan-out) and an existing tag " +
$"'{existing.Name}'. Rename one of the configured tags to resolve.");
$"'{existingMember.Name}'. Rename one of the configured tags to resolve.");
_tagsByName[memberTag.Name] = memberTag;
}
}
else
{
// Non-structure tag (or Structure with no declared members — kept readable
// so the whole-UDT path or declaration-only grouping can serve it).
if (_tagsByName.TryGetValue(tag.Name, out var existing))
throw new InvalidOperationException(
$"AbCip tag name collision: '{tag.Name}' is declared more than once. " +
$"Existing entry DeviceHostAddress='{existing.DeviceHostAddress}', " +
$"TagPath='{existing.TagPath}'. Rename or remove the duplicate.");
_tagsByName[tag.Name] = tag;
}
}
// Probe loops — one per device when enabled + a ProbeTagPath is configured.
@@ -434,6 +428,15 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadNodeIdUnknown, null, now);
return;
}
// Driver.AbCip-005: a Structure tag whose Members are declared is a container —
// its bare name is readable via the whole-UDT grouping path (ReadGroupAsync), not the
// per-tag path. Reading it here returns BadNotSupported rather than Good/null so the
// caller knows to address individual member paths (e.g. "Motor.Speed").
if (def.DataType == AbCipDataType.Structure && def.Members is { Count: > 0 })
{
results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadNotSupported, null, now);
return;
}
if (!_devices.TryGetValue(def.DeviceHostAddress, out var device))
{
results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadNodeIdUnknown, null, now);