From 2ef3cd0df25f80f6a18297f6aaea8cc2489b48ab Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 15:24:38 -0400 Subject: [PATCH] feat(modbus): block-read coalescing on by default; coils/DI/BitInRegister/String fold U2: MaxReadGap defaults to 8 and 0 now means adjacent-only rather than off; a new DisableReadCoalescing switch (factory DTO + AdminUI form) turns it off entirely. The planner also merges Coils/DiscreteInputs (FC01/FC02, gap in bits), String scalars and BitInRegister tags - sixteen bit tags on one status word are one FC03 instead of sixteen. Auto-prohibition and its re-probe / bisection use the region's own function code (a coil range was re-probed with FC04). Refs #741 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../ModbusDriverOptions.cs | 25 ++- .../ModbusDriver.cs | 113 ++++++++----- .../ModbusDriverFactoryExtensions.cs | 5 +- .../Drivers/Forms/ModbusDriverForm.razor | 13 +- .../ModbusCoalescingDefaultsTests.cs | 155 ++++++++++++++++++ .../ModbusCoalescingTests.cs | 6 +- .../ScriptedTransport.cs | 8 + 7 files changed, 269 insertions(+), 56 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingDefaultsTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusDriverOptions.cs index 39a4104e..9d6b5479 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusDriverOptions.cs @@ -107,14 +107,25 @@ public sealed class ModbusDriverOptions public TimeSpan? AutoProhibitReprobeInterval { get; init; } = null; /// - /// Block-read coalescing budget. When non-zero, the read planner combines tags - /// in the same (UnitId, Region) group whose addresses are at most this many registers - /// apart into a single FC03/FC04/FC01/FC02 read. The sliced response is then dispatched - /// back to per-tag values. Default 0 = no coalescing — every tag gets its own - /// PDU (preserves the previous behaviour). Typical opt-in values are 5..32 — large enough - /// to bridge a few unused registers, small enough to avoid trampling protected holes. + /// Block-read coalescing budget. The read planner combines tags in the same (UnitId, Region) + /// group whose addresses are at most this many registers (Coils / DiscreteInputs: bits) apart + /// into a single FC03/FC04/FC01/FC02 read, then slices the response back into per-tag values. + /// 0 coalesces only adjacent or overlapping tags (several BitInRegister tags on one + /// status word still fold into one read). Default 8. A range the device refuses is + /// auto-prohibited and read per tag, so a protected hole costs one extra round trip, not data. To + /// turn coalescing off entirely set , or exempt one tag with + /// its coalesceProhibited key. /// - public ushort MaxReadGap { get; init; } = 0; + public ushort MaxReadGap { get; init; } = DefaultMaxReadGap; + + /// The default (shared by the factory DTO and the AdminUI form). + public const ushort DefaultMaxReadGap = 8; + + /// + /// When true, every tag is read with its own PDU (no block coalescing at all) — for a device + /// that mis-answers merged reads in a way the auto-prohibition cannot catch. Default false. + /// + public bool DisableReadCoalescing { get; init; } = false; /// /// PLC family hint that drives the parser's family-native branch. When set to a diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index eedfb70b..cc252539 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -727,7 +727,7 @@ public sealed class ModbusDriver var results = new DataValueSnapshot[fullReferences.Count]; var batch = new BatchOutcome(); - var coalesced = _options.MaxReadGap > 0 + var coalesced = !_options.DisableReadCoalescing ? await ReadCoalescedAsync(transport, table, fullReferences, results, now, batch, cancellationToken).ConfigureAwait(false) : new HashSet(); @@ -1083,11 +1083,10 @@ public sealed class ModbusDriver private async Task StraightReprobeAsync(IModbusTransport transport, (byte Unit, ModbusRegion Region, ushort Start, ushort End) key, CancellationToken ct) { - var fc = key.Region == ModbusRegion.HoldingRegisters ? (byte)0x03 : (byte)0x04; var qty = (ushort)(key.End - key.Start + 1); try { - _ = await ReadRegisterBlockAsync(transport, key.Unit, fc, key.Start, qty, ct).ConfigureAwait(false); + _ = await ReadRangeAsync(transport, key.Unit, key.Region, key.Start, qty, ct).ConfigureAwait(false); lock (_autoProhibitedLock) _autoProhibited.Remove(key); LogProhibitionCleared(key.Unit, key.Region, key.Start, key.End); } @@ -1107,13 +1106,12 @@ public sealed class ModbusDriver private async Task BisectAndReprobeAsync(IModbusTransport transport, (byte Unit, ModbusRegion Region, ushort Start, ushort End) key, CancellationToken ct) { - var fc = key.Region == ModbusRegion.HoldingRegisters ? (byte)0x03 : (byte)0x04; var mid = (ushort)((key.Start + key.End) / 2); var leftEnd = mid; var rightStart = (ushort)(mid + 1); - var leftFailed = await ProbeFailsAsync(transport, fc, key.Unit, key.Start, leftEnd, ct).ConfigureAwait(false); - var rightFailed = await ProbeFailsAsync(transport, fc, key.Unit, rightStart, key.End, ct).ConfigureAwait(false); + var leftFailed = await ProbeFailsAsync(transport, key.Region, key.Unit, key.Start, leftEnd, ct).ConfigureAwait(false); + var rightFailed = await ProbeFailsAsync(transport, key.Region, key.Unit, rightStart, key.End, ct).ConfigureAwait(false); lock (_autoProhibitedLock) { @@ -1149,13 +1147,13 @@ public sealed class ModbusDriver } } - private async Task ProbeFailsAsync(IModbusTransport transport, byte fc, byte unit, + private async Task ProbeFailsAsync(IModbusTransport transport, ModbusRegion region, byte unit, ushort start, ushort end, CancellationToken ct) { var qty = (ushort)(end - start + 1); try { - _ = await ReadRegisterBlockAsync(transport, unit, fc, start, qty, ct).ConfigureAwait(false); + _ = await ReadRangeAsync(transport, unit, region, start, qty, ct).ConfigureAwait(false); return false; } catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } @@ -1163,12 +1161,15 @@ public sealed class ModbusDriver } /// - /// Block-read coalescing planner. Groups eligible tags by (UnitId, Region), sorts - /// by start address, and merges adjacent / near-adjacent (gap ≤ MaxReadGap) into single - /// FC03/FC04 reads. Per-block: emit one Modbus PDU, slice the response back into per-tag - /// values, populate and the WriteOnChangeOnly cache. Returns - /// the set of indices the planner handled — the - /// caller falls back to the per-tag path for the rest (arrays, coils, prohibited, unknown). + /// Block-read coalescing planner. Groups eligible tags by (UnitId, Region), sorts by start address, + /// and merges adjacent / near-adjacent tags (gap ≤ , 0 = + /// adjacent or overlapping only) into one read per block: FC03/FC04 for the register regions (scalar + /// numeric, String and BitInRegister tags — several bit tags on ONE status word fold into + /// a single read of it), FC01/FC02 for Coils / DiscreteInputs (gap counted in bits). Per block: emit + /// one PDU, slice the response back into per-tag values, populate and the + /// WriteOnChangeOnly cache. Returns the set of indices the planner + /// handled — the caller falls back to the per-tag path for the rest (arrays, lone tags, prohibited, + /// unknown, refused ranges). /// private async Task> ReadCoalescedAsync( IModbusTransport transport, @@ -1179,9 +1180,8 @@ public sealed class ModbusDriver BatchOutcome batch, CancellationToken ct) { - // Eligible: known tag, register region (HoldingRegisters or InputRegisters), scalar - // (no array, no string), not CoalesceProhibited, not BitInRegister. - var eligible = new List<(int Index, string Ref, ModbusTagDefinition Tag)>(); + // Eligible: known scalar tag, not CoalesceProhibited, with a region/type pair the codec can slice. + var eligible = new List<(int Index, ModbusTagDefinition Tag)>(); for (var i = 0; i < fullReferences.Count; i++) { if (!table.TryGetValue(fullReferences[i], out var tag)) continue; @@ -1190,9 +1190,7 @@ public sealed class ModbusDriver // throw for a tag that slipped through — one bad tag would otherwise fail the whole batch. if (!ModbusTagDefinitionFactory.IsRegionTypeCompatible(tag.Region, tag.DataType)) continue; if (tag.ArrayCount.HasValue) continue; - if (tag.Region is not (ModbusRegion.HoldingRegisters or ModbusRegion.InputRegisters)) continue; - if (tag.DataType is ModbusDataType.String or ModbusDataType.BitInRegister) continue; - eligible.Add((i, fullReferences[i], tag)); + eligible.Add((i, tag)); } if (eligible.Count == 0) return new HashSet(); @@ -1201,42 +1199,43 @@ public sealed class ModbusDriver // Group by (UnitId, Region) — coalescing across slaves or regions is unsafe. foreach (var group in eligible.GroupBy(e => (Unit: ResolveUnitId(e.Tag), e.Tag.Region))) { - var fc = group.Key.Region == ModbusRegion.HoldingRegisters ? (byte)0x03 : (byte)0x04; - var cap = _options.MaxRegistersPerRead == 0 ? (ushort)125 : _options.MaxRegistersPerRead; + var isBits = IsBitRegion(group.Key.Region); + var cap = isBits + ? (_options.MaxCoilsPerRead == 0 ? 2000 : _options.MaxCoilsPerRead) + : (_options.MaxRegistersPerRead == 0 ? 125 : _options.MaxRegistersPerRead); var sorted = group.OrderBy(e => e.Tag.Address).ToList(); - // Build merged blocks. A "block" is (start, lastEnd, members[]) where lastEnd is - // the inclusive end address of the last tag's register span. A new tag joins the - // block if its start ≤ lastEnd + 1 + MaxReadGap AND the resulting span ≤ cap. - var blocks = new List<(ushort Start, ushort End, List<(int Index, ModbusTagDefinition Tag)> Members)>(); - foreach (var (idx, _, tag) in sorted) + // Build merged blocks. A "block" is (start, lastEnd, members[]) where lastEnd is the inclusive + // end of the widest member span seen so far. A new tag joins the block if its start ≤ lastEnd + + // 1 + MaxReadGap AND the resulting span ≤ cap AND the span crosses no auto-prohibited range. + var blocks = new List<(int Start, int End, List<(int Index, ModbusTagDefinition Tag)> Members)>(); + foreach (var (idx, tag) in sorted) { - var tagStart = tag.Address; - var tagEnd = (ushort)(tag.Address + RegisterCount(tag) - 1); + var tagStart = (int)tag.Address; + var tagEnd = tagStart + (isBits ? 1 : RegisterCount(tag)) - 1; + if (tagEnd > ushort.MaxValue || tagEnd - tagStart + 1 > cap) continue; // per-tag path chunks / refuses it if (blocks.Count > 0) { var last = blocks[^1]; var gap = tagStart - last.End - 1; var newEnd = Math.Max(tagEnd, last.End); var newSpan = newEnd - last.Start + 1; - var crossesProhibition = RangeIsAutoProhibited(group.Key.Unit, group.Key.Region, last.Start, (ushort)newEnd); + var crossesProhibition = RangeIsAutoProhibited(group.Key.Unit, group.Key.Region, (ushort)last.Start, (ushort)newEnd); if (gap <= _options.MaxReadGap && newSpan <= cap && !crossesProhibition) { last.Members.Add((idx, tag)); - blocks[^1] = (last.Start, (ushort)newEnd, last.Members); + blocks[^1] = (last.Start, newEnd, last.Members); continue; } } blocks.Add((tagStart, tagEnd, new List<(int, ModbusTagDefinition)> { (idx, tag) })); } - // Issue one PDU per block. On a Modbus-level exception (illegal data address / - // protected register), record the range as auto-prohibited, leave the - // member indices UNhandled, and let the per-tag fallback in ReadAsync read each - // surviving address individually. On transport-level failure (timeout / socket - // drop) mark members Bad and short-circuit the per-tag fallback (hitting the - // dead socket again won't help). Bisection narrows the prohibition over - // subsequent re-probe ticks. + // Issue one PDU per block. On a device REFUSAL (illegal data address / protected register), + // record the range as auto-prohibited, leave the member indices UNhandled, and let the per-tag + // fallback in ReadAsync read each surviving address individually. On a transport-level failure + // (timeout / socket drop) mark members Bad and short-circuit the per-tag fallback (hitting the + // dead socket again won't help). Bisection narrows the prohibition over subsequent re-probe ticks. foreach (var block in blocks) { if (block.Members.Count == 1) @@ -1246,6 +1245,7 @@ public sealed class ModbusDriver continue; } + var start = (ushort)block.Start; var qty = (ushort)(block.End - block.Start + 1); if (batch.IsFailedFast(group.Key.Unit)) { @@ -1259,7 +1259,7 @@ public sealed class ModbusDriver byte[] data; try { - data = await ReadRegisterBlockAsync(transport, group.Key.Unit, fc, block.Start, qty, ct).ConfigureAwait(false); + data = await ReadRangeAsync(transport, group.Key.Unit, group.Key.Region, start, qty, ct).ConfigureAwait(false); } catch (OperationCanceledException) when (ct.IsCancellationRequested) { @@ -1271,7 +1271,7 @@ public sealed class ModbusDriver // as auto-prohibited and leave the members UNhandled so the per-tag fallback reads each // surviving address individually. The device answered, so health is not degraded. batch.Answered = true; - RecordAutoProhibition(group.Key.Unit, group.Key.Region, block.Start, block.End); + RecordAutoProhibition(group.Key.Unit, group.Key.Region, start, (ushort)block.End); continue; } catch (Exception ex) @@ -1292,12 +1292,21 @@ public sealed class ModbusDriver batch.Answered = true; foreach (var (idx, tag) in block.Members) { - var sliceOffsetBytes = (tag.Address - block.Start) * 2; - var sliceLenBytes = RegisterCount(tag) * 2; handled.Add(idx); try { - var value = Decode(() => DecodeRegister(data.AsSpan(sliceOffsetBytes, sliceLenBytes), tag), tag); + object value; + if (isBits) + { + var bit = tag.Address - block.Start; + value = ((data[bit / 8] >> (bit % 8)) & 0x01) == 1; + } + else + { + var sliceOffsetBytes = (tag.Address - block.Start) * 2; + var sliceLenBytes = RegisterCount(tag) * 2; + value = Decode(() => DecodeRegister(data.AsSpan(sliceOffsetBytes, sliceLenBytes), tag), tag); + } results[idx] = new DataValueSnapshot(value, 0u, timestamp, timestamp); InvalidateWriteCacheIfDiverged(fullReferences[idx], value); } @@ -1313,6 +1322,24 @@ public sealed class ModbusDriver return handled; } + private static bool IsBitRegion(ModbusRegion region) => region is ModbusRegion.Coils or ModbusRegion.DiscreteInputs; + + private static byte ReadFunctionCode(ModbusRegion region) => region switch + { + ModbusRegion.Coils => 0x01, + ModbusRegion.DiscreteInputs => 0x02, + ModbusRegion.HoldingRegisters => 0x03, + _ => 0x04, + }; + + /// Reads one contiguous range of the region with the matching function code — registers as + /// big-endian bytes, bits as an LSB-first bitmap. + private Task ReadRangeAsync( + IModbusTransport transport, byte unit, ModbusRegion region, ushort start, ushort qty, CancellationToken ct) + => IsBitRegion(region) + ? ReadBitBlockAsync(transport, unit, ReadFunctionCode(region), start, qty, ct) + : ReadRegisterBlockAsync(transport, unit, ReadFunctionCode(region), start, qty, ct); + private void InvalidateWriteCacheIfDiverged(string fullRef, object value) { if (!_options.WriteOnChangeOnly) return; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs index 31e33d69..9aa8e0d7 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriverFactoryExtensions.cs @@ -86,7 +86,8 @@ public static class ModbusDriverFactoryExtensions UseFC16ForSingleRegisterWrites = dto.UseFC16ForSingleRegisterWrites ?? false, DisableFC23 = dto.DisableFC23 ?? false, WriteOnChangeOnly = dto.WriteOnChangeOnly ?? false, - MaxReadGap = dto.MaxReadGap ?? 0, + MaxReadGap = dto.MaxReadGap ?? ModbusDriverOptions.DefaultMaxReadGap, + DisableReadCoalescing = dto.DisableReadCoalescing ?? false, Family = dto.Family is null ? ModbusFamily.Generic : ParseEnum(dto.Family, "", driverInstanceId, "Family"), MelsecSubFamily = dto.MelsecSubFamily is null ? MelsecFamily.Q_L_iQR @@ -171,6 +172,8 @@ public static class ModbusDriverFactoryExtensions public bool? WriteOnChangeOnly { get; init; } /// Gets or sets the maximum gap between register addresses for coalescing. public ushort? MaxReadGap { get; init; } + /// Gets or sets a value indicating whether block-read coalescing is turned off entirely. + public bool? DisableReadCoalescing { get; init; } /// Gets or sets the Modbus family type. public string? Family { get; init; } /// Gets or sets the Melsec subfamily. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor index 0c34a768..a46a255e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor @@ -80,7 +80,13 @@
-
0 = no coalescing. Typical 5–32.
+
Default 8. 0 = adjacent tags only. Refused ranges fall back per tag.
+
+
+
+ + +
@@ -311,7 +317,8 @@ public int MaxRegistersPerRead { get; set; } = 125; public int MaxRegistersPerWrite { get; set; } = 123; public int MaxCoilsPerRead { get; set; } = 2000; - public int MaxReadGap { get; set; } = 0; + public int MaxReadGap { get; set; } = ModbusDriverOptions.DefaultMaxReadGap; + public bool DisableReadCoalescing { get; set; } = false; // Write options public bool UseFC15ForSingleCoilWrites { get; set; } = false; @@ -357,6 +364,7 @@ MaxRegistersPerWrite = o.MaxRegistersPerWrite, MaxCoilsPerRead = o.MaxCoilsPerRead, MaxReadGap = o.MaxReadGap, + DisableReadCoalescing = o.DisableReadCoalescing, UseFC15ForSingleCoilWrites = o.UseFC15ForSingleCoilWrites, UseFC16ForSingleRegisterWrites = o.UseFC16ForSingleRegisterWrites, DisableFC23 = o.DisableFC23, @@ -399,6 +407,7 @@ AutoProhibitReprobeInterval = AutoProhibitReprobeIntervalSeconds > 0 ? TimeSpan.FromSeconds(AutoProhibitReprobeIntervalSeconds) : null, MaxReadGap = (ushort)Math.Clamp(MaxReadGap, 0, 65535), + DisableReadCoalescing = DisableReadCoalescing, Family = Family, MelsecSubFamily = MelsecSubFamily, Transport = Transport, diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingDefaultsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingDefaultsTests.cs new file mode 100644 index 00000000..45940c37 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingDefaultsTests.cs @@ -0,0 +1,155 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests; + +/// +/// Review 07 U2: block-read coalescing is ON by default (gap 8), and it covers coils, discrete inputs, +/// BitInRegister and String tags — 16 bit tags on one status word used to cost 16 reads of +/// the same register, and 64 adjacent coils 64 FC01s. +/// +[Trait("Category", "Unit")] +public sealed class ModbusCoalescingDefaultsTests +{ + private static async Task<(ModbusDriver, ScriptedTransport)> StartAsync(ModbusDriverOptions opts) + { + var t = new ScriptedTransport(); + var d = new ModbusDriver(opts, "m1", _ => t); + await d.InitializeAsync("{}", TestContext.Current.CancellationToken); + return (d, t); + } + + private static ModbusDriverOptions Opts(IEnumerable tags, bool disable = false) => new() + { + Host = "f", RawTags = ModbusRawTags.Entries(tags), DisableReadCoalescing = disable, + Probe = new ModbusProbeOptions { Enabled = false }, + }; + + [Fact] + public void Default_options_coalesce_with_a_gap_of_8() + { + new ModbusDriverOptions().MaxReadGap.ShouldBe((ushort)8); + new ModbusDriverOptions().DisableReadCoalescing.ShouldBeFalse(); + ModbusDriverFactoryExtensions.ParseOptions("m1", """{"host":"h"}""").MaxReadGap.ShouldBe((ushort)8); + } + + [Fact] + public async Task Default_configured_holding_registers_are_read_in_one_PDU() + { + var tags = Enumerable.Range(0, 10).Select(i => + new ModbusTagDefinition($"HR{i}", ModbusRegion.HoldingRegisters, (ushort)(100 + i * 3), ModbusDataType.Int16)).ToArray(); + var (drv, t) = await StartAsync(Opts(tags)); + await using var _ = drv; + for (var i = 0; i < 10; i++) t.Bank.HoldingRegisters[100 + i * 3] = (ushort)(i + 1); + + var r = await drv.ReadAsync([.. tags.Select(x => x.Name)], TestContext.Current.CancellationToken); + + t.Requests(0x03).Count.ShouldBe(1); + r.Select(s => (short)s.Value!).ShouldBe(Enumerable.Range(1, 10).Select(i => (short)i)); + } + + [Fact] + public async Task DisableReadCoalescing_reads_every_tag_alone() + { + // Positive control for the default-on test: the off switch really is per-tag. + var tags = new[] + { + new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 100, ModbusDataType.Int16), + new ModbusTagDefinition("B", ModbusRegion.HoldingRegisters, 101, ModbusDataType.Int16), + }; + var (drv, t) = await StartAsync(Opts(tags, disable: true)); + await using var _ = drv; + await drv.ReadAsync(["A", "B"], TestContext.Current.CancellationToken); + t.Requests(0x03).Count.ShouldBe(2); + } + + [Theory] + [InlineData(ModbusRegion.Coils, (byte)0x01)] + [InlineData(ModbusRegion.DiscreteInputs, (byte)0x02)] + public async Task Adjacent_bit_tags_fold_into_one_bit_read(ModbusRegion region, byte fc) + { + var tags = Enumerable.Range(0, 64).Select(i => + new ModbusTagDefinition($"B{i}", region, (ushort)(10 + i), ModbusDataType.Bool)).ToArray(); + var (drv, t) = await StartAsync(Opts(tags)); + await using var _ = drv; + var bank = region == ModbusRegion.Coils ? t.Bank.Coils : t.Bank.DiscreteInputs; + for (var i = 0; i < 64; i++) bank[10 + i] = i % 3 == 0; + + var r = await drv.ReadAsync([.. tags.Select(x => x.Name)], TestContext.Current.CancellationToken); + + t.Requests(fc).ShouldBe([(fc, (ushort)10, (ushort)64)]); + for (var i = 0; i < 64; i++) r[i].Value.ShouldBe(i % 3 == 0, $"bit {i}"); + } + + [Fact] + public async Task Bit_tags_on_one_status_word_fold_into_one_register_read() + { + var tags = Enumerable.Range(0, 16).Select(b => + new ModbusTagDefinition($"S{b}", ModbusRegion.HoldingRegisters, 40, ModbusDataType.BitInRegister, BitIndex: (byte)b)).ToArray(); + var (drv, t) = await StartAsync(Opts(tags)); + await using var _ = drv; + t.Bank.HoldingRegisters[40] = 0b1010_0000_0000_0101; + + var r = await drv.ReadAsync([.. tags.Select(x => x.Name)], TestContext.Current.CancellationToken); + + t.Requests(0x03).ShouldBe([((byte)0x03, (ushort)40, (ushort)1)]); + for (var b = 0; b < 16; b++) r[b].Value.ShouldBe(((0b1010_0000_0000_0101 >> b) & 1) == 1, $"bit {b}"); + } + + [Fact] + public async Task A_string_tag_coalesces_with_its_neighbours() + { + var tags = new[] + { + new ModbusTagDefinition("Num", ModbusRegion.HoldingRegisters, 50, ModbusDataType.Int16), + new ModbusTagDefinition("Str", ModbusRegion.HoldingRegisters, 51, ModbusDataType.String, StringLength: 4), + new ModbusTagDefinition("After", ModbusRegion.HoldingRegisters, 53, ModbusDataType.UInt16), + }; + var (drv, t) = await StartAsync(Opts(tags)); + await using var _ = drv; + t.Bank.HoldingRegisters[50] = 7; + t.Bank.HoldingRegisters[51] = ('A' << 8) | 'B'; + t.Bank.HoldingRegisters[52] = ('C' << 8) | 'D'; + t.Bank.HoldingRegisters[53] = 9; + + var r = await drv.ReadAsync(["Num", "Str", "After"], TestContext.Current.CancellationToken); + + t.Requests(0x03).ShouldBe([((byte)0x03, (ushort)50, (ushort)4)]); + r[0].Value.ShouldBe((short)7); + r[1].Value.ShouldBe("ABCD"); + r[2].Value.ShouldBe((ushort)9); + } + + [Fact] + public async Task A_refused_coil_range_is_prohibited_and_reprobed_with_FC01() + { + var tags = Enumerable.Range(0, 4).Select(i => + new ModbusTagDefinition($"C{i}", ModbusRegion.Coils, (ushort)(20 + i), ModbusDataType.Bool)).ToArray(); + var opts = new ModbusDriverOptions + { + Host = "f", RawTags = ModbusRawTags.Entries(tags), Probe = new ModbusProbeOptions { Enabled = false }, + AutoProhibitReprobeInterval = TimeSpan.FromHours(1), + }; + var (drv, t) = await StartAsync(opts); + await using var _ = drv; + var refuseMerged = true; + t.Handler = (_, pdu, _) => + { + var qty = (pdu[3] << 8) | pdu[4]; + return pdu[0] == 0x01 && qty > 1 && refuseMerged + ? Task.FromException(new ModbusException(0x01, 0x02, "protected")) + : null; + }; + + var r = await drv.ReadAsync([.. tags.Select(x => x.Name)], TestContext.Current.CancellationToken); + r.ShouldAllBe(s => s.StatusCode == 0u, "the per-tag fallback still reads every coil"); + drv.AutoProhibitedRangeCount.ShouldBe(1); + + refuseMerged = false; + while (t.Sent.TryDequeue(out var _1)) { } + await drv.RunReprobeOnceForTestAsync(TestContext.Current.CancellationToken); + t.Sent.ShouldAllBe(s => s.Pdu[0] == 0x01, "a coil range is re-probed with FC01, never a register read"); + drv.AutoProhibitedRangeCount.ShouldBe(0); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingTests.cs index 8bdea55f..72542014 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusCoalescingTests.cs @@ -42,9 +42,9 @@ public sealed class ModbusCoalescingTests public ValueTask DisposeAsync() => ValueTask.CompletedTask; } - /// Verifies that MaxReadGap=0 defaults to per-tag reads without coalescing. + /// MaxReadGap=0 coalesces only adjacent tags: a one-register hole splits the read. [Fact] - public async Task MaxReadGap_Zero_Defaults_To_Per_Tag_Reads() + public async Task MaxReadGap_Zero_Does_Not_Bridge_A_Gap() { var fake = new CountingTransport(); var t1 = new ModbusTagDefinition("T1", ModbusRegion.HoldingRegisters, 100, ModbusDataType.Int16); @@ -56,7 +56,7 @@ public sealed class ModbusCoalescingTests await drv.ReadAsync(["T1", "T2"], CancellationToken.None); - // With coalescing off, expect 2 separate FC03 reads. + // A one-register gap with MaxReadGap=0: expect 2 separate FC03 reads. var fc03Reads = fake.Reads.Where(r => r.Fc == 0x03).ToList(); fc03Reads.Count.ShouldBe(2); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ScriptedTransport.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ScriptedTransport.cs index e865bc34..b05a5ffd 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ScriptedTransport.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ScriptedTransport.cs @@ -30,6 +30,13 @@ internal sealed class ScriptedTransport : IModbusTransport public int TotalSends => _sendsByUnit.Values.Sum(); + /// Every PDU sent, in order. + public ConcurrentQueue<(byte Unit, byte[] Pdu)> Sent { get; } = new(); + + /// The (function code, start, quantity) of every sent request with that function code. + public List<(byte Fc, ushort Start, ushort Qty)> Requests(byte fc) => + [.. Sent.Where(s => s.Pdu[0] == fc).Select(s => (s.Pdu[0], (ushort)((s.Pdu[1] << 8) | s.Pdu[2]), (ushort)((s.Pdu[3] << 8) | s.Pdu[4])))]; + public async Task ConnectAsync(CancellationToken ct) { ConnectEntered.TrySetResult(); @@ -39,6 +46,7 @@ internal sealed class ScriptedTransport : IModbusTransport public Task SendAsync(byte unitId, byte[] pdu, CancellationToken ct) { _sendsByUnit.AddOrUpdate(unitId, 1, (_, n) => n + 1); + Sent.Enqueue((unitId, pdu)); var scripted = Handler?.Invoke(unitId, pdu, ct); return scripted ?? Bank.SendAsync(unitId, pdu, ct); }