Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions RELEASE_NOTES
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,70 @@ Incompatible changes
to one rather than kept, so a tag built as ":INT:0", or through
the (tag, type) constructor which used to leave the count at
zero, now reads one element instead of none.
- All Java drivers now select array elements with one shared notation, written
before the data type: `[n]` for a single element, `[lo..hi]` for an
inclusive range, an optional `;base` for an array the PLC declares as
starting somewhere other than zero, and one bracket per dimension. The
dimensions of one array may also be written comma-separated inside a single
bracket - `[1..2,3..4]` is the same as `[1..2][3..4]` - which is the form
Allen-Bradley and others use; addresses are always rendered back in the
one-bracket-per-dimension form. See the "Addressing arrays" page.

This replaces four incompatible spellings. `[4]` meant "four elements" in
seven tag classes and "the fifth element" in two; it now means one element
everywhere, and a count is written as a range. Addresses in the old form no
longer parse, and the error names the address to write instead - so an
upgrade reports the change rather than quietly returning different data.
The affected forms, by driver:

S7 %DB42:28.0:BYTE[4] -> %DB42:28.0[0..3]:BYTE
S7 (string) %DB1:0:STRING(20)[4] -> %DB1:0[0..3]:STRING(20)
Modbus holding-register:1:INT[4] -> holding-register:1[0..3]:INT
SLMP D100:INT[4] -> D100[0..3]:INT
ADS (direct) 0x4020/0:DINT[4] -> 0x4020/0[0..3]:DINT
EtherNet/IP myArray[0]:DINT:4 -> myArray[0..3]:DINT
Profinet tag:INT[4] -> tag[0..3]:INT
Profinet-NG 1.2.INPUT.0:INT[4] -> 1.2.INPUT.0[0..3]:INT
Simulated RANDOM/foo:INT[4] -> RANDOM/foo[0..3]:INT

OPC-UA addresses are unchanged - its implementation is the one the shared
notation was extracted from - and ADS and UMAS symbolic addresses keep
their existing form while gaining ranges.
- Firmata is the one driver whose addresses change meaning silently. They
carry no data type (`3[4]`), so the brackets did not move and there is
nothing to reject: `3[4]` used to read four pins starting at pin 3 and now
reads one pin, the fifth. Rewrite these as `3[0..3]`.
- An address that selects nothing now asks for the whole value rather than a
single element. For a scalar that is unchanged; for an array it is every
element, on the drivers that can determine the extent from the device
(OPC-UA, ADS, UMAS). The others read one element as before, because their
addresses are memory offsets with no declared array at them.
- A single index and a one-element range are no longer the same thing.
`myTag[4]` selects one element and yields a scalar, while `myTag[4..4]`
yields a list of one. `PlcTag.getArrayInfo()` reports the shape of the
value received - empty for a scalar, one entry per dimension for an array -
so a consumer can tell the two apart without knowing the protocol.
- `ArrayInfo` gains `getBase()` and `isRange()`, both as default methods, so
existing implementations keep compiling. Its javadoc described `[6]` as a
six-element array, which was never what the drivers did and is not what the
notation means.
- EtherNet/IP rejects an array index above 255 while parsing the address; a
CIP MemberID carries a `uint 8`. A range may run past it, since the request
carries a start and a count, but it cannot begin there.
- ADS and UMAS verify a `;base` written in the address against the bounds the
device declares, and report a disagreement. The device is authoritative; a
base that differs means the address was written against a different layout,
which would otherwise read silently shifted data.
- ADS rejects an address that names a member of an array without saying which
element - `MAIN.g_arr.member` on an array `g_arr`. It previously resolved
against the first element and reported the result as though it were the
whole path.
- Selecting array elements over UMAS is reported as UNSUPPORTED rather than
returning the whole variable. The driver has no per-element arithmetic yet;
the address parses, and the refusal is explicit.
- Fixed `getArrayInfo()` reporting one element too many on the ADS direct and
Firmata drivers, whose inclusive bounds were built from the element count
rather than the last index.

