diff --git a/docs/batch-read-write-performance-optimization.md b/docs/batch-read-write-performance-optimization.md new file mode 100644 index 0000000..570e374 --- /dev/null +++ b/docs/batch-read-write-performance-optimization.md @@ -0,0 +1,124 @@ +# S7CommPlus Batch Read/Write Performance Optimization + +Notes on the analysis and optimizations made to the batched read/write path of the +S7CommPlus driver, targeted at scenarios with a very large number of tags. + +## Context / problem statement + +The optimization was driven by a real-world case: + +- **~210,000 tags** read in a single logical operation. +- `_tagsPerReadRequestMax = 100`, i.e. the PLC negotiates a maximum of **100 items per + read request**. +- Requirement: strictly preserve the existing **hard-abort-on-error** behavior. Partial + results are deliberately *not* surfaced to the caller (the protocol layer produces + them, but the client layer intentionally discards them via `ThrowIfError`). Only + performance may change, not observable behavior. + +## Where the time goes (baseline analysis) + +Key methods: + +- `S7CommPlusClient.ReadTagValuesInBatches` (client-side outer batching loop) +- `S7CommPlusProtocolSession.ReadValues` / `WriteValues` (protocol-layer request building) +- `GetSerializedRequestLengthForBatching` (payload-size probe) +- `ExceedsSingleFramePayload` (comparison against the negotiated single-frame payload limit) + +Findings: + +1. **2100 sequential requests.** With 100 items per request, 210,000 tags turn into + 2100 sequential request/response round-trips. This is dominated by the PLC item limit, + so it is mostly I/O-bound and hard to change without protocol-level pipelining (see + backlog item 6). + +2. **Two nested batching layers.** The client batches into `_tagsPerReadRequestMax` + chunks, and the protocol layer *again* splits by the negotiated TPDU payload size. + +3. **O(n^2) payload check (the main avoidable CPU cost).** For every candidate item that + was added, the *entire* request was fully re-serialized to measure its byte length. + Building one chunk of `n` items therefore cost `O(n)` full serializations, i.e. + `O(n^2)` byte-level work per chunk. Across 2100 chunks this dominated the CPU time. + +## Optimizations implemented + +### 1. Remove the O(n^2) reserialization (committed) + +- Branch `feature/perf-fix-batching-payload-check`, commit `f0bccad`. +- New helper `AppendItemsWithinPayloadLimit` in `S7CommPlusProtocolSession.cs`, used by + both the read path (`ReadValues`) and the write path (`CreateWriteRequestBatch`). +- Technique: **galloping search followed by binary search** to find the exact number of + items that fit within the payload limit, using **O(log n)** full-request serializations + per chunk instead of O(n). +- Correctness basis: the serialized request length **grows monotonically** with the item + count (each item contributes at least one byte; VLQ-encoded counters never shrink as + items are added). This makes the search find exactly the same maximum as the original + linear scan. +- Preserved semantics: the **first item of a chunk is always included unconditionally** + (matching the legacy "never reject the first item" behavior), even if that item alone + would exceed the limit. +- Regression test added: `WriteBatchSplitPointMatchesLinearScanForVariousSizes` + (12 cases across power-of-two boundaries) proves identical chunk boundaries vs. the old + linear scan. + +### 2. Pool the probe MemoryStream (committed) + +- Commit `8851fed`. +- `GetSerializedRequestLengthForBatching` previously allocated a new `MemoryStream` + (and backing byte array) on **every** probe call. With galloping/binary search this can + still be thousands of allocations for large tag counts. +- Now reuses a `[ThreadStatic] MemoryStream` across calls, resetting it via + `SetLength(0)` instead of reallocating. +- Thread-safety rationale: a single batching operation runs **synchronously on one + thread**, and the helper is never called reentrantly, so `[ThreadStatic]` is safe + without locking. The session is never used concurrently for read and write. + +### 3. Optimistic fast path in the batching check (implemented) + +- Adds an optimistic path at the top of `AppendItemsWithinPayloadLimit`: add **all** + candidate items of a chunk at once and check the payload limit **once**. If it fits, + return immediately. +- Rationale: the PLC-negotiated item limit (`TagsPerReadRequestMax` / + `TagsPerWriteRequestMax`) already usually fits the negotiated PDU payload, so the whole + chunk typically fits in a single frame. +- Effect: reduces full-request serializations per chunk from **O(log n)** (item 1) to + **1** in the common case. When the optimistic attempt overflows the limit, the code + rolls back and falls back to the galloping/binary-search path from item 1, so behavior + stays identical. +- Covered by the existing split-point regression tests (which exercise the fallback). + +### 4. Conditionally skip the payload check + +Implemented as part of item 3: because the payload check is only performed when it can +actually bind (optimistic attempt fails), the expensive search path is skipped entirely +in the common case. + +## Verification + +- Build clean (0 warnings / 0 errors) for **net48** and **net8.0**. +- Full test suite: **468/468 passing** after each step, including the batch-split + regression tests in `S7CommPlusProtocolSessionReceiveTests`. +- Debug hooks used to drive the split logic from tests: e.g. + `DebugCreateWriteRequestBatchForTests` in `S7CommPlusProtocolSession.cs`. + +## Remaining backlog + +5. **Typed values instead of `object`.** The read path returns `List`, which + boxes value types. Switching to typed values would cut allocations further but is an + **API break** and was deferred. + +6. **Pipelining / parallel sessions.** Issuing multiple requests concurrently could + overlap I/O across the 2100 round-trips. Likely **not feasible** given protocol + constraints: fragmented S7CommPlus response bodies do not carry the request sequence + number, so concurrent fragment-producing requests make fragment ownership ambiguous + (see `online-multiplexing-todo.md`). One foreground request stays in flight per + physical connection. + +## Behavior preserved (invariants) + +- Hard-abort-on-error semantics unchanged; partial results still discarded at the client + layer via `ThrowIfError`. +- Chunk boundaries byte-identical to the original linear scan. +- First item of a chunk always included unconditionally. +- Serialized-length monotonicity is the core assumption behind the search-based + optimizations; if item serialization ever stopped being monotonic, the galloping/binary + search would need to be revisited. diff --git a/src/DriverTest/DriverTest.csproj b/src/DriverTest/DriverTest.csproj index a337fa3..dbd0fc8 100644 --- a/src/DriverTest/DriverTest.csproj +++ b/src/DriverTest/DriverTest.csproj @@ -22,5 +22,7 @@ 0.1.0.0 0.1.0.0 x86 + True + ..\S7CommPlusDriver.snk \ No newline at end of file diff --git a/src/S7CommPlusDriver.Tests/FakeS7CommPlusSession.cs b/src/S7CommPlusDriver.Tests/FakeS7CommPlusSession.cs index 695d96e..f54d8aa 100644 --- a/src/S7CommPlusDriver.Tests/FakeS7CommPlusSession.cs +++ b/src/S7CommPlusDriver.Tests/FakeS7CommPlusSession.cs @@ -76,6 +76,7 @@ internal sealed class FakeS7CommPlusSession : IS7CommPlusSession public Func<(int Error, S7CommPlusCpuCultureInfo CultureInfo)>? CpuCultureInfoHandler { get; set; } public Func, (int Error, S7CommPlusTextListCatalog TextLists)>? TextListsHandler { get; set; } public Func<(int Error, S7CommPlusCommunicationResourceSnapshot Resources)>? CommunicationResourcesHandler { get; set; } + public S7CommPlusCommunicationResourceSnapshot CachedCommunicationResources { get; set; } = new S7CommPlusCommunicationResourceSnapshot(); public Func, (int Error, List Values, List Errors)>? ReadHandler { get; set; } public Func, List, (int Error, List Errors)>? WriteHandler { get; set; } public Func, ushort, short, int>? CreateTagSubscriptionHandler { get; set; } @@ -254,11 +255,17 @@ public int GetTextLists(IEnumerable languageIds, out S7CommPlusTextListCata public int GetCommunicationResources(out S7CommPlusCommunicationResourceSnapshot resources) { - var result = CommunicationResourcesHandler?.Invoke() ?? (0, new S7CommPlusCommunicationResourceSnapshot()); + var result = CommunicationResourcesHandler?.Invoke() ?? (0, CachedCommunicationResources); resources = result.Resources; return result.Error; } + public int GetCachedCommunicationResources(out S7CommPlusCommunicationResourceSnapshot resources) + { + resources = CachedCommunicationResources; + return 0; + } + public int ReadValues(List addresslist, out List values, out List errors) { ReadCount++; diff --git a/src/S7CommPlusDriver.Tests/PlcTagMultiDimensionalArrayTests.cs b/src/S7CommPlusDriver.Tests/PlcTagMultiDimensionalArrayTests.cs new file mode 100644 index 0000000..16896a8 --- /dev/null +++ b/src/S7CommPlusDriver.Tests/PlcTagMultiDimensionalArrayTests.cs @@ -0,0 +1,268 @@ +using System; +using System.Linq; +using S7CommPlusDriver.ClientApi; +using Xunit; + +namespace S7CommPlusDriver.Tests +{ + /// + /// Verifies that a multidimensional primitive PLC array (e.g. Array[1..2, 1..3] of Int) is reshaped into a genuine + /// multidimensional CLR array on read, and correctly distributed back into its individual element tags on write, instead of + /// collapsing into (or requiring) a flat one-dimensional array. + /// + /// + /// Regression test for a design gap ported from the upstream fork's e91d8f2 ("Extended DataTypes") commit, which + /// introduced dedicated PlcTagXxxArrayMDim types there. This driver instead extends the existing + /// aggregate-element mechanism (, , + /// ) to reshape/distribute values without introducing new tag types. + /// + public sealed class PlcTagMultiDimensionalArrayTests + { + private static VarInfo CreateTwoByThreeIntArrayVarInfo() + { + return new VarInfo + { + Name = "DB.Matrix", + AccessSequence = "8A0E0001.F", + SymbolCrc = 0x11223344, + Softdatatype = Softdatatype.S7COMMP_SOFTDATATYPE_INT, + ArrayElementCount = 6, + ArrayDimensions = new[] + { + new S7CommPlusArrayDimension(1, 2), + new S7CommPlusArrayDimension(1, 3), + }, + }; + } + + [Fact] + public void CreateResolvedPlcTagReshapesMultiDimensionalReadIntoGenuineMultiDimensionalArray() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByThreeIntArrayVarInfo()); + + Assert.Equal(6, tag.AggregateElements.Count); + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(new ValueInt((short)(index + 1)), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(2, value.GetLength(0)); + Assert.Equal(3, value.GetLength(1)); + // Row-major: outermost (first declared) dimension varies slowest. + Assert.Equal(1, value[0, 0]); + Assert.Equal(2, value[0, 1]); + Assert.Equal(3, value[0, 2]); + Assert.Equal(4, value[1, 0]); + Assert.Equal(5, value[1, 1]); + Assert.Equal(6, value[1, 2]); + } + + [Fact] + public void CreateResolvedPlcTagDistributesMultiDimensionalWriteIntoElementTags() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByThreeIntArrayVarInfo()); + + var writeValue = new short[2, 3] { { 10, 20, 30 }, { 40, 50, 60 } }; + tag.SetValue(writeValue); + + var elementValues = tag.AggregateElements + .Select(element => ((PlcTagInt)element).Value) + .ToArray(); + Assert.Equal(new short[] { 10, 20, 30, 40, 50, 60 }, elementValues); + } + + [Fact] + public void CreateResolvedPlcTagKeepsFlatArrayForOneDimensionalAggregate() + { + var varInfo = new VarInfo + { + Name = "DB.Values", + AccessSequence = "8A0E0001.F", + SymbolCrc = 0x55667788, + Softdatatype = Softdatatype.S7COMMP_SOFTDATATYPE_INT, + ArrayElementCount = 3, + ArrayDimensions = new[] { new S7CommPlusArrayDimension(0, 3) }, + }; + + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(varInfo); + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(new ValueInt((short)(index + 1)), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(new short[] { 1, 2, 3 }, value); + } + + [Fact] + public void CreateResolvedPlcTagReshapesMultiDimensionalBoolReadIntoGenuineMultiDimensionalArray() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_BOOL, "DB.Flags", 0x22334455)); + var expected = new[] { true, false, false, true }; + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(new ValueBool(expected[index]), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(2, value.GetLength(0)); + Assert.Equal(2, value.GetLength(1)); + Assert.True(value[0, 0]); + Assert.False(value[0, 1]); + Assert.False(value[1, 0]); + Assert.True(value[1, 1]); + } + + [Fact] + public void CreateResolvedPlcTagDistributesMultiDimensionalBoolWriteIntoElementTags() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_BOOL, "DB.Flags", 0x22334455)); + tag.SetValue(new[,] { { true, false }, { false, true } }); + + var elementValues = tag.AggregateElements + .Select(element => ((PlcTagBool)element).Value) + .ToArray(); + Assert.Equal(new[] { true, false, false, true }, elementValues); + } + + [Fact] + public void CreateResolvedPlcTagReshapesMultiDimensionalRealReadIntoGenuineMultiDimensionalArray() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_REAL, "DB.Measurements", 0x33445566)); + var expected = new[] { 1.5f, 2.5f, 3.5f, 4.5f }; + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(new ValueReal(expected[index]), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(2, value.GetLength(0)); + Assert.Equal(2, value.GetLength(1)); + Assert.Equal(1.5f, value[0, 0]); + Assert.Equal(2.5f, value[0, 1]); + Assert.Equal(3.5f, value[1, 0]); + Assert.Equal(4.5f, value[1, 1]); + } + + [Fact] + public void CreateResolvedPlcTagDistributesMultiDimensionalRealWriteIntoElementTags() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_REAL, "DB.Measurements", 0x33445566)); + tag.SetValue(new[,] { { 1.5f, 2.5f }, { 3.5f, 4.5f } }); + + var elementValues = tag.AggregateElements + .Select(element => ((PlcTagReal)element).Value) + .ToArray(); + Assert.Equal(new[] { 1.5f, 2.5f, 3.5f, 4.5f }, elementValues); + } + + [Fact] + public void CreateResolvedPlcTagReshapesMultiDimensionalStringReadIntoGenuineMultiDimensionalArray() + { + var varInfo = CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_STRING, "DB.Texts", 0x44556677); + varInfo.MaxStringLength = 10; + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(varInfo); + var expected = new[] { "aa", "bb", "cc", "dd" }; + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(CreateStringReadValue(expected[index], 10), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(2, value.GetLength(0)); + Assert.Equal(2, value.GetLength(1)); + Assert.Equal("aa", value[0, 0]); + Assert.Equal("bb", value[0, 1]); + Assert.Equal("cc", value[1, 0]); + Assert.Equal("dd", value[1, 1]); + } + + [Fact] + public void CreateResolvedPlcTagDistributesMultiDimensionalStringWriteIntoElementTags() + { + var varInfo = CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_STRING, "DB.Texts", 0x44556677); + varInfo.MaxStringLength = 10; + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(varInfo); + tag.SetValue(new[,] { { "aa", "bb" }, { "cc", "dd" } }); + + var elementValues = tag.AggregateElements + .Select(element => ((PlcTagString)element).Value) + .ToArray(); + Assert.Equal(new[] { "aa", "bb", "cc", "dd" }, elementValues); + } + + [Fact] + public void CreateResolvedPlcTagReshapesMultiDimensionalDtlStructReadIntoGenuineMultiDimensionalArray() + { + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(CreateTwoByTwoVarInfo(Softdatatype.S7COMMP_SOFTDATATYPE_DTL, "DB.Timestamps", 0x66778899)); + var expected = new[] + { + new DateTime(2020, 1, 1), + new DateTime(2020, 1, 2), + new DateTime(2020, 1, 3), + new DateTime(2020, 1, 4), + }; + for (var index = 0; index < tag.AggregateElements.Count; index++) + { + tag.AggregateElements[index].ProcessReadResult(CreateDtlReadValue(expected[index]), 0); + } + tag.CompleteAggregateRead(0); + + var value = Assert.IsType(tag.GetValue()); + Assert.Equal(2, value.GetLength(0)); + Assert.Equal(2, value.GetLength(1)); + Assert.Equal(expected[0], value[0, 0]); + Assert.Equal(expected[1], value[0, 1]); + Assert.Equal(expected[2], value[1, 0]); + Assert.Equal(expected[3], value[1, 1]); + } + + private static VarInfo CreateTwoByTwoVarInfo(uint softdatatype, string name, uint symbolCrc) + { + return new VarInfo + { + Name = name, + AccessSequence = "8A0E0001.F", + SymbolCrc = symbolCrc, + Softdatatype = softdatatype, + ArrayElementCount = 4, + ArrayDimensions = new[] + { + new S7CommPlusArrayDimension(0, 2), + new S7CommPlusArrayDimension(0, 2), + }, + }; + } + + private static ValueUSIntArray CreateStringReadValue(string text, byte maxLength) + { + var bytes = System.Text.Encoding.GetEncoding("ISO-8859-1").GetBytes(text); + var b = new byte[maxLength + 2]; + b[0] = maxLength; + b[1] = (byte)bytes.Length; + Array.Copy(bytes, 0, b, 2, bytes.Length); + return new ValueUSIntArray(b); + } + + private static ValueStruct CreateDtlReadValue(DateTime value) + { + var structValue = new ValueStruct(0x02000043); + var barr = new byte[12]; + barr[0] = (byte)(value.Year >> 8); + barr[1] = (byte)value.Year; + barr[2] = (byte)value.Month; + barr[3] = (byte)value.Day; + barr[5] = (byte)value.Hour; + barr[6] = (byte)value.Minute; + barr[7] = (byte)value.Second; + structValue.AddStructElement(0x02000043, new ValueByteArray(barr)); + return structValue; + } + } +} diff --git a/src/S7CommPlusDriver.Tests/PlcTagStringMaxLengthTests.cs b/src/S7CommPlusDriver.Tests/PlcTagStringMaxLengthTests.cs new file mode 100644 index 0000000..42921b2 --- /dev/null +++ b/src/S7CommPlusDriver.Tests/PlcTagStringMaxLengthTests.cs @@ -0,0 +1,91 @@ +using S7CommPlusDriver.ClientApi; +using Xunit; + +namespace S7CommPlusDriver.Tests +{ + /// + /// Verifies that forwards the PLC-declared maximum string length to + /// , , and , and that the resulting + /// write telegram matches the declared length instead of always falling back to the built-in default of 254. + /// + /// + /// Regression test for a bug where TagFactory never received the declared maximum string length, so every + /// String/WString tag was created with the built-in default of 254, regardless of its actual PLC + /// declaration (e.g. String[10]). Writing such a tag then sent a telegram of the wrong size and with the wrong + /// declared-length header byte, which the PLC rejected. + /// + public sealed class PlcTagStringMaxLengthTests + { + [Fact] + public void TagFactoryAppliesDeclaredMaxStringLengthToScalarStringTag() + { + var tag = PlcTags.TagFactory( + "DB.Text", + new ItemAddress("8A0E0001.F"), + Softdatatype.S7COMMP_SOFTDATATYPE_STRING, + Is1Dim: false, + maxStringLength: 10); + + var stringTag = Assert.IsType(tag); + stringTag.Value = "0123456789"; // exactly 10 characters, must not throw + + var writeValue = Assert.IsType(GetWriteValue(stringTag)); + var bytes = writeValue.GetValue(); + + Assert.Equal(12, bytes.Length); // declared max length (10) + 2 header bytes + Assert.Equal(10, bytes[0]); // declared max length, not the built-in default of 254 + Assert.Equal(10, bytes[1]); // actual length + } + + [Fact] + public void TagFactoryFallsBackToDefaultMaxStringLengthWhenNotSupplied() + { + var tag = PlcTags.TagFactory( + "DB.Text", + new ItemAddress("8A0E0001.F"), + Softdatatype.S7COMMP_SOFTDATATYPE_STRING); + + var stringTag = Assert.IsType(tag); + stringTag.Value = "Hello"; + + var writeValue = Assert.IsType(GetWriteValue(stringTag)); + var bytes = writeValue.GetValue(); + + Assert.Equal(256, bytes.Length); // built-in default of 254 + 2 header bytes + Assert.Equal(254, bytes[0]); + } + + [Fact] + public void TagFactoryAppliesDeclaredMaxStringLengthToStringArrayElementTag() + { + var tag = PlcTags.TagFactory( + "DB.Texts", + new ItemAddress("8A0E0001.F"), + Softdatatype.S7COMMP_SOFTDATATYPE_STRING, + Is1Dim: true, + maxStringLength: 5); + + Assert.IsType(tag); + Assert.Equal(5, tag.GetMaxStringLength()); + } + + [Fact] + public void TagFactoryAppliesDeclaredMaxStringLengthToWStringTag() + { + var tag = PlcTags.TagFactory( + "DB.UnicodeText", + new ItemAddress("8A0E0001.F"), + Softdatatype.S7COMMP_SOFTDATATYPE_WSTRING, + Is1Dim: false, + maxStringLength: 20); + + var wstringTag = Assert.IsType(tag); + Assert.Equal(20, wstringTag.GetMaxStringLength()); + } + + private static PValue GetWriteValue(PlcTag tag) + { + return tag.GetWriteValue(); + } + } +} diff --git a/src/S7CommPlusDriver.Tests/PlcTagStructArrayTests.cs b/src/S7CommPlusDriver.Tests/PlcTagStructArrayTests.cs new file mode 100644 index 0000000..a385ce2 --- /dev/null +++ b/src/S7CommPlusDriver.Tests/PlcTagStructArrayTests.cs @@ -0,0 +1,166 @@ +using System.Linq; +using S7CommPlusDriver.ClientApi; +using Xunit; + +namespace S7CommPlusDriver.Tests +{ + /// + /// Verifies that a structural PLC member (UDT/STRUCT instance or struct array) resolves to a tag instead of + /// and that a struct array reports its PLC-declared shape through + /// without materializing one tag per element. + /// + /// + /// Regression test for a deviation ported into S7CommPlusProtocolSession.browsePlcTagBySymbol: the legacy + /// driver returned a tag for every relation-bearing member once the symbol path was exhausted, while the new driver + /// returned for everything except DTL and PlcTags.TagFactory had no + /// S7COMMP_SOFTDATATYPE_STRUCT case at all. Struct arrays are resolved through + /// because the legacy placeholder tag carried the array declaration as well. + /// + public sealed class PlcTagStructArrayTests + { + [Fact] + public void CreateResolvedPlcTagReturnsStructTagWithDeclaredArrayShape() + { + var varInfo = new VarInfo + { + Name = "fb_CaseVariables_DB.l_GML_Struct", + AccessSequence = "8A0E0001.F", + SymbolCrc = 0x10203040, + Softdatatype = Softdatatype.S7COMMP_SOFTDATATYPE_STRUCT, + ArrayElementCount = 10000, + ArrayDimensions = new[] { new S7CommPlusArrayDimension(1, 10000) }, + }; + + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(varInfo); + + var structTag = Assert.IsType(tag); + var dimension = Assert.Single(structTag.ArrayDimensions); + Assert.Equal(1, dimension.LowerBound); + Assert.Equal(10000u, dimension.ElementCount); + // Struct elements are structures, not scalar wire values, so no element tag is materialized. + Assert.Empty(structTag.AggregateElements); + } + + [Fact] + public void CreateResolvedPlcTagReturnsStructTagWithAllDeclaredDimensions() + { + var varInfo = new VarInfo + { + Name = "DB.Matrix", + AccessSequence = "8A0E0001.F", + SymbolCrc = 0x20304050, + Softdatatype = Softdatatype.S7COMMP_SOFTDATATYPE_STRUCT, + ArrayElementCount = 4, + ArrayDimensions = new[] + { + new S7CommPlusArrayDimension(1, 2), + new S7CommPlusArrayDimension(5, 2), + }, + }; + + var tag = Assert.IsType(S7CommPlusProtocolSession.CreateResolvedPlcTag(varInfo)); + + Assert.Collection( + tag.ArrayDimensions, + dimension => + { + Assert.Equal(1, dimension.LowerBound); + Assert.Equal(2u, dimension.ElementCount); + }, + dimension => + { + Assert.Equal(5, dimension.LowerBound); + Assert.Equal(2u, dimension.ElementCount); + }); + Assert.Empty(tag.AggregateElements); + } + + [Fact] + public void CreateResolvedPlcTagReturnsStructTagWithoutShapeForScalarStructMember() + { + var varInfo = new VarInfo + { + Name = "DB.Settings", + AccessSequence = "8A0E0001.F", + SymbolCrc = 0x30405060, + Softdatatype = Softdatatype.S7COMMP_SOFTDATATYPE_STRUCT, + }; + + var tag = Assert.IsType(S7CommPlusProtocolSession.CreateResolvedPlcTag(varInfo)); + + Assert.Empty(tag.ArrayDimensions); + Assert.Empty(tag.AggregateElements); + } + + [Fact] + public void CreateResolvedPlcTagKeepsElementExpansionAndShapeForPrimitiveArray() + { + var varInfo = new VarInfo + { + Name = "DB.Measurements", + AccessSequence = "8A0E0001.F", + SymbolCrc = 0x40506070, + Softdatatype = Softdatatype.S7COMMP_SOFTDATATYPE_REAL, + ArrayElementCount = 6, + ArrayDimensions = new[] + { + new S7CommPlusArrayDimension(1, 2), + new S7CommPlusArrayDimension(1, 3), + }, + }; + + var tag = S7CommPlusProtocolSession.CreateResolvedPlcTag(varInfo); + + Assert.NotNull(tag); + Assert.Equal(6, tag.AggregateElements.Count); + Assert.Equal(new[] { 2u, 3u }, tag.ArrayDimensions.Select(dimension => dimension.ElementCount).ToArray()); + Assert.Equal(new[] { 1, 1 }, tag.ArrayDimensions.Select(dimension => dimension.LowerBound).ToArray()); + } + + [Fact] + public void PlcTagStructProcessReadResultPublishesPackedStructIdentifier() + { + var tag = new PlcTagStruct("DB.Settings", new ItemAddress("8A0E0001.F.5"), Softdatatype.S7COMMP_SOFTDATATYPE_STRUCT); + + tag.ProcessReadResult(new ValueStruct(0x0200005A), 0); + + Assert.Equal(0x0200005Au, tag.Value); + Assert.Equal(PlcTagQC.TAG_QUALITY_GOOD, tag.Quality); + Assert.Equal(0UL, tag.LastReadError); + } + + [Fact] + public void PlcTagStructProcessReadResultFlagsItemErrors() + { + var tag = new PlcTagStruct("DB.Settings", new ItemAddress("8A0E0001.F.5"), Softdatatype.S7COMMP_SOFTDATATYPE_STRUCT); + + tag.ProcessReadResult(new ValueStruct(0x0200005A), 1); + + Assert.Equal(PlcTagQC.TAG_QUALITY_BAD, tag.Quality); + Assert.Equal(1UL, tag.LastReadError); + } + + [Fact] + public void PlcTagStructProcessReadResultFlagsUnexpectedValueType() + { + var tag = new PlcTagStruct("DB.Settings", new ItemAddress("8A0E0001.F.5"), Softdatatype.S7COMMP_SOFTDATATYPE_STRUCT); + + tag.ProcessReadResult(new ValueReal(1.5f), 0); + + Assert.Equal(PlcTagQC.TAG_QUALITY_BAD, tag.Quality); + } + + [Fact] + public void PlcTagStructGetWriteValueWritesPackedStructIdentifier() + { + var tag = new PlcTagStruct("DB.Settings", new ItemAddress("8A0E0001.F.5"), Softdatatype.S7COMMP_SOFTDATATYPE_STRUCT) + { + Value = 0x0200005A, + }; + + var writeValue = Assert.IsType(tag.GetWriteValue()); + + Assert.Equal(0x0200005Au, writeValue.GetValue()); + } + } +} \ No newline at end of file diff --git a/src/S7CommPlusDriver.Tests/S7CommPlusClientTests.cs b/src/S7CommPlusDriver.Tests/S7CommPlusClientTests.cs index da43374..ea27edf 100644 --- a/src/S7CommPlusDriver.Tests/S7CommPlusClientTests.cs +++ b/src/S7CommPlusDriver.Tests/S7CommPlusClientTests.cs @@ -3,6 +3,7 @@ using S7CommPlusDriver.Internal; using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.IO.Compression; using System.Linq; @@ -712,6 +713,140 @@ public async Task ReadReconnectsOnceAfterTransientDisconnect() Assert.True(result.Items[0].IsSuccess); } + [Fact] + public async Task ReconnectKeepsNegotiatedReadBatchSize() + { + const int negotiatedReadRequestMax = 25; + var observedBatchSizes = new List(); + var fake = new FakeS7CommPlusSession + { + CachedCommunicationResources = + new S7CommPlusCommunicationResourceSnapshot(negotiatedReadRequestMax, negotiatedReadRequestMax), + ReadHandler = addresses => + { + observedBatchSizes.Add(addresses.Count); + if (observedBatchSizes.Count == 1) + { + // Transient failure on the first attempt forces a reconnect and one retry. + return (S7Consts.errTCPDataReceive, new List(), new List()); + } + + return ( + 0, + addresses.Select(_ => (object?)new ValueInt(1)).ToList(), + new List(new ulong[addresses.Count])); + } + }; + var client = CreateClient(fake); + + // Must use the PlcTag overload: only that path batches by _tagsPerReadRequestMax, + // the ItemAddress overload sends all addresses in a single request. + var tags = Enumerable.Range(1, negotiatedReadRequestMax) + .Select(index => PlcTags.TagFactory( + $"DB.Value[{index}]", + new ItemAddress($"8A0E0001.{index:X}"), + Softdatatype.S7COMMP_SOFTDATATYPE_INT)) + .ToList(); + + var result = await client.ReadAsync(tags); + + Assert.True(result.Items.All(item => item.IsSuccess)); + // After the automatic reconnect the client must keep batching at the negotiated limit + // (one 25-item request per attempt), not fall back to the 20-item default which would + // produce a 20-item and a 5-item request. + Assert.Equal(new[] { negotiatedReadRequestMax, negotiatedReadRequestMax }, observedBatchSizes); + } + + [Fact] + public async Task ReconnectThenReadKeepsSteadyStateTiming() + { + // The negotiated limit is twice the hard-coded 20-item default; losing it on a reconnect + // doubles the number of read round trips and therefore the read duration. + const int negotiatedReadRequestMax = 40; + const int tagCount = 400; + const int readRequestLatencyMs = 25; + const int simulatedTimeoutMs = 250; + + var batches = new List(); + var failNextReadAttempt = false; + var fake = new FakeS7CommPlusSession + { + CachedCommunicationResources = + new S7CommPlusCommunicationResourceSnapshot(negotiatedReadRequestMax, negotiatedReadRequestMax), + ReadHandler = addresses => + { + batches.Add(addresses.Count); + + // The first attempt of the reconnect-carrying read simulates the transient timeout. + // That makes the driver reconnect and retry, and is intentionally slow. + if (failNextReadAttempt) + { + failNextReadAttempt = false; + Thread.Sleep(simulatedTimeoutMs); + return (S7Consts.errTCPDataReceive, new List(), new List()); + } + + Thread.Sleep(readRequestLatencyMs); + return ( + 0, + addresses.Select(_ => (object?)new ValueInt(1)).ToList(), + new List(new ulong[addresses.Count])); + } + }; + var client = CreateClient(fake, requestTimeoutMs: 5000); + + var tags = Enumerable.Range(1, tagCount) + .Select(index => PlcTags.TagFactory( + $"DB.Value[{index}]", + new ItemAddress($"8A0E0001.{index:X}"), + Softdatatype.S7COMMP_SOFTDATATYPE_INT)) + .ToList(); + + // Mirror the gateway: it explicitly negotiates the resources once after connecting. + // Without that step the baseline would already use the 20-item default and the reconnect + // regression would be invisible in the measured timings. + await client.GetCommunicationResourcesAsync(); + + // Baseline before the reconnect. + var baselineWatch = Stopwatch.StartNew(); + var baseline = await client.ReadAsync(tags); + baselineWatch.Stop(); + + // This call hits the simulated transient timeout, reconnects and retries. It is the + // "first call after the reconnect" and is expected to be slow, so it is only compared + // against steady state and never against the baseline. + failNextReadAttempt = true; + var reconnectWatch = Stopwatch.StartNew(); + var afterReconnect = await client.ReadAsync(tags); + reconnectWatch.Stop(); + + // Steady state after the reconnect. + var steadyWatch = Stopwatch.StartNew(); + var steady = await client.ReadAsync(tags); + steadyWatch.Stop(); + + Assert.True(baseline.Items.All(item => item.IsSuccess)); + Assert.True(afterReconnect.Items.All(item => item.IsSuccess)); + Assert.True(steady.Items.All(item => item.IsSuccess)); + + // The reconnect-carrying call must be clearly slower than steady state. + Assert.True( + reconnectWatch.ElapsedMilliseconds > steadyWatch.ElapsedMilliseconds, + $"Reconnect call ({reconnectWatch.ElapsedMilliseconds} ms) must be slower than steady state ({steadyWatch.ElapsedMilliseconds} ms)."); + + // Steady state must not roughly double compared to the baseline, i.e. the negotiated + // batch size must have survived the reconnect. + var steadyUpperBoundMs = baselineWatch.ElapsedMilliseconds * 1.5 + 40; + Assert.True( + steadyWatch.ElapsedMilliseconds <= steadyUpperBoundMs, + $"Steady-state read after reconnect took {steadyWatch.ElapsedMilliseconds} ms, baseline {baselineWatch.ElapsedMilliseconds} ms (limit {steadyUpperBoundMs:F0} ms). " + + $"Batches: [{string.Join(", ", batches)}]."); + + // Deterministic backing check independent of timing jitter: every request uses the + // negotiated limit instead of the 20-item default. + Assert.All(batches, batch => Assert.Equal(negotiatedReadRequestMax, batch)); + } + [Fact] public async Task ProtocolErrorDoesNotReconnect() { diff --git a/src/S7CommPlusDriver.Tests/S7CommPlusDriver.Tests.csproj b/src/S7CommPlusDriver.Tests/S7CommPlusDriver.Tests.csproj index 52fac51..1c6baa3 100644 --- a/src/S7CommPlusDriver.Tests/S7CommPlusDriver.Tests.csproj +++ b/src/S7CommPlusDriver.Tests/S7CommPlusDriver.Tests.csproj @@ -6,6 +6,8 @@ enable latest $(NoWarn);xUnit1051 + True + ..\S7CommPlusDriver.snk diff --git a/src/S7CommPlusDriver.Tests/S7CommPlusProtocolSessionReceiveTests.cs b/src/S7CommPlusDriver.Tests/S7CommPlusProtocolSessionReceiveTests.cs index 39a4455..c643828 100644 --- a/src/S7CommPlusDriver.Tests/S7CommPlusProtocolSessionReceiveTests.cs +++ b/src/S7CommPlusDriver.Tests/S7CommPlusProtocolSessionReceiveTests.cs @@ -254,6 +254,44 @@ public void WriteBatchAlsoHonorsPlcItemLimit() Assert.InRange(batch.SerializedLength, 1, 987); } + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(7)] + [InlineData(8)] + [InlineData(9)] + [InlineData(16)] + [InlineData(17)] + [InlineData(63)] + [InlineData(64)] + [InlineData(65)] + [InlineData(500)] + public void WriteBatchSplitPointMatchesLinearScanForVariousSizes(int payloadItemLimitByCount) + { + // Uses uniformly sized items so the exact split point can be pinned down: probe the + // serialized length of exactly N items via the real predicate, then use that length as + // the payload limit and assert the batching under test stops at exactly N items. This + // exercises the galloping/binary-search based implementation across split points that + // land before, on, and after several power-of-two boundaries. + var connection = new S7CommPlusProtocolSession(); + const int totalItems = 1000; + var addresses = Enumerable.Range(1, totalItems) + .Select(i => new ItemAddress($"8A0E{i % 0xFFFF:X4}.1")) + .ToArray(); + var values = Enumerable.Range(1, totalItems) + .Select(i => (PValue)new ValueDInt(i)) + .ToArray(); + + var probe = connection.DebugCreateWriteRequestBatchForTests(addresses, values, payloadItemLimitByCount, int.MaxValue); + var maxPayloadSize = (int)probe.SerializedLength; + + var batch = connection.DebugCreateWriteRequestBatchForTests(addresses, values, totalItems, maxPayloadSize); + + Assert.Equal(payloadItemLimitByCount, batch.ItemCount); + Assert.InRange(batch.SerializedLength, 1, maxPayloadSize); + } + [Fact] public void LargeLegacyPayloadIsProtectedOnceBeforeTransportFragmentation() { diff --git a/src/S7CommPlusDriver.snk b/src/S7CommPlusDriver.snk new file mode 100644 index 0000000..abd0895 Binary files /dev/null and b/src/S7CommPlusDriver.snk differ diff --git a/src/S7CommPlusDriver/ClientApi/Browser.cs b/src/S7CommPlusDriver/ClientApi/Browser.cs index 8fc194e..a1ae191 100644 --- a/src/S7CommPlusDriver/ClientApi/Browser.cs +++ b/src/S7CommPlusDriver/ClientApi/Browser.cs @@ -174,6 +174,7 @@ private void AddFlatSubnodes( ArrayElementCount = GetArrayElementCount(node), ArrayDimensions = GetArrayDimensions(node), ContainsIndexedArray = containsIndexedArray, + MaxStringLength = node.Vte?.OffsetInfoType?.MaxStringLength() ?? 0, }; // If an Array element of basic datatype, the Vte is here from the parent array base element and offsets not valid here. if (node.NodeType == eNodeType.Array) @@ -940,6 +941,12 @@ public class VarInfo /// public IReadOnlyList ArrayDimensions = Array.Empty(); + /// + /// Gets the maximum string length declared in the PLC for String/WString values, or zero for every + /// other datatype. Required to build a correctly sized write telegram; see . + /// + public int MaxStringLength; + /// Retains the parsed symbol path so tag creation can reuse the exact CRC inputs. internal List SymbolCrcPath; @@ -948,6 +955,11 @@ public class VarInfo /// without the CRC of the containing array declaration because the PLC rejects that CRC/address combination. /// internal bool ContainsIndexedArray; + + public override string ToString() + { + return Name; + } } internal enum eNodeType diff --git a/src/S7CommPlusDriver/ClientApi/PlcTag.cs b/src/S7CommPlusDriver/ClientApi/PlcTag.cs index 7cdef54..8450be3 100644 --- a/src/S7CommPlusDriver/ClientApi/PlcTag.cs +++ b/src/S7CommPlusDriver/ClientApi/PlcTag.cs @@ -18,6 +18,8 @@ namespace S7CommPlusDriver.ClientApi public abstract class PlcTag { private IReadOnlyList m_AggregateElements = Array.Empty(); + private IReadOnlyList m_AggregateDimensions = Array.Empty(); + private IReadOnlyList m_ArrayDimensions = Array.Empty(); public string Name; public ItemAddress Address; @@ -47,6 +49,18 @@ public abstract class PlcTag /// public object AggregateValue { get; private set; } + /// + /// Gets the PLC-declared array declaration of the member this tag was resolved from, in declaration order + /// (outermost dimension first), or an empty list for scalar members. + /// + /// + /// Every entry carries the lower bound and element count of one declared dimension exactly as written in the PLC + /// program, so Array[1..10000] of MyUdt reports one entry with lower bound 1 and element count + /// 10000. Struct/UDT arrays report their declaration through this property instead of exposing thousands of + /// individual element tags; single elements are resolved on demand through their indexed symbol. + /// + public IReadOnlyList ArrayDimensions => m_ArrayDimensions; + public PlcTag(string name, ItemAddress address, uint softdatatype) { Name = name; @@ -64,6 +78,48 @@ public virtual void ProcessWriteResult(UInt64 error) internal abstract PValue GetWriteValue(); + /// + /// Gets the PLC-declared maximum string length for , , and + /// tags, or zero for every other tag type. Used to persist and recreate the exact + /// wire length required by (see ). + /// + internal virtual int GetMaxStringLength() => 0; + + /// + /// Gets the tag's current typed value. For a multidimensional aggregate array (see and + /// ), the reshaped multidimensional array is returned because the concrete tag's Value + /// property can only expose a flat one-dimensional array. Every other tag returns its boxed public Value property. + /// + /// The boxed value, or when the concrete tag type has no public Value property. + public virtual object GetValue() + { + if (m_AggregateDimensions.Count > 1) + { + return AggregateValue; + } + var valueProperty = GetType().GetProperty("Value", BindingFlags.Public | BindingFlags.Instance); + return valueProperty?.GetValue(this); + } + + /// + /// Sets the tag's current typed value. For a multidimensional aggregate array, is expected to be + /// a multidimensional matching the PLC-declared dimensions; it is reshaped and distributed directly + /// into the individual because the concrete tag's Value property can only hold a + /// flat one-dimensional array. Every other tag assigns to its public Value property. + /// + /// The value to assign, which must be assignable to the concrete tag's Value property type. + public virtual void SetValue(object value) + { + if (m_AggregateDimensions.Count > 1 && value is Array multiDimValue) + { + DistributeMultiDimensionalValue(multiDimValue); + AggregateValue = multiDimValue; + return; + } + var valueProperty = GetType().GetProperty("Value", BindingFlags.Public | BindingFlags.Instance); + valueProperty?.SetValue(this, value); + } + internal void SetTraceAddressMetadata(VarInfo variable) { if (variable == null) @@ -98,14 +154,32 @@ protected static int CheckErrorAndType(ulong error, object valueObj, Type checkT /// /// Configures the scalar element tags that collectively represent this aggregate PLC array. /// - /// Element tags in PLC declaration order. - internal void SetAggregateElements(IReadOnlyList elements) + /// Element tags in PLC declaration order (row-major, outermost dimension first). + /// + /// The PLC-declared dimension lengths in declaration order (outermost first), or /empty for a + /// one-dimensional aggregate array. A single-element list is treated the same as . + /// + internal void SetAggregateElements(IReadOnlyList elements, IReadOnlyList dimensions = null) { m_AggregateElements = elements ?? throw new ArgumentNullException(nameof(elements)); + m_AggregateDimensions = dimensions != null && dimensions.Count > 1 ? dimensions : Array.Empty(); + } + + /// + /// Sets the PLC-declared array declaration this tag was resolved from, exposed through . + /// + /// + /// One entry per declared dimension in declaration order (outermost first), or for scalar members. + /// + internal void SetArrayDimensions(IReadOnlyList dimensions) + { + m_ArrayDimensions = dimensions ?? Array.Empty(); } /// /// Publishes a completed set of scalar element reads as one aggregate value and mirrors it into a compatible typed array tag. + /// A multidimensional aggregate array ( has more than one entry) is reshaped into a + /// genuine multidimensional instead of the concrete tag's inherently flat Value property. /// /// The first element error, or zero when every element succeeded. internal virtual void CompleteAggregateRead(ulong itemError) @@ -126,29 +200,39 @@ internal virtual void CompleteAggregateRead(ulong itemError) return; } - var values = Array.CreateInstance(elementValueProperty.PropertyType, m_AggregateElements.Count); + var flatValues = Array.CreateInstance(elementValueProperty.PropertyType, m_AggregateElements.Count); for (var index = 0; index < m_AggregateElements.Count; index++) { - values.SetValue(elementValueProperty.GetValue(m_AggregateElements[index]), index); + flatValues.SetValue(elementValueProperty.GetValue(m_AggregateElements[index]), index); } - AggregateValue = values; - var aggregateValueProperty = GetType().GetProperty("Value", BindingFlags.Instance | BindingFlags.Public); - if (aggregateValueProperty?.PropertyType == values.GetType()) + if (m_AggregateDimensions.Count > 1) { - aggregateValueProperty.SetValue(this, values); + AggregateValue = ReshapeToMultiDimensional(elementValueProperty.PropertyType, flatValues, m_AggregateDimensions); + } + else + { + AggregateValue = flatValues; + var aggregateValueProperty = GetType().GetProperty("Value", BindingFlags.Instance | BindingFlags.Public); + if (aggregateValueProperty?.PropertyType == flatValues.GetType()) + { + aggregateValueProperty.SetValue(this, flatValues); + } } Quality = PlcTagQC.TAG_QUALITY_GOOD; } /// /// Copies a typed parent array value into its scalar element tags before an aggregate write is encoded. - /// Unsupported native array types intentionally keep their explicitly populated element values unchanged. + /// Unsupported native array types intentionally keep their explicitly populated element values unchanged. A + /// multidimensional aggregate array is intentionally skipped here because already distributed + /// its values directly into at assignment time (its concrete tag's Value property + /// cannot hold a multidimensional shape). /// /// A typed parent array does not contain exactly one value per aggregate element. internal virtual void PrepareAggregateWrite() { - if (m_AggregateElements.Count == 0) + if (m_AggregateElements.Count == 0 || m_AggregateDimensions.Count > 1) { return; } @@ -177,6 +261,89 @@ internal virtual void PrepareAggregateWrite() } } + /// + /// Distributes a caller-supplied multidimensional array directly into the scalar element tags' Value properties, + /// in the same row-major order used by and the PLC's own element access IDs. + /// + /// The multidimensional array to distribute. Its shape must match . + /// The array's rank or per-dimension lengths do not match the PLC-declared dimensions. + private void DistributeMultiDimensionalValue(Array multiDimValue) + { + if (multiDimValue.Rank != m_AggregateDimensions.Count) + { + throw new ArgumentException( + $"Aggregate tag '{Name}' requires an array of rank {m_AggregateDimensions.Count}, but received rank {multiDimValue.Rank}.", + nameof(multiDimValue)); + } + for (var dimension = 0; dimension < m_AggregateDimensions.Count; dimension++) + { + if (multiDimValue.GetLength(dimension) != m_AggregateDimensions[dimension]) + { + throw new ArgumentException( + $"Aggregate tag '{Name}' requires dimension {dimension} to have length {m_AggregateDimensions[dimension]}, but received {multiDimValue.GetLength(dimension)}.", + nameof(multiDimValue)); + } + } + + var flatIndex = 0; + foreach (var indices in EnumerateRowMajorIndices(m_AggregateDimensions)) + { + var elementValueProperty = m_AggregateElements[flatIndex].GetType().GetProperty("Value", BindingFlags.Instance | BindingFlags.Public); + if (elementValueProperty == null) + { + throw new InvalidOperationException($"Aggregate element '{m_AggregateElements[flatIndex].Name}' has no writable Value property."); + } + elementValueProperty.SetValue(m_AggregateElements[flatIndex], multiDimValue.GetValue(indices)); + flatIndex++; + } + } + + /// + /// Reshapes a flat array, ordered exactly like , into a genuine multidimensional array. + /// + /// The CLR element type shared by every scalar element tag's Value property. + /// The flat values in row-major order (outermost dimension first). + /// The PLC-declared dimension lengths in declaration order (outermost first). + /// A multidimensional array of with one entry per element of . + private static Array ReshapeToMultiDimensional(Type elementType, Array flatValues, IReadOnlyList dimensions) + { + var lengths = dimensions.Select(dimension => checked((int)dimension)).ToArray(); + var multiDimValues = Array.CreateInstance(elementType, lengths); + var flatIndex = 0; + foreach (var indices in EnumerateRowMajorIndices(dimensions)) + { + multiDimValues.SetValue(flatValues.GetValue(flatIndex), indices); + flatIndex++; + } + return multiDimValues; + } + + /// + /// Enumerates every index combination of a multidimensional shape in row-major order (outermost dimension varies + /// slowest), matching the order in which the PLC protocol enumerates aggregate array element access IDs. + /// + /// The dimension lengths in declaration order (outermost first). + private static IEnumerable EnumerateRowMajorIndices(IReadOnlyList dimensions) + { + var indices = new int[dimensions.Count]; + var totalCount = 1L; + foreach (var dimension in dimensions) + { + totalCount = checked(totalCount * dimension); + } + + for (var flatIndex = 0L; flatIndex < totalCount; flatIndex++) + { + var remainder = flatIndex; + for (var dimension = dimensions.Count - 1; dimension >= 0; dimension--) + { + indices[dimension] = checked((int)(remainder % dimensions[dimension])); + remainder /= dimensions[dimension]; + } + yield return indices; + } + } + protected static string ResultString(PlcTag tag, string value ) { return String.Format("{0:X02}: {1}", tag.Quality, value); @@ -981,6 +1148,8 @@ public PlcTagString(string name, ItemAddress address, uint softdatatype, byte ma m_MaxLength = maxlength; } + internal override int GetMaxStringLength() => m_MaxLength; + public override void ProcessReadResult(object valueObj, ulong error) { LastReadError = error; @@ -1483,6 +1652,8 @@ public PlcTagWString(string name, ItemAddress address, uint softdatatype, ushort m_MaxLength = maxlength; } + internal override int GetMaxStringLength() => m_MaxLength; + public override void ProcessReadResult(object valueObj, ulong error) { LastReadError = error; @@ -1829,6 +2000,55 @@ public override string ToString() } } + /// + /// Structural PLC tag for relation-bearing members (UDT/STRUCT instances and their array elements) that are + /// resolved as a whole symbol without a remaining member path. + /// + /// + /// Mirrors the legacy driver's PlcTagStruct: the concrete value is the packed struct identifier reported by the + /// PLC, while the declared shape of a struct array is exposed through and the + /// individual members are read through their own member tags. Unlike this tag does not decode + /// packed system values, because a user-defined structure has no fixed wire layout. + /// + public class PlcTagStruct : PlcTag + { + private uint m_Value; + + /// Gets or sets the packed struct identifier transmitted by the PLC for this structural member. + public uint Value + { + get { return m_Value; } + set { m_Value = value; } + } + + public PlcTagStruct(string name, ItemAddress address, uint softdatatype) : base(name, address, softdatatype) { } + + public override void ProcessReadResult(object valueObj, ulong error) + { + LastReadError = error; + if (CheckErrorAndType(error, valueObj, typeof(ValueStruct)) == 0) + { + Value = ((ValueStruct)valueObj).GetValue(); + + Quality = PlcTagQC.TAG_QUALITY_GOOD; + } + else + { + Quality = PlcTagQC.TAG_QUALITY_BAD; + } + } + + internal override PValue GetWriteValue() + { + return new ValueStruct(Value); + } + + public override string ToString() + { + return ResultString(this, Value.ToString()); + } + } + #region Arrays public class PlcTagBoolArray : PlcTag @@ -2619,6 +2839,8 @@ public PlcTagStringArray(string name, ItemAddress address, uint softdatatype, by m_MaxLength = maxlength; } + internal override int GetMaxStringLength() => m_MaxLength; + public override void ProcessReadResult(object valueObj, ulong error) { LastReadError = error; diff --git a/src/S7CommPlusDriver/ClientApi/PlcTags.cs b/src/S7CommPlusDriver/ClientApi/PlcTags.cs index 09ad4e7..36fea8f 100644 --- a/src/S7CommPlusDriver/ClientApi/PlcTags.cs +++ b/src/S7CommPlusDriver/ClientApi/PlcTags.cs @@ -78,8 +78,13 @@ public static int WriteTags(this S7CommPlusProtocolSession conn, IEnumerable + /// + /// The PLC-declared maximum length for values, or zero when unknown. + /// Ignored for every other datatype. A value of zero falls back to the tag's built-in default of 254, matching the previous + /// behavior for callers that cannot supply this metadata. + /// /// The matching concrete tag implementation, or for an unsupported datatype. - public static PlcTag TagFactory(string name, ItemAddress address, uint softdatatype, bool Is1Dim = false) + public static PlcTag TagFactory(string name, ItemAddress address, uint softdatatype, bool Is1Dim = false, int maxStringLength = 0) { switch (softdatatype) { @@ -127,9 +132,15 @@ public static PlcTag TagFactory(string name, ItemAddress address, uint softdatat return new PlcTagDateAndTime(name, address, softdatatype); case Softdatatype.S7COMMP_SOFTDATATYPE_STRING: - if (Is1Dim) - return new PlcTagStringArray(name, address, softdatatype); - return new PlcTagString(name, address, softdatatype); + { + // A value of 0 (unknown/not supplied by the caller) keeps the tag's own built-in default (254). + var stringMaxLength = maxStringLength > 0 && maxStringLength <= byte.MaxValue + ? (byte)maxStringLength + : (byte)254; + if (Is1Dim) + return new PlcTagStringArray(name, address, softdatatype, stringMaxLength); + return new PlcTagString(name, address, softdatatype, stringMaxLength); + } case Softdatatype.S7COMMP_SOFTDATATYPE_POINTER: return new PlcTagPointer(name, address, softdatatype); @@ -139,6 +150,10 @@ public static PlcTag TagFactory(string name, ItemAddress address, uint softdatat return new PlcTagUInt(name, address, softdatatype); case Softdatatype.S7COMMP_SOFTDATATYPE_BLOCKFC: return new PlcTagUInt(name, address, softdatatype); + // Struct/UDT instances (and their array roots) have no scalar wire value; the tag reports the packed + // struct identifier and, for arrays, the PLC-declared shape through PlcTag.ArrayDimensions. + case Softdatatype.S7COMMP_SOFTDATATYPE_STRUCT: + return new PlcTagStruct(name, address, softdatatype); case Softdatatype.S7COMMP_SOFTDATATYPE_COUNTER: return new PlcTagUInt(name, address, softdatatype); @@ -178,7 +193,13 @@ public static PlcTag TagFactory(string name, ItemAddress address, uint softdatat case Softdatatype.S7COMMP_SOFTDATATYPE_WCHAR: return new PlcTagWChar(name, address, softdatatype); case Softdatatype.S7COMMP_SOFTDATATYPE_WSTRING: - return new PlcTagWString(name, address, softdatatype); + { + // A value of 0 (unknown/not supplied by the caller) keeps the tag's own built-in default (254). + var wstringMaxLength = maxStringLength > 0 && maxStringLength <= ushort.MaxValue + ? (ushort)maxStringLength + : (ushort)254; + return new PlcTagWString(name, address, softdatatype, wstringMaxLength); + } //case Softdatatype.S7COMMP_SOFTDATATYPE_VARIANT: //-> Variant isn't added inside of the instance-db as a variable! case Softdatatype.S7COMMP_SOFTDATATYPE_LTIME: diff --git a/src/S7CommPlusDriver/Core/POffsetInfoType.cs b/src/S7CommPlusDriver/Core/POffsetInfoType.cs index 1972a65..3eefd3f 100644 --- a/src/S7CommPlusDriver/Core/POffsetInfoType.cs +++ b/src/S7CommPlusDriver/Core/POffsetInfoType.cs @@ -69,6 +69,12 @@ public enum OffsetInfoType public abstract bool Is1Dim(); public abstract bool IsMDim(); + /// + /// Gets the maximum string length for a String/WString offset info, or 0 when not applicable. + /// + /// The maximum length in characters, or 0 for non-string offset info types. + public virtual int MaxStringLength() { return 0; } + public static POffsetInfoType Deserialize(Stream buffer, int offsetinfotype, out int length) { switch ((OffsetInfoType)offsetinfotype) @@ -542,6 +548,8 @@ public class POffsetInfoType_Array1Dim : POffsetInfoType, IOffsetInfoType_1Dim public override bool HasRelation() { return false; } public override bool Is1Dim() { return true; } public override bool IsMDim() { return false; } + // For a string array, UnspecifiedOffsetinfo1 carries the maximum string length of the array elements. + public override int MaxStringLength() { return UnspecifiedOffsetinfo1; } public static POffsetInfoType_Array1Dim Deserialize(Stream buffer, out int length) { @@ -599,6 +607,8 @@ public class POffsetInfoType_String: POffsetInfoType public override bool HasRelation() { return false; } public override bool Is1Dim() { return false; } public override bool IsMDim() { return false; } + // Some firmware versions swap UnspecifiedOffsetinfo1/2, so take the smaller value as the max. string length. + public override int MaxStringLength() { return Math.Min(UnspecifiedOffsetinfo1, UnspecifiedOffsetinfo2); } public static POffsetInfoType_String Deserialize(Stream buffer, out int length) { diff --git a/src/S7CommPlusDriver/Core/S7CommPlusCommunicationResourceSnapshot.cs b/src/S7CommPlusDriver/Core/S7CommPlusCommunicationResourceSnapshot.cs index cc33982..d3cb329 100644 --- a/src/S7CommPlusDriver/Core/S7CommPlusCommunicationResourceSnapshot.cs +++ b/src/S7CommPlusDriver/Core/S7CommPlusCommunicationResourceSnapshot.cs @@ -30,6 +30,20 @@ internal sealed class S7CommPlusCommunicationResourceSnapshot public int SubscriptionMemoryMax { get; private set; } public int SubscriptionMemoryFree { get; private set; } + public S7CommPlusCommunicationResourceSnapshot() + { + } + + /// + /// Creates a snapshot with explicit item limits, used by tests and internal callers that already + /// know the negotiated values. + /// + internal S7CommPlusCommunicationResourceSnapshot(int tagsPerReadRequestMax, int tagsPerWriteRequestMax) + { + TagsPerReadRequestMax = tagsPerReadRequestMax; + TagsPerWriteRequestMax = tagsPerWriteRequestMax; + } + public int ReadMax(S7CommPlusProtocolSession conn) { // Read SystemLimits diff --git a/src/S7CommPlusDriver/IS7CommPlusSession.cs b/src/S7CommPlusDriver/IS7CommPlusSession.cs index 9e8bddc..1366c4a 100644 --- a/src/S7CommPlusDriver/IS7CommPlusSession.cs +++ b/src/S7CommPlusDriver/IS7CommPlusSession.cs @@ -37,6 +37,14 @@ internal interface IS7CommPlusSession int GetCpuCultureInfo(out S7CommPlusCpuCultureInfo cultureInfo); int GetTextLists(IEnumerable languageIds, out S7CommPlusTextListCatalog textLists); int GetCommunicationResources(out S7CommPlusCommunicationResourceSnapshot resources); + + /// + /// Returns the communication resource limits that were negotiated during the connect handshake + /// without issuing additional PLC requests. Unlike , this + /// reflects the SystemLimits already read while establishing the session and is therefore safe to + /// call on every (re)connect. + /// + int GetCachedCommunicationResources(out S7CommPlusCommunicationResourceSnapshot resources); int GetActiveAlarms(out List alarmList, int languageId, Func textListResolver); int ReadValues(List addresses, out List values, out List errors); int WriteValues(List addresses, List values, out List errors); diff --git a/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs b/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs index da21d9a..5f513a4 100644 --- a/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs +++ b/src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs @@ -33,6 +33,8 @@ public class OpenSSLConnector : IDisposable private readonly byte[] m_buffer = new byte[4096]; private readonly DataBufferList m_pendingWriteList; private readonly DataBufferList m_pendingReadList; + private readonly object m_readListLock = new object(); + private readonly object m_writeListLock = new object(); private bool m_fatalError; private bool m_disposed; @@ -277,8 +279,16 @@ protected void RunSSL() while (!m_fatalError && ((!m_readRequired && dataToWrite) || dataToRead)) { - int pendingReadBefore = m_pendingReadList.Count; - int pendingWriteBefore = m_pendingWriteList.Count; + int pendingReadBefore; + int pendingWriteBefore; + lock (m_readListLock) + { + pendingReadBefore = m_pendingReadList.Count; + } + lock (m_writeListLock) + { + pendingWriteBefore = m_pendingWriteList.Count; + } bool readRequiredBefore = m_readRequired; var bioPendingBefore = Native.BIO_ctrl_pending(m_pBioOut); @@ -304,8 +314,18 @@ protected void RunSSL() GetPendingOperations(ref dataToRead, ref dataToWrite); var bioPendingAfter = Native.BIO_ctrl_pending(m_pBioOut); - if (pendingReadBefore == m_pendingReadList.Count && - pendingWriteBefore == m_pendingWriteList.Count && + int pendingReadAfter; + int pendingWriteAfter; + lock (m_readListLock) + { + pendingReadAfter = m_pendingReadList.Count; + } + lock (m_writeListLock) + { + pendingWriteAfter = m_pendingWriteList.Count; + } + if (pendingReadBefore == pendingReadAfter && + pendingWriteBefore == pendingWriteAfter && readRequiredBefore == m_readRequired && bioPendingBefore == bioPendingAfter) { @@ -341,8 +361,14 @@ public void ReadCompleted(byte[] pData, int dataLen) private void GetPendingOperations(ref bool dataToRead, ref bool dataToWrite) { - dataToRead = m_pendingReadList.Count > 0; - dataToWrite = m_pendingWriteList.Count > 0; + lock (m_readListLock) + { + dataToRead = m_pendingReadList.Count > 0; + } + lock (m_writeListLock) + { + dataToWrite = m_pendingWriteList.Count > 0; + } } private void PerformRead() @@ -437,23 +463,35 @@ private void UseData(DataBufferList list, DataBuffer pBuffer, int result) private void AppendBuffer(DataBufferList list, DataBuffer pBuffer) { - list.AddLast(pBuffer); + object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock; + lock (lockObject) + { + list.AddLast(pBuffer); + } } DataBuffer GetNextBuffer(DataBufferList list) { DataBuffer head = null; - if (list.Count > 0) + object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock; + lock (lockObject) { - head = list.First.Value; - list.RemoveFirst(); + if (list.Count > 0) + { + head = list.First.Value; + list.RemoveFirst(); + } } return head; } void PutbackBuffer(DataBufferList list, DataBuffer pBuffer) { - list.AddFirst(pBuffer); + object lockObject = (list == m_pendingReadList) ? m_readListLock : m_writeListLock; + lock (lockObject) + { + list.AddFirst(pBuffer); + } } /// diff --git a/src/S7CommPlusDriver/Properties/AssemblyInfo.cs b/src/S7CommPlusDriver/Properties/AssemblyInfo.cs index 9c30261..bbc8890 100644 --- a/src/S7CommPlusDriver/Properties/AssemblyInfo.cs +++ b/src/S7CommPlusDriver/Properties/AssemblyInfo.cs @@ -1,5 +1,5 @@ using System.Runtime.CompilerServices; -[assembly: InternalsVisibleTo("S7CommPlusDriver.Tests")] -[assembly: InternalsVisibleTo("DriverTest")] -[assembly: InternalsVisibleTo("S7CommPlusGUIBrowser")] +[assembly: InternalsVisibleTo("S7CommPlusDriver.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010055f800b37140fce7bf50c0bb2b02f81fa44af6e59c4b6bbf4fb3c058b37034a683e6a8e1efcd0cd579e636a5db262313ab556fd51a2b7cec8502ec1f7e96e807137a7e416a5311b9aea6b24eeba491b65b894dc87de28d65aedfcd00bad86527a1141502e7dbaf5c6d650da39ed0532f316575b6b4f052c64e3059091e374dbf")] +[assembly: InternalsVisibleTo("DriverTest, PublicKey=002400000480000094000000060200000024000052534131000400000100010055f800b37140fce7bf50c0bb2b02f81fa44af6e59c4b6bbf4fb3c058b37034a683e6a8e1efcd0cd579e636a5db262313ab556fd51a2b7cec8502ec1f7e96e807137a7e416a5311b9aea6b24eeba491b65b894dc87de28d65aedfcd00bad86527a1141502e7dbaf5c6d650da39ed0532f316575b6b4f052c64e3059091e374dbf")] +[assembly: InternalsVisibleTo("S7CommPlusGUIBrowser, PublicKey=002400000480000094000000060200000024000052534131000400000100010055f800b37140fce7bf50c0bb2b02f81fa44af6e59c4b6bbf4fb3c058b37034a683e6a8e1efcd0cd579e636a5db262313ab556fd51a2b7cec8502ec1f7e96e807137a7e416a5311b9aea6b24eeba491b65b894dc87de28d65aedfcd00bad86527a1141502e7dbaf5c6d650da39ed0532f316575b6b4f052c64e3059091e374dbf")] diff --git a/src/S7CommPlusDriver/S7CommPlusClient.cs b/src/S7CommPlusDriver/S7CommPlusClient.cs index 488057a..1e8d2d1 100644 --- a/src/S7CommPlusDriver/S7CommPlusClient.cs +++ b/src/S7CommPlusDriver/S7CommPlusClient.cs @@ -4,6 +4,7 @@ using S7CommPlusDriver.Internal; using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -1744,11 +1745,29 @@ private async Task ConnectCoreAsync(S7CommPlusConnectionState connectingState, C } SetState(S7CommPlusConnectionState.Connected); - _tagsPerReadRequestMax = DefaultTagsPerRequest; - _tagsPerWriteRequestMax = DefaultTagsPerRequest; + ApplyNegotiatedCommunicationResourceLimits(); _options.Logger.LogInformation("Connected to PLC {Endpoint}.", Endpoint); } + /// + /// Applies the communication resource limits that the protocol session negotiated during the connect + /// handshake instead of resetting them to the hard-coded defaults. The session reads the PLC + /// SystemLimits as part of connecting, so this keeps read/write batching at the real PLC limit even + /// across automatic reconnects, where the application does not get a chance to re-query the resources. + /// + private void ApplyNegotiatedCommunicationResourceLimits() + { + if (_session == null || _session.GetCachedCommunicationResources(out var resources) != 0) + { + _tagsPerReadRequestMax = DefaultTagsPerRequest; + _tagsPerWriteRequestMax = DefaultTagsPerRequest; + return; + } + + _tagsPerReadRequestMax = Math.Max(1, resources.TagsPerReadRequestMax); + _tagsPerWriteRequestMax = Math.Max(1, resources.TagsPerWriteRequestMax); + } + private async Task ReconnectCoreAsync(CancellationToken cancellationToken) { SetState(S7CommPlusConnectionState.Reconnecting); diff --git a/src/S7CommPlusDriver/S7CommPlusDriver.csproj b/src/S7CommPlusDriver/S7CommPlusDriver.csproj index 9326e25..1c85a6e 100644 --- a/src/S7CommPlusDriver/S7CommPlusDriver.csproj +++ b/src/S7CommPlusDriver/S7CommPlusDriver.csproj @@ -44,13 +44,11 @@ - - - 1.1.4 - - - 1.1.4 - + + + + + S7CommPlusDriver @@ -65,5 +63,7 @@ https://github.com/dotnetprojects/S7CommPlusDriver LGPL-3.0-or-later README.md + True + ..\S7CommPlusDriver.snk diff --git a/src/S7CommPlusDriver/S7CommPlusProtocolSession.Session.cs b/src/S7CommPlusDriver/S7CommPlusProtocolSession.Session.cs index a2d3143..d762bd6 100644 --- a/src/S7CommPlusDriver/S7CommPlusProtocolSession.Session.cs +++ b/src/S7CommPlusDriver/S7CommPlusProtocolSession.Session.cs @@ -160,6 +160,11 @@ int IS7CommPlusSession.GetCommunicationResources(out S7CommPlusCommunicationReso return GetCommunicationResources(out resources); } + int IS7CommPlusSession.GetCachedCommunicationResources(out S7CommPlusCommunicationResourceSnapshot resources) + { + return GetCachedCommunicationResources(out resources); + } + int IS7CommPlusSession.GetActiveAlarms(out List alarmList, int languageId, Func textListResolver) { return AlarmBrowser.GetActiveAlarms(out alarmList, languageId, textListResolver); diff --git a/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs b/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs index 58e8b11..7c43ea4 100644 --- a/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs +++ b/src/S7CommPlusDriver/S7CommPlusProtocolSession.cs @@ -679,6 +679,127 @@ private static bool ExceedsSingleFramePayload(IS7pRequest request, int maxPayloa return GetSerializedRequestLengthForBatching(request) > maxPayloadSize; } + /// + /// Appends as many candidate items as possible to a batch request without exceeding the negotiated + /// single-frame payload limit, mirroring the legacy "add one item, reserialize, roll back on overflow" + /// behavior but without the O(n^2) cost of fully reserializing the request after every single item. + /// + /// + /// The serialized request length grows monotonically with the number of appended items (each item + /// contributes at least one byte, and VLQ-encoded counters never shrink as items are added), so a + /// galloping search followed by a binary search finds the exact same maximum item count as the + /// original linear scan, using O(log n) full-request serializations per chunk instead of O(n). + /// The very first candidate item is always appended unconditionally (matching the legacy behavior of + /// never rejecting the first item of a chunk, even if it alone would exceed the payload limit). + /// + /// The maximum number of remaining candidate items available for this chunk. + /// The negotiated single-frame payload limit in bytes. + /// The request whose serialized size is checked via / side effects. + /// Appends the candidate item at the given zero-based offset (relative to the chunk start) to . + /// Removes the most recently appended candidate item from . + /// The number of candidate items that were appended to and fit within the payload limit. + private static int AppendItemsWithinPayloadLimit( + int maxCandidateCount, + int maxPayloadSize, + IS7pRequest request, + Action addItemAt, + Action removeLastItem) + { + if (maxCandidateCount <= 0) + { + return 0; + } + + // Fast path: the PLC's item-count limit (maxCandidateCount, derived from + // TagsPerReadRequestMax/TagsPerWriteRequestMax) is already negotiated to fit within the + // transport's PDU size in the vast majority of deployments, so the whole candidate set + // typically fits in a single frame. Optimistically add everything and check once; this + // costs a single full serialization instead of the O(log n) probes of the galloping/binary + // search below, which is only needed on the (uncommon) path where the optimistic attempt + // overflows the payload limit. + for (var i = 0; i < maxCandidateCount; i++) + { + addItemAt(i); + } + if (!ExceedsSingleFramePayload(request, maxPayloadSize)) + { + return maxCandidateCount; + } + for (var i = 0; i < maxCandidateCount; i++) + { + removeLastItem(); + } + + // The first item of a chunk is always included unconditionally, matching the legacy behavior. + addItemAt(0); + var added = 1; + + var step = 1; + while (added < maxCandidateCount) + { + var next = Math.Min(step, maxCandidateCount - added); + for (var k = 0; k < next; k++) + { + addItemAt(added + k); + } + + if (ExceedsSingleFramePayload(request, maxPayloadSize)) + { + for (var k = 0; k < next; k++) + { + removeLastItem(); + } + + // Binary search for the exact number of additional items (within this galloping + // step) that still fit, exploiting the monotonic growth of the serialized length. + int lo = 0, hi = next; + while (lo < hi) + { + var mid = (lo + hi + 1) / 2; + for (var k = 0; k < mid; k++) + { + addItemAt(added + k); + } + var fits = !ExceedsSingleFramePayload(request, maxPayloadSize); + for (var k = 0; k < mid; k++) + { + removeLastItem(); + } + + if (fits) + { + lo = mid; + } + else + { + hi = mid - 1; + } + } + + for (var k = 0; k < lo; k++) + { + addItemAt(added + k); + } + added += lo; + break; + } + + added += next; + step *= 2; + } + + return added; + } + + // Reused across calls to GetSerializedRequestLengthForBatching to avoid allocating a new + // MemoryStream (and its backing buffer) for every payload-size probe. Batching a large tag + // count can trigger many thousands of these probes (galloping + binary search per chunk), + // so pooling the buffer meaningfully reduces GC pressure. [ThreadStatic] keeps this safe + // without locking, since a single batching operation runs synchronously on one thread and + // this helper is never called reentrantly. + [ThreadStatic] + private static MemoryStream t_PayloadLengthProbeStream; + private static long GetSerializedRequestLengthForBatching(IS7pRequest request) { uint sessionId = request.SessionId; @@ -692,7 +813,17 @@ private static long GetSerializedRequestLengthForBatching(IS7pRequest request) request.SequenceNumber = UInt16.MaxValue; request.IntegrityId = UInt32.MaxValue; - using var stream = new MemoryStream(); + var stream = t_PayloadLengthProbeStream; + if (stream == null) + { + stream = new MemoryStream(1024); + t_PayloadLengthProbeStream = stream; + } + else + { + stream.SetLength(0); + } + request.Serialize(stream); return stream.Length; } @@ -1240,6 +1371,16 @@ public int GetCommunicationResources(out S7CommPlusCommunicationResourceSnapshot return resources.ReadFree(this); } + /// + /// Returns the communication resource limits negotiated during connection setup (SystemLimits are + /// read as part of the connect handshake) without issuing additional PLC requests. + /// + public int GetCachedCommunicationResources(out S7CommPlusCommunicationResourceSnapshot resources) + { + resources = m_CommunicationResources ?? new S7CommPlusCommunicationResourceSnapshot(); + return 0; + } + public void Disconnect() { TryDisconnect(m_ReadTimeout); @@ -1436,17 +1577,13 @@ public int ReadValues(List addresslist, out List values, ou var getMultiVarReq = new GetMultiVariablesRequest(ProtocolVersion.V2); getMultiVarReq.AddressList.Clear(); - count_perChunk = 0; - while (count_perChunk < maxTagsPerRequest && (chunk_startIndex + count_perChunk) < addresslist.Count) - { - getMultiVarReq.AddressList.Add(addresslist[chunk_startIndex + count_perChunk]); - if (count_perChunk > 0 && ExceedsSingleFramePayload(getMultiVarReq, maxPayloadSize)) - { - getMultiVarReq.AddressList.RemoveAt(getMultiVarReq.AddressList.Count - 1); - break; - } - count_perChunk++; - } + var maxCandidates = Math.Min(maxTagsPerRequest, addresslist.Count - chunk_startIndex); + count_perChunk = AppendItemsWithinPayloadLimit( + maxCandidates, + maxPayloadSize, + getMultiVarReq, + offset => getMultiVarReq.AddressList.Add(addresslist[chunk_startIndex + offset]), + () => getMultiVarReq.AddressList.RemoveAt(getMultiVarReq.AddressList.Count - 1)); res = SendS7plusFunctionObjectAndWait(getMultiVarReq, m_ReadTimeout); if (res != 0) @@ -1557,19 +1694,21 @@ private static SetMultiVariablesRequest CreateWriteRequestBatch( out int itemCount) { var request = new SetMultiVariablesRequest(ProtocolVersion.V2); - itemCount = 0; - while (itemCount < maxItems && startIndex + itemCount < addresses.Count) - { - request.AddressListVar.Add(addresses[startIndex + itemCount]); - request.ValueList.Add(values[startIndex + itemCount]); - if (itemCount > 0 && ExceedsSingleFramePayload(request, maxPayloadSize)) + var maxCandidates = Math.Min(maxItems, addresses.Count - startIndex); + itemCount = AppendItemsWithinPayloadLimit( + maxCandidates, + maxPayloadSize, + request, + offset => + { + request.AddressListVar.Add(addresses[startIndex + offset]); + request.ValueList.Add(values[startIndex + offset]); + }, + () => { request.AddressListVar.RemoveAt(request.AddressListVar.Count - 1); request.ValueList.RemoveAt(request.ValueList.Count - 1); - break; - } - itemCount++; - } + }); return request; } @@ -1996,13 +2135,12 @@ private PlcTag browsePlcTagBySymbol(uint ti_relid, ref string symbol, VarInfo va } if (varType.OffsetInfoType.HasRelation()) { - if (symbol.Length <= 0 && varType.Softdatatype == Softdatatype.S7COMMP_SOFTDATATYPE_DTL) - { - return CreateResolvedPlcTag(varInfo, varType, isAggregateArray); - } if (symbol.Length <= 0) { - return null; + // Relation-bearing members (struct/UDT instances, their arrays, FB instances and DTL) resolve to a tag + // instead of being reported as "not found" once the symbol path is exhausted, matching the legacy + // driver's browsePlcTagBySymbol(). Struct arrays report their declared shape through PlcTag.ArrayDimensions. + return CreateResolvedPlcTag(varInfo, varType, isAggregateArray); } else { @@ -2067,13 +2205,23 @@ internal static bool IsAggregatePrimitiveArray(PVartypeListElement varType, stri private static PlcTag CreateResolvedPlcTag(VarInfo varInfo, PVartypeListElement varType, bool isAggregateArray) { var address = CreateItemAddress(varInfo); - var tag = PlcTags.TagFactory(varInfo.Name, address, varType.Softdatatype, isAggregateArray); + var maxStringLength = varType.OffsetInfoType?.MaxStringLength() ?? 0; + var tag = PlcTags.TagFactory(varInfo.Name, address, varType.Softdatatype, isAggregateArray, maxStringLength); tag?.SetTraceAddressMetadata(varInfo); if (!isAggregateArray || tag == null) { return tag; } + // The caller requested the complete array, so the tag always reports the PLC-declared shape. Struct/UDT arrays + // are deliberately not expanded into element tags: their elements are individually addressable structures rather + // than scalar wire values, and a large declaration would otherwise materialize thousands of synthetic addresses. + tag.SetArrayDimensions(GetArrayDeclaration(varType)); + if (!SupportsAggregateElementExpansion(varType.Softdatatype)) + { + return tag; + } + var elementTags = GetAggregateArrayElementAccessIds(varType) .Select(accessId => { @@ -2081,14 +2229,95 @@ private static PlcTag CreateResolvedPlcTag(VarInfo varInfo, PVartypeListElement varInfo.AccessSequence, accessId, varType.OffsetInfoType.HasRelation()); - return PlcTags.TagFactory($"{varInfo.Name}[#{accessId}]", elementAddress, varType.Softdatatype); + return PlcTags.TagFactory($"{varInfo.Name}[#{accessId}]", elementAddress, varType.Softdatatype, false, maxStringLength); }) .Where(elementTag => elementTag != null) .ToList(); - tag.SetAggregateElements(elementTags); + tag.SetAggregateElements(elementTags, GetAggregateArrayDimensions(varType)); return tag; } + /// + /// Extracts the PLC-declared dimension lengths (outermost first) of an aggregate primitive array, for reshaping the + /// flat element-tag list into a genuine multidimensional array. Returns an empty list for one-dimensional arrays. + /// + /// The resolved member datatype and dimension metadata. + private static IReadOnlyList GetAggregateArrayDimensions(PVartypeListElement varType) + { + if (varType.OffsetInfoType is not IOffsetInfoType_MDim multiDimensional) + { + return Array.Empty(); + } + + // GetMdimArrayElementCount() stores dimensions inside-out; reverse to declaration order (outermost first) and + // drop trailing zero-length entries, matching GetAggregateArrayElementAccessIds's own filtering. + return multiDimensional.GetMdimArrayElementCount() + .TakeWhile(count => count > 0) + .Reverse() + .ToArray(); + } + + /// + /// Builds the PLC-declared array declaration of one member from its low-level dimension metadata, in declaration + /// order (outermost dimension first), for . + /// + /// The resolved member metadata containing the dimension information. + /// One entry per declared dimension, or an empty list for scalar members. + private static IReadOnlyList GetArrayDeclaration(PVartypeListElement varType) + { + if (varType.OffsetInfoType is IOffsetInfoType_MDim multiDimensional) + { + // The protocol stores both the counts and the lower bounds inside-out, so reverse them into declaration order. + var counts = multiDimensional.GetMdimArrayElementCount().TakeWhile(count => count > 0).ToArray(); + var lowerBounds = multiDimensional.GetMdimArrayLowerBounds(); + var declaration = new S7CommPlusArrayDimension[counts.Length]; + for (var dimension = 0; dimension < counts.Length; dimension++) + { + var protocolIndex = counts.Length - dimension - 1; + declaration[dimension] = new S7CommPlusArrayDimension(lowerBounds[protocolIndex], counts[protocolIndex]); + } + return declaration; + } + + if (varType.OffsetInfoType is IOffsetInfoType_1Dim oneDimensional) + { + var elementCount = oneDimensional.GetArrayElementCount(); + if (elementCount == 0) + { + return Array.Empty(); + } + return new[] { new S7CommPlusArrayDimension(oneDimensional.GetArrayLowerBounds(), elementCount) }; + } + + return Array.Empty(); + } + + /// + /// Determines whether an aggregate array is expanded into individually addressable element tags. + /// + /// The Siemens soft-datatype of the array member. + /// + /// for members whose elements are scalar wire values; for structural + /// members, whose tag reports only the declared array shape (see ). + /// + private static bool SupportsAggregateElementExpansion(uint softdatatype) + { + return !IsStructLikeSoftdatatype(softdatatype) && softdatatype != Softdatatype.S7COMMP_SOFTDATATYPE_FOLDER; + } + + /// + /// Determines whether a soft-datatype describes a user-defined structure or a block instance, which is transferred as + /// a packed instead of a scalar value. + /// + /// The Siemens soft-datatype identifier. + private static bool IsStructLikeSoftdatatype(uint softdatatype) + { + return softdatatype == Softdatatype.S7COMMP_SOFTDATATYPE_STRUCT + || softdatatype == Softdatatype.S7COMMP_SOFTDATATYPE_BLOCKFB + || softdatatype == Softdatatype.S7COMMP_SOFTDATATYPE_BLOCKFC + || softdatatype == Softdatatype.S7COMMP_SOFTDATATYPE_BLOCKUDT; + } + /// /// Creates a typed tag directly from one aggregate browse result, avoiding a second walk through the PLC type catalog. /// @@ -2106,13 +2335,21 @@ internal static PlcTag CreateResolvedPlcTag(VarInfo varInfo) var address = CreateItemAddress(varInfo); var isAggregateArray = varInfo.ArrayElementCount > 0; - var tag = PlcTags.TagFactory(varInfo.Name, address, varInfo.Softdatatype, isAggregateArray); + var tag = PlcTags.TagFactory(varInfo.Name, address, varInfo.Softdatatype, isAggregateArray, varInfo.MaxStringLength); tag?.SetTraceAddressMetadata(varInfo); if (!isAggregateArray || tag == null) { return tag; } + // Browse results already carry the caller-facing array declaration, so it can be attached without any conversion. + // Struct/UDT arrays keep that declaration only and are not expanded into per-element tags, see the symbol overload. + tag.SetArrayDimensions(varInfo.ArrayDimensions); + if (!SupportsAggregateElementExpansion(varInfo.Softdatatype)) + { + return tag; + } + var elementTags = GetAggregateArrayElementAccessIds(varInfo) .Select(accessId => PlcTags.TagFactory( $"{varInfo.Name}[#{accessId}]", @@ -2120,10 +2357,15 @@ internal static PlcTag CreateResolvedPlcTag(VarInfo varInfo) varInfo.AccessSequence, accessId, RequiresArrayElementRelationSelector(varInfo.Softdatatype)), - varInfo.Softdatatype)) + varInfo.Softdatatype, + false, + varInfo.MaxStringLength)) .Where(elementTag => elementTag != null) .ToList(); - tag.SetAggregateElements(elementTags); + var dimensions = varInfo.ArrayDimensions != null && varInfo.ArrayDimensions.Count > 1 + ? varInfo.ArrayDimensions.Select(dimension => dimension.ElementCount).ToArray() + : Array.Empty(); + tag.SetAggregateElements(elementTags, dimensions); return tag; } @@ -2240,9 +2482,17 @@ internal static ItemAddress CreateAggregateArrayElementAddress( return new ItemAddress($"{aggregateAccessSequence}.{accessId:X}{(requiresRelationSelector ? ".1" : string.Empty)}"); } + /// + /// Determines whether an aggregate array element address needs the protocol's literal relation selector (".1"). + /// Relation-bearing array members (structs/UDTs, block instances and DTL) place that selector between the element + /// access ID and the member access ID, exactly like calcAccessSeqFor1DimArray does for symbolic access. + /// + /// The Siemens soft-datatype of the array member. + /// when element addresses require the additional relation selector. private static bool RequiresArrayElementRelationSelector(uint softdatatype) { - return softdatatype == Softdatatype.S7COMMP_SOFTDATATYPE_DTL; + return IsStructLikeSoftdatatype(softdatatype) + || softdatatype == Softdatatype.S7COMMP_SOFTDATATYPE_DTL; } /// @@ -2321,7 +2571,9 @@ internal static bool TryCreateResolvedPrimitiveArrayElement( aggregateVariable.AccessSequence, accessId, RequiresArrayElementRelationSelector(aggregateVariable.Softdatatype)), - aggregateVariable.Softdatatype); + aggregateVariable.Softdatatype, + false, + aggregateVariable.MaxStringLength); return tag != null; } diff --git a/src/S7CommPlusDriver/S7CommPlusTagAccessorCatalog.cs b/src/S7CommPlusDriver/S7CommPlusTagAccessorCatalog.cs index 7658b5f..4a4d310 100644 --- a/src/S7CommPlusDriver/S7CommPlusTagAccessorCatalog.cs +++ b/src/S7CommPlusDriver/S7CommPlusTagAccessorCatalog.cs @@ -19,7 +19,7 @@ namespace S7CommPlusDriver /// public sealed class S7CommPlusTagAccessorCatalog { - private const int CurrentFormatVersion = 2; + private const int CurrentFormatVersion = 3; private const int MinimumSupportedFormatVersion = 1; private const int MaximumEntryCount = 2_000_000; private const int MaximumAggregateElementCount = 2_000_000; @@ -238,7 +238,7 @@ public static S7CommPlusTagAccessorCatalog ReadFrom(Stream source, string expect entries.Add( symbol, reader.ReadBoolean() - ? TagDescriptor.ReadFrom(reader, 0, version >= 2 ? symbol : null) + ? TagDescriptor.ReadFrom(reader, 0, version, version >= 2 ? symbol : null) : null); } return new S7CommPlusTagAccessorCatalog(structureHash, entries); @@ -329,16 +329,21 @@ private sealed class TagDescriptor /// The Siemens soft-datatype identifier. /// The immutable item-address descriptor. /// Scalar elements used to transfer an aggregate array. + /// + /// The PLC-declared maximum string length for String/WString tags, or zero for every other datatype. + /// private TagDescriptor( string name, uint datatype, AddressDescriptor address, - IReadOnlyList aggregateElements) + IReadOnlyList aggregateElements, + int maxStringLength) { Name = name; Datatype = datatype; _address = address; _aggregateElements = aggregateElements; + MaxStringLength = maxStringLength; } /// Gets the symbolic or generated element name. @@ -347,6 +352,9 @@ private TagDescriptor( /// Gets the Siemens soft-datatype identifier. private uint Datatype { get; } + /// Gets the PLC-declared maximum string length, or zero for every non-string datatype. + private int MaxStringLength { get; } + /// Creates a detached immutable descriptor from a resolved mutable driver tag. /// The resolved source tag. /// A recursively captured descriptor. @@ -357,14 +365,15 @@ internal static TagDescriptor FromTag(PlcTag tag) tag.Name, tag.Datatype, AddressDescriptor.FromAddress(tag.Address), - tag.AggregateElements.Select(FromTag).ToArray()); + tag.AggregateElements.Select(FromTag).ToArray(), + tag.GetMaxStringLength()); } /// Creates a fresh mutable driver tag and recursively recreates aggregate element tags. /// An accessor that shares no mutable state with previous materializations. internal PlcTag CreateTag(string nameOverride = null) { - var tag = PlcTags.TagFactory(nameOverride ?? Name, _address.CreateAddress(), Datatype, _aggregateElements.Count > 0); + var tag = PlcTags.TagFactory(nameOverride ?? Name, _address.CreateAddress(), Datatype, _aggregateElements.Count > 0, MaxStringLength); if (tag == null) { throw new InvalidDataException($"The cached datatype {Datatype} for symbol '{Name}' is not supported by this driver version."); @@ -391,13 +400,18 @@ internal void WriteTo(BinaryWriter writer, bool includeName = true) { element.WriteTo(writer); } + writer.Write(MaxStringLength); } /// Reads and validates one recursive descriptor from the catalog stream. /// The source reader. /// The current nesting depth used to reject maliciously recursive cache files. + /// + /// The catalog format version being read. Versions below 3 () predate persisted + /// string-length metadata; such caches fall back to 0, which recreates the tag's built-in default (254). + /// /// The validated descriptor. - internal static TagDescriptor ReadFrom(BinaryReader reader, int depth, string nameOverride = null) + internal static TagDescriptor ReadFrom(BinaryReader reader, int depth, int formatVersion, string nameOverride = null) { if (depth > MaximumDescriptorDepth) { @@ -414,9 +428,10 @@ internal static TagDescriptor ReadFrom(BinaryReader reader, int depth, string na var elements = new TagDescriptor[elementCount]; for (var index = 0; index < elementCount; index++) { - elements[index] = ReadFrom(reader, depth + 1); + elements[index] = ReadFrom(reader, depth + 1, formatVersion); } - return new TagDescriptor(name, datatype, address, elements); + var maxStringLength = formatVersion >= 3 ? reader.ReadInt32() : 0; + return new TagDescriptor(name, datatype, address, elements, maxStringLength); } } diff --git a/src/S7CommPlusGUIBrowser/S7CommPlusGUIBrowser.csproj b/src/S7CommPlusGUIBrowser/S7CommPlusGUIBrowser.csproj index bf88170..6febeac 100644 --- a/src/S7CommPlusGUIBrowser/S7CommPlusGUIBrowser.csproj +++ b/src/S7CommPlusGUIBrowser/S7CommPlusGUIBrowser.csproj @@ -6,7 +6,9 @@ true true true - false + false + True + ..\S7CommPlusDriver.snk 1701;1702;CA1416