This is an automated email from the ASF dual-hosted git repository.
cdutz pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git
The following commit(s) were added to refs/heads/develop by this push:
new b4782c0b3a fix: [Bug]: The block optimizer used in s7-light causes
errors, if a tag references the same byte multiple times
b4782c0b3a is described below
commit b4782c0b3a76010c4316c106391478caac310acd
Author: Christofer Dutz <[email protected]>
AuthorDate: Sun Aug 10 17:11:21 2025 +0200
fix: [Bug]: The block optimizer used in s7-light causes errors, if a tag
references the same byte multiple times
closes: #2213
---
.../readwrite/optimizer/S7BlockReadOptimizer.java | 46 +++++++++++++---------
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git
a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7light/readwrite/optimizer/S7BlockReadOptimizer.java
b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7light/readwrite/optimizer/S7BlockReadOptimizer.java
index a17f8639a9..41422a2ad6 100644
---
a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7light/readwrite/optimizer/S7BlockReadOptimizer.java
+++
b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7light/readwrite/optimizer/S7BlockReadOptimizer.java
@@ -69,7 +69,7 @@ public class S7BlockReadOptimizer extends S7Optimizer {
}
// Sort the tags by area
- // (We can only read multiple tags in one byte array, if they are
located in the same area)
+ // (We can only read multiple tags in one byte array if they are
located in the same area)
Map<String, Map<PlcTag, String>> sortedTagsPerArea = new HashMap<>();
for (String tagName : readRequest.getTagNames()) {
PlcTag tag = readRequest.getTag(tagName);
@@ -86,7 +86,7 @@ public class S7BlockReadOptimizer extends S7Optimizer {
} else if(tag instanceof S7Tag) {
S7Tag s7Tag = (S7Tag) tag;
MemoryArea memoryArea = s7Tag.getMemoryArea();
- // When reading DATA_BLOCKS we need to also use the block
number.
+ // When reading DATA_BLOCKS, we need to also use the block
number.
String areaName = memoryArea.getShortName();
if(memoryArea == MemoryArea.DATA_BLOCKS) {
areaName += s7Tag.getBlockNumber();
@@ -123,7 +123,7 @@ public class S7BlockReadOptimizer extends S7Optimizer {
// TODO: Implement the size
optimizedTagMap.put(new TagNameSize(tagList.get(plcTag),
0), new DefaultPlcTagItem<>(plcTag));
}
- // Var-length strings, are a performance nightmare. Trying to
optimize reading them is probably not
+ // Var-length strings are a performance nightmare. Trying to
optimize reading them is probably not
// worth the effort. For now, we simply handle them as
un-chunked tags.
else if(plcTag instanceof S7StringVarLengthTag) {
// A var-length string tag simply reads 2 or 4 bytes.
@@ -136,8 +136,11 @@ public class S7BlockReadOptimizer extends S7Optimizer {
else if (plcTag instanceof S7Tag) {
S7Tag s7Tag = (S7Tag) plcTag;
- int curTagSize = s7Tag.getDataType().getSizeInBytes() *
s7Tag.getNumberOfElements();
- // In case of fixed length strings, a string starts with
two bytes: max length,
+ // If the dataType is BOOL, the size in bytes needs to be
calculated differently.
+ // Especially if it's more than one element.
+ int curTagSize = s7Tag.getDataType() == TransportSize.BOOL
? (s7Tag.getNumberOfElements() + 7) / 8 :
+ s7Tag.getDataType().getSizeInBytes() *
s7Tag.getNumberOfElements();
+ // In the case of fixed length strings, a string starts
with two bytes: max length,
// actual length and then the string bytes after that.
if(s7Tag instanceof S7StringFixedLengthTag) {
S7StringFixedLengthTag stringFixedLengthTag =
(S7StringFixedLengthTag) s7Tag;
@@ -145,14 +148,14 @@ public class S7BlockReadOptimizer extends S7Optimizer {
curTagSize = ((2 * bytesPerChar) +
(stringFixedLengthTag.getStringLength() * bytesPerChar)) *
s7Tag.getNumberOfElements();
}
- // If this is the first tag, use that as starting point.
+ // If this is the first tag, use that as a starting point.
if(currentMemoryArea == null) {
currentMemoryArea = s7Tag.getMemoryArea();
currentDataBlockNumber = s7Tag.getBlockNumber();
currentChunkStartByteOffset = s7Tag.getByteOffset();
currentChunkEndByteOffset = s7Tag.getByteOffset() +
curTagSize;
}
- // If the next tag would be more bytes away than a s7
address item requires, it's cheaper to
+ // If the next tag is more bytes away than a s7 address
item requires, it cost fewer resources to
// split up into multiple items.
else if(currentChunkEndByteOffset + S7_ADDRESS_ANY_SIZE <
s7Tag.getByteOffset()) {
// Save the current chunk.
@@ -170,7 +173,8 @@ public class S7BlockReadOptimizer extends S7Optimizer {
}
// Otherwise extend the array size to include this tag.
else {
- currentChunkEndByteOffset = s7Tag.getByteOffset() +
curTagSize;
+ // Check if adding this tag would increase the size of
the array.
+ currentChunkEndByteOffset =
Math.max(currentChunkEndByteOffset, s7Tag.getByteOffset() + curTagSize);
}
// Add the tag to the list of tags for the current chunk.
@@ -188,7 +192,7 @@ public class S7BlockReadOptimizer extends S7Optimizer {
}
// Go through all chunks. If there are ones larger than the max PDU
size, split them up into
- // multiple tags, that utilize the packets to the maximum.
+ // multiple tags that use the packets to the maximum.
final int maxRequestSize = ((S7DriverContext)
driverContext).getPduSize() - (EMPTY_READ_RESPONSE_SIZE + 4);
Map<TagNameSize, PlcTagItem<PlcTag>> optimizedTagMap2 = new
TreeMap<>();
for (TagNameSize tagNameSize : optimizedTagMap.keySet()) {
@@ -225,17 +229,17 @@ public class S7BlockReadOptimizer extends S7Optimizer {
}
}
- // Using the First Fit Decreasing (FFD) bin-packing algorithm try to
find the ideal
- // packing for utilizing request sizes.
+ // Using the First Fit Decreasing (FFD) bin-packing algorithm, try to
find the ideal
+ // packing for using request sizes.
// 1. Assign a size to each tag
// 2. Sort the tags by size (biggest first)
- // 3. Repeat this, till all tags are consumed
+ // 3. Repeat this till all tags are consumed
// 1. Take the first packet of the list
// 2. If the tag itself exceeds the max request size, keep on
splitting it into chunks until
- // the rest would fit into a request. Then proceed with the
rest as if it was a normal tag
+ // the rest fit into a request. Then proceed with the rest as
if it was a normal tag
// 2. Go through the existing list of requests and check if the
current tag would fit
// 1. If it fits, add it to the request
- // 2. If it doesn't fit go to the next request and check
+ // 2. If it doesn't fit, go to the next request and check
// 3. If you reach the end, and it didn't fit any of the
previous requests, add a new one
LinkedHashMap<String, PlcTagItem<PlcTag>> executableTagMap = new
LinkedHashMap<>();
for (TagNameSize tagNameSize : optimizedTagMap2.keySet()) {
@@ -266,7 +270,7 @@ public class S7BlockReadOptimizer extends S7Optimizer {
// Have the upstream optimizer handle its thing.
PlcReadResponse rawReadResponse = super.processReadResponses(new
DefaultPlcReadRequest(((DefaultPlcReadRequest) readRequest).getReader(), tags),
readResponses, driverContext);
- // Merge together split-up chunks.
+ // Merge split-up chunks.
LinkedHashMap<String, PlcTagItem<PlcTag>> mergedTagItems = new
LinkedHashMap<>();
Map<String, PlcResponseItem<PlcValue>> mergedValues = new
LinkedHashMap<>();
for (String tagName : rawReadResponse.getTagNames()) {
@@ -308,7 +312,7 @@ public class S7BlockReadOptimizer extends S7Optimizer {
mergedValues.put(tagBaseName, new
DefaultPlcResponseItem<>(PlcResponseCode.OK, new PlcRawByteArray(chunkData)));
}
}
- // All others are just un-split chunks
+ // All others are just unsplit chunks
else {
PlcResponseCode responseCode =
rawReadResponse.getResponseCode(tagName);
PlcValue plcValue = rawReadResponse.getPlcValue(tagName);
@@ -318,7 +322,7 @@ public class S7BlockReadOptimizer extends S7Optimizer {
}
PlcReadResponse mergedReadResponse = new DefaultPlcReadResponse(new
DefaultPlcReadRequest(((DefaultPlcReadRequest)rawReadResponse.getRequest()).getReader(),
mergedTagItems), mergedValues);
- // If a Tag is a normal tag, just copy it over. However, if it's a
S7TagChunk, process it.
+ // If a Tag is a normal tag, just copy it over. However, if it's an
S7TagChunk, process it.
Map<String, PlcResponseItem<PlcValue>> values = new HashMap<>();
for (String tagName : mergedReadResponse.getTagNames()) {
PlcResponseCode responseCode =
mergedReadResponse.getResponseCode(tagName);
@@ -342,7 +346,8 @@ public class S7BlockReadOptimizer extends S7Optimizer {
S7Tag s7Tag = (S7Tag) plcTag;
String curTagName = s7TagChunk.getChunkTags().get(plcTag);
int curTagStartPosition = s7Tag.getByteOffset() -
chunkByteOffset;
- int curTagDataSize = s7Tag.getDataType().getSizeInBytes() *
s7Tag.getNumberOfElements();
+ int curTagDataSize = s7Tag.getDataType() == TransportSize.BOOL
? (s7Tag.getNumberOfElements() + 7) / 8 :
+ s7Tag.getDataType().getSizeInBytes() *
s7Tag.getNumberOfElements();
if(s7Tag instanceof S7StringFixedLengthTag) {
S7StringFixedLengthTag s7StringFixedLengthTag =
(S7StringFixedLengthTag) s7Tag;
if(s7Tag.getDataType() == TransportSize.WSTRING) {
@@ -372,13 +377,16 @@ public class S7BlockReadOptimizer extends S7Optimizer {
}
return s7Tag1.getSzlId() - s7Tag2.getSzlId();
} else if (tag1 instanceof S7ClkTag) {
- // Technically CLK tags should be identical as there's
+ // Technically, CLK tags should be identical as there's
// only one address for reading the PLC clock information.
return 0;
} else if (tag1 instanceof S7Tag) {
S7Tag s7Tag1 = (S7Tag) tag1;
S7Tag s7Tag2 = (S7Tag) tag2;
if (s7Tag1.getByteOffset() == s7Tag2.getByteOffset()) {
+ if (s7Tag1.getBitOffset() == s7Tag2.getBitOffset()) {
+ return s7Tag1.getNumberOfElements() -
s7Tag2.getNumberOfElements();
+ }
return s7Tag1.getBitOffset() - s7Tag2.getBitOffset();
}
return s7Tag1.getByteOffset() - s7Tag2.getByteOffset();