Changed Maven Coordinates
-------------------------
Expand Down
2 changes: 1 addition & 1 deletion plc4go/assets/testing/protocols/eip/DriverTestsuite.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1103,7 +1103,7 @@
<tags isList="true">
<tag className="org.apache.plc4x.test.driver.internal.api.TestTag">
<name>hurz</name>
<address>%rate:DINT:4</address>
<address>%rate[0..3]:DINT</address>
</tag>
</tags>
</TestReadRequest>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
<tags isList="true">
<tag className="org.apache.plc4x.test.driver.internal.api.TestTag">
<name>hurz</name>
<address>holding-register:1:REAL[2]</address>
<address>holding-register:1[0..1]:REAL</address>
Comment thread
chrisdutz marked this conversation as resolved.
</tag>
</tags>
</TestReadRequest>
Expand Down Expand Up @@ -482,7 +482,7 @@
<tags isList="true">
<tag className="org.apache.plc4x.test.driver.internal.api.TestValueTag">
<name>hurz</name>
<address>holding-register:1:REAL[2]</address>
<address>holding-register:1[0..1]:REAL</address>
<value>3.1415927</value>
<value>3.1415927</value>
</tag>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@
<tags isList="true">
<tag className="org.apache.plc4x.test.driver.internal.api.TestTag">
<name>hurz</name>
<address>holding-register:1:REAL[2]</address>
<address>holding-register:1[0..1]:REAL</address>
</tag>
</tags>
</TestReadRequest>
Expand Down Expand Up @@ -452,7 +452,7 @@
<tags isList="true">
<tag className="org.apache.plc4x.test.driver.internal.api.TestValueTag">
<name>hurz</name>
<address>holding-register:1:REAL[2]</address>
<address>holding-register:1[0..1]:REAL</address>
<value>3.1415927</value>
<value>3.1415927</value>
</tag>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,45 @@ public interface ArrayInfo {
int getSize();

/**
* As in PLCs not every array starts at 0, we need to be flexible with this.
* In the default usage scenario of a simple array [6] this index will be 0 by default.
* The lower index of the selection, as it was written in the address. For a single element
* such as [6] this is 6, and {@link #getUpperBound()} is 6 as well - a bare index selects
* one element, not a range starting at zero.
* @return Returns the index of lower bound of the array.
*/
int getLowerBound();

/**
* As in PLCs not every array starts at 0, we need to be flexible with this.
* In the default usage scenario of a simple array [6] this index will be match the array size.
* The upper index of the selection, as it was written in the address. For the range [0..7]
* this is 7 and {@link #getSize()} is 8, both bounds being inclusive.
* @return Returns the index of upper bound of the array.
*/
int getUpperBound();

/**
* The array's declared lower bound, as in PLCs not every array starts at 0. An address may
* state it explicitly - [4..7;1] selects elements 4 to 7 of an array declared from 1 - so
* that the bounds above can be written the way the PLC program declares them. The offset of
* an element from the start of the array is its index minus this value.
*
* <p>Defaults to 0, which is correct for any array that does not declare otherwise.
*
* @return Returns the index the array is declared to start at.
*/
default int getBase() {
return 0;
}

/**
* Whether the address wrote this dimension as a range rather than a single index.
*
* <p>The two mean different things to a caller: a single index selects one element and yields
* a scalar, while a range yields an array - even a range spanning one element. Equal bounds
* alone cannot tell them apart, so the written form has to be remembered.
*
* @return true when the dimension was written as a range.
*/
default boolean isRange() {
return getLowerBound() != getUpperBound();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@
import org.apache.plc4x.java.ads.readwrite.AdsDataTypeTableEntry;
import org.apache.plc4x.java.ads.readwrite.AdsSymbolTableEntry;
import org.apache.plc4x.java.ads.tag.SymbolicAdsTag;
import org.apache.plc4x.java.api.model.ArrayInfo;
import org.apache.plc4x.java.spi.drivers.model.ArrayNotationParser;
import org.apache.plc4x.java.api.exceptions.PlcInvalidTagException;
import org.apache.plc4x.java.api.types.PlcValueType;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -70,7 +73,13 @@ public ResolvedAdsTag resolve(SymbolicAdsTag tag) {
+ " option or address the value directly as"
+ " '{IndexGroup}/{IndexOffset}:{TYPE}'.");
}
AddressParser.AddressPart root = AddressParser.parse(tag.getSymbolicAddress());
// The trailing selection is not part of the symbolic path; it says which elements of the
// resolved location to read. Its first index is appended to the path's own indices so the
// existing bounds checking and lower-bound arithmetic apply to it unchanged.
String path = ArrayNotationParser.addressPart(tag.getSymbolicAddress());
List<ArrayInfo> selection = tag.getSelection();
AddressParser.AddressPart root = withSelectionStart(AddressParser.parse(path), selection);

AdsSymbolTableEntry symbol = symbolTable.get(root.baseSegment());
if (symbol == null) {
throw new PlcInvalidTagException("Unknown symbol: " + root.baseSegment());
Expand All @@ -80,8 +89,80 @@ public ResolvedAdsTag resolve(SymbolicAdsTag tag) {
throw new PlcInvalidTagException(
"Unknown data type for symbol " + root.baseSegment() + ": " + symbol.getDataTypeName());
}
return resolvePart(symbol.getGroup(), symbol.getOffset(), dataType,
verifyDeclaredBase(tag, symbol, dataType);

ResolvedAdsTag resolved = resolvePart(symbol.getGroup(), symbol.getOffset(), dataType,
root.arrayIndices(), root.child());
return scaleToSelection(resolved, selection);
}

/**
* Appends the first index of each selected dimension to the deepest segment of the path, so
* that the location the read starts at is resolved by the same code that resolves an index
* written in the path itself.
*/
private static AddressParser.AddressPart withSelectionStart(AddressParser.AddressPart part,
List<ArrayInfo> selection) {
if (selection.isEmpty()) {
return part;
}
if (part.child() != null) {
return new AddressParser.AddressPart(part.baseSegment(), part.arrayIndices(),
withSelectionStart(part.child(), selection));
}
List<Integer> indices = new ArrayList<>(part.arrayIndices());
for (ArrayInfo dimension : selection) {
indices.add(dimension.getLowerBound());
}
return new AddressParser.AddressPart(part.baseSegment(), indices, null);
}

/**
* Checks a declared lower bound written in the address against the one the symbol table
* declares. The table is authoritative; a base in the address is the user's statement of
* intent, and a disagreement means the address was written against a different layout than
* the PLC has - which would otherwise read silently shifted data.
*
* <p>This is the one rule of the notation that cannot be checked while the address is
* parsed, because the symbol table is not loaded then.
*/
private void verifyDeclaredBase(SymbolicAdsTag tag, AdsSymbolTableEntry symbol,
AdsDataTypeTableEntry dataType) {
Integer declared = tag.getDeclaredBase();
if (declared == null || dataType.getArrayInfo().isEmpty()) {
return;
}
long actual = dataType.getArrayInfo().get(dataType.getArrayInfo().size() - 1).getLowerBound();
if (declared != actual) {
throw new PlcInvalidTagException(String.format(
"Address '%s' declares the array to start at %d, but %s declares it to start at %d",
tag.getSymbolicAddress(), declared, symbol.getName(), actual));
}
}

/**
* Widens a location resolved for a single element to cover the whole selection: the same
* start, as many bytes as the selection spans, decoded as a list.
*/
private static ResolvedAdsTag scaleToSelection(ResolvedAdsTag resolved, List<ArrayInfo> selection) {
int elements = 1;
for (ArrayInfo dimension : selection) {
elements *= dimension.getSize();
}
if (elements <= 1) {
return resolved;
}
// The decoder builds its lists from the ADS array-info shape, so the selection is
// restated in those terms: one dimension, starting where the user asked, as many
// elements as it spans.
List<AdsDataTypeArrayInfo> dimensions = new ArrayList<>(selection.size());
for (ArrayInfo dimension : selection) {
dimensions.add(new AdsDataTypeArrayInfo(
(long) dimension.getLowerBound(), (long) dimension.getSize()));
}
return new ResolvedAdsTag(resolved.indexGroup(), resolved.indexOffset(),
resolved.sizeInBytes() * elements, resolved.dataTypeName(), PlcValueType.List,
resolved.stringLength(), dimensions);
}

private ResolvedAdsTag resolvePart(long indexGroup, long indexOffset,
Expand All @@ -92,8 +173,22 @@ private ResolvedAdsTag resolvePart(long indexGroup, long indexOffset,
return resolveArray(indexGroup, indexOffset, dataType, arrayIndices, child);
}
if (child != null) {
if (!dataType.getArrayInfo().isEmpty()) {
// Omitting the brackets asks for the whole array, so a member access after one
// asks for that member of every element - which is not one contiguous read. The
// partially indexed form is refused in resolveArray for the same reason; this is
// the same rule where no index was given at all. Without it the address would
// silently resolve against the first element.
throw new PlcInvalidTagException(
"Field access requires an array element to be specified for "
+ dataType.getMainName() + ": an address that omits the index asks for the"
+ " whole array, and a member of every element is not a single read");
}
return resolveChild(indexGroup, indexOffset, dataType, child);
}
// remainingArrayInfo stays empty for a whole-array read: it means "dimensions still to
// be applied", and the decoder reads the full shape from the type table itself. This is
// an internal signal, not the caller-facing report - see SymbolicAdsTag#getArrayInfo.
return finalizeLeaf(indexGroup, indexOffset, dataType, Collections.emptyList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.apache.plc4x.java.ads.tag;

import org.apache.plc4x.java.spi.drivers.model.ArrayNotationParser;
import org.apache.plc4x.java.api.exceptions.PlcInvalidTagException;
import org.apache.plc4x.java.spi.buffers.api.WithOption;
import org.apache.plc4x.java.spi.buffers.api.exceptions.BufferException;
Expand All @@ -35,8 +36,8 @@ public class DirectAdsStringTag extends DirectAdsTag implements AdsStringTag {

private static final Pattern RESOURCE_STRING_ADDRESS_PATTERN = Pattern.compile("^((0[xX](?<indexGroupHex>[0-9a-fA-F]{1,8}))|(?<indexGroup>\\d{1,10}))" +
"/((0[xX](?<indexOffsetHex>[0-9a-fA-F]{1,8}))|(?<indexOffset>\\d{1,10}))" +
":(?<adsDataType>STRING|WSTRING)\\((?<stringLength>\\d{1,3})\\)" +
"(\\[(?<numberOfElements>\\d{1,10})])?");
ArrayNotationParser.ARRAY_GROUP +
":(?<adsDataType>STRING|WSTRING)\\((?<stringLength>\\d{1,3})\\)");

private final int stringLength;

Expand Down Expand Up @@ -69,9 +70,9 @@ public static DirectAdsStringTag of(String address) {
String stringLengthString = matcher.group("stringLength");
int stringLength = stringLengthString != null ? Integer.parseInt(stringLengthString) : 0;

String numberOfElementsString = matcher.group("numberOfElements");
Integer numberOfElements = numberOfElementsString != null
? parseElementCount(numberOfElementsString) : null;
int[] selection = selectionOf(matcher, address);
indexOffset += selection[0];
Integer numberOfElements = selection[1];
Comment thread
sruehl marked this conversation as resolved.
Comment thread
sruehl marked this conversation as resolved.

return new DirectAdsStringTag(indexGroup, indexOffset, adsDataTypeName, stringLength, numberOfElements);
}
Expand Down
Loading
Loading