diff --git a/CLAUDE.md b/CLAUDE.md index c1661950..37641a9a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1133,7 +1133,7 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa - **TwinCAT (#645, #310/#314):** opt-in ADS sum-read (`useSumRead`, `sumReadMaxItems` default 200 / ceiling 500, 32 KiB payload budget). It reads raw bytes and decodes as the declared type, so only a scalar passing the strict width+kind check (`CheckNotification`) rides a batch; value-converted pairs (DInt over INT, Real over LREAL/DINT) read singly. Only overall - ADS 1793/1794 latch the device to single reads; 1792/1797/1798 split that chunk without latching; a comms failure neither. + ADS 1793/1794 latch the device to single reads; any other non-comms overall code (1792/1795/1797/1798...) reads that chunk singly for the poll without latching, with a once-per-device Warning; a comms failure neither. Opt-in `diagnosticsEnabled` (default off) puts ADS state/app/online-change/cycle/exec/`HeadroomMs` on the `/hosts` `IDriverDiagnostics` bag (up to 4 extra reads per device per probe tick). Unit-tested only; live gate pending a user-approved route (the presses' routes name the Mac's old VPN IP). Sum-write not ported. diff --git a/docs/drivers/TwinCAT.md b/docs/drivers/TwinCAT.md index c1074553..24796bb0 100644 --- a/docs/drivers/TwinCAT.md +++ b/docs/drivers/TwinCAT.md @@ -208,11 +208,12 @@ command is unverified, hence opt-in. without joining the batch; a bit-indexed BOOL and a handle-addressed symbol (`0xF005`) are read singly. - Per-item ADS error codes map to that tag only (`MapAdsError`); a response shorter than asked is a size error for that item, not a failed chunk. The overall code is the command's. -- **Latch vs split.** Only overall ADS **1793 / 1794** (service / index group not supported) throws +- **Latch vs read-singly.** Only overall ADS **1793 / 1794** (service / index group not supported) throws `TwinCATSumReadRefusedException`; the driver logs once, marks the device `SumReadUnsupported` and reads - singly until re-initialise. **1792 / 1797 / 1798** (general error, bad size, bad data) mean *this request* - was rejected — typically too big — so that chunk is read singly for this poll and nothing is latched. A - comms-level failure is neither: that chunk goes `BadCommunicationError` and the next poll tries again. + singly until re-initialise. **Any other non-comms overall code** (1792 general error, 1795 offset, 1797 size, 1798 data, ...) means + *this request* was rejected — typically too big — so that chunk is read singly for this poll, nothing is + latched, and a Warning is logged once per device so the live gate can see sum-read is not working there. A + comms-level failure (router / port / timeout) is neither: that chunk goes `BadCommunicationError` and the next poll tries again. - A one-item chunk is just a single read. Sum-**write** is not implemented (writes stay per-tag). ### `/hosts` diagnostics (`IDriverDiagnostics`, opt-in) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs index 05ff055c..d73ff860 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs @@ -289,6 +289,7 @@ internal sealed class AdsTwinCATClient : ITwinCATClient { // The library slices the response by the REQUESTED lengths and throws when a target answers with // less. That is this target mangling this response, not a comms fault: read the group singly. + NoteSumFallback(0); await FallBackAsync().ConfigureAwait(false); return; } @@ -301,6 +302,7 @@ internal sealed class AdsTwinCATClient : ITwinCATClient throw new TwinCATSumReadRefusedException(overall); if (TwinCATSumReadPlanner.IsRequestRejected(overall)) { + NoteSumFallback(overall); await FallBackAsync().ConfigureAwait(false); // too big / bad size for THIS request: no latch return; } @@ -325,6 +327,20 @@ internal sealed class AdsTwinCATClient : ITwinCATClient private readonly record struct PlannedSumRead(int Index, uint IndexGroup, uint IndexOffset, TwinCATArrayLayout Layout, bool IsArray); private string? _lastAdsState; + private long _sumFallbackCount; + private long _lastSumFallbackCode; + + private void NoteSumFallback(uint code) + { + Interlocked.Exchange(ref _lastSumFallbackCode, code); + Interlocked.Increment(ref _sumFallbackCount); + } + + /// + public long SumFallbackCount => Interlocked.Read(ref _sumFallbackCount); + + /// + public uint LastSumFallbackCode => (uint)Interlocked.Read(ref _lastSumFallbackCode); /// public string? LastAdsState => Volatile.Read(ref _lastAdsState); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs index 0584d542..6d5a8577 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs @@ -92,6 +92,16 @@ public interface ITwinCATClient : IDisposable /// string? LastAdsState => null; + /// + /// How many sum-read chunks this client has read singly because the target rejected the sum request + /// without refusing the service (non-latching). The driver warns once per device when it grows, so the + /// live gate can see sum-read is not working on that target. Default 0. + /// + long SumFallbackCount => 0; + + /// The overall ADS code of the most recent such fallback (0 = a malformed response). + uint LastSumFallbackCode => 0; + /// /// Write a symbolic value. Returns the mapped OPC UA status for the operation /// (0 = Good, non-zero = error mapped via ). diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs index 1d8ec16f..811cc6a1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -393,7 +393,17 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery for (var k = 0; k < chunk.Count; k++) items[k] = new TwinCATBulkReadItem(chunk[k].SymbolName, chunk[k].Def.DataType, chunk[k].BitIndex, chunk[k].Def.ArrayLength); + var fallbacksBefore = client.SumFallbackCount; var bulk = await client.ReadValuesAsync(items, cancellationToken).ConfigureAwait(false); + if (client.SumFallbackCount > fallbacksBefore && !device.SumFallbackWarned) + { + device.SumFallbackWarned = true; + _logger.LogWarning( + "TwinCAT driver '{DriverInstanceId}' device '{Device}': the target rejected an ADS sum-read " + + "request (ADS error {AdsError}; 0 = malformed response) and the chunk was read singly. Sum-read " + + "is not working on this target; this warning is logged once per device", + _driverInstanceId, device.Options.HostAddress, client.LastSumFallbackCode); + } for (var k = 0; k < chunk.Count; k++) { var e = chunk[k]; @@ -994,7 +1004,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// /// The /hosts diagnostics bag (Gitea #640 channel): per device, ADS state, PLC app name, online - /// change count, task cycle time, last execution time and jitter (last exec − cycle). A single-device + /// change count, task cycle time, last execution time and headroom (cycle − last exec). A single-device /// driver uses flat TwinCAT.* keys; several devices prefix the device name (or host address). /// A pure in-memory read — the probe loop does the sampling. The host bounds the bag /// (), so a many-device driver keeps the first keys in @@ -1464,6 +1474,9 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// (driver re-initialise). public volatile bool SumReadUnsupported; + /// True once the "sum request rejected, chunk read singly" warning was logged for this device. + public volatile bool SumFallbackWarned; + /// Latest probe-sampled PLC diagnostics (#645 / #314); null before the first sample. public TwinCATDeviceDiagnostics? LastDiagnostics { get; set; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadPlanner.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadPlanner.cs index 57b40f66..a0e98eb0 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadPlanner.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadPlanner.cs @@ -37,11 +37,21 @@ internal static class TwinCATSumReadPlanner /// True for 1793 (service not supported) and 1794 (invalid index group). public static bool IsRefusal(uint adsError) => adsError is 1793 or 1794; - /// The target rejected THIS request (general error / bad size / bad data), typically because it was - /// too big. Not a device property: read the chunk singly for this poll, do not latch. + /// A transport-level failure (router / port / timeout), not an answer from the sum service. /// The overall ADS code. - /// True for 1792, 1797 and 1798. - public static bool IsRequestRejected(uint adsError) => adsError is 1792 or 1797 or 1798; + /// True when the code maps to a communication error or timeout. + public static bool IsCommsFailure(uint adsError) => + TwinCATStatusMapper.MapAdsError(adsError) is var s + && (s == TwinCATStatusMapper.BadCommunicationError || s == TwinCATStatusMapper.BadTimeout); + + /// + /// The sum service answered with an overall error that is neither "unsupported" nor a comms failure + /// (general error, bad size / data / offset, ...): it rejected THIS request. Read the chunk singly for this + /// poll, do not latch. + /// + /// The overall ADS code. + /// True for any such code. + public static bool IsRequestRejected(uint adsError) => !IsRefusal(adsError) && !IsCommsFailure(adsError); /// /// Splits items (in order) into groups whose summed length + PerItemOverhead stays within diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs index 6103d956..2ad33e42 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs @@ -85,7 +85,7 @@ internal class FakeTwinCATClient : ITwinCATClient /// Gets or sets an exception throws instead of answering — a /// models a target without sum commands. public Exception? SumReadException { get; set; } - /// Gets or sets the ADS state text returns. + /// Gets or sets the ADS state text returns. public string? AdsStateText { get; set; } /// Simulates an ADS sum-read: records the batch size, then answers each item from the same @@ -99,6 +99,7 @@ internal class FakeTwinCATClient : ITwinCATClient if (ThrowOnRead) throw Exception ?? new InvalidOperationException(); SumReadBatches.Add(reads.Count); if (SumReadException is { } ex) throw ex; + if (SimulateSumFallback) SumFallbackCount++; var results = new (object? value, uint status)[reads.Count]; for (var i = 0; i < reads.Count; i++) { @@ -109,6 +110,12 @@ internal class FakeTwinCATClient : ITwinCATClient return Task.FromResult>(results); } + /// Gets or sets whether each batch counts as a non-latching sum fallback. + public bool SimulateSumFallback { get; set; } + /// Gets the simulated count of non-latching sum fallbacks. + public long SumFallbackCount { get; private set; } + /// Gets the simulated overall code of the last fallback. + public uint LastSumFallbackCode => 1795; /// Gets the ADS state the "probe" last read. public string? LastAdsState => AdsStateText; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDiagnosticsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDiagnosticsTests.cs index fb027ed5..d135373d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDiagnosticsTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDiagnosticsTests.cs @@ -49,7 +49,7 @@ public sealed class TwinCATDiagnosticsTests diag.GetDiagnostics().ShouldBeEmpty(); } - /// Verifies a single-device driver publishes flat keys with state, app, cycle, exec and jitter. + /// Verifies a single-device driver publishes flat keys with state, app, cycle, exec and headroom. [Fact] public async Task Single_device_publishes_flat_keys() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadPlannerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadPlannerTests.cs index f59fe2ba..cd2612f9 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadPlannerTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadPlannerTests.cs @@ -46,10 +46,12 @@ public sealed class TwinCATSumReadPlannerTests [InlineData(1792u, false, true)] [InlineData(1797u, false, true)] [InlineData(1798u, false, true)] - [InlineData(1795u, false, false)] - [InlineData(6u, false, false)] + [InlineData(1795u, false, true)] // invalid offset: this request rejected, read singly + [InlineData(1796u, false, true)] + [InlineData(1808u, false, true)] + [InlineData(6u, false, false)] // comms: every item goes Bad + [InlineData(7u, false, false)] [InlineData(1861u, false, false)] - [InlineData(1808u, false, false)] public void Overall_ads_codes_classify(uint code, bool refusal, bool rejected) { TwinCATSumReadPlanner.IsRefusal(code).ShouldBe(refusal); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadTests.cs index 40471b7f..863f382d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadTests.cs @@ -156,6 +156,22 @@ public sealed class TwinCATSumReadTests factory.Clients[0].SumReadBatches.ShouldBe([2, 2]); } + /// Verifies a non-latching fallback is surfaced once per device, not once per poll. + [Fact] + public async Task Non_latching_fallback_warns_once_per_device() + { + var (drv, factory) = NewDriver(true, 200, Tag("A"), Tag("B")); + await drv.InitializeAsync("{}", CancellationToken.None); + factory.Customise = () => new FakeTwinCATClient { Values = { ["MAIN.A"] = 1, ["MAIN.B"] = 2 }, SimulateSumFallback = true }; + + await drv.ReadAsync(["A", "B"], CancellationToken.None); + var state = drv.GetDeviceState(Host)!; + state.SumFallbackWarned.ShouldBeTrue(); + state.SumReadUnsupported.ShouldBeFalse(); // not latched: the next poll tries the sum command again + await drv.ReadAsync(["A", "B"], CancellationToken.None); + factory.Clients[0].SumReadBatches.ShouldBe([2, 2]); + } + /// Verifies unknown references do not disturb result order. [Fact] public async Task Unknown_reference_keeps_its_slot_in_a_batched_poll()