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

fix(abcip): a refused probe tag proves the device reachable (no blackout)

Browse Source 
H10: the probe loop recorded EVERY exception - including the controller
answering NotFound for a typo'd or download-deleted Probe.ProbeTagPath - as a
Forward-Open failure. At a 5 s tick against a 30 s backoff cap ShouldAttempt
was never true again, so every cold or evicted data tag on a reachable device
read BadCommunicationError indefinitely and the host showed Stopped.

The probe now decides the backoff verdict and the host state separately:
- a device refusal (thrown or as a status word) is backoff success + host
  Running + one Warning per device naming the probe tag; an initialised probe
  runtime is kept so its lease is not re-opened every tick;
- a malformed probe tag name (AbCipTagConfigurationException) never reached
  the device: no backoff signal, no host transition, one Warning;
- a genuine transport failure is unchanged (backoff failure, host Stopped).

Refs #737
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 14:17:03 -04:00
parent f087f01d2b
commit 22d4680e7b
2 changed files with 145 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs
+59 -11
View File
@@ -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 { }
}
/// <summary>Logs, once per device state, that the controller answered but refused the configured probe tag.</summary>
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,
/// <summary>Gets or sets whether the probe has been initialized.</summary>
public bool ProbeInitialized { get; set; }
/// <summary>1 once the probe-tag refusal Warning has been logged for this device state.</summary>
public int ProbeRefusalWarned;
/// <summary>
/// The fire-and-forget probe loop's <see cref="Task"/>. Stored so
/// <see cref="AbCipDriver.ShutdownAsync"/> can await the loop's clean exit after
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipProbeRefusalTests.cs
+86
View File
@@ -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<ZB.MOM.WW.OtOpcUa.Driver.AbCip.AbCipDriver>;
namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
/// <summary>
/// Review 07 H10: a typo'd (or download-deleted) <c>Probe.ProbeTagPath</c> 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
/// <c>BadCommunicationError</c> 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.
/// </summary>
[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<bool> condition)
{
var deadline = DateTime.UtcNow.AddSeconds(5);
while (!condition())
{
if (DateTime.UtcNow > deadline) throw new TimeoutException("condition never met");
await Task.Delay(5);
}
}
/// <summary>A NotFound probe tag leaves the device reachable: data tags still connect, host is Running, warned once.</summary>
[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();
}
/// <summary>Positive control: a probe that genuinely cannot connect still stops the host and throttles cold creates.</summary>
[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();
}
}