diff --git a/docs/drivers/OpcUaClient.md b/docs/drivers/OpcUaClient.md index aa9d4e3f..d189f57c 100644 --- a/docs/drivers/OpcUaClient.md +++ b/docs/drivers/OpcUaClient.md @@ -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: "`, 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 diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index 4e757d6d..a6497254 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -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; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientWriteHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientWriteHealthTests.cs new file mode 100644 index 00000000..10e4637e --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientWriteHealthTests.cs @@ -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; + +/// +/// 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 +/// ; 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). +/// +[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 Tags = + [new(SpeedRef, JsonSerializer.Serialize(new { nodeId = "ns=0;s=Speed" }), WriteIdempotent: true)]; + + private static (OpcUaClientDriver Driver, Mock Session) NewConnectedDriver(DriverHealth health) + { + var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions { RawTags = Tags }, "opcua-write-health"); + var session = new Mock(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)]; + + /// A refused write (BadNotWritable) is a reply: Healthy again, stamp advanced. + [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(), It.IsAny(), It.IsAny())) + .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(); + } + + /// A write with no reply (transport failure) degrades health and preserves the stamp. + [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(), It.IsAny(), It.IsAny())) + .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"); + } + + /// + /// 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.) + /// + [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(), It.IsAny(), It.IsAny())) + .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); + } + + /// A good keep-alive is a reply: it clears a Degraded a failed read left behind. + [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(); + } +}