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 921164b4..6a656a36 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -634,6 +634,20 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, { if (items.Count < 2) continue; var plans = FocasPmcCoalescer.Plan(items, _options.PmcCoalesce.MaxSpanBytes, _options.PmcCoalesce.MaxGapBytes); + if (!plans.Any(p => p.Members.Count >= 2 && !device.CoalesceRejected.ContainsKey(p.Key))) continue; + + // One connect attempt per device per batch: if it fails, every PMC tag of the device fails with + // that status here instead of paying a connect attempt per plan and again per tag. + IFocasClient client; + try { client = await EnsureConnectedAsync(device, ct).ConfigureAwait(false); } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } + catch (Exception) + { + FailMembers(items, FocasStatusMapper.BadCommunicationError, references, results, now, ref served, + $"FOCAS connect failed reading PMC tags of {device.Options.HostAddress}"); + continue; + } + foreach (var plan in plans) { if (plan.Members.Count < 2) continue; @@ -642,7 +656,6 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, var members = plan.Members; try { - var client = await EnsureConnectedAsync(device, ct).ConfigureAwait(false); var (slots, status) = await client .ReadPmcSlotsAsync(plan.Letter, plan.SlotType, plan.Start, plan.End, ct) .ConfigureAwait(false); @@ -659,44 +672,41 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, } Volatile.Write(ref _health, new DriverHealth(DriverState.Healthy, now, null)); } - else if (status == FocasStatusMapper.BadCommunicationError) - { - FailMembers(plan, status, references, results, now, ref served); - } else { - // The CNC refused the merged range. Don't guess which member is at fault: remember the - // range and let the per-tag loop give each member its own answer. - device.CoalesceRejected[plan.Key] = 0; + // The CNC REFUSED the merged range (any return code, mapped or not). Don't guess which + // member is at fault: let the per-tag loop give each its own answer. Remember the range + // so later polls skip the doomed read — except for a transient refusal (EW_BUSY maps to + // BadDeviceFailure), which says nothing about the range. + if (status != FocasStatusMapper.BadDeviceFailure) + device.CoalesceRejected[plan.Key] = 0; _logger.LogDebug( "FOCAS PMC range {Key} on {Host} rejected (0x{Status:X8}) — reading its {Count} tags individually", plan.Key, device.Options.HostAddress, status, members.Count); } } catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } - catch (OperationCanceledException) - { - FailMembers(plan, FocasStatusMapper.BadCommunicationError, references, results, now, ref served); - } catch (Exception) { - FailMembers(plan, FocasStatusMapper.BadCommunicationError, references, results, now, ref served); + // Transport failure / timeout: the link is suspect, so fail the members without per-tag retries. + FailMembers(members, FocasStatusMapper.BadCommunicationError, references, results, now, ref served, + $"FOCAS transport failure reading PMC range {plan.Key}"); } } } return served; - void FailMembers(FocasPmcRangePlan plan, uint status, IReadOnlyList refs, - DataValueSnapshot[] res, DateTime stamp, ref bool[]? servedRef) + void FailMembers(IEnumerable failed, uint status, IReadOnlyList refs, + DataValueSnapshot[] res, DateTime stamp, ref bool[]? servedRef, string reason) { - foreach (var m in plan.Members) + foreach (var m in failed) { res[m.Index] = new DataValueSnapshot(null, status, null, stamp); (servedRef ??= new bool[refs.Count])[m.Index] = true; } Volatile.Write(ref _health, new DriverHealth(DriverState.Degraded, Volatile.Read(ref _health).LastSuccessfulRead, - $"FOCAS status 0x{status:X8} reading PMC range {plan.Key}")); + $"{reason} (0x{status:X8})")); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs index cdcecfea..ed69e3cf 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/IFocasClient.cs @@ -84,7 +84,10 @@ public interface IFocasClient : IDisposable /// First byte address. /// Last byte address, inclusive; a whole number of slots from . /// The cancellation token. - /// The raw slot values (null when not Good) and the mapped OPC UA status. + /// + /// The raw slot values (null when not Good) and the mapped OPC UA status. A returned non-Good status is + /// always a CNC return-code refusal; a transport failure or timeout throws instead. + /// Task<(IReadOnlyList? slots, uint status)> ReadPmcSlotsAsync( string letter, FocasDataType slotType, int start, int end, CancellationToken cancellationToken); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs index 311d3cbd..d4789061 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs @@ -71,7 +71,10 @@ public sealed class WireFocasClient : IFocasClient public async Task<(IReadOnlyList? slots, uint status)> ReadPmcSlotsAsync( string letter, FocasDataType slotType, int start, int end, CancellationToken cancellationToken) { - if (!_wire.IsConnected) return (null, FocasStatusMapper.BadCommunicationError); + // Contract: a RETURNED status is always a CNC return code (a refusal); a transport failure THROWS. + // The coalescing caller needs that distinction — it falls back per tag on a refusal but must not + // re-hit a dead link (#266). + RequireConnected(); cancellationToken.ThrowIfCancellationRequested(); var area = FocasPmcAreaLookup.FromLetter(letter); @@ -85,9 +88,9 @@ public sealed class WireFocasClient : IFocasClient return (null, FocasStatusMapper.MapFocasReturn(result.Rc)); return (result.Value.Values, FocasStatusMapper.Good); } - catch (FocasWireException ex) + catch (FocasWireException ex) when (ex.Rc is short rc) { - return (null, ex.Rc is short rc ? FocasStatusMapper.MapFocasReturn(rc) : FocasStatusMapper.BadCommunicationError); + return (null, FocasStatusMapper.MapFocasReturn(rc)); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcCoalescingDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcCoalescingDriverTests.cs index 6d26c83e..8c1219d7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcCoalescingDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPmcCoalescingDriverTests.cs @@ -132,6 +132,68 @@ public sealed class FocasPmcCoalescingDriverTests await drv.ShutdownAsync(CancellationToken.None); } + [Fact] + public async Task A_refusal_with_an_unmapped_rc_that_maps_to_BadCommunicationError_still_falls_back_per_tag() + { + var (drv, c) = await StartAsync(null, + Tag("A", "R100", FocasDataType.Byte), Tag("B", "R101", FocasDataType.Byte)); + c.PmcRangeRejectors.Add((_, _, _) => true); + c.PmcRangeRejectStatus = FocasStatusMapper.BadCommunicationError; // what an unlisted rc / EW_PARITY / EW_SOCKET maps to + c.Values["R100"] = (sbyte)1; c.Values["R101"] = (sbyte)2; + + var r = await drv.ReadAsync(["A", "B"], CancellationToken.None); + r.Select(s => s.Value).ShouldBe([(object)(sbyte)1, (sbyte)2]); + r.ShouldAllBe(s => s.StatusCode == FocasStatusMapper.Good); + + await drv.ReadAsync(["A", "B"], CancellationToken.None); + c.PmcRangeReads.Count.ShouldBe(1, "a deterministic rc refusal is memoised"); + await drv.ShutdownAsync(CancellationToken.None); + } + + [Fact] + public async Task A_transient_busy_refusal_is_not_memoised() + { + var (drv, c) = await StartAsync(null, + Tag("A", "R100", FocasDataType.Byte), Tag("B", "R101", FocasDataType.Byte)); + var busy = true; + c.PmcRangeRejectors.Add((_, _, _) => busy); + c.PmcRangeRejectStatus = FocasStatusMapper.BadDeviceFailure; // EW_BUSY + c.Values["R100"] = (sbyte)1; c.Values["R101"] = (sbyte)2; + + (await drv.ReadAsync(["A", "B"], CancellationToken.None)).ShouldAllBe(s => s.StatusCode == FocasStatusMapper.Good); + busy = false; + c.PmcMemory[("R", 100)] = 5; c.PmcMemory[("R", 101)] = 6; + var r = await drv.ReadAsync(["A", "B"], CancellationToken.None); + + c.PmcRangeReads.Count.ShouldBe(2, "the range is retried once the CNC is no longer busy"); + r.Select(s => s.Value).ShouldBe([(object)(sbyte)5, (sbyte)6]); + await drv.ShutdownAsync(CancellationToken.None); + } + + [Fact] + public async Task A_down_device_costs_one_connect_attempt_per_batch_not_one_per_plan_and_tag() + { + var client = new FakeFocasClient { ThrowOnConnect = true }; + var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions(Host)], + Probe = new FocasProbeOptions { Enabled = false }, + RawTags = FocasRawTags.Entries(new[] + { + Tag("A", "R100", FocasDataType.Byte), Tag("B", "R101", FocasDataType.Byte), + Tag("C", "R900", FocasDataType.Byte), Tag("D", "R2000", FocasDataType.Byte), + }), + }, "drv-1", new FakeFocasClientFactory { Customise = () => client }); + await drv.InitializeAsync("{}", CancellationToken.None); + var before = client.ConnectCount; + + var r = await drv.ReadAsync(["A", "B", "C", "D"], CancellationToken.None); + + r.ShouldAllBe(s => s.StatusCode == FocasStatusMapper.BadCommunicationError); + (client.ConnectCount - before).ShouldBe(1); + await drv.ShutdownAsync(CancellationToken.None); + } + [Fact] public async Task A_transport_failure_fails_every_member_without_per_tag_retries() {