Repository navigation
Feature/optimize 08 strong nameing - #18
Merged
jogibear9988 merged 30 commits intoOct 7, 2026
Merged
jogibear9988 merged 30 commits into
jogibear9988 merged 30 commits into
Conversation
Port net functional changes from fork package 01 (DataTypes extension / string array fix, commits 6aef271/519cdeb/ffe8daa): - PlcTag: add virtual GetValue()/SetValue(object) reflecting the concrete tag's public Value property, for generic tag value access. - POffsetInfoType: add virtual MaxStringLength(), returning 0 by default, UnspecifiedOffsetinfo1 for Array1Dim (string array max length), and Min(UnspecifiedOffsetinfo1, UnspecifiedOffsetinfo2) for String (fixes cases where firmware swaps both fields). All other changes from this fork package are already superseded by the target repo's own PlcTag/PlcTags implementation and are documented as excluded in package 16.
…ster' Add PlcTag.GetValue/SetValue and POffsetInfoType.MaxStringLength See merge request pohg/stiwas7commplusdriver!1
…r lists (GML-1187)
Fix thread-unsafe LinkedList access in OpenSSLConnector pending buffer lists (GML-1187) See merge request pohg/stiwas7commplusdriver!2
…ING/WSTRING writes to fail
Feature/17 string write maxlength fix See merge request pohg/stiwas7commplusdriver!3
…ltidimensional CLR arrays
Feature/18 multidim array shape fix See merge request pohg/stiwas7commplusdriver!4
ExceedsSingleFramePayload previously reserialized the entire batch request from scratch on every single item added while building a chunk in ReadValues and CreateWriteRequestBatch (used by WriteValues). For large tag counts (e.g. 210k tags at 100 tags/request) this caused ~n^2 serializations per chunk and a large number of MemoryStream allocations. Introduces AppendItemsWithinPayloadLimit, which uses a galloping search followed by a binary search to find the exact same maximum item count as the previous linear scan (the serialized length grows monotonically with the number of appended items), reducing full-request serializations per chunk from O(n) to O(log n). Behavior is unchanged: same chunk contents, same first-item-always-included semantics, same error handling on communication failures (hard abort, no partial-result handling changed). Adds a theory-based regression test asserting the batch split point matches exactly across several sizes, including power-of-two boundaries where the galloping search transitions to its binary-search phase.
…atching GetSerializedRequestLengthForBatching is called repeatedly per chunk by the galloping/binary-search batching introduced previously (potentially thousands of times for large tag counts). It previously allocated a new MemoryStream (and backing byte array) on every call just to measure the serialized request length. Reuses a [ThreadStatic] MemoryStream across calls, resetting its length instead of reallocating. Safe without locking because a single batching operation runs synchronously on one thread and this helper is never called reentrantly. Reduces GC pressure for large batched read/write operations without changing any observable behavior.
perf: reduce O(n^2) payload-check serialization in read/write batching See merge request pohg/stiwas7commplusdriver!5
…d-check' into master(gitlab)
Restore the legacy driver behavior in browsePlcTagBySymbol: a relation-bearing member (struct/UDT instance, struct array, FB instance or DTL) resolves to a tag once the symbolic path is exhausted instead of returning null. The previous port restricted that branch to DTL, so GetTagBySymbolAsync threw for struct symbols and GetTagsBySymbolsAsync silently dropped them. Add the missing PlcTagStruct tag and the S7COMMP_SOFTDATATYPE_STRUCT case in TagFactory, which both had no counterpart in this repository, so the restored branch would still have produced null. Expose the PLC-declared array declaration through the new PlcTag.ArrayDimensions property (lower bound and element count per dimension, outermost first) and populate it from the low-level dimension metadata on the symbolic path and from VarInfo.ArrayDimensions on the browse path. Struct/UDT arrays report their shape this way instead of materializing one tag per element; single elements stay resolvable through their indexed symbol. Align RequiresArrayElementRelationSelector with HasRelation() so struct/UDT element addresses keep the literal .1 relation selector. Add PlcTagStructArrayTests covering the struct tag, the declared shape, the selective element expansion and the packed struct read/write behavior.
Reuse the SystemLimits negotiated during the connect handshake instead of resetting _tagsPerReadRequestMax/_tagsPerWriteRequestMax to the hard-coded default on every (re)connect via new GetCachedCommunicationResources.
Port net functional changes from fork package 01 (DataTypes extension / string array fix, commits 6aef271/519cdeb/ffe8daa): - PlcTag: add virtual GetValue()/SetValue(object) reflecting the concrete tag's public Value property, for generic tag value access. - POffsetInfoType: add virtual MaxStringLength(), returning 0 by default, UnspecifiedOffsetinfo1 for Array1Dim (string array max length), and Min(UnspecifiedOffsetinfo1, UnspecifiedOffsetinfo2) for String (fixes cases where firmware swaps both fields). All other changes from this fork package are already superseded by the target repo's own PlcTag/PlcTags implementation and are documented as excluded in package 16.
…r lists (GML-1187)
…ING/WSTRING writes to fail
…ltidimensional CLR arrays
ExceedsSingleFramePayload previously reserialized the entire batch request from scratch on every single item added while building a chunk in ReadValues and CreateWriteRequestBatch (used by WriteValues). For large tag counts (e.g. 210k tags at 100 tags/request) this caused ~n^2 serializations per chunk and a large number of MemoryStream allocations. Introduces AppendItemsWithinPayloadLimit, which uses a galloping search followed by a binary search to find the exact same maximum item count as the previous linear scan (the serialized length grows monotonically with the number of appended items), reducing full-request serializations per chunk from O(n) to O(log n). Behavior is unchanged: same chunk contents, same first-item-always-included semantics, same error handling on communication failures (hard abort, no partial-result handling changed). Adds a theory-based regression test asserting the batch split point matches exactly across several sizes, including power-of-two boundaries where the galloping search transitions to its binary-search phase.
…atching GetSerializedRequestLengthForBatching is called repeatedly per chunk by the galloping/binary-search batching introduced previously (potentially thousands of times for large tag counts). It previously allocated a new MemoryStream (and backing byte array) on every call just to measure the serialized request length. Reuses a [ThreadStatic] MemoryStream across calls, resetting its length instead of reallocating. Safe without locking because a single batching operation runs synchronously on one thread and this helper is never called reentrantly. Reduces GC pressure for large batched read/write operations without changing any observable behavior.
Restore the legacy driver behavior in browsePlcTagBySymbol: a relation-bearing member (struct/UDT instance, struct array, FB instance or DTL) resolves to a tag once the symbolic path is exhausted instead of returning null. The previous port restricted that branch to DTL, so GetTagBySymbolAsync threw for struct symbols and GetTagsBySymbolsAsync silently dropped them. Add the missing PlcTagStruct tag and the S7COMMP_SOFTDATATYPE_STRUCT case in TagFactory, which both had no counterpart in this repository, so the restored branch would still have produced null. Expose the PLC-declared array declaration through the new PlcTag.ArrayDimensions property (lower bound and element count per dimension, outermost first) and populate it from the low-level dimension metadata on the symbolic path and from VarInfo.ArrayDimensions on the browse path. Struct/UDT arrays report their shape this way instead of materializing one tag per element; single elements stay resolvable through their indexed symbol. Align RequiresArrayElementRelationSelector with HasRelation() so struct/UDT element addresses keep the literal .1 relation selector. Add PlcTagStructArrayTests covering the struct tag, the declared shape, the selective element expansion and the packed struct read/write behavior.
Reuse the SystemLimits negotiated during the connect handshake instead of resetting _tagsPerReadRequestMax/_tagsPerWriteRequestMax to the hard-coded default on every (re)connect via new GetCachedCommunicationResources.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Build-breaking external references, incomplete metadata handling, and unresolved OpenSSL concurrency remain.
Review effort: Balanced
Findings: 2
Open (5)
What changed in this PR
Adds strong naming while expanding tag metadata support, multidimensional arrays, batching performance, reconnect behavior, and OpenSSL synchronization.
Changes:
- Signs driver, test, and GUI assemblies.
- Adds structural, string-length, and multidimensional-array tag support.
- Optimizes request batching and preserves negotiated limits across reconnects.
| File | Description |
|---|---|
src/S7CommPlusGUIBrowser/S7CommPlusGUIBrowser.csproj |
Enables assembly signing. |
src/S7CommPlusDriver/S7CommPlusTagAccessorCatalog.cs |
Persists string-length metadata. |
src/S7CommPlusDriver/S7CommPlusProtocolSession.Session.cs |
Exposes cached resource limits. |
src/S7CommPlusDriver/S7CommPlusProtocolSession.cs |
Optimizes batching and extends array/tag resolution. |
src/S7CommPlusDriver/S7CommPlusDriver.csproj |
Adds signing and local HarpoS7 references. |
src/S7CommPlusDriver/S7CommPlusClient.cs |
Applies negotiated limits after connection. |
src/S7CommPlusDriver/Properties/AssemblyInfo.cs |
Adds strong-name friend keys. |
src/S7CommPlusDriver/OpenSSL/OpenSSLConnector.cs |
Adds queue locking. |
src/S7CommPlusDriver/IS7CommPlusSession.cs |
Adds cached-resource API. |
src/S7CommPlusDriver/Core/S7CommPlusCommunicationResourceSnapshot.cs |
Adds explicit-limit constructor. |
src/S7CommPlusDriver/Core/POffsetInfoType.cs |
Extracts declared string lengths. |
src/S7CommPlusDriver/ClientApi/PlcTags.cs |
Extends tag factory metadata handling. |
src/S7CommPlusDriver/ClientApi/PlcTag.cs |
Adds array shape and structural tags. |
src/S7CommPlusDriver/ClientApi/Browser.cs |
Publishes string metadata. |
src/S7CommPlusDriver.Tests/S7CommPlusProtocolSessionReceiveTests.cs |
Tests optimized batch boundaries. |
src/S7CommPlusDriver.Tests/S7CommPlusDriver.Tests.csproj |
Signs the test assembly. |
src/S7CommPlusDriver.Tests/S7CommPlusClientTests.cs |
Tests reconnect batching limits. |
src/S7CommPlusDriver.Tests/PlcTagStructArrayTests.cs |
Tests structural arrays. |
src/S7CommPlusDriver.Tests/PlcTagStringMaxLengthTests.cs |
Tests string-length propagation. |
src/S7CommPlusDriver.Tests/PlcTagMultiDimensionalArrayTests.cs |
Tests multidimensional values. |
src/S7CommPlusDriver.Tests/FakeS7CommPlusSession.cs |
Supports cached limits in tests. |
src/DriverTest/DriverTest.csproj |
Enables assembly signing. |
docs/batch-read-write-performance-optimization.md |
Documents batching optimization. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+36
to
+37
| private readonly object m_readListLock = new object(); | ||
| private readonly object m_writeListLock = new object(); |
Comment on lines
+48
to
+51
| <ProjectReference Include="..\..\..\HarpoS7\HarpoS7.Family0\HarpoS7.Family0.csproj" /> | ||
| <ProjectReference Include="..\..\..\HarpoS7\HarpoS7.PublicKeys\HarpoS7.PublicKeys.csproj" /> | ||
| <ProjectReference Include="..\..\..\HarpoS7\HarpoS7.Utilities\HarpoS7.Utilities.csproj" /> | ||
| <ProjectReference Include="..\..\..\HarpoS7\HarpoS7\HarpoS7.csproj" /> |
Comment on lines
+153
to
+156
| // 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); |
Comment on lines
+551
to
+552
| // For a string array, UnspecifiedOffsetinfo1 carries the maximum string length of the array elements. | ||
| public override int MaxStringLength() { return UnspecifiedOffsetinfo1; } |
Comment on lines
+368
to
+369
| tag.AggregateElements.Select(FromTag).ToArray(), | ||
| tag.GetMaxStringLength()); |
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contains the sources from branch feature/optimize-07