You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by cd...@apache.org on 2026/08/14 09:30:17 UTC
(plc4x) 04/04: fix(plc4j/eip): Fixed an issue that generally prevented the reading of arrays.
This is an automated email from the ASF dual-hosted git repository.
chrisdutz pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git
commit dd9667b5031f1bb7787a7289bb159936d6f682f5
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Fri Aug 14 11:18:48 2026 +0200
fix(plc4j/eip): Fixed an issue that generally prevented the reading of arrays.
fixes: #1008
---
.../plc4x/java/eip/base/EipTcpConnection.java | 48 +++++++++--
.../plc4x/java/eip/base/EipArrayReadTest.java | 99 ++++++++++++++++++++++
.../apache/plc4x/java/eip/base/EipDockerIT.java | 38 +++++++++
3 files changed, 180 insertions(+), 5 deletions(-)
diff --git a/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/EipTcpConnection.java b/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/EipTcpConnection.java
index 94e160fe92..629f6dfda6 100644
--- a/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/EipTcpConnection.java
+++ b/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/EipTcpConnection.java
@@ -444,7 +444,7 @@ public class EipTcpConnection extends PollingSubscriptionConnectionBase<EIPConfi
EipTag eipTag = (EipTag) request.getTag(tagName);
CompletableFuture<Void> tagFuture = chain.thenComposeAsync(v -> executeThrottled(() -> {
try {
- CipReadRequest req = new CipReadRequest(toAnsi(eipTag.getTag()), 1);
+ CipReadRequest req = new CipReadRequest(toAnsi(eipTag.getTag()), elementCount(eipTag));
CipUnconnectedRequest requestItem = new CipUnconnectedRequest(
classSegment, instanceSegment, req,
(byte) getConfiguration().getBackplane(),
@@ -495,7 +495,7 @@ public class EipTcpConnection extends PollingSubscriptionConnectionBase<EIPConfi
for (String tagName : sendableTagNames(request)) {
EipTag eipTag = (EipTag) request.getTag(tagName);
try {
- requests.add(new CipReadRequest(toAnsi(eipTag.getTag()), 1));
+ requests.add(new CipReadRequest(toAnsi(eipTag.getTag()), elementCount(eipTag)));
} catch (BufferException e) {
return CompletableFuture.failedFuture(new PlcRuntimeException("Failed to read field", e));
}
@@ -538,7 +538,7 @@ public class EipTcpConnection extends PollingSubscriptionConnectionBase<EIPConfi
for (String tagName : sendableTagNames(request)) {
EipTag eipTag = (EipTag) request.getTag(tagName);
try {
- requests.add(new CipReadRequest(toAnsi(eipTag.getTag()), 1));
+ requests.add(new CipReadRequest(toAnsi(eipTag.getTag()), elementCount(eipTag)));
} catch (BufferException e) {
return CompletableFuture.failedFuture(new PlcRuntimeException("Failed to read field", e));
}
@@ -835,6 +835,9 @@ public class EipTcpConnection extends PollingSubscriptionConnectionBase<EIPConfi
PlcValue plcValue = null;
if (code == PlcResponseCode.OK) {
plcValue = parsePlcValue(tag, resp.getData().getData(), resp.getData().getDataType());
+ if (plcValue == null) {
+ code = PlcResponseCode.INTERNAL_ERROR;
+ }
}
values.put(tagName, new DefaultPlcResponseItem<>(code, plcValue));
} else if (p instanceof MultipleServiceResponse responses) {
@@ -866,6 +869,9 @@ public class EipTcpConnection extends PollingSubscriptionConnectionBase<EIPConfi
PlcValue plcValue = null;
if (code == PlcResponseCode.OK) {
plcValue = parsePlcValue(tag, rr.getData().getData(), rr.getData().getDataType());
+ if (plcValue == null) {
+ code = PlcResponseCode.INTERNAL_ERROR;
+ }
}
values.put(tagName, new DefaultPlcResponseItem<>(code, plcValue));
}
@@ -884,17 +890,49 @@ public class EipTcpConnection extends PollingSubscriptionConnectionBase<EIPConfi
PlcValue plcValue = null;
if (code == PlcResponseCode.OK) {
plcValue = parsePlcValue((EipTag) tag, resp.getData().getData(), resp.getData().getDataType());
+ if (plcValue == null) {
+ // Undecodable payload (unsupported type or a reply shorter than the requested
+ // number of elements) - don't report that as a successful read.
+ code = PlcResponseCode.INTERNAL_ERROR;
+ }
}
values.put(tagName, new DefaultPlcResponseItem<>(code, plcValue));
return values;
}
- private PlcValue parsePlcValue(EipTag tag, byte[] rawData, CIPDataTypeCode type) {
+ /**
+ * Number of elements to ask the device for. An array tag ({@code %tag[0]:DINT:8}) has to
+ * request all of its elements - see GH-1008 - otherwise the device returns a single element
+ * and the decoder below has nothing to read the remaining ones from.
+ */
+ private static int elementCount(EipTag tag) {
+ return Math.max(tag.getElementNb(), 1);
+ }
+
+ /** Whether the type is stored as a flat sequence of equally sized elements. */
+ private static boolean isFixedSize(CIPDataTypeCode type) {
+ return switch (type) {
+ case SINT, INT, DINT, LINT, REAL, LREAL, BOOL -> true;
+ default -> false;
+ };
+ }
+
+ // Package-private and static so the decoding can be tested without a connection.
+ static PlcValue parsePlcValue(EipTag tag, byte[] rawData, CIPDataTypeCode type) {
final int STRING_LEN_OFFSET = 2;
final int STRING_DATA_OFFSET = 6;
ByteBuffer data = ByteBuffer.wrap(rawData).order(ByteOrder.LITTLE_ENDIAN);
- int nb = tag.getElementNb();
+ int nb = elementCount(tag);
if (nb > 1) {
+ // Never read past what the device actually sent - a short reply must not blow up
+ // the whole response with an IndexOutOfBoundsException. Only the fixed-size types
+ // below are laid out element by element; STRING/STRUCTURED carry their own length.
+ int elementSize = isFixedSize(type) ? type.getSize() : 0;
+ if (elementSize > 0 && rawData.length < nb * elementSize) {
+ LOGGER.warn("Device returned {} bytes for tag '{}', expected {} for {} elements of {}.",
+ rawData.length, tag.getTag(), nb * elementSize, nb, type);
+ return null;
+ }
List<PlcValue> list = new ArrayList<>();
int index = 0;
for (int i = 0; i < nb; i++) {
diff --git a/plc4j/drivers/eip/src/test/java/org/apache/plc4x/java/eip/base/EipArrayReadTest.java b/plc4j/drivers/eip/src/test/java/org/apache/plc4x/java/eip/base/EipArrayReadTest.java
new file mode 100644
index 0000000000..3395a1b190
--- /dev/null
+++ b/plc4j/drivers/eip/src/test/java/org/apache/plc4x/java/eip/base/EipArrayReadTest.java
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.plc4x.java.eip.base;
+
+import org.apache.plc4x.java.api.value.PlcValue;
+import org.apache.plc4x.java.eip.base.tag.EipTag;
+import org.apache.plc4x.java.eip.readwrite.CIPDataTypeCode;
+import org.apache.plc4x.java.spi.values.PlcDINT;
+import org.apache.plc4x.java.spi.values.PlcList;
+import org.junit.jupiter.api.Test;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+/**
+ * Decoding of multi-element (array) reads - see GH-1008. The driver used to request a single
+ * element while decoding {@code elementNb} of them, which either misdecoded the value or threw
+ * an IndexOutOfBoundsException out of the response handler.
+ */
+class EipArrayReadTest {
+
+ @Test
+ void arrayOfDintsIsFullyDecoded() {
+ EipTag tag = EipTag.of("%N40[0]:DINT:8");
+ assertNotNull(tag);
+ assertEquals(8, tag.getElementNb());
+
+ byte[] raw = dints(1, 2, 3, 4, 5, 6, 7, 8);
+ PlcValue value = EipTcpConnection.parsePlcValue(tag, raw, CIPDataTypeCode.DINT);
+
+ assertInstanceOf(PlcList.class, value);
+ assertEquals(8, value.getLength());
+ for (int i = 0; i < 8; i++) {
+ assertEquals(i + 1, value.getIndex(i).getInt(), "element " + i);
+ }
+ }
+
+ @Test
+ void scalarStillDecodesToAScalar() {
+ EipTag tag = EipTag.of("%N40:DINT");
+ PlcValue value = EipTcpConnection.parsePlcValue(tag, dints(42), CIPDataTypeCode.DINT);
+
+ assertInstanceOf(PlcDINT.class, value);
+ assertEquals(42, value.getInt());
+ }
+
+ /**
+ * The reporter's crash: a reply carrying fewer elements than the tag declares must not throw.
+ */
+ @Test
+ void shortReplyIsReportedInsteadOfThrowing() {
+ EipTag tag = EipTag.of("%N40[0]:DINT:8");
+
+ // Only one element's worth of data for an 8-element tag.
+ assertNull(EipTcpConnection.parsePlcValue(tag, dints(1), CIPDataTypeCode.DINT));
+ }
+
+ @Test
+ void arrayOfIntsIsFullyDecoded() {
+ EipTag tag = EipTag.of("%N40[0]:INT:4");
+
+ ByteBuffer buffer = ByteBuffer.allocate(8).order(ByteOrder.LITTLE_ENDIAN);
+ for (short s : new short[]{10, 20, 30, 40}) {
+ buffer.putShort(s);
+ }
+ PlcValue value = EipTcpConnection.parsePlcValue(tag, buffer.array(), CIPDataTypeCode.INT);
+
+ assertNotNull(value);
+ assertEquals(4, value.getLength());
+ assertEquals(10, value.getIndex(0).getInt());
+ assertEquals(40, value.getIndex(3).getInt());
+ }
+
+ private static byte[] dints(int... values) {
+ ByteBuffer buffer = ByteBuffer.allocate(values.length * 4).order(ByteOrder.LITTLE_ENDIAN);
+ for (int value : values) {
+ buffer.putInt(value);
+ }
+ return buffer.array();
+ }
+}
diff --git a/plc4j/drivers/eip/src/test/java/org/apache/plc4x/java/eip/base/EipDockerIT.java b/plc4j/drivers/eip/src/test/java/org/apache/plc4x/java/eip/base/EipDockerIT.java
index 13e106750b..02af9f0366 100644
--- a/plc4j/drivers/eip/src/test/java/org/apache/plc4x/java/eip/base/EipDockerIT.java
+++ b/plc4j/drivers/eip/src/test/java/org/apache/plc4x/java/eip/base/EipDockerIT.java
@@ -25,6 +25,7 @@ import org.apache.plc4x.java.api.messages.PlcReadResponse;
import org.apache.plc4x.java.api.messages.PlcWriteRequest;
import org.apache.plc4x.java.api.messages.PlcWriteResponse;
import org.apache.plc4x.java.api.types.PlcResponseCode;
+import org.apache.plc4x.java.api.value.PlcValue;
import org.junit.jupiter.api.*;
import org.testcontainers.containers.GenericContainer;
import org.testcontainers.containers.wait.strategy.Wait;
@@ -230,4 +231,41 @@ public class EipDockerIT {
}
}
+ /**
+ * Reading several array elements at once - see GH-1008. The driver used to ask the device
+ * for a single element while decoding as many as the tag declared, so this returned either
+ * a wrong value or an IndexOutOfBoundsException.
+ *
+ * <p>The element count has to reach the wire in the {@code CipReadRequest}, which is what
+ * separates this from {@link #writeReadArrayElement()} - that one reads a single element
+ * and passes either way.
+ */
+ @Test
+ void readMultipleArrayElements() throws Exception {
+ try (PlcConnection connection = new DefaultPlcDriverManager().getConnection(connectionUrl)) {
+ // Seed the first four elements with recognisable values.
+ PlcWriteRequest.Builder writeBuilder = connection.writeRequestBuilder();
+ for (int i = 0; i < 4; i++) {
+ writeBuilder.addTagAddress("e" + i, "hurz_DINT_ARR[" + i + "]:DINT", 0x1000 + i);
+ }
+ PlcWriteResponse writeResponse = writeBuilder.build().execute().get();
+ for (int i = 0; i < 4; i++) {
+ assertEquals(PlcResponseCode.OK, writeResponse.getResponseCode("e" + i));
+ }
+
+ // Read all four back with a single array read.
+ PlcReadRequest readRequest = connection.readRequestBuilder()
+ .addTagAddress("arr", "hurz_DINT_ARR[0]:DINT:4")
+ .build();
+ PlcReadResponse readResponse = readRequest.execute().get();
+
+ assertEquals(PlcResponseCode.OK, readResponse.getResponseCode("arr"));
+ PlcValue value = readResponse.getPlcValue("arr");
+ assertEquals(4, value.getLength());
+ for (int i = 0; i < 4; i++) {
+ assertEquals(0x1000 + i, value.getIndex(i).getInt(), "element " + i);
+ }
+ }
+ }
+
}