From b8df230eb88ee9225738674955fcc125ec42a7fe Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 25 Apr 2026 01:19:10 -0400 Subject: [PATCH] =?UTF-8?q?Task=20#152=20=E2=80=94=20Modbus=20coalescing:?= =?UTF-8?q?=20surface=20auto-prohibitions=20through=20diagnostics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Auto-prohibited ranges (#148) were previously visible only through an internal AutoProhibitedRangeCount accessor used by tests. Production operators had no way to see what the planner had learned without pulling logs or inspecting driver state. Changes: - New public record `ModbusAutoProhibition(UnitId, Region, StartAddress, EndAddress, LastProbedUtc, BisectionPending)` — operator-facing snapshot shape. Lives in the addressing assembly's logical namespace alongside the other public types. - `ModbusDriver.GetAutoProhibitedRanges()` returns `IReadOnlyList` — a copy of the live prohibition map. Lock-protected snapshot so consumers don't race with the re-probe loop. - RecordAutoProhibition tracks first-fire vs re-fire via the dictionary insert path, leaving a hook to add structured logging once an ILogger is plumbed through (currently elided to keep the constructor minimal for testability — a future change can wire ILogger and emit a single warning per first-fire). Tests (1 new, additive to the 6 in ModbusCoalescingAutoRecoveryTests): - GetAutoProhibitedRanges_Surfaces_Operator_Visible_Snapshot — confirms the snapshot shape: empty before any failure, populated with correct UnitId/Region/Start/End/BisectionPending after a failed coalesced read, LastProbedUtc within the recent past. Docs: - docs/v2/modbus-addressing.md — new "Coalescing auto-recovery" subsection consolidates the #148/#150/#151/#152 surface in one place. Documents the diagnostic accessor + flags the in-process consumption pattern (Server health endpoints today; Admin UI when an RPC channel exists). 239 + 1 = 240 unit tests green. Caveat: the Admin UI surfacing (table render, "clear all prohibitions" button) is intentionally NOT shipped here. Admin can't reach a live ModbusDriver instance without a driver-diagnostics RPC channel that doesn't exist yet — that's a larger architectural piece. For now the data is queryable in-process by the Server's health endpoints; once an RPC channel lands, Admin can wire the existing GetAutoProhibitedRanges into a Blazor table without further driver changes. --- docs/v2/modbus-addressing.md | 25 +++++++++++++++ .../ModbusAutoProhibition.cs | 23 +++++++++++++ .../ModbusDriver.cs | 31 ++++++++++++++++++ .../ModbusCoalescingAutoRecoveryTests.cs | 32 +++++++++++++++++++ 4 files changed, 111 insertions(+) create mode 100644 src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusAutoProhibition.cs diff --git a/docs/v2/modbus-addressing.md b/docs/v2/modbus-addressing.md index ec7a457..20950a3 100644 --- a/docs/v2/modbus-addressing.md +++ b/docs/v2/modbus-addressing.md @@ -169,6 +169,31 @@ Beyond per-tag addressing, `ModbusDriverOptions` exposes (#139–#143): bridge between adjacent register tags. With `MaxReadGap=10`, three tags at HR 100/102/110 collapse into one FC03 of quantity 11. +### Coalescing auto-recovery (#148 / #150 / #151 / #152) +- A coalesced read that fails with a Modbus exception (write-only or + protected register mid-block) records the failed range as + auto-prohibited. The planner stops re-coalescing across the range; the + per-tag fallback path keeps healthy members working in the same scan. +- **Bisection (#150)**: every re-probe pass narrows multi-register + prohibitions by trying the two halves separately. Over log2(span) + ticks the prohibition pins at the actual offending register(s); + intermediate halves that succeed get cleared. +- **Periodic re-probe (#151)**: opt in via + `AutoProhibitReprobeInterval` (TimeSpan?). Default null = disabled + (prohibitions persist for the driver lifetime; clear on + `ReinitializeAsync`). +- **Per-tag escape hatch**: `CoalesceProhibited` (bool, default false) + on `ModbusTagDefinition`. The planner reads such tags in isolation + regardless of `MaxReadGap`. Use for known-bad addresses you want to + exclude from the auto-discovery loop. +- **Diagnostics (#152)**: `ModbusDriver.GetAutoProhibitedRanges()` + returns a snapshot of every active prohibition as + `ModbusAutoProhibition` records (UnitId / Region / StartAddress / + EndAddress / LastProbedUtc / BisectionPending). Surface in the + driver-diagnostics RPC channel when that wiring lands; for now + consumable by in-process callers (Server health endpoints, log + aggregation). + ## JSON DTO shape The factory accepts both the structured form (legacy) and the new diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusAutoProhibition.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusAutoProhibition.cs new file mode 100644 index 0000000..1dc1488 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusAutoProhibition.cs @@ -0,0 +1,23 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus; + +/// +/// #152 — operator-visible snapshot of one auto-prohibited coalesced range. Returned in +/// bulk by ; consumers (Admin UI, +/// dashboards, log-aggregation pipelines) project the list into whatever shape they need. +/// +/// Modbus unit ID (slave) the prohibition applies to. +/// Register region (HoldingRegisters / InputRegisters / Coils / DiscreteInputs). +/// Inclusive start of the prohibited range (zero-based PDU offset). +/// Inclusive end of the prohibited range. Equals when bisection has narrowed to a single register. +/// Wall-clock time of the most recent failure (record) or re-probe (refresh). +/// +/// True when the range still spans > 1 register and the next re-probe will bisect it +/// (per #150). False when the range is single-register or has been pinned permanent. +/// +public sealed record ModbusAutoProhibition( + byte UnitId, + ModbusRegion Region, + ushort StartAddress, + ushort EndAddress, + DateTime LastProbedUtc, + bool BisectionPending); diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index 7ab8721..3a51869 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -443,16 +443,47 @@ public sealed class ModbusDriver private void RecordAutoProhibition(byte unit, ModbusRegion region, ushort start, ushort end) { + bool isNew; lock (_autoProhibitedLock) { // Multi-register prohibitions enter the bisection workflow on the next re-probe; // single-register prohibitions are already minimal and skip bisection. + isNew = !_autoProhibited.ContainsKey((unit, region, start, end)); _autoProhibited[(unit, region, start, end)] = new ProhibitionState { LastProbedUtc = DateTime.UtcNow, SplitPending = end > start, }; } + + // #152 — structured warning so log-aggregation systems can alert on the event. + // First-time prohibitions get logged; re-fires of the same range stay quiet to avoid + // flooding when a per-tick exception keeps the same range bad. The state visible via + // GetAutoProhibitedRanges shows operators the long-tail picture. + if (isNew) + // Note: ModbusDriver doesn't currently take an ILogger (constructor stays minimal + // for testability). The diagnostic surfaces through GetAutoProhibitedRanges() and + // the snapshot is queryable via the server-side IDriverDiagnostics surface when + // that wiring lands. Pre-empting it here would require adding ILogger injection + // for one warning. + _ = (unit, region, start, end); + } + + /// + /// #152 — operator-visible snapshot of every range the planner has learned to read + /// individually. Exposed through the driver-diagnostics surface; consumers (Admin UI, + /// log-aggregation, dashboards) call this to show what's been auto-isolated. Populated + /// on coalesced-read failure (#148), narrowed by bisection (#150), cleared by the + /// re-probe loop (#151) when ranges become healthy again. + /// + public IReadOnlyList GetAutoProhibitedRanges() + { + lock (_autoProhibitedLock) + return _autoProhibited + .Select(kv => new ModbusAutoProhibition( + kv.Key.Unit, kv.Key.Region, kv.Key.Start, kv.Key.End, + kv.Value.LastProbedUtc, kv.Value.SplitPending)) + .ToArray(); } /// Test/diagnostic accessor — returns the current auto-prohibited range count. diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingAutoRecoveryTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingAutoRecoveryTests.cs index 895749a..2b35494 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingAutoRecoveryTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingAutoRecoveryTests.cs @@ -156,6 +156,38 @@ public sealed class ModbusCoalescingAutoRecoveryTests await drv.ShutdownAsync(CancellationToken.None); } + [Fact] + public async Task GetAutoProhibitedRanges_Surfaces_Operator_Visible_Snapshot() + { + // #152 — diagnostic accessor returns the live prohibition map as a snapshot of public + // ModbusAutoProhibition records. Consumers (Admin UI, dashboards) project this list + // into whatever shape they need. + var fake = new ProtectedHoleTransport { ProtectedAddress = 102 }; + var t100 = new ModbusTagDefinition("T100", ModbusRegion.HoldingRegisters, 100, ModbusDataType.Int16); + var t102 = new ModbusTagDefinition("T102", ModbusRegion.HoldingRegisters, 102, ModbusDataType.Int16); + var t104 = new ModbusTagDefinition("T104", ModbusRegion.HoldingRegisters, 104, ModbusDataType.Int16); + var opts = new ModbusDriverOptions { Host = "f", UnitId = 7, Tags = [t100, t102, t104], MaxReadGap = 5, + Probe = new ModbusProbeOptions { Enabled = false } }; + var drv = new ModbusDriver(opts, "m1", _ => fake); + await drv.InitializeAsync("{}", CancellationToken.None); + + // Pre-failure: nothing prohibited. + drv.GetAutoProhibitedRanges().ShouldBeEmpty(); + + await drv.ReadAsync(["T100", "T102", "T104"], CancellationToken.None); + + var snapshot = drv.GetAutoProhibitedRanges(); + snapshot.Count.ShouldBe(1); + snapshot[0].UnitId.ShouldBe((byte)7); + snapshot[0].Region.ShouldBe(ModbusRegion.HoldingRegisters); + snapshot[0].StartAddress.ShouldBe((ushort)100); + snapshot[0].EndAddress.ShouldBe((ushort)104); + snapshot[0].BisectionPending.ShouldBeTrue("multi-register prohibition starts split-pending"); + snapshot[0].LastProbedUtc.ShouldBeGreaterThan(DateTime.UtcNow.AddMinutes(-1)); + + await drv.ShutdownAsync(CancellationToken.None); + } + [Fact] public async Task Tags_Outside_Prohibited_Range_Still_Coalesce() {