Skip to content

Commit

Permalink
Fixed the last problems with writing complex types to ADS (#1394)
Browse files Browse the repository at this point in the history
fixes: #1391

fix: Fixed some issues with using enum discriminators in the XML testsuites
fix: Fixed a problem, making the java language-templates use remote SNAPSHOT version when running tests (now they correctly use the artifacts built in the current build)
feat: The test-helper not only outputs the diff, but also the line number of the elment having the diff in the test-document.
fix: Fixed an issue in the Xml test-runner, that was limiting the size of an incoming package to 1KB by increasing this buffer to 100KB
refactor: Changed PlcSINT to serialize to int and not as byte (So it matches the way the other numerical types are serialized to XML)
fix: Fixed isssues in the ADS Driver that were causing problems when Writing Structs/UDTs
fix: Fixed how WString strings are serialized.
  • Loading branch information
chrisdutz authored Feb 10, 2024
1 parent a65b483 commit 6cc7d4b
Show file tree
Hide file tree
Showing 48 changed files with 3,113 additions and 78 deletions.
2 changes: 2 additions & 0 deletions code-generation/language-java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
<configuration>
<skipInvocation>${skip-code-generation-tests}</skipInvocation>
<debug>true</debug>
<!-- Use the "test" scope for filling the tests local repository -->
<scope>test</scope>
<streamLogsOnFailures>true</streamLogsOnFailures>
<localRepositoryPath>${project.build.directory}/local-repo</localRepositoryPath>
<projectsDirectory>src/test/resources</projectsDirectory>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,11 @@ public<#if type.isDiscriminatedParentTypeDefinition()> abstract</#if> class ${ty
<#assign typedField = field.asTypedField().orElseThrow()>
<#assign namedField = field.asNamedField().orElseThrow()>

${helper.getLanguageTypeNameForField(field)} ${namedField.name} = read${field.typeName?cap_first}Field("${namedField.name}", ${helper.getDataReaderCall(typedField.type)}${helper.getFieldOptions(typedField, parserArguments)});
<#if typedField.type.isEnumTypeReference()>
${helper.getLanguageTypeNameForField(field)} ${namedField.name} = readDiscriminatorEnumField("${namedField.name}", "${typedField.type.asEnumTypeReference().orElseThrow().typeDefinition.name}", ${helper.getDataReaderCall(typedField.type)}${helper.getFieldOptions(typedField, parserArguments)});
<#else>
${helper.getLanguageTypeNameForField(field)} ${namedField.name} = read${field.typeName?cap_first}Field("${namedField.name}", ${helper.getDataReaderCall(typedField.type)}${helper.getFieldOptions(typedField, parserArguments)});
</#if>
<#break>
<#case "enum">
<#assign enumField = field.asEnumField().orElseThrow()>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ public static EnumDiscriminatedType staticParse(ReadBuffer readBuffer) throws Pa
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

EnumType discr =
readDiscriminatorField(
readDiscriminatorEnumField(
"discr",
"EnumType",
new DataReaderEnumDefault<>(EnumType::enumForValue, readUnsignedShort(readBuffer, 8)));

// Switch Field (Depending on the discriminator values, passes the instantiation to a sub-type)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,15 @@ public static EnumDiscriminatedTypeMultiple staticParse(ReadBuffer readBuffer)
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

EnumType discr1 =
readDiscriminatorField(
readDiscriminatorEnumField(
"discr1",
"EnumType",
new DataReaderEnumDefault<>(EnumType::enumForValue, readUnsignedShort(readBuffer, 8)));

EnumTypeInt discr2 =
readDiscriminatorField(
readDiscriminatorEnumField(
"discr2",
"EnumTypeInt",
new DataReaderEnumDefault<>(EnumTypeInt::enumForValue, readSignedByte(readBuffer, 8)));

// Switch Field (Depending on the discriminator values, passes the instantiation to a sub-type)
Expand Down
1,396 changes: 1,392 additions & 4 deletions plc4go/assets/testing/protocols/ads/DriverTestsuite.xml

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@
<ResponseItem>
<code dataType="string" bitLength="16" encoding="UTF-8">OK</code>
<value>
<PlcSINT dataType="byte" bitLength="8">0xd6</PlcSINT>
<PlcSINT dataType="int" bitLength="8">-42</PlcSINT>
</value>
</ResponseItem>
</hurz>
Expand Down Expand Up @@ -957,7 +957,7 @@
</PlcTagRequest>
<values isList="true">
<hurz>
<PlcSINT dataType="byte" bitLength="8">0xd6</PlcSINT>
<PlcSINT dataType="int" bitLength="8">-42</PlcSINT>
</hurz>
</values>
</PlcWriteRequest>
Expand Down
26 changes: 21 additions & 5 deletions plc4go/protocols/knxnetip/readwrite/model/KnxManufacturer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,9 @@ public static AdsDiscoveryBlock staticParse(ReadBuffer readBuffer) throws ParseE
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

AdsDiscoveryBlockType blockType =
readDiscriminatorField(
readDiscriminatorEnumField(
"blockType",
"AdsDiscoveryBlockType",
new DataReaderEnumDefault<>(
AdsDiscoveryBlockType::enumForValue, readUnsignedInt(readBuffer, 16)));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,9 @@ public static AmsPacket staticParse(ReadBuffer readBuffer) throws ParseException
int sourceAmsPort = readSimpleField("sourceAmsPort", readUnsignedInt(readBuffer, 16));

CommandId commandId =
readDiscriminatorField(
readDiscriminatorEnumField(
"commandId",
"CommandId",
new DataReaderEnumDefault<>(CommandId::enumForValue, readUnsignedInt(readBuffer, 16)));

boolean initCommand =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -883,12 +883,15 @@ private PlcValue parsePlcValue(PlcValueType plcValueType, AdsDataTypeTableEntry
int startPos = readBuffer.getPos();
int curPos = 0;
for (AdsDataTypeTableChildEntry child : adsDataTypeTableEntry.getChildren()) {
// In some cases the starting position of the data is not where we are expecting it.
// So we need to skip some bytes.
if (child.getOffset() > curPos) {
long skipBytes = child.getOffset() - curPos;
for (long i = 0; i < skipBytes; i++) {
readBuffer.readByte();
}
}

String propertyName = child.getPropertyName();
AdsDataTypeTableEntry propertyDataTypeTableEntry = dataTypeTable.get(child.getDataTypeName());
PlcValueType propertyPlcValueType = getPlcValueTypeForAdsDataType(propertyDataTypeTableEntry);
Expand All @@ -900,6 +903,7 @@ private PlcValue parsePlcValue(PlcValueType plcValueType, AdsDataTypeTableEntry
}
PlcValue propertyValue = parsePlcValue(propertyPlcValueType, propertyDataTypeTableEntry, strLen, readBuffer);
properties.put(propertyName, propertyValue);

curPos = readBuffer.getPos() - startPos;
}
return new PlcStruct(properties);
Expand Down Expand Up @@ -1130,7 +1134,7 @@ protected void serializeInternal(PlcValue contextValue,
WriteBufferByteBased writeBuffer) throws SerializationException {

// An array type: Recursively iterate over the elements
if (arrayInfo.size() > 0) {
if (!arrayInfo.isEmpty()) {
if (!contextValue.isList()) {
throw new SerializationException("Expected a PlcList, but got a " + contextValue.getPlcValueType().name());
}
Expand All @@ -1146,18 +1150,31 @@ protected void serializeInternal(PlcValue contextValue,
}

// A complex type
else if (dataType.getChildren().size() > 0) {
else if (!dataType.getChildren().isEmpty()) {
if (!contextValue.isStruct()) {
throw new SerializationException("Expected a PlcStruct, but got a " + contextValue.getPlcValueType().name());
}
PlcStruct plcStruct = (PlcStruct) contextValue;
int startPos = writeBuffer.getPos();
int curPos = 0;
for (AdsDataTypeTableChildEntry child : dataType.getChildren()) {
AdsDataTypeTableEntry childDataType = dataTypeTable.get(child.getDataTypeName());
if (!plcStruct.hasKey(child.getPropertyName())) {
throw new SerializationException("PlcStruct is missing a child with the name " + child.getPropertyName());
}
// In some cases the starting position of the data is not where we are expecting it.
// So we need to add some fill-bytes.
if (child.getOffset() > curPos) {
long fillBytes = child.getOffset() - curPos;
for(long i = 0; i < fillBytes; i++) {
writeBuffer.writeByte("fillByte", (byte) 0x00);
}
}

PlcValue childValue = plcStruct.getValue(child.getPropertyName());
serializeInternal(childValue, childDataType, childDataType.getArrayInfo(), writeBuffer);

curPos = writeBuffer.getPos() - startPos;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.apache.plc4x.test.driver.DriverTestsuiteRunner;
import org.junit.jupiter.api.Disabled;

@Disabled("I have to port the commands for reading the symbol-table first")
public class AdsDriverIT extends DriverTestsuiteRunner {

public AdsDriverIT() {
Expand Down
2 changes: 1 addition & 1 deletion plc4j/drivers/ads/src/test/resources/logback-test.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
</encoder>
</appender>

<root level="error">
<root level="warn">
<appender-ref ref="STDOUT" />
</root>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,9 @@ public static APDU staticParse(ReadBuffer readBuffer, Integer apduLength) throws
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

ApduType apduType =
readDiscriminatorField(
readDiscriminatorEnumField(
"apduType",
"ApduType",
new DataReaderEnumDefault<>(ApduType::enumForValue, readUnsignedByte(readBuffer, 4)));

// Switch Field (Depending on the discriminator values, passes the instantiation to a sub-type)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,9 @@ public static BACnetConfirmedServiceRequest staticParse(
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

BACnetConfirmedServiceChoice serviceChoice =
readDiscriminatorField(
readDiscriminatorEnumField(
"serviceChoice",
"BACnetConfirmedServiceChoice",
new DataReaderEnumDefault<>(
BACnetConfirmedServiceChoice::enumForValue, readUnsignedShort(readBuffer, 8)));
long serviceRequestPayloadLength =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,9 @@ public static BACnetServiceAck staticParse(ReadBuffer readBuffer, Long serviceAc
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

BACnetConfirmedServiceChoice serviceChoice =
readDiscriminatorField(
readDiscriminatorEnumField(
"serviceChoice",
"BACnetConfirmedServiceChoice",
new DataReaderEnumDefault<>(
BACnetConfirmedServiceChoice::enumForValue, readUnsignedShort(readBuffer, 8)));
long serviceAckPayloadLength =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,9 @@ public static BACnetUnconfirmedServiceRequest staticParse(
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

BACnetUnconfirmedServiceChoice serviceChoice =
readDiscriminatorField(
readDiscriminatorEnumField(
"serviceChoice",
"BACnetUnconfirmedServiceChoice",
new DataReaderEnumDefault<>(
BACnetUnconfirmedServiceChoice::enumForValue, readUnsignedShort(readBuffer, 8)));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -766,9 +766,11 @@ public enum KnxManufacturer {
M_THE_AKUVOX_COMPANY((int) 664, (int) 722, (String) "The Akuvox Company"),
M_NINGBO_YINZHOU_SHENGRUIJIE_ELECTRONICS_CO__LTD_(
(int) 665, (int) 723, (String) "NingBo Yinzhou ShengRuiJie Electronics Co. Ltd."),
M_ABB___RESERVED((int) 666, (int) 43954, (String) "ABB - reserved"),
M_SHENZHEN_HAIZHICHUANG_TECHNOLOGY_CO___LTD(
(int) 666, (int) 724, (String) "Shenzhen Haizhichuang Technology Co., Ltd"),
M_ABB___RESERVED((int) 667, (int) 43954, (String) "ABB - reserved"),
M_BUSCH_JAEGER_ELEKTRO___RESERVED(
(int) 667, (int) 43959, (String) "Busch-Jaeger Elektro - reserved");
(int) 668, (int) 43959, (String) "Busch-Jaeger Elektro - reserved");
private static final Map<Integer, KnxManufacturer> map;

static {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,9 @@ public static NodeIdTypeDefinition staticParse(ReadBuffer readBuffer) throws Par
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

NodeIdType nodeType =
readDiscriminatorField(
readDiscriminatorEnumField(
"nodeType",
"NodeIdType",
new DataReaderEnumDefault<>(NodeIdType::enumForValue, readUnsignedByte(readBuffer, 6)));

// Switch Field (Depending on the discriminator values, passes the instantiation to a sub-type)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,9 @@ public static OpenProtocolMessage staticParse(ReadBuffer readBuffer, Integer rev
"length", readUnsignedLong(readBuffer, 32), WithOption.WithEncoding("ASCII"));

Mid mid =
readDiscriminatorField(
readDiscriminatorEnumField(
"mid",
"Mid",
new DataReaderEnumDefault<>(Mid::enumForValue, readUnsignedLong(readBuffer, 32)),
WithOption.WithEncoding("ASCII"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,9 @@ public static Plc4xMessage staticParse(ReadBuffer readBuffer) throws ParseExcept
WithOption.WithByteOrder(ByteOrder.BIG_ENDIAN));

Plc4xRequestType requestType =
readDiscriminatorField(
readDiscriminatorEnumField(
"requestType",
"Plc4xRequestType",
new DataReaderEnumDefault<>(
Plc4xRequestType::enumForValue, readUnsignedShort(readBuffer, 8)),
WithOption.WithByteOrder(ByteOrder.BIG_ENDIAN));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ public static LldpUnit staticParse(ReadBuffer readBuffer) throws ParseException
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

TlvType tlvId =
readDiscriminatorField(
readDiscriminatorEnumField(
"tlvId",
"TlvType",
new DataReaderEnumDefault<>(TlvType::enumForValue, readUnsignedByte(readBuffer, 7)));

short tlvIdLength = readSimpleField("tlvIdLength", readUnsignedShort(readBuffer, 9));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,9 @@ public static PnDcp_Block staticParse(ReadBuffer readBuffer) throws ParseExcepti
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

PnDcp_BlockOptions option =
readDiscriminatorField(
readDiscriminatorEnumField(
"option",
"PnDcp_BlockOptions",
new DataReaderEnumDefault<>(
PnDcp_BlockOptions::enumForValue, readUnsignedShort(readBuffer, 8)),
WithOption.WithByteOrder(ByteOrder.BIG_ENDIAN));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,9 @@ public static PnDcp_Pdu_IdentifyRes_Payload staticParse(ReadBuffer readBuffer)
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

PnDcp_ServiceId serviceId =
readDiscriminatorField(
readDiscriminatorEnumField(
"serviceId",
"PnDcp_ServiceId",
new DataReaderEnumDefault<>(
PnDcp_ServiceId::enumForValue, readUnsignedShort(readBuffer, 8)),
WithOption.WithByteOrder(ByteOrder.BIG_ENDIAN));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,9 @@ public static PnIoCm_Block staticParse(ReadBuffer readBuffer) throws ParseExcept
boolean _lastItem = ThreadLocalHelper.lastItemThreadLocal.get();

PnIoCm_BlockType blockType =
readDiscriminatorField(
readDiscriminatorEnumField(
"blockType",
"PnIoCm_BlockType",
new DataReaderEnumDefault<>(
PnIoCm_BlockType::enumForValue, readUnsignedInt(readBuffer, 16)),
WithOption.WithByteOrder(ByteOrder.BIG_ENDIAN));
Expand Down
Loading

0 comments on commit 6cc7d4b

Please sign in to comment.