fix(s7): a refused span falls back per member and is not regrouped (R07:P2)

A span's block is the union of its members' ranges, so one member that runs
past the DB's real end (or over-declares a STRING) made the CPU refuse the
whole block, and every neighbour within it was Bad on every poll. The
planner's stated invariant (a grouped read cannot fault where per-tag reads
would succeed) was false; the multi-var path already fell back, spans did not.

- A refused multi-member span hands its members back; they are read one at a
  time for that pass, bypassing the multi-var pass.
- _noGroup remembers the members refused on their own (or every member when
  none was) and keeps them out of later plans, so the rest group again. It is
  cleared by every initialize publish. Non-refusal span failures are
  unchanged (every member stamped).
- The once-per-tag refusal Warning memo (R07:H4) is now also cleared by the
  publish, and shutdown resets the write-side Degraded owner, as documented.
- Live gate: MW10 + MD254 (2 bytes past the 256-byte M area) merged into one
  refused block; MW10 keeps reading 4097 on every pass.

Refs #742
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 15:30:12 -04:00
parent 1d83cde6bc
commit 1f7ab80b81
5 changed files with 295 additions and 6 deletions
+7
View File
@@ -344,6 +344,13 @@ to one doomed per-tag read per member. Without this an op that ignores its cance
one full deadline per remaining item under the gate, and `Degraded` (and so the comms-loss watchdog's
clock) only became visible after the whole pass.
**A refused span falls back per member (R07:P2).** A grouped block read covers the union of its members'
ranges, so one member the CPU refuses (running past the DB's real end, an over-declared `stringLength`)
used to make every neighbour in the block Bad on every poll. When the CPU refuses a multi-member span the
driver re-reads its members one at a time for that pass and remembers which tags must not be grouped —
the members refused on their own, or every member when none was — so later passes group only the rest.
The memo lasts until the next initialize. A span failure that is not a refusal still stamps every member.
**Connect backoff.** A dead PLC costs one connect attempt per backoff window (1 s doubling to a 30 s
cap), not one per read / poll tick / probe tick: inside the window `EnsureConnectedAsync` fails fast
(`BadCommunicationError`, `Degraded`) without a connect. A successful connect clears the window, so a
@@ -221,6 +221,14 @@ public sealed class S7Driver
/// by every initialize publish.
/// </summary>
private readonly ConcurrentDictionary<string, byte> _refusalWarned = new(StringComparer.Ordinal);
/// <summary>
/// Tags the span planner must not group (R07:P2): members of a span the CPU refused as a block —
/// the ones that were refused on their own, or every member when none was. They are read
/// individually for the rest of the generation; a re-initialize (a corrected DB, a fixed tag)
/// groups everything again. Guarded by <see cref="_gate"/>.
/// </summary>
private readonly HashSet<string> _noGroup = new(StringComparer.Ordinal);
private bool _disposed;
/// <inheritdoc />
@@ -338,6 +346,8 @@ public sealed class S7Driver
_options = options;
Volatile.Write(ref _tables, new TagTables(tags, parsed));
_refusalWarned.Clear();
_noGroup.Clear();
_skippedTags.BeginRebuild(options.RawTags.Count);
foreach (var (rawPath, reason) in skipped)
{
@@ -453,6 +463,7 @@ public sealed class S7Driver
{
if (_generation != gen) return; // a newer initialize owns the state now
_writeFault = null;
_writeOwnedError = null;
_connectBackoff.RecordSuccess();
Health = new DriverHealth(DriverState.Unknown, Health.LastSuccessfulRead, null);
}
@@ -732,6 +743,13 @@ public sealed class S7Driver
// Indexes the per-tag path must serve: everything, when grouping is off; otherwise the
// planner's leftovers (Timer/Counter) plus any tag whose address won't parse.
var perTag = new List<int>();
// Indexes that must take the per-tag path WITHOUT the multi-var pass: tags the planner may
// not group (R07:P2) and the members of a span refused this pass. Batching them into a
// multi-var request would just refuse the whole chunk again.
List<int>? forcedPerTag = null;
// Members of each span refused this pass, and which of them were then refused on their own.
List<IReadOnlyList<int>>? refusedSpans = null;
HashSet<int>? refusedAlone = null;
if (_options.SpanReadMaxGapBytes >= 0)
{
@@ -739,6 +757,11 @@ public sealed class S7Driver
for (var i = 0; i < fullReferences.Count; i++)
{
if (resolved[i] is not { } tag) continue;
if (_noGroup.Contains(tag.Name))
{
(forcedPerTag ??= []).Add(i);
continue;
}
if (!TryGetParsedAddress(tag, out var addr))
{
// Defensive: every tag pre-parses at init. A tag that somehow didn't goes
@@ -769,7 +792,12 @@ public sealed class S7Driver
foreach (var member in span.Members) results[member.OriginalIndex] = DeadHandleSnapshot(now);
continue;
}
await ReadSpanAsync(plc, span, results, now, outcome, cancellationToken).ConfigureAwait(false);
if (await ReadSpanAsync(plc, span, results, now, outcome, cancellationToken).ConfigureAwait(false)
is { } refusedMembers)
{
(refusedSpans ??= []).Add(refusedMembers);
(forcedPerTag ??= []).AddRange(refusedMembers);
}
}
perTag.AddRange(plan.LeftoverIndexes);
}
@@ -794,6 +822,11 @@ public sealed class S7Driver
perTag = await ReadLeftoversByMultiVarAsync(
plc, resolved, perTag, results, now, outcome, cancellationToken).ConfigureAwait(false);
}
if (forcedPerTag is not null)
{
perTag.AddRange(forcedPerTag);
perTag.Sort();
}
foreach (var i in perTag)
{
@@ -823,9 +856,25 @@ public sealed class S7Driver
}
catch (Exception ex)
{
if (refusedSpans is not null && IsRefusal(ex)) (refusedAlone ??= []).Add(i);
results[i] = new DataValueSnapshot(null, RecordReadFailure(ex, outcome, [resolved[i]!.Name]), null, now);
}
}
// Remember what must not be grouped again (R07:P2). Skipped when the handle died mid-pass:
// the per-member reads then proved nothing about which member the CPU refuses.
if (refusedSpans is not null && !_plcDead)
{
foreach (var members in refusedSpans)
{
var bad = members.Where(m => refusedAlone?.Contains(m) == true).ToList();
var memo = bad.Count > 0 ? bad : [.. members];
foreach (var m in memo) _noGroup.Add(resolved[m]!.Name);
_logger.LogInformation(
"S7 CPU refused a grouped read of {MemberCount} tag(s); reading {Tags} individually from now on. Driver={DriverInstanceId}",
members.Count, string.Join(", ", memo.Select(m => resolved[m]!.Name)), _driverInstanceId);
}
}
}
finally
{
@@ -1222,8 +1271,12 @@ public sealed class S7Driver
/// <param name="now">The batch timestamp, shared by every tag in the batch.</param>
/// <param name="outcome">The pass's outcome accumulator; every member records into it.</param>
/// <param name="ct">Cancellation token for the read.</param>
/// <returns>A task that completes once every member of the span has been stamped.</returns>
private async Task ReadSpanAsync(
/// <returns>
/// <see langword="null"/> once every member has been stamped; or, when the CPU REFUSED the block
/// of a multi-member span (R07:P2 — one member past the DB's end refuses them all), the members'
/// original indexes, unstamped, for the caller to read one at a time.
/// </returns>
private async Task<IReadOnlyList<int>?> ReadSpanAsync(
IS7Plc plc, S7ReadSpanPlanner.Span span, DataValueSnapshot[] results, DateTime now,
ReadPassOutcome outcome, CancellationToken ct)
{
@@ -1244,12 +1297,24 @@ public sealed class S7Driver
_plcDead = true;
throw;
}
catch (Exception ex) when (span.Members.Count > 1 && IsRefusal(ex))
{
// The planner's old invariant ("a grouped read cannot fault where the per-tag reads would
// have succeeded") is false: the block is the UNION of the members' ranges, so one member
// running past the DB's real end, or over-declaring a STRING, makes the CPU refuse every
// neighbour with it. Hand the members back for individual reads.
_logger.LogDebug(ex,
"S7 driver {DriverInstanceId}: CPU refused the block read of {Area} db{Db} @{Start}+{Length}; " +
"re-reading its {Count} member(s) individually", _driverInstanceId, span.Area, span.DbNumber,
span.StartByte, span.LengthBytes, span.Members.Count);
return [.. span.Members.Select(m => m.OriginalIndex)];
}
catch (Exception ex)
{
var status = RecordReadFailure(ex, outcome, span.Members.Select(m => m.Tag.Name));
foreach (var member in span.Members)
results[member.OriginalIndex] = new DataValueSnapshot(null, status, null, now);
return;
return null;
}
foreach (var member in span.Members)
@@ -1268,6 +1333,7 @@ public sealed class S7Driver
new DataValueSnapshot(null, RecordReadFailure(ex, outcome, [member.Tag.Name]), null, now);
}
}
return null;
}
/// <summary>
@@ -11,8 +11,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.S7;
/// <b>The safety invariant is that a span never extends past its last member's end.</b> A
/// span may deliberately read bytes nobody asked for BETWEEN its members (that is what the
/// gap budget buys), but every byte it requests lies inside a range some member was going
/// to read anyway. A grouped read therefore cannot fault (area overrun, DB overrun) where
/// the individual per-tag reads would have succeeded.
/// to read anyway.
/// </para>
/// <para>
/// <b>That does NOT make a grouped read as safe as the per-tag reads</b> (R07:P2): the block is
/// the union of its members' ranges, so ONE member the CPU refuses (running past the DB's real
/// end, an over-declared STRING) makes it refuse the whole block, neighbours included.
/// <see cref="S7Driver.ReadAsync"/> owns that case: a refused multi-member span is re-read one
/// member at a time, and the members that must not be grouped are left out of later plans.
/// </para>
/// <para>
/// <b>Timer and Counter tags are never grouped.</b> Their
@@ -0,0 +1,49 @@
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
namespace ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests.S7_1500;
/// <summary>
/// Live gate for R07:P2 over real S7comm. The fixture's M area is 256 bytes. <c>MW10</c> is seeded
/// (4097); <c>MD254</c> runs two bytes past the end of the area. With the gap budget opened wide
/// the planner merges them into ONE block <c>MB10..258</c>, which the CPU refuses — before the fix
/// that made the perfectly readable <c>MW10</c> Bad (and the driver Degraded) on every poll.
/// </summary>
[Collection(Snap7ServerCollection.Name)]
[Trait("Category", "Integration")]
[Trait("Device", "S7_1500")]
public sealed class S7_1500SpanRefusalTests(Snap7ServerFixture sim)
{
/// <summary>The good neighbour of an over-running member keeps reading its seeded value, every pass.</summary>
[Fact]
public async Task A_member_past_the_area_end_does_not_spoil_its_span_neighbour()
{
if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason);
var ct = TestContext.Current.CancellationToken;
var options = new S7DriverOptions
{
Host = sim.Host, Port = sim.Port, CpuType = S7CpuType.S71500,
Timeout = TimeSpan.FromSeconds(5),
Probe = new S7ProbeOptions { Enabled = false },
// Wide enough that MW10 and MD254 land in one planned span.
SpanReadMaxGapBytes = 256,
SpanReadMaxSpanBytes = 256,
RawTags = S7RawTags.Entries(
new S7TagDefinition("Mk/Word10", "MW10", S7DataType.UInt16, Writable: false),
new S7TagDefinition("Mk/PastEnd", "MD254", S7DataType.Int32, Writable: false)),
};
await using var drv = new S7Driver(options, driverInstanceId: "s7-span-refusal-live");
await drv.InitializeAsync("{}", ct);
for (var pass = 0; pass < 3; pass++)
{
var r = await drv.ReadAsync(["Mk/Word10", "Mk/PastEnd"], ct);
r[0].StatusCode.ShouldBe(0u, $"pass {pass}: the in-range neighbour must be served");
Convert.ToInt32(r[0].Value).ShouldBe(4097);
r[1].StatusCode.ShouldNotBe(0u, "the over-running member is refused on its own");
drv.GetHealth().State.ShouldBe(DriverState.Healthy, "a refusal is not a comms fault");
}
}
}
@@ -0,0 +1,161 @@
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests;
/// <summary>
/// R07:P2 — a span's block is the union of its members' ranges, so ONE member that runs past the
/// DB's real end (or over-declares a STRING) made the CPU refuse the whole block and every
/// neighbour inside it went Bad on every poll. A refused span now re-reads its members one at a
/// time for that pass and remembers which tags must not be grouped, so later passes group only the
/// members that can be grouped.
/// </summary>
[Trait("Category", "Unit")]
public sealed class S7SpanRefusalFallbackTests
{
private const uint BadDeviceFailure = 0x808B0000u;
/// <summary>DB1 is 4 bytes long on this fake CPU: anything reaching past byte 4 is refused.</summary>
private const int DbLength = 4;
private static Exception OutOfRange() => new("Received error from PLC: Address out of range.");
private static (S7Driver Driver, ScriptedS7PlcFactory Factory, Func<int> BlockReads, Func<int> TagReads) Start(
params S7TagDefinition[] tags)
{
var factory = new ScriptedS7PlcFactory();
var blockReads = 0;
var tagReads = 0;
factory.ReadBytes = (_, _, start, count, _) =>
{
Interlocked.Increment(ref blockReads);
if (start + count > DbLength) throw OutOfRange();
var b = new byte[count];
for (var i = 0; i < count; i++) b[i] = (byte)(start + i + 1);
return Task.FromResult(b);
};
factory.Read = (addr, _) =>
{
Interlocked.Increment(ref tagReads);
var offset = int.Parse(addr[(addr.LastIndexOf('W') + 1)..], System.Globalization.CultureInfo.InvariantCulture);
if (offset + 2 > DbLength) throw OutOfRange();
return Task.FromResult<object?>((ushort)(((offset + 1) << 8) | (offset + 2)));
};
var drv = new S7Driver(new S7DriverOptions
{
Host = "192.0.2.1",
Probe = new S7ProbeOptions { Enabled = false },
RawTags = S7RawTags.Entries(tags),
}, "s7-span-fallback", factory);
return (drv, factory, () => Volatile.Read(ref blockReads), () => Volatile.Read(ref tagReads));
}
private static S7TagDefinition Word(string name, int offset) =>
new(name, $"DB1.DBW{offset}", S7DataType.UInt16, Writable: false);
/// <summary>
/// The over-running member is refused on its own; its neighbours are served from the very first
/// pass, and from the second pass on the neighbours are grouped again WITHOUT the bad member.
/// </summary>
[Fact]
public async Task A_refused_span_falls_back_per_member_and_stops_grouping_the_bad_member()
{
var ct = TestContext.Current.CancellationToken;
var (drv, _, blockReads, tagReads) = Start(Word("A", 0), Word("B", 2), Word("C", 4));
using var _ = drv;
await drv.InitializeAsync("{}", ct);
string[] refs = ["A", "B", "C"];
// Pass 1: the 6-byte span is refused, then each member is read on its own.
var r1 = await drv.ReadAsync(refs, ct);
r1[0].StatusCode.ShouldBe(0u);
r1[0].Value.ShouldBe((ushort)0x0102);
r1[1].StatusCode.ShouldBe(0u);
r1[1].Value.ShouldBe((ushort)0x0304);
r1[2].StatusCode.ShouldBe(BadDeviceFailure);
drv.GetHealth().State.ShouldBe(DriverState.Healthy);
blockReads().ShouldBe(1);
tagReads().ShouldBe(3);
// Pass 2: A+B ride ONE block read again (the planner no longer sees C); C reads alone.
var r2 = await drv.ReadAsync(refs, ct);
r2[0].Value.ShouldBe((ushort)0x0102);
r2[1].Value.ShouldBe((ushort)0x0304);
r2[2].StatusCode.ShouldBe(BadDeviceFailure);
blockReads().ShouldBe(2, "A+B grouped again, without the refused member");
tagReads().ShouldBe(4, "only C took the per-tag path on pass 2");
}
/// <summary>
/// When the block is refused but every member reads fine alone, the span itself is the problem:
/// all of its members stop being grouped (and stop re-trying the refused block every pass).
/// </summary>
[Fact]
public async Task A_refused_span_whose_members_all_read_alone_is_not_grouped_again()
{
var ct = TestContext.Current.CancellationToken;
var factory = new ScriptedS7PlcFactory();
var blockReads = 0;
factory.ReadBytes = (_, _, _, _, _) => { Interlocked.Increment(ref blockReads); throw OutOfRange(); };
factory.Read = (_, _) => Task.FromResult<object?>((ushort)7);
using var drv = new S7Driver(new S7DriverOptions
{
Host = "192.0.2.1",
Probe = new S7ProbeOptions { Enabled = false },
RawTags = S7RawTags.Entries(Word("A", 0), Word("B", 2)),
}, "s7-span-fallback-all", factory);
await drv.InitializeAsync("{}", ct);
(await drv.ReadAsync(["A", "B"], ct)).ShouldAllBe(s => s.StatusCode == 0u);
(await drv.ReadAsync(["A", "B"], ct)).ShouldAllBe(s => s.StatusCode == 0u);
Volatile.Read(ref blockReads).ShouldBe(1, "the refused block is not re-issued on later passes");
}
/// <summary>
/// Positive control: a span failure that is NOT a refusal (a device-layer fault) does not fall
/// back — re-reading N members one by one against a failing device would only multiply the fault.
/// </summary>
[Fact]
public async Task A_non_refusal_span_failure_does_not_fall_back()
{
var ct = TestContext.Current.CancellationToken;
var factory = new ScriptedS7PlcFactory
{
ReadBytes = (_, _, _, _, _) => throw new global::S7.Net.PlcException(global::S7.Net.ErrorCode.SendData, "device fault"),
};
using var drv = new S7Driver(new S7DriverOptions
{
Host = "192.0.2.1",
Probe = new S7ProbeOptions { Enabled = false },
RawTags = S7RawTags.Entries(Word("A", 0), Word("B", 2)),
}, "s7-span-nofallback", factory);
await drv.InitializeAsync("{}", ct);
var before = factory.WireOps;
var r = await drv.ReadAsync(["A", "B"], ct);
r.ShouldAllBe(s => s.StatusCode == BadDeviceFailure);
(factory.WireOps - before).ShouldBe(1);
drv.GetHealth().State.ShouldBe(DriverState.Degraded);
}
/// <summary>The memo is per generation: a re-initialize (e.g. a corrected DB) groups everything again.</summary>
[Fact]
public async Task Reinitialize_clears_the_do_not_group_memo()
{
var ct = TestContext.Current.CancellationToken;
var (drv, _, blockReads, tagReads) = Start(Word("A", 0), Word("B", 2), Word("C", 4));
using var _ = drv;
await drv.InitializeAsync("{}", ct);
await drv.ReadAsync(["A", "B", "C"], ct);
blockReads().ShouldBe(1);
tagReads().ShouldBe(3);
await drv.InitializeAsync("{}", ct);
await drv.ReadAsync(["A", "B", "C"], ct);
// With the memo still in force pass 2 would be one A+B block read plus ONE per-tag read (C).
// Cleared, the full A..C span is tried again, refused again, and all three re-read alone.
blockReads().ShouldBe(2);
tagReads().ShouldBe(6, "after re-init the full span is tried again (and refused again)");
}
}