diff --git a/docs/drivers/AbCip.md b/docs/drivers/AbCip.md index 6d7b7193..fb65f619 100644 --- a/docs/drivers/AbCip.md +++ b/docs/drivers/AbCip.md @@ -77,7 +77,7 @@ the driver's `DriverConfig` JSON. Key fields: - **`Tags`** — pre-declared `AbCipTagDefinition` list; `Members` for UDT fan-out, `SafetyTag` for GuardLogix safety-partition tags. - **`Probe`** — connectivity-probe `Enabled` / `Interval` / `Timeout` / `ProbeTagPath`. - **`Transport`** — absent (default ⇒ `Native`), `Native`, or `Libplctag` (fallback); see [Transport](#transport-transport-option). -- **Discovery** — `EnableControllerBrowse` (`@tags` walk) and `EnableDeclarationOnlyUdtGrouping` (whole-UDT read fast path). +- **Discovery** — `EnableControllerBrowse` (`@tags` walk), `EnableDeclarationOnlyUdtGrouping` (whole-UDT read fast path) and `browsePageDelayMs` (native browse pacing, see [Native browse](#native-browse-paging-pacing-and-template-leases)). - **Alarms** — `EnableAlarmProjection` + `AlarmPollInterval` for the ALMD projection. Full per-field descriptions live in `AbCipDriverOptions.cs`. The JSON skeleton is @@ -195,8 +195,11 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi [native-ab-transport-gate-results](../plans/2026-10-07-native-ab-transport-gate-results.md). - **libplctag cannot be deleted** until a bench controller allows write verification. - Wide-struct fragmented writes (8-byte alignment) are unit-tested only, never proven on a controller (#567). -- Native browse: listing pages are unpaced, and template reads outside an enumeration open a connection per - call (#572). +- Native browse (#572, resolved in code): listing paging no longer assumes ascending instance order, pages + are paced, and a browse's template reads share one lease. One residual: whether a real controller ever + lists a page out of order is unobserved — the reader is now correct either way. `browsePageDelayMs` is + not editable in the AdminUI form (its typed model drops unknown keys on save, #576); author it in the + driver-config JSON. - Micro800, routed and `logixpccc` paths are covered by `ab_server` only. The Micro800 profile passes natively. The AbLegacy default excludes `logixpccc` (and PLC-5). - Follow-up issues for this workstream: #564 to #577. @@ -241,6 +244,27 @@ integration fixture is down during normal dev (Docker host fixture currently off See [Uns.md §Array tags](../Uns.md#array-tags-1-d) for the cross-driver coverage matrix and the UI authoring flow. +## Native browse: paging, pacing and template leases + +Native symbol browse (`transport` = `Native`; libplctag pages inside its own library and is unaffected): + +- **Paging continues past the highest instance seen**, not the last entry of a page. The Symbol Object + listing is requested from `start`; the next request starts at `max(instance in the reply) + 1`, so the + order a controller lists a page in does not matter. A "more" reply whose highest instance is still below + its `start` (or equals `uint.MaxValue`) made no progress and fails with `BadReply` ("made no progress") + rather than looping against the controller. +- **Pages are paced.** `browsePageDelayMs` (driver config, default **5**, `0` = back-to-back, negative = + `0`) pauses before every page after the first. 5 ms is small against a page's own round-trip, so a + 50k-symbol browse gains a few seconds, but it breaks the back-to-back burst (one request in flight, roughly + a Studio 5000 online browse) into slices the controller's message processing can interleave with. Raise it + for a controller that is tight on CPU; the pause applies per listing (the controller listing and each + program listing). +- **Template reads in a browse share one lease.** `DiscoverAsync` reads the UDT templates (and nested + templates) of a device through a single batch reader (`IAbCipTemplateReaderFactory.CreateBatch`); the + native batch reader holds one device lease from its first read until the browse of that device ends, + instead of leasing per template. `FetchUdtShapeAsync` outside a discovery run is unchanged: one + self-contained lease per call. + ## Controller-discovered UDT member variables When `EnableControllerBrowse` is set, the driver walks the controller's `@tags` symbol table and diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/SymbolObjectServices.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/SymbolObjectServices.cs index dd1f853e..7d7b5507 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/SymbolObjectServices.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport/Logix/SymbolObjectServices.cs @@ -26,18 +26,19 @@ public static class SymbolObjectServices } /// Entries are u32 instance, u16 type, u16 elemLen, u32×3 dims, u16 nameLen, name. - /// LastInstance is the last entry's instance id (next start = LastInstance + 1). + /// MaxInstance is the HIGHEST instance id in the page, not the last entry's: the next start is + /// MaxInstance + 1, which does not rely on the controller listing a page in ascending order. /// More is true on general status 0x06 (partial transfer). - public static (byte[] Entries, uint LastInstance, bool More) ParseListingReply(CipReply reply) + public static (byte[] Entries, uint MaxInstance, bool More) ParseListingReply(CipReply reply) { if (reply.GeneralStatus is not (0x00 or 0x06)) throw new AbTransportException(reply.Status, $"Symbol listing failed: general 0x{reply.GeneralStatus:X2}."); var span = reply.Data.Span; var r = new CipReader(span); - uint last = 0; + uint max = 0; while (r.Remaining > 0) { - last = r.U32(); + max = Math.Max(max, r.U32()); r.Bytes(2 + 2 + 12); var nameLen = r.U16(); r.Bytes(nameLen); @@ -45,6 +46,6 @@ public static class SymbolObjectServices var more = reply.GeneralStatus == 0x06; if (more && span.IsEmpty) throw new AbTransportException(AbStatus.BadReply, "Symbol listing reported more data but returned no entries."); - return (span.ToArray(), last, more); + return (span.ToArray(), max, more); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs index 93d5a636..e559efc3 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs @@ -67,6 +67,15 @@ public sealed class AbCipDriverOptions /// public bool EnableControllerBrowse { get; init; } + /// + /// Pause between the pages of a symbol listing, NATIVE transport only (libplctag pages inside its + /// own library). A large controller's listing is hundreds of pages; unpaced they are a back-to-back + /// burst on the controller's message-processing slice. Default 5 ms — small next to a page's own + /// round-trip, but it breaks up the burst. restores back-to-back paging. + /// Config key browsePageDelayMs; a negative value is treated as zero. + /// + public TimeSpan BrowsePageDelay { get; init; } = TimeSpan.FromMilliseconds(5); + /// /// When true, declared ALMD tags are surfaced as alarm conditions /// via IAlarmSource; the driver polls each subscribed diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index f8f03953..bd05170d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -40,6 +40,13 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, private readonly IAbCipTagFactory _tagFactory; private readonly IAbCipTagEnumeratorFactory _enumeratorFactory; private readonly IAbCipTemplateReaderFactory _templateReaderFactory; + + /// + /// The batch template reader of the discovery run in progress on this async flow, if any. While set, + /// reads through it so a browse that fetches many templates holds ONE + /// device lease across them instead of opening a connection per template (Gitea #572). + /// + private readonly AsyncLocal _batchTemplateReader = new(); private readonly AbCipTemplateCache _templateCache = new(); /// @@ -158,7 +165,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, var nodeDefault = defaultTransport ?? AbCipTransportResolver.Default; _transport = AbCipTransportResolver.Resolve(options, nodeDefault); _transportDescription = AbCipTransportResolver.Describe(options.Transport, nodeDefault); - var defaults = AbCipTransportFactories.For(_transport); + var defaults = AbCipTransportFactories.For(_transport, options.BrowsePageDelay); _tagFactory = tagFactory ?? defaults.Tags; _enumeratorFactory = enumeratorFactory ?? defaults.Enumerators; _templateReaderFactory = templateReaderFactory ?? defaults.Templates; @@ -247,7 +254,9 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { - using var reader = _templateReaderFactory.Create(); + var batch = _batchTemplateReader.Value; + using var own = batch is null ? _templateReaderFactory.Create() : null; + var reader = batch ?? own!; var buffer = await reader.ReadAsync(deviceParams, templateInstanceId, cancellationToken).ConfigureAwait(false); var shape = CipTemplateObjectDecoder.Decode(buffer); if (shape is not null) @@ -1411,6 +1420,10 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, { using var enumerator = _enumeratorFactory.Create(); var deviceParams = state.BuildCreateParams("@tags", _options.Timeout); + // One template-reader lease across every UDT shape this device's browse fetches. + using var batchReader = _templateReaderFactory.CreateBatch(); + _batchTemplateReader.Value = batchReader; + using var clearBatch = new ClearOnDispose(_batchTemplateReader); IAddressSpaceBuilder? discoveredFolder = null; // Dedup discovered member full-names within this device's Discovered folder. @@ -1488,6 +1501,12 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } } + /// Clears an when the scope that set it ends. + private sealed class ClearOnDispose(AsyncLocal slot) : IDisposable + { + public void Dispose() => slot.Value = null; + } + /// /// true for a UDT member the controller generates rather than the user declares: the /// hidden SINT/DINT hosts Logix packs BOOL members into (ZZZZZZZZZZAlarms0) and AOI diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs index 66236c62..e3c814dd 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs @@ -109,6 +109,7 @@ public static class AbCipDriverFactoryExtensions ? null : ParseEnum(dto.Transport, tagName: null, driverInstanceId, "Transport"), EnableControllerBrowse = dto.EnableControllerBrowse ?? false, + BrowsePageDelay = TimeSpan.FromMilliseconds(Math.Max(0, dto.BrowsePageDelayMs ?? 5)), EnableAlarmProjection = dto.EnableAlarmProjection ?? false, AlarmPollInterval = TimeSpan.FromMilliseconds(dto.AlarmPollIntervalMs ?? 1_000), EnableDeclarationOnlyUdtGrouping = dto.EnableDeclarationOnlyUdtGrouping ?? false, @@ -176,6 +177,11 @@ public static class AbCipDriverFactoryExtensions /// public bool? EnableControllerBrowse { get; init; } + /// + /// Gets or sets the pause in milliseconds between symbol-listing pages (native transport); absent means 5. + /// + public int? BrowsePageDelayMs { get; init; } + /// /// Gets or sets whether alarm projection is enabled. /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportFactories.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportFactories.cs index 6362875a..02c5442a 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportFactories.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportFactories.cs @@ -4,10 +4,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; internal static class AbCipTransportFactories { /// Returns the tag, enumerator and template-reader factories for . + /// The transport. + /// Pause between native symbol-listing pages; null is the compiled default. public static (IAbCipTagFactory Tags, IAbCipTagEnumeratorFactory Enumerators, IAbCipTemplateReaderFactory Templates) - For(AbCipTransport transport) => transport switch + For(AbCipTransport transport, TimeSpan? browsePageDelay = null) => transport switch { - AbCipTransport.Native => (new NativeAbCipTagFactory(), new NativeAbCipTagEnumeratorFactory(), + AbCipTransport.Native => (new NativeAbCipTagFactory(), + new NativeAbCipTagEnumeratorFactory(browsePageDelay ?? NativeListingReader.DefaultPageDelay), new NativeAbCipTemplateReaderFactory()), _ => (new LibplctagTagFactory(), new LibplctagTagEnumeratorFactory(), new LibplctagTemplateReaderFactory()), }; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTemplateReader.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTemplateReader.cs index dfe1ac1e..0d085efe 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTemplateReader.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTemplateReader.cs @@ -29,4 +29,13 @@ public interface IAbCipTemplateReaderFactory /// Creates a new template reader instance. /// A new instance. IAbCipTemplateReader Create(); + + /// + /// Creates a reader meant to serve MANY calls: the native + /// reader then holds one device lease from its first read until it is disposed, so a caller fetching + /// N templates opens one connection rather than one per template. Dispose it when the batch ends. + /// The default is (a transport whose reader has no per-read connection cost). + /// + /// A new instance. + IAbCipTemplateReader CreateBatch() => Create(); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagEnumerator.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagEnumerator.cs index c05d3259..b071183c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagEnumerator.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagEnumerator.cs @@ -17,27 +17,34 @@ internal sealed class NativeAbCipTagEnumerator : IAbCipTagEnumerator { private readonly Func Release)> _acquire; private readonly ILogger _logger; + private readonly TimeSpan _pageDelay; + private readonly TimeProvider _time; /// Production constructor: leases from . /// The connection pool (the factory passes ). - public NativeAbCipTagEnumerator(DeviceSessionPool pool) + /// Pause between listing pages (see ). + public NativeAbCipTagEnumerator(DeviceSessionPool pool, TimeSpan pageDelay = default) : this(p => { var lease = pool.Acquire(NativeAbCipDevice.Key(p), NativeAbCipDevice.Options(p)); return (lease.Scheduler, lease.DisposeAsync); - }) + }, pageDelay: pageDelay) { } /// Test seam: supplies the scheduler and its release directly. /// Acquires the scheduler for a device, with the callback that releases it. /// Receives release failures; null discards them. + /// Pause between listing pages; zero is back-to-back. + /// Clock for the pause; null is the system clock. internal NativeAbCipTagEnumerator( Func Release)> acquire, - ILogger? logger = null) + ILogger? logger = null, TimeSpan pageDelay = default, TimeProvider? time = null) { _acquire = acquire; _logger = logger ?? NullLogger.Instance; + _pageDelay = pageDelay; + _time = time ?? TimeProvider.System; } /// @@ -49,7 +56,7 @@ internal sealed class NativeAbCipTagEnumerator : IAbCipTagEnumerator try { using var inner = new SymbolListingEnumerator( - (_, name, ct) => NativeListingReader.ReadAsync(scheduler, name, ct)); + (_, name, ct) => NativeListingReader.ReadAsync(scheduler, name, ct, _pageDelay, _time)); await foreach (var tag in inner.EnumerateAsync(deviceParams, cancellationToken).ConfigureAwait(false)) yield return tag; } @@ -75,13 +82,25 @@ internal static class NativeListingReader /// Byte cap for one listing (64 MiB). internal const long MaxListingBytes = 64L * 1024 * 1024; + /// + /// Default pause between listing pages. A big controller's listing is hundreds of pages and, unpaced, + /// a back-to-back burst (one request in flight, roughly a Studio 5000 online browse). 5 ms is small + /// against a page's own round-trip (tens of ms on a real network), so a 50k-symbol browse gains a few + /// seconds, yet it yields the controller's message-processing slice between pages. Zero disables it. + /// Operator knob: browsePageDelayMs on the driver config. + /// + internal static readonly TimeSpan DefaultPageDelay = TimeSpan.FromMilliseconds(5); + /// Reads @tags or Program:X.@tags in full. /// The device's shared scheduler. /// The listing pseudo-tag name. /// Cancels the read. + /// Pause before each page after the first; zero is back-to-back. + /// Clock for the pause; null is the system clock. /// The raw entries, byte for byte what expects. public static async Task ReadAsync( - RequestScheduler scheduler, string listingName, CancellationToken cancellationToken) + RequestScheduler scheduler, string listingName, CancellationToken cancellationToken, + TimeSpan pageDelay = default, TimeProvider? time = null) { string? program = null; if (listingName != "@tags") @@ -96,21 +115,24 @@ internal static class NativeListingReader uint start = 0; for (var page = 0; page < MaxPages; page++) { + if (page > 0 && pageDelay > TimeSpan.Zero) + await Task.Delay(pageDelay, time ?? TimeProvider.System, cancellationToken).ConfigureAwait(false); var request = SymbolObjectServices.GetInstanceAttributeList(program, start); // Exclusive: the reply can fill the connection, and the packer budgets request bytes only. var reply = await scheduler.SendAsync(request, cancellationToken, exclusive: true).ConfigureAwait(false); - var (entries, last, more) = SymbolObjectServices.ParseListingReply(reply); + var (entries, pageMax, more) = SymbolObjectServices.ParseListingReply(reply); all.Write(entries); if (all.Length > MaxListingBytes) throw new AbTransportException(AbStatus.BadReply, $"Symbol listing '{listingName}' exceeded {MaxListingBytes} bytes."); if (!more) return all.ToArray(); - // Instances ascend; a page that does not advance past its start, or a last id that would - // wrap, is a misbehaving reply and would otherwise loop forever against the controller. - if (last < start || last == uint.MaxValue) + // Continue past the HIGHEST instance seen so far, so the order a controller lists a page in does + // not matter. A page whose highest instance is still below its start made no progress, and a + // highest id that would wrap cannot be continued: both would loop forever against the controller. + if (pageMax < start || pageMax == uint.MaxValue) throw new AbTransportException(AbStatus.BadReply, - $"Symbol listing '{listingName}' made no progress (start {start}, last {last})."); - start = last + 1; + $"Symbol listing '{listingName}' made no progress (start {start}, highest instance {pageMax})."); + start = pageMax + 1; } throw new AbTransportException(AbStatus.BadReply, $"Symbol listing '{listingName}' exceeded {MaxPages} pages."); @@ -120,8 +142,17 @@ internal static class NativeListingReader /// Factory for . internal sealed class NativeAbCipTagEnumeratorFactory : IAbCipTagEnumeratorFactory { + private readonly TimeSpan _pageDelay; + + /// Factory with the compiled default pacing. + public NativeAbCipTagEnumeratorFactory() : this(NativeListingReader.DefaultPageDelay) { } + + /// Factory pacing listing pages by . + /// Pause between pages; zero is back-to-back. + public NativeAbCipTagEnumeratorFactory(TimeSpan pageDelay) => _pageDelay = pageDelay; + /// - public IAbCipTagEnumerator Create() => new NativeAbCipTagEnumerator(DeviceSessionPool.Shared); + public IAbCipTagEnumerator Create() => new NativeAbCipTagEnumerator(DeviceSessionPool.Shared, _pageDelay); } /// Bounded lease release shared by the native enumerator and template reader. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTemplateReader.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTemplateReader.cs index 0a9d5bbe..fd9fea59 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTemplateReader.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTemplateReader.cs @@ -9,7 +9,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// /// Native : Get Attribute List then chunked Read Template on the /// device's shared scheduler, returning the 14-byte-header buffer -/// parses. The lease is held for one read and released when it completes. +/// parses. By default the lease is held for one read and released when it completes; a reader built for a +/// batch () instead keeps one lease per device from its +/// first read until it is disposed. /// internal sealed class NativeAbCipTemplateReader : IAbCipTemplateReader { @@ -24,27 +26,52 @@ internal sealed class NativeAbCipTemplateReader : IAbCipTemplateReader private readonly Func Release)> _acquire; private readonly ILogger _logger; + private readonly bool _holdLeases; + private readonly object _heldLock = new(); + private readonly Dictionary Release)> _held = new(); + private bool _disposed; /// Production constructor: leases from . /// The connection pool (the factory passes ). - public NativeAbCipTemplateReader(DeviceSessionPool pool) + /// True for a batch reader: keep each device's lease until disposed. + public NativeAbCipTemplateReader(DeviceSessionPool pool, bool holdLeases = false) : this(p => { var lease = pool.Acquire(NativeAbCipDevice.Key(p), NativeAbCipDevice.Options(p)); return (lease.Scheduler, lease.Channel, lease.DisposeAsync); - }) + }, holdLeases: holdLeases) { } /// Test seam: supplies the scheduler, channel and release directly. /// Acquires the connection parts for a device. /// Receives release failures; null discards them. + /// True for a batch reader: keep each device's lease until disposed. internal NativeAbCipTemplateReader( Func Release)> acquire, - ILogger? logger = null) + ILogger? logger = null, bool holdLeases = false) { _acquire = acquire; _logger = logger ?? NullLogger.Instance; + _holdLeases = holdLeases; + } + + /// The connection parts for one read, and whether this read owns (must release) them. + private (RequestScheduler Scheduler, ICipChannel Channel, Func Release, bool Owned) Lease(AbCipTagCreateParams p) + { + if (!_holdLeases) + { + var one = _acquire(p); + return (one.Scheduler, one.Channel, one.Release, true); + } + var key = NativeAbCipDevice.Key(p); + lock (_heldLock) + { + ObjectDisposedException.ThrowIf(_disposed, this); + if (!_held.TryGetValue(key, out var held)) + _held[key] = held = _acquire(p); + return (held.Scheduler, held.Channel, held.Release, false); + } } /// @@ -55,7 +82,7 @@ internal sealed class NativeAbCipTemplateReader : IAbCipTemplateReader throw new ArgumentOutOfRangeException(nameof(templateInstanceId), "Template ids are 16-bit."); var id = (ushort)templateInstanceId; - var (scheduler, channel, release) = _acquire(deviceParams); + var (scheduler, channel, release, owned) = Lease(deviceParams); try { var attrReply = await scheduler.SendAsync(TemplateObjectServices.GetAttributes(id), cancellationToken) @@ -91,12 +118,22 @@ internal sealed class NativeAbCipTemplateReader : IAbCipTemplateReader } finally { - NativeLease.ReleaseBounded(release, _logger); + if (owned) NativeLease.ReleaseBounded(release, _logger); } } - /// Nothing to release — each read owns and releases its own lease. - public void Dispose() { } + /// Releases the leases a batch reader held; a per-read reader has none. + public void Dispose() + { + List> releases; + lock (_heldLock) + { + _disposed = true; + releases = [.. _held.Values.Select(h => h.Release)]; + _held.Clear(); + } + foreach (var release in releases) NativeLease.ReleaseBounded(release, _logger); + } } /// Factory for . @@ -104,4 +141,7 @@ internal sealed class NativeAbCipTemplateReaderFactory : IAbCipTemplateReaderFac { /// public IAbCipTemplateReader Create() => new NativeAbCipTemplateReader(DeviceSessionPool.Shared); + + /// + public IAbCipTemplateReader CreateBatch() => new NativeAbCipTemplateReader(DeviceSessionPool.Shared, holdLeases: true); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/SymbolObjectServicesTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/SymbolObjectServicesTests.cs index d17ba8de..1d87ab10 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/SymbolObjectServicesTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests/Logix/SymbolObjectServicesTests.cs @@ -56,6 +56,17 @@ public sealed class SymbolObjectServicesTests more.ShouldBeTrue(); last.ShouldBe(5u); entries.ShouldBe(entry); } + [Fact] + public void MaxInstance_is_the_highest_in_the_page_whatever_its_order() + { + static string Entry(uint i) => BitConverter.ToString(BitConverter.GetBytes(i)).Replace("-", "") + + "C400" + "0400" + "00000000" + "00000000" + "00000000" + "0100" + "41"; + var entries = Convert.FromHexString(Entry(9) + Entry(3) + Entry(5)); + var (_, max, more) = SymbolObjectServices.ParseListingReply(new CipReply(0x55, 0x06, [], entries)); + more.ShouldBeTrue(); + max.ShouldBe(9u); + } + [Fact] public void Failure_status_throws() => Should.Throw(() => diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialRun.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialRun.cs index 5be2d046..ee9e9b7c 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialRun.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialRun.cs @@ -27,6 +27,13 @@ internal sealed class TransportObservations public Dictionary Shapes { get; } = []; public Dictionary TemplateErrors { get; } = []; + /// + /// The transport status of each refused template, where the refusal was an . + /// Lets parity tell a template BOTH transports refuse with the same status (an equal refusal) from a real + /// divergence without parsing text. + /// + public Dictionary TemplateErrorStatuses { get; } = []; + /// Template ids that were queued but not read because AB_DIFF_MAX_TEMPLATES was reached. public int TemplatesTruncated { get; set; } public Dictionary Reads { get; } = new(StringComparer.Ordinal); @@ -320,6 +327,7 @@ internal static class DifferentialRun { if (IsConnectionLevel(ex)) throw Abort(phase, $"template {id}", ex); into.TemplateErrors[id] = Describe(ex); + if (ex is AbTransportException ate) into.TemplateErrorStatuses[id] = ate.Status; } afterEach?.Invoke($"template {id}"); } @@ -659,9 +667,22 @@ internal static class DifferentialRun a is not null && b is not null && a.TypeName == b.TypeName && a.TotalSize == b.TotalSize && a.Members.SequenceEqual(b.Members); + /// + /// Template ids BOTH transports refused with the SAME and no shape on either side: + /// the controller refuses that template and both transports report it identically, which is parity + /// (Gitea #575) — not a divergence, and not a shape comparison either. A refusal with differing statuses, + /// or one side refusing while the other decoded, is not listed here and stays a problem. + /// + internal static List EqualRefusals(TransportObservations lib, TransportObservations native) => + [.. lib.TemplateErrorStatuses.Keys + .Where(id => !lib.Shapes.ContainsKey(id) && !native.Shapes.ContainsKey(id) + && native.TemplateErrorStatuses.TryGetValue(id, out var n) && n == lib.TemplateErrorStatuses[id]) + .Order()]; + /// /// Template-parity problems; empty means parity. Vacuity is a problem: browse found structured tags - /// but no template id was compared (review r1 I3). + /// but no template id was compared (review r1 I3). An equal refusal () is + /// parity and is not a problem, but is not a comparison either: it does not satisfy the vacuity check. /// internal static List TemplateProblems(TransportObservations lib, TransportObservations native) { @@ -669,19 +690,29 @@ internal static class DifferentialRun var ids = lib.Shapes.Keys.Concat(lib.TemplateErrors.Keys) .Concat(native.Shapes.Keys).Concat(native.TemplateErrors.Keys) .Distinct().Order().ToList(); + var refused = EqualRefusals(lib, native).ToHashSet(); var structured = lib.Browse.Concat(native.Browse).Count(t => t.DataType == AbCipDataType.Structure); - if (structured > 0 && ids.Count == 0) + if (structured > 0 && ids.Count(id => !refused.Contains(id)) == 0) problems.Add($"vacuous: browse found {structured} structured tag(s) but no template id was compared"); foreach (var id in ids) { + if (refused.Contains(id)) continue; var libShape = lib.Shapes.GetValueOrDefault(id); var nativeShape = native.Shapes.GetValueOrDefault(id); if (!ShapesEqual(libShape, nativeShape)) - problems.Add($"template {id}: " + (libShape is null || nativeShape is null ? "undecodable or unread on a side" : "shapes differ")); + problems.Add($"template {id}: " + Divergence(lib, native, id, libShape, nativeShape)); } return problems; } + private static string Divergence( + TransportObservations lib, TransportObservations native, uint id, AbCipUdtShape? libShape, AbCipUdtShape? nativeShape) + { + if (lib.TemplateErrorStatuses.TryGetValue(id, out var l) && native.TemplateErrorStatuses.TryGetValue(id, out var n) && l != n) + return $"refused with different statuses (libplctag {l}, native {n})"; + return libShape is null || nativeShape is null ? "undecodable or unread on a side" : "shapes differ"; + } + internal static string Describe(Exception ex) => ex is AbTransportException ate ? $"{ate.Status}: {ate.Message}" : $"{ex.GetType().Name}: {ex.Message}"; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialSafetyTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialSafetyTests.cs index 5a6bbd3a..d7842605 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialSafetyTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/DifferentialSafetyTests.cs @@ -2,6 +2,7 @@ using System.Net; using System.Net.Sockets; using Shouldly; using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport; using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip; namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.Differential; @@ -99,6 +100,91 @@ public sealed class DifferentialSafetyTests DifferentialRun.TemplateProblems(lib, native).ShouldBeEmpty(); } + [Fact] + public void A_template_both_transports_refuse_with_the_same_status_is_an_equal_refusal_not_a_problem() + { + var shape = new AbCipUdtShape("Udt", 4, [new AbCipUdtMember("M", 0, AbCipDataType.DInt, 1)]); + var lib = new TransportObservations(); + var native = new TransportObservations(); + lib.Browse.Add(new AbCipDiscoveredTag("U", null, AbCipDataType.Structure, false, TemplateInstanceId: 7)); + lib.Browse.Add(new AbCipDiscoveredTag("V", null, AbCipDataType.Structure, false, TemplateInstanceId: 8)); + // 8 decodes on both sides (a real comparison, so the run is not vacuous); 7 is refused by both. + lib.Shapes[8] = shape; + native.Shapes[8] = shape; + lib.TemplateErrors[7] = "NotFound: general 0x05 (libplctag wording)"; + native.TemplateErrors[7] = "NotFound: Read Template 7 failed: general 0x05 (native wording)"; + lib.TemplateErrorStatuses[7] = AbStatus.NotFound; + native.TemplateErrorStatuses[7] = AbStatus.NotFound; + + DifferentialRun.EqualRefusals(lib, native).ShouldBe(new uint[] { 7 }); + DifferentialRun.TemplateProblems(lib, native).ShouldBeEmpty(); + + // Positive control: the same ids with DIFFERENT statuses are still a problem, naming both statuses. + native.TemplateErrorStatuses[7] = AbStatus.NotAllowed; + DifferentialRun.EqualRefusals(lib, native).ShouldBeEmpty(); + var differ = DifferentialRun.TemplateProblems(lib, native).ShouldHaveSingleItem(); + differ.ShouldContain("template 7"); + differ.ShouldContain("different statuses"); + + // One side refusing while the other decoded is a problem too. + native.TemplateErrors.Remove(7); + native.TemplateErrorStatuses.Remove(7); + native.Shapes[7] = shape; + DifferentialRun.EqualRefusals(lib, native).ShouldBeEmpty(); + DifferentialRun.TemplateProblems(lib, native).ShouldHaveSingleItem().ShouldContain("template 7"); + + // A run whose only templates are equal refusals compared no shape: still vacuous, not silently green. + var onlyLib = new TransportObservations(); + var onlyNative = new TransportObservations(); + onlyLib.Browse.Add(new AbCipDiscoveredTag("U", null, AbCipDataType.Structure, false, TemplateInstanceId: 7)); + onlyLib.TemplateErrorStatuses[7] = onlyNative.TemplateErrorStatuses[7] = AbStatus.NotFound; + onlyLib.TemplateErrors[7] = onlyNative.TemplateErrors[7] = "NotFound"; + DifferentialRun.TemplateProblems(onlyLib, onlyNative).ShouldHaveSingleItem().ShouldContain("vacuous"); + } + + [Fact] + public async Task The_open_attempt_budget_alone_stops_a_second_open_even_when_nothing_aborts_the_phase() + { + // Unlike the abort test above, the body here SWALLOWS every failure and keeps going, so the harness's + // abort-on-first-failure can never end the phase: only CipConnection.OpenAttemptBudget stands between + // the second request and the wire. + await using var endpoint = HangUpListener.Start(); + var target = Target(endpoint.Port); + var statuses = new List(); + string? secondMessage = null; + var attemptsBefore = Volatile.Read(ref CipConnection.OpenAttempts); + var tags = AbCipTransportFactories.For(AbCipTransport.Native).Tags; + + await DifferentialRun.WithNativeAnchorAsync("native", target, async _ => + { + // Two independent tag initialisations, each of which needs the connection open. Failures are + // swallowed here on purpose (the real harness would abort on the first). + for (var i = 0; i < 2; i++) + { + using var runtime = tags.Create(target.Params("A", 1, false, AbCipDataType.DInt)); + try + { + await runtime.InitializeAsync(TestContext.Current.CancellationToken); + } + catch (AbTransportException ex) + { + statuses.Add(ex.Status); + if (i == 1) secondMessage = ex.Message; + } + } + return true; + }); + + statuses.Count.ShouldBe(2, "both requests failed; neither aborted the phase"); + statuses[1].ShouldBe(AbStatus.Abort); + secondMessage.ShouldNotBeNull().ShouldContain("budget"); + Volatile.Read(ref CipConnection.OpenAttempts).ShouldBe(attemptsBefore + 2, "the second attempt was counted, then refused"); + // Long enough for a throttled reconnect to have reached the listener had the budget let one through. + await Task.Delay(TimeSpan.FromSeconds(1.5), TestContext.Current.CancellationToken); + endpoint.Connections.ShouldBe(1, "exactly the one budgeted open attempt reached the wire"); + Volatile.Read(ref CipConnection.OpenAttemptBudget).ShouldBe(-1, "the budget is restored afterwards"); + } + [Fact] public void Volume_knobs_fail_fast_above_the_ceiling_instead_of_clamping() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/TransportDifferentialTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/TransportDifferentialTests.cs index 453cb1f8..4d42a0a3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/TransportDifferentialTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Differential/TransportDifferentialTests.cs @@ -77,11 +77,15 @@ public sealed class TransportDifferentialTests(DifferentialRunFixture fixture) Log("| Template | libplctag | native | Equal? |"); Log("|---|---|---|---|"); } + var refused = DifferentialRun.EqualRefusals(r.Libplctag, r.Native).ToHashSet(); foreach (var id in ids) { var equal = DifferentialRun.ShapesEqual(r.Libplctag.Shapes.GetValueOrDefault(id), r.Native.Shapes.GetValueOrDefault(id)); - Log($"| {id} | {Shape(r.Libplctag, id)} | {Shape(r.Native, id)} | {(equal ? "yes" : "**NO**")} |"); + var verdict = refused.Contains(id) ? "equal refusal" : equal ? "yes" : "**NO**"; + Log($"| {id} | {Shape(r.Libplctag, id)} | {Shape(r.Native, id)} | {verdict} |"); } + if (refused.Count > 0) + Log($"- EQUAL REFUSAL (not a divergence; both transports refuse with the same status): {string.Join(", ", refused)}"); var problems = DifferentialRun.TemplateProblems(r.Libplctag, r.Native); foreach (var p in problems) Log($"- PROBLEM: {p}"); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs index 404ef00c..7fe5ebd6 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs @@ -253,6 +253,50 @@ public sealed class AbCipDriverDiscoveryTests byName["Motor1.Running"].DriverDataType.ShouldBe(DriverDataType.Boolean); } + /// + /// Gitea #572: a browse that fetches many templates (a UDT and its nested UDT here) reads them all + /// through ONE batch reader, so the native transport holds one device lease across them rather than + /// leasing per template. The batch reader is disposed when the device's browse ends. + /// + [Fact] + public async Task Discovery_reads_every_template_through_one_batch_reader() + { + const string device = "ab://10.0.0.5/1,0"; + const uint motorTemplateId = 0x2A; + const uint statusTemplateId = 0x123; + var enumeratorFactory = new FakeEnumeratorFactory( + new AbCipDiscoveredTag("Motor1", null, AbCipDataType.Structure, ReadOnly: false, TemplateInstanceId: motorTemplateId), + new AbCipDiscoveredTag("Motor2", null, AbCipDataType.Structure, ReadOnly: false, TemplateInstanceId: 0x2B)); + var templates = new IdKeyedTemplateReaderFactory + { + Templates = + { + [motorTemplateId] = BuildSimpleTemplate("MotorUdt", 12, + ("Speed", 0xCA, 0, 0), ("Status", (ushort)(0x8000 | statusTemplateId), 0, 8)), + [0x2B] = BuildSimpleTemplate("Motor2Udt", 4, ("Speed", 0xCA, 0, 0)), + [statusTemplateId] = BuildSimpleTemplate("StatusUdt", 4, ("Code", 0xC4, 0, 0)), + }, + }; + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(device)], + EnableControllerBrowse = true, + }, "drv-1", enumeratorFactory: enumeratorFactory, templateReaderFactory: templates); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.DiscoverAsync(new RecordingBuilder(), CancellationToken.None); + + templates.RequestedIds.Count.ShouldBe(3); + templates.BatchReaders.ShouldBe(1); + templates.PerCallReaders.ShouldBe(0); + templates.BatchReadersDisposed.ShouldBe(1); + + // Outside a discovery run a fetch is still a self-contained per-call read. + await drv.FetchUdtShapeAsync(device, 0x2B, CancellationToken.None); + await drv.FetchUdtShapeAsync(device, 0x999, CancellationToken.None); + templates.PerCallReaders.ShouldBe(1, "0x2B is cached; 0x999 is not"); + } + /// /// PRODUCTION PATH (no test seam): a controller-discovered UDT whose Template Object carries /// a NESTED struct member fans the nested member out into its own atomic leaves WITHOUT any @@ -799,10 +843,20 @@ public sealed class AbCipDriverDiscoveryTests /// Gets every template id any reader was asked for, in request order. public List RequestedIds { get; } = new(); - /// Creates a reader over the shared template map. - public IAbCipTemplateReader Create() => new Reader(this); + /// Gets how many per-call readers were created. + public int PerCallReaders { get; private set; } + /// Gets how many batch readers were created. + public int BatchReaders { get; private set; } + /// Gets how many batch readers have been disposed. + public int BatchReadersDisposed { get; set; } - private sealed class Reader(IdKeyedTemplateReaderFactory outer) : IAbCipTemplateReader + /// Creates a reader over the shared template map. + public IAbCipTemplateReader Create() { PerCallReaders++; return new Reader(this); } + + /// Creates a batch reader (disposal is counted). + public IAbCipTemplateReader CreateBatch() { BatchReaders++; return new Reader(this, batch: true); } + + private sealed class Reader(IdKeyedTemplateReaderFactory outer, bool batch = false) : IAbCipTemplateReader { public Task ReadAsync( AbCipTagCreateParams deviceParams, uint templateInstanceId, CancellationToken cancellationToken) @@ -813,7 +867,7 @@ public sealed class AbCipDriverDiscoveryTests outer.Templates.TryGetValue(templateInstanceId, out var blob) ? blob : []); } - public void Dispose() { } + public void Dispose() { if (batch) outer.BatchReadersDisposed++; } } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs index b8bfb43c..93ac9033 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs @@ -260,4 +260,18 @@ public sealed class AbCipTransportSwitchTests pinned.Ok.ShouldBeFalse(); pinned.Message!.ShouldNotContain("Connection lost"); } + + /// Gitea #572: browsePageDelayMs parses (default 5 ms, 0 allowed, negative clamps to 0). + [Theory] + [InlineData(null, 5)] + [InlineData("0", 0)] + [InlineData("25", 25)] + [InlineData("-3", 0)] + public void Config_browsePageDelayMs_parses(string? json, int expectedMs) + { + var config = $$"""{{{(json is null ? "" : $"\"browsePageDelayMs\":{json},")}}"devices":[{{Device}}]}"""; + + AbCipDriverFactoryExtensions.ParseOptions("ab1", config).BrowsePageDelay + .ShouldBe(TimeSpan.FromMilliseconds(expectedMs)); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagEnumeratorTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagEnumeratorTests.cs index c2f2a445..6ec19043 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagEnumeratorTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTagEnumeratorTests.cs @@ -201,6 +201,108 @@ public sealed class NativeAbCipTagEnumeratorTests ch.Gate.TrySetResult(); } + /// One listing entry named "A" with the given instance id. + private static byte[] EntryOf(uint instance) + { + var entry = new byte[22 + 1]; + BinaryPrimitives.WriteUInt32LittleEndian(entry, instance); + BinaryPrimitives.WriteUInt16LittleEndian(entry.AsSpan(20), 1); + entry[22] = (byte)'A'; + return entry; + } + + /// The start instance of a controller-scope 0x55 listing request. + private static uint StartOf(byte[] req) => + req.AsSpan(2, req[1] * 2)[2] == 0x25 + ? BinaryPrimitives.ReadUInt16LittleEndian(req.AsSpan(2 + 4)) + : BinaryPrimitives.ReadUInt32LittleEndian(req.AsSpan(2 + 4)); + + /// Serves scripted pages (instances per page, in the order given); the last page is final. + private static FakeCipChannel ServePages(List starts, params uint[][] pages) + { + var ch = new FakeCipChannel(); + var i = 0; + ch.RespondPerService = req => + { + starts.Add(StartOf(req)); + var page = pages[Math.Min(i, pages.Length - 1)]; + var last = i >= pages.Length - 1; + i++; + return FakeCipChannel.Reply(0x55, last ? (byte)0x00 : (byte)0x06, [.. page.SelectMany(EntryOf)]); + }; + return ch; + } + + [Fact] + public async Task Paging_continues_past_the_highest_instance_even_when_a_page_is_listed_out_of_order() + { + // Page 1 ends on a LOW instance (5) though it holds 9: continuing at last + 1 would restart at 6 and + // re-list 9. The reader must continue at max + 1 = 10. + var starts = new List(); + var ch = ServePages(starts, [9, 3, 5], [12, 10]); + await using var sched = new RequestScheduler(ch, allowPacking: false); + + var bytes = await NativeListingReader.ReadAsync(sched, "@tags", TestContext.Current.CancellationToken); + + starts.ShouldBe(new uint[] { 0, 10 }); + bytes.Length.ShouldBe(5 * 23); + } + + [Fact] + public async Task An_out_of_order_page_that_stays_below_its_start_is_a_no_progress_failure() + { + // Page 2 starts at 10 but only answers instances below it: highest-so-far did not advance. + var starts = new List(); + var ch = new FakeCipChannel(); + var n = 0; + ch.RespondPerService = req => + { + starts.Add(StartOf(req)); + return FakeCipChannel.Reply(0x55, 0x06, [.. (n++ == 0 ? new uint[] { 9, 3 } : new uint[] { 4, 2 }).SelectMany(EntryOf)]); + }; + await using var sched = new RequestScheduler(ch, allowPacking: false); + + var ex = await Should.ThrowAsync( + () => NativeListingReader.ReadAsync(sched, "@tags", TestContext.Current.CancellationToken)); + + ex.Status.ShouldBe(AbStatus.BadReply); + ex.Message.ShouldContain("no progress"); + starts.ShouldBe(new uint[] { 0, 10 }); + } + + [Fact] + public async Task Pages_after_the_first_are_paced_and_a_zero_delay_is_back_to_back() + { + var time = new InstantTimeProvider(); + var pages = new uint[][] { [1], [2], [3], [4] }; + await using var sched = new RequestScheduler(ServePages([], pages), allowPacking: false); + + await NativeListingReader.ReadAsync(sched, "@tags", TestContext.Current.CancellationToken, + TimeSpan.FromMilliseconds(7), time); + + // 4 pages => 3 pauses, none before the first request. + time.Delays.ShouldBe(Enumerable.Repeat(TimeSpan.FromMilliseconds(7), 3)); + + var none = new InstantTimeProvider(); + await using var sched2 = new RequestScheduler(ServePages([], pages), allowPacking: false); + await NativeListingReader.ReadAsync(sched2, "@tags", TestContext.Current.CancellationToken, TimeSpan.Zero, none); + none.Delays.ShouldBeEmpty(); + } + + [Fact] + public async Task The_enumerator_hands_its_page_delay_to_every_listing_it_reads() + { + var time = new InstantTimeProvider(); + var ch = Serve(new() { [""] = Entries(ControllerEntries) }); // 3 pages + await using var sched = new RequestScheduler(ch, allowPacking: false); + using var enumerator = new NativeAbCipTagEnumerator( + _ => (sched, () => ValueTask.CompletedTask), pageDelay: TimeSpan.FromMilliseconds(3), time: time); + + await foreach (var _ in enumerator.EnumerateAsync(Params, TestContext.Current.CancellationToken)) { } + + time.Delays.Count.ShouldBe(2 + 0); // controller listing: 3 pages => 2 pauses; the program listing answers 0x05 + } + [Fact] public void A_throwing_release_delegate_never_escapes_ReleaseBounded() { @@ -209,4 +311,24 @@ public sealed class NativeAbCipTagEnumeratorTests NativeLease.ReleaseBounded(() => ValueTask.FromException(new ObjectDisposedException("x")), Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); } + + /// A clock whose timers fire at once, recording each requested due time. + private sealed class InstantTimeProvider : TimeProvider + { + public List Delays { get; } = []; + + public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) + { + lock (Delays) Delays.Add(dueTime); + ThreadPool.QueueUserWorkItem(_ => callback(state)); + return new NoopTimer(); + } + + private sealed class NoopTimer : ITimer + { + public bool Change(TimeSpan dueTime, TimeSpan period) => true; + public void Dispose() { } + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTemplateReaderTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTemplateReaderTests.cs index fe0cbaa4..49ab98e9 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTemplateReaderTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/NativeAbCipTemplateReaderTests.cs @@ -194,4 +194,80 @@ public sealed class NativeAbCipTemplateReaderTests ex.Status.ShouldBe(AbStatus.BadReply); ch.Sent.ShouldNotContain(r => r[0] == 0x4C); } + + [Fact] + public async Task A_batch_reader_leases_once_for_many_reads_and_releases_on_dispose() + { + var captured = Convert.FromHexString(CipTemplateObjectWireCaptureTests.StringTemplate); + var id = BinaryPrimitives.ReadUInt16LittleEndian(captured); + var ch = ServeTemplate(captured, 504); + await using var sched = new RequestScheduler(ch, allowPacking: false); + int acquired = 0, released = 0; + var reader = new NativeAbCipTemplateReader(_ => + { + acquired++; + return (sched, ch, () => { released++; return ValueTask.CompletedTask; }); + }, holdLeases: true); + + for (var i = 0; i < 5; i++) + await reader.ReadAsync(Params, id, TestContext.Current.CancellationToken); + + acquired.ShouldBe(1); + released.ShouldBe(0, "the lease is held across the batch"); + reader.Dispose(); + released.ShouldBe(1); + reader.Dispose(); + released.ShouldBe(1, "a second dispose releases nothing more"); + } + + [Fact] + public async Task A_batch_reader_leases_each_device_once() + { + var captured = Convert.FromHexString(CipTemplateObjectWireCaptureTests.StringTemplate); + var id = BinaryPrimitives.ReadUInt16LittleEndian(captured); + var ch = ServeTemplate(captured, 504); + await using var sched = new RequestScheduler(ch, allowPacking: false); + var acquired = new List(); + using var reader = new NativeAbCipTemplateReader(p => + { + acquired.Add(p.Gateway); + return (sched, ch, () => ValueTask.CompletedTask); + }, holdLeases: true); + + foreach (var host in new[] { "10.0.0.5", "10.0.0.6", "10.0.0.5", "10.0.0.6" }) + await reader.ReadAsync(Params with { Gateway = host }, id, TestContext.Current.CancellationToken); + + acquired.ShouldBe(["10.0.0.5", "10.0.0.6"]); + } + + [Fact] + public async Task The_default_reader_still_leases_per_read() + { + var captured = Convert.FromHexString(CipTemplateObjectWireCaptureTests.StringTemplate); + var id = BinaryPrimitives.ReadUInt16LittleEndian(captured); + var ch = ServeTemplate(captured, 504); + await using var sched = new RequestScheduler(ch, allowPacking: false); + int acquired = 0, released = 0; + using var reader = new NativeAbCipTemplateReader(_ => + { + acquired++; + return (sched, ch, () => { released++; return ValueTask.CompletedTask; }); + }); + + for (var i = 0; i < 3; i++) + await reader.ReadAsync(Params, id, TestContext.Current.CancellationToken); + + (acquired, released).ShouldBe((3, 3)); + } + + [Fact] + public void The_native_factory_hands_out_a_lease_holding_reader_for_a_batch() + { + var factory = new NativeAbCipTemplateReaderFactory(); + using var batch = factory.CreateBatch(); + batch.ShouldBeOfType(); + // The libplctag factory keeps the default: a batch is just a reader. + using var lib = ((IAbCipTemplateReaderFactory)new LibplctagTemplateReaderFactory()).CreateBatch(); + lib.ShouldBeOfType(); + } }