Auto: twincat-2.1 — ADS Sum-read / Sum-write

Closes #310

src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs

@@ -1,7 +1,9 @@
 using System.Collections.Concurrent;
 using System.Runtime.CompilerServices;
+using System.Text;
 using TwinCAT;
 using TwinCAT.Ads;
+using TwinCAT.Ads.SumCommand;
 using TwinCAT.Ads.TypeSystem;
 using TwinCAT.TypeSystem;
 
@@ -347,6 +349,111 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
         return true;
     }
 
+    public async Task<IReadOnlyList<(object? value, uint status)>> ReadValuesAsync(
+        IReadOnlyList<TwinCATBulkReadItem> reads, CancellationToken cancellationToken)
+    {
+        if (reads.Count == 0) return Array.Empty<(object?, uint)>();
+
+        // Build the (path, AnyTypeSpecifier) request envelope. SumInstancePathAnyTypeRead
+        // batches all paths into a single ADS Sum-read round-trip (IndexGroup 0xF080 = read
+        // multiple items by symbol name with ANY-type marshalling).
+        var typeSpecs = new List<(string instancePath, AnyTypeSpecifier spec)>(reads.Count);
+        foreach (var r in reads)
+            typeSpecs.Add((r.SymbolPath, BuildAnyTypeSpecifier(r.Type, r.StringLength)));
+
+        var sumCmd = new SumInstancePathAnyTypeRead(_client, typeSpecs);
+
+        try
+        {
+            var sumResult = await sumCmd.ReadAsync(cancellationToken).ConfigureAwait(false);
+
+            // ResultSumValues2.ValueResults is a per-item array with Source / Value /
+            // ErrorCode. Even when the overall ADS request succeeds, individual sub-items can
+            // carry their own ADS error (e.g. SymbolNotFound).
+            var output = new (object? value, uint status)[reads.Count];
+            var valueResults = sumResult.ValueResults;
+            for (var i = 0; i < reads.Count; i++)
+            {
+                var vr = valueResults[i];
+                if (vr.ErrorCode != 0)
+                {
+                    output[i] = (null, TwinCATStatusMapper.MapAdsError((uint)vr.ErrorCode));
+                    continue;
+                }
+                var raw = vr.Value;
+                output[i] = (PostProcessIecTime(reads[i].Type, raw), TwinCATStatusMapper.Good);
+            }
+            return output;
+        }
+        catch (AdsErrorException ex)
+        {
+            // Whole-batch failure (no symbol-server ack, router unreachable, etc.). Map the
+            // overall ADS status onto every entry so callers see uniform status — partial-
+            // success marshalling lives in the success branch above.
+            var status = TwinCATStatusMapper.MapAdsError((uint)ex.ErrorCode);
+            var failed = new (object? value, uint status)[reads.Count];
+            for (var i = 0; i < reads.Count; i++) failed[i] = (null, status);
+            return failed;
+        }
+    }
+
+    public async Task<IReadOnlyList<uint>> WriteValuesAsync(
+        IReadOnlyList<TwinCATBulkWriteItem> writes, CancellationToken cancellationToken)
+    {
+        if (writes.Count == 0) return Array.Empty<uint>();
+
+        // SumWriteBySymbolPath internally requests symbol handles + issues a single sum-write
+        // (IndexGroup 0xF081) carrying all values. One AMS round-trip for N writes.
+        var paths = new List<string>(writes.Count);
+        var values = new object[writes.Count];
+        for (var i = 0; i < writes.Count; i++)
+        {
+            paths.Add(writes[i].SymbolPath);
+            values[i] = ConvertForWrite(writes[i].Type, writes[i].Value);
+        }
+
+        var sumCmd = new SumWriteBySymbolPath(_client, paths);
+
+        try
+        {
+            var result = await sumCmd.WriteAsync(values, cancellationToken).ConfigureAwait(false);
+            var output = new uint[writes.Count];
+            var subErrors = result.SubErrors;
+            for (var i = 0; i < writes.Count; i++)
+            {
+                // SubErrors can be null when the overall request failed before sub-dispatch —
+                // surface the OverallError on every slot in that case.
+                var code = subErrors is { Length: > 0 } && i < subErrors.Length
+                    ? (uint)subErrors[i]
+                    : (uint)result.ErrorCode;
+                output[i] = TwinCATStatusMapper.MapAdsError(code);
+            }
+            return output;
+        }
+        catch (AdsErrorException ex)
+        {
+            var status = TwinCATStatusMapper.MapAdsError((uint)ex.ErrorCode);
+            var failed = new uint[writes.Count];
+            for (var i = 0; i < writes.Count; i++) failed[i] = status;
+            return failed;
+        }
+    }
+
+    /// <summary>
+    ///     Build an <see cref="AnyTypeSpecifier"/> for one bulk-read entry. STRING uses ASCII +
+    ///     the supplied <paramref name="stringLength"/>; WSTRING uses Unicode (UTF-16). All other
+    ///     types resolve to a primitive CLR type via <see cref="MapToClrType"/>. IEC time/date
+    ///     symbols flow as their underlying UDINT (matching the per-tag path in
+    ///     <see cref="ReadValueAsync"/>) and are post-processed CLR-side after the sum-read.
+    /// </summary>
+    private static AnyTypeSpecifier BuildAnyTypeSpecifier(TwinCATDataType type, int stringLength) =>
+        type switch
+        {
+            TwinCATDataType.String => new AnyTypeSpecifier(typeof(string), stringLength, Encoding.ASCII),
+            TwinCATDataType.WString => new AnyTypeSpecifier(typeof(string), stringLength, Encoding.Unicode),
+            _ => new AnyTypeSpecifier(MapToClrType(type)),
+        };
+
     public void Dispose()
     {
         _client.AdsNotificationEx -= OnAdsNotificationEx;

src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs

@@ -48,6 +48,38 @@ public interface ITwinCATClient : IDisposable
         object? value,
         CancellationToken cancellationToken);
 
