Skip to content

Commit

Permalink
ModbusBridge: Fix ClassCastException to Register[] (#2320)
Browse files Browse the repository at this point in the history 
This fixes the issue reported in https://community.openems.io/t/modbus-tcp-gerat-mit-eclipse-debuggen/1839/2

* Add JUnit test that shows previous implementation error
* Parse FC4 response to Register[]

Tested on private setup
  • Loading branch information
@sfeilmeier
sfeilmeier authored Aug 23, 2023
1 parent b8e5747 commit 4c8f3a6
Show file tree
Hide file tree
Showing 10 changed files with 149 additions and 33 deletions.
16 changes: 9 additions & 7 deletions .../task/AbstractReadInputRegistersTask.java → ...s/api/task/AbstractReadRegistersTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,33 @@

import com.ghgande.j2mod.modbus.msg.ModbusRequest;
import com.ghgande.j2mod.modbus.msg.ModbusResponse;
import com.ghgande.j2mod.modbus.procimg.InputRegister;
import com.ghgande.j2mod.modbus.procimg.Register;

import io.openems.common.exceptions.OpenemsException;
import io.openems.edge.bridge.modbus.api.element.ModbusElement;
import io.openems.edge.bridge.modbus.api.element.ModbusRegisterElement;
import io.openems.edge.common.taskmanager.Priority;

@SuppressWarnings("rawtypes")
public abstract class AbstractReadInputRegistersTask<REQUEST extends ModbusRequest, RESPONSE extends ModbusResponse>
extends AbstractReadTask<REQUEST, RESPONSE, ModbusRegisterElement, InputRegister> {
public abstract class AbstractReadRegistersTask<//
REQUEST extends ModbusRequest, //
RESPONSE extends ModbusResponse> //
extends AbstractReadTask<REQUEST, RESPONSE, ModbusRegisterElement, Register> {

public AbstractReadInputRegistersTask(String name, Class<RESPONSE> responseClazz, int startAddress,
Priority priority, ModbusElement... elements) {
public AbstractReadRegistersTask(String name, Class<RESPONSE> responseClazz, int startAddress, Priority priority,
ModbusElement... elements) {
super(name, responseClazz, ModbusRegisterElement.class, startAddress, priority, elements);
}

@SuppressWarnings("unchecked")
@Override
protected void handleResponse(ModbusRegisterElement element, int position, InputRegister[] response)
protected final void handleResponse(ModbusRegisterElement element, int position, Register[] response)
throws OpenemsException {
element.setInputValue(Arrays.copyOfRange(response, position, position + element.length));
}

@Override
protected int calculateNextPosition(ModbusElement modbusElement, int position) {
protected final int calculateNextPosition(ModbusElement modbusElement, int position) {
return position + modbusElement.length;
}
}
6 changes: 3 additions & 3 deletions ...s.edge.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/FC3ReadRegistersTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import com.ghgande.j2mod.modbus.msg.ReadMultipleRegistersRequest;
import com.ghgande.j2mod.modbus.msg.ReadMultipleRegistersResponse;
import com.ghgande.j2mod.modbus.procimg.InputRegister;
import com.ghgande.j2mod.modbus.procimg.Register;

import io.openems.common.exceptions.OpenemsException;
import io.openems.edge.bridge.modbus.api.ModbusUtils;
Expand All @@ -14,7 +14,7 @@
* (http://www.simplymodbus.ca/FC03.htm).
*/
public class FC3ReadRegistersTask
extends AbstractReadInputRegistersTask<ReadMultipleRegistersRequest, ReadMultipleRegistersResponse> {
extends AbstractReadRegistersTask<ReadMultipleRegistersRequest, ReadMultipleRegistersResponse> {

public FC3ReadRegistersTask(int startAddress, Priority priority, ModbusElement... elements) {
super("FC3ReadHoldingRegisters", ReadMultipleRegistersResponse.class, startAddress, priority, elements);
Expand All @@ -26,7 +26,7 @@ protected ReadMultipleRegistersRequest createModbusRequest() {
}

@Override
protected InputRegister[] parseResponse(ReadMultipleRegistersResponse response) throws OpenemsException {
protected Register[] parseResponse(ReadMultipleRegistersResponse response) throws OpenemsException {
return response.getRegisters();
}

Expand Down
16 changes: 12 additions & 4 deletions ...e.bridge.modbus/src/io/openems/edge/bridge/modbus/api/task/FC4ReadInputRegistersTask.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package io.openems.edge.bridge.modbus.api.task;

import java.util.stream.Stream;

import com.ghgande.j2mod.modbus.msg.ReadInputRegistersRequest;
import com.ghgande.j2mod.modbus.msg.ReadInputRegistersResponse;
import com.ghgande.j2mod.modbus.procimg.InputRegister;
import com.ghgande.j2mod.modbus.procimg.Register;
import com.ghgande.j2mod.modbus.procimg.SimpleRegister;

import io.openems.common.exceptions.OpenemsException;
import io.openems.edge.bridge.modbus.api.ModbusUtils;
Expand All @@ -14,7 +17,7 @@
* (http://www.simplymodbus.ca/FC04.htm).
*/
public class FC4ReadInputRegistersTask
extends AbstractReadInputRegistersTask<ReadInputRegistersRequest, ReadInputRegistersResponse> {
extends AbstractReadRegistersTask<ReadInputRegistersRequest, ReadInputRegistersResponse> {

public FC4ReadInputRegistersTask(int startAddress, Priority priority, ModbusElement... elements) {
super("FC4ReadInputRegisters", ReadInputRegistersResponse.class, startAddress, priority, elements);
Expand All @@ -26,8 +29,13 @@ protected ReadInputRegistersRequest createModbusRequest() {
}

@Override
protected InputRegister[] parseResponse(ReadInputRegistersResponse response) throws OpenemsException {
return response.getRegisters();
protected Register[] parseResponse(ReadInputRegistersResponse response) throws OpenemsException {
return Stream.of(response.getRegisters()) //
.map(r -> {
var bs = r.toBytes();
return new SimpleRegister(bs[0], bs[1]);
}) //
.toArray(Register[]::new);
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions io.openems.edge.bridge.modbus/test/io/openems/edge/bridge/modbus/DummyModbusComponent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

import io.openems.common.exceptions.OpenemsException;
import io.openems.common.utils.ConfigUtils;
import io.openems.edge.bridge.modbus.api.AbstractModbusBridge;
import io.openems.edge.bridge.modbus.api.AbstractOpenemsModbusComponent;
import io.openems.edge.bridge.modbus.api.BridgeModbus;
import io.openems.edge.bridge.modbus.api.ModbusComponent;
import io.openems.edge.bridge.modbus.api.ModbusProtocol;
import io.openems.edge.bridge.modbus.test.DummyModbusBridge;
Expand All @@ -29,7 +29,7 @@ public DummyModbusComponent(String id, String bridgeId) throws OpenemsException
this(id, new DummyModbusBridge(bridgeId), DEFAULT_UNIT_ID, new io.openems.edge.common.channel.ChannelId[0]);
}

public DummyModbusComponent(String id, BridgeModbus bridge, int unitId,
public DummyModbusComponent(String id, AbstractModbusBridge bridge, int unitId,
io.openems.edge.common.channel.ChannelId[] additionalChannelIds) throws OpenemsException {
super(//
OpenemsComponent.ChannelId.values(), //
Expand Down
1 change: 0 additions & 1 deletion ...ge.modbus/test/io/openems/edge/bridge/modbus/api/element/SignedDoublewordElementTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public void testReadBigEndianLswMsw() throws OpenemsException {
new SimpleRegister((byte) 0xAB, (byte) 0xCD), //
new SimpleRegister((byte) 0x12, (byte) 0x34) //
});
System.out.println(sut.channel.getNextValue().get());
assertEquals(0x1234_ABCDL, sut.channel.getNextValue().get());
}

Expand Down
3 changes: 0 additions & 3 deletions ...e.bridge.modbus/test/io/openems/edge/bridge/modbus/api/element/StringWordElementTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ public void testWriteBigEndian() throws IllegalArgumentException, OpenemsNamedEx
var sut = new ModbusTest.FC16WriteRegisters<>(new StringWordElement(0, 6), STRING);
sut.channel.setNextWriteValueFromObject("OpenEMS");
var registers = sut.element.getNextWriteValueAndReset();
for (var reg : registers) {
System.out.println(Integer.toHexString(reg.toBytes()[0]) + " " + Integer.toHexString(reg.toBytes()[1]));
}
assertEquals(6, registers.length);
assertArrayEquals(new byte[] { (byte) 0x4F, (byte) 0x70 }, registers[0].toBytes());
assertArrayEquals(new byte[] { (byte) 0x65, (byte) 0x6E }, registers[1].toBytes());
Expand Down
23 changes: 21 additions & 2 deletions ....edge.bridge.modbus/test/io/openems/edge/bridge/modbus/api/task/FC1ReadCoilsTaskTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package io.openems.edge.bridge.modbus.api.task;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.IntStream;

import org.junit.Test;

Expand All @@ -15,9 +19,14 @@
public class FC1ReadCoilsTaskTest {

@Test
public void testToLogMessage() throws OpenemsException {
public void test() throws OpenemsException {
var component = new DummyModbusComponent();
var task = new FC1ReadCoilsTask(10, Priority.HIGH, new CoilElement(10), new CoilElement(11));
var value = new AtomicReference<Boolean>();
var element10 = new CoilElement(10);
element10.onUpdateCallback(v -> value.set(v));
var element11 = new CoilElement(11);
element11.onUpdateCallback(v -> value.set(v));
var task = new FC1ReadCoilsTask(10, Priority.HIGH, element10, element11);
task.setParent(component);
var request = task.createModbusRequest();
var response = (ReadCoilsResponse) request.getResponse();
Expand All @@ -26,5 +35,15 @@ public void testToLogMessage() throws OpenemsException {

assertEquals("FC1ReadCoils [device0;unitid=1;priority=HIGH;ref=10/0xa;length=2;response=10]",
task.toLogMessage(LogVerbosity.READS_AND_WRITES_VERBOSE, request, response));

var coils = response.getCoils();
var values = IntStream.range(0, coils.size()) //
.mapToObj(i -> Boolean.valueOf(coils.getBit(i))) //
.toArray(Boolean[]::new);
assertNull(value.get());
task.handleResponse(element10, 0, values);
assertEquals(Boolean.valueOf(true), value.get());
task.handleResponse(element11, 1, values);
assertEquals(Boolean.valueOf(false), value.get());
}
}
23 changes: 21 additions & 2 deletions ...edge.bridge.modbus/test/io/openems/edge/bridge/modbus/api/task/FC2ReadInputsTaskTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package io.openems.edge.bridge.modbus.api.task;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.IntStream;

import org.junit.Test;

Expand All @@ -15,9 +19,14 @@
public class FC2ReadInputsTaskTest {

@Test
public void testToLogMessage() throws OpenemsException {
public void test() throws OpenemsException {
var component = new DummyModbusComponent();
var task = new FC2ReadInputsTask(10, Priority.HIGH, new CoilElement(10), new CoilElement(11));
var value = new AtomicReference<Boolean>();
var element10 = new CoilElement(10);
element10.onUpdateCallback(v -> value.set(v));
var element11 = new CoilElement(11);
element11.onUpdateCallback(v -> value.set(v));
var task = new FC2ReadInputsTask(10, Priority.HIGH, element10, element11);
task.setParent(component);
var request = task.createModbusRequest();
var response = (ReadInputDiscretesResponse) request.getResponse();
Expand All @@ -26,5 +35,15 @@ public void testToLogMessage() throws OpenemsException {

assertEquals("FC2ReadCoils [device0;unitid=1;priority=HIGH;ref=10/0xa;length=2;response=10]",
task.toLogMessage(LogVerbosity.READS_AND_WRITES_VERBOSE, request, response));

var discretes = response.getDiscretes();
var values = IntStream.range(0, discretes.size()) //
.mapToObj(i -> Boolean.valueOf(discretes.getBit(i))) //
.toArray(Boolean[]::new);
assertNull(value.get());
task.handleResponse(element10, 0, values);
assertEquals(Boolean.valueOf(true), value.get());
task.handleResponse(element11, 1, values);
assertEquals(Boolean.valueOf(false), value.get());
}
}
27 changes: 23 additions & 4 deletions ...e.bridge.modbus/test/io/openems/edge/bridge/modbus/api/task/FC3ReadRegistersTaskTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package io.openems.edge.bridge.modbus.api.task;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

import java.util.concurrent.atomic.AtomicReference;

import org.junit.Test;

Expand All @@ -17,15 +20,31 @@
public class FC3ReadRegistersTaskTest {

@Test
public void testToLogMessage() throws OpenemsException {
public void test() throws OpenemsException {
var component = new DummyModbusComponent();
var task = new FC3ReadRegistersTask(20, Priority.LOW, new UnsignedDoublewordElement(20));
var value = new AtomicReference<Long>();
var element20 = new UnsignedDoublewordElement(20);
element20.onUpdateCallback(v -> value.set(v));
var element22 = new UnsignedDoublewordElement(22);
element22.onUpdateCallback(v -> value.set(v));

var task = new FC3ReadRegistersTask(20, Priority.LOW, element20, element22);
task.setParent(component);
var request = task.createModbusRequest();
var response = (ReadMultipleRegistersResponse) request.getResponse();
response.setRegisters(new Register[] { new SimpleRegister(100), new SimpleRegister(200) });
response.setRegisters(new Register[] { //
new SimpleRegister(100), new SimpleRegister(200), //
new SimpleRegister(300), new SimpleRegister(400) });

assertEquals("FC3ReadHoldingRegisters [device0;unitid=1;priority=LOW;ref=20/0x14;length=2;response=0064 00c8]",
assertEquals(
"FC3ReadHoldingRegisters [device0;unitid=1;priority=LOW;ref=20/0x14;length=4;response=0064 00c8 012c 0190]",
task.toLogMessage(LogVerbosity.READS_AND_WRITES_VERBOSE, request, response));

var registers = task.parseResponse(response);
assertNull(value.get());
task.handleResponse(element20, 0, registers);
assertEquals(Long.valueOf(6553800), value.get());
task.handleResponse(element22, 2, registers);
assertEquals(Long.valueOf(19661200), value.get());
}
}
63 changes: 58 additions & 5 deletions ...dge.modbus/test/io/openems/edge/bridge/modbus/api/task/FC4ReadInputRegistersTaskTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package io.openems.edge.bridge.modbus.api.task;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

import java.util.concurrent.atomic.AtomicReference;

import org.junit.Test;

import com.ghgande.j2mod.modbus.procimg.InputRegister;
import com.ghgande.j2mod.modbus.procimg.Register;
import com.ghgande.j2mod.modbus.procimg.SimpleRegister;
import com.ghgande.j2mod.modbus.procimg.SimpleInputRegister;

import io.openems.common.exceptions.OpenemsException;
import io.openems.edge.bridge.modbus.DummyModbusComponent;
Expand All @@ -15,16 +19,65 @@

public class FC4ReadInputRegistersTaskTest {

/**
* Custom implementation that only implements {@link InputRegister} and not
* {@link Register}. This is the difference to {@link SimpleInputRegister}.
*/
public static class MyInputRegister implements InputRegister {

private final SimpleInputRegister delegate;

public MyInputRegister(int value) {
this.delegate = new SimpleInputRegister(value);
}

@Override
public int getValue() {
return this.delegate.getValue();
}

@Override
public int toUnsignedShort() {
return this.delegate.toUnsignedShort();
}

@Override
public short toShort() {
return this.delegate.toShort();
}

@Override
public byte[] toBytes() {
return this.delegate.toBytes();
}

}

@Test
public void testToLogMessage() throws OpenemsException {
public void test() throws OpenemsException {
var component = new DummyModbusComponent();
var task = new FC4ReadInputRegistersTask(20, Priority.LOW, new UnsignedDoublewordElement(20));
var value = new AtomicReference<Long>();
var element20 = new UnsignedDoublewordElement(20);
element20.onUpdateCallback(v -> value.set(v));
var element22 = new UnsignedDoublewordElement(22);
element22.onUpdateCallback(v -> value.set(v));
var task = new FC4ReadInputRegistersTask(20, Priority.LOW, element20, element22);
task.setParent(component);
var request = task.createModbusRequest();
var response = request.getResponse();
response.setRegisters(new Register[] { new SimpleRegister(987), new SimpleRegister(654) });
response.setRegisters(new InputRegister[] { //
new MyInputRegister(987), new MyInputRegister(654), //
new MyInputRegister(321), new MyInputRegister(0) });

assertEquals("FC4ReadInputRegisters [device0;unitid=1;priority=LOW;ref=20/0x14;length=2;response=03db 028e]",
assertEquals(
"FC4ReadInputRegisters [device0;unitid=1;priority=LOW;ref=20/0x14;length=4;response=03db 028e 0141 0000]",
task.toLogMessage(LogVerbosity.READS_AND_WRITES_VERBOSE, request, response));

var registers = task.parseResponse(response);
assertNull(value.get());
task.handleResponse(element20, 0, registers);
assertEquals(Long.valueOf(64684686), value.get());
task.handleResponse(element22, 2, registers);
assertEquals(Long.valueOf(21037056), value.get());
}
}

0 comments on commit 4c8f3a6

Please sign in to comment.