This is an automated email from the ASF dual-hosted git repository. sruehl pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git
commit 917c7fbb479c5f57ce75cc37bf4b5ddaa65fd850 Author: Sebastian Rühl <sru...@apache.org> AuthorDate: Thu Jul 19 11:14:45 2018 +0200 added sanity checks for produced values and cleaned up type checks --- .../java/modbus/netty/Plc4XModbusProtocol.java | 91 +++++++++++++--------- 1 file changed, 56 insertions(+), 35 deletions(-) diff --git a/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/netty/Plc4XModbusProtocol.java b/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/netty/Plc4XModbusProtocol.java index a12aca4..9a050f4 100644 --- a/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/netty/Plc4XModbusProtocol.java +++ b/plc4j/protocols/modbus/src/main/java/org/apache/plc4x/java/modbus/netty/Plc4XModbusProtocol.java @@ -84,6 +84,10 @@ public class Plc4XModbusProtocol extends MessageToMessageCodec<ModbusTcpPayload, RegisterModbusAddress registerModbusAddress = (RegisterModbusAddress) address; if (quantity > 1) { byte[] bytesToWrite = produceRegisterValue(writeRequestItem.getValues()); + int requiredLength = 2 * quantity; + if (bytesToWrite.length != requiredLength) { + throw new PlcProtocolException("Invalid register values created. Should be at least quantity * 2 = N bytes. Was " + bytesToWrite.length + ", expected " + requiredLength); + } modbusRequest = new WriteMultipleRegistersRequest(registerModbusAddress.getAddress(), quantity, bytesToWrite); } else { byte[] register = produceRegisterValue(writeRequestItem.getValues()); @@ -94,6 +98,10 @@ public class Plc4XModbusProtocol extends MessageToMessageCodec<ModbusTcpPayload, CoilModbusAddress coilModbusAddress = (CoilModbusAddress) address; if (quantity > 1) { byte[] bytesToWrite = produceCoilValues(writeRequestItem.getValues()); + int requiredLength = (quantity + 7) / 8; + if (bytesToWrite.length != requiredLength) { + throw new PlcProtocolException("Invalid coil values created. Should be at least (quantity + 7) / 8 = N bytes. Was " + bytesToWrite.length + ", expected " + requiredLength); + } modbusRequest = new WriteMultipleCoilsRequest(coilModbusAddress.getAddress(), quantity, bytesToWrite); } else { boolean booleanToWrite = produceCoilValue(writeRequestItem.getValues()); @@ -167,7 +175,7 @@ public class Plc4XModbusProtocol extends MessageToMessageCodec<ModbusTcpPayload, // TODO: only single Item supported for now PlcRequest<?> request = plcRequestContainer.getRequest(); RequestItem requestItem = request.getRequestItem().orElseThrow(() -> new PlcProtocolException("Only single message supported for now")); - Class datatype = requestItem.getDatatype(); + Class<?> dataType = requestItem.getDatatype(); ModbusPdu modbusPdu = msg.getModbusPdu(); short unitId = msg.getUnitId(); @@ -197,14 +205,14 @@ public class Plc4XModbusProtocol extends MessageToMessageCodec<ModbusTcpPayload, ReadCoilsResponse readCoilsResponse = (ReadCoilsResponse) modbusPdu; LOGGER.debug("{}: Nothing", readCoilsResponse); ByteBuf byteBuf = readCoilsResponse.getCoilStatus(); - List data = produceCoilValueList(requestItem, datatype, byteBuf); + List<?> data = produceCoilValueList(requestItem, dataType, byteBuf); plcRequestContainer.getResponseFuture().complete(new PlcReadResponse((PlcReadRequest) request, new ReadResponseItem((ReadRequestItem) requestItem, ResponseCode.OK, data))); } else if (modbusPdu instanceof ReadDiscreteInputsResponse) { // TODO: finish implementation ReadDiscreteInputsResponse readDiscreteInputsResponse = (ReadDiscreteInputsResponse) modbusPdu; LOGGER.debug("{}: Nothing", readDiscreteInputsResponse); ByteBuf byteBuf = readDiscreteInputsResponse.getInputStatus(); - List data = produceCoilValueList(requestItem, datatype, byteBuf); + List<?> data = produceCoilValueList(requestItem, dataType, byteBuf); plcRequestContainer.getResponseFuture().complete(new PlcReadResponse((PlcReadRequest) request, new ReadResponseItem((ReadRequestItem) requestItem, ResponseCode.OK, data))); } else if (modbusPdu instanceof ReadHoldingRegistersResponse) { // TODO: finish implementation @@ -212,7 +220,7 @@ public class Plc4XModbusProtocol extends MessageToMessageCodec<ModbusTcpPayload, LOGGER.debug("{}: Nothing", readHoldingRegistersResponse); ByteBuf byteBuf = readHoldingRegistersResponse.getRegisters(); // TODO: use register method - List data = produceRegisterValueList(requestItem, datatype, byteBuf); + List<?> data = produceRegisterValueList(requestItem, dataType, byteBuf); plcRequestContainer.getResponseFuture().complete(new PlcReadResponse((PlcReadRequest) request, new ReadResponseItem((ReadRequestItem) requestItem, ResponseCode.OK, data))); } else if (modbusPdu instanceof ReadInputRegistersResponse) { // TODO: finish implementation @@ -220,7 +228,7 @@ public class Plc4XModbusProtocol extends MessageToMessageCodec<ModbusTcpPayload, LOGGER.debug("{}: Nothing", readInputRegistersResponse); ByteBuf byteBuf = readInputRegistersResponse.getRegisters(); // TODO: use register method - List data = produceRegisterValueList(requestItem, datatype, byteBuf); + List<?> data = produceRegisterValueList(requestItem, dataType, byteBuf); plcRequestContainer.getResponseFuture().complete(new PlcReadResponse((PlcReadRequest) request, new ReadResponseItem((ReadRequestItem) requestItem, ResponseCode.OK, data))); } else if (modbusPdu instanceof MaskWriteRegisterResponse) { // TODO: finish implementation @@ -317,7 +325,6 @@ public class Plc4XModbusProtocol extends MessageToMessageCodec<ModbusTcpPayload, // We only have one coil return new byte[]{actualCoil}; } - // TODO: ensure we have a least (quantity + 7) / 8 = N bytes return ArrayUtils.toPrimitive(coils.toArray(new Byte[0])); } @@ -345,7 +352,6 @@ public class Plc4XModbusProtocol extends MessageToMessageCodec<ModbusTcpPayload, buffer.writeShort((int) value); } } - // TODO: ensure we have a least quantity * 2 = N bytes byte[] result = new byte[buffer.writerIndex()]; buffer.readBytes(result); return result; @@ -354,65 +360,80 @@ public class Plc4XModbusProtocol extends MessageToMessageCodec<ModbusTcpPayload, //////////////////////////////////////////////////////////////////////////////// // Decoding helpers. //////////////////////////////////////////////////////////////////////////////// - - @SuppressWarnings("unchecked") - private List produceCoilValueList(RequestItem requestItem, Class datatype, ByteBuf byteBuf) { + private <T> List<T> produceCoilValueList(RequestItem requestItem, Class<T> dataType, ByteBuf byteBuf) { ReadRequestItem readRequestItem = (ReadRequestItem) requestItem; byte[] bytes = new byte[byteBuf.readableBytes()]; if (bytes.length < 1) { return Collections.emptyList(); } byteBuf.readBytes(bytes); - List data = new LinkedList(); + List<T> data = new LinkedList<>(); for (int i = 0, j = 0; i < readRequestItem.getSize(); i++) { if (i != 0 && i % 8 == 0) { // Every 8 Coils we need to increase the access j++; } - Boolean coilValue = (1 << i & bytes[j]) == 1; - if (datatype == Boolean.class) { - data.add(coilValue); - } else if (datatype == Byte.class) { - data.add((byte) (coilValue ? 1 : 0)); - } else if (datatype == byte[].class) { - data.add(new byte[]{(byte) (coilValue ? 1 : 0)}); - } else if (datatype == Short.class) { - data.add((short) (coilValue ? 1 : 0)); - } else if (datatype == Integer.class) { - data.add(coilValue ? 1 : 0); + boolean coilSet = (1 << i & bytes[j]) == 1; + byte coilFlag = coilSet ? (byte) 1 : (byte) 0; + if (dataType == Boolean.class) { + @SuppressWarnings("unchecked") + T itemToBeAdded = (T) Boolean.valueOf(coilSet); + data.add(itemToBeAdded); + } else if (dataType == Byte.class) { + @SuppressWarnings("unchecked") + T itemToBeAdded = (T) Byte.valueOf(coilFlag); + data.add(itemToBeAdded); + } else if (dataType == byte[].class) { + data.add((T) new byte[]{coilFlag}); + } else if (dataType == Short.class) { + @SuppressWarnings("unchecked") + T itemToBeAdded = (T) Short.valueOf(coilFlag); + data.add(itemToBeAdded); + } else if (dataType == Integer.class) { + @SuppressWarnings("unchecked") + T itemToBeAdded = (T) Integer.valueOf(coilFlag); + data.add(itemToBeAdded); } } return data; } - @SuppressWarnings("unchecked") - private List produceRegisterValueList(RequestItem requestItem, Class datatype, ByteBuf byteBuf) throws PlcProtocolException { + private <T> List<T> produceRegisterValueList(RequestItem requestItem, Class<T> dataType, ByteBuf byteBuf) throws PlcProtocolException { ReadRequestItem readRequestItem = (ReadRequestItem) requestItem; int readableBytes = byteBuf.readableBytes(); if (readableBytes % 2 != 0) { throw new PlcProtocolException("Readables bytes should even: " + readableBytes); } - List data = new LinkedList(); + List<T> data = new LinkedList<>(); for (int i = 0; i < readRequestItem.getSize(); i++) { byte[] register = new byte[2]; byteBuf.readBytes(register); int intValue = register[0] << 8 | register[1]; - if (datatype == Boolean.class) { - data.add(intValue == 1); - } else if (datatype == Byte.class) { + if (dataType == Boolean.class) { + @SuppressWarnings("unchecked") + T itemToBeAdded = (T) Boolean.valueOf(intValue == 1); + data.add(itemToBeAdded); + } else if (dataType == Byte.class) { if (intValue > Byte.MAX_VALUE) { throw new PlcProtocolException("Value to high to fit into Byte: " + intValue); } - data.add((byte) intValue); - } else if (datatype == byte[].class) { - data.add(register); - } else if (datatype == Short.class) { + @SuppressWarnings("unchecked") + T itemToBeADded = (T) Byte.valueOf((byte) intValue); + data.add(itemToBeADded); + } else if (dataType == byte[].class) { + T itemToBeAdded = (T) register; + data.add(itemToBeAdded); + } else if (dataType == Short.class) { if (intValue > Short.MAX_VALUE) { throw new PlcProtocolException("Value to high to fit into Short: " + intValue); } - data.add((short) intValue); - } else if (datatype == Integer.class) { - data.add(intValue); + @SuppressWarnings("unchecked") + T itemToBeAdded = (T) Short.valueOf((short) intValue); + data.add(itemToBeAdded); + } else if (dataType == Integer.class) { + @SuppressWarnings("unchecked") + T itemToBeAdded = (T) Integer.valueOf(intValue); + data.add(itemToBeAdded); } } return data;