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
This commit is contained in:
Joseph Doherty
2026-10-22 15:37:41 -04:00
parent 4a3d7a9361
commit 6ffd881d42
2 changed files with 43 additions and 0 deletions
@@ -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);
}
}
@@ -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]