From 6ffd881d425435d1be9450c703d877f530fe1bba Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Thu, 22 Oct 2026 15:37:41 -0400
Subject: [PATCH] fix(modbus): a failed read or write drops the
 WriteOnChangeOnly suppression for the tag

After a comms fault the device value is unknown, so re-asserting the same
setpoint must reach the wire - the write-side twin of the value-only
recovery dedup trap.

Refs #741
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
---
 .../ModbusDriver.cs                           | 17 ++++++++++++
 .../ModbusHealthContractTests.cs              | 26 +++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs
index 2bc8e8ef..ad9b7295 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs
@@ -746,6 +746,7 @@ public sealed class ModbusDriver
                 // Fail fast: this unit (or the whole endpoint) already failed at the transport this batch;
                 // paying the same timeout once per remaining tag would stall the poll N-fold.
                 results[i] = new DataValueSnapshot(null, StatusBadCommunicationError, null, now);
+                InvalidateWriteCache(fullReferences[i]);
                 continue;
             }
             try
@@ -767,6 +768,7 @@ public sealed class ModbusDriver
             catch (Exception ex)
             {
                 results[i] = new DataValueSnapshot(null, ClassifyFailure(ex, unit, fullReferences[i], batch, isWrite: false), null, now);
+                InvalidateWriteCache(fullReferences[i]);
             }
         }
 
@@ -1253,6 +1255,7 @@ public sealed class ModbusDriver
                     foreach (var (idx, _) in block.Members)
                     {
                         results[idx] = new DataValueSnapshot(null, StatusBadCommunicationError, null, timestamp);
+                        InvalidateWriteCache(fullReferences[idx]);
                         handled.Add(idx);
                     }
                     continue;
@@ -1285,6 +1288,7 @@ public sealed class ModbusDriver
                     foreach (var (idx, _) in block.Members)
                     {
                         results[idx] = new DataValueSnapshot(null, status, null, timestamp);
+                        InvalidateWriteCache(fullReferences[idx]);
                         handled.Add(idx);
                     }
                     continue;
@@ -1341,6 +1345,17 @@ public sealed class ModbusDriver
             ? ReadBitBlockAsync(transport, unit, ReadFunctionCode(region), start, qty, ct)
             : ReadRegisterBlockAsync(transport, unit, ReadFunctionCode(region), start, qty, ct);
 
+    /// <summary>
+    ///     A failed read leaves the device's value UNKNOWN (it may have rebooted holding anything), so the
+    ///     WriteOnChangeOnly suppression for that tag is dropped — otherwise re-asserting the same setpoint
+    ///     after an outage would be swallowed, the write-side twin of the value-only recovery dedup trap.
+    /// </summary>
+    private void InvalidateWriteCache(string fullRef)
+    {
+        if (!_options.WriteOnChangeOnly) return;
+        lock (_lastWrittenLock) _lastWrittenByRef.Remove(fullRef);
+    }
+
     private void InvalidateWriteCacheIfDiverged(string fullRef, object value)
     {
         if (!_options.WriteOnChangeOnly) return;
@@ -1483,6 +1498,8 @@ public sealed class ModbusDriver
             catch (Exception ex)
             {
                 results[i] = new WriteResult(MapWriteFailure(ex, unit, w.FullReference, batch, cancellationToken));
+                // A failed write may or may not have landed — the device value is unknown now.
+                InvalidateWriteCache(w.FullReference);
             }
         }
 
diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusHealthContractTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusHealthContractTests.cs
index 50282ec3..0e591189 100644
--- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusHealthContractTests.cs
+++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusHealthContractTests.cs
@@ -215,6 +215,32 @@ public sealed class ModbusHealthContractTests
         drv.GetHealth().State.ShouldBe(DriverState.Healthy);
     }
 
+    [Fact]
+    public async Task A_failed_read_invalidates_the_WriteOnChangeOnly_cache()
+    {
+        // Value-only suppression is the write-side twin of the #527 dedup trap: after a comms fault the
+        // device's register is unknown (it may have rebooted), so re-asserting the same setpoint must reach it.
+        var opts = new ModbusDriverOptions
+        {
+            Host = "f", RawTags = ModbusRawTags.Entries([Hr("A", 10)]), WriteOnChangeOnly = true,
+            Probe = new ModbusProbeOptions { Enabled = false },
+        };
+        var (drv, t) = await StartAsync(opts);
+        await using var _ = drv;
+        int Fc06() => t.Sent.Count(s => s.Pdu[0] == 0x06);
+
+        await drv.WriteAsync([new WriteRequest("A", (short)5)], TestContext.Current.CancellationToken);
+        await drv.WriteAsync([new WriteRequest("A", (short)5)], TestContext.Current.CancellationToken);
+        Fc06().ShouldBe(1, "positive control: an identical re-write is suppressed while nothing failed");
+
+        t.Handler = (_, pdu, _) => pdu[0] == 0x03 ? Task.FromException<byte[]>(new IOException("dropped")) : null;
+        await drv.ReadAsync(["A"], TestContext.Current.CancellationToken);
+        t.Handler = null;
+
+        await drv.WriteAsync([new WriteRequest("A", (short)5)], TestContext.Current.CancellationToken);
+        Fc06().ShouldBe(2, "after a failed read the device value is unknown; the write must go to the wire");
+    }
+
     // ---------------- lifecycle generation ----------------
 
     [Fact]