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>
339 lines
14 KiB
C#
339 lines
14 KiB
C#
using libplctag;
|
|
using Shouldly;
|
|
using Xunit;
|
|
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
|
using ZB.MOM.WW.OtOpcUa.Driver.AbCip;
|
|
|
|
namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
|
|
|
|
/// <summary>
|
|
/// Regression tests for the High code-review findings Driver.AbCip-001 / -003 / -008,
|
|
/// and the Medium findings Driver.AbCip-004 / -005 / -010.
|
|
/// (Driver.AbCip-002 is covered by <see cref="AbCipStatusMapperTests"/>.)
|
|
/// </summary>
|
|
[Trait("Category", "Unit")]
|
|
public sealed class AbCipDriverCodeReviewRegressionTests
|
|
{
|
|
private const string Device = "ab://10.0.0.5/1,0";
|
|
|
|
// ---- Driver.AbCip-001 — ReinitializeAsync must apply a changed config JSON ----
|
|
|
|
[Fact]
|
|
public async Task InitializeAsync_applies_devices_and_tags_from_the_config_json()
|
|
{
|
|
// Constructed with NO devices/tags — the JSON is the only source of config.
|
|
var drv = new AbCipDriver(new AbCipDriverOptions(), "drv-1");
|
|
const string json = """
|
|
{
|
|
"Devices": [ { "HostAddress": "ab://10.0.0.9/1,0", "PlcFamily": "ControlLogix" } ],
|
|
"Tags": [ { "Name": "Speed", "DeviceHostAddress": "ab://10.0.0.9/1,0",
|
|
"TagPath": "Speed", "DataType": "DInt" } ]
|
|
}
|
|
""";
|
|
|
|
await drv.InitializeAsync(json, CancellationToken.None);
|
|
|
|
drv.DeviceCount.ShouldBe(1);
|
|
drv.GetDeviceState("ab://10.0.0.9/1,0").ShouldNotBeNull();
|
|
drv.GetHealth().State.ShouldBe(DriverState.Healthy);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ReinitializeAsync_with_a_changed_config_json_picks_up_the_new_device()
|
|
{
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
}, "drv-1");
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
drv.GetDeviceState(Device).ShouldNotBeNull();
|
|
|
|
// Reinitialize with a JSON that names a DIFFERENT device — the change must take effect
|
|
// instead of being silently discarded (Driver.AbCip-001).
|
|
const string changed = """
|
|
{ "Devices": [ { "HostAddress": "ab://10.0.0.99/1,0" } ] }
|
|
""";
|
|
await drv.ReinitializeAsync(changed, CancellationToken.None);
|
|
|
|
drv.DeviceCount.ShouldBe(1);
|
|
drv.GetDeviceState("ab://10.0.0.99/1,0").ShouldNotBeNull();
|
|
drv.GetDeviceState(Device).ShouldBeNull();
|
|
}
|
|
|
|
[Fact]
|
|
public async Task InitializeAsync_with_blank_json_keeps_construction_time_options()
|
|
{
|
|
// The test seam: a driver constructed with explicit options + handed "{}" must keep
|
|
// those options (otherwise every fake-backed unit test would lose its config).
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
}, "drv-1");
|
|
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
drv.DeviceCount.ShouldBe(1);
|
|
drv.GetDeviceState(Device).ShouldNotBeNull();
|
|
}
|
|
|
|
// ---- Driver.AbCip-003 — declaration-only whole-UDT grouping is opt-in ----
|
|
|
|
[Fact]
|
|
public async Task Whole_udt_grouping_is_off_by_default_so_members_read_per_tag()
|
|
{
|
|
// Default options — EnableDeclarationOnlyUdtGrouping is false. Reading two members of
|
|
// a UDT must NOT collapse into a single declaration-order whole-UDT read, because the
|
|
// controller may not lay members out in declaration order.
|
|
var factory = new FakeAbCipTagFactory();
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
Tags =
|
|
[
|
|
new AbCipTagDefinition("Motor", Device, "Motor", AbCipDataType.Structure, Members:
|
|
[
|
|
new AbCipStructureMember("Speed", AbCipDataType.DInt),
|
|
new AbCipStructureMember("Torque", AbCipDataType.Real),
|
|
]),
|
|
],
|
|
}, "drv-1", factory);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
await drv.ReadAsync(["Motor.Speed", "Motor.Torque"], CancellationToken.None);
|
|
|
|
// Each member got its own per-tag runtime; the parent "Motor" runtime was never created.
|
|
factory.Tags.ShouldContainKey("Motor.Speed");
|
|
factory.Tags.ShouldContainKey("Motor.Torque");
|
|
factory.Tags.ShouldNotContainKey("Motor");
|
|
}
|
|
|
|
[Fact]
|
|
public void Planner_forms_no_groups_when_declaration_only_grouping_is_disabled()
|
|
{
|
|
var members = new[]
|
|
{
|
|
new AbCipStructureMember("Speed", AbCipDataType.DInt),
|
|
new AbCipStructureMember("Torque", AbCipDataType.Real),
|
|
};
|
|
var tags = new Dictionary<string, AbCipTagDefinition>(StringComparer.OrdinalIgnoreCase)
|
|
{
|
|
["Motor"] = new("Motor", Device, "Motor", AbCipDataType.Structure, Members: members),
|
|
["Motor.Speed"] = new("Motor.Speed", Device, "Motor.Speed", AbCipDataType.DInt),
|
|
["Motor.Torque"] = new("Motor.Torque", Device, "Motor.Torque", AbCipDataType.Real),
|
|
};
|
|
|
|
var plan = AbCipUdtReadPlanner.Build(
|
|
["Motor.Speed", "Motor.Torque"], tags, enableDeclarationOnlyGrouping: false);
|
|
|
|
plan.Groups.ShouldBeEmpty();
|
|
plan.Fallbacks.Count.ShouldBe(2);
|
|
}
|
|
|
|
// ---- Driver.AbCip-008 — ShutdownAsync awaits probe loops; reads are concurrency-safe ----
|
|
|
|
[Fact]
|
|
public async Task ShutdownAsync_awaits_the_probe_loop_before_returning()
|
|
{
|
|
var factory = new FakeAbCipTagFactory();
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
Probe = new AbCipProbeOptions
|
|
{
|
|
Enabled = true,
|
|
ProbeTagPath = "ProbeTag",
|
|
Interval = TimeSpan.FromMilliseconds(20),
|
|
},
|
|
}, "drv-1", factory);
|
|
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
// Give the probe loop a moment to actually start spinning.
|
|
await Task.Delay(60);
|
|
|
|
// Must complete cleanly — no ObjectDisposedException from a loop racing a disposed CTS.
|
|
await Should.NotThrowAsync(() => drv.ShutdownAsync(CancellationToken.None));
|
|
drv.DeviceCount.ShouldBe(0);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task ShutdownAsync_is_idempotent()
|
|
{
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
}, "drv-1");
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
await drv.ShutdownAsync(CancellationToken.None);
|
|
await Should.NotThrowAsync(() => drv.ShutdownAsync(CancellationToken.None));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Concurrent_first_reads_of_the_same_tag_do_not_corrupt_the_runtime_cache()
|
|
{
|
|
// Two concurrent ReadAsync calls that both miss the runtime cache must not throw and
|
|
// must not leave the device with two un-disposed runtimes for one tag (Driver.AbCip-008
|
|
// ConcurrentDictionary + TryAdd loser-disposal).
|
|
var factory = new FakeAbCipTagFactory();
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
Tags = [new AbCipTagDefinition("Speed", Device, "Speed", AbCipDataType.DInt)],
|
|
}, "drv-1", factory);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
var reads = Enumerable.Range(0, 16)
|
|
.Select(_ => drv.ReadAsync(["Speed"], CancellationToken.None))
|
|
.ToArray();
|
|
var allResults = await Task.WhenAll(reads);
|
|
|
|
foreach (var result in allResults)
|
|
result.Single().StatusCode.ShouldBe(AbCipStatusMapper.Good);
|
|
}
|
|
|
|
// ---- Driver.AbCip-004 — LInt/ULInt/UDInt declared type must agree with runtime value type ----
|
|
|
|
[Theory]
|
|
[InlineData(AbCipDataType.LInt, DriverDataType.Int64)]
|
|
[InlineData(AbCipDataType.ULInt, DriverDataType.UInt64)]
|
|
[InlineData(AbCipDataType.UDInt, DriverDataType.UInt32)]
|
|
public void AbCipDataType_maps_large_integer_types_to_correct_driver_types(
|
|
AbCipDataType abType, DriverDataType expected)
|
|
{
|
|
// Regression for Driver.AbCip-004: LInt/ULInt were mapped to Int32 (truncation);
|
|
// UDInt was mapped to Int32 (negative wrap for values > Int32.MaxValue).
|
|
abType.ToDriverDataType().ShouldBe(expected);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Read_UDInt_tag_returns_uint_value_not_negative_wrapped_int()
|
|
{
|
|
// A UDInt value above Int32.MaxValue (e.g. uint.MaxValue) used to be decoded as
|
|
// (int)GetUInt32, which wraps to -1. After the fix it must be decoded as uint.
|
|
const uint largeUDInt = uint.MaxValue; // would wrap to -1 as (int)
|
|
var factory = new FakeAbCipTagFactory();
|
|
factory.Customise = p => new FakeAbCipTag(p) { Value = largeUDInt };
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
Tags = [new AbCipTagDefinition("Counter", Device, "Counter", AbCipDataType.UDInt)],
|
|
}, "drv-1", factory);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
var results = await drv.ReadAsync(["Counter"], CancellationToken.None);
|
|
|
|
results.Single().StatusCode.ShouldBe(AbCipStatusMapper.Good);
|
|
// The value must be the uint — if it were (int)GetUInt32 it would be -1 (wrong type).
|
|
results.Single().Value.ShouldBe(largeUDInt);
|
|
results.Single().Value.ShouldBeOfType<uint>();
|
|
}
|
|
|
|
// ---- Driver.AbCip-005 — Structure parent not registered; duplicate key check ----
|
|
|
|
[Fact]
|
|
public async Task Structure_parent_tag_read_returns_BadNotSupported_not_Good_null()
|
|
{
|
|
// Regression for Driver.AbCip-005: reading the bare parent "Motor" used to return
|
|
// Good/null because DecodeValue(Structure, ...) returns null. After the fix,
|
|
// the per-tag read path detects a Structure-with-Members and returns BadNotSupported
|
|
// so callers know to address individual member paths instead.
|
|
var factory = new FakeAbCipTagFactory();
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
Tags =
|
|
[
|
|
new AbCipTagDefinition("Motor", Device, "Motor", AbCipDataType.Structure, Members:
|
|
[
|
|
new AbCipStructureMember("Speed", AbCipDataType.DInt),
|
|
new AbCipStructureMember("Torque", AbCipDataType.Real),
|
|
]),
|
|
],
|
|
}, "drv-1", factory);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
var results = await drv.ReadAsync(["Motor"], CancellationToken.None);
|
|
|
|
// Parent is a container, not a scalar — BadNotSupported, not Good/null.
|
|
results.Single().StatusCode.ShouldBe(AbCipStatusMapper.BadNotSupported);
|
|
results.Single().Value.ShouldBeNull();
|
|
}
|
|
|
|
[Fact]
|
|
public void InitializeAsync_throws_on_duplicate_tag_name()
|
|
{
|
|
// Regression for Driver.AbCip-005: silently-overwritten duplicate keys.
|
|
// Two independently-declared tags with the same name must throw.
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
Tags =
|
|
[
|
|
new AbCipTagDefinition("Speed", Device, "Speed", AbCipDataType.DInt),
|
|
new AbCipTagDefinition("Speed", Device, "SpeedAlias", AbCipDataType.Real), // same name
|
|
],
|
|
}, "drv-1");
|
|
|
|
Should.Throw<InvalidOperationException>(() =>
|
|
drv.InitializeAsync("{}", CancellationToken.None).GetAwaiter().GetResult());
|
|
}
|
|
|
|
[Fact]
|
|
public void InitializeAsync_throws_when_member_name_collides_with_independent_tag()
|
|
{
|
|
// A Structure fan-out member path ("Motor.Speed") that collides with a separately-
|
|
// declared tag ("Motor.Speed") must throw rather than silently overwrite.
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
Tags =
|
|
[
|
|
new AbCipTagDefinition("Motor", Device, "Motor", AbCipDataType.Structure, Members:
|
|
[
|
|
new AbCipStructureMember("Speed", AbCipDataType.DInt),
|
|
]),
|
|
new AbCipTagDefinition("Motor.Speed", Device, "Motor.Speed", AbCipDataType.DInt), // collision
|
|
],
|
|
}, "drv-1");
|
|
|
|
Should.Throw<InvalidOperationException>(() =>
|
|
drv.InitializeAsync("{}", CancellationToken.None).GetAwaiter().GetResult());
|
|
}
|
|
|
|
// ---- Driver.AbCip-010 — stale runtime evicted on failure ----
|
|
|
|
[Fact]
|
|
public async Task Read_failure_evicts_runtime_so_next_read_creates_fresh_handle()
|
|
{
|
|
// Regression for Driver.AbCip-010: a non-zero status was returned forever because
|
|
// the cached runtime was never evicted. After the fix the next read creates a new one.
|
|
var factory = new FakeAbCipTagFactory();
|
|
var callCount = 0;
|
|
factory.Customise = p =>
|
|
{
|
|
// First tag creation → returns error; second → returns success.
|
|
callCount++;
|
|
return callCount == 1
|
|
? new FakeAbCipTag(p) { Status = (int)libplctag.Status.ErrorBadConnection }
|
|
: new FakeAbCipTag(p) { Status = 0, Value = 42 };
|
|
};
|
|
var drv = new AbCipDriver(new AbCipDriverOptions
|
|
{
|
|
Devices = [new AbCipDeviceOptions(Device)],
|
|
Tags = [new AbCipTagDefinition("Speed", Device, "Speed", AbCipDataType.DInt)],
|
|
}, "drv-1", factory);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
// First read — bad connection, runtime is evicted.
|
|
var first = await drv.ReadAsync(["Speed"], CancellationToken.None);
|
|
first.Single().StatusCode.ShouldBe(AbCipStatusMapper.BadCommunicationError);
|
|
|
|
// Second read — fresh handle, succeeds.
|
|
var second = await drv.ReadAsync(["Speed"], CancellationToken.None);
|
|
second.Single().StatusCode.ShouldBe(AbCipStatusMapper.Good);
|
|
second.Single().Value.ShouldBe(42);
|
|
// The factory was called twice — once for the failed handle, once for the fresh one.
|
|
callCount.ShouldBe(2);
|
|
}
|
|
}
|