Skip to content

Commit 793f370

Browse files
chrisdutzchrisdutz
committed
fix: Fixed an issue with the ADS driver, causing the entire request to fail if one item address was wrong.
#1776
1 parent 49fbc18 commit 793f370

File tree

9 files changed

+259
-138
lines changed

9 files changed

+259
-138
lines changed

‎plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcBrowseItem.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ public interface PlcBrowseItem {
5151
*/
5252
boolean isSubscribable();
5353

54+
/**
55+
* @return returns 'true' if we can publish this variable.
56+
*/
57+
boolean isPublishable();
58+
5459
boolean isArray();
5560

5661
/**
@@ -68,4 +73,14 @@ public interface PlcBrowseItem {
6873
*/
6974
Map<String, PlcValue> getOptions();
7075

76+
/**
77+
* Sometimes it would be beneficial for clients to have the array information resolved to
78+
* individual elements, allowing to treat array items as children. As not all drivers form addresses
79+
* the same way, this option allows the driver to override the structure.
80+
*
81+
* @return if a browse item has array information, resolve this information
82+
* translating the array elements to child elements.
83+
*/
84+
//PlcBrowseItem resolveArrayItems();
85+
7186
}

‎plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/protocol/AdsProtocolLogic.java‎

Lines changed: 173 additions & 124 deletions
Large diffs are not rendered by default.

‎plc4j/drivers/ads/src/test/java/org/apache/plc4x/protocol/ads/ManualAdsDriverTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public class ManualAdsDriverTest extends ManualTest {
7171
*/
7272

7373
public ManualAdsDriverTest(String connectionString) {
74-
super(connectionString);
74+
super(connectionString, true, true, true, true, 100);
7575
}
7676

7777
public static void main(String[] args) throws Exception {

‎plc4j/drivers/ctrlx/src/main/java/org/apache/plc4x/java/ctrlx/readwrite/connection/CtrlXConnection.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ public CompletableFuture<PlcBrowseResponse> browseWithInterceptor(PlcBrowseReque
260260
matchingQueryNames.forEach(queryName -> responseItems.get(queryName).add(
261261
new DefaultListPlcBrowseItem(
262262
new CtrlXTag(curNode, PlcValueType.BOOL, Collections.emptyList()),
263-
curNode, true, true, true,
263+
curNode, true, true, true, false,
264264
Collections.emptyMap(), Collections.emptyMap(), Collections.emptyList())));
265265
}
266266
}

‎plc4j/drivers/profinet-ng/src/main/java/org/apache/plc4x/java/profinet/protocol/ProfinetProtocolLogic.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ public CompletableFuture<PlcBrowseResponse> browse(PlcBrowseRequest browseReques
356356
items.add(new DefaultPlcBrowseItem(new ProfinetTag(
357357
slot, subslot, ProfinetTag.Direction.INPUT,
358358
i, dataTypeInformation.getPlcValueType(), dataTypeInformation.getNumElements()),
359-
name, false, true, true,
359+
name, false, true, true, false,
360360
Collections.emptyMap(), Collections.emptyMap()));
361361
}
362362
}
@@ -374,7 +374,7 @@ public CompletableFuture<PlcBrowseResponse> browse(PlcBrowseRequest browseReques
374374
items.add(new DefaultPlcBrowseItem(new ProfinetTag(
375375
slot, subslot, ProfinetTag.Direction.OUTPUT,
376376
i, dataTypeInformation.getPlcValueType(), dataTypeInformation.getNumElements()),
377-
name, false, true, true,
377+
name, false, true, true, false,
378378
Collections.emptyMap(), Collections.emptyMap()));
379379
}
380380
}

‎plc4j/drivers/profinet/src/main/java/org/apache/plc4x/java/profinet/device/ProfinetModuleImpl.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,19 +264,19 @@ public List<PlcBrowseItem> browseTags(List<PlcBrowseItem> browseItems, String ad
264264
}
265265

