fix: Fixed a small issue in serializing STRING and WSTRING values.

fixes: #2328

Author: chrisdutz

plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/protocol/S7ProtocolLogic.java

@@ -2055,6 +2055,9 @@ private PlcResponse decodeWriteResponse(S7Message responseMessage, PlcWriteReque
     private S7VarPayloadDataItem serializePlcValue(S7Tag tag, PlcValue plcValue) {
         try {
             DataTransportSize transportSize = tag.getDataType().getDataTransportSize();
+            // In the case of STRING, the default and the max stringLength are both 254. With WSTRING the default
+            // is also 254. For WSTRING, the max stringLength is 16382. Here we're only handling the default, the
+            // max is handled in the StaticHelper.
             int stringLength = (tag instanceof S7StringFixedLengthTag) ? ((S7StringFixedLengthTag) tag).getStringLength() : 254;
             ByteBuffer byteBuffer = null;
             if((tag.getDataType() == TransportSize.BYTE) && (tag.getNumberOfElements() > 1)) {

plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/utils/StaticHelper.java

@@ -2690,12 +2690,13 @@ public static String parseS7String(ReadBuffer io, int stringLength, String encod
      * the String as char arrays from your application.
      */
     public static void serializeS7String(WriteBuffer io, PlcValue value, int stringLength, String encoding) {
-        int maxStringLength = 0xFF & Math.min(stringLength, 250);
-        int actStringLength = 0xFF & value.getString().length();
-        actStringLength = Math.min(maxStringLength, actStringLength);
-
         switch (encoding) {
             case "UTF-8": {
+                // In the case of STRING, the default and the max stringLength is 254.
+                int maxStringLength = 0xFF & Math.min(stringLength, 254);
+                int actStringLength = 0xFF & value.getString().length();
+                actStringLength = Math.min(maxStringLength, actStringLength);
+
                 byte[] chars = new byte[maxStringLength];
                 byte[] actChars = value.getString().substring(0, actStringLength).getBytes(StandardCharsets.UTF_8);
                 System.arraycopy(actChars, 0, chars, 0, actChars.length);
@@ -2709,6 +2710,12 @@ public static void serializeS7String(WriteBuffer io, PlcValue value, int stringL
                 break;
             }
             case "UTF-16": {
+                // In the case of WSTRING the default is also 254. However, the max stringLength is 16382.
+                // As we've settled the default handling in S7ProtocolLogic, we'll only handle the max here.
+                int maxStringLength = 0xFFFF & Math.min(stringLength, 16382);
+                int actStringLength = 0xFFFF & value.getString().length();
+                actStringLength = Math.min(maxStringLength, actStringLength);
+
                 byte[] chars = new byte[maxStringLength * 2];
                 byte[] actChars = value.getString().substring(0, actStringLength).getBytes(StandardCharsets.UTF_16BE);
                 System.arraycopy(actChars, 0, chars, 0, actChars.length);

plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7light/readwrite/protocol/S7ProtocolLogic.java

@@ -71,9 +71,9 @@ public void setDriverContext(DriverContext driverContext) {
         super.setDriverContext(driverContext);
         this.s7DriverContext = (S7DriverContext) driverContext;
         // Initialize Transaction Manager.
-        // Until the number of concurrent requests is successfully negotiated we set it to a
-        // maximum of only one request being able to be sent at a time. During the login process
-        // No concurrent requests can be sent anyway. It will be updated when receiving the
+        // Until the number of concurrent requests is successfully negotiated, we set it to a
+        // maximum of only one request being able to be sent at a time. During the login process,
+        // no concurrent requests can be sent anyway. It will be updated when receiving the
         // S7ParameterSetupCommunication response.
         this.tm = new RequestTransactionManager(1);
     }
@@ -136,7 +136,7 @@ public void onConnect(ConversationContext<TPKTPacket> context) {
 
                         // If the controller type is explicitly set, were finished with the login
                         // process. If it's set to ANY, we have to query the serial number information
-                        // in order to detect the type of PLC.
+                        // to detect the type of PLC.
                         if (s7DriverContext.getControllerType() != ControllerType.ANY) {
                             // Send an event that connection setup is complete.
                             context.fireConnected();
@@ -225,7 +225,7 @@ public CompletableFuture<PlcWriteResponse> write(PlcWriteRequest writeRequest) {
         CompletableFuture<S7Message> responseFuture = new CompletableFuture<>();
 
         // If the list of tags contains at least one STRING/WSTRING element,
-        // we need to check the sizes of the string fields in a first request.
+        // we need to check the sizes of the string fields in the first request.
         if (request.getTagNames().stream().anyMatch(t -> request.getTag(t) instanceof S7StringVarLengthTag)) {
             responseFuture = performVarLengthStringWriteRequest((DefaultPlcWriteRequest) writeRequest);
         }
@@ -776,6 +776,9 @@ private PlcResponse decodeWriteResponse(S7Message responseMessage, PlcWriteReque
     private S7VarPayloadDataItem serializePlcValue(S7Tag tag, PlcValue plcValue) {
         try {
             DataTransportSize transportSize = tag.getDataType().getDataTransportSize();
+            // In the case of STRING, the default and the max stringLength are both 254. With WSTRING the default
+            // is also 254. For WSTRING, the max stringLength is 16382. Here we're only handling the default, the
+            // max is handled in the StaticHelper.
             int stringLength = (tag instanceof S7StringFixedLengthTag) ? ((S7StringFixedLengthTag) tag).getStringLength() : 254;
             ByteBuffer byteBuffer = null;
             if((tag.getDataType() == TransportSize.BYTE) && (tag.getNumberOfElements() > 1)) {
@@ -956,15 +959,15 @@ protected S7Address encodeS7Address(PlcTag tag) {
 
     private int getTpduId() {
         int tpduId = tpduGenerator.getAndIncrement();
-        // If we've reached the max value for a 16 bit transaction identifier, reset back to 1
+        // If we've reached the max value for a 16-bit transaction identifier, reset back to 1
         if (tpduGenerator.get() == 0xFFFF) {
             tpduGenerator.set(1);
         }
         return tpduId;
     }
 
     /**
-     * A generic purpose error handler which terminates transaction and calls back given future with error message.
+     * A generic purpose error handler which terminates a transaction and calls back the given future with an error message.
      */
     static class TransactionErrorCallback<T, E extends Throwable> implements Consumer<TimeoutException>, BiConsumer<TPKTPacket, E> {
 
@@ -1007,7 +1010,7 @@ protected CompletableFuture<Map<S7StringVarLengthTag, StringSizes>> getStringSiz
             .collect(Collectors.toList());
         List<S7VarRequestParameterItem> stringFields = new ArrayList<>(varLengthStringTags.size());
         for (S7StringVarLengthTag varLengthStringTag : varLengthStringTags) {
-            // For STRING, the header is 2 bytes (first byte contains the max length and the second the actual length)
+            // For STRING, the header is 2 bytes (The first byte contains the max length, and the second the actual length)
             if (varLengthStringTag.getDataType() == TransportSize.STRING) {
                 stringFields.add(new S7VarRequestParameterItemAddress(
                     new S7AddressAny(
@@ -1019,7 +1022,7 @@ protected CompletableFuture<Map<S7StringVarLengthTag, StringSizes>> getStringSiz
                         varLengthStringTag.getBitOffset()
                     )));
             }
-            // For WSTRING, the header is 4 bytes (first word contains the max length and the second the actual length)
+            // For WSTRING, the header is 4 bytes (The first word contains the max length, and the second the actual length)
             else if (varLengthStringTag.getDataType() == TransportSize.WSTRING) {
                 stringFields.add(new S7VarRequestParameterItemAddress(
                     new S7AddressAny(