dohertj2/lmxopcua
1
0
Fork 0
Code Issues 43 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(abcip): write-side health neither latches nor overwrites

Browse Source 
H19: Format/InvalidCast/Overflow on a client value never touched the wire but
latched Degraded; a good write set Healthy and so undid a Degraded the poll
loop had just recorded for other tags (the batch read path is worst-wins for
exactly this reason).

- The three conversion failures map to BadTypeMismatch/BadOutOfRange with a
  once-per-tag Warning and no health write (AbLegacy's rule).
- A good write (scalar or bit RMW) advances LastSuccessfulRead only - the
  device answered - and leaves the state alone.
- A write comms fault (thrown, or a non-refusal status word, which used to
  write no health at all) is Degraded with LastSuccessfulRead preserved and
  never downgrades Faulted.

Refs #737
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 14:24:06 -04:00
parent cb16ebc75e
commit 895cc28886
2 changed files with 142 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs
+29 -16
View File
@@ -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;
}
/// <summary>
/// A good write proves the device answered: advance <c>LastSuccessfulRead</c> but leave the STATE alone. Writing
/// <c>Healthy</c> here (H19) overwrote a <c>Degraded</c> the poll loop had just recorded for other tags.
/// </summary>
private void RecordWriteAnswered(DateTime now) =>
UpdateHealth(h => h with { LastSuccessfulRead = now });
/// <summary>A write-side comms fault: <c>Degraded</c> (never downgrading <c>Faulted</c>), <c>LastSuccessfulRead</c> preserved.</summary>
private void RecordWriteCommsFault(string error) =>
UpdateHealth(h => h.State == DriverState.Faulted
? h
: new DriverHealth(DriverState.Degraded, h.LastSuccessfulRead, error));
/// <summary>
/// Read-modify-write one bit within an integer parent (SINT / INT / DINT / LINT — Logix allows <c>.n</c> on
/// each). Creates / reuses a parallel parent runtime (distinct from the bit-selector handle) + serialises
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipWriteHealthTests.cs
+113
View File
@@ -0,0 +1,113 @@
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
/// <summary>
/// Review 07 H19: write-side health. A client value that cannot be converted never touched the wire, so it must
/// not latch <c>Degraded</c>; and a good write must not overwrite a <c>Degraded</c> the read path just recorded
/// with <c>Healthy</c> (the batch read path is worst-wins for exactly this reason). A good write proves the device
/// answered, so it advances <c>LastSuccessfulRead</c> without touching the state.
/// </summary>
[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<AbCipTagCreateParams, FakeAbCipTag>? 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);
}
/// <summary>A fake whose encode converts like the real runtimes do (Convert.ToInt32 / ToInt16).</summary>
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),
};
}
/// <summary>Unconvertible / out-of-range client values: refused per tag, health untouched, nothing sent.</summary>
[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);
}
/// <summary>A good write keeps a Degraded the read path recorded, and advances LastSuccessfulRead.</summary>
[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
}
/// <summary>Positive control: a genuine write transport fault still degrades and preserves LastSuccessfulRead.</summary>
[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();
}
}