+    /// <summary>
+    ///     Bulk-read N scalar symbols in a single AMS request via Beckhoff's ADS Sum-command
+    ///     family (IndexGroup <c>0xF080..0xF084</c>). The result is a parallel array preserving
+    ///     <paramref name="reads"/> ordering — element <c>i</c>'s outcome maps to request <c>i</c>.
+    ///     Empty input returns an empty result without a wire round-trip.
+    /// </summary>
+    /// <remarks>
+    ///     <para>This is the throughput-optimised path used by <see cref="TwinCATDriver.ReadAsync"/>
+    ///     to replace the per-tag <see cref="ReadValueAsync"/> loop — one ADS sum-read for N
+    ///     symbols beats N individual round-trips by ~10× on the typical PLC link.</para>
+    ///
+    ///     <para>Whole-array reads + bit-extracted BOOL reads stay on the per-tag path because
+    ///     the Sum-command surface only marshals scalars + bitIndex needs CLR-side post-processing.
+    ///     Callers should pre-filter or fall back as appropriate.</para>
+    /// </remarks>
+    Task<IReadOnlyList<(object? value, uint status)>> ReadValuesAsync(
+        IReadOnlyList<TwinCATBulkReadItem> reads,
+        CancellationToken cancellationToken);
+
+    /// <summary>
+    ///     Bulk-write N scalar symbols in a single AMS request via Beckhoff's
+    ///     <c>SumWriteBySymbolPath</c>. Result is a parallel status array preserving
+    ///     <paramref name="writes"/> ordering. Empty input returns an empty result.
+    /// </summary>
+    /// <remarks>
+    ///     Whole-array writes + bit-RMW writes are not in scope for the bulk path — those continue
+    ///     through the per-tag <see cref="WriteValueAsync"/> path. The driver layer pre-filters.
+    /// </remarks>
+    Task<IReadOnlyList<uint>> WriteValuesAsync(
+        IReadOnlyList<TwinCATBulkWriteItem> writes,
+        CancellationToken cancellationToken);
+
     /// <summary>
     ///     Cheap health probe — returns <c>true</c> when the target's AMS state is reachable.
     ///     Used by <see cref="Core.Abstractions.IHostConnectivityProbe"/>'s probe loop.
@@ -105,3 +137,19 @@ public interface ITwinCATClientFactory
 {
     ITwinCATClient Create();
 }
+
+/// <summary>One element of an <see cref="ITwinCATClient.ReadValuesAsync"/> request — the symbol path
+/// + the IEC type for marshalling. Strings carry an explicit <paramref name="StringLength"/> for
+/// fixed-size <c>STRING(n)</c> declarations (defaults to <c>80</c> matching IEC 61131-3).</summary>
+public sealed record TwinCATBulkReadItem(
+    string SymbolPath,
+    TwinCATDataType Type,
+    int StringLength = 80);
+
+/// <summary>One element of an <see cref="ITwinCATClient.WriteValuesAsync"/> request.
+/// Mirror of <see cref="TwinCATBulkReadItem"/> with the value to push.</summary>
+public sealed record TwinCATBulkWriteItem(
+    string SymbolPath,
+    TwinCATDataType Type,
+    object? Value,
+    int StringLength = 80);

