chrisdutz commented on code in PR #1833:
URL: https://github.com/apache/plc4x/pull/1833#discussion_r1865863710
##########
plc4j/drivers/modbus/src/test/java/org/apache/plc4x/java/modbus/base/optimizer/LittleEndianByteSwapTest.java:
##########
@@ -248,7 +249,8 @@ public void testLittleEndianByteSwap() throws Exception {
Map<PlcReadRequest, BaseOptimizer.SubResponse<PlcReadResponse>>
readResponses = new HashMap<>();
readResponses.put(firstRequest, new BaseOptimizer.SubResponse<>(
new DefaultPlcReadResponse(firstRequest, Map.of(
- "coils0", new DefaultPlcResponseItem<>(PlcResponseCode.OK, new
PlcRawByteArray(Hex.decodeHex("3060480c00c084000000000000000000000000000000")))))));
+ //"coils0", new DefaultPlcResponseItem<>(PlcResponseCode.OK,
new
PlcRawByteArray(Hex.decodeHex("3060480c00c084000000000000000000000000000000")))))));
+ "coils0", new DefaultPlcResponseItem<>(PlcResponseCode.OK, new
PlcRawByteArray(Hex.decodeHex("10404004004004000000000000000000000000000044")))))));
Review Comment:
I assume the reason the data here changed, is because of the changed
endianess in the connection string, right? In that case would I prefer us to
have different tests for different encodings. Now it's impossible to switch
between different types of PLCs ... in order to switch back to mine (if I
updated the data in them) I would still need to change this code.
##########
plc4j/drivers/modbus/src/test/java/org/apache/plc4x/java/modbus/base/optimizer/LittleEndianByteSwapTest.java:
##########
@@ -183,9 +184,9 @@ public void testLittleEndianByteSwap() throws Exception {
input.put("variable129", new String[]{"coil:165:BOOL",
"false"});
input.put("variable130", new String[]{"coil:167:BOOL",
"false"});
input.put("variable131", new String[]{"coil:169:BOOL",
"false"});
- input.put("variable132", new String[]{"coil:171:BOOL",
"false"});
+ input.put("variable132", new String[]{"coil:171:BOOL",
"true"});
Review Comment:
Why change this? This should not matter at all when reading and just
requires me to change the program on my test-devices.
##########
plc4j/drivers/modbus/src/test/java/org/apache/plc4x/java/modbus/ManualModbusTCPDriverTest.java:
##########
@@ -69,22 +92,22 @@ public ManualModbusTCPDriverTest(String connectionString) {
}
public static void main(String[] args) throws Exception {
- ManualModbusTCPDriverTest test = new
ManualModbusTCPDriverTest("modbus-tcp://192.168.23.30");
+ ManualModbusTCPDriverTest test = new
ManualModbusTCPDriverTest("modbus-tcp://10.10.1.200:10502?default-payload-byte-order=LITTLE_ENDIAN_BYTE_SWAP");
test.addTestCase("holding-register:1:BOOL", new PlcBOOL(true)); // 0001
test.addTestCase("holding-register:2:BYTE", new PlcBYTE(42)); // 2A
//test.addTestCase("holding-register:3:WORD", new PlcWORD(42424)); //
A5B8
- test.addTestCase("holding-register:4:DWORD", new
PlcDWORD(4242442424L)); // FCDE 88B8
+ test.addTestCase("holding-register:4:DWORD", new
PlcDWORD(424244242L)); // 1949 7412
Review Comment:
Why change this to the smaller number? 4242442424 is a 32 bit unsigned
value. Adding one more digit would make it a 33 bit number ... if you needed to
go down to a 29-bit number, then I think there might be something wrong with
the decoding.
##########
plc4j/drivers/modbus/src/test/java/org/apache/plc4x/java/modbus/ManualModbusTCPDriverTest.java:
##########
@@ -69,22 +92,22 @@ public ManualModbusTCPDriverTest(String connectionString) {
}
public static void main(String[] args) throws Exception {
- ManualModbusTCPDriverTest test = new
ManualModbusTCPDriverTest("modbus-tcp://192.168.23.30");
+ ManualModbusTCPDriverTest test = new
ManualModbusTCPDriverTest("modbus-tcp://10.10.1.200:10502?default-payload-byte-order=LITTLE_ENDIAN_BYTE_SWAP");
test.addTestCase("holding-register:1:BOOL", new PlcBOOL(true)); // 0001
test.addTestCase("holding-register:2:BYTE", new PlcBYTE(42)); // 2A
//test.addTestCase("holding-register:3:WORD", new PlcWORD(42424)); //
A5B8
- test.addTestCase("holding-register:4:DWORD", new
PlcDWORD(4242442424L)); // FCDE 88B8
+ test.addTestCase("holding-register:4:DWORD", new
PlcDWORD(424244242L)); // 1949 7412
// test.addTestCase("holding-register:6:LWORD", new
PlcLWORD(4242442424242424242L)); // FCDE 88B8 FCDE 88B8
test.addTestCase("holding-register:10:SINT", new PlcSINT(-42)); // D6
test.addTestCase("holding-register:11:USINT", new PlcUSINT(42)); // 2A
test.addTestCase("holding-register:12:INT", new PlcINT(-2424)); // F688
test.addTestCase("holding-register:13:UINT", new PlcUINT(42424)); //
A5B8
- test.addTestCase("holding-register:14:DINT", new PlcDINT(-242442424));
// F18C 9F48
- test.addTestCase("holding-register:16:UDINT", new
PlcUDINT(4242442424L));// FCDE 88B8
+ test.addTestCase("holding-register:14:DINT", new PlcDINT(-242442424));
// 1949 7412
+ test.addTestCase("holding-register:16:UDINT", new
PlcUDINT(424244242L));// FCDE 88B8
test.addTestCase("holding-register:18:LINT", new
PlcLINT(-4242442424242424242L));// C51F D117 B2FB B64E
test.addTestCase("holding-register:22:ULINT", new
PlcULINT(4242442424242424242L));// 3AE0 2EE8 4D04 49B2
test.addTestCase("holding-register:26:REAL", new
PlcREAL(3.141593F));// 4049 0FDC
- test.addTestCase("holding-register:28:LREAL", new
PlcLREAL(2.71828182846D)); // 4005 BF0A 8B14 5FCF
+ test.addTestCase("holding-register:28:LREAL", new PlcLREAL(2.71)); //
4005 BF0A 8B14 5FCF
Review Comment:
Why cut off the fractional digits? if you enter the hex value here:
https://baseconvert.com/ieee-754-floating-point you get the number that I had
in there (I think this was the precision that Java has). So if your driver
returns only 2.71, I would assume that something's wrong.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
