fix(opcuaclient): writes feed health; a good keep-alive clears a stale Degraded

Health contract: a write response (per-node refusals included) is the server
answering, so it restores Healthy and advances LastSuccessfulRead; a write with no
reply (transport failure / timeout) writes Degraded with the stamp preserved; a
caller-cancelled write touches nothing. A good keep-alive with no outage or dead
subscription latched now clears a Degraded left by a failed read/write instead of
only moving the stamp (a subscription-only driver otherwise stayed Degraded until
its next re-initialise). All writes are generation-stamped.

Refs #743

Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 15:26:43 -04:00
parent 66d100a9e9
commit cba3b2e960
3 changed files with 142 additions and 3 deletions
+5
View File
@@ -91,6 +91,11 @@ The session keep-alive (`KeepAliveInterval`, default 5 s) is the liveness signal
- **Good keep-alive / any data or event notification** advances `DriverHealth.LastSuccessfulRead` — a
subscription-only driver never calls `ReadAsync`, so without this its read stamp froze at connect time.
A good keep-alive with no outage latched also **clears a `Degraded` left by a failed read/write** (it used to move
only the stamp, so a subscription-only driver stayed `Degraded` until its next re-initialise).
- **Read / write / ack responses are replies.** Any service response — per-node refusals such as `BadNotWritable`
included — restores `Healthy` and advances the stamp; only a call that got **no** response (transport failure,
timeout) writes `Degraded` with the stamp preserved. A write the caller cancelled touches no health.
- **Bad keep-alive** ⇒ health `Degraded` (`LastError = "keep-alive: <status>"`, read stamp preserved), host status
`Stopped`, and — **once per outage**, not per keep-alive — every subscribed ref is published `BadNoCommunication`
with its last value retained and **source + server timestamp = the detection time** (live-gate F1: stamped with the
@@ -1437,6 +1437,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
_ = RequireSession();
var results = new WriteResult[writes.Count];
var generation = Volatile.Read(ref _lifecycleGeneration);
var session = Session;
if (session is null)
{
@@ -1483,8 +1484,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
// the severity bits set is a Bad.
results[r] = new WriteResult(codes[w].Code);
}
// A response — refusals included (BadNotWritable, BadTypeMismatch, ...) — is the server answering.
RecordDeviceAnswered(generation, Clock.GetUtcNow().UtcDateTime);
}
catch (OperationCanceledException)
catch (OperationCanceledException ex)
{
// Timeout / cancellation after the wire request may have been dispatched.
// Writes are non-idempotent — BadTimeout ("outcome unknown, do not blindly
@@ -1494,12 +1497,15 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
const uint StatusBadTimeout = 0x800A0000u;
for (var w = 0; w < indexMap.Count; w++)
results[indexMap[w]] = new WriteResult(StatusBadTimeout);
// No reply arrived. Only a timeout is a comms fault — a caller that cancelled says nothing about upstream.
if (!cancellationToken.IsCancellationRequested) RecordCommsFault(generation, $"write: {ex.Message}");
}
catch (Exception)
catch (Exception ex)
{
// Pre-wire transport failure — the write definitely did not reach the server.
for (var w = 0; w < indexMap.Count; w++)
results[indexMap[w]] = new WriteResult(StatusBadCommunicationError);
RecordCommsFault(generation, $"write: {ex.Message}");
}
return results;
}
@@ -3125,7 +3131,15 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
return;
}
UpdateHealth(generation, h => h.State is DriverState.Healthy or DriverState.Degraded ? h with { LastSuccessfulRead = now } : null);
// A good keep-alive is the server answering. With no outage and no dead subscription latched, a Degraded
// here can only be a failed read/write the server has since contradicted, so it clears too — otherwise a
// subscription-only driver (which never reads again) stayed Degraded until its next re-initialise.
UpdateHealth(generation, h => h.State switch
{
DriverState.Healthy => h with { LastSuccessfulRead = now },
DriverState.Degraded => new DriverHealth(DriverState.Healthy, now, null),
_ => null,
});
return;
}
@@ -0,0 +1,120 @@
using System.Text.Json;
using Moq;
using Opc.Ua;
using Opc.Ua.Client;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests;
/// <summary>
/// The health contract for the write path and the steady keep-alive: a write RESPONSE — whatever per-node
/// statuses it carries, a refusal included — is the server answering, so it restores Healthy and advances
/// <see cref="DriverHealth.LastSuccessfulRead"/>; a write that got NO response degrades health with the stamp
/// preserved; a caller-cancelled write is not a comms fault. A good keep-alive clears a Degraded left by a failed
/// read/write (it used to only move the stamp, leaving a subscription-only driver Degraded forever).
/// </summary>
[Trait("Category", "Unit")]
public sealed class OpcUaClientWriteHealthTests
{
private const string SpeedRef = "Plant/dev/Speed";
private static readonly DateTime Stale = new(2020, 1, 1, 0, 0, 0, DateTimeKind.Utc);
private static readonly IReadOnlyList<RawTagEntry> Tags =
[new(SpeedRef, JsonSerializer.Serialize(new { nodeId = "ns=0;s=Speed" }), WriteIdempotent: true)];
private static (OpcUaClientDriver Driver, Mock<ISession> Session) NewConnectedDriver(DriverHealth health)
{
var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions { RawTags = Tags }, "opcua-write-health");
var session = new Mock<ISession>(MockBehavior.Loose);
session.SetupGet(s => s.MessageContext).Returns(new ServiceMessageContext(SdkTelemetry.Context));
session.SetupGet(s => s.NamespaceUris).Returns(new NamespaceTable());
drv.SetSessionForTest(session.Object);
drv.SetNamespaceMapForTest(NamespaceMap.FromTable(new NamespaceTable(), Tags));
drv.SetHealthForTest(health);
return (drv, session);
}
private static ZB.MOM.WW.OtOpcUa.Core.Abstractions.WriteRequest[] Write() => [new(SpeedRef, 7)];
/// <summary>A refused write (BadNotWritable) is a reply: Healthy again, stamp advanced.</summary>
[Fact]
public async Task A_refused_write_is_a_reply_and_restores_healthy()
{
var (drv, session) = NewConnectedDriver(new DriverHealth(DriverState.Degraded, Stale, "read failed"));
using var _ = drv;
session.Setup(s => s.WriteAsync(It.IsAny<RequestHeader>(), It.IsAny<WriteValueCollection>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(new WriteResponse
{
ResponseHeader = new ResponseHeader(),
Results = [StatusCodes.BadNotWritable],
DiagnosticInfos = [],
});
var results = await drv.WriteAsync(Write(), TestContext.Current.CancellationToken);
results.Single().StatusCode.ShouldBe(StatusCodes.BadNotWritable, "the upstream refusal still passes through verbatim");
var h = drv.GetHealth();
h.State.ShouldBe(DriverState.Healthy);
h.LastSuccessfulRead.ShouldNotBeNull().ShouldBeGreaterThan(Stale);
h.LastError.ShouldBeNull();
}
/// <summary>A write with no reply (transport failure) degrades health and preserves the stamp.</summary>
[Fact]
public async Task A_write_with_no_reply_degrades_and_preserves_the_stamp()
{
var (drv, session) = NewConnectedDriver(new DriverHealth(DriverState.Healthy, Stale, null));
using var _ = drv;
session.Setup(s => s.WriteAsync(It.IsAny<RequestHeader>(), It.IsAny<WriteValueCollection>(), It.IsAny<CancellationToken>()))
.ThrowsAsync(new ServiceResultException(StatusCodes.BadConnectionClosed));
var results = await drv.WriteAsync(Write(), TestContext.Current.CancellationToken);
results.Single().StatusCode.ShouldBe(0x80050000u); // BadCommunicationError
var h = drv.GetHealth();
h.State.ShouldBe(DriverState.Degraded);
h.LastSuccessfulRead.ShouldBe(Stale, "the comms-loss watchdog measures silence from this stamp");
}
/// <summary>
/// A write the CALLER cancelled says nothing about the upstream: health is left alone. (The positive control is
/// the test above — the same throw-path does write health when the failure is the transport's.)
/// </summary>
[Fact]
public async Task A_caller_cancelled_write_leaves_health_alone()
{
var healthy = new DriverHealth(DriverState.Healthy, Stale, null);
var (drv, session) = NewConnectedDriver(healthy);
using var _ = drv;
using var cts = new CancellationTokenSource();
session.Setup(s => s.WriteAsync(It.IsAny<RequestHeader>(), It.IsAny<WriteValueCollection>(), It.IsAny<CancellationToken>()))
.Returns(async (RequestHeader _, WriteValueCollection _, CancellationToken ct) =>
{
cts.Cancel();
await Task.Delay(Timeout.Infinite, ct);
return new WriteResponse();
});
var results = await drv.WriteAsync(Write(), cts.Token);
results.Single().StatusCode.ShouldBe(0x800A0000u); // BadTimeout — outcome unknown
drv.GetHealth().ShouldBe(healthy);
}
/// <summary>A good keep-alive is a reply: it clears a Degraded a failed read left behind.</summary>
[Fact]
public void A_good_keep_alive_clears_a_degraded_left_by_a_failed_read()
{
var (drv, _) = NewConnectedDriver(new DriverHealth(DriverState.Degraded, Stale, "read: BadTimeout"));
using var __ = drv;
drv.RaiseKeepAliveForTest(StatusCodes.Good);
var h = drv.GetHealth();
h.State.ShouldBe(DriverState.Healthy);
h.LastSuccessfulRead.ShouldNotBeNull().ShouldBeGreaterThan(Stale);
h.LastError.ShouldBeNull();
}
}