src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs

@@ -108,6 +108,14 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
 
     // ---- IReadable ----
 
+    /// <summary>
+    ///     Read the supplied tag references in as few AMS round-trips as possible.
+    ///     Tags resolved to the same <c>DeviceHostAddress</c> are bucketed + sent as one
+    ///     ADS Sum-read (<see cref="ITwinCATClient.ReadValuesAsync"/>) — N tags in one
+    ///     request beats N individual <c>ReadValueAsync</c> calls by ~10× for typical PLC
+    ///     loads. Tags with bit-extracted BOOL or whole-array shape stay on the per-tag
+    ///     path because the sum-read surface only marshals scalars.
+    /// </summary>
     public async Task<IReadOnlyList<DataValueSnapshot>> ReadAsync(
         IReadOnlyList<string> fullReferences, CancellationToken cancellationToken)
     {
@@ -115,6 +123,12 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
         var now = DateTime.UtcNow;
         var results = new DataValueSnapshot[fullReferences.Count];
 
+        // Resolve tag definitions + bucket bulk-eligible reads by device. Anything that
+        // doesn't fit the bulk surface (unknown ref, bit BOOL, whole-array) is processed
+        // through the per-tag path inline so we still return a full result array in
+        // request order.
+        var bulkBuckets = new Dictionary<string, List<(int origIndex, string symbol, TwinCATTagDefinition def, int? bitIndex)>>(StringComparer.OrdinalIgnoreCase);
+
         for (var i = 0; i < fullReferences.Count; i++)
         {
             var reference = fullReferences[i];
@@ -123,31 +137,66 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
                 results[i] = new DataValueSnapshot(null, TwinCATStatusMapper.BadNodeIdUnknown, null, now);
                 continue;
             }
-            if (!_devices.TryGetValue(def.DeviceHostAddress, out var device))
+            if (!_devices.TryGetValue(def.DeviceHostAddress, out _))
             {
                 results[i] = new DataValueSnapshot(null, TwinCATStatusMapper.BadNodeIdUnknown, null, now);
                 continue;
             }
 
+            var parsed = TwinCATSymbolPath.TryParse(def.SymbolPath);
+            var symbolName = parsed?.ToAdsSymbolName() ?? def.SymbolPath;
+            var bitIndex = parsed?.BitIndex;
+            var isWholeArray = def.ArrayDimensions is { Length: > 0 };
+            var isBitBool = bitIndex is int && def.DataType == TwinCATDataType.Bool;
+
+            if (isWholeArray || isBitBool)
+            {
+                // Per-tag fallback path — preserves bit-extract / whole-array logic in
+                // AdsTwinCATClient.ReadValueAsync.
+                results[i] = await ReadOneAsync(reference, def, symbolName, bitIndex, cancellationToken, now)
+                    .ConfigureAwait(false);
+                continue;
+            }
+
+            if (!bulkBuckets.TryGetValue(def.DeviceHostAddress, out var bucket))
+            {
+                bucket = new List<(int, string, TwinCATTagDefinition, int?)>();
+                bulkBuckets[def.DeviceHostAddress] = bucket;
+            }
+            bucket.Add((i, symbolName, def, bitIndex));
+        }
+
+        // One sum-read per device bucket. Ordering inside a bucket is preserved by the
+        // (origIndex, ...) tuple — the result array entry comes from the parallel index.
+        foreach (var (hostAddress, bucket) in bulkBuckets)
+        {
+            if (!_devices.TryGetValue(hostAddress, out var device)) continue;
             try
             {
                 var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false);
-                var parsed = TwinCATSymbolPath.TryParse(def.SymbolPath);
-                var symbolName = parsed?.ToAdsSymbolName() ?? def.SymbolPath;
-                var (value, status) = await client.ReadValueAsync(
-                    symbolName, def.DataType, parsed?.BitIndex, def.ArrayDimensions, cancellationToken).ConfigureAwait(false);
+                var items = new TwinCATBulkReadItem[bucket.Count];
+                for (var k = 0; k < bucket.Count; k++)
+                    items[k] = new TwinCATBulkReadItem(bucket[k].symbol, bucket[k].def.DataType);
 
-                results[i] = new DataValueSnapshot(value, status, now, now);
-                if (status == TwinCATStatusMapper.Good)
-                    _health = new DriverHealth(DriverState.Healthy, now, null);
-                else
-                    _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead,
-                        $"ADS status {status:X8} reading {reference}");
+                var bulk = await client.ReadValuesAsync(items, cancellationToken).ConfigureAwait(false);
+
+                for (var k = 0; k < bucket.Count; k++)
+                {
+                    var (origIndex, _, def, _) = bucket[k];
+                    var (value, status) = bulk[k];
+                    results[origIndex] = new DataValueSnapshot(value, status, now, now);
+                    if (status == TwinCATStatusMapper.Good)
+                        _health = new DriverHealth(DriverState.Healthy, now, null);
+                    else
+                        _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead,
+                            $"ADS status {status:X8} reading {fullReferences[origIndex]}");
+                }
             }
             catch (OperationCanceledException) { throw; }
             catch (Exception ex)
             {
-                results[i] = new DataValueSnapshot(null, TwinCATStatusMapper.BadCommunicationError, null, now);
+                foreach (var (origIndex, _, _, _) in bucket)
+                    results[origIndex] = new DataValueSnapshot(null, TwinCATStatusMapper.BadCommunicationError, null, now);
                 _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message);
             }
         }