266266
String statusName = addressSpace + "." + this.slot + "." + block.getSubSlotNumber() + "." + virtual.getId() + ".Status";
267-
browseItems.add(new DefaultPlcBrowseItem(ProfinetTag.of(statusName + ":INT"), statusName, false, false, true, new HashMap<>(), options));
267+
browseItems.add(new DefaultPlcBrowseItem(ProfinetTag.of(statusName + ":INT"), statusName, false, false, true, false, new HashMap<>(), options));
268268
if (virtual.getIoData() != null && virtual.getIoData().getInput() != null) {
269269
for (ProfinetIoDataInput input : virtual.getIoData().getInput()) {
270270
for (ProfinetDataItem item : input.getDataItemList()) {
271271
if (item.isUseAsBits()) {
272272
for (int i = 0; i < ProfinetDataType.firstEnumForFieldConversion(item.getDataType().toUpperCase()).getDataTypeSize() * 8; i++) {
273273
String tagName = addressSpace + "." + this.slot + "." + block.getSubSlotNumber() + "." + item.getTextId() + "." + i;
274-
browseItems.add(new DefaultPlcBrowseItem(ProfinetTag.of(tagName + ":BOOL"), tagName, false, false, true, new HashMap<>(), options));
274+
browseItems.add(new DefaultPlcBrowseItem(ProfinetTag.of(tagName + ":BOOL"), tagName, false, false, true, false, new HashMap<>(), options));
275275
}
276276
} else {
277277
String tagName = addressSpace + "." + this.slot + "." + block.getSubSlotNumber() + "." + item.getTextId();
278278
String datatype = ProfinetDataType.firstEnumForFieldConversion(item.getDataType().toUpperCase()).toString();
279-
browseItems.add(new DefaultPlcBrowseItem(ProfinetTag.of(tagName + ":" + datatype), tagName, false, false, true, new HashMap<>(), options));
279+
browseItems.add(new DefaultPlcBrowseItem(ProfinetTag.of(tagName + ":" + datatype), tagName, false, false, true, false, new HashMap<>(), options));
280280
}
281281
}
282282
}
@@ -287,13 +287,13 @@ public List<PlcBrowseItem> browseTags(List<PlcBrowseItem> browseItems, String ad
287287
for (ProfinetInterfaceSubmoduleItem systemInterface : module.getSystemDefinedSubmoduleList().getInterfaceSubmodules()) {
288288
if (identNumber == systemInterface.getSubslotNumber()) {
289289
String statusName = addressSpace + "." + this.slot + "." + block.getSubSlotNumber() + "." + systemInterface.getId() + ".Status";
290-
browseItems.add(new DefaultPlcBrowseItem(ProfinetTag.of(statusName + ":INT"), statusName, false, false, true, new HashMap<>(), options));
290+
browseItems.add(new DefaultPlcBrowseItem(ProfinetTag.of(statusName + ":INT"), statusName, false, false, true, false, new HashMap<>(), options));
291291
}
292292
}
293293
for (ProfinetPortSubmoduleItem systemPort : module.getSystemDefinedSubmoduleList().getPortSubmodules()) {
294294
if (identNumber == systemPort.getSubslotNumber()) {
295295
String statusName = addressSpace + "." + this.slot + "." + block.getSubSlotNumber() + "." + systemPort.getId() + ".Status";
296-
browseItems.add(new DefaultPlcBrowseItem(ProfinetTag.of(statusName + ":INT"), statusName, false, false, true, new HashMap<>(), options));
296+
browseItems.add(new DefaultPlcBrowseItem(ProfinetTag.of(statusName + ":INT"), statusName, false, false, true, false, new HashMap<>(), options));
297297
}
298298
}
299299
}

‎plc4j/spi/src/main/java/org/apache/plc4x/java/spi/messages/DefaultListPlcBrowseItem.java‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.apache.plc4x.java.spi.generation.WriteBuffer;
2828

2929
import java.nio.charset.StandardCharsets;
30+
import java.util.ArrayList;
3031
import java.util.List;
3132
import java.util.Map;
3233

@@ -39,10 +40,11 @@ public DefaultListPlcBrowseItem(PlcTag tag,
3940
boolean readable,
4041
boolean writable,
4142
boolean subscribable,
43+
boolean publishable,
4244
Map<String, PlcBrowseItem> children,
4345
Map<String, PlcValue> options,
4446
List<PlcBrowseItemArrayInfo> arrayInformation) {
45-
super(tag, name, readable, writable, subscribable, children, options);
47+
super(tag, name, readable, writable, subscribable, publishable, children, options);
4648
this.arrayInformation = arrayInformation;
4749
}
4850

@@ -56,6 +58,26 @@ public List<PlcBrowseItemArrayInfo> getArrayInformation() {
5658
return arrayInformation;
5759
}
5860

61+
/*@Override
62+
public PlcBrowseItem resolveArrayItems() {
63+
if((arrayInformation == null) || (arrayInformation.isEmpty())) {
64+
return super.resolveArrayItems();
65+
}
66+
return resolveInternal(getTag() ,arrayInformation);
67+
}
68+
69+
protected PlcBrowseItem resolveInternal(PlcTag tag, List<PlcBrowseItemArrayInfo> arrayInformation) {
70+
if(arrayInformation.isEmpty()) {
71+
72+
}
73+
for (PlcBrowseItemArrayInfo arrayInfo : arrayInformation) {
74+
for(long i = arrayInfo.getLowerBound(); i <= arrayInfo.getUpperBound(); i++) {
75+
76+
}
77+
}
78+
return this;
79+
}*/
80+
5981
@Override
6082
public void serialize(WriteBuffer writeBuffer) throws SerializationException {
6183
writeBuffer.pushContext(getClass().getSimpleName());

‎plc4j/spi/src/main/java/org/apache/plc4x/java/spi/messages/DefaultPlcBrowseItem.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public class DefaultPlcBrowseItem implements PlcBrowseItem, Serializable {
4141
private final boolean readable;
4242
private final boolean writable;
4343
private final boolean subscribable;
44+
private final boolean publishable;
4445

4546
private final Map<String, PlcBrowseItem> children;
4647

@@ -51,17 +52,30 @@ public DefaultPlcBrowseItem(PlcTag tag,
5152
boolean readable,
5253
boolean writable,
5354
boolean subscribable,
55+
boolean publishable,
5456
Map<String, PlcBrowseItem> children,
5557
Map<String, PlcValue> options) {
5658
this.tag = tag;
5759
this.name = name;
5860
this.readable = readable;
5961
this.writable = writable;
6062
this.subscribable = subscribable;
63+
this.publishable = publishable;
6164
this.children = children;
6265
this.options = options;
6366
}
6467

68+
protected DefaultPlcBrowseItem(PlcBrowseItem original, Map<String, PlcBrowseItem> children) {
69+
this.tag = original.getTag();
70+
this.name = original.getName();
71+
this.readable = original.isReadable();
72+
this.writable = original.isWritable();
73+
this.subscribable = original.isSubscribable();
74+
this.publishable = original.isPublishable();
75+
this.options = original.getOptions();
76+
this.children = children;
77+
}
78+
6579
@Override
6680
public PlcTag getTag() {
6781
return tag;
@@ -72,23 +86,32 @@ public String getName() {
7286
return name;
7387
}
7488

89+
@Override
7590
public boolean isReadable() {
7691
return readable;
7792
}
7893

94+
@Override
7995
public boolean isWritable() {
8096
return writable;
8197
}
8298

99+
@Override
83100
public boolean isSubscribable() {
84101
return subscribable;
85102
}
86103

104+
@Override
105+
public boolean isPublishable() {
106+
return publishable;
107+
}
108+
87109
@Override
88110
public boolean isArray() {
89111
return false;
90112
}
91113

114+
@Override
92115
public List<PlcBrowseItemArrayInfo> getArrayInformation() {
93116
return Collections.emptyList();
94117
}
@@ -103,6 +126,15 @@ public Map<String, PlcValue> getOptions() {
103126
return options;
104127
}
105128

129+
/**
130+
* For simple non-array elements we usually don't have to do anything here
131+
* @return a one-element list the unchanged item
132+
*/
133+
/*@Override
134+
public PlcBrowseItem resolveArrayItems() {
135+
return this;
136+
}*/
137+
106138
@Override
107139
public void serialize(WriteBuffer writeBuffer) throws SerializationException {
108140
writeBuffer.pushContext(getClass().getSimpleName());

‎plc4j/utils/test-utils/src/main/java/org/apache/plc4x/test/manual/ManualTest.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,19 @@ public abstract class ManualTest {
4242
private final boolean testWrite;
4343
private final boolean enableSingleIteTests;
4444
private final List<TestCase> testCases;
45-
45+
private final boolean shuffleMultiItemRequests;
4646
private final int numRandomMultiItemRequests;
4747

4848
public ManualTest(String connectionString) {
49-
this(connectionString, true, true, true, 100);
49+
this(connectionString, true, true, true, true, 100);
5050
}
5151

52-
public ManualTest(String connectionString, boolean testRead, boolean testWrite, boolean enableSingleIteTests, int numRandomMultiItemRequests) {
52+
public ManualTest(String connectionString, boolean testRead, boolean testWrite, boolean enableSingleIteTests, boolean shuffleMultiItemRequests, int numRandomMultiItemRequests) {
5353
this.connectionString = connectionString;
5454
this.testRead = testRead;
5555
this.testWrite = testWrite;
5656
this.enableSingleIteTests = enableSingleIteTests;
57+
this.shuffleMultiItemRequests = shuffleMultiItemRequests;
5758
this.numRandomMultiItemRequests = numRandomMultiItemRequests;
5859
testCases = new ArrayList<>();
5960
}
@@ -152,7 +153,9 @@ public void run() throws Exception {
152153
for (int i = 0; i < numRandomMultiItemRequests; i++) {
153154
System.out.println(" - run number " + i + " of " + numRandomMultiItemRequests);
154155
final List<TestCase> shuffledTestcases = new ArrayList<>(testCases);
155-
Collections.shuffle(shuffledTestcases);
156+
if(shuffleMultiItemRequests) {
157+
Collections.shuffle(shuffledTestcases);
158+
}
156159

157160
StringBuilder sb = new StringBuilder();
158161
for (TestCase testCase : shuffledTestcases) {

0 commit comments

Comments
 (0)