From 419ecb3ce94c2ed7f0f543d6e654dac7d9ba88f5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:08:44 -0400 Subject: [PATCH] fix(mtconnect): a delivered /sample chunk clears an unanswered-write degradation (review r2 N-5) Coordinator ruling: a delivered chunk proves the Agent is alive (the analogue of OpcUaClient's good keep-alive), so TryMarkStreamDelivered - generation-checked under _subscriptionLock - also clears _writeDegraded. The degradation no longer sticks forever on a subscription-only driver. Test: unanswered write -> Degraded, stays Degraded with nothing else happening (positive control), the next chunk -> Healthy; red with the clear removed. Round-2 doc text corrected. Refs #743 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- docs/drivers/MTConnect.md | 10 +++++----- .../MTConnectDriver.cs | 17 ++++++++++------ .../MTConnectBatchedWriteTests.cs | 20 +++++++++---------- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/docs/drivers/MTConnect.md b/docs/drivers/MTConnect.md index 9999b3e4..42cf80ca 100644 --- a/docs/drivers/MTConnect.md +++ b/docs/drivers/MTConnect.md @@ -437,11 +437,11 @@ refused — advances `LastSuccessfulRead`, with the state following the read / ` (a write cannot clear either of those paths' degradation). A write that got **no** answer (transport failure, deadline) sets **Degraded** with the stamp preserved and a type-only `LastError` (review 2026-10-21 I-3, coordinator ruling). One success writer is shared: that -degradation is **latched until the next answered write or the next answered `/current` read**. -Neither a `/sample` chunk nor a read served from the live pump index clears it (both are stream -evidence, not an answer to a request), so on a **subscription-only** driver one timed-out write -keeps the driver Degraded until the next write — the comms-loss watchdog does not trip on it, -because the stream keeps advancing `LastSuccessfulRead`. +degradation is cleared by the next answered **write**, the next answered **`/current` read**, or +the next **`/sample` chunk** the current session's pump delivers (review r2 N-5: a delivered +chunk proves the Agent is alive — the analogue of OpcUaClient's good keep-alive — so it cannot +stick on a subscription-only driver). A read served from the live pump index does not clear it +by itself; the chunks behind that index do. **One request per device (review 2026-10-21 T-6).** The slots that survive the refusals above go out as one `POST {agent}/{device}?id1=v1&id2=v2…` per device (the Agent's PUT surface takes diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index 816b877b..74e84efd 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -371,9 +371,10 @@ public sealed partial class MTConnectDriver /// /// Whether the last Agent write got no reply (review I-3 / coordinator ruling: an unanswered write degrades). - /// Latched until the next answered /current read or answered write ( / - /// ). Neither a /sample chunk nor a live-index read clears it, so on - /// a subscription-only driver one timed-out write keeps the driver Degraded until the next write (r1 N-5). + /// Cleared by the next answered /current read, answered write ( / + /// ) or delivered /sample chunk of the current session + /// (, r2 N-5) — so it cannot stick on a subscription-only driver. A read served + /// from the live index does not clear it directly; the chunks behind that index do. /// private volatile bool _writeDegraded; @@ -856,9 +857,8 @@ public sealed partial class MTConnectDriver // The read path is answering (from a stream that is delivering), so a degradation a failed /current left // behind is no longer true of it. The stamp stays the pump's — this read heard nothing from the Agent itself. - // Clears only the READ degradation (r1 N-5): an index read is the stream's evidence, not an Agent answer to a - // request — the same evidence a /sample chunk carries, which by the I-3 ruling does not clear an unanswered - // write either. + // Clears only the READ degradation: an index read is derived from the stream's evidence — the chunks that fed the + // index are what clear an unanswered write (TryMarkStreamDelivered, r2 N-5). if (_readDegraded) { _readDegraded = false; @@ -2751,6 +2751,11 @@ public sealed partial class MTConnectDriver _streamDegraded = false; _pumpLive = true; + + // r2 N-5 (coordinator): a delivered chunk proves the Agent is alive — the MTConnect analogue of OpcUaClient's + // good keep-alive clearing a read/write Degraded — so it also clears an unanswered-write degradation, which + // otherwise stuck forever on a subscription-only driver. Generation-checked with the rest (above). + _writeDegraded = false; PublishHealthy(DateTime.UtcNow); return true; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBatchedWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBatchedWriteTests.cs index c230abbb..8858fbe3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBatchedWriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBatchedWriteTests.cs @@ -167,11 +167,12 @@ public sealed class MTConnectBatchedWriteTests } /// - /// r1 N-5: stream evidence does not clear an unanswered write — neither a delivered /sample chunk nor a read - /// served from the live pump index. The next answered write does (the positive control). + /// r2 N-5 (coordinator ruling): on a subscription-only driver an unanswered write degrades, and the next delivered + /// /sample chunk restores Healthy (a delivered chunk proves the Agent is alive). Positive control: with no chunk + /// it stays Degraded. /// [Fact] - public async Task Stream_evidence_does_not_clear_an_unanswered_write_but_an_answered_write_does() + public async Task A_delivered_chunk_clears_an_unanswered_write_and_without_one_it_stays_degraded() { var (driver, client) = await WritableAsync(); try @@ -181,20 +182,19 @@ public sealed class MTConnectBatchedWriteTests client.WriteFailure = new HttpRequestException("connection refused"); await driver.WriteAsync([new WriteRequest(PosRaw, 9.5)], Ct); driver.GetHealth().State.ShouldBe(DriverState.Degraded); + + // Positive control: nothing else happens — it stays Degraded (no read, no write, no chunk). var currentCalls = client.CurrentCallCount; + driver.GetHealth().State.ShouldBe(DriverState.Degraded); + client.CurrentCallCount.ShouldBe(currentCalls); await client.PumpAsync(MTConnectPumpGenerationChunks.Pos(113, 118, "2.5")).WaitAsync(TimeSpan.FromSeconds(30), Ct); - (await driver.ReadAsync([PosRaw], Ct)).Single().StatusCode.ShouldBe(Good); - client.CurrentCallCount.ShouldBe(currentCalls, "the read was served from the live index"); - driver.GetHealth().State.ShouldBe(DriverState.Degraded, "stream evidence is not an answer to the write"); - - client.WriteFailure = null; - await driver.WriteAsync([new WriteRequest(PosRaw, 3.5)], Ct); - driver.GetHealth().State.ShouldBe(DriverState.Healthy); + driver.GetHealth().State.ShouldBe(DriverState.Healthy, "a delivered chunk proves the Agent is alive"); } finally { + client.WriteFailure = null; await driver.ShutdownAsync(CancellationToken.None); } }