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));