diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 3ddd7e82..c9c2f669 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -712,12 +712,12 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, IAbCipTagRuntime? probeRuntime = null; while (!ct.IsCancellationRequested) { - var success = false; - // The probe's backoff verdict for this tick: true = record success, false = record - // failure, null = no signal (a read that returned a non-zero status without throwing - // never recorded either, and still doesn't). Recorded AFTER the try/catch so the - // per-device connect gate is taken outside a catch block — see below. + // The probe's verdicts for this tick, decided SEPARATELY (review 07 H10): + // backoffVerdict — true = record success, false = record failure, null = no signal; + // hostState — the host transition, null = no evidence either way (leave it). + // Recorded AFTER the try/catch so the per-device connect gate is taken outside a catch block. bool? backoffVerdict = null; + HostState? hostState = HostState.Stopped; try { probeRuntime ??= _tagFactory.Create(probeParams); @@ -729,14 +729,51 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, state.ProbeInitialized = true; } await probeRuntime.ReadAsync(ct).ConfigureAwait(false); - success = probeRuntime.GetStatus() == 0; - if (success) + var status = probeRuntime.GetStatus(); + if (status == 0) + { backoffVerdict = true; // Device reachable — reset the data-path window + hostState = HostState.Running; + } + else if (AbCipRefusals.IsDeviceRefusal(status)) + { + // The controller answered, refusing the probe tag: reachable. + backoffVerdict = true; + hostState = HostState.Running; + WarnProbeRefusedOnce(state, ((AbStatus)status).ToString()); + } } catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; } + catch (Exception ex) when (AbCipRefusals.IsDeviceRefusal(ex)) + { + // H10: a missing / typo'd / download-deleted probe tag. The controller ANSWERED over a working session, + // so this is a config error on the probe tag — not a Forward-Open failure. Recording a failure here + // every tick saturated the backoff (5 s ticks vs a 30 s cap ⇒ ShouldAttempt never true) and blacked out + // every cold / evicted data tag on a reachable device, indefinitely. + backoffVerdict = true; + hostState = HostState.Running; + WarnProbeRefusedOnce(state, ex.Message); + // Keep an initialised runtime (its lease stays up — no Forward Open/Close per tick); one whose + // initialisation threw is re-created next tick, as before. + if (!state.ProbeInitialized) + { + try { probeRuntime?.Dispose(); } catch { } + probeRuntime = null; + } + } + catch (AbCipTagConfigurationException ace) + { + // The probe tag's own name cannot become a request — it never reached the device, so it is no evidence + // either way: no backoff signal, no host transition, one Warning. + try { probeRuntime?.Dispose(); } catch { } + probeRuntime = null; + state.ProbeInitialized = false; + hostState = null; + WarnProbeRefusedOnce(state, ace.Message); + } catch (Exception ex) { // Wire / init error — tear down the probe runtime so the next tick re-creates it. @@ -748,9 +785,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { probeRuntime?.Dispose(); } catch { } probeRuntime = null; state.ProbeInitialized = false; - // Keep the data-path window open — unless the probe tag's own name is malformed - // — it never reached the device, so it is no evidence either way. - backoffVerdict = ex is AbCipTagConfigurationException ? null : false; + backoffVerdict = false; // the device did not answer — keep the data-path window open } // ConnectionBackoff is caller-serialized, and the data path now records under the @@ -780,7 +815,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } } - TransitionDeviceState(state, success ? HostState.Running : HostState.Stopped); + if (hostState is { } host) TransitionDeviceState(state, host); try { await Task.Delay(_options.Probe.Interval, ct).ConfigureAwait(false); } catch (OperationCanceledException) { break; } @@ -789,6 +824,16 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { probeRuntime?.Dispose(); } catch { } } + /// Logs, once per device state, that the controller answered but refused the configured probe tag. + private void WarnProbeRefusedOnce(DeviceState state, string reason) + { + if (Interlocked.Exchange(ref state.ProbeRefusalWarned, 1) != 0) return; + _logger.LogWarning( + "AbCip probe tag {ProbeTag} on device {Device} cannot be read: {Reason}. The controller answered, so the device " + + "is treated as reachable; fix Probe.ProbeTagPath for driver {DriverInstanceId} (logged once per device)", + _options.Probe.ProbeTagPath, state.Options.HostAddress, reason, _driverInstanceId); + } + private void TransitionDeviceState(DeviceState state, HostState newState) { HostState old; @@ -2096,6 +2141,9 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// Gets or sets whether the probe has been initialized. public bool ProbeInitialized { get; set; } + /// 1 once the probe-tag refusal Warning has been logged for this device state. + public int ProbeRefusalWarned; + /// /// The fire-and-forget probe loop's . Stored so /// can await the loop's clean exit after diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipProbeRefusalTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipProbeRefusalTests.cs new file mode 100644 index 00000000..c0693b8b --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipProbeRefusalTests.cs @@ -0,0 +1,86 @@ +using Microsoft.Extensions.Logging; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; +using CapturingLogger = ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests.AbCipLoggingTests.CapturingLogger; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Review 07 H10: a typo'd (or download-deleted) Probe.ProbeTagPath used to record a Forward-Open +/// FAILURE every probe tick, so the device's backoff window never closed and every cold / evicted data tag read +/// BadCommunicationError forever — a permanent device blackout that a restart did not cure. The controller +/// ANSWERED, so the probe now counts it as reachable: backoff success, host Running, one config Warning. +/// +[Trait("Category", "Unit")] +public sealed class AbCipProbeRefusalTests +{ + private const string Device = "ab://10.0.0.5/1,0"; + + private static (AbCipDriver Driver, CapturingLogger Logger) Build(Exception probeFault) + { + var logger = new CapturingLogger(); + var factory = new FakeAbCipTagFactory + { + Customise = p => p.TagName == "ProbeTagTypo" + ? new FakeAbCipTag(p) { ThrowOnInitialize = true, Exception = probeFault } + : new FakeAbCipTag(p) { Value = 7 }, + }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(Device)], + RawTags = AbCipRawTags.From(new AbCipTagDefinition("Speed", Device, "Motor1.Speed", AbCipDataType.DInt)), + Probe = new AbCipProbeOptions + { + Enabled = true, + ProbeTagPath = "ProbeTagTypo", + Interval = TimeSpan.FromMilliseconds(10), + Timeout = TimeSpan.FromMilliseconds(100), + }, + }, "abcip-probe-refusal", factory, logger: logger); + return (drv, logger); + } + + private static async Task WaitUntilAsync(Func condition) + { + var deadline = DateTime.UtcNow.AddSeconds(5); + while (!condition()) + { + if (DateTime.UtcNow > deadline) throw new TimeoutException("condition never met"); + await Task.Delay(5); + } + } + + /// A NotFound probe tag leaves the device reachable: data tags still connect, host is Running, warned once. + [Fact] + public async Task A_missing_probe_tag_does_not_black_out_the_device() + { + var (drv, logger) = Build(new AbTransportException(AbStatus.NotFound, "NotFound")); + await drv.InitializeAsync("{}", CancellationToken.None); + await WaitUntilAsync(() => drv.GetHostStatuses().Single().State == HostState.Running); + await Task.Delay(100); // many probe ticks + + var r = await drv.ReadAsync(["Speed"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); // a cold create, NOT throttled + r[0].Value.ShouldBe(7); + drv.GetHostStatuses().Single().State.ShouldBe(HostState.Running); + logger.Entries.Count(e => e.Level == LogLevel.Warning && e.Message.Contains("ProbeTagTypo")).ShouldBe(1); + await drv.DisposeAsync(); + } + + /// Positive control: a probe that genuinely cannot connect still stops the host and throttles cold creates. + [Fact] + public async Task An_unreachable_device_still_stops_the_host_and_throttles() + { + var (drv, _) = Build(new AbTransportException(AbStatus.Timeout, "Timeout")); + await drv.InitializeAsync("{}", CancellationToken.None); + await WaitUntilAsync(() => drv.GetHostStatuses().Single().State == HostState.Stopped); + + var r = await drv.ReadAsync(["Speed"], CancellationToken.None); + + r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadCommunicationError); // throttled by the open window + await drv.DisposeAsync(); + } +}