Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix coil read problem. #1833

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Fix coil read problem. #1833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0e72fb0
Fix coil read problem.
ceos01 Oct 18, 2024
c2fdd23
Merge branch 'develop' into fix/modbus
ceos01 Oct 22, 2024
9bb6943
Add test case for coils reading.
ceos01 Oct 25, 2024
ea130df
Adds COUNTER type for read/write.
ceos01 Oct 28, 2024
ed93a1c
Merge branch 'feature/s7counter' into fix/modbus
ceos01 Oct 28, 2024
4d90f46
Merge commit '6037919a3e' into fix/modbus
ceos01 Nov 30, 2024
eb05ae9
Fix read coil and inputregisters.
ceos01 Dec 1, 2024
a670337
Fix Bacnet Id enum duplicate.
ceos01 Dec 2, 2024
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
2 changes: 1 addition & 1 deletion ...rs/bacnet/src/main/generated/org/apache/plc4x/java/bacnetip/readwrite/BACnetVendorId.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public enum BACnetVendorId {
TACAB((int) 19, (int) 19, (String) "TAC AB"),
HEWLETT_PACKARD_COMPANY((int) 20, (int) 20, (String) "Hewlett-Packard Company"),
DORSETTES_INC((int) 21, (int) 21, (String) "Dorsette’s Inc."),
SIEMENS_SCHWEIZAG_FORMERLY_CERBERUSAG(
SIEMENS_SCHWEIZA_FORMERLY_CERBERUSAG(
(int) 22, (int) 22, (String) "Siemens Schweiz AG (Formerly: Cerberus AG)"),
YORK_CONTROLS_GROUP((int) 23, (int) 23, (String) "York Controls Group"),
AUTOMATED_LOGIC_CORPORATION((int) 24, (int) 24, (String) "Automated Logic Corporation"),
Expand Down
87 changes: 58 additions & 29 deletions ...ers/modbus/src/main/java/org/apache/plc4x/java/modbus/base/optimizer/ModbusOptimizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package org.apache.plc4x.java.modbus.base.optimizer;

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import org.apache.plc4x.java.api.messages.PlcReadRequest;
import org.apache.plc4x.java.api.messages.PlcReadResponse;
import org.apache.plc4x.java.api.model.PlcTag;
Expand Down Expand Up @@ -192,33 +194,7 @@ protected PlcReadResponse processReadResponses(PlcReadRequest readRequest, Map<P
}
// Go through all responses till we find one where that contains the current tag's data.
for (Response response : responses.get(tagType)) {
if(modbusTag instanceof ModbusTagCoil) {
if(response.matchesCoil(modbusTag)) {
// If this response was invalid, return all associated addresses as equally invalid.
// TODO: Possibly it would be worth doing a single item request for each of these
// tags in order to find out which ones are actually invalid as if one item in the
// current request exceeds the address range, all items in this chunk will fail, even
// if only one element was invalid.
if(response.getResponseCode() != PlcResponseCode.OK) {
values.put(tagName, new DefaultPlcResponseItem<>(response.getResponseCode(), null));
break;
}

// Coils are read completely different from registers.
ModbusTagCoil coilTag = (ModbusTagCoil) modbusTag;

// Calculate the byte that contains the response for this Coil
byte[] responseData = response.getResponseData();
int bitPosition = coilTag.getAddress() - response.startingAddress;
int bytePosition = bitPosition / 8;
int bitPositionInByte = bitPosition % 8;
boolean isBitSet = (responseData[bytePosition] & (1 << bitPositionInByte)) != 0;
values.put(tagName, new DefaultPlcResponseItem<>(PlcResponseCode.OK, new PlcBOOL(isBitSet)));
break;
}
}
// Read a normal register.
else if (response.matchesRegister(modbusTag)) {
if (response.matchesRegister(modbusTag)) {
// If this response was invalid, return all associated addresses as equally invalid.
// TODO: Possibly it would be worth doing a single item request for each of these
// tags in order to find out which ones are actually invalid as if one item in the
Expand All @@ -230,7 +206,21 @@ else if (response.matchesRegister(modbusTag)) {
}

byte[] responseData = response.getResponseDataForTag(modbusTag);
ReadBuffer readBuffer = getReadBuffer(responseData, modbusContext.getByteOrder());
ReadBuffer readBuffer = null;
if ((modbusTag instanceof ModbusTagCoil) || (modbusTag instanceof ModbusTagDiscreteInput)) {
//1. If this condition exists it is because we have an optimized buffer.
if ((modbusTag.getNumberOfElements() == 1) & (responseData.length > 2)) {
byte[] ret = new byte[2];
ret[1] = 0x00;
ret[0] = (isSet(responseData, modbusTag.getLogicalAddress()) ?(byte) 0x01 : 0x00);
readBuffer = getReadBuffer(ret, ModbusByteOrder.BIG_ENDIAN);
} else {
readBuffer = getReadBuffer(responseData, ModbusByteOrder.BIG_ENDIAN);
}
} else {
readBuffer = getReadBuffer(responseData, modbusContext.getByteOrder());
}

try {
PlcValue plcValue = DataItem.staticParse(readBuffer, modbusTag.getDataType(),
modbusTag.getNumberOfElements(),
Expand Down Expand Up @@ -407,11 +397,50 @@ public byte[] getResponseData() {

public byte[] getResponseDataForTag(ModbusTag modbusTag) {
byte[] itemData = new byte[modbusTag.getLengthBytes()];
System.arraycopy(responseData, (modbusTag.getAddress() - startingAddress) * 2, itemData, 0, modbusTag.getLengthBytes());
int value = 0;
switch(modbusTag.getDataType()) {
case BOOL: {
itemData = new byte[responseData.length];
if ((modbusTag instanceof ModbusTagCoil) || (modbusTag instanceof ModbusTagDiscreteInput)) {
for (int i= 0; i < responseData.length; i++){
itemData[i] = byteReverse(responseData[i]);
}
} else {
for (int i= 0; i < responseData.length; i++){
itemData[i] = responseData[i];
}
}
}
break;
default:
System.arraycopy(responseData,
(modbusTag.getAddress() - startingAddress) * 2,
itemData, 0, modbusTag.getLengthBytes());
}

return itemData;
}

public static byte byteReverse(byte x) {
byte b = 0;
for (int i = 0; i < 8; ++i) {
b<<=1;
b|=( x &1);
x>>=1;
}
return b;
}

}


public boolean isSet(byte[] arr, int bit) {
int index = bit / 8;
int bitPosition = 8 - bit % 8;
return (arr[index] >> bitPosition & 1) == 1;
}


protected interface TagFactory {
PlcTag createTag(int address, int count, ModbusDataType dataType);
}
Expand Down
2 changes: 1 addition & 1 deletion plc4j/drivers/modbus/src/test/java/org/apache/plc4x/java/modbus/ManualDriverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class ManualDriverTest {
*/
public static void main(String[] args) throws Exception {
//final PlcConnection connection = new DefaultPlcDriverManager().getConnection("modbus-tcp://10.211.55.3?default-payload-byte-order=BIG_ENDIAN");
final PlcConnection connection = new DefaultPlcDriverManager().getConnection("modbus-tcp://10.211.55.3?default-payload-byte-order=LITTLE_ENDIAN");
final PlcConnection connection = new DefaultPlcDriverManager().getConnection("modbus-tcp://10.10.1.200:10502?default-payload-byte-order=LITTLE_ENDIAN");
//final PlcConnection connection = new DefaultPlcDriverManager().getConnection("modbus-tcp://10.211.55.3?default-payload-byte-order=BIG_ENDIAN_BYTE_SWAP");
//final PlcConnection connection = new DefaultPlcDriverManager().getConnection("modbus-tcp://10.211.55.3?default-payload-byte-order=LITTLE_ENDIAN_BYTE_SWAP");
final PlcReadRequest readRequest = connection.readRequestBuilder()
Expand Down
33 changes: 28 additions & 5 deletions .../drivers/modbus/src/test/java/org/apache/plc4x/java/modbus/ManualModbusTCPDriverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,29 @@ public class ManualModbusTCPDriverTest extends ManualTest {
*
* Located in "main"
*
* Reference server/client: ModbusTools
* https://github.com/serhmarch/ModbusTools/releases/tag/v0.3.8
*
* If you report any improvement points on the Modbus driver,
* please run the tests on the reference device indicated above.
* One of the virtues and weaknesses of the Modbus protocol is
* that it gives creative freedom to manufacturers for the implementation
* of Scalar types, so the indicated device is the reference.
*
* Modbus Application Protocol Specification, V1.1.b
* Section 4.2 Data Encoding
* MODBUS uses a ‘big-Endian’ representation for addresses and data items.
* This means that when a numerical quantity larger than a single byte is
* transmitted, the most significant byte is sent first. So for example
*
* Register size value
* 16 - bits 0x1234 the first byte sent is 0x12 then 0x34
*
*
* https://www.h-schmidt.net/FloatConverter/IEEE754.html
* 123456.00 -> 47 f1 20 00
* Modbus -> 20 00 47 f1


hurz_BOOL := TRUE;
hurz_BYTE := 42;
Expand Down Expand Up @@ -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
chrisdutz marked this conversation as resolved.
Show resolved Hide resolved
// 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
chrisdutz marked this conversation as resolved.
Show resolved Hide resolved
//test.addTestCase("holding-register:32:TIME", "PT1.234S"); // 04D2
//test.addTestCase("holding-register::LTIME", "PT24015H23M12.034002044S");
//test.addTestCase("holding-register::DATE", "1998-03-28");
Expand Down
9 changes: 6 additions & 3 deletions ...s/src/test/java/org/apache/plc4x/java/modbus/base/optimizer/LittleEndianByteSwapTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public void testLittleEndianByteSwap() throws Exception {
input.put("variable44", new String[]{"holding-register:177:REAL", "0.3768"});
input.put("variable45", new String[]{"holding-register:181:REAL", "0.0"});
input.put("variable46", new String[]{"holding-register:185:REAL", "-0.01143"});

input.put("variable47", new String[]{"coil:1:BOOL", "false"});
input.put("variable48", new String[]{"coil:3:BOOL", "false"});
input.put("variable49", new String[]{"coil:5:BOOL", "true"});
Expand Down Expand Up @@ -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"});
chrisdutz marked this conversation as resolved.
Show resolved Hide resolved
input.put("variable133", new String[]{"coil:173:BOOL", "false"});
input.put("variable134", new String[]{"coil:175:BOOL", "false"});
input.put("variable134", new String[]{"coil:175:BOOL", "true"});
PlcReader mockPlcReader = Mockito.mock(PlcReader.class);
PlcTagHandler modbusTagHandler = new ModbusTagHandler();
PlcReadRequest.Builder builder = new DefaultPlcReadRequest.Builder(mockPlcReader, modbusTagHandler);
Expand Down Expand Up @@ -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")))))));
chrisdutz marked this conversation as resolved.
Show resolved Hide resolved
readResponses.put(secondRequest, new BaseOptimizer.SubResponse<>(
new DefaultPlcReadResponse(secondRequest, Map.of(
"registers0", new DefaultPlcResponseItem<>(PlcResponseCode.OK, new PlcRawByteArray(Hex.decodeHex("134141b000000000f80146c3000000003852c31f000000000b7841ae00000000fc0046e000000000028fc31f00000000c50441ab00000000540046dd00000000c000c31e000000006e973f32000000009998420300000000a5e33c9b00000000ccd041fc00000000020c3f2f00000000c3f9409e0000000047ae3f39000000009fbe3f3a00000000fbe83ff900000000d70a3aa30000000033333e33000000008f5c412400000000b83143d4000000005c293f5700000000c28f3f0500000000ac093f3c00000000fbe7413a0000000000000000000000000000000000000000e0df3f3b0000000023a33f3900000000a3d74220")))))));
Expand All @@ -270,6 +272,7 @@ public void testLittleEndianByteSwap() throws Exception {
for (String name : input.keySet()) {
String[] data = input.get(name);
String readValue = readResponse.getString(name);
System.out.println(name + " : " + data[1] + " : " + readValue );
Assertions.assertEquals(data[1], readValue);
}
}
Expand Down
Loading
Loading