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
The following commit(s) were added to refs/heads/develop by this push:
new aa26615734 bug(plc4j/ads): Disabling symbol-table reading disables
symbolic address usage.
aa26615734 is described below
commit aa26615734ea428da513d143c68059f632ae611c
Author: Christofer Dutz <[email protected]>
AuthorDate: Fri Aug 14 16:10:05 2026 +0200
bug(plc4j/ads): Disabling symbol-table reading disables symbolic address
usage.
Fixes: #1626
---
.../apache/plc4x/java/ads/AdsTcpConnection.java | 59 +++++++++++++---------
.../java/ads/configuration/AdsConfiguration.java | 2 +-
.../plc4x/java/ads/resolution/TagResolver.java | 24 +++++++++
.../plc4x/java/ads/resolution/TagResolverTest.java | 29 +++++++++++
.../modules/users/pages/protocols/ads.adoc | 13 +++--
website/asciidoc/modules/users/partials/ads.adoc | 2 +-
6 files changed, 98 insertions(+), 31 deletions(-)
diff --git
a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/AdsTcpConnection.java
b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/AdsTcpConnection.java
index c6a6032d00..89868dd126 100644
---
a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/AdsTcpConnection.java
+++
b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/AdsTcpConnection.java
@@ -60,9 +60,7 @@ import
org.apache.plc4x.java.spi.drivers.model.DefaultArrayInfo;
import org.apache.plc4x.java.spi.drivers.tags.PlcTagHandler;
import org.apache.plc4x.java.spi.transports.api.TransportInstance;
import org.apache.plc4x.java.spi.values.DefaultPlcValueHandler;
-import org.apache.plc4x.java.spi.values.PlcList;
import org.apache.plc4x.java.spi.values.PlcSTRING;
-import org.apache.plc4x.java.spi.values.PlcStruct;
import org.apache.plc4x.java.spi.values.PlcUDINT;
import org.apache.plc4x.java.spi.values.PlcValueHandler;
import org.apache.plc4x.java.utils.auditlog.api.AuditLog;
@@ -81,7 +79,6 @@ import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Consumer;
import java.util.stream.Collectors;
-import java.util.stream.IntStream;
/**
* ADS over TCP connection.
@@ -486,7 +483,8 @@ public class AdsTcpConnection extends
ConnectionBase<AdsConfiguration> {
// through the legacy ADSIGRP_SYM_HNDBYNAME round-trip — direct
(group, offset, size)
// resolution from the locally-cached tables is enough and lets us
support the full
// path syntax (".field", "[i]", multi-dim, mixed).
- TagResolver resolver = new TagResolver(symbolTable, dataTypeTable);
+ TagResolver resolver = new TagResolver(symbolTable, dataTypeTable,
+ getConfiguration().isLoadSymbolAndDataTypeTables());
Map<String, ResolvedAdsTag> resolved = new LinkedHashMap<>();
Map<String, PlcResponseCode> initialFailures = new HashMap<>();
for (String tagName : readRequest.getTagNames()) {
@@ -508,25 +506,19 @@ public class AdsTcpConnection extends
ConnectionBase<AdsConfiguration> {
}
private ResolvedAdsTag resolveForReadOrWrite(TagResolver resolver, PlcTag
tag) {
- if (tag == null) {
- throw new PlcInvalidTagException("Tag could not be parsed");
- }
- if (tag instanceof SymbolicAdsTag s) {
- return resolver.resolve(s);
- }
- if (tag instanceof DirectAdsStringTag s) {
- return new ResolvedAdsTag(s.getIndexGroup(), s.getIndexOffset(),
+ return switch (tag) {
+ case null -> throw new PlcInvalidTagException("Tag could not be
parsed");
+ case SymbolicAdsTag s -> resolver.resolve(s);
+ case DirectAdsStringTag s -> new ResolvedAdsTag(s.getIndexGroup(),
s.getIndexOffset(),
computeDirectSize(s.getPlcDataType(), s.getStringLength(),
s.getNumberOfElements()),
s.getPlcDataType(),
TagResolver.plcValueTypeForName(s.getPlcDataType(), null),
s.getStringLength(), Collections.emptyList());
- }
- if (tag instanceof DirectAdsTag d) {
- return new ResolvedAdsTag(d.getIndexGroup(), d.getIndexOffset(),
+ case DirectAdsTag d -> new ResolvedAdsTag(d.getIndexGroup(),
d.getIndexOffset(),
computeDirectSize(d.getPlcDataType(), 0,
d.getNumberOfElements()),
d.getPlcDataType(),
TagResolver.plcValueTypeForName(d.getPlcDataType(), null),
0, Collections.emptyList());
- }
- throw new PlcInvalidTagException("Unsupported tag type: " +
tag.getClass().getName());
+ default -> throw new PlcInvalidTagException("Unsupported tag type:
" + tag.getClass().getName());
+ };
}
private long computeDirectSize(String typeName, int stringLength, int
numberOfElements) {
@@ -598,7 +590,7 @@ public class AdsTcpConnection extends
ConnectionBase<AdsConfiguration> {
Map<String,
PlcResponseCode> initialFailures) {
// Tags participating in the sum-up read, in request order.
List<String> orderedNames = readRequest.getTagNames().stream()
- .filter(resolved::containsKey).collect(Collectors.toList());
+ .filter(resolved::containsKey).toList();
long expectedDataSize = orderedNames.stream()
.mapToLong(n -> 4L + resolved.get(n).sizeInBytes()) // 4-byte
per-item return code + data
@@ -710,7 +702,8 @@ public class AdsTcpConnection extends
ConnectionBase<AdsConfiguration> {
@Override
protected CompletableFuture<PlcWriteResponse> onWrite(PlcWriteRequest
writeRequest) {
- TagResolver resolver = new TagResolver(symbolTable, dataTypeTable);
+ TagResolver resolver = new TagResolver(symbolTable, dataTypeTable,
+ getConfiguration().isLoadSymbolAndDataTypeTables());
Map<String, ResolvedAdsTag> resolved = new LinkedHashMap<>();
Map<String, PlcResponseCode> initialFailures = new HashMap<>();
for (String tagName : writeRequest.getTagNames()) {
@@ -895,8 +888,11 @@ public class AdsTcpConnection extends
ConnectionBase<AdsConfiguration> {
}
AdsDataTypeTableEntry dataType = dataTypeOpt.get();
DirectAdsTag directAdsTag =
getDirectAdsTagForSymbolicName(t.getTag());
+ if (directAdsTag == null) {
+ continue;
+ }
// TODO: We should implement multi-dimensional arrays here ...
- int numberOfElements = (t.getArrayInfo().isEmpty()) ? 1 :
t.getArrayInfo().get(0).getSize();
+ int numberOfElements = (t.getArrayInfo().isEmpty()) ? 1 :
t.getArrayInfo().getFirst().getSize();
amsTCPPackets.add(new AmsTCPPacket(new
AdsAddDeviceNotificationRequest(
getConfiguration().getTargetAmsNetId(),
getConfiguration().getTargetAmsPort(),
getConfiguration().getSourceAmsNetId(),
getConfiguration().getSourceAmsPort(), ReturnCode.OK, getInvokeId(),
@@ -1084,6 +1080,19 @@ public class AdsTcpConnection extends
ConnectionBase<AdsConfiguration> {
protected CompletableFuture<PlcBrowseResponse>
onBrowseWithInterceptor(PlcBrowseRequest browseRequest,
PlcBrowseRequestInterceptor interceptor) {
Map<String, PlcResponseCode> responseCodes = new HashMap<>();
Map<String, List<PlcBrowseItem>> values = new HashMap<>();
+ if (!getConfiguration().isLoadSymbolAndDataTypeTables()) {
+ // Browsing lists the symbol table, so without it there is nothing
to browse. Report
+ // that instead of answering with an empty result, which would
look like a PLC without
+ // any symbols - see GH-1626.
+ LOGGER.warn("Cannot browse: the symbol and data-type tables were
not loaded because"
+ + " 'load-symbol-and-data-type-tables' is disabled.");
+ for (String queryName : browseRequest.getQueryNames()) {
+ responseCodes.put(queryName, PlcResponseCode.UNSUPPORTED);
+ values.put(queryName, Collections.emptyList());
+ }
+ return CompletableFuture.completedFuture(
+ new DefaultPlcBrowseResponse(browseRequest, responseCodes,
values));
+ }
for (String queryName : browseRequest.getQueryNames()) {
PlcQuery query = browseRequest.getQuery(queryName);
List<PlcBrowseItem> resultsForQuery = new ArrayList<>();
@@ -1191,11 +1200,11 @@ public class AdsTcpConnection extends
ConnectionBase<AdsConfiguration> {
List<SymbolicAdsTag> referencedSymbolicTags = tags.stream()
.filter(SymbolicAdsTag.class::isInstance)
.map(SymbolicAdsTag.class::cast)
- .collect(Collectors.toList());
+ .toList();
List<SymbolicAdsTag> symbolicTagsNeedingResolution =
referencedSymbolicTags.stream()
.filter(t -> getDirectAdsTagForSymbolicName(t) == null)
- .collect(Collectors.toList());
+ .toList();
if (!symbolicTagsNeedingResolution.isEmpty()) {
List<SymbolicAdsTag> requiredResolutionTags =
symbolicTagsNeedingResolution.stream()
@@ -1204,7 +1213,7 @@ public class AdsTcpConnection extends
ConnectionBase<AdsConfiguration> {
if (!requiredResolutionTags.isEmpty()) {
CompletableFuture<Void> resolutionFuture;
if (requiredResolutionTags.size() == 1) {
- SymbolicAdsTag t = requiredResolutionTags.get(0);
+ SymbolicAdsTag t = requiredResolutionTags.getFirst();
resolutionFuture = resolveSingleSymbolicAddress(t);
pendingResolutionRequests.put(t, resolutionFuture);
} else {
@@ -1364,7 +1373,7 @@ public class AdsTcpConnection extends
ConnectionBase<AdsConfiguration> {
}
}
for (AdsDataTypeTableEntry child :
adsDataTypeTableEntry.getChildren()) {
- if (child.getMainName().equals(remainingAddressParts.get(0))) {
+ if (child.getMainName().equals(remainingAddressParts.getFirst())) {
Optional<AdsDataTypeTableEntry> opt =
getDataTypeTableEntry(child.getSecondaryName());
if (opt.isEmpty()) {
throw new PlcRuntimeException("Could not resolve data type
" + child.getSecondaryName());
@@ -1375,7 +1384,7 @@ public class AdsTcpConnection extends
ConnectionBase<AdsConfiguration> {
}
}
throw new PlcRuntimeException(String.format("Couldn't find child with
name '%s' for type '%s'",
- remainingAddressParts.get(0),
adsDataTypeTableEntry.getMainName()));
+ remainingAddressParts.getFirst(),
adsDataTypeTableEntry.getMainName()));
}
private PlcValueType
getPlcValueTypeForAdsDataTypeForBrowse(AdsDataTypeTableEntry
dataTypeTableEntry) {
diff --git
a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/configuration/AdsConfiguration.java
b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/configuration/AdsConfiguration.java
index b299b56ec2..b876ba18eb 100644
---
a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/configuration/AdsConfiguration.java
+++
b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/configuration/AdsConfiguration.java
@@ -66,7 +66,7 @@ public class AdsConfiguration implements Configuration {
@ConfigurationParameter("load-symbol-and-data-type-tables")
@BooleanDefaultValue(true)
- @Description("Configures, if when connecting the data-type- and
symbol-table should be read. This is an optimization that can help in cases,
where the PLC program is pretty large and downloading the full tables is
causing problems. When disabled, only direct addresses
(`{IndexGroup}/{IndexOffset}:{TYPE}`) can be used: symbolic addresses are
resolved against these tables and will fail with `Unknown symbol` if they were
not loaded. Browsing is unavailable too, as it lists the symbol table.")
+ @Description("Configures, if when connecting the data-type- and
symbol-table should be read. This is an optimization that can help in cases,
where the PLC program is pretty large and downloading the full tables is
causing problems. When disabled, reading and writing is limited to direct
addresses (`{IndexGroup}/{IndexOffset}:{TYPE}`): symbolic addresses cannot be
resolved without the tables and are rejected with a corresponding error.
Browsing is unavailable for the same reason. Subs [...]
protected boolean loadSymbolAndDataTypeTables;
public AmsNetId getTargetAmsNetId() {
diff --git
a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/TagResolver.java
b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/TagResolver.java
index eccf9dec0f..2827d0a4b6 100644
---
a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/TagResolver.java
+++
b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/TagResolver.java
@@ -39,13 +39,37 @@ public final class TagResolver {
private final Map<String, AdsSymbolTableEntry> symbolTable;
private final Map<String, AdsDataTypeTableEntry> dataTypeTable;
+ private final boolean symbolTablesLoaded;
+
public TagResolver(Map<String, AdsSymbolTableEntry> symbolTable,
Map<String, AdsDataTypeTableEntry> dataTypeTable) {
+ this(symbolTable, dataTypeTable, true);
+ }
+
+ /**
+ * @param symbolTablesLoaded whether the symbol- and data-type tables were
loaded at all. When
+ * they weren't (the connection option
+ * {@code load-symbol-and-data-type-tables} is
disabled), symbolic
+ * addresses simply cannot be resolved and
saying so is more useful
+ * than reporting every symbol as unknown - see
GH-1626.
+ */
+ public TagResolver(Map<String, AdsSymbolTableEntry> symbolTable,
+ Map<String, AdsDataTypeTableEntry> dataTypeTable,
+ boolean symbolTablesLoaded) {
this.symbolTable = symbolTable;
this.dataTypeTable = dataTypeTable;
+ this.symbolTablesLoaded = symbolTablesLoaded;
}
public ResolvedAdsTag resolve(SymbolicAdsTag tag) {
+ if (!symbolTablesLoaded) {
+ throw new PlcInvalidTagException(
+ "Cannot resolve the symbolic address '" +
tag.getSymbolicAddress()
+ + "': the symbol and data-type tables were not loaded
because the connection"
+ + " option 'load-symbol-and-data-type-tables' is disabled.
Either enable that"
+ + " option or address the value directly as"
+ + " '{IndexGroup}/{IndexOffset}:{TYPE}'.");
+ }
AddressParser.AddressPart root =
AddressParser.parse(tag.getSymbolicAddress());
AdsSymbolTableEntry symbol = symbolTable.get(root.baseSegment());
if (symbol == null) {
diff --git
a/plc4j/drivers/ads/src/test/java/org/apache/plc4x/java/ads/resolution/TagResolverTest.java
b/plc4j/drivers/ads/src/test/java/org/apache/plc4x/java/ads/resolution/TagResolverTest.java
index 68dfe0ffdb..9254e15131 100644
---
a/plc4j/drivers/ads/src/test/java/org/apache/plc4x/java/ads/resolution/TagResolverTest.java
+++
b/plc4j/drivers/ads/src/test/java/org/apache/plc4x/java/ads/resolution/TagResolverTest.java
@@ -231,6 +231,35 @@ class TagResolverTest {
assertEquals(PlcValueType.Struct,
TagResolver.plcValueTypeForName(null, null));
}
+ /**
+ * With 'load-symbol-and-data-type-tables' disabled the tables are never
fetched, so symbolic
+ * addresses cannot be resolved at all. Saying that is far more useful
than reporting every
+ * symbol as unknown, which is what a user saw before - see GH-1626.
+ */
+ @Test
+ void symbolicAddressIsRejectedWhenTablesWereNotLoaded() {
+ TagResolver withoutTables = new TagResolver(symbols, types, false);
+
+ PlcInvalidTagException exception =
assertThrows(PlcInvalidTagException.class,
+ () -> withoutTables.resolve(new SymbolicAdsTag("MAIN.g_s32", null,
List.of())));
+
+ assertTrue(exception.getMessage().contains("MAIN.g_s32"),
exception.getMessage());
+
assertTrue(exception.getMessage().contains("load-symbol-and-data-type-tables"),
+ "the message has to name the option that disabled the tables: " +
exception.getMessage());
+ }
+
+ /**
+ * A genuinely unknown symbol still reports as unknown when the tables
*are* loaded - the new
+ * message must not swallow that case.
+ */
+ @Test
+ void unknownSymbolStillReportsAsUnknownWhenTablesAreLoaded() {
+ PlcInvalidTagException exception =
assertThrows(PlcInvalidTagException.class,
+ () -> resolver.resolve(new SymbolicAdsTag("MAIN.doesNotExist",
null, List.of())));
+
+ assertTrue(exception.getMessage().contains("Unknown symbol"),
exception.getMessage());
+ }
+
@Test
void extractStringLength_parsesParens() {
assertEquals(80, TagResolver.extractStringLength("STRING(80)"));
diff --git a/website/asciidoc/modules/users/pages/protocols/ads.adoc
b/website/asciidoc/modules/users/pages/protocols/ads.adoc
index f8803f4752..ccfe415254 100644
--- a/website/asciidoc/modules/users/pages/protocols/ads.adoc
+++ b/website/asciidoc/modules/users/pages/protocols/ads.adoc
@@ -47,9 +47,13 @@ The driver accepts three kinds of addresses.
MAIN.g_plant.channels[1].setpoints[4]
----
-Array indices may appear at any position of the path, not only at the end.
Symbolic addresses are
-resolved against the symbol- and data-type-table, so they require
-`load-symbol-and-data-type-tables` to be enabled (the default).
+Array indices may appear at any position of the path, not only at the end.
+
+Symbolic addresses are resolved against the symbol- and data-type-table, so
they require
+`load-symbol-and-data-type-tables` to be enabled (the default). With that
option disabled the
+tables are never fetched and a symbolic address is rejected with an error
saying so - use a direct
+address instead. Subscriptions are the exception: they resolve symbol handles
on the device and
+work either way.
*Direct addresses* name the index group and index offset explicitly, in
decimal or hexadecimal, and
carry the data type:
@@ -76,7 +80,8 @@ carry the data type:
----
Unlike symbolic addresses, direct addresses need no symbol table and therefore
work regardless of
-the `load-symbol-and-data-type-tables` setting.
+the `load-symbol-and-data-type-tables` setting. Browsing does need it, since
it lists the symbol
+table; with the option disabled a browse request is answered with
`UNSUPPORTED`.
== Connection String Options
diff --git a/website/asciidoc/modules/users/partials/ads.adoc
b/website/asciidoc/modules/users/partials/ads.adoc
index 3f999b856f..c29b2a5752 100644
--- a/website/asciidoc/modules/users/partials/ads.adoc
+++ b/website/asciidoc/modules/users/partials/ads.adoc
@@ -41,7 +41,7 @@
|`source-ams-net-id` |STRUCT | |required |AMS-Net-Id of the source. An
AMS-Net-Id has the regular format of an IPv4 IP-Address, however with 6
segments instead of 4.
|`source-ams-port` |INT | |required |AMS port of the source.
|`timeout-request` |INT |4000| |Default timeout for all types of requests.
-|`load-symbol-and-data-type-tables` |BOOLEAN |true| |Configures, if when
connecting the data-type- and symbol-table should be read. This is an
optimization that can help in cases, where the PLC program is pretty large and
downloading the full tables is causing problems. When disabled, symbolic
addresses will manually be resolved as soon as an address is used.
+|`load-symbol-and-data-type-tables` |BOOLEAN |true| |Configures, if when
connecting the data-type- and symbol-table should be read. This is an
optimization that can help in cases, where the PLC program is pretty large and
downloading the full tables is causing problems. When disabled, reading and
writing is limited to direct addresses (`{IndexGroup}/{IndexOffset}:{TYPE}`):
symbolic addresses cannot be resolved without the tables and are rejected with
a corresponding error. Browsing is un [...]
5+|Transport config options:
5+|
+++
