From 39f67a0fce1b38bc5bc1cb090c83d651ec345af4 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 16 Aug 2026 04:15:37 -0400 Subject: [PATCH] =?UTF-8?q?perf(s7):=20span-grouped=20reads=20=E2=80=94=20?= =?UTF-8?q?one=20PDU=20per=20contiguous=20block=20instead=20of=20one=20per?= =?UTF-8?q?=20tag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../S7DriverOptions.cs | 50 +++ .../ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs | 293 ++++++++++++-- .../S7DriverFactoryExtensions.cs | 10 + .../S7ReadSpanPlanner.cs | 198 ++++++++++ .../S7ReadSpanPlannerTests.cs | 301 ++++++++++++++ .../S7SpanReadParityTests.cs | 373 ++++++++++++++++++ 6 files changed, 1188 insertions(+), 37 deletions(-) create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7ReadSpanPlanner.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ReadSpanPlannerTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7DriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7DriverOptions.cs index 15a09eb7..4b6912e8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7DriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7DriverOptions.cs @@ -79,6 +79,56 @@ public sealed class S7DriverOptions [Display(Name = "Probe timeout (seconds)", Description = "Connection test timeout. Default 5s.", GroupName = "Diagnostics")] [Range(1, 60)] public int ProbeTimeoutSeconds { get; init; } = 5; + + /// + /// Largest hole, in bytes, the read planner will read THROUGH when merging two neighbouring + /// tags into one block request. It is a budget for deliberately wasted bytes: raising it + /// merges more aggressively (fewer wire requests, more bytes nobody asked for), lowering it + /// to 0 merges only tags that are strictly adjacent or overlapping. + /// + /// + /// + /// Any value <= -1 disables span grouping entirely — every tag is read + /// on its own, exactly as the driver behaved before grouping existed. That is the + /// escape hatch for a CPU that dislikes wide block reads, and the control arm the + /// parity tests read through. + /// + /// + /// Grouping cannot make a read fail where the per-tag reads would have succeeded: a + /// span never extends past its LAST member's end, so every byte it requests lies inside + /// a range some tag was already going to read. Timer and Counter tags are never grouped + /// — their offset is a device number, not a byte address. + /// + /// + [Display(Name = "Span read max gap (bytes)", + Description = "Largest hole read through when merging neighbouring tags into one block request. -1 disables grouping.", + GroupName = "Performance")] + public int SpanReadMaxGapBytes { get; init; } = DefaultSpanReadMaxGapBytes; + + /// Default for , shared with the config DTO so the two can't drift. + public const int DefaultSpanReadMaxGapBytes = 16; + + /// + /// Largest single planned span, in bytes. A run of tags that would exceed it is broken into + /// several spans; a single tag wider than the cap still gets its own span, because one tag + /// is never split across requests. + /// + /// + /// This is not a correctness bound: S7.Net transparently chunks a ReadBytes + /// call larger than the negotiated PDU into several wire requests, so an oversized span + /// reads correctly either way. The cap exists to bound wasted-gap amplification — without + /// it, a long sparse run of tags each separated by an under-budget hole would coalesce into + /// one enormous request that is mostly padding. The 220-byte default sits just under the + /// usable payload of the common 240-byte S7-300/400 PDU, so a typical span is one request + /// on the wire. + /// + [Display(Name = "Span read max span (bytes)", + Description = "Largest single planned block read. Bounds wasted-gap amplification, not correctness.", + GroupName = "Performance")] + public int SpanReadMaxSpanBytes { get; init; } = DefaultSpanReadMaxSpanBytes; + + /// Default for , shared with the config DTO so the two can't drift. + public const int DefaultSpanReadMaxSpanBytes = 220; } public sealed class S7ProbeOptions diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs index 7ef1ba03..1dfafaf5 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs @@ -500,6 +500,25 @@ public sealed class S7Driver // ---- IReadable ---- /// + /// + /// + /// The batch is served in two passes under ONE gate acquisition. First + /// groups the resolved tags into contiguous + /// (area, DB) byte spans and each span is read with a single + /// ReadBytesAsync, its members decoded by slicing the returned block — this is + /// what turns one wire request per tag into one per contiguous block. Whatever the + /// planner declines (Timer/Counter, whose offsets are device numbers) plus any span not + /// worth issuing (see ) then goes through the unchanged + /// per-tag path. + /// + /// + /// Setting SpanReadMaxGapBytes below zero skips the first pass entirely, which + /// restores the pre-grouping behaviour exactly. Either way the result array is written + /// by original index, so the caller sees its own ordering regardless of how the batch + /// was partitioned, and both passes share so a fault + /// stamps the same status whichever path observed it. + /// + /// public async Task> ReadAsync( IReadOnlyList fullReferences, CancellationToken cancellationToken) { @@ -534,17 +553,69 @@ public sealed class S7Driver return results; } + // Resolve every reference first. An unresolvable one is a miss, stamped here and never + // offered to the planner — grouping must not be able to change what "unknown tag" means. + var resolved = new S7TagDefinition?[fullReferences.Count]; for (var i = 0; i < fullReferences.Count; i++) { - var name = fullReferences[i]; - if (!_resolver.TryResolve(name, out var tag)) - { + if (_resolver.TryResolve(fullReferences[i], out var tag)) + resolved[i] = tag; + else results[i] = new DataValueSnapshot(null, StatusBadNodeIdUnknown, null, now); - continue; + } + + // 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(); + + if (_options.SpanReadMaxGapBytes >= 0) + { + List<(int OriginalIndex, S7TagDefinition Tag, S7ParsedAddress Addr)>? plannable = null; + for (var i = 0; i < fullReferences.Count; i++) + { + if (resolved[i] is not { } tag) continue; + if (!TryGetParsedAddress(tag, out var addr)) + { + // Defensive: every tag pre-parses at init. A tag that somehow didn't goes + // to the per-tag path, where ReadOneAsync's Parse throws the FormatException + // the catch ladder has always mapped to BadCommunicationError. + perTag.Add(i); + continue; + } + (plannable ??= []).Add((i, tag, addr)); } + + if (plannable is not null) + { + var plan = S7ReadSpanPlanner.Build( + plannable, _options.SpanReadMaxGapBytes, _options.SpanReadMaxSpanBytes); + foreach (var span in plan.Spans) + { + // A span that saves no round-trip is not worth changing the wire shape for. + if (PrefersPerTagRead(span)) + { + perTag.Add(span.Members[0].OriginalIndex); + continue; + } + await ReadSpanAsync(plc, span, results, now, cancellationToken).ConfigureAwait(false); + } + perTag.AddRange(plan.LeftoverIndexes); + } + } + else + { + for (var i = 0; i < fullReferences.Count; i++) + if (resolved[i] is not null) perTag.Add(i); + } + + // Sorted so a batch is served in reference order regardless of how it was partitioned — + // the per-tag tail is otherwise ordered by the planner's grouping, not by the caller's. + perTag.Sort(); + foreach (var i in perTag) + { try { - var value = await ReadOneAsync(plc, tag, cancellationToken).ConfigureAwait(false); + var value = await ReadOneAsync(plc, resolved[i]!, cancellationToken).ConfigureAwait(false); results[i] = new DataValueSnapshot(value, 0u, now, now); _health = new DriverHealth(DriverState.Healthy, now, null); } @@ -555,42 +626,14 @@ public sealed class S7Driver // dead (we hold _gate) and propagate: the caller cancelled, nobody consumes the // batch. Marking is unconditional on OCE (not routed through the type classifier): // whether bytes moved is unknowable, and a false positive costs one cheap reopen - // versus permanent desync for a false negative. + // versus permanent desync for a false negative. Kept AT the call site rather + // than inside RecordReadFailure so the rethrow preserves the original stack. _plcDead = true; throw; } - catch (NotSupportedException) - { - results[i] = new DataValueSnapshot(null, StatusBadNotSupported, null, now); - } - catch (PlcException pex) when (IsAccessDenied(pex)) - { - // PUT/GET-disabled (S7-1200/1500) / access-protection — a permanent - // configuration fault, NOT a transient one. Blind retry is wasted effort, - // so map it to BadNotSupported and flag the driver as a config alert - // (Faulted) rather than Degraded — per driver-specs.md §5. - results[i] = new DataValueSnapshot(null, StatusBadNotSupported, null, now); - _health = new DriverHealth(DriverState.Faulted, _health.LastSuccessfulRead, - "S7 access denied — enable PUT/GET communication in TIA Portal " + - $"(Protection & Security) for this CPU. PLC reported: {pex.Message}"); - } - catch (PlcException pex) - { - // A genuine device-layer fault (CPU error, hardware fault) — transient - // enough to keep retrying; report BadDeviceFailure and degrade health. A - // connection-fatal PlcException (socket drop / ConnectionError) additionally - // marks the handle dead so the next read reopens it. - MarkConnectionDeadIfFatal(pex); - results[i] = new DataValueSnapshot(null, StatusBadDeviceFailure, null, now); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, pex.Message); - } catch (Exception ex) { - // Socket / timeout / transport fault — mark the handle dead when it's a - // connection loss so EnsureConnectedAsync reopens on the next call. - MarkConnectionDeadIfFatal(ex); - results[i] = new DataValueSnapshot(null, StatusBadCommunicationError, null, now); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); + results[i] = new DataValueSnapshot(null, RecordReadFailure(ex), null, now); } } } @@ -598,6 +641,182 @@ public sealed class S7Driver return results; } + /// + /// True when a planned span is better served by the per-tag path: it holds exactly ONE + /// narrow scalar, so issuing it as a block read saves no round-trip and buys nothing. + /// + /// + /// The point of grouping is to replace N wire requests with one. A lone narrow tag is one + /// wire request either way — but a block read reaches the CPU differently from S7.Net's + /// per-address string read, so converting it would change the observable wire shape of every + /// single-tag read on the server for zero benefit. A lone ARRAY or wide/buffer scalar is NOT + /// demoted: those already read a block on the per-tag path, so serving them as a + /// single-member span is byte-for-byte the request they always issued. + /// + /// The planned span to judge. + /// true when the span's single member should go through . + private static bool PrefersPerTagRead(S7ReadSpanPlanner.Span span) + => span.Members.Count == 1 + && span.Members[0].Tag.ArrayCount is not >= 1 + && !IsBufferType(span.Members[0].Tag, span.Members[0].Addr); + + /// + /// Reads one planned span — a SINGLE ReadBytesAsync serving every member — and + /// decodes each member from its slice of the returned block. + /// + /// + /// A span read is one wire request standing in for many per-tag requests, so a transport + /// fault that used to spoil one tag now spoils all of them: every member is stamped with + /// the SAME status would have produced for that exception on + /// the per-tag path, and health degrades identically. Decoding, by contrast, is per member + /// inside its own guard — a single mis-authored tag (declared type vs. address size) must + /// not spoil the neighbours that shared its block. + /// + /// The live connection (caller holds ). + /// The planned block read. + /// The batch's result array, written in place at each member's original index. + /// The batch timestamp, shared by every tag in the batch. + /// Cancellation token for the read. + /// A task that completes once every member of the span has been stamped. + private async Task ReadSpanAsync( + IS7Plc plc, S7ReadSpanPlanner.Span span, DataValueSnapshot[] results, DateTime now, CancellationToken ct) + { + byte[] block; + try + { + block = await plc.ReadBytesAsync( + ToS7NetArea(span.Area), span.DbNumber, span.StartByte, span.LengthBytes, ct) + .ConfigureAwait(false) + ?? throw new System.IO.InvalidDataException( + $"S7.Net returned null block for {span.Area} db{span.DbNumber} " + + $"@{span.StartByte}+{span.LengthBytes}"); + } + catch (OperationCanceledException) + { + // Same reasoning as the per-tag path: a cancelled block read leaves the single gated + // stream at an untrustworthy position, so the handle must be reopened (STAB-15a). + _plcDead = true; + throw; + } + catch (Exception ex) + { + var status = RecordReadFailure(ex); + foreach (var member in span.Members) + results[member.OriginalIndex] = new DataValueSnapshot(null, status, null, now); + return; + } + + foreach (var member in span.Members) + { + try + { + var value = DecodeMember(member, block); + results[member.OriginalIndex] = new DataValueSnapshot(value, 0u, now, now); + _health = new DriverHealth(DriverState.Healthy, now, null); + } + catch (Exception ex) + { + // Decoding is pure and synchronous — no cancellation can surface here, so there is + // deliberately no OperationCanceledException arm to shadow the block-read one above. + results[member.OriginalIndex] = new DataValueSnapshot(null, RecordReadFailure(ex), null, now); + } + } + } + + /// + /// Decodes one span member from its slice of the block, routing to exactly the decoder + /// would have chosen for that tag, in the same order. + /// + /// The member to decode, carrying its slice position and width. + /// The block the span read. + /// The decoded value, boxed identically to the per-tag path's. + private static object DecodeMember(S7ReadSpanPlanner.Member member, byte[] block) + { + // The two block decoders take byte[] (they hand the buffer to S7.Net's own + // S7String/DateTime/Timer/Counter FromByteArray helpers, which are byte[]-only), so a + // member that is not the whole block is copied out. Narrow scalars need no copy at all — + // DecodeNarrowScalar reads straight from a ReadOnlySpan positioned at the tag's own byte. + if (member.Tag.ArrayCount is >= 1) + return DecodeArrayBlock(member.Tag, member.Addr, SliceForMember(member, block)); + if (IsBufferType(member.Tag, member.Addr)) + return DecodeScalarBlock(member.Tag, member.Addr, SliceForMember(member, block)); + return DecodeNarrowScalar(member.Tag, member.Addr, block.AsSpan(member.RelativeOffset, member.WidthBytes)); + } + + /// + /// The member's own bytes as a byte[] for the block decoders. Returns the block + /// itself — no copy — when the member IS the whole block, which is the single-member span + /// every array / wide scalar produces today. + /// + /// The member whose bytes are wanted. + /// The block the span read. + /// The member's bytes, starting at its own first byte. + private static byte[] SliceForMember(S7ReadSpanPlanner.Member member, byte[] block) + => member.RelativeOffset == 0 && member.WidthBytes == block.Length + ? block + : block[member.RelativeOffset..(member.RelativeOffset + member.WidthBytes)]; + + /// + /// The tag's parsed address: the table built at init, with an on-demand parse as the + /// defensive fallback for a tag that isn't in it. Mirrors 's + /// lookup, but never throws — an unparseable address returns false so the caller can + /// route the tag to the per-tag path, where the surfaces + /// through the ladder exactly as it always has. + /// + /// The tag whose address is wanted. + /// The parsed address, valid only when this returns true. + /// true when the address is known or parses. + private bool TryGetParsedAddress(S7TagDefinition tag, out S7ParsedAddress addr) + => _parsedByRawPath.TryGetValue(tag.Name, out addr) || S7AddressParser.TryParse(tag.Address, out addr); + + /// + /// The read-failure classification ladder, shared by the per-tag path and the span path so + /// a failed span stamps every member with exactly the status the same exception would have + /// produced on a single tag. Applies the health / dead-handle side effects and returns the + /// status code the caller must stamp. + /// + /// + /// is deliberately NOT handled here: it is control + /// flow rather than a per-tag status (it marks the handle dead and propagates), and + /// rethrowing it from a helper would rewrite the stack. Both call sites catch it first. + /// + /// The exception observed on a read. + /// The OPC UA status code every affected tag must be stamped with. + private uint RecordReadFailure(Exception ex) + { + switch (ex) + { + case NotSupportedException: + return StatusBadNotSupported; + + case PlcException pex when IsAccessDenied(pex): + // PUT/GET-disabled (S7-1200/1500) / access-protection — a permanent configuration + // fault, NOT a transient one. Blind retry is wasted effort, so map it to + // BadNotSupported and flag the driver as a config alert (Faulted) rather than + // Degraded — per driver-specs.md §5. + _health = new DriverHealth(DriverState.Faulted, _health.LastSuccessfulRead, + "S7 access denied — enable PUT/GET communication in TIA Portal " + + $"(Protection & Security) for this CPU. PLC reported: {pex.Message}"); + return StatusBadNotSupported; + + case PlcException pex: + // A genuine device-layer fault (CPU error, hardware fault) — transient enough to + // keep retrying; report BadDeviceFailure and degrade health. A connection-fatal + // PlcException (socket drop / ConnectionError) additionally marks the handle dead + // so the next read reopens it. + MarkConnectionDeadIfFatal(pex); + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, pex.Message); + return StatusBadDeviceFailure; + + default: + // Socket / timeout / transport fault — mark the handle dead when it's a connection + // loss so EnsureConnectedAsync reopens on the next call. + MarkConnectionDeadIfFatal(ex); + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); + return StatusBadCommunicationError; + } + } + private async Task ReadOneAsync(IS7Plc plc, S7TagDefinition tag, CancellationToken ct) { // Every tag pre-parses its address at init (_parsedByRawPath); the on-demand @@ -681,7 +900,7 @@ public sealed class S7Driver /// Tag definition carrying the . /// Parsed address — its routes Timer/Counter into the seam regardless of . /// true if the tag routes through the byte-buffer codec. - private static bool IsBufferType(S7TagDefinition tag, S7ParsedAddress addr) => + internal static bool IsBufferType(S7TagDefinition tag, S7ParsedAddress addr) => addr.Area is S7Area.Timer or S7Area.Counter || tag.DataType is S7DataType.Int64 or S7DataType.UInt64 or S7DataType.Float64 or S7DataType.String or S7DataType.DateTime; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverFactoryExtensions.cs index 5b4670e4..9548caf9 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverFactoryExtensions.cs @@ -69,6 +69,11 @@ public static class S7DriverFactoryExtensions // tags. The driver maps each RawTagEntry's TagConfig blob through S7TagDefinitionFactory // at Initialize; the legacy pre-declared DTO tag path (Name/Address → BuildTag) is retired. RawTags = dto.RawTags is { Count: > 0 } ? [.. dto.RawTags] : [], + // Block-read grouping knobs. Mapped explicitly like every other key on this DTO — the + // options object is never deserialised directly — so omitting them here would leave the + // documented "-1 disables grouping" escape hatch unreachable from a deployed config. + SpanReadMaxGapBytes = dto.SpanReadMaxGapBytes ?? S7DriverOptions.DefaultSpanReadMaxGapBytes, + SpanReadMaxSpanBytes = dto.SpanReadMaxSpanBytes ?? S7DriverOptions.DefaultSpanReadMaxSpanBytes, Probe = new S7ProbeOptions { Enabled = dto.Probe?.Enabled ?? true, @@ -122,6 +127,11 @@ public static class S7DriverFactoryExtensions public List? RawTags { get; init; } /// Gets the probe configuration. public S7ProbeDto? Probe { get; init; } + /// Gets the largest hole the read planner reads through when merging tags into one + /// block request; any value below zero disables block-read grouping. Null keeps the default. + public int? SpanReadMaxGapBytes { get; init; } + /// Gets the largest single planned block read in bytes. Null keeps the default. + public int? SpanReadMaxSpanBytes { get; init; } } /// Data transfer object for S7 probe configuration. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7ReadSpanPlanner.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7ReadSpanPlanner.cs new file mode 100644 index 00000000..96f7770a --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7ReadSpanPlanner.cs @@ -0,0 +1,198 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.S7; + +/// +/// Pure planner that turns one read batch into contiguous (area, DB) byte spans, so the +/// driver can serve many tags with one ReadBytesAsync instead of one wire request per +/// tag. No I/O, no driver state — the grouping rules are decided here and executed by +/// . +/// +/// +/// +/// The safety invariant is that a span never extends past its last member's end. 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. +/// +/// +/// Timer and Counter tags are never grouped. Their +/// holds the timer/counter DEVICE NUMBER rather +/// than a byte address, and S7.Net addresses those areas in per-device units — merging one +/// with a byte-addressed neighbour, or even with another Timer, would read from a +/// completely unrelated place. They come back as leftovers for the per-tag path. +/// +/// +internal static class S7ReadSpanPlanner +{ + /// One tag inside a planned span, positioned relative to the span's start byte. + /// The tag's index in the caller's read batch — where its result must be written. + /// The resolved tag definition (carries the data type, array count and string length). + /// The tag's parsed address (carries the area, DB, access size and bit position). + /// Bytes this tag occupies — the exact number its decoder consumes. + /// The tag's first byte expressed against ; the slice offset into the block. + internal sealed record Member( + int OriginalIndex, + S7TagDefinition Tag, + S7ParsedAddress Addr, + int WidthBytes, + int RelativeOffset); + + /// One planned block read: a contiguous byte range in one area, serving several tags. + /// The S7 area the block is read from. + /// Data-block number (0 for non-DB areas). + /// First byte of the block — the lowest member's byte offset. + /// Block length — always exactly the furthest member's end minus . + /// The tags this block serves, in ascending byte order. + internal sealed record Span( + S7Area Area, + int DbNumber, + int StartByte, + int LengthBytes, + IReadOnlyList Members); + + /// The planned grouping of one read batch. + /// Block reads to issue, one wire request each. + /// Batch indexes the planner declined — the caller reads these one at a time. + internal sealed record Plan(IReadOnlyList Spans, IReadOnlyList LeftoverIndexes); + + /// + /// Groups the resolved tags of one read batch into block-read spans. + /// + /// + /// The batch's resolved tags, each paired with its parsed address and its index in the + /// caller's reference list. Unresolvable references must be handled by the caller and left + /// out of this list. + /// + /// + /// Largest hole to read through when merging (inclusive). 0 merges only strictly + /// adjacent or overlapping tags; any value below zero disables grouping and returns every + /// index as a leftover. + /// + /// + /// Largest planned span. A run that would exceed it is broken; a single tag wider than it + /// still forms its own span, because a tag is never split across requests. + /// + /// The spans to read plus the indexes the caller must read individually. + internal static Plan Build( + IReadOnlyList<(int OriginalIndex, S7TagDefinition Tag, S7ParsedAddress Addr)> resolved, + int maxGapBytes, + int maxSpanBytes) + { + ArgumentNullException.ThrowIfNull(resolved); + + var leftovers = new List(); + + // The off switch. Checked here (as well as at the call site) so the disabled behaviour is + // a property of the planner and can be proven without standing up a driver. + if (maxGapBytes < 0) + { + foreach (var (index, _, _) in resolved) leftovers.Add(index); + return new Plan([], leftovers); + } + + List<(int Index, S7TagDefinition Tag, S7ParsedAddress Addr, int Width)>? eligible = null; + foreach (var (index, tag, addr) in resolved) + { + // Timer/Counter offsets are device numbers, not byte addresses — see the type remarks. + if (addr.Area is S7Area.Timer or S7Area.Counter) + { + leftovers.Add(index); + continue; + } + (eligible ??= []).Add((index, tag, addr, MemberWidthBytes(tag, addr))); + } + + if (eligible is null) return new Plan([], leftovers); + + var spans = new List(); + + // Group by (area, DB): two areas — or two data blocks — are separate address spaces, so a + // byte range in one says nothing about the same range in the other. + foreach (var group in eligible.GroupBy(e => (e.Addr.Area, e.Addr.DbNumber))) + { + OpenSpan? open = null; + + // Ascending byte order. OrderBy is stable, so tags sharing a byte (two Bools at + // different bit positions) keep their batch order inside the span. + foreach (var (index, tag, addr, width) in group.OrderBy(e => e.Addr.ByteOffset)) + { + var start = addr.ByteOffset; + var end = start + width; // exclusive + + if (open is not null) + { + // A negative gap means the tags overlap (a word inside an already-open dword, + // or a second Bool in the same byte) — that is a gap of zero or better, so it + // always passes a non-negative budget without any clamping. + var gap = start - open.EndByte; + var mergedEnd = Math.Max(open.EndByte, end); + + if (gap <= maxGapBytes && mergedEnd - open.StartByte <= maxSpanBytes) + { + open.Members.Add(new Member(index, tag, addr, width, start - open.StartByte)); + // Max, not `end`: an overlapping member must never pull the span's end back. + open.EndByte = mergedEnd; + continue; + } + + spans.Add(Seal(group.Key.Area, group.Key.DbNumber, open)); + } + + // Start a fresh span. Deliberately unconditional — a tag wider than maxSpanBytes + // gets its own oversized span rather than being split, and because the merge test + // above measures the RESULTING length, that oversized span can never absorb a + // neighbour afterwards. + open = new OpenSpan(start, end); + open.Members.Add(new Member(index, tag, addr, width, 0)); + } + + if (open is not null) spans.Add(Seal(group.Key.Area, group.Key.DbNumber, open)); + } + + return new Plan(spans, leftovers); + } + + /// + /// Bytes one tag occupies inside a contiguous block. The three arms are checked in exactly + /// the order routes a tag to its decoder, so the width + /// planned here is always the width the decoder that runs will consume. + /// + /// The resolved tag definition. + /// The tag's parsed address. + /// The tag's byte width. + internal static int MemberWidthBytes(S7TagDefinition tag, S7ParsedAddress addr) + { + if (tag.ArrayCount is >= 1) + return tag.ArrayCount.Value * S7Driver.ElementByteSize(addr.Size); + if (S7Driver.IsBufferType(tag, addr)) + return S7Driver.ScalarByteWidth(tag, addr); + return S7Driver.NarrowScalarWidth(addr); + } + + /// Freezes an in-progress span into its immutable form. + /// The area the span reads from. + /// The data-block number. + /// The in-progress span. + /// The sealed span; its length is exactly the furthest member's end minus the start. + private static Span Seal(S7Area area, int dbNumber, OpenSpan open) + => new(area, dbNumber, open.StartByte, open.EndByte - open.StartByte, open.Members); + + /// + /// Mutable accumulator for a span still being extended. is fixed at + /// creation (members arrive in ascending order, so nothing can ever start earlier); + /// is exclusive and only ever grows. + /// + /// First byte of the span. + /// Exclusive end byte of the span so far. + private sealed class OpenSpan(int startByte, int endByte) + { + /// First byte of the span — fixed for the span's lifetime. + public int StartByte { get; } = startByte; + + /// Exclusive end byte of the furthest member seen so far. + public int EndByte { get; set; } = endByte; + + /// Members accumulated so far, in ascending byte order. + public List Members { get; } = []; + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ReadSpanPlannerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ReadSpanPlannerTests.cs new file mode 100644 index 00000000..b896ccdb --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ReadSpanPlannerTests.cs @@ -0,0 +1,301 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// Unit tests for — the pure grouping step that turns a batch +/// of resolved S7 tags into contiguous (area, DB) byte spans, one wire request each. +/// The planner does no I/O, so every rule it enforces (gap ceiling, span cap, area/DB +/// separation, Timer/Counter exclusion, never-overshoot) is provable here without a PLC. +/// +/// +/// The single most important invariant in this file is +/// : a span is allowed to read +/// bytes NOBODY asked for in the middle (that is the whole point of a gap budget), but it must +/// never read past the end of its last member. Reading past the end is what could make a +/// grouped read fail (area/DB overrun) where the individual per-tag reads would have +/// succeeded — the one way this optimization could lose data the old path served. +/// +[Trait("Category", "Unit")] +public sealed class S7ReadSpanPlannerTests +{ + // ── Helpers ────────────────────────────────────────────────────────────────────────── + + /// Builds one planner input row: a resolved tag paired with its parsed address. + private static (int OriginalIndex, S7TagDefinition Tag, S7ParsedAddress Addr) T( + int index, string address, S7DataType dataType, int? arrayCount = null, int stringLength = 254) + => (index, + new S7TagDefinition($"Tag{index}", address, dataType, + Writable: false, StringLength: stringLength, ArrayCount: arrayCount), + S7AddressParser.Parse(address)); + + /// Plans with the shipped defaults (gap 16, cap 220). + private static S7ReadSpanPlanner.Plan Plan( + params (int OriginalIndex, S7TagDefinition Tag, S7ParsedAddress Addr)[] tags) + => S7ReadSpanPlanner.Build(tags, maxGapBytes: 16, maxSpanBytes: 220); + + // ── Merging ────────────────────────────────────────────────────────────────────────── + + /// Three back-to-back words collapse into one 6-byte span with correct member offsets. + [Fact] + public void Contiguous_tags_merge_into_a_single_span() + { + var plan = Plan( + T(0, "DB1.DBW0", S7DataType.UInt16), + T(1, "DB1.DBW2", S7DataType.Int16), + T(2, "DB1.DBW4", S7DataType.UInt16)); + + plan.LeftoverIndexes.ShouldBeEmpty(); + var span = plan.Spans.ShouldHaveSingleItem(); + span.Area.ShouldBe(S7Area.DataBlock); + span.DbNumber.ShouldBe(1); + span.StartByte.ShouldBe(0); + span.LengthBytes.ShouldBe(6); + span.Members.Select(m => m.RelativeOffset).ShouldBe([0, 2, 4]); + span.Members.Select(m => m.WidthBytes).ShouldBe([2, 2, 2]); + } + + /// A hole wider than the gap budget breaks the run into two independent spans. + [Fact] + public void A_gap_wider_than_the_budget_splits_the_span() + { + // 0..2 then 100..102 — a 98-byte hole, far past the 16-byte budget. + var plan = Plan( + T(0, "DB1.DBW0", S7DataType.UInt16), + T(1, "DB1.DBW100", S7DataType.UInt16)); + + plan.Spans.Count.ShouldBe(2); + plan.Spans[0].StartByte.ShouldBe(0); + plan.Spans[0].LengthBytes.ShouldBe(2); + plan.Spans[1].StartByte.ShouldBe(100); + plan.Spans[1].LengthBytes.ShouldBe(2); + } + + /// A hole exactly equal to the budget still merges — the bound is inclusive. + [Fact] + public void A_gap_exactly_equal_to_the_budget_still_merges() + { + // First tag ends at byte 2; the second starts at 18 => a gap of exactly 16. + var plan = Plan( + T(0, "DB1.DBW0", S7DataType.UInt16), + T(1, "DB1.DBW18", S7DataType.UInt16)); + + var span = plan.Spans.ShouldHaveSingleItem(); + span.StartByte.ShouldBe(0); + span.LengthBytes.ShouldBe(20); + } + + /// A zero gap budget merges strictly-adjacent tags but nothing with a hole in it. + [Fact] + public void A_zero_gap_budget_merges_only_strictly_adjacent_tags() + { + var plan = S7ReadSpanPlanner.Build( + [T(0, "DB1.DBW0", S7DataType.UInt16), + T(1, "DB1.DBW2", S7DataType.UInt16), + T(2, "DB1.DBW6", S7DataType.UInt16)], + maxGapBytes: 0, maxSpanBytes: 220); + + plan.Spans.Count.ShouldBe(2); + plan.Spans[0].StartByte.ShouldBe(0); + plan.Spans[0].LengthBytes.ShouldBe(4); + plan.Spans[1].StartByte.ShouldBe(6); + } + + /// The span cap breaks a contiguous run that would otherwise merge end-to-end. + [Fact] + public void The_span_cap_splits_a_run_that_would_otherwise_merge() + { + // Eight adjacent words = 16 bytes contiguous; a 6-byte cap admits 3 words per span. + var tags = Enumerable.Range(0, 8) + .Select(i => T(i, $"DB1.DBW{i * 2}", S7DataType.UInt16)) + .ToArray(); + + var plan = S7ReadSpanPlanner.Build(tags, maxGapBytes: 16, maxSpanBytes: 6); + + plan.Spans.Count.ShouldBe(3); + plan.Spans.Select(s => s.Members.Count).ShouldBe([3, 3, 2]); + plan.Spans.ShouldAllBe(s => s.LengthBytes <= 6); + // Every tag is still served — the cap splits, it never drops. + plan.Spans.SelectMany(s => s.Members).Select(m => m.OriginalIndex) + .OrderBy(i => i).ShouldBe(Enumerable.Range(0, 8)); + } + + /// One tag wider than the cap still forms its own span — a tag is never split. + [Fact] + public void A_single_tag_wider_than_the_cap_forms_its_own_span() + { + // A 200-char S7 STRING occupies 202 bytes; the cap is 8. + var plan = S7ReadSpanPlanner.Build( + [T(0, "DB1.DBB0", S7DataType.String, stringLength: 200)], + maxGapBytes: 16, maxSpanBytes: 8); + + var span = plan.Spans.ShouldHaveSingleItem(); + span.LengthBytes.ShouldBe(202); + span.Members.ShouldHaveSingleItem().WidthBytes.ShouldBe(202); + plan.LeftoverIndexes.ShouldBeEmpty(); + } + + /// An oversized span does not swallow the neighbour that follows it. + [Fact] + public void An_oversized_span_does_not_absorb_the_next_tag() + { + var plan = S7ReadSpanPlanner.Build( + [T(0, "DB1.DBB0", S7DataType.String, stringLength: 200), + T(1, "DB1.DBW202", S7DataType.UInt16)], + maxGapBytes: 16, maxSpanBytes: 8); + + plan.Spans.Count.ShouldBe(2); + plan.Spans[0].LengthBytes.ShouldBe(202); + plan.Spans[1].StartByte.ShouldBe(202); + } + + // ── Separation ─────────────────────────────────────────────────────────────────────── + + /// Two DBs at the same byte offsets are different address spaces and never merge. + [Fact] + public void Distinct_data_blocks_never_merge() + { + var plan = Plan( + T(0, "DB1.DBW0", S7DataType.UInt16), + T(1, "DB2.DBW2", S7DataType.UInt16)); + + plan.Spans.Count.ShouldBe(2); + plan.Spans.Select(s => s.DbNumber).OrderBy(n => n).ShouldBe([1, 2]); + } + + /// Different areas (M vs I vs Q) are different address spaces and never merge. + [Fact] + public void Distinct_areas_never_merge() + { + var plan = Plan( + T(0, "MW0", S7DataType.UInt16), + T(1, "IW2", S7DataType.UInt16), + T(2, "QW4", S7DataType.UInt16)); + + plan.Spans.Count.ShouldBe(3); + plan.Spans.Select(s => s.Area).ShouldBe([S7Area.Memory, S7Area.Input, S7Area.Output], ignoreOrder: true); + } + + /// + /// Timer and Counter tags are excluded from every span. Their ByteOffset holds the + /// DEVICE NUMBER, not a byte address, so grouping them with byte-addressed tags — or with + /// each other — would read from a completely unrelated place on the wire. + /// + [Fact] + public void Timer_and_Counter_tags_are_excluded_to_leftovers() + { + var plan = Plan( + T(0, "DB1.DBW0", S7DataType.UInt16), + T(1, "T1", S7DataType.Float64), + T(2, "C1", S7DataType.Int32), + T(3, "DB1.DBW2", S7DataType.UInt16)); + + plan.LeftoverIndexes.OrderBy(i => i).ShouldBe([1, 2]); + var span = plan.Spans.ShouldHaveSingleItem(); + span.Members.Select(m => m.OriginalIndex).ShouldBe([0, 3]); + } + + // ── Overlap ────────────────────────────────────────────────────────────────────────── + + /// + /// Two Bools at different bit positions in the SAME byte cost one byte between them — + /// overlap is a gap of zero, and the span covers exactly that one byte. + /// + [Fact] + public void Two_bools_in_the_same_byte_merge_into_one_single_byte_span() + { + var plan = Plan( + T(0, "DB1.DBX0.0", S7DataType.Bool), + T(1, "DB1.DBX0.7", S7DataType.Bool)); + + var span = plan.Spans.ShouldHaveSingleItem(); + span.StartByte.ShouldBe(0); + span.LengthBytes.ShouldBe(1); + span.Members.Count.ShouldBe(2); + span.Members.ShouldAllBe(m => m.RelativeOffset == 0); + span.Members.Select(m => m.Addr.BitOffset).ShouldBe([0, 7]); + } + + /// A word overlapping an already-open byte does not shrink the span's end. + [Fact] + public void An_overlapping_tag_extends_the_span_to_the_furthest_end() + { + // DBD0 covers 0..4; DBW2 sits INSIDE it. The span must stay 4 bytes, not shrink to 2+2. + var plan = Plan( + T(0, "DB1.DBD0", S7DataType.UInt32), + T(1, "DB1.DBW2", S7DataType.UInt16)); + + var span = plan.Spans.ShouldHaveSingleItem(); + span.StartByte.ShouldBe(0); + span.LengthBytes.ShouldBe(4); + span.Members.Select(m => m.RelativeOffset).ShouldBe([0, 2]); + } + + // ── The never-overshoot invariant ──────────────────────────────────────────────────── + + /// + /// Across a deliberately messy mixed batch, every span starts at its first member's byte + /// and ends at its furthest member's end — never one byte further. A span that overshot + /// could fault (area / DB overrun) on a read the per-tag path would have served. + /// + [Fact] + public void A_span_never_extends_past_its_last_members_end() + { + var plan = Plan( + T(0, "DB1.DBX0.3", S7DataType.Bool), + T(1, "DB1.DBB1", S7DataType.Byte), + T(2, "DB1.DBW2", S7DataType.UInt16), + T(3, "DB1.DBD8", S7DataType.Float32), + T(4, "DB1.DBB24", S7DataType.Int64), + T(5, "DB1.DBB36", S7DataType.String, stringLength: 10), + T(6, "DB1.DBW60", S7DataType.UInt16, arrayCount: 4), + T(7, "DB1.DBW200", S7DataType.UInt16), + T(8, "MW0", S7DataType.UInt16)); + + plan.Spans.ShouldNotBeEmpty(); + foreach (var span in plan.Spans) + { + span.Members.ShouldNotBeEmpty(); + span.StartByte.ShouldBe(span.Members.Min(m => m.Addr.ByteOffset)); + span.LengthBytes.ShouldBe(span.Members.Max(m => m.RelativeOffset + m.WidthBytes)); + // Every member must be wholly inside the block the span will read. + span.Members.ShouldAllBe(m => m.RelativeOffset >= 0 + && m.RelativeOffset + m.WidthBytes <= span.LengthBytes); + // RelativeOffset is the member's own byte, expressed against the span start. + span.Members.ShouldAllBe(m => m.RelativeOffset == m.Addr.ByteOffset - span.StartByte); + } + + // Nothing is lost or duplicated between spans and leftovers. + plan.Spans.SelectMany(s => s.Members).Select(m => m.OriginalIndex) + .Concat(plan.LeftoverIndexes) + .OrderBy(i => i) + .ShouldBe(Enumerable.Range(0, 9)); + } + + // ── The off switch ─────────────────────────────────────────────────────────────────── + + /// A negative gap budget disables grouping entirely — every tag becomes a leftover. + [Fact] + public void A_negative_max_gap_disables_grouping_entirely() + { + var plan = S7ReadSpanPlanner.Build( + [T(0, "DB1.DBW0", S7DataType.UInt16), + T(1, "DB1.DBW2", S7DataType.UInt16), + T(2, "T1", S7DataType.Float64)], + maxGapBytes: -1, maxSpanBytes: 220); + + plan.Spans.ShouldBeEmpty(); + plan.LeftoverIndexes.OrderBy(i => i).ShouldBe([0, 1, 2]); + } + + /// An empty batch plans nothing rather than throwing. + [Fact] + public void An_empty_batch_produces_an_empty_plan() + { + var plan = Plan(); + + plan.Spans.ShouldBeEmpty(); + plan.LeftoverIndexes.ShouldBeEmpty(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs new file mode 100644 index 00000000..4f8361f6 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs @@ -0,0 +1,373 @@ +using S7.Net; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using S7NetDataType = global::S7.Net.DataType; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests; + +/// +/// Falsification gate for span-grouped S7 reads. Every tag must decode to the SAME value, the +/// same CLR box type and the same status code whether it was served by a wire request of its +/// own (the pre-existing per-tag path, reachable by setting +/// SpanReadMaxGapBytes = -1) or by a block read shared with its neighbours. +/// +/// +/// +/// The two drivers read the same in-memory PLC image through the same fake, so any +/// difference between the two result sets is a defect in the grouping path — there is no +/// other variable. Grouping is proven to have actually engaged by +/// : without that +/// assertion this whole file would pass vacuously if the planner ever produced only +/// leftovers, because "leftover" IS the per-tag path. +/// +/// +/// The fake serves ReadBytesAsync (block reads — the span path, plus the wide / +/// array / Timer-Counter tags that always read blocks) and ReadAsync(string) (the +/// narrow per-tag path) from ONE byte image per (area, DB), decoding the latter +/// exactly as S7.Net does: DBX boxes a bool, DBB a byte, +/// DBW a ushort, DBD a uint. +/// +/// +[Trait("Category", "Unit")] +public sealed class S7SpanReadParityTests +{ + /// OPC UA BadDeviceFailure — what a device-layer read fault stamps. + private const uint StatusBadDeviceFailure = 0x808B0000u; + /// OPC UA BadNodeIdUnknown — what an unresolvable reference stamps. + private const uint StatusBadNodeIdUnknown = 0x80340000u; + + // ── The mixed tag set ──────────────────────────────────────────────────────────────── + + /// + /// A deliberately awkward batch: two Bools sharing one byte (bit 0 and bit 7), a Byte, a + /// word pair, three dwords behind a small hole, a wide Int64, an S7 STRING, a UInt16 array, + /// a far-away word past the gap budget, a second AREA, and a Counter (whose offset is a + /// device number, so it can never join a byte span). + /// + private static IReadOnlyList MixedTags() => S7RawTags.Entries( + new S7TagDefinition("Bit0", "DB1.DBX0.0", S7DataType.Bool, Writable: false), + new S7TagDefinition("Bit7", "DB1.DBX0.7", S7DataType.Bool, Writable: false), + new S7TagDefinition("Byte1", "DB1.DBB1", S7DataType.Byte, Writable: false), + new S7TagDefinition("U16", "DB1.DBW2", S7DataType.UInt16, Writable: false), + new S7TagDefinition("I16", "DB1.DBW4", S7DataType.Int16, Writable: false), + new S7TagDefinition("U32", "DB1.DBD8", S7DataType.UInt32, Writable: false), + new S7TagDefinition("I32", "DB1.DBD12", S7DataType.Int32, Writable: false), + new S7TagDefinition("F32", "DB1.DBD16", S7DataType.Float32, Writable: false), + new S7TagDefinition("I64", "DB1.DBB24", S7DataType.Int64, Writable: false), + new S7TagDefinition("Str", "DB1.DBB36", S7DataType.String, Writable: false, StringLength: 10), + new S7TagDefinition("Arr", "DB1.DBW60", S7DataType.UInt16, Writable: false, ArrayCount: 4), + new S7TagDefinition("Far", "DB1.DBW200", S7DataType.UInt16, Writable: false), + new S7TagDefinition("Mem", "MW0", S7DataType.UInt16, Writable: false), + new S7TagDefinition("Cnt", "C1", S7DataType.Int32, Writable: false)); + + /// Read references, in the order the batch presents them. + private static readonly string[] MixedRefs = + [ + "Bit0", "Bit7", "Byte1", "U16", "I16", "U32", "I32", "F32", + "Str", "I64", "Arr", "Far", "Mem", "Cnt", + ]; + + /// Only the narrow, byte-addressed DB1 tags — every one of them lands in one span. + private static IReadOnlyList NarrowTags() => S7RawTags.Entries( + new S7TagDefinition("A", "DB1.DBW0", S7DataType.UInt16, Writable: false), + new S7TagDefinition("B", "DB1.DBW2", S7DataType.UInt16, Writable: false), + new S7TagDefinition("C", "DB1.DBW4", S7DataType.UInt16, Writable: false)); + + // ── Parity ─────────────────────────────────────────────────────────────────────────── + + /// + /// THE gate: span-grouped reads and per-tag reads produce identical snapshots — same + /// value, same box type, same status — for every tag in the mixed batch. + /// + [Fact] + public async Task Span_grouped_reads_match_the_per_tag_path_snapshot_for_snapshot() + { + var images = Images(); + var ct = TestContext.Current.CancellationToken; + + var grouping = new ImageS7PlcFactory(images); + var perTag = new ImageS7PlcFactory(images); + + using var groupedDriver = new S7Driver(Options(MixedTags(), maxGap: 16), "s7-grouped", grouping); + using var perTagDriver = new S7Driver(Options(MixedTags(), maxGap: -1), "s7-per-tag", perTag); + await groupedDriver.InitializeAsync("{}", ct); + await perTagDriver.InitializeAsync("{}", ct); + + var grouped = await groupedDriver.ReadAsync(MixedRefs, ct); + var individual = await perTagDriver.ReadAsync(MixedRefs, ct); + + grouped.Count.ShouldBe(MixedRefs.Length); + for (var i = 0; i < MixedRefs.Length; i++) + { + var name = MixedRefs[i]; + grouped[i].StatusCode.ShouldBe(individual[i].StatusCode, $"status differs for '{name}'"); + grouped[i].StatusCode.ShouldBe(0u, $"'{name}' should have read Good"); + grouped[i].Value.ShouldNotBeNull($"'{name}' returned no value"); + grouped[i].Value!.GetType().ShouldBe(individual[i].Value!.GetType(), $"box type differs for '{name}'"); + grouped[i].Value.ShouldBe(individual[i].Value, $"value differs for '{name}'"); + } + + // The per-tag control really did take the per-tag path (no grouping happened there): its + // only block reads are the four tags that ALWAYS read blocks (Int64, String, array, Counter). + perTag.ReadBytesCalls.ShouldBe(4); + perTag.StringReadCalls.ShouldBe(10); + } + + /// + /// Proves grouping actually engaged. Without this the parity test above would pass + /// vacuously if the planner degenerated to all-leftovers, since a leftover IS a per-tag + /// read. + /// + /// + /// 14 tags collapse to 4 wire requests: ONE block read covering the eleven DB1 tags from + /// byte 0 to byte 68, the Counter's own unavoidable block read, and two per-address string + /// reads — the far DB1 word (past the gap budget) and the lone Memory word, each of which + /// the driver deliberately leaves on the per-tag path because a single-member narrow span + /// saves no round-trip. + /// + [Fact] + public async Task Grouping_issues_far_fewer_wire_requests_than_there_are_tags() + { + var ct = TestContext.Current.CancellationToken; + var factory = new ImageS7PlcFactory(Images()); + using var driver = new S7Driver(Options(MixedTags(), maxGap: 16), "s7-wire-count", factory); + await driver.InitializeAsync("{}", ct); + + _ = await driver.ReadAsync(MixedRefs, ct); + + (factory.ReadBytesCalls + factory.StringReadCalls).ShouldBeLessThan(MixedRefs.Length); + factory.ReadBytesCalls.ShouldBeLessThan(MixedRefs.Length); + factory.ReadBytesCalls.ShouldBe(2); + // The nine narrow tags inside the big span issue NO string reads — only the two tags that + // ended up alone in a span of their own do. + factory.StringReadCalls.ShouldBe(2); + } + + // ── Failure parity ─────────────────────────────────────────────────────────────────── + + /// + /// A failed span stamps EVERY one of its members with exactly the status the per-tag path + /// would have produced for that exception, and degrades health identically. A span read is + /// one wire request serving many tags, so a fault that used to affect one tag now affects + /// all of them — they must all be told the same thing. + /// + [Fact] + public async Task A_span_read_failure_degrades_every_member_like_the_per_tag_path() + { + var ct = TestContext.Current.CancellationToken; + // ErrorCode.ReadData with a message that is NOT access-denied ("not allowed") — a genuine + // device-layer fault, which the driver maps to BadDeviceFailure + Degraded. + var fault = new PlcException(ErrorCode.ReadData, "S7 CPU reported a device fault"); + + var groupingFactory = new ImageS7PlcFactory(Images()) { ReadFault = fault }; + var perTagFactory = new ImageS7PlcFactory(Images()) { ReadFault = fault }; + + using var groupedDriver = new S7Driver(Options(NarrowTags(), maxGap: 16), "s7-grouped-fail", groupingFactory); + using var perTagDriver = new S7Driver(Options(NarrowTags(), maxGap: -1), "s7-per-tag-fail", perTagFactory); + await groupedDriver.InitializeAsync("{}", ct); + await perTagDriver.InitializeAsync("{}", ct); + + string[] refs = ["A", "B", "C"]; + var grouped = await groupedDriver.ReadAsync(refs, ct); + var individual = await perTagDriver.ReadAsync(refs, ct); + + // Every member of the failed span — not just the first — is stamped Bad. + grouped.Count.ShouldBe(3); + grouped.ShouldAllBe(s => s.StatusCode == StatusBadDeviceFailure); + grouped.ShouldAllBe(s => s.Value == null); + grouped.Select(s => s.StatusCode).ShouldBe(individual.Select(s => s.StatusCode)); + + groupedDriver.GetHealth().State.ShouldBe(DriverState.Degraded); + groupedDriver.GetHealth().State.ShouldBe(perTagDriver.GetHealth().State); + + // One failed block read, not three — the span path fails once for the whole group. + groupingFactory.ReadBytesCalls.ShouldBe(1); + } + + /// + /// Unresolved references still stamp BadNodeIdUnknown and never enter a span, and + /// the resolvable tags around them still read Good. + /// + [Fact] + public async Task An_unresolved_reference_is_still_BadNodeIdUnknown_under_grouping() + { + var ct = TestContext.Current.CancellationToken; + var factory = new ImageS7PlcFactory(Images()); + using var driver = new S7Driver(Options(NarrowTags(), maxGap: 16), "s7-unknown", factory); + await driver.InitializeAsync("{}", ct); + + var results = await driver.ReadAsync(["A", "NoSuchTag", "C"], ct); + + results[0].StatusCode.ShouldBe(0u); + results[1].StatusCode.ShouldBe(StatusBadNodeIdUnknown); + results[1].Value.ShouldBeNull(); + results[2].StatusCode.ShouldBe(0u); + } + + // ── Configurability ────────────────────────────────────────────────────────────────── + + /// + /// The grouping knobs must be reachable from a deployed driver config. ParseOptions + /// maps an explicit DTO rather than deserialising , so a knob + /// that is not mapped there is silently pinned to its default forever — which would leave + /// the documented "-1 disables grouping" escape hatch unreachable in production. + /// + [Fact] + public void The_span_read_knobs_bind_from_driver_config_json() + { + var configured = S7DriverFactoryExtensions.ParseOptions( + "s7-cfg", + """{"host":"10.0.0.1","spanReadMaxGapBytes":-1,"spanReadMaxSpanBytes":64}"""); + + configured.SpanReadMaxGapBytes.ShouldBe(-1); + configured.SpanReadMaxSpanBytes.ShouldBe(64); + + // Omitted keys keep the shipped defaults rather than collapsing to zero. + var defaulted = S7DriverFactoryExtensions.ParseOptions("s7-cfg", """{"host":"10.0.0.1"}"""); + defaulted.SpanReadMaxGapBytes.ShouldBe(S7DriverOptions.DefaultSpanReadMaxGapBytes); + defaulted.SpanReadMaxSpanBytes.ShouldBe(S7DriverOptions.DefaultSpanReadMaxSpanBytes); + } + + // ── Fixture ────────────────────────────────────────────────────────────────────────── + + /// Driver options over the given tags with the span-grouping knobs under test. + private static S7DriverOptions Options(IReadOnlyList tags, int maxGap) => new() + { + Host = "192.0.2.1", + Timeout = TimeSpan.FromMilliseconds(250), + Probe = new S7ProbeOptions { Enabled = false }, + SpanReadMaxGapBytes = maxGap, + SpanReadMaxSpanBytes = 220, + RawTags = tags, + }; + + /// + /// The in-memory PLC images the fake serves, one byte array per (area, DB). Filled + /// with a deterministic non-repeating pattern so no tag reads a vacuous all-zero value, then + /// seeded with a well-formed S7 STRING field at DB1 byte 36 ([maxLen][curLen][chars…], + /// which S7String.FromByteArray validates). + /// + private static Dictionary<(S7NetDataType Area, int Db), byte[]> Images() + { + var db1 = new byte[256]; + for (var i = 0; i < db1.Length; i++) db1[i] = (byte)((i * 37 + 11) & 0xFF); + + db1[36] = 10; // declared max length + db1[37] = 5; // current length + var chars = "HELLO"u8.ToArray(); + chars.CopyTo(db1, 38); + for (var i = 38 + chars.Length; i < 36 + 12; i++) db1[i] = 0x20; // reserved padding + + var mem = new byte[64]; + for (var i = 0; i < mem.Length; i++) mem[i] = (byte)((i * 53 + 7) & 0xFF); + + var counters = new byte[64]; + for (var i = 0; i < counters.Length; i++) counters[i] = (byte)((i * 29 + 3) & 0xFF); + + return new Dictionary<(S7NetDataType, int), byte[]> + { + [(S7NetDataType.DataBlock, 1)] = db1, + [(S7NetDataType.Memory, 0)] = mem, + [(S7NetDataType.Counter, 0)] = counters, + }; + } + + /// + /// Fake serving both read shapes from one shared byte image per + /// (area, DB), and counting each shape so a test can prove which path ran. Counters + /// live on the FACTORY (not the connection) so a reconnect cannot reset them. + /// + private sealed class ImageS7PlcFactory(Dictionary<(S7NetDataType Area, int Db), byte[]> images) : IS7PlcFactory + { + /// The shared per-area byte images both read shapes decode from. + public Dictionary<(S7NetDataType Area, int Db), byte[]> Images { get; } = images; + + /// Number of block reads issued — one per planned span, plus the always-block tags. + public int ReadBytesCalls; + + /// Number of per-address string reads issued — the un-grouped narrow path. + public int StringReadCalls; + + /// When set, every read throws this instead of serving the image. + public Exception? ReadFault { get; init; } + + /// + public IS7Plc Create(S7CpuType cpuType, string host, int port, short rack, short slot, TimeSpan timeout) + => new ImageS7Plc(this); + + private sealed class ImageS7Plc(ImageS7PlcFactory owner) : IS7Plc + { + /// + public bool IsConnected { get; private set; } + + /// + public Task OpenAsync(CancellationToken ct) { IsConnected = true; return Task.CompletedTask; } + + /// + public void Close() => IsConnected = false; + + /// + public Task ReadBytesAsync(S7NetDataType area, int db, int startByteAdr, int count, CancellationToken ct) + { + // Counted BEFORE the injected fault so a test can assert how many block reads were + // ATTEMPTED — a failure test that counted only successes would read zero. + Interlocked.Increment(ref owner.ReadBytesCalls); + if (owner.ReadFault is { } fault) throw fault; + var image = owner.Images[(area, db)]; + if (startByteAdr + count > image.Length) + { + throw new PlcException(ErrorCode.ReadData, + $"fake image overrun: {area} db{db} @{startByteAdr}+{count} exceeds {image.Length}"); + } + return Task.FromResult(image[startByteAdr..(startByteAdr + count)]); + } + + /// + public Task ReadAsync(string address, CancellationToken ct) + { + Interlocked.Increment(ref owner.StringReadCalls); + if (owner.ReadFault is { } fault) throw fault; + var addr = S7AddressParser.Parse(address); + var image = owner.Images[(ToS7NetArea(addr.Area), addr.DbNumber)]; + var at = addr.ByteOffset; + + // Box exactly as S7.Net's string read does — the size suffix picks the type. + return Task.FromResult(addr.Size switch + { + S7Size.Bit => (object)((image[at] & (1 << addr.BitOffset)) != 0), + S7Size.Byte => image[at], + S7Size.Word => (ushort)((image[at] << 8) | image[at + 1]), + S7Size.DWord => ((uint)image[at] << 24) | ((uint)image[at + 1] << 16) + | ((uint)image[at + 2] << 8) | image[at + 3], + _ => throw new NotSupportedException($"fake cannot serve size {addr.Size}"), + }); + } + + /// Mirrors the driver's own area mapping so both read shapes hit the same image. + private static S7NetDataType ToS7NetArea(S7Area area) => area switch + { + S7Area.DataBlock => S7NetDataType.DataBlock, + S7Area.Memory => S7NetDataType.Memory, + S7Area.Input => S7NetDataType.Input, + S7Area.Output => S7NetDataType.Output, + S7Area.Timer => S7NetDataType.Timer, + S7Area.Counter => S7NetDataType.Counter, + _ => throw new NotSupportedException($"fake cannot map area {area}"), + }; + + /// + public Task WriteAsync(string address, object value, CancellationToken ct) => Task.CompletedTask; + + /// + public Task WriteBytesAsync(S7NetDataType area, int db, int startByteAdr, byte[] value, CancellationToken ct) + => Task.CompletedTask; + + /// + public Task ReadStatusAsync(CancellationToken ct) => Task.CompletedTask; + + /// + public void Dispose() => IsConnected = false; + } + } +}