diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs
index 89f06793..f675234e 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs
@@ -1390,7 +1390,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
await WriteBitInDIntAsync(device, parsedPath, bit, w.Value, cancellationToken)
.ConfigureAwait(false));
if (results[i].StatusCode == AbCipStatusMapper.Good)
- _health = new DriverHealth(DriverState.Healthy, now, null);
+ RecordWriteAnswered(now);
continue;
}
@@ -1455,6 +1455,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
{
EvictRuntime(device, def.Name);
results[i] = new WriteResult(AbCipStatusMapper.MapStatus(status));
+ RecordWriteCommsFault($"transport status {status} writing {w.FullReference}");
_logger.LogWarning(
"AbCip write returned non-zero transport status {Status} for tag {Tag} on device {Device}; " +
"evicting cached runtime so next call re-creates it",
@@ -1463,7 +1464,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
else
{
results[i] = new WriteResult(AbCipStatusMapper.Good);
- _health = new DriverHealth(DriverState.Healthy, now, null);
+ RecordWriteAnswered(now);
}
}
catch (OperationCanceledException)
@@ -1481,30 +1482,29 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
"AbCip write not supported for tag {Tag} on device {Device}: {Reason} (logged once per tag)",
w.FullReference, def.DeviceHostAddress, nse.Message);
}
+ // H19: the three client-value conversion failures below never touched the wire — they say nothing about
+ // the device, so they write no health (a Degraded here latched until the next good read, and AbLegacy
+ // already maps the same three with no health write). Not a transport fault either: no evict.
catch (FormatException fe)
{
- // Value conversion error — not a transport fault; don't evict.
results[i] = new WriteResult(AbCipStatusMapper.BadTypeMismatch);
- _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, fe.Message);
- _logger.LogWarning(fe,
- "AbCip write value-conversion error for tag {Tag} on device {Device}",
- w.FullReference, def.DeviceHostAddress);
+ WarnWriteRefusedOnce(def.Name,
+ "AbCip write to {Tag} on device {Device}: the value cannot be converted to {DataType}: {Reason} (logged once per tag)",
+ w.FullReference, def.DeviceHostAddress, def.DataType, fe.Message);
}
catch (InvalidCastException ice)
{
results[i] = new WriteResult(AbCipStatusMapper.BadTypeMismatch);
- _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ice.Message);
- _logger.LogWarning(ice,
- "AbCip write type-cast error for tag {Tag} on device {Device}",
- w.FullReference, def.DeviceHostAddress);
+ WarnWriteRefusedOnce(def.Name,
+ "AbCip write to {Tag} on device {Device}: the value cannot be converted to {DataType}: {Reason} (logged once per tag)",
+ w.FullReference, def.DeviceHostAddress, def.DataType, ice.Message);
}
catch (OverflowException oe)
{
results[i] = new WriteResult(AbCipStatusMapper.BadOutOfRange);
- _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, oe.Message);
- _logger.LogWarning(oe,
- "AbCip write value out of range for tag {Tag} on device {Device}",
- w.FullReference, def.DeviceHostAddress);
+ WarnWriteRefusedOnce(def.Name,
+ "AbCip write to {Tag} on device {Device}: the value is out of range for {DataType}: {Reason} (logged once per tag)",
+ w.FullReference, def.DeviceHostAddress, def.DataType, oe.Message);
}
catch (AbTransportException ate) when (ate.Status == AbStatus.Unsupported)
{
@@ -1549,7 +1549,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
if (rmwParent is not null) EvictParentRuntime(device, rmwParent);
else EvictRuntime(device, def.Name);
results[i] = new WriteResult(AbCipStatusMapper.BadCommunicationError);
- _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message);
+ RecordWriteCommsFault(ex.Message);
_logger.LogWarning(ex,
"AbCip write transport exception for tag {Tag} on device {Device}",
w.FullReference, def.DeviceHostAddress);
@@ -1559,6 +1559,19 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
return results;
}
+ ///
+ /// A good write proves the device answered: advance LastSuccessfulRead but leave the STATE alone. Writing
+ /// Healthy here (H19) overwrote a Degraded the poll loop had just recorded for other tags.
+ ///
+ private void RecordWriteAnswered(DateTime now) =>
+ UpdateHealth(h => h with { LastSuccessfulRead = now });
+
+ /// A write-side comms fault: Degraded (never downgrading Faulted), LastSuccessfulRead preserved.
+ private void RecordWriteCommsFault(string error) =>
+ UpdateHealth(h => h.State == DriverState.Faulted
+ ? h
+ : new DriverHealth(DriverState.Degraded, h.LastSuccessfulRead, error));
+
///
/// Read-modify-write one bit within an integer parent (SINT / INT / DINT / LINT — Logix allows .n on
/// each). Creates / reuses a parallel parent runtime (distinct from the bit-selector handle) + serialises
diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipWriteHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipWriteHealthTests.cs
new file mode 100644
index 00000000..4821a43a
--- /dev/null
+++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipWriteHealthTests.cs
@@ -0,0 +1,113 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
+
+///
+/// Review 07 H19: write-side health. A client value that cannot be converted never touched the wire, so it must
+/// not latch Degraded; and a good write must not overwrite a Degraded the read path just recorded
+/// with Healthy (the batch read path is worst-wins for exactly this reason). A good write proves the device
+/// answered, so it advances LastSuccessfulRead without touching the state.
+///
+[Trait("Category", "Unit")]
+public sealed class AbCipWriteHealthTests
+{
+ private const string Device = "ab://10.0.0.5/1,0";
+
+ private sealed class ThrowingWriteTag(AbCipTagCreateParams p) : FakeAbCipTag(p)
+ {
+ public override Task WriteAsync(CancellationToken cancellationToken) =>
+ throw new InvalidOperationException("socket reset");
+ }
+
+ private static async Task<(AbCipDriver Driver, FakeAbCipTagFactory Factory)> BuildAsync(
+ Func? make = null)
+ {
+ var factory = new FakeAbCipTagFactory
+ {
+ Customise = make ?? (p => p.TagName == "Broken"
+ ? new FakeAbCipTag(p) { ThrowOnRead = true }
+ : new RealEncodingTag(p) { Value = 0 }),
+ };
+ var drv = new AbCipDriver(new AbCipDriverOptions
+ {
+ Devices = [new AbCipDeviceOptions(Device)],
+ RawTags = AbCipRawTags.From(
+ new AbCipTagDefinition("D", Device, "D", AbCipDataType.DInt),
+ new AbCipTagDefinition("I", Device, "I", AbCipDataType.Int),
+ new AbCipTagDefinition("Broken", Device, "Broken", AbCipDataType.DInt)),
+ Probe = new AbCipProbeOptions { Enabled = false },
+ }, "abcip-write-health", factory);
+ await drv.InitializeAsync("{}", CancellationToken.None);
+ return (drv, factory);
+ }
+
+ /// A fake whose encode converts like the real runtimes do (Convert.ToInt32 / ToInt16).
+ private sealed class RealEncodingTag(AbCipTagCreateParams p) : FakeAbCipTag(p)
+ {
+ public override void EncodeValue(AbCipDataType type, int? bitIndex, object? value) =>
+ Value = type switch
+ {
+ AbCipDataType.Int => Convert.ToInt16(value),
+ _ => Convert.ToInt32(value),
+ };
+ }
+
+ /// Unconvertible / out-of-range client values: refused per tag, health untouched, nothing sent.
+ [Fact]
+ public async Task A_client_conversion_error_writes_no_health()
+ {
+ var (drv, factory) = await BuildAsync();
+ var before = drv.GetHealth();
+
+ var r = await drv.WriteAsync(
+ [
+ new WriteRequest("D", "not a number"), // FormatException
+ new WriteRequest("D", new object()), // InvalidCastException
+ new WriteRequest("I", 1_000_000), // OverflowException
+ ], CancellationToken.None);
+
+ r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadTypeMismatch);
+ r[1].StatusCode.ShouldBe(AbCipStatusMapper.BadTypeMismatch);
+ r[2].StatusCode.ShouldBe(AbCipStatusMapper.BadOutOfRange);
+ drv.GetHealth().ShouldBe(before);
+ factory.Tags["D"].WriteCount.ShouldBe(0);
+ factory.Tags["I"].WriteCount.ShouldBe(0);
+ }
+
+ /// A good write keeps a Degraded the read path recorded, and advances LastSuccessfulRead.
+ [Fact]
+ public async Task A_good_write_does_not_overwrite_Degraded_with_Healthy()
+ {
+ var (drv, _) = await BuildAsync();
+ await drv.ReadAsync(["Broken"], CancellationToken.None);
+ var degraded = drv.GetHealth();
+ degraded.State.ShouldBe(DriverState.Degraded);
+ await Task.Delay(20);
+
+ var r = await drv.WriteAsync([new WriteRequest("D", 5)], CancellationToken.None);
+
+ r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good);
+ var after = drv.GetHealth();
+ after.State.ShouldBe(DriverState.Degraded);
+ after.LastError.ShouldBe(degraded.LastError);
+ after.LastSuccessfulRead!.Value.ShouldBeGreaterThan(degraded.LastSuccessfulRead!.Value); // the device answered
+ }
+
+ /// Positive control: a genuine write transport fault still degrades and preserves LastSuccessfulRead.
+ [Fact]
+ public async Task A_write_transport_fault_still_degrades()
+ {
+ var (drv, factory) = await BuildAsync(p => new ThrowingWriteTag(p));
+ var before = drv.GetHealth();
+
+ var r = await drv.WriteAsync([new WriteRequest("D", 5)], CancellationToken.None);
+
+ r[0].StatusCode.ShouldBe(AbCipStatusMapper.BadCommunicationError);
+ var after = drv.GetHealth();
+ after.State.ShouldBe(DriverState.Degraded);
+ after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead);
+ factory.Tags["D"].Disposed.ShouldBeTrue();
+ }
+}