@@ -155,14 +204,53 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
         return results;
     }
 
+    private async Task<DataValueSnapshot> ReadOneAsync(
+        string reference, TwinCATTagDefinition def, string symbolName, int? bitIndex,
+        CancellationToken cancellationToken, DateTime timestamp)
+    {
+        if (!_devices.TryGetValue(def.DeviceHostAddress, out var device))
+            return new DataValueSnapshot(null, TwinCATStatusMapper.BadNodeIdUnknown, null, timestamp);
+
+        try
+        {
+            var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false);
+            var (value, status) = await client.ReadValueAsync(
+                symbolName, def.DataType, bitIndex, def.ArrayDimensions, cancellationToken).ConfigureAwait(false);
+
+            if (status == TwinCATStatusMapper.Good)
+                _health = new DriverHealth(DriverState.Healthy, timestamp, null);
+            else
+                _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead,
+                    $"ADS status {status:X8} reading {reference}");
+
+            return new DataValueSnapshot(value, status, timestamp, timestamp);
+        }
+        catch (OperationCanceledException) { throw; }
+        catch (Exception ex)
+        {
+            _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message);
+            return new DataValueSnapshot(null, TwinCATStatusMapper.BadCommunicationError, null, timestamp);
+        }
+    }
+
     // ---- IWritable ----
 
+    /// <summary>
+    ///     Write the supplied requests, bucketing scalar writes by device + dispatching
+    ///     each bucket as one ADS Sum-write. Bit-RMW BOOL writes + whole-array writes use
+    ///     the per-tag <see cref="ITwinCATClient.WriteValueAsync"/> path so the per-parent
+    ///     RMW lock stays in play.
+    /// </summary>
     public async Task<IReadOnlyList<WriteResult>> WriteAsync(
         IReadOnlyList<WriteRequest> writes, CancellationToken cancellationToken)
     {
         ArgumentNullException.ThrowIfNull(writes);
         var results = new WriteResult[writes.Count];
 
+        // Bucket scalar writes by device. Bit-BOOL + whole-array writes route through the
+        // per-tag fallback below.
+        var bulkBuckets = new Dictionary<string, List<(int origIndex, string symbol, TwinCATTagDefinition def, object? value)>>(StringComparer.OrdinalIgnoreCase);
+
         for (var i = 0; i < writes.Count; i++)
         {
             var w = writes[i];
@@ -176,38 +264,68 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
                 results[i] = new WriteResult(TwinCATStatusMapper.BadNotWritable);
                 continue;
             }
-            if (!_devices.TryGetValue(def.DeviceHostAddress, out var device))
+            if (!_devices.TryGetValue(def.DeviceHostAddress, out _))
             {
                 results[i] = new WriteResult(TwinCATStatusMapper.BadNodeIdUnknown);
                 continue;
             }
 
+            var parsed = TwinCATSymbolPath.TryParse(def.SymbolPath);
+            var symbolName = parsed?.ToAdsSymbolName() ?? def.SymbolPath;
+            var bitIndex = parsed?.BitIndex;
+            var isWholeArray = def.ArrayDimensions is { Length: > 0 };
+            var isBitBool = bitIndex is int && def.DataType == TwinCATDataType.Bool;
+
+            if (isWholeArray || isBitBool)
+            {
+                results[i] = await WriteOneAsync(def, symbolName, bitIndex, w.Value, cancellationToken)
+                    .ConfigureAwait(false);
+                continue;
+            }
+
+            if (!bulkBuckets.TryGetValue(def.DeviceHostAddress, out var bucket))
+            {
+                bucket = new List<(int, string, TwinCATTagDefinition, object?)>();
+                bulkBuckets[def.DeviceHostAddress] = bucket;
+            }
+            bucket.Add((i, symbolName, def, w.Value));
+        }
+
+        foreach (var (hostAddress, bucket) in bulkBuckets)
+        {
+            if (!_devices.TryGetValue(hostAddress, out var device)) continue;
             try
             {
                 var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false);
-                var parsed = TwinCATSymbolPath.TryParse(def.SymbolPath);
-                var symbolName = parsed?.ToAdsSymbolName() ?? def.SymbolPath;
-                var status = await client.WriteValueAsync(
-                    symbolName, def.DataType, parsed?.BitIndex, def.ArrayDimensions, w.Value, cancellationToken).ConfigureAwait(false);
-                results[i] = new WriteResult(status);
+                var items = new TwinCATBulkWriteItem[bucket.Count];
+                for (var k = 0; k < bucket.Count; k++)
+                    items[k] = new TwinCATBulkWriteItem(bucket[k].symbol, bucket[k].def.DataType, bucket[k].value);
+
+                var bulk = await client.WriteValuesAsync(items, cancellationToken).ConfigureAwait(false);
+                for (var k = 0; k < bucket.Count; k++)
+                    results[bucket[k].origIndex] = new WriteResult(bulk[k]);
             }
             catch (OperationCanceledException) { throw; }
