This is an automated email from the ASF dual-hosted git repository.
cdutz 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 3b35391 PLC4X-208 - [S7] When trying to write to a S7 device and
writing is not explicitly enabled, the PLC will respond with an error code
3b35391 is described below
commit 3b35391a376690315b9e4ebdb152df51dd07feac
Author: Christofer Dutz <[email protected]>
AuthorDate: Wed Jul 8 22:06:05 2020 +0200
PLC4X-208 - [S7] When trying to write to a S7 device and writing is not
explicitly enabled, the PLC will respond with an error code
- Fixed some wrong constant values in the TransportSize
- Changed the way the "length" is calculated in S7VarPayloadDataItem
(Changed from 'simple' to 'implicit')
- Fixed a 2 code-generation problems in C that were displayed due to these
changes
---
RELEASE_NOTES | 5 ++++-
.../plc4x/language/c/CLanguageTemplateHelper.java | 21 ++++++++++++++++++++-
.../java/s7/readwrite/protocol/S7ProtocolLogic.java | 5 ++---
.../s7/src/test/resources/testsuite/S7DriverIT.xml | 1 -
.../resources/testsuite/S7ParserSerializerTest.xml | 8 --------
.../s7/src/main/resources/protocols/s7/s7.mspec | 18 +++++++++---------
.../server/s7/protocol/S7Step7ServerAdapter.java | 4 ++--
.../s7/includes/s7_var_payload_data_item.h | 1 -
.../generated-sources/s7/includes/transport_size.h | 4 ++--
.../s7/src/s7_var_payload_data_item.c | 9 ++++-----
10 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/RELEASE_NOTES b/RELEASE_NOTES
index abcb595..a0551b1 100644
--- a/RELEASE_NOTES
+++ b/RELEASE_NOTES
@@ -16,7 +16,10 @@ Bug Fixes
---------
PLC4X-206 When writing short values exceptions are thrown
-while preparing the write request.
+ while preparing the write request.
+PLC4X-208 [S7] When trying to write to a S7 device and writing
+ is not explicitly enabled, the PLC will respond with
+ an error code
==============================================================
Apache PLC4X 0.7.0
diff --git
a/build-utils/language-c/src/main/java/org/apache/plc4x/language/c/CLanguageTemplateHelper.java
b/build-utils/language-c/src/main/java/org/apache/plc4x/language/c/CLanguageTemplateHelper.java
index efe2f5d..9b444e2 100644
---
a/build-utils/language-c/src/main/java/org/apache/plc4x/language/c/CLanguageTemplateHelper.java
+++
b/build-utils/language-c/src/main/java/org/apache/plc4x/language/c/CLanguageTemplateHelper.java
@@ -526,7 +526,7 @@ public class CLanguageTemplateHelper extends
BaseFreemarkerLanguageTemplateHelpe
return "lastItem";
// If this literal references an Enum type, then we have to
output it differently.
} else if (getTypeDefinitions().get(variableLiteral.getName())
instanceof EnumTypeDefinition) {
- return variableLiteral.getName() + "." +
variableLiteral.getChild().getName();
+ return getCTypeName(variableLiteral.getName()) + "_" +
variableLiteral.getChild().getName();
} else {
return variableExpressionGenerator.apply(term);
}
@@ -863,9 +863,28 @@ public class CLanguageTemplateHelper extends
BaseFreemarkerLanguageTemplateHelpe
}
} else {
StringBuilder sb = new StringBuilder("_message->");
+ // If this is a property of a sub-type, add the sub-type name to
the property.
if(baseType != getThisTypeDefinition()) {
sb.append(camelCaseToSnakeCase(baseType.getName())).append("_");
}
+
+ // If this expression references enum constants we need to do
things differently
+ final Optional<TypeReference> typeReferenceForProperty =
+ getTypeReferenceForProperty(baseType, vl.getName());
+ if(typeReferenceForProperty.isPresent()) {
+ final TypeReference typeReference =
typeReferenceForProperty.get();
+ if(typeReference instanceof ComplexTypeReference) {
+ final TypeDefinition typeDefinitionForTypeReference =
+ getTypeDefinitionForTypeReference(typeReference);
+ if ((typeDefinitionForTypeReference instanceof
EnumTypeDefinition) && (vl.getChild() != null)){
+ sb.append(camelCaseToSnakeCase(vl.getName()));
+ return
getCTypeName(typeDefinitionForTypeReference.getName()) +
+ "_get_" +
camelCaseToSnakeCase(vl.getChild().getName()) +
+ "(" + sb.toString() + ")";
+ }
+ }
+ }
+ // If it wasn't an enum, treat it as a normal property.
appendVariableExpressionRest(sb, baseType, vl);
return sb.toString();
}
diff --git
a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/protocol/S7ProtocolLogic.java
b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/protocol/S7ProtocolLogic.java
index 0221603..ccefd87 100644
---
a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/protocol/S7ProtocolLogic.java
+++
b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/protocol/S7ProtocolLogic.java
@@ -431,12 +431,11 @@ public class S7ProtocolLogic extends
Plc4xProtocolBase<TPKTPacket> {
private S7VarPayloadDataItem serializePlcValue(S7Field field, PlcValue
plcValue) {
try {
- DataTransportSize transportSize =
(field.getDataType().getDataProtocolId() == 1) ?
- DataTransportSize.BIT : DataTransportSize.BYTE_WORD_DWORD;
+ DataTransportSize transportSize =
field.getDataType().getDataTransportSize();
WriteBuffer writeBuffer = DataItemIO.staticSerialize(plcValue,
field.getDataType().getDataProtocolId());
if(writeBuffer != null) {
byte[] data = writeBuffer.getData();
- return new S7VarPayloadDataItem(DataTransportErrorCode.OK,
transportSize, data.length, data);
+ return new S7VarPayloadDataItem(DataTransportErrorCode.OK,
transportSize, data);
}
} catch (ParseException e) {
logger.warn(String.format("Error serializing field item of type:
'%s'", field.getDataType().name()), e);
diff --git a/plc4j/drivers/s7/src/test/resources/testsuite/S7DriverIT.xml
b/plc4j/drivers/s7/src/test/resources/testsuite/S7DriverIT.xml
index 2802439..9facd9e 100644
--- a/plc4j/drivers/s7/src/test/resources/testsuite/S7DriverIT.xml
+++ b/plc4j/drivers/s7/src/test/resources/testsuite/S7DriverIT.xml
@@ -270,7 +270,6 @@
<items
className="org.apache.plc4x.java.s7.readwrite.S7VarPayloadDataItem">
<returnCode>OK</returnCode>
<transportSize>BIT</transportSize>
- <dataLength>1</dataLength>
<data>AQ==</data>
</items>
</items>
diff --git
a/plc4j/drivers/s7/src/test/resources/testsuite/S7ParserSerializerTest.xml
b/plc4j/drivers/s7/src/test/resources/testsuite/S7ParserSerializerTest.xml
index 4c702a9..1c3b735 100644
--- a/plc4j/drivers/s7/src/test/resources/testsuite/S7ParserSerializerTest.xml
+++ b/plc4j/drivers/s7/src/test/resources/testsuite/S7ParserSerializerTest.xml
@@ -322,25 +322,21 @@
<items
className="org.apache.plc4x.java.s7.readwrite.S7VarPayloadDataItem">
<returnCode>OK</returnCode>
<transportSize>BIT</transportSize>
- <dataLength>1</dataLength>
<data>AQ==</data>
</items>
<items
className="org.apache.plc4x.java.s7.readwrite.S7VarPayloadDataItem">
<returnCode>OK</returnCode>
<transportSize>BIT</transportSize>
- <dataLength>1</dataLength>
<data>AQ==</data>
</items>
<items
className="org.apache.plc4x.java.s7.readwrite.S7VarPayloadDataItem">
<returnCode>OK</returnCode>
<transportSize>BIT</transportSize>
- <dataLength>1</dataLength>
<data>AQ==</data>
</items>
<items
className="org.apache.plc4x.java.s7.readwrite.S7VarPayloadDataItem">
<returnCode>OK</returnCode>
<transportSize>BIT</transportSize>
- <dataLength>1</dataLength>
<data>AQ==</data>
</items>
</items>
@@ -436,25 +432,21 @@
<items
className="org.apache.plc4x.java.s7.readwrite.S7VarPayloadDataItem">
<returnCode>OK</returnCode>
<transportSize>BIT</transportSize>
- <dataLength>1</dataLength>
<data>AQ==</data>
</items>
<items
className="org.apache.plc4x.java.s7.readwrite.S7VarPayloadDataItem">
<returnCode>OK</returnCode>
<transportSize>BIT</transportSize>
- <dataLength>1</dataLength>
<data>AQ==</data>
</items>
<items
className="org.apache.plc4x.java.s7.readwrite.S7VarPayloadDataItem">
<returnCode>OK</returnCode>
<transportSize>BIT</transportSize>
- <dataLength>1</dataLength>
<data>AQ==</data>
</items>
<items
className="org.apache.plc4x.java.s7.readwrite.S7VarPayloadDataItem">
<returnCode>OK</returnCode>
<transportSize>BIT</transportSize>
- <dataLength>1</dataLength>
<data>AQ==</data>
</items>
</items>
diff --git a/protocols/s7/src/main/resources/protocols/s7/s7.mspec
b/protocols/s7/src/main/resources/protocols/s7/s7.mspec
index 5b91e79..de1d087 100644
--- a/protocols/s7/src/main/resources/protocols/s7/s7.mspec
+++ b/protocols/s7/src/main/resources/protocols/s7/s7.mspec
@@ -230,11 +230,11 @@
// This is actually not quite correct as depending pon the transportSize the
length is either defined in bits or bytes.
[type 'S7VarPayloadDataItem' [bit 'lastItem']
- [enum DataTransportErrorCode 'returnCode']
- [enum DataTransportSize 'transportSize']
- [simple uint 16 'dataLength']
- [array int 8 'data' count 'transportSize.sizeInBits ?
CEIL(dataLength / 8.0) : dataLength']
- [padding uint 8 'pad' '0x00' '!lastItem && ((COUNT(data) %
2) == 1)']
+ [enum DataTransportErrorCode 'returnCode']
+ [enum DataTransportSize 'transportSize']
+ [implicit uint 16 'dataLength' 'COUNT(data) *
((transportSize == DataTransportSize.BIT) ? 1 : (transportSize.sizeInBits ? 8 :
1))']
+ [array int 8 'data' count
'transportSize.sizeInBits ? CEIL(dataLength / 8.0) : dataLength']
+ [padding uint 8 'pad' '0x00' '!lastItem &&
((COUNT(data) % 2) == 1)']
]
[type 'S7VarPayloadStatusItem'
@@ -410,13 +410,13 @@
// Integer values
// INT and UINT moved out of order as the enum constant INT needs to be
generated before it's used in java
- ['0x05' INT ['W' , '2' , 'null'
, 'DataTransportSize.BYTE_WORD_DWORD' , '23'
, 'true' , 'true' , 'true' ,
'true' , 'true' ]]
- ['0x05' UINT ['W' , '2' ,
'TransportSize.INT' , 'DataTransportSize.BYTE_WORD_DWORD' , '24'
, 'false' , 'false' , 'true'
, 'true' , 'true' ]]
+ ['0x05' INT ['W' , '2' , 'null'
, 'DataTransportSize.INTEGER' , '23'
, 'true' , 'true' , 'true' ,
'true' , 'true' ]]
+ ['0x05' UINT ['W' , '2' ,
'TransportSize.INT' , 'DataTransportSize.INTEGER' , '24'
, 'false' , 'false' , 'true'
, 'true' , 'true' ]]
// ...
['0x02' SINT ['B' , '1' ,
'TransportSize.INT' , 'DataTransportSize.BYTE_WORD_DWORD' , '21'
, 'false' , 'false' , 'true'
, 'true' , 'true' ]]
['0x02' USINT ['B' , '1' ,
'TransportSize.INT' , 'DataTransportSize.BYTE_WORD_DWORD' , '22'
, 'false' , 'false' , 'true'
, 'true' , 'true' ]]
- ['0x07' DINT ['D' , '4' ,
'TransportSize.INT' , 'DataTransportSize.BYTE_WORD_DWORD' , '25'
, 'true' , 'true' , 'true'
, 'true' , 'true' ]]
- ['0x07' UDINT ['D' , '4' ,
'TransportSize.INT' , 'DataTransportSize.BYTE_WORD_DWORD' , '26'
, 'false' , 'false' , 'true'
, 'true' , 'true' ]]
+ ['0x07' DINT ['D' , '4' ,
'TransportSize.INT' , 'DataTransportSize.INTEGER' , '25'
, 'true' , 'true' , 'true'
, 'true' , 'true' ]]
+ ['0x07' UDINT ['D' , '4' ,
'TransportSize.INT' , 'DataTransportSize.INTEGER' , '26'
, 'false' , 'false' , 'true'
, 'true' , 'true' ]]
['0x00' LINT ['X' , '8' ,
'TransportSize.INT' , 'null' , '27'
, 'false' , 'false' , 'false'
, 'true' , 'false' ]]
['0x00' ULINT ['X' , '16' ,
'TransportSize.INT' , 'null' , '28'
, 'false' , 'false' , 'false'
, 'true' , 'false' ]]
diff --git
a/sandbox/plc-simulator/src/main/java/org/apache/plc4x/simulator/server/s7/protocol/S7Step7ServerAdapter.java
b/sandbox/plc-simulator/src/main/java/org/apache/plc4x/simulator/server/s7/protocol/S7Step7ServerAdapter.java
index 9c60514..594bbe1 100644
---
a/sandbox/plc-simulator/src/main/java/org/apache/plc4x/simulator/server/s7/protocol/S7Step7ServerAdapter.java
+++
b/sandbox/plc-simulator/src/main/java/org/apache/plc4x/simulator/server/s7/protocol/S7Step7ServerAdapter.java
@@ -265,7 +265,7 @@ public class S7Step7ServerAdapter extends
ChannelInboundHandlerAdapter {
byte[] data = new
byte[2];
data[0] = (byte)
(shortValue & 0xff);
data[1] = (byte)
((shortValue >> 8) & 0xff);
- payloadItems[i] =
new S7VarPayloadDataItem(DataTransportErrorCode.OK,
DataTransportSize.BYTE_WORD_DWORD, 8 * data.length, data);
+ payloadItems[i] =
new S7VarPayloadDataItem(DataTransportErrorCode.OK,
DataTransportSize.BYTE_WORD_DWORD, data);
break;
}
default: {
@@ -281,7 +281,7 @@ public class S7Step7ServerAdapter extends
ChannelInboundHandlerAdapter {
addressAny.getNumberOfElements() :
addressAny.getTransportSize().getSizeInBytes() * 8;
final BitSet bitSet =
toBitSet(context.getDigitalInputs(), ioNumber, numElements);
final byte[] data =
Arrays.copyOf(bitSet.toByteArray(), (numElements + 7) / 8);
- payloadItems[i] = new
S7VarPayloadDataItem(DataTransportErrorCode.OK,
DataTransportSize.BYTE_WORD_DWORD, 8 * data.length, data);
+ payloadItems[i] = new
S7VarPayloadDataItem(DataTransportErrorCode.OK,
DataTransportSize.BYTE_WORD_DWORD, data);
break;
}
}
diff --git
a/sandbox/plc4c/generated-sources/s7/includes/s7_var_payload_data_item.h
b/sandbox/plc4c/generated-sources/s7/includes/s7_var_payload_data_item.h
index be57402..3f8c5b4 100644
--- a/sandbox/plc4c/generated-sources/s7/includes/s7_var_payload_data_item.h
+++ b/sandbox/plc4c/generated-sources/s7/includes/s7_var_payload_data_item.h
@@ -32,7 +32,6 @@ struct plc4c_s7_read_write_s7_var_payload_data_item {
/* Properties */
plc4c_s7_read_write_data_transport_error_code return_code;
plc4c_s7_read_write_data_transport_size transport_size;
- uint16_t data_length;
plc4c_list* data;
};
typedef struct plc4c_s7_read_write_s7_var_payload_data_item
plc4c_s7_read_write_s7_var_payload_data_item;
diff --git a/sandbox/plc4c/generated-sources/s7/includes/transport_size.h
b/sandbox/plc4c/generated-sources/s7/includes/transport_size.h
index 141ec56..298cc07 100644
--- a/sandbox/plc4c/generated-sources/s7/includes/transport_size.h
+++ b/sandbox/plc4c/generated-sources/s7/includes/transport_size.h
@@ -364,13 +364,13 @@ plc4c_s7_read_write_data_transport_size
plc4c_s7_read_write_transport_size_get_d
return plc4c_s7_read_write_data_transport_size_BYTE_WORD_DWORD;
}
case 5: { /* '0x05' */
- return plc4c_s7_read_write_data_transport_size_BYTE_WORD_DWORD;
+ return plc4c_s7_read_write_data_transport_size_INTEGER;
}
case 6: { /* '0x06' */
return plc4c_s7_read_write_data_transport_size_BYTE_WORD_DWORD;
}
case 7: { /* '0x07' */
- return plc4c_s7_read_write_data_transport_size_BYTE_WORD_DWORD;
+ return plc4c_s7_read_write_data_transport_size_INTEGER;
}
case 8: { /* '0x08' */
return plc4c_s7_read_write_data_transport_size_BYTE_WORD_DWORD;
diff --git a/sandbox/plc4c/generated-sources/s7/src/s7_var_payload_data_item.c
b/sandbox/plc4c/generated-sources/s7/src/s7_var_payload_data_item.c
index d92d5d0..d5f6de2 100644
--- a/sandbox/plc4c/generated-sources/s7/src/s7_var_payload_data_item.c
+++ b/sandbox/plc4c/generated-sources/s7/src/s7_var_payload_data_item.c
@@ -51,13 +51,12 @@ plc4c_return_code
plc4c_s7_read_write_s7_var_payload_data_item_parse(plc4c_spi_r
}
(*_message)->transport_size = transportSize;
- // Simple Field (dataLength)
+ // Implicit Field (dataLength) (Used for parsing, but it's value is not
stored as it's implicitly given by the objects content)
uint16_t dataLength = 0;
_res = plc4c_spi_read_unsigned_short(buf, 16, (uint16_t*) &dataLength);
if(_res != OK) {
return _res;
}
- (*_message)->data_length = dataLength;
// Array field (data)
plc4c_list* data = malloc(sizeof(plc4c_list));
@@ -111,8 +110,8 @@ plc4c_return_code
plc4c_s7_read_write_s7_var_payload_data_item_serialize(plc4c_s
return _res;
}
- // Simple Field (dataLength)
- _res = plc4c_spi_write_unsigned_short(buf, 16, _message->data_length);
+ // Implicit Field (dataLength) (Used for parsing, but it's value is not
stored as it's implicitly given by the objects content)
+ _res = plc4c_spi_write_unsigned_short(buf, 16,
(plc4c_spi_evaluation_helper_count(_message->data)) *
((((((_message->transport_size) ==
(plc4c_s7_read_write_data_transport_size_BIT))) ? 1 :
(((plc4c_s7_read_write_data_transport_size_get_size_in_bits(_message->transport_size))
? 8 : 1))))));
if(_res != OK) {
return _res;
}
@@ -155,7 +154,7 @@ uint8_t
plc4c_s7_read_write_s7_var_payload_data_item_length_in_bits(plc4c_s7_rea
// Enum Field (transportSize)
lengthInBits += 8;
- // Simple field (dataLength)
+ // Implicit Field (dataLength)
lengthInBits += 16;
// Array field
