diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index bdd6b0af..305efd21 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -306,7 +306,16 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, Volatile.Read(ref _health).LastSuccessfulRead, $"FOCAS status 0x{status:X8} reading {reference}")); } - catch (OperationCanceledException) { throw; } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { throw; } + catch (OperationCanceledException) + { + // Per-call timeout (not external cancellation) — the read stalled past the device + // Timeout budget. Surface a recoverable comm error so the BadWaitingForInitialData + // seed is overwritten and health degrades, instead of the read hanging forever. + results[i] = new DataValueSnapshot(null, FocasStatusMapper.BadCommunicationError, null, now); + Volatile.Write(ref _health, new DriverHealth(DriverState.Degraded, + Volatile.Read(ref _health).LastSuccessfulRead, $"FOCAS read timed out for {reference}")); + } catch (Exception ex) { results[i] = new DataValueSnapshot(null, FocasStatusMapper.BadCommunicationError, null, now); @@ -356,7 +365,15 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, var status = await client.WriteAsync(parsed, def.DataType, w.Value, cancellationToken).ConfigureAwait(false); results[i] = new WriteResult(status); } - catch (OperationCanceledException) { throw; } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { throw; } + catch (OperationCanceledException) + { + // Per-call timeout (not external cancellation) — the write stalled past the device + // Timeout budget. Surface a recoverable comm error rather than aborting the batch. + results[i] = new WriteResult(FocasStatusMapper.BadCommunicationError); + Volatile.Write(ref _health, new DriverHealth(DriverState.Degraded, + Volatile.Read(ref _health).LastSuccessfulRead, $"FOCAS write timed out for {w.FullReference}")); + } catch (NotSupportedException nse) { results[i] = new WriteResult(FocasStatusMapper.BadNotSupported); @@ -1113,7 +1130,11 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, device.Client = null; } - device.Client = _clientFactory.Create(); + // Wrap the raw wire client so every operation on the device's single FOCAS/2 socket is + // serialized (request→response on one socket cannot interleave) and time-bounded. Without + // this, the equipment poll, fixed-tree loop, probe, and recycle loop collide on the shared + // socket and a stalled read blocks forever — leaving bound tags at BadWaitingForInitialData. + device.Client = new SynchronizedFocasClient(_clientFactory.Create(), _options.Timeout); try { await device.Client.ConnectAsync(device.ParsedAddress, _options.Timeout, ct).ConfigureAwait(false); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs index 0ccf9cb4..49f2d667 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriverFactoryExtensions.cs @@ -195,12 +195,41 @@ public static class FocasDriverFactoryExtensions AllowTrailingCommas = true, }; + /// + /// Reads a JSON property as a string, tolerating a JSON number token as well. The + /// AdminUI persists the FOCAS Series enum as its integer value (e.g. "series":6), + /// while this DTO models Series as a string handed to + /// (Enum.TryParse accepts the numeric form). Without this, System.Text.Json throws + /// "Cannot get the value of a token type 'Number' as a string" on the bare number and the + /// driver falls back to a stub. Accepts string / number / null and emits a string. + /// + internal sealed class FlexibleStringConverter : JsonConverter + { + public override string? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => + reader.TokenType switch + { + JsonTokenType.String => reader.GetString(), + JsonTokenType.Number => reader.TryGetInt64(out var n) + ? n.ToString(System.Globalization.CultureInfo.InvariantCulture) + : reader.GetDouble().ToString(System.Globalization.CultureInfo.InvariantCulture), + JsonTokenType.Null => null, + _ => throw new JsonException($"Expected string, number, or null but got {reader.TokenType}."), + }; + + public override void Write(Utf8JsonWriter writer, string? value, JsonSerializerOptions options) + { + if (value is null) writer.WriteNullValue(); + else writer.WriteStringValue(value); + } + } + internal sealed class FocasDriverConfigDto { /// Gets or sets the FOCAS client factory backend name (e.g. "wire" or "stub"). public string? Backend { get; init; } /// Gets or sets the CNC series for this driver. + [JsonConverter(typeof(FlexibleStringConverter))] public string? Series { get; init; } /// Gets or sets the operation timeout in milliseconds. @@ -234,6 +263,7 @@ public static class FocasDriverFactoryExtensions public string? DeviceName { get; init; } /// Gets or sets the CNC series for this device (overrides top-level series if provided). + [JsonConverter(typeof(FlexibleStringConverter))] public string? Series { get; init; } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasHostAddress.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasHostAddress.cs index ff81bb49..a8f14a08 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasHostAddress.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasHostAddress.cs @@ -21,9 +21,19 @@ public sealed record FocasHostAddress(string Host, int Port) { if (string.IsNullOrWhiteSpace(value)) return null; const string prefix = "focas://"; - if (!value.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) return null; - var body = value[prefix.Length..]; + // Canonical form is focas://{ip}[:{port}], but the AdminUI persists the device host as a + // scheme-less "{ip}[:{port}]" (e.g. "10.201.31.5:8193"). Accept that too: take the body + // after focas:// when present, else the whole value when it carries NO other URI scheme + // (a "://" that isn't ours — e.g. http:// — is still rejected). The host-contains-colon + // guard below then rejects malformed scheme typos like "focas:10.0.0.5:8193". + string body; + if (value.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + body = value[prefix.Length..]; + else if (!value.Contains("://", StringComparison.Ordinal)) + body = value; + else + return null; if (string.IsNullOrEmpty(body)) return null; var colonIdx = body.LastIndexOf(':'); @@ -39,7 +49,9 @@ public sealed record FocasHostAddress(string Host, int Port) { host = body; } - if (string.IsNullOrEmpty(host)) return null; + // Empty host, or a host still carrying a colon (e.g. the malformed "focas:10.0.0.5" left + // when someone wrote "focas:10.0.0.5:8193" without the //), is invalid. + if (string.IsNullOrEmpty(host) || host.Contains(':', StringComparison.Ordinal)) return null; return new FocasHostAddress(host, port); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/SynchronizedFocasClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/SynchronizedFocasClient.cs new file mode 100644 index 00000000..e3f64078 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/SynchronizedFocasClient.cs @@ -0,0 +1,152 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + +/// +/// Decorates an so that every wire operation on the device's +/// single FOCAS/2 socket is (1) serialized against all other operations and +/// (2) time-bounded. +/// +/// +/// FOCAS/2 over TCP:8193 is a strict request→response protocol on ONE socket. The +/// driver holds a single per device, but several independent loops +/// read from it concurrently — the equipment poll (), the +/// fixed-tree loop (FixedTreeLoopAsync), the connectivity probe, and the recycle loop. +/// Without serialization, two reads interleave their send(request); read(response) on the +/// same socket: one reader consumes the other's response PDU and the victim then blocks forever +/// waiting for bytes that never arrive — leaving the bound OPC UA node stuck at +/// BadWaitingForInitialData. This was the root cause of FOCAS equipment tags never +/// surfacing a value while the probe reported HEALTHY (the probe reads work single-threaded on a +/// dev box, but collide deployed once the fixed-tree loop runs concurrently). +/// +/// The gate ( of count 1) makes each request→response atomic on +/// the socket. The per-call timeout ensures a stalled response can never hold the gate — and thus +/// the socket — indefinitely; a hung read surfaces as a recoverable error at the configured +/// Timeout budget instead of permanent silence. The gate and timeout are paired +/// deliberately: a lock around an unbounded read would deadlock all I/O for the device. +/// +/// and are serialized but NOT bounded by +/// this decorator's call timeout — they carry their own budgets (the connect timeout argument and +/// the probe's caller-supplied linked token respectively), and double-bounding would shrink them. +/// +public sealed class SynchronizedFocasClient : IFocasClient +{ + private readonly IFocasClient _inner; + private readonly TimeSpan _callTimeout; + private readonly SemaphoreSlim _gate = new(1, 1); + + /// Wraps with per-device serialization + a per-call timeout. + /// The underlying FOCAS client to serialize access to. + /// + /// The budget applied to each data read/write. or negative disables + /// the per-call timeout (callers' own cancellation tokens still apply). + /// + public SynchronizedFocasClient(IFocasClient inner, TimeSpan callTimeout) + { + _inner = inner ?? throw new ArgumentNullException(nameof(inner)); + _callTimeout = callTimeout; + } + + /// + public bool IsConnected => _inner.IsConnected; + + /// + public Task ConnectAsync(FocasHostAddress address, TimeSpan timeout, CancellationToken cancellationToken) => + RunGatedAsync(ct => _inner.ConnectAsync(address, timeout, ct), cancellationToken); + + /// + public Task ProbeAsync(CancellationToken cancellationToken) => + RunGatedAsync(ct => _inner.ProbeAsync(ct), cancellationToken); + + /// + public Task<(object? value, uint status)> ReadAsync( + FocasAddress address, FocasDataType type, CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.ReadAsync(address, type, ct), cancellationToken); + + /// + public Task WriteAsync( + FocasAddress address, FocasDataType type, object? value, CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.WriteAsync(address, type, value, ct), cancellationToken); + + /// + public Task> ReadAlarmsAsync(CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.ReadAlarmsAsync(ct), cancellationToken); + + /// + public Task GetSysInfoAsync(CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.GetSysInfoAsync(ct), cancellationToken); + + /// + public Task> GetAxisNamesAsync(CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.GetAxisNamesAsync(ct), cancellationToken); + + /// + public Task> GetSpindleNamesAsync(CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.GetSpindleNamesAsync(ct), cancellationToken); + + /// + public Task ReadDynamicAsync(int axisIndex, CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.ReadDynamicAsync(axisIndex, ct), cancellationToken); + + /// + public Task GetProgramInfoAsync(CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.GetProgramInfoAsync(ct), cancellationToken); + + /// + public Task GetTimerAsync(FocasTimerKind kind, CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.GetTimerAsync(kind, ct), cancellationToken); + + /// + public Task> GetServoLoadsAsync(CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.GetServoLoadsAsync(ct), cancellationToken); + + /// + public Task> GetSpindleLoadsAsync(CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.GetSpindleLoadsAsync(ct), cancellationToken); + + /// + public Task> GetSpindleMaxRpmsAsync(CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.GetSpindleMaxRpmsAsync(ct), cancellationToken); + + /// + public Task> GetPositionFiguresAsync(CancellationToken cancellationToken) => + RunBoundedAsync(ct => _inner.GetPositionFiguresAsync(ct), cancellationToken); + + /// + public void Dispose() + { + _inner.Dispose(); + _gate.Dispose(); + } + + // Gate only — the caller already governs the budget (connect timeout arg / probe linked token). + private async Task RunGatedAsync(Func> op, CancellationToken ct) + { + await _gate.WaitAsync(ct).ConfigureAwait(false); + try { return await op(ct).ConfigureAwait(false); } + finally { _gate.Release(); } + } + + private async Task RunGatedAsync(Func op, CancellationToken ct) + { + await _gate.WaitAsync(ct).ConfigureAwait(false); + try { await op(ct).ConfigureAwait(false); } + finally { _gate.Release(); } + } + + // Gate + per-call timeout. A fired timeout surfaces as OperationCanceledException whose token is + // the linked (not the caller's) token — callers distinguish it from real cancellation by testing + // their own token's IsCancellationRequested. + private async Task RunBoundedAsync(Func> op, CancellationToken ct) + { + await _gate.WaitAsync(ct).ConfigureAwait(false); + try + { + if (_callTimeout <= TimeSpan.Zero) + return await op(ct).ConfigureAwait(false); + + using var linked = CancellationTokenSource.CreateLinkedTokenSource(ct); + linked.CancelAfter(_callTimeout); + return await op(linked.Token).ConfigureAwait(false); + } + finally { _gate.Release(); } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasDriverProbeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasDriverProbeTests.cs index 81abcb08..3dcc77e5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasDriverProbeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasDriverProbeTests.cs @@ -70,9 +70,10 @@ public sealed class FocasDriverProbeTests [Fact] public async Task MalformedHostAddress_Returns_OkFalse_WithNoHostPortMessage() { - // "not-a-focas-url" is not a focas:// URL — TryParse returns null. + // A foreign URI scheme ("http://…") is rejected by TryParse → null. (A bare + // "{ip}[:{port}]" without a scheme is now tolerated, so it can't be the malformed case.) var result = await Probe.ProbeAsync( - "{\"devices\":[{\"hostAddress\":\"not-a-focas-url\"}]}", + "{\"devices\":[{\"hostAddress\":\"http://10.0.0.5/\"}]}", TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFactoryConfigTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFactoryConfigTests.cs index 94531969..852ab00d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFactoryConfigTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFactoryConfigTests.cs @@ -38,6 +38,25 @@ public sealed class FocasFactoryConfigTests drv.Options.FixedTree.TimerPollInterval.ShouldBe(TimeSpan.FromSeconds(30)); } + /// + /// The AdminUI persists FocasCncSeries as its integer value (e.g. "series":6 = Thirty_i) — + /// a bare JSON number. The factory must tolerate it (via FlexibleStringConverter) and build the + /// real driver, not throw + fall back to a stub. Regression for the 2026-06-26 wonder data-plane + /// deploy where the driver stubbed on "Cannot get the value of a token type 'Number' as a string". + /// + [Fact] + public void CreateInstance_accepts_numeric_Series_from_AdminUI_serialization() + { + const string json = """ + {"Backend":"wire","series":6,"devices":[{"hostAddress":"10.0.0.5:8193","deviceName":"Makino","series":6,"positionDecimalPlaces":0}]} + """; + + var drv = FocasDriverFactoryExtensions.CreateInstance("drv-1", json); + + drv.Options.Devices.ShouldHaveSingleItem(); + drv.Options.Devices[0].Series.ShouldBe(FocasCncSeries.Thirty_i); + } + /// Verifies that the AlarmProjection configuration section is mapped to driver options. [Fact] public void CreateInstance_maps_AlarmProjection_section_onto_options() diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasIoSerializationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasIoSerializationTests.cs new file mode 100644 index 00000000..4bc271f6 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasIoSerializationTests.cs @@ -0,0 +1,207 @@ +using System.Diagnostics; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// Coverage for the FOCAS data-plane fix (2026-06-25 equipment-tag investigation): all wire I/O +/// on a device's single FOCAS/2 socket must be serialized (request→response cannot interleave) +/// and every steady-state read/write must be time-bounded so a stalled CNC read surfaces as a +/// recoverable error instead of hanging forever at BadWaitingForInitialData. See +/// docs/plans/2026-06-25-otopcua-equipment-dataplane-investigation.md. +/// +[Trait("Category", "Unit")] +public sealed class FocasIoSerializationTests +{ + private static readonly FocasAddress Macro500 = new(FocasAreaKind.Macro, null, 500, null); + + // ---- SynchronizedFocasClient: serialization ---- + + [Fact] + public async Task Concurrent_reads_are_serialized_onto_the_inner_client() + { + var inner = new RecordingClient { ReadDelay = TimeSpan.FromMilliseconds(20) }; + await using var _ = NoopDispose(inner); + var client = new SynchronizedFocasClient(inner, TimeSpan.FromSeconds(5)); + + var reads = Enumerable.Range(0, 8) + .Select(_ => client.ReadAsync(Macro500, FocasDataType.Float64, CancellationToken.None)); + await Task.WhenAll(reads); + + inner.MaxConcurrency.ShouldBe(1); // never more than one wire op on the socket at a time + inner.ReadCount.ShouldBe(8); + } + + // ---- SynchronizedFocasClient: per-call timeout ---- + + [Fact] + public async Task A_hung_read_is_bounded_by_the_call_timeout() + { + var inner = new RecordingClient { BlockReadUntilCancelled = true }; + var client = new SynchronizedFocasClient(inner, TimeSpan.FromMilliseconds(100)); + + var sw = Stopwatch.StartNew(); + await Should.ThrowAsync( + () => client.ReadAsync(Macro500, FocasDataType.Float64, CancellationToken.None)); + sw.Stop(); + + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(2)); // bounded, not the indefinite OS TCP wait + } + + [Fact] + public async Task A_hung_read_does_not_hold_the_socket_for_the_next_call() + { + // The gate must be released when a bounded call times out, otherwise one stall would wedge + // every subsequent op on the device. Read #1 hangs (times out); read #2 must still proceed. + var inner = new TimeoutThenServeClient { FirstCallBlocks = true }; + var client = new SynchronizedFocasClient(inner, TimeSpan.FromMilliseconds(100)); + + await Should.ThrowAsync( + () => client.ReadAsync(Macro500, FocasDataType.Float64, CancellationToken.None)); + + var (value, status) = await client.ReadAsync(Macro500, FocasDataType.Float64, CancellationToken.None); + status.ShouldBe(FocasStatusMapper.Good); + value.ShouldBe(42); + } + + [Fact] + public async Task Probe_is_not_bounded_by_the_call_timeout() + { + // Connect/Probe carry their own budgets; the decorator must not shrink them to its read budget. + var inner = new RecordingClient { ProbeDelay = TimeSpan.FromMilliseconds(200) }; + var client = new SynchronizedFocasClient(inner, TimeSpan.FromMilliseconds(50)); + + var result = await client.ProbeAsync(CancellationToken.None); + + result.ShouldBeTrue(); + } + + [Fact] + public async Task Zero_call_timeout_disables_the_per_call_bound() + { + var inner = new RecordingClient { ReadDelay = TimeSpan.FromMilliseconds(120) }; + var client = new SynchronizedFocasClient(inner, TimeSpan.Zero); + + var (value, status) = await client.ReadAsync(Macro500, FocasDataType.Float64, CancellationToken.None); + + status.ShouldBe(FocasStatusMapper.Good); + value.ShouldBe(42); + } + + [Fact] + public void Dispose_disposes_the_inner_client() + { + var inner = new RecordingClient(); + var client = new SynchronizedFocasClient(inner, TimeSpan.FromSeconds(1)); + + client.Dispose(); + + inner.DisposeCount.ShouldBe(1); + } + + // ---- Driver level: a timed-out read overwrites the seed with a recoverable status ---- + + [Fact] + public async Task Driver_read_that_times_out_returns_BadCommunicationError_not_a_hang() + { + var factory = new FakeFocasClientFactory { Customise = () => new RecordingClient { BlockReadUntilCancelled = true } }; + var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], + Tags = [new FocasTagDefinition("CustomVar", "focas://10.0.0.5:8193", "MACRO:500", FocasDataType.Float64)], + Probe = new FocasProbeOptions { Enabled = false }, + Timeout = TimeSpan.FromMilliseconds(150), + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var sw = Stopwatch.StartNew(); + var snap = (await drv.ReadAsync(["CustomVar"], CancellationToken.None)).Single(); + sw.Stop(); + + snap.StatusCode.ShouldBe(FocasStatusMapper.BadCommunicationError); + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(2)); // bounded by Timeout, not hung + } + + [Fact] + public async Task Driver_read_does_not_propagate_a_call_timeout_as_cancellation() + { + // The per-call timeout must NOT bubble out of ReadAsync as OperationCanceledException — that + // would abort the whole poll batch. It must be caught and turned into a per-tag Bad status. + var factory = new FakeFocasClientFactory { Customise = () => new RecordingClient { BlockReadUntilCancelled = true } }; + var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], + Tags = [new FocasTagDefinition("CustomVar", "focas://10.0.0.5:8193", "MACRO:500", FocasDataType.Float64)], + Probe = new FocasProbeOptions { Enabled = false }, + Timeout = TimeSpan.FromMilliseconds(120), + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + // Should complete (not throw) with a Bad snapshot, even though the caller's token is never cancelled. + var snaps = await drv.ReadAsync(["CustomVar"], CancellationToken.None); + snaps.Single().StatusCode.ShouldBe(FocasStatusMapper.BadCommunicationError); + } + + private static DisposeGuard NoopDispose(IDisposable d) => new(d); + + private sealed class DisposeGuard(IDisposable inner) : IAsyncDisposable + { + public ValueTask DisposeAsync() { inner.Dispose(); return ValueTask.CompletedTask; } + } + + /// Fake that records concurrency + optionally delays/blocks reads and probes. + private class RecordingClient : FakeFocasClient + { + private int _current; + public int MaxConcurrency; + public int ReadCount; + public TimeSpan ReadDelay = TimeSpan.Zero; + public bool BlockReadUntilCancelled; + public TimeSpan ProbeDelay = TimeSpan.Zero; + + public override async Task<(object? value, uint status)> ReadAsync( + FocasAddress address, FocasDataType type, CancellationToken ct) + { + Interlocked.Increment(ref ReadCount); + var observed = Interlocked.Increment(ref _current); + InterlockedMax(ref MaxConcurrency, observed); + try + { + if (BlockReadUntilCancelled) await Task.Delay(Timeout.Infinite, ct).ConfigureAwait(false); + else if (ReadDelay > TimeSpan.Zero) await Task.Delay(ReadDelay, ct).ConfigureAwait(false); + return ((object?)42, FocasStatusMapper.Good); + } + finally { Interlocked.Decrement(ref _current); } + } + + public override async Task ProbeAsync(CancellationToken ct) + { + if (ProbeDelay > TimeSpan.Zero) await Task.Delay(ProbeDelay, ct).ConfigureAwait(false); + return true; + } + + private static void InterlockedMax(ref int target, int value) + { + int seen; + do { seen = Volatile.Read(ref target); if (value <= seen) return; } + while (Interlocked.CompareExchange(ref target, value, seen) != seen); + } + } + + /// First read blocks until cancelled; subsequent reads serve a Good value immediately. + private sealed class TimeoutThenServeClient : FakeFocasClient + { + public bool FirstCallBlocks; + private int _calls; + + public override async Task<(object? value, uint status)> ReadAsync( + FocasAddress address, FocasDataType type, CancellationToken ct) + { + var n = Interlocked.Increment(ref _calls); + if (n == 1 && FirstCallBlocks) await Task.Delay(Timeout.Infinite, ct).ConfigureAwait(false); + return ((object?)42, FocasStatusMapper.Good); + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasReadWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasReadWriteTests.cs index 0310ade5..09df7adc 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasReadWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasReadWriteTests.cs @@ -245,6 +245,32 @@ public sealed class FocasReadWriteTests /// Verifies that cancellation signals are propagated. [Fact] public async Task Cancellation_propagates() + { + var (drv, factory) = NewDriver( + new FocasTagDefinition("X", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte)); + await drv.InitializeAsync("{}", CancellationToken.None); + using var cts = new CancellationTokenSource(); + cts.Cancel(); + factory.Customise = () => new FakeFocasClient + { + ThrowOnRead = true, + Exception = new OperationCanceledException(cts.Token), + }; + + // A CANCELLATION of the caller's token must propagate (abort the read). This is distinct + // from a per-call timeout — an OCE raised while the caller's token is still live is swallowed + // to a per-tag BadCommunicationError (see Swallows_a_spurious_read_OCE_when_caller_not_cancelled). + await Should.ThrowAsync( + () => drv.ReadAsync(["X"], cts.Token)); + } + + /// + /// An OperationCanceledException from the wire read while the CALLER'S token is NOT cancelled + /// (e.g. a per-call timeout firing) must be turned into a per-tag BadCommunicationError, not + /// propagated — otherwise one stalled tag would abort the whole poll batch. + /// + [Fact] + public async Task Swallows_a_spurious_read_OCE_when_caller_not_cancelled() { var (drv, factory) = NewDriver( new FocasTagDefinition("X", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte)); @@ -255,8 +281,8 @@ public sealed class FocasReadWriteTests Exception = new OperationCanceledException(), }; - await Should.ThrowAsync( - () => drv.ReadAsync(["X"], CancellationToken.None)); + var snap = (await drv.ReadAsync(["X"], CancellationToken.None)).Single(); + snap.StatusCode.ShouldBe(FocasStatusMapper.BadCommunicationError); } /// Verifies that ShutdownAsync disposes the client. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasScaffoldingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasScaffoldingTests.cs index 4d5e6c30..290074b5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasScaffoldingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasScaffoldingTests.cs @@ -20,6 +20,9 @@ public sealed class FocasScaffoldingTests [InlineData("focas://cnc-01.factory.internal:8193", "cnc-01.factory.internal", 8193)] [InlineData("focas://10.0.0.5:12345", "10.0.0.5", 12345)] [InlineData("FOCAS://10.0.0.5:8193", "10.0.0.5", 8193)] // case-insensitive scheme + [InlineData("10.201.31.5:8193", "10.201.31.5", 8193)] // scheme-less (AdminUI-persisted form) + [InlineData("10.0.0.5", "10.0.0.5", 8193)] // scheme-less, default port + [InlineData("cnc-01.factory.internal:8193", "cnc-01.factory.internal", 8193)] // scheme-less hostname public void HostAddress_parses_valid(string input, string host, int port) { var parsed = FocasHostAddress.TryParse(input); @@ -224,9 +227,11 @@ public sealed class FocasScaffoldingTests [Fact] public async Task InitializeAsync_malformed_address_faults() { + // A non-focas:// URI scheme is rejected by TryParse (a bare "{ip}[:{port}]" is now + // tolerated, so the malformed case must carry a foreign scheme). var drv = new FocasDriver(new FocasDriverOptions { - Devices = [new FocasDeviceOptions("not-an-address")], + Devices = [new FocasDeviceOptions("http://10.0.0.5/")], }, "drv-1"); await Should.ThrowAsync(