-            catch (NotSupportedException nse)
-            {
-                results[i] = new WriteResult(TwinCATStatusMapper.BadNotSupported);
-                _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, nse.Message);
-            }
             catch (Exception ex) when (ex is FormatException or InvalidCastException)
             {
-                results[i] = new WriteResult(TwinCATStatusMapper.BadTypeMismatch);
+                foreach (var (origIndex, _, _, _) in bucket)
+                    results[origIndex] = new WriteResult(TwinCATStatusMapper.BadTypeMismatch);
             }
             catch (OverflowException)
             {
-                results[i] = new WriteResult(TwinCATStatusMapper.BadOutOfRange);
+                foreach (var (origIndex, _, _, _) in bucket)
+                    results[origIndex] = new WriteResult(TwinCATStatusMapper.BadOutOfRange);
+            }
+            catch (NotSupportedException nse)
+            {
+                foreach (var (origIndex, _, _, _) in bucket)
+                    results[origIndex] = new WriteResult(TwinCATStatusMapper.BadNotSupported);
+                _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, nse.Message);
             }
             catch (Exception ex)
             {
-                results[i] = new WriteResult(TwinCATStatusMapper.BadCommunicationError);
+                foreach (var (origIndex, _, _, _) in bucket)
+                    results[origIndex] = new WriteResult(TwinCATStatusMapper.BadCommunicationError);
                 _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message);
             }
         }
@@ -215,6 +333,40 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
         return results;
     }
 
+    private async Task<WriteResult> WriteOneAsync(
+        TwinCATTagDefinition def, string symbolName, int? bitIndex, object? value, CancellationToken cancellationToken)
+    {
+        if (!_devices.TryGetValue(def.DeviceHostAddress, out var device))
+            return new WriteResult(TwinCATStatusMapper.BadNodeIdUnknown);
+
+        try
+        {
+            var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false);
+            var status = await client.WriteValueAsync(
+                symbolName, def.DataType, bitIndex, def.ArrayDimensions, value, cancellationToken).ConfigureAwait(false);
+            return new WriteResult(status);
+        }
+        catch (OperationCanceledException) { throw; }
+        catch (NotSupportedException nse)
+        {
+            _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, nse.Message);
+            return new WriteResult(TwinCATStatusMapper.BadNotSupported);
+        }
+        catch (Exception ex) when (ex is FormatException or InvalidCastException)
+        {
+            return new WriteResult(TwinCATStatusMapper.BadTypeMismatch);
+        }
+        catch (OverflowException)
+        {
+            return new WriteResult(TwinCATStatusMapper.BadOutOfRange);
+        }
+        catch (Exception ex)
+        {
+            _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message);
+            return new WriteResult(TwinCATStatusMapper.BadCommunicationError);
+        }
+    }
+
     // ---- ITagDiscovery ----
 
     public async Task DiscoverAsync(IAddressSpaceBuilder builder, CancellationToken cancellationToken)