Feature/optimize 03 string write maxlength fix

src/S7CommPlusDriver.Tests/PlcTagStringMaxLengthTests.cs

@@ -0,0 +1,91 @@
+using S7CommPlusDriver.ClientApi;
+using Xunit;
+
+namespace S7CommPlusDriver.Tests
+{
+    /// <summary>
+    /// Verifies that <see cref="PlcTags.TagFactory"/> forwards the PLC-declared maximum string length to
+    /// <see cref="PlcTagString"/>, <see cref="PlcTagStringArray"/>, and <see cref="PlcTagWString"/>, and that the resulting
+    /// write telegram matches the declared length instead of always falling back to the built-in default of 254.
+    /// </summary>
+    /// <remarks>
+    /// Regression test for a bug where <c>TagFactory</c> never received the declared maximum string length, so every
+    /// <c>String</c>/<c>WString</c> tag was created with the built-in default of 254, regardless of its actual PLC
+    /// declaration (e.g. <c>String[10]</c>). Writing such a tag then sent a telegram of the wrong size and with the wrong
+    /// declared-length header byte, which the PLC rejected.
+    /// </remarks>
+    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<PlcTagString>(tag);
+            stringTag.Value = "0123456789"; // exactly 10 characters, must not throw
+
+            var writeValue = Assert.IsType<ValueUSIntArray>(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<PlcTagString>(tag);
+            stringTag.Value = "Hello";
+
+            var writeValue = Assert.IsType<ValueUSIntArray>(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<PlcTagStringArray>(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<PlcTagWString>(tag);
+            Assert.Equal(20, wstringTag.GetMaxStringLength());
+        }
+
+        private static PValue GetWriteValue(PlcTag tag)
+        {
+            return tag.GetWriteValue();
+        }
+    }
+}

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
         /// </summary>
         public IReadOnlyList<S7CommPlusArrayDimension> ArrayDimensions = Array.Empty<S7CommPlusArrayDimension>();
 
+        /// <summary>
+        /// Gets the maximum string length declared in the PLC for <c>String</c>/<c>WString</c> values, or zero for every
+        /// other datatype. Required to build a correctly sized write telegram; see <see cref="POffsetInfoType.MaxStringLength"/>.
+        /// </summary>
+        public int MaxStringLength;
+
         /// <summary>Retains the parsed symbol path so tag creation can reuse the exact CRC inputs.</summary>
         internal List<S7CommPlusSymbolCrc.PathSegment> SymbolCrcPath;
 

src/S7CommPlusDriver/ClientApi/PlcTag.cs

@@ -64,6 +64,33 @@ public virtual void ProcessWriteResult(UInt64 error)
 
         internal abstract PValue GetWriteValue();
 
+        /// <summary>
+        /// Gets the PLC-declared maximum string length for <see cref="PlcTagString"/>, <see cref="PlcTagWString"/>, and
+        /// <see cref="PlcTagStringArray"/> tags, or zero for every other tag type. Used to persist and recreate the exact
+        /// wire length required by <see cref="GetWriteValue"/> (see <see cref="S7CommPlusTagAccessorCatalog"/>).
+        /// </summary>
+        internal virtual int GetMaxStringLength() => 0;
+
+        /// <summary>
+        /// Gets the tag's current typed value as a boxed <see cref="object"/> via its public <c>Value</c> property.
+        /// </summary>
+        /// <returns>The boxed value, or <see langword="null"/> when the concrete tag type has no public <c>Value</c> property.</returns>
+        public virtual object GetValue()
+        {
+            var valueProperty = GetType().GetProperty("Value", BindingFlags.Public | BindingFlags.Instance);
+            return valueProperty?.GetValue(this);
+        }
+
+        /// <summary>
+        /// Sets the tag's current typed value from a boxed <see cref="object"/> via its public <c>Value</c> property.
+        /// </summary>
+        /// <param name="value">The value to assign, which must be assignable to the concrete tag's <c>Value</c> property type.</param>
+        public virtual void SetValue(object value)
+        {
+            var valueProperty = GetType().GetProperty("Value", BindingFlags.Public | BindingFlags.Instance);
+            valueProperty?.SetValue(this, value);
+        }
+
         internal void SetTraceAddressMetadata(VarInfo variable)
         {
             if (variable == null)
@@ -981,6 +1008,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 +1512,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;
@@ -2619,6 +2650,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;

src/S7CommPlusDriver/ClientApi/PlcTags.cs

@@ -78,8 +78,13 @@ public static int WriteTags(this S7CommPlusProtocolSession conn, IEnumerable<Plc
         /// Whether the address represents a complete primitive array. The legacy parameter name is retained for source compatibility,
         /// but the flag also applies to multidimensional arrays because their wire value uses the same flattened array tag type.
         /// </param>
+        /// <param name="maxStringLength">
+        /// The PLC-declared maximum length for <see cref="Softdatatype.S7COMMP_SOFTDATATYPE_STRING"/> 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.
+        /// </param>
         /// <returns>The matching concrete tag implementation, or <see langword="null"/> for an unsupported datatype.</returns>
-        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);
 
@@ -178,7 +189,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:

src/S7CommPlusDriver/Core/POffsetInfoType.cs

@@ -69,6 +69,12 @@ public enum OffsetInfoType
         public abstract bool Is1Dim();
         public abstract bool IsMDim();
 
+        /// <summary>
+        /// Gets the maximum string length for a String/WString offset info, or 0 when not applicable.
+        /// </summary>
+        /// <returns>The maximum length in characters, or 0 for non-string offset info types.</returns>
+        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)
         {

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);
+            }
         }
 
         /// <summary>