From af19db4ed1156df39df816fc6ea5d09b195e6cfd Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 19:18:42 -0400 Subject: [PATCH 1/3] feat(twincat): opt-in ADS sum-read and /hosts diagnostics bag; refresh TwinCAT backlog (#645, #612) Sum-read (#310): batches a poll's reads per device into sum commands capped at sumReadMaxItems, per-item ADS errors mapped per tag, falls back to individual reads when the target refuses the command. Diagnostics (#314): ADS state, app name, online-change count, task cycle/exec/jitter via IDriverDiagnostics. Re-implemented against the current raw-byte read path (stranded auto/driver-gaps commits predate it). Sum-write not ported. Unit tests only; live gate pending a route. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 5 + docs/drivers/TwinCAT.md | 40 ++- docs/v3/twincat-backlog.md | 70 +++-- .../TwinCATDriverOptions.cs | 16 + .../AdsTwinCATClient.cs | 146 ++++++++++ .../ITwinCATClient.cs | 51 ++++ .../TwinCATDeviceDiagnostics.cs | 25 ++ .../TwinCATDriver.cs | 275 ++++++++++++++++-- .../TwinCATDriverFactoryExtensions.cs | 8 + .../TwinCATSumReadRefusedException.cs | 21 ++ .../Drivers/Forms/TwinCATDriverForm.razor | 20 ++ .../FakeTwinCATClient.cs | 37 +++ .../TwinCATDiagnosticsTests.cs | 148 ++++++++++ .../TwinCATSumReadTests.cs | 237 +++++++++++++++ ...TwinCATDriverPageFormSerializationTests.cs | 4 + 15 files changed, 1055 insertions(+), 48 deletions(-) create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDeviceDiagnostics.cs create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadRefusedException.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDiagnosticsTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 3051ac43..c76b579b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1130,6 +1130,11 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa `DriverHealthChanged.Diagnostics` / proto `has_diagnostics`+`diagnostics`; its digest is in the publish fingerprint). - **TwinCAT (#637):** TIME/TOD publish Float64 ms, DATE/DT a UTC DateTime; a tag authored `UDInt` keeps the raw counter (and keeps historizing — DateTime is not historized). +- **TwinCAT (#645, #310/#314):** opt-in ADS sum-read (`useSumRead`, `sumReadMaxItems` default 200; reads raw bytes by index + group/offset and decodes with the same codecs as a single read; a target refusing the sum command (overall ADS 1792-1798) + flips that device to individual reads, a comms failure does not). `/hosts` diagnostics via `IDriverDiagnostics` + (`TwinCAT.AdsState`/`AppName`/`OnlineChangeCnt`/`CycleTimeMs`/`LastExecTimeMs`/`JitterMs`, probe-sampled). Unit-tested only; live + gate pending a user-approved route (the presses' routes name the Mac's old VPN IP). Sum-write not ported. - **AbLegacy (#636):** `AbLegacyAddress` (in `AbLegacy.Contracts`) is family-aware: PLC-5 `I:`/`O:` indices are OCTAL and are emitted to libplctag as decimal (libplctag's `pccc.c` parses decimal only). MicroLogix function files parse but read `BadNotSupported`. Indirect `N7:[N7:0]` (#646) is resolved by the driver at read/write time (index re-read diff --git a/docs/drivers/TwinCAT.md b/docs/drivers/TwinCAT.md index a878f170..a55fac9e 100644 --- a/docs/drivers/TwinCAT.md +++ b/docs/drivers/TwinCAT.md @@ -185,9 +185,47 @@ notifications; `DInt` over a `STRING(80)` → `BadTypeMismatch` in both paths wi the rest of the subscription Good. No PLC value held a genuine non-ASCII string to exercise Windows-1252 live — that rule is unit-tested only. +## Sum-read and diagnostics (Gitea #645) + +Two items ported from the stranded `auto/driver-gaps` branch (#310, #314), re-implemented against the +current raw-byte read path rather than cherry-picked (the branch predates the type guard, the string codec +and the array codec). Unit-tested against the fake client only; the live gate is read-only and pending a +route (see `docs/v3/twincat-backlog.md`). + +### Sum-read (opt-in) + +`useSumRead: true` (default **off**) batches a poll's reads for one device into ADS sum-read commands +(`IndexGroup 0xF080`) of at most `sumReadMaxItems` symbols (default 200) instead of one round-trip per tag. +TC2 CE 2.11 support for the sum command is unverified, hence opt-in. + +- The client plans each item exactly as a single read does — symbol from the Beckhoff symbol cache, declared + type checked by `TwinCATTypeGuard` — and then reads the raw bytes at the symbol's index group / offset in + one command, decoding with the same `TwinCATArrayCodec` (a scalar is a one-element array). A type-mismatched + item gets `BadTypeMismatch` without joining the batch; a bit-indexed BOOL and a handle-addressed symbol + (`0xF005`) are read individually inside the call. +- Per-item ADS error codes map to that tag only (`MapAdsError`); the overall code is the command's. +- **Fallback.** If the target refuses the command as a whole (overall ADS 1792–1798: service / group / + offset / size not supported) the client throws `TwinCATSumReadRefusedException`; the driver logs once, + marks the device `SumReadUnsupported`, and re-reads that batch with individual reads. The flag is sticky + until the driver is re-initialised. A comms-level failure is **not** a refusal: that chunk goes + `BadCommunicationError` and the next poll tries the sum command again. +- A one-item chunk is just a single read. Sum-**write** is not implemented (writes stay per-tag). + +### `/hosts` diagnostics (`IDriverDiagnostics`) + +The driver implements the optional `IDriverDiagnostics` bag (the S7 `NegotiatedPduSize` precedent, #640), +so these ride the health snapshot to `/hosts` — deliberately not synthetic raw tags. Sampled once per +healthy probe tick: ADS device state (`ReadState`), and `TwinCAT_SystemInfoVarList` `_AppInfo.AppName`, +`_AppInfo.OnlineChangeCnt`, `_TaskInfo[1].CycleTime`, `_TaskInfo[1].LastExecTime` (100 ns ticks). +Keys: `TwinCAT.AdsState`, `.AppName`, `.OnlineChangeCnt`, `.CycleTimeMs`, `.LastExecTimeMs`, `.JitterMs` +(last exec − cycle; positive = the task overran). Several devices prefix `TwinCAT..`; +the host caps the bag at 16 entries. Times are rounded to 0.1 ms because the bag's digest is in the +publish fingerprint. Each symbol is best-effort: a runtime that does not export one (older TC2) keeps the +others, and a symbol answering "not found" is not re-asked until the client is rebuilt. + ## Capability surface -`TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery, ISubscribable, IHostConnectivityProbe, IPerCallHostResolver, IRediscoverable` +`TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery, ISubscribable, IHostConnectivityProbe, IPerCallHostResolver, IRediscoverable, ISkippedTagReporter, IDriverDiagnostics` (`src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs`). | Capability | Path | Notes | diff --git a/docs/v3/twincat-backlog.md b/docs/v3/twincat-backlog.md index ba8874e4..70ba41e3 100644 --- a/docs/v3/twincat-backlog.md +++ b/docs/v3/twincat-backlog.md @@ -5,27 +5,61 @@ The v2 TwinCAT driver is considered solid: 28 integration tests (14 `[TwinCATFac three latent driver bugs shaken out (notification cycle units, `STRING(N)` mapper, bit-indexed BOOL path). Further work is deferred. -Archived from `docs/drivers/TwinCAT-Test-Fixture.md` § Follow-up candidates. +Archived from `docs/drivers/TwinCAT-Test-Fixture.md` § Follow-up candidates. Refreshed 2026-10-08 +(Gitea #612 / #645); live-validation results live in `docs/drivers/TwinCAT.md` § "Live validation". -## Deferred items +## Hardware available (as of 2026-10-08) -1. **TC2 coverage** — spin up a TC2 runtime (Windows CE IPC or legacy XAR) - and run the same suite; any delta surfaces. Blocked on hardware. +The Elma cleaner fleet (25 presses, 23 live) is the only real TwinCAT hardware in reach, and it is +**production — strictly read-only**: -2. **Notification coalescing under load** — run the subscribe test while - the PLC cycle is saturated (bump `lineSim` complexity, watch for - dropped notifications). Doable on current rig; deferred as lower - priority than v3 feature work. +| Press | Runtime | AMS | Notes | +|---|---|---|---| +| 22 presses (e.g. Z33856 `10.201.45.236.1.1`, Z33733 `5.25.53.46.1.1`) | **TC2 2.11 on WinCE 6.0** | port **801** | read-validated 2026-10-07 | +| **Z35474** (`5.118.56.93.1.1`, 10.201.23.227) | **TC3 3.1.4024** (Win NT 10) | port **851** | the **only TC3 press**; "Plc30 App" | +| TCBSD fixture `10.100.0.128` (`41.169.163.43.1.1:851`) | TC3 | 851 | not a press; unreachable from the Mac | -3. **Multi-hop AMS route** — add a test behind an IPC gateway with a - chained route entry. Blocked on hardware (gateway IPC). +Access facts: ADS is TCP 48898. The driver sends no AddRoute (routes are provisioned out of band) and has +no IP field, so a host needs a local AMS router — TwinCAT XAR, or `Beckhoff.TwinCAT.Ads.TcpRouter` hosted +in-process (the driver csproj does not reference TcpRouter; the 2026-10-07 run used a scratch harness). +The Mac reaches the presses directly over the VPN. **The PLC routes left by earlier tooling are STALE:** +they name the Mac's old VPN IP (10.181.198.189), and the Mac is now 10.181.207.140, so a source NetId of +`10.181.207.140.1.1` has no route on any press. We must not AddRoute on production, so the **live gate for +this backlog is deferred pending a user-approved route** (owner refreshes the route, or the user approves +reusing the old source NetId). Nothing below that says "live" has been run against TC3. -4. **License-rotation automation** — XAR's 7-day trial expires on - schedule. Either automate `TcActivate.exe /reactivate` via a scheduled - task on the VM (not officially supported; reportedly works for some - TC3 builds), or buy a paid runtime license (~$1k one-time per runtime - per CPU) to kill the rotation. Ops item, not code. +## Status of the original items -5. **Lab rig** — cheapest IPC (CX7000 / CX9020) on a dedicated network; - the only route that covers TC2 + real EtherCAT I/O timing + cycle - jitter under CPU load. Blocked on hardware + budget. +1. **TC2 coverage — DONE (read side).** No longer "blocked on hardware": the 2026-10-07 Elma run + (TwinCAT 2.11) verified browse, scalar/array/string reads and native notifications on TC2. What is + still unverified there: sum-read support (see item 6) and anything that writes. + +2. **Notification coalescing under load** — run the subscribe test while the PLC cycle is saturated. + Needs the TCBSD fixture (loading a production press is out of the question) and the `GVL_Perf` / + `FB_PerfChurn` fixture project from the stranded `auto/driver-gaps` branch. Fixture-gated. + +3. **Multi-hop AMS route** — a test behind an IPC gateway with a chained route entry. No hardware. + +4. **License-rotation automation** — XAR's 7-day trial expires on schedule. Ops item, not code. + +5. **Lab rig** — cheapest IPC on a dedicated network; the only route that covers real EtherCAT I/O + timing and cycle jitter under CPU load. Blocked on hardware + budget. + +## Added by #612 / #645 + +6. **ADS sum-read (#310) — code DONE, live gate pending.** Opt-in `useSumRead` / `sumReadMaxItems` + (see `docs/drivers/TwinCAT.md` § "Sum-read"). Unit-tested against the fake client only. Live gate + (read-only, valid on a press): enable on Z33856 / Z33733 (TC2) and Z35474 (TC3), compare a batched poll + to individual reads value-for-value, and record whether TC2 CE 2.11 answers or refuses the sum command + (either outcome is a result — a refusal must fall back cleanly). Needs the route above. + +7. **Driver diagnostics (#314) — code DONE, live gate pending.** ADS state, app name, online-change + count, task cycle time, last exec time and jitter on the `/hosts` diagnostics bag. Live gate: confirm + `TwinCAT_SystemInfoVarList._TaskInfo[1]` resolves on TC2 2.11 (the driver tolerates its absence). + +8. **Flat vs VirtualTree browse on TC3 (#612)** — open. Only Z35474 is TC3; read-only symbol-table + upload in both modes, compare leaf sets. Add a `SymbolsLoadMode` option only if Flat drops struct + members. Needs the route. (Flat on TC2 already proved fine: 45,083 / 42,666 variables.) + +9. **Discovered struct/FB member writes (#612)** — deferred: writes, never on a press; fixture-only. + Likewise **sum-write** was deliberately NOT ported from the stranded branch. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDriverOptions.cs index b37430e1..6f3f44f1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDriverOptions.cs @@ -61,6 +61,22 @@ public sealed class TwinCATDriverOptions /// public int NotificationMaxDelayMs { get; init; } + /// + /// When true, a poll's reads for one device are batched into ADS sum-read commands + /// (IndexGroup 0xF080) of up to symbols each instead of one + /// round-trip per tag (Gitea #645 / #310). Default false (opt-in): a TwinCAT 2 CE runtime's + /// sum-command support is unverified. A target that refuses the sum command is detected at runtime + /// and that device falls back to individual reads for the rest of the driver's life. + /// + public bool UseSumRead { get; init; } + + /// + /// Maximum symbols per ADS sum-read command; larger polls are split into chunks of this size. + /// Bounds the request/response size against the target's ADS buffer. Values below 1 are treated as 1. + /// Only consulted when is true. + /// + public int SumReadMaxItems { get; init; } = 200; + /// /// Timeout for the AdminUI Test Connect probe, in seconds. The AdminUI clamps to a /// 60s server-side maximum; this default is what the form pre-fills for new instances. 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 396db079..8388de7c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs @@ -3,6 +3,7 @@ using System.Runtime.CompilerServices; using System.Threading.Channels; using TwinCAT; using TwinCAT.Ads; +using TwinCAT.Ads.SumCommand; using TwinCAT.Ads.TypeSystem; using TwinCAT.TypeSystem; @@ -166,6 +167,151 @@ internal sealed class AdsTwinCATClient : ITwinCATClient } } + /// + public async Task> ReadValuesAsync( + IReadOnlyList reads, CancellationToken cancellationToken) + { + var results = new (object? value, uint status)[reads.Count]; + var planned = new List(reads.Count); + + // Plan every item exactly as the single read would (symbol resolved from the Beckhoff symbol cache, + // declared type checked against it), but stop short of the wire read: those become ONE sum command. + // Anything the sum command cannot carry is read on its own right here. + for (var i = 0; i < reads.Count; i++) + { + var r = reads[i]; + try + { + if (r.BitIndex is not null && r.Type == TwinCATDataType.Bool) + { + results[i] = await ReadValueAsync(r.SymbolPath, r.Type, r.BitIndex, r.ArrayCount, cancellationToken) + .ConfigureAwait(false); + continue; + } + + var (shape, array, symbol, symbolStatus) = await DescribeSymbolAsync(r.SymbolPath, cancellationToken) + .ConfigureAwait(false); + if (symbolStatus != TwinCATStatusMapper.Good) { results[i] = (null, symbolStatus); continue; } + + TwinCATArrayLayout layout; + if (r.ArrayCount is int count) + { + if (TwinCATTypeGuard.CheckArray(r.Type, count, shape, array, out layout) is not null) + { + results[i] = (null, TwinCATStatusMapper.BadTypeMismatch); + continue; + } + } + else + { + if (TwinCATTypeGuard.CheckRead(r.Type, shape) is not null) + { + results[i] = (null, TwinCATStatusMapper.BadTypeMismatch); + continue; + } + // A scalar is a one-element "array" at the type's own width (a STRING at the PLC's declared + // width), so scalar and array share one raw decode. + layout = new TwinCATArrayLayout(1, r.Type is TwinCATDataType.String or TwinCATDataType.WString + ? shape.ByteSize + : TwinCATTypeGuard.DeclaredByteSize(r.Type)); + } + + // A handle-addressed symbol (0xF005) needs a fresh variable handle per read — not expressible + // as an index-group/offset sum entry. Read it individually. + if (symbol!.IndexGroup == SymbolValueByHandle) + { + results[i] = await ReadValueAsync(r.SymbolPath, r.Type, r.BitIndex, r.ArrayCount, cancellationToken) + .ConfigureAwait(false); + continue; + } + + planned.Add(new PlannedSumRead(i, symbol.IndexGroup, symbol.IndexOffset, layout, r.ArrayCount is not null)); + } + catch (AdsErrorException ex) + { + results[i] = (null, MapAndSignal((uint)ex.ErrorCode)); + } + } + + if (planned.Count == 0) return results; + + ResultSumReadRaw sum; + try + { + var entities = new List(planned.Count); + foreach (var p in planned) + entities.Add(new IgIoSumReadEntity(p.IndexGroup, p.IndexOffset, p.Layout.ByteLength)); + sum = await new SumRead(_client, entities, SumCommandErrorStrategy.Relaxed) + .ReadRawAsync(cancellationToken).ConfigureAwait(false); + } + catch (AdsErrorException ex) when (IsSumRefusal((uint)ex.ErrorCode)) + { + throw new TwinCATSumReadRefusedException((uint)ex.ErrorCode, ex); + } + catch (AdsSumCommandException) + { + // The library gave up on the batch without an overall ADS code (a sub-item failure under its + // strict handling). Not a refusal of the service: read these items one by one this time. + foreach (var p in planned) + results[p.Index] = await ReadValueAsync(reads[p.Index].SymbolPath, reads[p.Index].Type, + reads[p.Index].BitIndex, reads[p.Index].ArrayCount, cancellationToken).ConfigureAwait(false); + return results; + } + + // The overall code is the sum command itself; per-item codes are in SubErrors. + if (sum.ErrorCode != AdsErrorCode.NoError) + { + var overall = (uint)sum.ErrorCode; + if (IsSumRefusal(overall)) + throw new TwinCATSumReadRefusedException(overall); + var mapped = MapAndSignal(overall); + foreach (var p in planned) results[p.Index] = (null, mapped); + return results; + } + + for (var k = 0; k < planned.Count; k++) + { + var p = planned[k]; + var subError = sum.SubErrors is { } errs && k < errs.Length ? errs[k] : AdsErrorCode.NoError; + if (subError != AdsErrorCode.NoError) + { + results[p.Index] = (null, MapAndSignal((uint)subError)); + continue; + } + if (sum.ReadBlocks is not { } blocks || k >= blocks.Count || blocks[k].Length < p.Layout.ByteLength) + { + // A short block would decode garbage; treat it as a size mismatch, not a value. + results[p.Index] = (null, TwinCATStatusMapper.MapAdsError(1797)); + continue; + } + var decoded = TwinCATArrayCodec.Decode(reads[p.Index].Type, sum.ReadBlocks![k].Span, p.Layout, StringsAreUtf8()); + results[p.Index] = (p.IsArray ? decoded : decoded.GetValue(0), TwinCATStatusMapper.Good); + } + return results; + } + + private const uint SymbolValueByHandle = 0xF005; + + private readonly record struct PlannedSumRead(int Index, uint IndexGroup, uint IndexOffset, TwinCATArrayLayout Layout, bool IsArray); + + /// + /// True for an overall ADS code that means "this target does not do sum commands" (service / index + /// group / offset / size not supported) rather than a comms fault. + /// + internal static bool IsSumRefusal(uint adsError) => adsError is 1792 or 1793 or 1794 or 1795 or 1797 or 1798; + + /// + public async Task ReadAdsStateAsync(CancellationToken cancellationToken) + { + try + { + var state = await _client.ReadStateAsync(cancellationToken).ConfigureAwait(false); + return state.ErrorCode == AdsErrorCode.NoError ? state.State.AdsState.ToString() : null; + } + catch (OperationCanceledException) { throw; } + catch { return null; } + } + /// /// Resolves the PLC symbol behind into the facts /// compares a declared type against. Uses the Beckhoff symbol 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 6267aa1c..b7daebac 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs @@ -55,6 +55,45 @@ public interface ITwinCATClient : IDisposable int? arrayCount, CancellationToken cancellationToken); + /// + /// Reads several symbols at once — the ADS sum-read path (Gitea #645 / #310). The result is a parallel + /// list: element i is the outcome of [i], each decoded and + /// status-mapped exactly as would (an item the sum command cannot carry — + /// a bit-indexed BOOL, a handle-addressed symbol — is read individually inside the call). + /// + /// + /// The default implementation reads one item at a time, so a client without a sum path (or a test + /// fake) is correct if not faster. An implementation that issues a real sum command throws + /// when the TARGET refuses the command as a whole + /// (service / index group not supported); the caller then falls back to individual reads. A + /// comms-level failure propagates as the ordinary exception. + /// + /// The symbols to read, at most the caller's chunk size. + /// Cancellation token for the read. + /// One (value, status) per item, in request order. + /// The target refused the sum command itself. + async Task> ReadValuesAsync( + IReadOnlyList reads, CancellationToken cancellationToken) + { + var results = new (object? value, uint status)[reads.Count]; + for (var i = 0; i < reads.Count; i++) + { + var r = reads[i]; + results[i] = await ReadValueAsync(r.SymbolPath, r.Type, r.BitIndex, r.ArrayCount, cancellationToken) + .ConfigureAwait(false); + } + return results; + } + + /// + /// The target's ADS device state (Run, Stop, Config, …) as text, or null + /// when it cannot be read. Feeds the driver's /hosts diagnostics (Gitea #645 / #314). + /// Default null — a client without a state read simply reports none. + /// + /// Cancellation token for the read. + /// The state name, or null. + Task ReadAdsStateAsync(CancellationToken cancellationToken) => Task.FromResult(null); + /// /// Write a symbolic value. Returns the mapped OPC UA status for the operation /// (0 = Good, non-zero = error mapped via ). @@ -132,6 +171,18 @@ public interface ITwinCATClient : IDisposable IAsyncEnumerable BrowseSymbolsAsync(CancellationToken cancellationToken); } +/// One element of an request — the same four facts +/// takes. +/// The ADS symbol path (a bit-indexed BOOL keeps its .N suffix, as for a single read). +/// The declared data type. +/// The bit to extract for a BOOL-within-word tag; null otherwise. +/// The authored array length for an array tag; null for a scalar. +public sealed record TwinCATBulkReadItem( + string SymbolPath, + TwinCATDataType Type, + int? BitIndex = null, + int? ArrayCount = null); + /// Opaque handle for a registered ADS notification. tears it down. public interface ITwinCATNotificationHandle : IDisposable { } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDeviceDiagnostics.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDeviceDiagnostics.cs new file mode 100644 index 00000000..23c1e1f8 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDeviceDiagnostics.cs @@ -0,0 +1,25 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +/// +/// One device's probe-sampled PLC facts (Gitea #645 / #314), surfaced through the driver's +/// bag to the /hosts page. Every field is +/// best-effort: a runtime that does not export a system symbol leaves that field at its last known value +/// (null if never seen). +/// +/// The ADS device state name (Run, Stop, Config, …). +/// The running PLC application's name (_AppInfo.AppName). +/// How many online changes the application has taken (_AppInfo.OnlineChangeCnt). +/// The first PLC task's configured cycle time in milliseconds. +/// How long the task's last cycle took to execute, in milliseconds. +/// When the snapshot was taken. +internal sealed record TwinCATDeviceDiagnostics( + string? AdsState, + string? AppName, + uint? OnlineChangeCnt, + double? CycleTimeMs, + double? LastExecTimeMs, + DateTime UpdatedUtc) +{ + /// Gets last execution time minus cycle time: positive means the task overran its cycle. + public double? JitterMs => CycleTimeMs is { } c && LastExecTimeMs is { } x ? x - c : null; +} 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 5782300a..28b3c8e2 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -12,7 +12,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; /// resolver land in PRs 2 and 3. /// public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery, ISubscribable, - IHostConnectivityProbe, IPerCallHostResolver, IRediscoverable, ISkippedTagReporter, IDisposable, IAsyncDisposable + IHostConnectivityProbe, IPerCallHostResolver, IRediscoverable, ISkippedTagReporter, IDriverDiagnostics, + IDisposable, IAsyncDisposable { /// Authored tags this driver could not build into its poll table (Gitea #541). private readonly SkippedTagLedger _skippedTags = new(); @@ -271,6 +272,10 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery var now = DateTime.UtcNow; var results = new DataValueSnapshot[fullReferences.Count]; + // #645/#310: with UseSumRead the reads for one device are gathered and issued as ADS sum-reads + // after this pass; everything else (and a device that refused the sum command) reads one by one. + Dictionary>? buckets = null; + for (var i = 0; i < fullReferences.Count; i++) { var reference = fullReferences[i]; @@ -285,47 +290,135 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery continue; } + if (_options.UseSumRead && !device.SumReadUnsupported) + { + var parsed = TwinCATSymbolPath.TryParse(def.SymbolPath); + var symbolName = parsed?.ToAdsSymbolName() ?? def.SymbolPath; + buckets ??= []; + if (!buckets.TryGetValue(device, out var bucket)) + buckets[device] = bucket = []; + bucket.Add(new SumReadEntry(i, reference, def, symbolName, parsed?.BitIndex)); + continue; + } + + results[i] = await ReadOneAsync(reference, def, device, now, cancellationToken).ConfigureAwait(false); + } + + if (buckets is not null) + foreach (var (device, bucket) in buckets) + await ReadBucketAsync(device, bucket, results, now, cancellationToken).ConfigureAwait(false); + + return results; + } + + private readonly record struct SumReadEntry( + int Index, string Reference, TwinCATTagDefinition Def, string SymbolName, int? BitIndex); + + /// One tag read on its own — the pre-sum-read path, also the fallback for a refused sum command. + private async Task ReadOneAsync( + string reference, TwinCATTagDefinition def, DeviceState device, DateTime now, CancellationToken cancellationToken) + { + try + { + var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false); + var parsed = TwinCATSymbolPath.TryParse(def.SymbolPath); + var symbolName = parsed?.ToAdsSymbolName() ?? def.SymbolPath; + // An array-typed tag (def.ArrayLength != null) reads the array as raw bytes and decodes + // it with TwinCATArrayCodec — the same decode a native array notification runs — into an + // element-typed CLR array. Scalar tags pass null (scalar path) — bit-indexed BOOLs are + // inherently scalar so the client ignores arrayCount when a bitIndex is present. + var (value, status) = await client.ReadValueAsync( + symbolName, def.DataType, parsed?.BitIndex, def.ArrayLength, cancellationToken) + .ConfigureAwait(false); + return CompleteRead(reference, def, symbolName, value, status, now); + } + catch (OperationCanceledException) { throw; } + catch (Exception ex) + { + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); + return new DataValueSnapshot(null, TwinCATStatusMapper.BadCommunicationError, null, now); + } + } + + /// Maps one read outcome to a snapshot and applies the health / log side effects. + private DataValueSnapshot CompleteRead( + string reference, TwinCATTagDefinition def, string symbolName, object? value, uint status, DateTime now) + { + if (status == TwinCATStatusMapper.Good) + { + _health = new DriverHealth(DriverState.Healthy, now, null); + return new DataValueSnapshot(value, status, now, now); + } + + if (status == TwinCATStatusMapper.BadTypeMismatch) + _logger.LogWarning( + "TwinCAT driver '{DriverInstanceId}' refused '{Reference}': declared {DataType} " + + "cannot decode the PLC symbol '{Symbol}' (string/numeric kind mismatch, array element type " + + "or length mismatch, or a reference/container)", + _driverInstanceId, reference, def.DataType, symbolName); + else + _logger.LogWarning( + "TwinCAT driver '{DriverInstanceId}' ADS read error 0x{Status:X8} for '{Reference}'", + _driverInstanceId, status, reference); + _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, + $"ADS status {status:X8} reading {reference}"); + return new DataValueSnapshot(value, status, now, now); + } + + /// + /// Reads one device's gathered tags as ADS sum-reads of at most SumReadMaxItems symbols each. + /// A target that refuses the sum command flips and the + /// rest of the bucket is read individually; any other failure marks only the affected chunk Bad. + /// + private async Task ReadBucketAsync( + DeviceState device, List bucket, DataValueSnapshot[] results, DateTime now, + CancellationToken cancellationToken) + { + var chunkSize = Math.Max(1, _options.SumReadMaxItems); + for (var start = 0; start < bucket.Count; start += chunkSize) + { + var chunk = bucket.GetRange(start, Math.Min(chunkSize, bucket.Count - start)); + + if (device.SumReadUnsupported || chunk.Count == 1) + { + foreach (var e in chunk) + results[e.Index] = await ReadOneAsync(e.Reference, e.Def, device, now, cancellationToken).ConfigureAwait(false); + continue; + } + try { var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false); - var parsed = TwinCATSymbolPath.TryParse(def.SymbolPath); - var symbolName = parsed?.ToAdsSymbolName() ?? def.SymbolPath; - // An array-typed tag (def.ArrayLength != null) reads the array as raw bytes and decodes - // it with TwinCATArrayCodec — the same decode a native array notification runs — into an - // element-typed CLR array. Scalar tags pass null (scalar path) — bit-indexed BOOLs are - // inherently scalar so the client ignores arrayCount when a bitIndex is present. - var (value, status) = await client.ReadValueAsync( - symbolName, def.DataType, parsed?.BitIndex, def.ArrayLength, cancellationToken) - .ConfigureAwait(false); + var items = new TwinCATBulkReadItem[chunk.Count]; + 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); - results[i] = new DataValueSnapshot(value, status, now, now); - if (status == TwinCATStatusMapper.Good) - _health = new DriverHealth(DriverState.Healthy, now, null); - else + var bulk = await client.ReadValuesAsync(items, cancellationToken).ConfigureAwait(false); + for (var k = 0; k < chunk.Count; k++) { - if (status == TwinCATStatusMapper.BadTypeMismatch) - _logger.LogWarning( - "TwinCAT driver '{DriverInstanceId}' refused '{Reference}': declared {DataType} " + - "cannot decode the PLC symbol '{Symbol}' (string/numeric kind mismatch, array element type " + - "or length mismatch, or a reference/container)", - _driverInstanceId, reference, def.DataType, symbolName); - else - _logger.LogWarning( - "TwinCAT driver '{DriverInstanceId}' ADS read error 0x{Status:X8} for '{Reference}'", - _driverInstanceId, status, reference); - _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, - $"ADS status {status:X8} reading {reference}"); + var e = chunk[k]; + var (value, status) = k < bulk.Count ? bulk[k] : (null, TwinCATStatusMapper.BadCommunicationError); + results[e.Index] = CompleteRead(e.Reference, e.Def, e.SymbolName, value, status, now); } } catch (OperationCanceledException) { throw; } + catch (TwinCATSumReadRefusedException refused) + { + device.SumReadUnsupported = true; + _logger.LogWarning( + "TwinCAT driver '{DriverInstanceId}' device '{Device}' refused the ADS sum-read command " + + "(ADS error {AdsError}); falling back to individual reads for this device", + _driverInstanceId, device.Options.HostAddress, refused.AdsErrorCode); + foreach (var e in chunk) + results[e.Index] = await ReadOneAsync(e.Reference, e.Def, device, now, cancellationToken).ConfigureAwait(false); + } catch (Exception ex) { - results[i] = new DataValueSnapshot(null, TwinCATStatusMapper.BadCommunicationError, null, now); + foreach (var e in chunk) + results[e.Index] = new DataValueSnapshot(null, TwinCATStatusMapper.BadCommunicationError, null, now); _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message); } } - - return results; } // ---- IWritable ---- @@ -793,7 +886,11 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery else // 05/STAB-16: on a healthy tick, re-register any notifications whose replay // failed (a transient AddNotification blip) so they recover without a client swap. + { await RetryDeadRegistrationsAsync(state, ct).ConfigureAwait(false); + // #645/#314: sample the PLC state / task-cycle facts for /hosts. Best-effort. + await SampleDeviceDiagnosticsAsync(state, client, ct).ConfigureAwait(false); + } } catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; } catch @@ -813,6 +910,114 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery } } + // ---- IDriverDiagnostics (#645 / #314) ---- + + // Well-known system symbols, exported by TC3 (and most TC2) PLC runtimes under TwinCAT_SystemInfoVarList. + // _TaskInfo[1] is the first PLC task; CycleTime / LastExecTime are UDINT counts of 100 ns. + internal const string SymOnlineChangeCnt = "TwinCAT_SystemInfoVarList._AppInfo.OnlineChangeCnt"; + internal const string SymAppName = "TwinCAT_SystemInfoVarList._AppInfo.AppName"; + internal const string SymCycleTime = "TwinCAT_SystemInfoVarList._TaskInfo[1].CycleTime"; + internal const string SymLastExecTime = "TwinCAT_SystemInfoVarList._TaskInfo[1].LastExecTime"; + + /// Converts a TwinCAT 100 ns tick count to milliseconds. + internal static double TicksToMilliseconds(uint ticks100ns) => ticks100ns / 10_000.0; + + /// + /// Most recent for a device, or null before its first + /// successful probe sample (or for an unknown host address). + /// + /// The device's ADS host address. + /// The snapshot, or null. + internal TwinCATDeviceDiagnostics? GetDeviceDiagnostics(string hostAddress) => + _devices.TryGetValue(hostAddress, out var s) ? s.LastDiagnostics : null; + + /// + /// Samples the ADS device state and the four system symbols once per healthy probe tick and stores the + /// result on . Every read is individually best-effort: a + /// runtime that does not export a symbol (older TwinCAT 2 builds) keeps the previous value, and a + /// symbol that answers "not found" is not asked again until the client is rebuilt. Never throws + /// (except cancellation) — diagnostics must not disturb the probe. + /// + internal async Task SampleDeviceDiagnosticsAsync(DeviceState state, ITwinCATClient client, CancellationToken ct) + { + try + { + var prev = state.LastDiagnostics; + var adsState = prev?.AdsState; + try { adsState = await client.ReadAdsStateAsync(ct).ConfigureAwait(false) ?? adsState; } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } + catch { /* keep previous */ } + + var appName = await ReadDiagSymbolAsync(state, client, SymAppName, TwinCATDataType.String, ct).ConfigureAwait(false) as string + ?? prev?.AppName; + var onlineChange = ToUInt(await ReadDiagSymbolAsync(state, client, SymOnlineChangeCnt, TwinCATDataType.UDInt, ct).ConfigureAwait(false)) + ?? prev?.OnlineChangeCnt; + var cycleTicks = ToUInt(await ReadDiagSymbolAsync(state, client, SymCycleTime, TwinCATDataType.UDInt, ct).ConfigureAwait(false)); + var execTicks = ToUInt(await ReadDiagSymbolAsync(state, client, SymLastExecTime, TwinCATDataType.UDInt, ct).ConfigureAwait(false)); + var cycleMs = cycleTicks is { } c ? TicksToMilliseconds(c) : prev?.CycleTimeMs; + var execMs = execTicks is { } x ? TicksToMilliseconds(x) : prev?.LastExecTimeMs; + + state.LastDiagnostics = new TwinCATDeviceDiagnostics(adsState, appName, onlineChange, cycleMs, execMs, DateTime.UtcNow); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } + catch { /* best-effort */ } + } + + private static uint? ToUInt(object? v) => v switch + { + uint u => u, + null => null, + _ => Convert.ToUInt32(v, System.Globalization.CultureInfo.InvariantCulture), + }; + + private async Task ReadDiagSymbolAsync( + DeviceState state, ITwinCATClient client, string symbol, TwinCATDataType type, CancellationToken ct) + { + if (state.UnavailableDiagSymbols.ContainsKey(symbol)) return null; + try + { + var (value, status) = await client.ReadValueAsync(symbol, type, null, null, ct).ConfigureAwait(false); + if (status == TwinCATStatusMapper.Good) return value; + if (status == TwinCATStatusMapper.BadNodeIdUnknown) + state.UnavailableDiagSymbols[symbol] = 0; + return null; + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } + catch { return null; } + } + + /// + /// 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 + /// 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 + /// ordinal order. + /// + /// The key/value pairs; empty before the first sample. + IReadOnlyDictionary IDriverDiagnostics.GetDiagnostics() + { + var bag = new Dictionary(StringComparer.Ordinal); + var multi = _devices.Count > 1; + foreach (var state in _devices.Values) + { + if (state.LastDiagnostics is not { } d) continue; + var prefix = multi ? $"TwinCAT.{state.Options.DeviceName ?? state.Options.HostAddress}." : "TwinCAT."; + if (d.AdsState is not null) bag[prefix + "AdsState"] = d.AdsState; + if (!string.IsNullOrEmpty(d.AppName)) bag[prefix + "AppName"] = d.AppName; + if (d.OnlineChangeCnt is { } oc) bag[prefix + "OnlineChangeCnt"] = oc.ToString(System.Globalization.CultureInfo.InvariantCulture); + if (d.CycleTimeMs is { } cy) bag[prefix + "CycleTimeMs"] = FormatMs(cy); + if (d.LastExecTimeMs is { } ex) bag[prefix + "LastExecTimeMs"] = FormatMs(ex); + if (d.JitterMs is { } j) bag[prefix + "JitterMs"] = FormatMs(j); + } + return bag; + } + + // 0.1 ms resolution: the bag is published on a heartbeat and its digest is in the publish fingerprint, + // so sub-0.1 ms wobble should not read as a change. + private static string FormatMs(double ms) => + Math.Round(ms, 1, MidpointRounding.AwayFromZero).ToString("0.0", System.Globalization.CultureInfo.InvariantCulture); + private void TransitionDeviceState(DeviceState state, HostState newState) { HostState old; @@ -1234,10 +1439,22 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery public Task? ProbeTask { get; set; } /// Disposes the active ADS client if any. + /// Set when the target refused the ADS sum-read command (#645); the device reads one tag at a + /// time from then on. Cleared only by a fresh (driver re-initialise). + public volatile bool SumReadUnsupported; + + /// Latest probe-sampled PLC diagnostics (#645 / #314); null before the first sample. + public TwinCATDeviceDiagnostics? LastDiagnostics { get; set; } + + /// Diagnostic system symbols the target answered "not found" for — not re-asked until the client is rebuilt. + public ConcurrentDictionary UnavailableDiagSymbols { get; } = new(StringComparer.Ordinal); + public void DisposeClient() { Client?.Dispose(); Client = null; + // A rebuilt client may face a PLC that now exports the symbols (e.g. after a program download). + UnavailableDiagSymbols.Clear(); } /// Disposes the connection gate semaphore. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverFactoryExtensions.cs index ceb18c20..521f748f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverFactoryExtensions.cs @@ -78,6 +78,8 @@ public static class TwinCATDriverFactoryExtensions UseNativeNotifications = dto.UseNativeNotifications ?? true, EnableControllerBrowse = dto.EnableControllerBrowse ?? false, NotificationMaxDelayMs = dto.NotificationMaxDelayMs ?? 0, + UseSumRead = dto.UseSumRead ?? false, + SumReadMaxItems = dto.SumReadMaxItems ?? 200, }; } @@ -114,6 +116,12 @@ public static class TwinCATDriverFactoryExtensions /// Gets or sets the maximum notification delay in milliseconds. public int? NotificationMaxDelayMs { get; init; } + /// Gets or sets a value indicating whether a poll's reads are batched into ADS sum-reads. + public bool? UseSumRead { get; init; } + + /// Gets or sets the maximum symbols per sum-read command. + public int? SumReadMaxItems { get; init; } + /// Gets or sets the list of configured devices. public List? Devices { get; init; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadRefusedException.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadRefusedException.cs new file mode 100644 index 00000000..a6497a7a --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadRefusedException.cs @@ -0,0 +1,21 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +/// +/// Thrown by when the TARGET refuses the ADS sum-read +/// command as a whole (service or index group not supported). It is not a comms fault: the driver marks +/// the device "no sum-read" and re-reads the batch with individual reads, so the poll still completes. +/// +internal sealed class TwinCATSumReadRefusedException : Exception +{ + /// Initializes a new instance of the class. + /// The overall ADS error code the target answered with. + /// The underlying exception, if any. + public TwinCATSumReadRefusedException(uint adsErrorCode, Exception? inner = null) + : base($"ADS sum-read refused by the target (ADS error {adsErrorCode})", inner) + { + AdsErrorCode = adsErrorCode; + } + + /// Gets the overall ADS error code. + public uint AdsErrorCode { get; } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/TwinCATDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/TwinCATDriverForm.razor index 97c5828f..fd8fed93 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/TwinCATDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/TwinCATDriverForm.razor @@ -43,6 +43,20 @@ class="form-control form-control-sm" />
0 = push immediately. Increase to coalesce high-churn signals.
+
+
+ + +
+
One round-trip per chunk instead of per tag. Falls back to single reads if the target refuses it. Default off.
+
+
+ + +
Symbols per sum-read command (default 200).
+
@@ -151,6 +165,8 @@ public bool UseNativeNotifications { get; set; } = true; public bool EnableControllerBrowse { get; set; } = false; public int NotificationMaxDelayMs { get; set; } = 0; + public bool UseSumRead { get; set; } = false; + public int SumReadMaxItems { get; set; } = 200; // Probe public bool ProbeEnabled { get; set; } = true; @@ -168,6 +184,8 @@ UseNativeNotifications = o.UseNativeNotifications, EnableControllerBrowse = o.EnableControllerBrowse, NotificationMaxDelayMs = o.NotificationMaxDelayMs, + UseSumRead = o.UseSumRead, + SumReadMaxItems = o.SumReadMaxItems, ProbeEnabled = o.Probe.Enabled, ProbeIntervalSeconds = (int)o.Probe.Interval.TotalSeconds, ProbeTimeoutSeconds = (int)o.Probe.Timeout.TotalSeconds, @@ -181,6 +199,8 @@ UseNativeNotifications = UseNativeNotifications, EnableControllerBrowse = EnableControllerBrowse, NotificationMaxDelayMs = NotificationMaxDelayMs, + UseSumRead = UseSumRead, + SumReadMaxItems = SumReadMaxItems, Probe = new TwinCATProbeOptions { Enabled = ProbeEnabled, 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 ba5d8023..71e9960a 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 @@ -71,12 +71,49 @@ internal class FakeTwinCATClient : ITwinCATClient string symbolPath, TwinCATDataType type, int? bitIndex, int? arrayCount, CancellationToken ct) { if (ThrowOnRead) throw Exception ?? new InvalidOperationException(); + SingleReadCount++; LastReadArrayCount = arrayCount; var status = ReadStatuses.TryGetValue(symbolPath, out var s) ? s : TwinCATStatusMapper.Good; var value = Values.TryGetValue(symbolPath, out var v) ? v : null; return Task.FromResult((value, status)); } + /// Gets the number of individual calls served. + public int SingleReadCount { get; private set; } + /// Gets the size of each batch the driver issued, in order (#645). + public List SumReadBatches { get; } = new(); + /// 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. + public string? AdsStateText { get; set; } + + /// Simulates an ADS sum-read: records the batch size, then answers each item from the same + /// tables the single read uses. + /// The batch. + /// The cancellation token. + /// One outcome per item. + public virtual Task> ReadValuesAsync( + IReadOnlyList reads, CancellationToken ct) + { + if (ThrowOnRead) throw Exception ?? new InvalidOperationException(); + SumReadBatches.Add(reads.Count); + if (SumReadException is { } ex) throw ex; + var results = new (object? value, uint status)[reads.Count]; + for (var i = 0; i < reads.Count; i++) + { + var path = reads[i].SymbolPath; + results[i] = (Values.TryGetValue(path, out var v) ? v : null, + ReadStatuses.TryGetValue(path, out var s) ? s : TwinCATStatusMapper.Good); + } + return Task.FromResult>(results); + } + + /// Simulates reading the ADS device state. + /// The cancellation token. + /// The configured state text. + public virtual Task ReadAdsStateAsync(CancellationToken ct) => Task.FromResult(AdsStateText); + /// Simulates writing a value to a symbol. /// The path to the symbol to write. /// The data type of the symbol. 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 new file mode 100644 index 00000000..2e540eaa --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDiagnosticsTests.cs @@ -0,0 +1,148 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; + +/// Gitea #645 / #314 — PLC state / task-cycle facts on the optional IDriverDiagnostics bag. +[Trait("Category", "Unit")] +public sealed class TwinCATDiagnosticsTests +{ + private const string Host1 = "ads://5.23.91.23.1.1:851"; + private const string Host2 = "ads://5.23.91.24.1.1:851"; + + private static TwinCATDriver NewDriver(FakeTwinCATClientFactory factory, bool probe, params (string host, string? name)[] devices) => + new(new TwinCATDriverOptions + { + Devices = [.. devices.Select(d => new TwinCATDeviceOptions(d.host, d.name))], + Probe = new TwinCATProbeOptions { Enabled = probe, Interval = TimeSpan.FromMilliseconds(20) }, + }, "drv-diag", factory); + + private static FakeTwinCATClient PlcClient() => new() + { + AdsStateText = "Run", + Values = + { + [TwinCATDriver.SymAppName] = "Plc30 App", + [TwinCATDriver.SymOnlineChangeCnt] = 7u, + [TwinCATDriver.SymCycleTime] = 100_000u, // 10.0 ms + [TwinCATDriver.SymLastExecTime] = 25_000u, // 2.5 ms + }, + }; + + private static async Task Sample(TwinCATDriver drv, FakeTwinCATClient client, string host) + { + var state = drv.GetDeviceState(host)!; + await drv.SampleDeviceDiagnosticsAsync(state, client, CancellationToken.None); + } + + /// Verifies the driver advertises the optional capability and is empty before any sample. + [Fact] + public async Task Bag_is_empty_before_the_first_sample() + { + var drv = NewDriver(new FakeTwinCATClientFactory(), false, (Host1, null)); + await drv.InitializeAsync("{}", CancellationToken.None); + + var diag = drv.ShouldBeAssignableTo()!; + diag.GetDiagnostics().ShouldBeEmpty(); + } + + /// Verifies a single-device driver publishes flat keys with state, app, cycle, exec and jitter. + [Fact] + public async Task Single_device_publishes_flat_keys() + { + var drv = NewDriver(new FakeTwinCATClientFactory(), false, (Host1, null)); + await drv.InitializeAsync("{}", CancellationToken.None); + await Sample(drv, PlcClient(), Host1); + + var bag = ((IDriverDiagnostics)drv).GetDiagnostics(); + + bag["TwinCAT.AdsState"].ShouldBe("Run"); + bag["TwinCAT.AppName"].ShouldBe("Plc30 App"); + bag["TwinCAT.OnlineChangeCnt"].ShouldBe("7"); + bag["TwinCAT.CycleTimeMs"].ShouldBe("10.0"); + bag["TwinCAT.LastExecTimeMs"].ShouldBe("2.5"); + bag["TwinCAT.JitterMs"].ShouldBe("-7.5"); + DriverDiagnostics.Bound(bag).Count.ShouldBe(bag.Count); // within the host's 16-entry cap + } + + /// Verifies several devices are told apart by device name, falling back to host address. + [Fact] + public async Task Multiple_devices_are_prefixed() + { + var drv = NewDriver(new FakeTwinCATClientFactory(), false, (Host1, "Elma01"), (Host2, null)); + await drv.InitializeAsync("{}", CancellationToken.None); + await Sample(drv, PlcClient(), Host1); + await Sample(drv, PlcClient(), Host2); + + var bag = ((IDriverDiagnostics)drv).GetDiagnostics(); + + bag["TwinCAT.Elma01.AdsState"].ShouldBe("Run"); + bag[$"TwinCAT.{Host2}.CycleTimeMs"].ShouldBe("10.0"); + bag.ContainsKey("TwinCAT.AdsState").ShouldBeFalse(); + } + + /// Verifies a runtime without _TaskInfo (older TwinCAT 2) still yields the facts it has, and + /// the missing symbols are asked for once, not on every probe tick. + [Fact] + public async Task Missing_task_info_gives_a_partial_bag_and_is_not_re_asked() + { + var drv = NewDriver(new FakeTwinCATClientFactory(), false, (Host1, null)); + await drv.InitializeAsync("{}", CancellationToken.None); + var client = new FakeTwinCATClient { AdsStateText = "Run" }; + client.Values[TwinCATDriver.SymAppName] = "Plc1"; + client.ReadStatuses[TwinCATDriver.SymCycleTime] = TwinCATStatusMapper.BadNodeIdUnknown; + client.ReadStatuses[TwinCATDriver.SymLastExecTime] = TwinCATStatusMapper.BadNodeIdUnknown; + client.ReadStatuses[TwinCATDriver.SymOnlineChangeCnt] = TwinCATStatusMapper.BadNodeIdUnknown; + + await Sample(drv, client, Host1); + var readsAfterFirst = client.SingleReadCount; + await Sample(drv, client, Host1); + + var bag = ((IDriverDiagnostics)drv).GetDiagnostics(); + bag["TwinCAT.AdsState"].ShouldBe("Run"); + bag["TwinCAT.AppName"].ShouldBe("Plc1"); + bag.Keys.ShouldNotContain("TwinCAT.CycleTimeMs"); + bag.Keys.ShouldNotContain("TwinCAT.JitterMs"); + client.SingleReadCount.ShouldBe(readsAfterFirst + 1); // only AppName is read again + } + + /// Verifies a failed sample keeps the previous snapshot rather than blanking the bag. + [Fact] + public async Task Failed_sample_keeps_the_last_values() + { + var drv = NewDriver(new FakeTwinCATClientFactory(), false, (Host1, null)); + await drv.InitializeAsync("{}", CancellationToken.None); + var client = PlcClient(); + await Sample(drv, client, Host1); + + client.ThrowOnRead = true; + client.AdsStateText = null; + await Sample(drv, client, Host1); + + var bag = ((IDriverDiagnostics)drv).GetDiagnostics(); + bag["TwinCAT.CycleTimeMs"].ShouldBe("10.0"); + bag["TwinCAT.AdsState"].ShouldBe("Run"); + } + + /// Verifies the probe loop itself fills the bag — no manual sampling. + [Fact] + public async Task Probe_loop_populates_the_bag() + { + var factory = new FakeTwinCATClientFactory { Customise = PlcClient }; + var drv = NewDriver(factory, true, (Host1, null)); + await drv.InitializeAsync("{}", CancellationToken.None); + try + { + IReadOnlyDictionary bag = new Dictionary(); + for (var i = 0; i < 100 && bag.Count == 0; i++) + { + await Task.Delay(50); + bag = ((IDriverDiagnostics)drv).GetDiagnostics(); + } + bag["TwinCAT.OnlineChangeCnt"].ShouldBe("7"); + } + finally { await drv.ShutdownAsync(CancellationToken.None); } + } +} 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 new file mode 100644 index 00000000..663e309b --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadTests.cs @@ -0,0 +1,237 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; + +/// Gitea #645 / #310 — a poll's reads batched into ADS sum-read commands, with per-item status, +/// a chunk cap and a fallback to individual reads when the target refuses the command. +[Trait("Category", "Unit")] +public sealed class TwinCATSumReadTests +{ + private const string Host = "ads://5.23.91.23.1.1:851"; + + private static TwinCATTagDefinition Tag(string name, TwinCATDataType type = TwinCATDataType.DInt) => + new(name, Host, "MAIN." + name, type); + + private static (TwinCATDriver drv, FakeTwinCATClientFactory factory) NewDriver( + bool useSumRead, int maxItems, params TwinCATTagDefinition[] tags) + { + var factory = new FakeTwinCATClientFactory(); + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions(Host)], + RawTags = TwinCATRawTags.Entries(tags), + Probe = new TwinCATProbeOptions { Enabled = false }, + UseSumRead = useSumRead, + SumReadMaxItems = maxItems, + }, "drv-sum", factory); + return (drv, factory); + } + + private static FakeTwinCATClient Seeded(params (string symbol, object value)[] values) + { + var c = new FakeTwinCATClient(); + foreach (var (symbol, value) in values) c.Values[symbol] = value; + return c; + } + + /// Verifies that sum-read is opt-in: off, every tag is read on its own. + [Fact] + public async Task Disabled_reads_each_tag_individually() + { + var (drv, factory) = NewDriver(false, 200, Tag("A"), Tag("B"), Tag("C")); + await drv.InitializeAsync("{}", CancellationToken.None); + factory.Customise = () => Seeded(("MAIN.A", 1), ("MAIN.B", 2), ("MAIN.C", 3)); + + var snaps = await drv.ReadAsync(["A", "B", "C"], CancellationToken.None); + + snaps.Select(s => s.Value).ShouldBe([1, 2, 3]); + factory.Clients[0].SumReadBatches.ShouldBeEmpty(); + factory.Clients[0].SingleReadCount.ShouldBe(3); + } + + /// Verifies that one poll's tags go out as a single sum command, values in request order. + [Fact] + public async Task Enabled_batches_a_poll_into_one_sum_command() + { + var (drv, factory) = NewDriver(true, 200, Tag("A"), Tag("B"), Tag("C")); + await drv.InitializeAsync("{}", CancellationToken.None); + factory.Customise = () => Seeded(("MAIN.A", 1), ("MAIN.B", 2), ("MAIN.C", 3)); + + var snaps = await drv.ReadAsync(["C", "A", "B"], CancellationToken.None); + + snaps.Select(s => s.Value).ShouldBe([3, 1, 2]); + snaps.ShouldAllBe(s => s.StatusCode == TwinCATStatusMapper.Good); + factory.Clients[0].SumReadBatches.ShouldBe([3]); + factory.Clients[0].SingleReadCount.ShouldBe(0); + } + + /// Verifies that one item's error code lands on that tag only. + [Fact] + public async Task Per_item_error_code_is_mapped_to_that_tag_only() + { + var (drv, factory) = NewDriver(true, 200, Tag("A"), Tag("B"), Tag("C")); + await drv.InitializeAsync("{}", CancellationToken.None); + factory.Customise = () => + { + var c = Seeded(("MAIN.A", 1), ("MAIN.C", 3)); + c.ReadStatuses["MAIN.B"] = TwinCATStatusMapper.BadNodeIdUnknown; // ADS 1808 DeviceSymbolNotFound + return c; + }; + + var snaps = await drv.ReadAsync(["A", "B", "C"], CancellationToken.None); + + snaps[0].StatusCode.ShouldBe(TwinCATStatusMapper.Good); + snaps[1].StatusCode.ShouldBe(TwinCATStatusMapper.BadNodeIdUnknown); + snaps[1].Value.ShouldBeNull(); + snaps[2].StatusCode.ShouldBe(TwinCATStatusMapper.Good); + snaps[2].Value.ShouldBe(3); + } + + /// Verifies the size cap: a bigger poll splits into chunks and a lone remainder reads singly. + [Fact] + public async Task Poll_larger_than_the_cap_is_split_into_chunks() + { + var tags = Enumerable.Range(0, 5).Select(i => Tag("T" + i)).ToArray(); + var (drv, factory) = NewDriver(true, 2, tags); + await drv.InitializeAsync("{}", CancellationToken.None); + factory.Customise = () => + { + var c = new FakeTwinCATClient(); + for (var i = 0; i < 5; i++) c.Values["MAIN.T" + i] = i * 10; + return c; + }; + + var snaps = await drv.ReadAsync([.. tags.Select(t => t.Name)], CancellationToken.None); + + snaps.Select(s => s.Value).ShouldBe([0, 10, 20, 30, 40]); + factory.Clients[0].SumReadBatches.ShouldBe([2, 2]); + factory.Clients[0].SingleReadCount.ShouldBe(1); // the 5th tag: a one-item "batch" is just a read + } + + /// Verifies the refusal fallback: the poll still completes and the device stops asking. + [Fact] + public async Task Refused_sum_command_falls_back_to_individual_reads_and_sticks() + { + var (drv, factory) = NewDriver(true, 200, Tag("A"), Tag("B"), Tag("C")); + await drv.InitializeAsync("{}", CancellationToken.None); + factory.Customise = () => + { + var c = Seeded(("MAIN.A", 1), ("MAIN.B", 2), ("MAIN.C", 3)); + c.SumReadException = new TwinCATSumReadRefusedException(1793); + return c; + }; + + var first = await drv.ReadAsync(["A", "B", "C"], CancellationToken.None); + var second = await drv.ReadAsync(["A", "B", "C"], CancellationToken.None); + + first.Select(s => s.Value).ShouldBe([1, 2, 3]); + first.ShouldAllBe(s => s.StatusCode == TwinCATStatusMapper.Good); + second.Select(s => s.Value).ShouldBe([1, 2, 3]); + factory.Clients[0].SumReadBatches.ShouldBe([3]); // tried once, never again + factory.Clients[0].SingleReadCount.ShouldBe(6); + drv.GetHealth().State.ShouldBe(Core.Abstractions.DriverState.Healthy); + } + + /// Verifies a comms failure is not mistaken for a refusal: the chunk goes Bad, sum stays on. + [Fact] + public async Task Comms_failure_marks_the_chunk_bad_and_does_not_disable_sum_read() + { + var (drv, factory) = NewDriver(true, 200, Tag("A"), Tag("B")); + await drv.InitializeAsync("{}", CancellationToken.None); + factory.Customise = () => + { + var c = Seeded(("MAIN.A", 1), ("MAIN.B", 2)); + c.SumReadException = new InvalidOperationException("router gone"); + return c; + }; + + var failed = await drv.ReadAsync(["A", "B"], CancellationToken.None); + factory.Clients[0].SumReadException = null; + var recovered = await drv.ReadAsync(["A", "B"], CancellationToken.None); + + failed.ShouldAllBe(s => s.StatusCode == TwinCATStatusMapper.BadCommunicationError); + recovered.Select(s => s.Value).ShouldBe([1, 2]); + 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() + { + var (drv, factory) = NewDriver(true, 200, Tag("A"), Tag("B")); + await drv.InitializeAsync("{}", CancellationToken.None); + factory.Customise = () => Seeded(("MAIN.A", 1), ("MAIN.B", 2)); + + var snaps = await drv.ReadAsync(["A", "nope", "B"], CancellationToken.None); + + snaps[0].Value.ShouldBe(1); + snaps[1].StatusCode.ShouldBe(TwinCATStatusMapper.BadNodeIdUnknown); + snaps[2].Value.ShouldBe(2); + factory.Clients[0].SumReadBatches.ShouldBe([2]); + } + + /// Verifies the batch carries bit index and array length so the client decodes like a single read. + [Fact] + public async Task Bulk_items_carry_array_length_and_bit_index() + { + var captured = new List(); + var factory = new FakeTwinCATClientFactory { Customise = () => new CapturingClient(captured) }; + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions(Host)], + RawTags = TwinCATRawTags.Entries([ + new TwinCATTagDefinition("Arr", Host, "MAIN.Arr", TwinCATDataType.Int, ArrayLength: 4), + new TwinCATTagDefinition("Bit", Host, "MAIN.Word.3", TwinCATDataType.Bool), + ]), + Probe = new TwinCATProbeOptions { Enabled = false }, + UseSumRead = true, + }, "drv-sum", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.ReadAsync(["Arr", "Bit"], CancellationToken.None); + + captured.Count.ShouldBe(2); + captured[0].ArrayCount.ShouldBe(4); + captured[1].BitIndex.ShouldBe(3); + } + + /// Verifies the JSON keys reach the options. + [Fact] + public void Options_parse_from_driver_config_json() + { + var on = TwinCATDriverFactoryExtensions.ParseOptionsForTests( + """{"useSumRead": true, "sumReadMaxItems": 50}""", "d"); + on.UseSumRead.ShouldBeTrue(); + on.SumReadMaxItems.ShouldBe(50); + + var off = TwinCATDriverFactoryExtensions.ParseOptionsForTests("{}", "d"); + off.UseSumRead.ShouldBeFalse(); + off.SumReadMaxItems.ShouldBe(200); + } + + /// Verifies which overall ADS codes mean "no sum commands here" rather than a comms fault. + [Theory] + [InlineData(1792u, true)] + [InlineData(1793u, true)] + [InlineData(1794u, true)] + [InlineData(1795u, true)] + [InlineData(1797u, true)] + [InlineData(1798u, true)] + [InlineData(6u, false)] // TargetPortNotFound + [InlineData(1861u, false)] // ClientSyncTimeOut + [InlineData(1808u, false)] // symbol not found: a per-item matter + public void Overall_ads_codes_classify_as_refusal_only_for_service_errors(uint code, bool refusal) => + AdsTwinCATClient.IsSumRefusal(code).ShouldBe(refusal); + + private sealed class CapturingClient(List sink) : FakeTwinCATClient + { + public override Task> ReadValuesAsync( + IReadOnlyList reads, CancellationToken ct) + { + sink.AddRange(reads); + return base.ReadValuesAsync(reads, ct); + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/TwinCATDriverPageFormSerializationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/TwinCATDriverPageFormSerializationTests.cs index 2664b942..2f53ddca 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/TwinCATDriverPageFormSerializationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/TwinCATDriverPageFormSerializationTests.cs @@ -72,6 +72,8 @@ public sealed class TwinCATDriverPageFormSerializationTests UseNativeNotifications = true, EnableControllerBrowse = false, NotificationMaxDelayMs = 100, + UseSumRead = true, + SumReadMaxItems = 64, Probe = new TwinCATProbeOptions { Enabled = true, @@ -88,6 +90,8 @@ public sealed class TwinCATDriverPageFormSerializationTests roundTripped.UseNativeNotifications.ShouldBeTrue(); roundTripped.EnableControllerBrowse.ShouldBeFalse(); roundTripped.NotificationMaxDelayMs.ShouldBe(100); + roundTripped.UseSumRead.ShouldBeTrue(); + roundTripped.SumReadMaxItems.ShouldBe(64); roundTripped.Probe.Enabled.ShouldBeTrue(); roundTripped.Probe.Interval.ShouldBe(TimeSpan.FromSeconds(6)); roundTripped.Probe.Timeout.ShouldBe(TimeSpan.FromSeconds(2)); From fbcb521d1beee2d9a3289b4194222e3b009585f9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 19:29:35 -0400 Subject: [PATCH 2/3] fix(twincat): review fixes for sum-read and diagnostics (#645) Strict width+kind eligibility for sum-batched scalars (others read singly); latch only on ADS 1793/1794, split 1792/1797/1798 without latching; 32 KiB per-command budget and 500-item ceiling; short sum response handled per item. Diagnostics: opt-in diagnosticsEnabled (default off), reuse the probe's ReadState, sample after the host-state transition, JitterMs renamed HeadroomMs, compact per-device line so many devices fit the 16-entry cap, unavailable-symbol latch cleared on reconnect. AdminUI: min/max on sum-read max, diagnostics checkbox. Pure planner extracted and unit-tested. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 12 +- docs/drivers/TwinCAT.md | 54 ++++---- docs/v3/twincat-backlog.md | 6 +- .../TwinCATDriverOptions.cs | 12 +- .../AdsTwinCATClient.cs | 123 ++++++++++-------- .../ITwinCATClient.cs | 10 +- .../TwinCATDeviceDiagnostics.cs | 5 +- .../TwinCATDriver.cs | 52 +++++--- .../TwinCATDriverFactoryExtensions.cs | 4 + .../TwinCATSumReadPlanner.cs | 103 +++++++++++++++ .../Drivers/Forms/TwinCATDriverForm.razor | 15 ++- .../FakeTwinCATClient.cs | 6 +- .../TwinCATDiagnosticsTests.cs | 83 ++++++++++-- .../TwinCATSumReadPlannerTests.cs | 107 +++++++++++++++ .../TwinCATSumReadTests.cs | 14 -- ...TwinCATDriverPageFormSerializationTests.cs | 2 + 16 files changed, 470 insertions(+), 138 deletions(-) create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadPlanner.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadPlannerTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index c76b579b..c1661950 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1130,11 +1130,13 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa `DriverHealthChanged.Diagnostics` / proto `has_diagnostics`+`diagnostics`; its digest is in the publish fingerprint). - **TwinCAT (#637):** TIME/TOD publish Float64 ms, DATE/DT a UTC DateTime; a tag authored `UDInt` keeps the raw counter (and keeps historizing — DateTime is not historized). -- **TwinCAT (#645, #310/#314):** opt-in ADS sum-read (`useSumRead`, `sumReadMaxItems` default 200; reads raw bytes by index - group/offset and decodes with the same codecs as a single read; a target refusing the sum command (overall ADS 1792-1798) - flips that device to individual reads, a comms failure does not). `/hosts` diagnostics via `IDriverDiagnostics` - (`TwinCAT.AdsState`/`AppName`/`OnlineChangeCnt`/`CycleTimeMs`/`LastExecTimeMs`/`JitterMs`, probe-sampled). Unit-tested only; live - gate pending a user-approved route (the presses' routes name the Mac's old VPN IP). Sum-write not ported. +- **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. + 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. - **AbLegacy (#636):** `AbLegacyAddress` (in `AbLegacy.Contracts`) is family-aware: PLC-5 `I:`/`O:` indices are OCTAL and are emitted to libplctag as decimal (libplctag's `pccc.c` parses decimal only). MicroLogix function files parse but read `BadNotSupported`. Indirect `N7:[N7:0]` (#646) is resolved by the driver at read/write time (index re-read diff --git a/docs/drivers/TwinCAT.md b/docs/drivers/TwinCAT.md index a55fac9e..c1074553 100644 --- a/docs/drivers/TwinCAT.md +++ b/docs/drivers/TwinCAT.md @@ -195,33 +195,41 @@ route (see `docs/v3/twincat-backlog.md`). ### Sum-read (opt-in) `useSumRead: true` (default **off**) batches a poll's reads for one device into ADS sum-read commands -(`IndexGroup 0xF080`) of at most `sumReadMaxItems` symbols (default 200) instead of one round-trip per tag. -TC2 CE 2.11 support for the sum command is unverified, hence opt-in. +(`IndexGroup 0xF080`) of at most `sumReadMaxItems` symbols (default 200, ceiling 500) and at most 32 KiB of +payload (read lengths + 4 bytes each) instead of one round-trip per tag. TC2 CE 2.11 support for the sum +command is unverified, hence opt-in. -- The client plans each item exactly as a single read does — symbol from the Beckhoff symbol cache, declared - type checked by `TwinCATTypeGuard` — and then reads the raw bytes at the symbol's index group / offset in - one command, decoding with the same `TwinCATArrayCodec` (a scalar is a one-element array). A type-mismatched - item gets `BadTypeMismatch` without joining the batch; a bit-indexed BOOL and a handle-addressed symbol - (`0xF005`) are read individually inside the call. -- Per-item ADS error codes map to that tag only (`MapAdsError`); the overall code is the command's. -- **Fallback.** If the target refuses the command as a whole (overall ADS 1792–1798: service / group / - offset / size not supported) the client throws `TwinCATSumReadRefusedException`; the driver logs once, - marks the device `SumReadUnsupported`, and re-reads that batch with individual reads. The flag is sticky - until the driver is re-initialised. A comms-level failure is **not** a refusal: that chunk goes - `BadCommunicationError` and the next poll tries the sum command again. +- **Eligibility is strict.** The sum path reads raw bytes at the symbol's index group / offset and decodes them + as the DECLARED type (the notification rules), so a scalar rides the batch only when it passes + `TwinCATTypeGuard.CheckNotification` — same width, same float/integer kind. A tag the lenient single read + accepts by value conversion (`DInt` over a PLC `INT`, `Real` over `LREAL` or `DINT`) is read **individually** + inside the call, keeping the Beckhoff conversion; the batch never publishes reinterpreted bytes at Good. + Arrays use the same raw-bytes rule as on the single path. A type-mismatched item gets `BadTypeMismatch` + 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 + `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. - A one-item chunk is just a single read. Sum-**write** is not implemented (writes stay per-tag). -### `/hosts` diagnostics (`IDriverDiagnostics`) +### `/hosts` diagnostics (`IDriverDiagnostics`, opt-in) -The driver implements the optional `IDriverDiagnostics` bag (the S7 `NegotiatedPduSize` precedent, #640), -so these ride the health snapshot to `/hosts` — deliberately not synthetic raw tags. Sampled once per -healthy probe tick: ADS device state (`ReadState`), and `TwinCAT_SystemInfoVarList` `_AppInfo.AppName`, -`_AppInfo.OnlineChangeCnt`, `_TaskInfo[1].CycleTime`, `_TaskInfo[1].LastExecTime` (100 ns ticks). -Keys: `TwinCAT.AdsState`, `.AppName`, `.OnlineChangeCnt`, `.CycleTimeMs`, `.LastExecTimeMs`, `.JitterMs` -(last exec − cycle; positive = the task overran). Several devices prefix `TwinCAT..`; -the host caps the bag at 16 entries. Times are rounded to 0.1 ms because the bag's digest is in the -publish fingerprint. Each symbol is best-effort: a runtime that does not export one (older TC2) keeps the -others, and a symbol answering "not found" is not re-asked until the client is rebuilt. +Behind `diagnosticsEnabled` (default **off** — not yet live-gated). The driver implements the optional +`IDriverDiagnostics` bag (the S7 `NegotiatedPduSize` precedent, #640), deliberately not synthetic raw tags. +When enabled, after each healthy probe tick **and after the host-state transition** it reads up to four +`TwinCAT_SystemInfoVarList` symbols per device — `_AppInfo.AppName`, `_AppInfo.OnlineChangeCnt`, +`_TaskInfo[1].CycleTime`, `_TaskInfo[1].LastExecTime` (100 ns ticks) — so up to **4 extra read requests per +device per probe interval** (default 5 s). The ADS state is the one `ProbeAsync` already read; no second +ReadState. One device: flat keys `TwinCAT.AdsState`, `.AppName`, `.OnlineChangeCnt`, `.CycleTimeMs`, +`.LastExecTimeMs`, `.HeadroomMs` (cycle − last exec; negative = the task overran; this is spare time, not +jitter). Several devices: one line per device, `TwinCAT.` = +`Run; Plc30 App; online-changes 7; cycle 10.0 ms; exec 2.5 ms; headroom 7.5 ms`, so the host's 16-entry cap +covers 16 devices. Times round to 0.1 ms (the bag's digest is in the publish fingerprint). Each symbol is +best-effort: a runtime that does not export one (older TC2) keeps the others, and a symbol answering "not +found" is not re-asked until a new client is installed (reconnect). ## Capability surface diff --git a/docs/v3/twincat-backlog.md b/docs/v3/twincat-backlog.md index 70ba41e3..1d0c6678 100644 --- a/docs/v3/twincat-backlog.md +++ b/docs/v3/twincat-backlog.md @@ -48,13 +48,13 @@ reusing the old source NetId). Nothing below that says "live" has been run again ## Added by #612 / #645 6. **ADS sum-read (#310) — code DONE, live gate pending.** Opt-in `useSumRead` / `sumReadMaxItems` - (see `docs/drivers/TwinCAT.md` § "Sum-read"). Unit-tested against the fake client only. Live gate + (strict width+kind eligibility; latch only on ADS 1793/1794; see `docs/drivers/TwinCAT.md` § "Sum-read"). Unit-tested against the fake client only. Live gate (read-only, valid on a press): enable on Z33856 / Z33733 (TC2) and Z35474 (TC3), compare a batched poll to individual reads value-for-value, and record whether TC2 CE 2.11 answers or refuses the sum command (either outcome is a result — a refusal must fall back cleanly). Needs the route above. -7. **Driver diagnostics (#314) — code DONE, live gate pending.** ADS state, app name, online-change - count, task cycle time, last exec time and jitter on the `/hosts` diagnostics bag. Live gate: confirm +7. **Driver diagnostics (#314) — code DONE (opt-in `diagnosticsEnabled`, default off), live gate pending.** ADS state, app name, online-change + count, task cycle time, last exec time and headroom on the `/hosts` diagnostics bag. Live gate: confirm `TwinCAT_SystemInfoVarList._TaskInfo[1]` resolves on TC2 2.11 (the driver tolerates its absence). 8. **Flat vs VirtualTree browse on TC3 (#612)** — open. Only Z35474 is TC3; read-only symbol-table diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDriverOptions.cs index 6f3f44f1..3f87bf05 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Contracts/TwinCATDriverOptions.cs @@ -73,10 +73,20 @@ public sealed class TwinCATDriverOptions /// /// Maximum symbols per ADS sum-read command; larger polls are split into chunks of this size. /// Bounds the request/response size against the target's ADS buffer. Values below 1 are treated as 1. - /// Only consulted when is true. + /// Values above 500 (the ADS sub-command ceiling) are treated as 500. Only consulted when + /// is true. /// public int SumReadMaxItems { get; init; } = 200; + /// + /// When true, each healthy probe tick also reads up to four TwinCAT_SystemInfoVarList + /// symbols (app name, online-change count, task cycle and last-exec time) and the ADS state the probe + /// already fetched, and publishes them on the /hosts diagnostics bag (Gitea #645 / #314). + /// Default false: that is up to four extra read requests per device per probe interval against + /// the PLC, not yet live-gated. Pending the read-only live gate on a TC2 and a TC3 target. + /// + public bool DiagnosticsEnabled { get; init; } + /// /// Timeout for the AdminUI Test Connect probe, in seconds. The AdminUI clamps to a /// 60s server-side maximum; this default is what the form pre-fills for new instances. 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 8388de7c..05ff055c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs @@ -174,9 +174,13 @@ internal sealed class AdsTwinCATClient : ITwinCATClient var results = new (object? value, uint status)[reads.Count]; var planned = new List(reads.Count); + Task<(object? value, uint status)> ReadSingly(int index) => + ReadValueAsync(reads[index].SymbolPath, reads[index].Type, reads[index].BitIndex, + reads[index].ArrayCount, cancellationToken); + // Plan every item exactly as the single read would (symbol resolved from the Beckhoff symbol cache, - // declared type checked against it), but stop short of the wire read: those become ONE sum command. - // Anything the sum command cannot carry is read on its own right here. + // declared type checked against it), but stop short of the wire read: eligible items become sum + // commands. Anything the sum command cannot carry exactly is read on its own right here. for (var i = 0; i < reads.Count; i++) { var r = reads[i]; @@ -184,8 +188,7 @@ internal sealed class AdsTwinCATClient : ITwinCATClient { if (r.BitIndex is not null && r.Type == TwinCATDataType.Bool) { - results[i] = await ReadValueAsync(r.SymbolPath, r.Type, r.BitIndex, r.ArrayCount, cancellationToken) - .ConfigureAwait(false); + results[i] = await ReadSingly(i).ConfigureAwait(false); continue; } @@ -196,6 +199,7 @@ internal sealed class AdsTwinCATClient : ITwinCATClient TwinCATArrayLayout layout; if (r.ArrayCount is int count) { + // Arrays are raw bytes on the single path too, with the same strict element rule. if (TwinCATTypeGuard.CheckArray(r.Type, count, shape, array, out layout) is not null) { results[i] = (null, TwinCATStatusMapper.BadTypeMismatch); @@ -209,6 +213,14 @@ internal sealed class AdsTwinCATClient : ITwinCATClient results[i] = (null, TwinCATStatusMapper.BadTypeMismatch); continue; } + // The single read value-converts a numeric pair (DInt over a PLC INT is fine there); raw + // bytes decoded as the declared type would not. Only an exact width + kind match rides the + // sum command — the rest keeps the Beckhoff conversion by reading singly. + if (!TwinCATSumReadPlanner.ScalarEligible(r.Type, shape)) + { + results[i] = await ReadSingly(i).ConfigureAwait(false); + continue; + } // A scalar is a one-element "array" at the type's own width (a STRING at the PLC's declared // width), so scalar and array share one raw decode. layout = new TwinCATArrayLayout(1, r.Type is TwinCATDataType.String or TwinCATDataType.WString @@ -220,8 +232,7 @@ internal sealed class AdsTwinCATClient : ITwinCATClient // as an index-group/offset sum entry. Read it individually. if (symbol!.IndexGroup == SymbolValueByHandle) { - results[i] = await ReadValueAsync(r.SymbolPath, r.Type, r.BitIndex, r.ArrayCount, cancellationToken) - .ConfigureAwait(false); + results[i] = await ReadSingly(i).ConfigureAwait(false); continue; } @@ -235,82 +246,88 @@ internal sealed class AdsTwinCATClient : ITwinCATClient if (planned.Count == 0) return results; + // Split to the byte budget; a lone item (or an oversized one) is just a read. + var groups = TwinCATSumReadPlanner.Partition( + [.. planned.Select(p => p.Layout.ByteLength)], TwinCATSumReadPlanner.MaxBatchBytes); + foreach (var group in groups) + { + var items = group.Select(g => planned[g]).ToList(); + if (items.Count == 1) + { + results[items[0].Index] = await ReadSingly(items[0].Index).ConfigureAwait(false); + continue; + } + await ReadGroupAsync(items, reads, results, ReadSingly, cancellationToken).ConfigureAwait(false); + } + return results; + } + + private async Task ReadGroupAsync( + List items, IReadOnlyList reads, + (object? value, uint status)[] results, + Func> readSingly, CancellationToken cancellationToken) + { + async Task FallBackAsync() + { + foreach (var p in items) results[p.Index] = await readSingly(p.Index).ConfigureAwait(false); + } + ResultSumReadRaw sum; try { - var entities = new List(planned.Count); - foreach (var p in planned) + var entities = new List(items.Count); + foreach (var p in items) entities.Add(new IgIoSumReadEntity(p.IndexGroup, p.IndexOffset, p.Layout.ByteLength)); sum = await new SumRead(_client, entities, SumCommandErrorStrategy.Relaxed) .ReadRawAsync(cancellationToken).ConfigureAwait(false); } - catch (AdsErrorException ex) when (IsSumRefusal((uint)ex.ErrorCode)) + catch (AdsErrorException ex) when (TwinCATSumReadPlanner.IsRefusal((uint)ex.ErrorCode)) { throw new TwinCATSumReadRefusedException((uint)ex.ErrorCode, ex); } - catch (AdsSumCommandException) + catch (ArgumentException) { - // The library gave up on the batch without an overall ADS code (a sub-item failure under its - // strict handling). Not a refusal of the service: read these items one by one this time. - foreach (var p in planned) - results[p.Index] = await ReadValueAsync(reads[p.Index].SymbolPath, reads[p.Index].Type, - reads[p.Index].BitIndex, reads[p.Index].ArrayCount, cancellationToken).ConfigureAwait(false); - return results; + // 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. + await FallBackAsync().ConfigureAwait(false); + return; } - // The overall code is the sum command itself; per-item codes are in SubErrors. + // On an overall failure the library RETURNS the code (SubErrors all set, ReadBlocks null). if (sum.ErrorCode != AdsErrorCode.NoError) { var overall = (uint)sum.ErrorCode; - if (IsSumRefusal(overall)) + if (TwinCATSumReadPlanner.IsRefusal(overall)) throw new TwinCATSumReadRefusedException(overall); + if (TwinCATSumReadPlanner.IsRequestRejected(overall)) + { + await FallBackAsync().ConfigureAwait(false); // too big / bad size for THIS request: no latch + return; + } var mapped = MapAndSignal(overall); - foreach (var p in planned) results[p.Index] = (null, mapped); - return results; + foreach (var p in items) results[p.Index] = (null, mapped); + return; } - for (var k = 0; k < planned.Count; k++) + var utf8 = StringsAreUtf8(); + for (var k = 0; k < items.Count; k++) { - var p = planned[k]; + var p = items[k]; var subError = sum.SubErrors is { } errs && k < errs.Length ? errs[k] : AdsErrorCode.NoError; - if (subError != AdsErrorCode.NoError) - { - results[p.Index] = (null, MapAndSignal((uint)subError)); - continue; - } - if (sum.ReadBlocks is not { } blocks || k >= blocks.Count || blocks[k].Length < p.Layout.ByteLength) - { - // A short block would decode garbage; treat it as a size mismatch, not a value. - results[p.Index] = (null, TwinCATStatusMapper.MapAdsError(1797)); - continue; - } - var decoded = TwinCATArrayCodec.Decode(reads[p.Index].Type, sum.ReadBlocks![k].Span, p.Layout, StringsAreUtf8()); - results[p.Index] = (p.IsArray ? decoded : decoded.GetValue(0), TwinCATStatusMapper.Good); + ReadOnlyMemory? block = sum.ReadBlocks is { } blocks && k < blocks.Count ? blocks[k] : null; + results[p.Index] = TwinCATSumReadPlanner.DecodeItem( + reads[p.Index].Type, subError, block, p.Layout, p.IsArray, utf8, MapAndSignal); } - return results; } private const uint SymbolValueByHandle = 0xF005; private readonly record struct PlannedSumRead(int Index, uint IndexGroup, uint IndexOffset, TwinCATArrayLayout Layout, bool IsArray); - /// - /// True for an overall ADS code that means "this target does not do sum commands" (service / index - /// group / offset / size not supported) rather than a comms fault. - /// - internal static bool IsSumRefusal(uint adsError) => adsError is 1792 or 1793 or 1794 or 1795 or 1797 or 1798; + private string? _lastAdsState; /// - public async Task ReadAdsStateAsync(CancellationToken cancellationToken) - { - try - { - var state = await _client.ReadStateAsync(cancellationToken).ConfigureAwait(false); - return state.ErrorCode == AdsErrorCode.NoError ? state.State.AdsState.ToString() : null; - } - catch (OperationCanceledException) { throw; } - catch { return null; } - } + public string? LastAdsState => Volatile.Read(ref _lastAdsState); /// /// Resolves the PLC symbol behind into the facts @@ -435,7 +452,9 @@ internal sealed class AdsTwinCATClient : ITwinCATClient try { var state = await _client.ReadStateAsync(cancellationToken).ConfigureAwait(false); - return state.ErrorCode == AdsErrorCode.NoError; + var ok = state.ErrorCode == AdsErrorCode.NoError; + Volatile.Write(ref _lastAdsState, ok ? state.State.AdsState.ToString() : null); + return ok; } catch { 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 b7daebac..0584d542 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs @@ -86,13 +86,11 @@ public interface ITwinCATClient : IDisposable } /// - /// The target's ADS device state (Run, Stop, Config, …) as text, or null - /// when it cannot be read. Feeds the driver's /hosts diagnostics (Gitea #645 / #314). - /// Default null — a client without a state read simply reports none. + /// The ADS device state name (Run, Stop, Config, …) the most recent + /// read, or null when unknown. Reuses the probe's own ReadState so + /// diagnostics add no request (Gitea #645 / #314). Default null. /// - /// Cancellation token for the read. - /// The state name, or null. - Task ReadAdsStateAsync(CancellationToken cancellationToken) => Task.FromResult(null); + string? LastAdsState => null; /// /// Write a symbolic value. Returns the mapped OPC UA status for the operation diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDeviceDiagnostics.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDeviceDiagnostics.cs index 23c1e1f8..e901f3b1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDeviceDiagnostics.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDeviceDiagnostics.cs @@ -20,6 +20,7 @@ internal sealed record TwinCATDeviceDiagnostics( double? LastExecTimeMs, DateTime UpdatedUtc) { - /// Gets last execution time minus cycle time: positive means the task overran its cycle. - public double? JitterMs => CycleTimeMs is { } c && LastExecTimeMs is { } x ? x - c : null; + /// Gets cycle time minus last execution time: how much of the cycle was spare. Negative means the + /// task overran its cycle. (Not jitter — that needs the variation across cycles.) + public double? HeadroomMs => CycleTimeMs is { } c && LastExecTimeMs is { } x ? c - x : null; } 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 28b3c8e2..1d8ec16f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -374,7 +374,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery DeviceState device, List bucket, DataValueSnapshot[] results, DateTime now, CancellationToken cancellationToken) { - var chunkSize = Math.Max(1, _options.SumReadMaxItems); + var chunkSize = Math.Clamp(_options.SumReadMaxItems, 1, TwinCATSumReadPlanner.MaxItemsCeiling); for (var start = 0; start < bucket.Count; start += chunkSize) { var chunk = bucket.GetRange(start, Math.Min(chunkSize, bucket.Count - start)); @@ -867,6 +867,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery while (!ct.IsCancellationRequested) { var success = false; + ITwinCATClient? probed = null; try { // Probe-initiated connects honor TwinCATProbeOptions.Timeout — distinct from @@ -884,12 +885,11 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery if (!success) await RecycleClientAsync(state, ct).ConfigureAwait(false); else + { // 05/STAB-16: on a healthy tick, re-register any notifications whose replay // failed (a transient AddNotification blip) so they recover without a client swap. - { await RetryDeadRegistrationsAsync(state, ct).ConfigureAwait(false); - // #645/#314: sample the PLC state / task-cycle facts for /hosts. Best-effort. - await SampleDeviceDiagnosticsAsync(state, client, ct).ConfigureAwait(false); + probed = client; } } catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; } @@ -905,6 +905,14 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery TransitionDeviceState(state, success ? HostState.Running : HostState.Stopped); + // #645/#314: opt-in PLC state / task-cycle sampling for /hosts. After the host-state transition so a + // slow target cannot delay it; never throws (except cancellation). + if (_options.DiagnosticsEnabled && probed is not null) + { + try { await SampleDeviceDiagnosticsAsync(state, probed, ct).ConfigureAwait(false); } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; } + } + try { await Task.Delay(_options.Probe.Interval, ct).ConfigureAwait(false); } catch (OperationCanceledException) { break; } } @@ -943,10 +951,8 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery try { var prev = state.LastDiagnostics; - var adsState = prev?.AdsState; - try { adsState = await client.ReadAdsStateAsync(ct).ConfigureAwait(false) ?? adsState; } - catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } - catch { /* keep previous */ } + // The probe's own ReadState (ProbeAsync) already fetched this; no second request. + var adsState = client.LastAdsState ?? prev?.AdsState; var appName = await ReadDiagSymbolAsync(state, client, SymAppName, TwinCATDataType.String, ct).ConfigureAwait(false) as string ?? prev?.AppName; @@ -1002,13 +1008,27 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery foreach (var state in _devices.Values) { if (state.LastDiagnostics is not { } d) continue; - var prefix = multi ? $"TwinCAT.{state.Options.DeviceName ?? state.Options.HostAddress}." : "TwinCAT."; + var name = state.Options.DeviceName ?? state.Options.HostAddress; + if (multi) + { + // One compact line per device so the host's 16-entry cap shows up to 16 devices, not 2. + var parts = new List(6); + if (d.AdsState is not null) parts.Add(d.AdsState); + if (!string.IsNullOrEmpty(d.AppName)) parts.Add(d.AppName); + if (d.OnlineChangeCnt is { } n) parts.Add("online-changes " + n.ToString(System.Globalization.CultureInfo.InvariantCulture)); + if (d.CycleTimeMs is { } cy) parts.Add("cycle " + FormatMs(cy) + " ms"); + if (d.LastExecTimeMs is { } ex) parts.Add("exec " + FormatMs(ex) + " ms"); + if (d.HeadroomMs is { } h) parts.Add("headroom " + FormatMs(h) + " ms"); + bag[$"TwinCAT.{name}"] = string.Join("; ", parts); + continue; + } + const string prefix = "TwinCAT."; if (d.AdsState is not null) bag[prefix + "AdsState"] = d.AdsState; if (!string.IsNullOrEmpty(d.AppName)) bag[prefix + "AppName"] = d.AppName; if (d.OnlineChangeCnt is { } oc) bag[prefix + "OnlineChangeCnt"] = oc.ToString(System.Globalization.CultureInfo.InvariantCulture); - if (d.CycleTimeMs is { } cy) bag[prefix + "CycleTimeMs"] = FormatMs(cy); - if (d.LastExecTimeMs is { } ex) bag[prefix + "LastExecTimeMs"] = FormatMs(ex); - if (d.JitterMs is { } j) bag[prefix + "JitterMs"] = FormatMs(j); + if (d.CycleTimeMs is { } cy2) bag[prefix + "CycleTimeMs"] = FormatMs(cy2); + if (d.LastExecTimeMs is { } ex2) bag[prefix + "LastExecTimeMs"] = FormatMs(ex2); + if (d.HeadroomMs is { } h2) bag[prefix + "HeadroomMs"] = FormatMs(h2); } return bag; } @@ -1181,6 +1201,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery throw; } device.Client = client; + device.UnavailableDiagSymbols.Clear(); // a rebuilt client may face a PLC that now exports them device.Backoff.RecordSuccess(); // 05/STAB-8: recovery resets the window immediately // Re-register every native notification onto the fresh client. Native ADS is push, so @@ -1438,9 +1459,9 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// so the loop cannot touch a disposed client. public Task? ProbeTask { get; set; } - /// Disposes the active ADS client if any. - /// Set when the target refused the ADS sum-read command (#645); the device reads one tag at a - /// time from then on. Cleared only by a fresh (driver re-initialise). + /// Set when the target answered the ADS sum-read command with "service / index group not + /// supported" (#645); the device reads one tag at a time from then on. Cleared only by a fresh + /// (driver re-initialise). public volatile bool SumReadUnsupported; /// Latest probe-sampled PLC diagnostics (#645 / #314); null before the first sample. @@ -1449,6 +1470,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// Diagnostic system symbols the target answered "not found" for — not re-asked until the client is rebuilt. public ConcurrentDictionary UnavailableDiagSymbols { get; } = new(StringComparer.Ordinal); + /// Disposes the active ADS client if any. public void DisposeClient() { Client?.Dispose(); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverFactoryExtensions.cs index 521f748f..0ba10d9c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverFactoryExtensions.cs @@ -80,6 +80,7 @@ public static class TwinCATDriverFactoryExtensions NotificationMaxDelayMs = dto.NotificationMaxDelayMs ?? 0, UseSumRead = dto.UseSumRead ?? false, SumReadMaxItems = dto.SumReadMaxItems ?? 200, + DiagnosticsEnabled = dto.DiagnosticsEnabled ?? false, }; } @@ -119,6 +120,9 @@ public static class TwinCATDriverFactoryExtensions /// Gets or sets a value indicating whether a poll's reads are batched into ADS sum-reads. public bool? UseSumRead { get; init; } + /// Gets or sets a value indicating whether probe-tick PLC diagnostics are sampled. + public bool? DiagnosticsEnabled { get; init; } + /// Gets or sets the maximum symbols per sum-read command. public int? SumReadMaxItems { get; init; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadPlanner.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadPlanner.cs new file mode 100644 index 00000000..57b40f66 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATSumReadPlanner.cs @@ -0,0 +1,103 @@ +using Beckhoff = TwinCAT.Ads; + +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +/// +/// The pure decisions behind an ADS sum-read (Gitea #645 / #310), split out of +/// so they are unit-testable without an ADS connection: which items may +/// ride a sum command, how a poll is split to fit a request budget, which overall ADS codes mean what, +/// and how one item's raw block becomes a value. +/// +internal static class TwinCATSumReadPlanner +{ + /// Hard ceiling on items per sum command (ADS documents roughly 500 sub-commands). + public const int MaxItemsCeiling = 500; + + /// Per-command byte budget: the sum of the items' read lengths plus each. + /// Keeps a poll that includes large arrays inside a modest router / CE buffer. + public const int MaxBatchBytes = 32 * 1024; + + /// Bytes of per-item ADS error code carried in a sum response. + public const int PerItemOverhead = 4; + + /// + /// True when a SCALAR may be read as raw bytes in a sum command. The sum path decodes the bytes as the + /// DECLARED type, so it needs the strict width + float/integer-kind rule (the notification rule); the + /// lenient relies on the Beckhoff value conversion a raw read + /// does not do. An item failing this but passing CheckRead must be read individually. + /// + /// The tag's declared type. + /// The PLC symbol's shape. + /// True when the raw-bytes decode is exact. + public static bool ScalarEligible(TwinCATDataType declared, TwinCATPlcSymbolShape plc) => + TwinCATTypeGuard.CheckNotification(declared, plc) is null; + + /// The service or index group itself is unsupported: the device never does sum commands. Latches. + /// The overall ADS code. + /// 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. + /// The overall ADS code. + /// True for 1792, 1797 and 1798. + public static bool IsRequestRejected(uint adsError) => adsError is 1792 or 1797 or 1798; + + /// + /// Splits items (in order) into groups whose summed length + PerItemOverhead stays within + /// . An item that alone exceeds the budget gets its own group (the caller reads a + /// one-item group individually). + /// + /// Each item's read length. + /// The byte budget per command. + /// Groups of positions into . + public static List> Partition(IReadOnlyList byteLengths, int budget) + { + var groups = new List>(); + List? current = null; + var used = 0; + for (var i = 0; i < byteLengths.Count; i++) + { + var cost = byteLengths[i] + PerItemOverhead; + if (current is null || used + cost > budget) + { + current = []; + groups.Add(current); + used = 0; + } + current.Add(i); + used += cost; + } + return groups; + } + + /// + /// Turns one item's sum-response slice into a value. A per-item ADS error maps to its status; a missing or + /// short block (a target answering with less than was asked for) is a size error for that item only. + /// + /// The declared type. + /// The item's ADS error code (0 = none). + /// The item's bytes, or null when the response had none. + /// The planned layout (a scalar is one element). + /// True to return the whole array, false for element 0. + /// True when the target's STRINGs are UTF-8. + /// Maps an ADS code to an OPC UA status (also raises symbol-version signals). + /// The value and status. + public static (object? value, uint status) DecodeItem( + TwinCATDataType type, Beckhoff.AdsErrorCode subError, ReadOnlyMemory? block, + TwinCATArrayLayout layout, bool isArray, bool utf8, Func mapError) + { + if (subError != Beckhoff.AdsErrorCode.NoError) return (null, mapError((uint)subError)); + if (block is not { } b || b.Length < layout.ByteLength) + return (null, TwinCATStatusMapper.MapAdsError(1797)); + try + { + var decoded = TwinCATArrayCodec.Decode(type, b.Span, layout, utf8); + return (isArray ? decoded : decoded.GetValue(0), TwinCATStatusMapper.Good); + } + catch (ArgumentException) + { + return (null, TwinCATStatusMapper.MapAdsError(1797)); + } + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/TwinCATDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/TwinCATDriverForm.razor index fd8fed93..56198a17 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/TwinCATDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/TwinCATDriverForm.razor @@ -54,8 +54,16 @@
-
Symbols per sum-read command (default 200).
+ min="1" max="500" class="form-control form-control-sm" /> +
Symbols per sum-read command (1-500, default 200).
+
+
+
+ + +
+
Up to 4 extra reads per device per probe tick. Default off (not yet live-gated).
@@ -167,6 +175,7 @@ public int NotificationMaxDelayMs { get; set; } = 0; public bool UseSumRead { get; set; } = false; public int SumReadMaxItems { get; set; } = 200; + public bool DiagnosticsEnabled { get; set; } = false; // Probe public bool ProbeEnabled { get; set; } = true; @@ -186,6 +195,7 @@ NotificationMaxDelayMs = o.NotificationMaxDelayMs, UseSumRead = o.UseSumRead, SumReadMaxItems = o.SumReadMaxItems, + DiagnosticsEnabled = o.DiagnosticsEnabled, ProbeEnabled = o.Probe.Enabled, ProbeIntervalSeconds = (int)o.Probe.Interval.TotalSeconds, ProbeTimeoutSeconds = (int)o.Probe.Timeout.TotalSeconds, @@ -201,6 +211,7 @@ NotificationMaxDelayMs = NotificationMaxDelayMs, UseSumRead = UseSumRead, SumReadMaxItems = SumReadMaxItems, + DiagnosticsEnabled = DiagnosticsEnabled, Probe = new TwinCATProbeOptions { Enabled = ProbeEnabled, 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 71e9960a..6103d956 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 @@ -109,10 +109,8 @@ internal class FakeTwinCATClient : ITwinCATClient return Task.FromResult>(results); } - /// Simulates reading the ADS device state. - /// The cancellation token. - /// The configured state text. - public virtual Task ReadAdsStateAsync(CancellationToken ct) => Task.FromResult(AdsStateText); + /// Gets the ADS state the "probe" last read. + public string? LastAdsState => AdsStateText; /// Simulates writing a value to a symbol. /// The path to the symbol to write. 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 2e540eaa..fb027ed5 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 @@ -12,11 +12,12 @@ public sealed class TwinCATDiagnosticsTests private const string Host1 = "ads://5.23.91.23.1.1:851"; private const string Host2 = "ads://5.23.91.24.1.1:851"; - private static TwinCATDriver NewDriver(FakeTwinCATClientFactory factory, bool probe, params (string host, string? name)[] devices) => + private static TwinCATDriver NewDriver(FakeTwinCATClientFactory factory, bool probe, bool diagnostics, params (string host, string? name)[] devices) => new(new TwinCATDriverOptions { Devices = [.. devices.Select(d => new TwinCATDeviceOptions(d.host, d.name))], Probe = new TwinCATProbeOptions { Enabled = probe, Interval = TimeSpan.FromMilliseconds(20) }, + DiagnosticsEnabled = diagnostics, }, "drv-diag", factory); private static FakeTwinCATClient PlcClient() => new() @@ -41,7 +42,7 @@ public sealed class TwinCATDiagnosticsTests [Fact] public async Task Bag_is_empty_before_the_first_sample() { - var drv = NewDriver(new FakeTwinCATClientFactory(), false, (Host1, null)); + var drv = NewDriver(new FakeTwinCATClientFactory(), false, true, (Host1, null)); await drv.InitializeAsync("{}", CancellationToken.None); var diag = drv.ShouldBeAssignableTo()!; @@ -52,7 +53,7 @@ public sealed class TwinCATDiagnosticsTests [Fact] public async Task Single_device_publishes_flat_keys() { - var drv = NewDriver(new FakeTwinCATClientFactory(), false, (Host1, null)); + var drv = NewDriver(new FakeTwinCATClientFactory(), false, true, (Host1, null)); await drv.InitializeAsync("{}", CancellationToken.None); await Sample(drv, PlcClient(), Host1); @@ -63,7 +64,7 @@ public sealed class TwinCATDiagnosticsTests bag["TwinCAT.OnlineChangeCnt"].ShouldBe("7"); bag["TwinCAT.CycleTimeMs"].ShouldBe("10.0"); bag["TwinCAT.LastExecTimeMs"].ShouldBe("2.5"); - bag["TwinCAT.JitterMs"].ShouldBe("-7.5"); + bag["TwinCAT.HeadroomMs"].ShouldBe("7.5"); DriverDiagnostics.Bound(bag).Count.ShouldBe(bag.Count); // within the host's 16-entry cap } @@ -71,15 +72,15 @@ public sealed class TwinCATDiagnosticsTests [Fact] public async Task Multiple_devices_are_prefixed() { - var drv = NewDriver(new FakeTwinCATClientFactory(), false, (Host1, "Elma01"), (Host2, null)); + var drv = NewDriver(new FakeTwinCATClientFactory(), false, true, (Host1, "Elma01"), (Host2, null)); await drv.InitializeAsync("{}", CancellationToken.None); await Sample(drv, PlcClient(), Host1); await Sample(drv, PlcClient(), Host2); var bag = ((IDriverDiagnostics)drv).GetDiagnostics(); - bag["TwinCAT.Elma01.AdsState"].ShouldBe("Run"); - bag[$"TwinCAT.{Host2}.CycleTimeMs"].ShouldBe("10.0"); + bag["TwinCAT.Elma01"].ShouldBe("Run; Plc30 App; online-changes 7; cycle 10.0 ms; exec 2.5 ms; headroom 7.5 ms"); + bag[$"TwinCAT.{Host2}"].ShouldContain("cycle 10.0 ms"); bag.ContainsKey("TwinCAT.AdsState").ShouldBeFalse(); } @@ -88,7 +89,7 @@ public sealed class TwinCATDiagnosticsTests [Fact] public async Task Missing_task_info_gives_a_partial_bag_and_is_not_re_asked() { - var drv = NewDriver(new FakeTwinCATClientFactory(), false, (Host1, null)); + var drv = NewDriver(new FakeTwinCATClientFactory(), false, true, (Host1, null)); await drv.InitializeAsync("{}", CancellationToken.None); var client = new FakeTwinCATClient { AdsStateText = "Run" }; client.Values[TwinCATDriver.SymAppName] = "Plc1"; @@ -104,7 +105,7 @@ public sealed class TwinCATDiagnosticsTests bag["TwinCAT.AdsState"].ShouldBe("Run"); bag["TwinCAT.AppName"].ShouldBe("Plc1"); bag.Keys.ShouldNotContain("TwinCAT.CycleTimeMs"); - bag.Keys.ShouldNotContain("TwinCAT.JitterMs"); + bag.Keys.ShouldNotContain("TwinCAT.HeadroomMs"); client.SingleReadCount.ShouldBe(readsAfterFirst + 1); // only AppName is read again } @@ -112,7 +113,7 @@ public sealed class TwinCATDiagnosticsTests [Fact] public async Task Failed_sample_keeps_the_last_values() { - var drv = NewDriver(new FakeTwinCATClientFactory(), false, (Host1, null)); + var drv = NewDriver(new FakeTwinCATClientFactory(), false, true, (Host1, null)); await drv.InitializeAsync("{}", CancellationToken.None); var client = PlcClient(); await Sample(drv, client, Host1); @@ -131,7 +132,7 @@ public sealed class TwinCATDiagnosticsTests public async Task Probe_loop_populates_the_bag() { var factory = new FakeTwinCATClientFactory { Customise = PlcClient }; - var drv = NewDriver(factory, true, (Host1, null)); + var drv = NewDriver(factory, true, true, (Host1, null)); await drv.InitializeAsync("{}", CancellationToken.None); try { @@ -145,4 +146,64 @@ public sealed class TwinCATDiagnosticsTests } finally { await drv.ShutdownAsync(CancellationToken.None); } } + + /// Verifies a bag for the maximum number of devices fits the host's cap (one line per device). + [Fact] + public async Task Many_devices_all_fit_the_bag_cap() + { + var hosts = Enumerable.Range(1, 12).Select(i => ($"ads://5.23.91.{i}.1.1:851", (string?)$"D{i}")).ToArray(); + var drv = NewDriver(new FakeTwinCATClientFactory(), false, true, hosts); + await drv.InitializeAsync("{}", CancellationToken.None); + foreach (var (host, _) in hosts) await Sample(drv, PlcClient(), host); + + var bag = DriverDiagnostics.Bound(((IDriverDiagnostics)drv).GetDiagnostics()); + + bag.Count.ShouldBe(12); + } + + /// Verifies sampling is opt-in: with it off the probe loop adds no reads and the bag stays empty. + [Fact] + public async Task Disabled_by_default_adds_no_reads() + { + var factory = new FakeTwinCATClientFactory { Customise = PlcClient }; + var drv = NewDriver(factory, true, false, (Host1, null)); + await drv.InitializeAsync("{}", CancellationToken.None); + try + { + for (var i = 0; i < 100 && drv.GetHostStatuses().All(h => h.State != HostState.Running); i++) + await Task.Delay(20); + await Task.Delay(100); // several more probe ticks + } + finally { await drv.ShutdownAsync(CancellationToken.None); } + + factory.Clients[0].SingleReadCount.ShouldBe(0); + ((IDriverDiagnostics)drv).GetDiagnostics().ShouldBeEmpty(); + } + + /// Verifies a symbol the PLC did not export is asked for again once the client is rebuilt. + [Fact] + public async Task Unavailable_symbols_are_cleared_when_a_new_client_is_installed() + { + var factory = new FakeTwinCATClientFactory(); + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions(Host1)], + RawTags = TwinCATRawTags.Entries([new TwinCATTagDefinition("A", Host1, "MAIN.A", TwinCATDataType.DInt)]), + Probe = new TwinCATProbeOptions { Enabled = false }, + DiagnosticsEnabled = true, + }, "drv-diag", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + var first = new FakeTwinCATClient(); + first.ReadStatuses[TwinCATDriver.SymCycleTime] = TwinCATStatusMapper.BadNodeIdUnknown; + factory.Customise = () => first; + await drv.ReadAsync(["A"], CancellationToken.None); // installs the first client + await Sample(drv, first, Host1); + drv.GetDeviceState(Host1)!.UnavailableDiagSymbols.ContainsKey(TwinCATDriver.SymCycleTime).ShouldBeTrue(); + + first.SimulateWireDrop(); + factory.Customise = () => new FakeTwinCATClient(); + await drv.ReadAsync(["A"], CancellationToken.None); // reconnect installs a new client + + drv.GetDeviceState(Host1)!.UnavailableDiagSymbols.ShouldBeEmpty(); + } } 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 new file mode 100644 index 00000000..f59fe2ba --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATSumReadPlannerTests.cs @@ -0,0 +1,107 @@ +using Shouldly; +using TwinCAT.Ads; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; + +/// The pure sum-read decisions (Gitea #645) — the part of the wire path a fake client cannot reach. +[Trait("Category", "Unit")] +public sealed class TwinCATSumReadPlannerTests +{ + private static TwinCATPlcSymbolShape Plc(string name, int size, bool isFloat = false) => + new(TwinCATPlcKind.Scalar, size, isFloat, name); + + /// Verifies the strict width + kind rule: only an exact match rides a sum command, the lenient + /// CheckRead pairs (value-converted by Beckhoff on a single read) are read singly instead. + [Theory] + [InlineData(TwinCATDataType.DInt, "DINT", 4, false, true)] + [InlineData(TwinCATDataType.DInt, "UDINT", 4, false, true)] // same width + kind: tolerated + [InlineData(TwinCATDataType.Real, "REAL", 4, true, true)] + [InlineData(TwinCATDataType.Bool, "BOOL", 1, false, true)] + [InlineData(TwinCATDataType.DInt, "INT", 2, false, false)] // would over-read into the neighbour + [InlineData(TwinCATDataType.Real, "LREAL", 8, true, false)] // half a double + [InlineData(TwinCATDataType.Real, "DINT", 4, false, false)] // bit pattern reinterpreted + [InlineData(TwinCATDataType.DInt, "REAL", 4, true, false)] + [InlineData(TwinCATDataType.Int, "DINT", 4, false, false)] // truncation + public void Scalar_eligibility_needs_exact_width_and_kind( + TwinCATDataType declared, string plcType, int plcSize, bool plcFloat, bool eligible) + { + var shape = Plc(plcType, plcSize, plcFloat); + + TwinCATSumReadPlanner.ScalarEligible(declared, shape).ShouldBe(eligible); + // The mismatching pairs are exactly the ones the lenient single-read check lets through. + if (!eligible) TwinCATTypeGuard.CheckRead(declared, shape).ShouldBeNull(); + } + + /// Verifies a string tag over a numeric symbol is not eligible. + [Fact] + public void A_string_tag_over_a_numeric_symbol_is_not_eligible() => + TwinCATSumReadPlanner.ScalarEligible(TwinCATDataType.String, Plc("DINT", 4)).ShouldBeFalse(); + + /// Verifies only service / index-group errors latch; request-size errors split without latching. + [Theory] + [InlineData(1793u, true, false)] + [InlineData(1794u, true, false)] + [InlineData(1792u, false, true)] + [InlineData(1797u, false, true)] + [InlineData(1798u, false, true)] + [InlineData(1795u, false, false)] + [InlineData(6u, 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); + TwinCATSumReadPlanner.IsRequestRejected(code).ShouldBe(rejected); + } + + /// Verifies the byte budget splits a poll and gives an oversized item a group of its own. + [Fact] + public void Partition_respects_the_byte_budget() + { + // cost = length + 4: 104, 104, 104 | 1004 (own group) | 14 + var groups = TwinCATSumReadPlanner.Partition([100, 100, 100, 1000, 10], 330); + + groups.Select(g => g.ToArray()).ToArray().ShouldBe([[0, 1, 2], [3], [4]]); + } + + /// Verifies an empty poll partitions to nothing. + [Fact] + public void Partition_of_nothing_is_empty() => + TwinCATSumReadPlanner.Partition([], 1000).ShouldBeEmpty(); + + /// Verifies a Good block decodes to a scalar / an array. + [Fact] + public void Decode_returns_scalar_and_array_values() + { + byte[] four = [0x2A, 0, 0, 0]; + var scalar = TwinCATSumReadPlanner.DecodeItem(TwinCATDataType.DInt, AdsErrorCode.NoError, four, + new TwinCATArrayLayout(1, 4), false, false, TwinCATStatusMapper.MapAdsError); + scalar.ShouldBe((42, TwinCATStatusMapper.Good)); + + byte[] arr = [1, 0, 2, 0]; + var array = TwinCATSumReadPlanner.DecodeItem(TwinCATDataType.Int, AdsErrorCode.NoError, arr, + new TwinCATArrayLayout(2, 2), true, false, TwinCATStatusMapper.MapAdsError); + array.status.ShouldBe(TwinCATStatusMapper.Good); + array.value.ShouldBe(new short[] { 1, 2 }); + } + + /// Verifies a per-item ADS error maps to that item. + [Fact] + public void Decode_maps_a_sub_error() => + TwinCATSumReadPlanner.DecodeItem(TwinCATDataType.DInt, (AdsErrorCode)1808, null, + new TwinCATArrayLayout(1, 4), false, false, TwinCATStatusMapper.MapAdsError) + .ShouldBe((null, TwinCATStatusMapper.BadNodeIdUnknown)); + + /// Verifies a short or missing block is a size error for that item, never an uncaught throw. + [Fact] + public void Decode_of_a_short_or_missing_block_is_a_per_item_size_error() + { + var expected = (default(object), TwinCATStatusMapper.MapAdsError(1797)); + TwinCATSumReadPlanner.DecodeItem(TwinCATDataType.DInt, AdsErrorCode.NoError, new byte[] { 1, 2 }, + new TwinCATArrayLayout(1, 4), false, false, TwinCATStatusMapper.MapAdsError).ShouldBe(expected); + TwinCATSumReadPlanner.DecodeItem(TwinCATDataType.DInt, AdsErrorCode.NoError, null, + new TwinCATArrayLayout(1, 4), false, false, TwinCATStatusMapper.MapAdsError).ShouldBe(expected); + } +} 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 663e309b..40471b7f 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 @@ -211,20 +211,6 @@ public sealed class TwinCATSumReadTests off.SumReadMaxItems.ShouldBe(200); } - /// Verifies which overall ADS codes mean "no sum commands here" rather than a comms fault. - [Theory] - [InlineData(1792u, true)] - [InlineData(1793u, true)] - [InlineData(1794u, true)] - [InlineData(1795u, true)] - [InlineData(1797u, true)] - [InlineData(1798u, true)] - [InlineData(6u, false)] // TargetPortNotFound - [InlineData(1861u, false)] // ClientSyncTimeOut - [InlineData(1808u, false)] // symbol not found: a per-item matter - public void Overall_ads_codes_classify_as_refusal_only_for_service_errors(uint code, bool refusal) => - AdsTwinCATClient.IsSumRefusal(code).ShouldBe(refusal); - private sealed class CapturingClient(List sink) : FakeTwinCATClient { public override Task> ReadValuesAsync( diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/TwinCATDriverPageFormSerializationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/TwinCATDriverPageFormSerializationTests.cs index 2f53ddca..aee4a088 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/TwinCATDriverPageFormSerializationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/TwinCATDriverPageFormSerializationTests.cs @@ -74,6 +74,7 @@ public sealed class TwinCATDriverPageFormSerializationTests NotificationMaxDelayMs = 100, UseSumRead = true, SumReadMaxItems = 64, + DiagnosticsEnabled = true, Probe = new TwinCATProbeOptions { Enabled = true, @@ -92,6 +93,7 @@ public sealed class TwinCATDriverPageFormSerializationTests roundTripped.NotificationMaxDelayMs.ShouldBe(100); roundTripped.UseSumRead.ShouldBeTrue(); roundTripped.SumReadMaxItems.ShouldBe(64); + roundTripped.DiagnosticsEnabled.ShouldBeTrue(); roundTripped.Probe.Enabled.ShouldBeTrue(); roundTripped.Probe.Interval.ShouldBe(TimeSpan.FromSeconds(6)); roundTripped.Probe.Timeout.ShouldBe(TimeSpan.FromSeconds(2)); From 802781b645d0b5593ce15cf3e64874e6ce53d046 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 19:33:09 -0400 Subject: [PATCH 3/3] fix(twincat): read-singly fallback for any non-comms overall sum error, once-per-device warning, wording (#645 re-review) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 2 +- docs/drivers/TwinCAT.md | 9 +++++---- .../AdsTwinCATClient.cs | 16 ++++++++++++++++ .../ITwinCATClient.cs | 10 ++++++++++ .../TwinCATDriver.cs | 15 ++++++++++++++- .../TwinCATSumReadPlanner.cs | 18 ++++++++++++++---- .../FakeTwinCATClient.cs | 9 ++++++++- .../TwinCATDiagnosticsTests.cs | 2 +- .../TwinCATSumReadPlannerTests.cs | 8 +++++--- .../TwinCATSumReadTests.cs | 16 ++++++++++++++++ 10 files changed, 90 insertions(+), 15 deletions(-) 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()