dohertj2/lmxopcua
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(galaxy,s7): R2 review minors — subscribe-once deadline, probe vs backoff, write-fault ownership, null array elements, write opt-in

Browse Source 
- Galaxy: deadline bounds the wait, not the subscribe RPC; BadTimeout for all tags and late handles unsubscribed; doc comment relocated
- S7: probe connect runs on the connect deadline so a timeout opens the backoff window
- S7: a successful write only retires a Faulted the write side owns
- S7: null array elements rejected (BadTypeMismatch); multi-PDU array writes documented non-atomic
- S7 integration: array write test gated on S7_SIM_ALLOW_WRITES=1

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-08 07:31:22 -04:00
parent ca0f1e7735
commit e77bca385a
9 changed files with 208 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyDriverReadTests.cs
+57
View File
@@ -1,3 +1,4 @@
using ZB.MOM.WW.MxGateway.Contracts.Proto;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
@@ -179,4 +180,60 @@ public sealed class GalaxyDriverReadTests
result[0].StatusCode.ShouldBe(StatusCodeMap.BadTimeout);
subscriber.UnsubscribedHandles.ShouldContain(subscriber.Map["Silent.Tag"]);
}
/// <summary>
/// A subscribe RPC that outlives the internal deadline (caller passed None) must yield BadTimeout for
/// every tag, not an OperationCanceledException, and the handles the late-completing RPC created must
/// still be unsubscribed.
/// </summary>
[Fact]
public async Task ReadAsync_SubscribeOncePath_SlowSubscribeBulk_NoCallerToken_ReturnsBadTimeoutAndUnsubscribesLateHandles()
{
var subscriber = new SlowSubscribeSubscriber();
using var driver = new GalaxyDriver(
"g", Opts(), hierarchySource: null, dataReader: null, dataWriter: null, subscriber: subscriber)
{
ReadViaSubscribeDeadline = TimeSpan.FromMilliseconds(200),
};
var result = await driver.ReadAsync(["Slow.A", "Slow.B"], CancellationToken.None)
.WaitAsync(TimeSpan.FromSeconds(10));
result.Count.ShouldBe(2);
result.ShouldAllBe(r => r.StatusCode == StatusCodeMap.BadTimeout);
// The RPC finally completes after the read gave up; its handles must not leak.
subscriber.Gate.SetResult();
var deadline = DateTime.UtcNow.AddSeconds(10);
while (subscriber.UnsubscribedHandles.Count < 2 && DateTime.UtcNow < deadline) await Task.Delay(20);
subscriber.UnsubscribedHandles.OrderBy(h => h).ShouldBe([101, 102]);
}
private sealed class SlowSubscribeSubscriber : IGalaxySubscriber
{
private readonly List<int> _unsubscribed = [];
public TaskCompletionSource Gate { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
public IReadOnlyList<int> UnsubscribedHandles { get { lock (_unsubscribed) return _unsubscribed.ToArray(); } }
public async Task<IReadOnlyList<SubscribeResult>> SubscribeBulkAsync(
IReadOnlyList<string> fullReferences, int bufferedUpdateIntervalMs, CancellationToken cancellationToken)
{
await Gate.Task.WaitAsync(cancellationToken).ConfigureAwait(false);
var h = 100;
return fullReferences.Select(r => new SubscribeResult
{
TagAddress = r, ItemHandle = ++h, WasSuccessful = true,
}).ToList();
}
public Task UnsubscribeBulkAsync(IReadOnlyList<int> itemHandles, CancellationToken cancellationToken)
{
lock (_unsubscribed) _unsubscribed.AddRange(itemHandles);
return Task.CompletedTask;
}
public IAsyncEnumerable<ZB.MOM.WW.MxGateway.Contracts.Proto.MxEvent> StreamEventsAsync(CancellationToken cancellationToken)
=> System.Threading.Channels.Channel.CreateUnbounded<ZB.MOM.WW.MxGateway.Contracts.Proto.MxEvent>()
.Reader.ReadAllAsync(cancellationToken);
}
}
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/Docker/README.md
+2
View File
@@ -57,6 +57,8 @@ rsync -av --rsync-path="sudo rsync" \
- Default: `localhost:1102` (non-privileged; sidesteps Windows Firewall
prompt + Linux's root-required bind on port 102).
- Override with `S7_SIM_ENDPOINT` to point at a real S7 CPU on `:102`.
- Tests that write (`S7_1500ArrayWriteTests`) run only with `S7_SIM_ALLOW_WRITES=1`, so a real-CPU
endpoint is never written by accident.
- The driver's S7DriverOptions.Port flows through S7netplus's 5-arg
`Plc(CpuType, host, port, rack, slot)` ctor so the non-standard port
works end-to-end.
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500ArrayWriteTests.cs
+5 -1
View File
@@ -7,7 +7,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests.S7_1500;
/// <summary>
/// #610 item 2 (S7-016): array write round trip against the python-snap7 S7-1500 profile. Skips
/// cleanly when the sim is absent. Uses DB1 bytes 700+ (the profile's 1024-byte DB1 has no seed
/// there), so the writes cannot disturb another test's seeds.
/// there), so the writes cannot disturb another test's seeds. Writes also require
/// <c>S7_SIM_ALLOW_WRITES=1</c>.
/// </summary>
[Collection(Snap7ServerCollection.Name)]
[Trait("Category", "Integration")]
@@ -33,6 +34,9 @@ public sealed class S7_1500ArrayWriteTests(Snap7ServerFixture sim)
public async Task Array_write_then_read_round_trips_through_real_S7comm()
{
if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason);
// S7_SIM_ENDPOINT may point at a real CPU; writing needs an explicit opt-in (simulator runs only).
if (Environment.GetEnvironmentVariable("S7_SIM_ALLOW_WRITES") != "1")
Assert.Skip("Write test: set S7_SIM_ALLOW_WRITES=1 to allow writes to the endpoint under test (simulator only).");
await using var drv = new S7Driver(Options(sim.Host, sim.Port), "s7-arr-write");
await drv.InitializeAsync("{}", TestContext.Current.CancellationToken);
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ArrayWriteAndHealthTests.cs
+49
View File
@@ -85,6 +85,31 @@ public sealed class S7ArrayWriteAndHealthTests
Should.Throw<S7Driver.S7WriteValueException>(() => S7Driver.EncodeArrayBlock(tag, addr, new object[] { 1, "x" })); // format
}
/// <summary>
/// A null element must not silently coerce to 0 (Convert.ToXxx(null) == 0) and land on the PLC: it is a
/// client error, and nothing reaches the wire. The valid write in the same batch is the positive control.
/// </summary>
[Fact]
public async Task Write_array_with_a_null_element_is_BadTypeMismatch_and_writes_nothing()
{
var tag = Arr(S7DataType.Int16, "DB1.DBW0", 2);
Should.Throw<S7Driver.S7WriteValueException>(() =>
S7Driver.EncodeArrayBlock(tag, S7AddressParser.Parse("DB1.DBW0"), new object?[] { 1, null }));
var factory = new RecordingFactory();
using var drv = new S7Driver(Options(tag), "s7-arr-null", factory);
await drv.InitializeAsync("{}", TestContext.Current.CancellationToken);
var res = await drv.WriteAsync(
[new WriteRequest("Arr", new object?[] { 1, null }), new WriteRequest("Arr", new short[] { 7, 8 })],
TestContext.Current.CancellationToken);
res[0].StatusCode.ShouldBe(BadTypeMismatch);
res[1].StatusCode.ShouldBe(0u);
factory.Created[0].BlockWrites.ShouldHaveSingleItem().Bytes.ShouldBe(new byte[] { 0, 7, 0, 8 });
drv.GetHealth().State.ShouldBe(DriverState.Healthy);
}
/// <summary>Bool and wide-type arrays are refused with NotSupported (BadNotSupported on the wire).</summary>
[Theory]
[InlineData(S7DataType.Bool, "DB1.DBX0.0")]
@@ -260,6 +285,30 @@ public sealed class S7ArrayWriteAndHealthTests
drv.GetHealth().State.ShouldBe(DriverState.Healthy);
}
/// <summary>
/// A Faulted that a READ pass raised (access denied on a read) is not the write side's to retire: a
/// successful write must leave it in place until a clean read pass clears it.
/// </summary>
[Fact]
public async Task Successful_write_does_not_clear_a_read_side_Faulted()
{
var (drv, factory) = await TwoTagDriver();
using var _ = drv;
var plc = factory.Created[0];
plc.NextReadThrows = new PlcException(ErrorCode.ReadData, "Accessing object not allowed");
await drv.ReadAsync(["A"], TestContext.Current.CancellationToken);
drv.GetHealth().State.ShouldBe(DriverState.Faulted);
var res = await drv.WriteAsync([new WriteRequest("B", (ushort)2)], TestContext.Current.CancellationToken);
res[0].StatusCode.ShouldBe(0u); // the write really got through
drv.GetHealth().State.ShouldBe(DriverState.Faulted); // ... but it does not own this fault
// Positive control: the read side retires its own fault.
(await drv.ReadAsync(["B"], TestContext.Current.CancellationToken))[0].StatusCode.ShouldBe(0u);
drv.GetHealth().State.ShouldBe(DriverState.Healthy);
}
/// <summary>A transient (Degraded) write failure is still erased by a clean read, as before.</summary>
[Fact]
public async Task Degraded_write_failure_is_still_cleared_by_a_clean_read()
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverReconnectTests.cs
+40
View File
@@ -442,6 +442,46 @@ public sealed class S7DriverReconnectTests
drv.GetHostStatuses()[0].State.ShouldBe(HostState.Running);
}
/// <summary>
/// STAB-21 vs the probe. On a black-holed host the probe's connect used to be cut by the (shorter) probe
/// deadline, which is not a caller cancel but was treated as one, so NO failure was recorded and every
/// probe tick paid a fresh connect attempt. The connect must run on the connect deadline and a timeout
/// must open the backoff window: far fewer attempts than ticks.
/// </summary>
[Fact]
public async Task Probe_connect_timeout_on_a_black_holed_host_opens_the_backoff_window()
{
var opts = new S7DriverOptions
{
Host = "192.0.2.1",
Timeout = TimeSpan.FromMilliseconds(400),
Probe = new S7ProbeOptions
{
Enabled = true,
Interval = TimeSpan.FromMilliseconds(20),
Timeout = TimeSpan.FromMilliseconds(80),
},
RawTags = S7RawTags.Entries(new S7TagDefinition("W0", "DB1.DBW0", S7DataType.UInt16, Writable: false)),
};
var factory = new FakeS7PlcFactory();
using var drv = new S7Driver(opts, "s7-probe-blackhole", factory);
await drv.InitializeAsync("{}", TestContext.Current.CancellationToken);
// Kill the live handle (probe fault), then black-hole every reconnect.
factory.OpenHangsUntilCancelledCount = int.MaxValue;
factory.Created[0].ReadStatusThrows = new System.Net.Sockets.SocketException();
// First reconnect attempt must have been made and failed (positive control), then observe a window
// well inside the 1 s first backoff window.
await WaitUntilAsync(() => factory.Created.Count >= 2, TimeSpan.FromSeconds(5));
factory.Created.Count.ShouldBeGreaterThanOrEqualTo(2);
var atFirstAttempt = factory.Created.Count;
await Task.Delay(TimeSpan.FromMilliseconds(900));
// Without the fix this is ~10+ (one 80 ms-cut attempt per tick); with it the window suppresses them.
(factory.Created.Count - atFirstAttempt).ShouldBeLessThanOrEqualTo(1);
}
private static S7DriverOptions ProbeEnabledOptions() => new()
{
Host = "192.0.2.1",