Skip to content

Commit

Permalink
feat(plc4j)!: Improve stability of code through stricter compiler che…
Browse files Browse the repository at this point in the history
…cks of generated contents.

BREAKING CHANGE: Removed `staticParse(io, ... args)` methods.

Signed-off-by: Łukasz Dywicki <[email protected]>
  • Loading branch information
splatch committed Aug 23, 2024
1 parent b6226d6 commit 5a2e560
Show file tree
Hide file tree
Showing 32 changed files with 159 additions and 194 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -580,32 +580,6 @@ public<#if type.isDiscriminatedParentTypeDefinition()> abstract</#if> class ${ty
return lengthInBits;
}

<#-- The parse and serialize methods here are just proxies for forwardning the requests to static counterparts -->
<#if !type.isDiscriminatedChildTypeDefinition()>
public static ${type.name} staticParse(ReadBuffer readBuffer, Object... args) throws ParseException {
PositionAware positionAware = readBuffer;
<#if parserArguments?has_content>
if((args == null) || (args.length != ${parserArguments?size})) {
throw new PlcRuntimeException("Wrong number of arguments, expected ${parserArguments?size}, but got " + args.length);
}
<#list parserArguments as parserArgument>
<#assign languageName=helper.getLanguageTypeNameForTypeReference(parserArgument.type, false)>
${languageName} ${parserArgument.name};
if(args[${parserArgument?index}] instanceof ${languageName}) {
${parserArgument.name} = (${languageName}) args[${parserArgument?index}];
<#if parserArgument.type.isSimpleTypeReference() || parserArgument.type.isEnumTypeReference()>
} else if (args[${parserArgument?index}] instanceof String) {
${parserArgument.name} = ${languageName}.valueOf((String) args[${parserArgument?index}]);
</#if>
} else {
throw new PlcRuntimeException("Argument ${parserArgument?index} expected to be of type ${languageName} or a string which is parseable but was " + args[${parserArgument?index}].getClass().getName());
}
</#list>
</#if>
return staticParse(readBuffer<#if parserArguments?has_content>, <#list parserArguments as parserArgument>${parserArgument.name}<#sep>, </#sep></#list></#if>);
}

</#if>
<#-- Here come the actual parse and serialize methods that actually do the parsing and serlaizing -->
<#assign hasParserArguments=parserArguments?has_content/>
<#assign parserArgumentList><#if hasParserArguments><#list parserArguments as parserArgument>${helper.getLanguageTypeNameForTypeReference(parserArgument.type, false)} ${parserArgument.name}<#sep>, </#sep></#list></#if></#assign>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
package org.apache.plc4x.java.cbus;

import io.netty.buffer.ByteBuf;
import org.apache.plc4x.java.cbus.readwrite.CBusMessage;
import org.apache.plc4x.java.cbus.readwrite.CBusOptions;
import org.apache.plc4x.java.cbus.readwrite.RequestContext;
import org.apache.plc4x.java.spi.configuration.PlcConnectionConfiguration;
import org.apache.plc4x.java.spi.configuration.PlcTransportConfiguration;
import org.apache.plc4x.java.api.value.PlcValueHandler;
Expand Down Expand Up @@ -92,7 +95,8 @@ protected PlcValueHandler getValueHandler() {

@Override
protected ProtocolStackConfigurer<CBusCommand> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(CBusCommand.class, CBusCommand::staticParse)
return SingleProtocolStackConfigurer.builder(CBusCommand.class, io ->
CBusCommand.staticParse(io, new CBusOptions(false, false, false, false, false, false, false, false, false)))
.withProtocol(CBusProtocolLogic.class)
.withDriverContext(CBusDriverContext.class)
.withPacketSizeEstimator(ByteLengthEstimator.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public CANOpenFrameDataHandler(Supplier<CANFrameBuilder<Message>> builder) {
public CANOpenFrame fromCAN(FrameData frame) {
CANOpenService service = StaticHelper.serviceId((short) frame.getNodeId());
int nodeId = Math.abs(service.getMin() - frame.getNodeId());
return new CANOpenFrame((short) nodeId, service, frame.read(CANOpenPayload::staticParse, service));
return new CANOpenFrame((short) nodeId, service, frame.read(io -> CANOpenPayload.staticParse(io, service)));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public int getNodeId() {
}

@Override
public <T extends Message> T read(MessageInput<T> input, Object... args) {
public <T extends Message> T read(MessageInput<T> input) {
return (T) frame.getPayload();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,9 @@ protected boolean canWrite() {

@Override
protected ProtocolStackConfigurer<EipPacket> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(EipPacket.class, EipPacket::staticParse)
return SingleProtocolStackConfigurer.builder(EipPacket.class, io -> EipPacket.staticParse(io, true))
.withProtocol(EipProtocolLogic.class)
.withPacketSizeEstimator(ByteLengthEstimator.class)
.withParserArgs(true)
.withCorruptPacketRemover(CorruptPackageCleaner.class)
.byteOrder(this.configuration.getByteOrder())
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,9 @@ protected boolean canWrite() {

@Override
protected ProtocolStackConfigurer<EipPacket> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(EipPacket.class, EipPacket::staticParse)
return SingleProtocolStackConfigurer.builder(EipPacket.class, io -> EipPacket.staticParse(io, true))
.withProtocol(EipProtocolLogic.class)
.withPacketSizeEstimator(ByteLengthEstimator.class)
.withParserArgs(true)
.withCorruptPacketRemover(CorruptPackageCleaner.class)
.littleEndian()
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,11 @@ protected boolean awaitDisconnectComplete() {

@Override
protected ProtocolStackConfigurer<FirmataMessage> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(FirmataMessage.class, FirmataMessage::staticParse)
return SingleProtocolStackConfigurer.builder(FirmataMessage.class, io -> FirmataMessage.staticParse(io, true))
.withProtocol(FirmataProtocolLogic.class)
.withDriverContext(FirmataDriverContext.class)
.withPacketSizeEstimator(ByteLengthEstimator.class)
.withCorruptPacketRemover(CorruptPackageCleaner.class)
// Every incoming message is to be treated as a response.
.withParserArgs(true)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,10 @@ protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {

@Override
protected ProtocolStackConfigurer<ModbusAsciiADU> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(ModbusAsciiADU.class,
new ModbusAsciiInput(), new ModbusAsciiOutput())
return SingleProtocolStackConfigurer.builder(ModbusAsciiADU.class, new ModbusAsciiInput(), new ModbusAsciiOutput())
.withProtocol(ModbusAsciiProtocolLogic.class)
.withPacketSizeEstimator(ModbusAsciiDriver.ByteLengthEstimator.class)
.withCorruptPacketRemover(ModbusAsciiDriver.CorruptPackageCleaner.class)
// Every incoming message is to be treated as a response.
.withParserArgs(DriverType.MODBUS_ASCII, true)
.build();
}

Expand All @@ -160,7 +157,7 @@ public int applyAsInt(ByteBuf byteBuf) {
// Try to parse the buffer content.
try {
ModbusAsciiInput input = new ModbusAsciiInput();
ModbusAsciiADU modbusADU = input.parse(reader, DriverType.MODBUS_ASCII, true);
ModbusAsciiADU modbusADU = input.parse(reader);

// Make sure we only read one message.
return (modbusADU.getLengthInBytes() * 2) + 3;
Expand Down Expand Up @@ -198,7 +195,7 @@ public ModbusTag prepareTag(String tagAddress){

public static class ModbusAsciiInput implements MessageInput<ModbusAsciiADU> {
@Override
public ModbusAsciiADU parse(ReadBuffer io, Object... args) throws ParseException {
public ModbusAsciiADU parse(ReadBuffer io) throws ParseException {
// A Modbus ASCII message starts with an ASCII character ":" and is ended by two characters CRLF
// (Carriage-Return + Line-Feed)
// The actual payload is that each byte of the message is encoded by a string representation of it's
Expand Down Expand Up @@ -228,7 +225,7 @@ public ModbusAsciiADU parse(ReadBuffer io, Object... args) throws ParseException

public static class ModbusAsciiOutput implements MessageOutput<ModbusAsciiADU> {
@Override
public WriteBufferByteBased serialize(ModbusAsciiADU value, Object... args) throws SerializationException {
public WriteBufferByteBased serialize(ModbusAsciiADU value) throws SerializationException {
// First serialize the packet the normal way.
WriteBufferByteBased writeBufferByteBased = new WriteBufferByteBased(value.getLengthInBytes());
value.serialize(writeBufferByteBased);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.plc4x.java.spi.connection.GeneratedDriverBase;
import org.apache.plc4x.java.spi.connection.ProtocolStackConfigurer;
import org.apache.plc4x.java.spi.connection.SingleProtocolStackConfigurer;
import org.apache.plc4x.java.spi.generation.MessageInput;
import org.apache.plc4x.java.spi.generation.ParseException;
import org.apache.plc4x.java.spi.generation.ReadBufferByteBased;
import org.apache.plc4x.java.spi.optimizer.BaseOptimizer;
Expand Down Expand Up @@ -129,12 +130,9 @@ protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {

@Override
protected ProtocolStackConfigurer<ModbusRtuADU> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(ModbusRtuADU.class,
(io, args) -> (ModbusRtuADU) ModbusRtuADU.staticParse(io, args))
return SingleProtocolStackConfigurer.builder(ModbusRtuADU.class, io -> (ModbusRtuADU) ModbusRtuADU.staticParse(io, DriverType.MODBUS_RTU, true))
.withProtocol(ModbusRtuProtocolLogic.class)
.withPacketSizeEstimator(ModbusRtuDriver.ByteLengthEstimator.class)
// Every incoming message is to be treated as a response.
.withParserArgs(DriverType.MODBUS_RTU, true)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,9 @@ protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {

@Override
protected ProtocolStackConfigurer<ModbusTcpADU> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(ModbusTcpADU.class,
(io, args) -> (ModbusTcpADU) ModbusTcpADU.staticParse(io, args))
return SingleProtocolStackConfigurer.builder(ModbusTcpADU.class, (io) -> (ModbusTcpADU) ModbusTcpADU.staticParse(io, DriverType.MODBUS_TCP, true))
.withProtocol(ModbusTcpProtocolLogic.class)
.withPacketSizeEstimator(ByteLengthEstimator.class)
// Every incoming message is to be treated as a response.
.withParserArgs(DriverType.MODBUS_TCP, true)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,9 @@ protected boolean awaitDiscoverComplete() {

@Override
protected ProtocolStackConfigurer<OpcuaAPU> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(OpcuaAPU.class, OpcuaAPU::staticParse)
return SingleProtocolStackConfigurer.builder(OpcuaAPU.class, io -> OpcuaAPU.staticParse(io, true))
.withProtocol(OpcuaProtocolLogic.class)
.withPacketSizeEstimator(ByteLengthEstimator.class)
.withParserArgs(true)
.withDriverContext(OpcuaDriverContext.class)
.littleEndian()
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ public static Payload fromStream(byte[] payload, boolean extensible) throws Pars
return Payload.staticParse(buffer, extensible, (long) (extensible ? -1 : payload.length - 8));
}

public static MessagePDU fromStream(ByteBuffer chunkBuffer, boolean response, boolean encrypted) throws ParseException {
public static MessagePDU fromStream(ByteBuffer chunkBuffer, boolean response) throws ParseException {
ReadBufferByteBased buffer = new ReadBufferByteBased(chunkBuffer.array(), ByteOrder.LITTLE_ENDIAN);
return MessagePDU.staticParse(buffer, response, encrypted);
return MessagePDU.staticParse(buffer, response);
}

public static MessagePDU pduFromStream(byte[] message, boolean response) throws ParseException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ protected org.apache.plc4x.java.api.value.PlcValueHandler getValueHandler() {

@Override
protected ProtocolStackConfigurer<OpenProtocolMessage> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(OpenProtocolMessage.class,
OpenProtocolMessage::staticParse)
return SingleProtocolStackConfigurer.builder(OpenProtocolMessage.class, io -> OpenProtocolMessage.staticParse(io, 1))
.withProtocol(OpenProtocolProtocolLogic.class)
.withPacketSizeEstimator(ByteLengthEstimator.class)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ protected ProtocolStackConfigurer<Ethernet_Frame> getStackConfigurer() {
.withProtocol(ProfinetProtocolLogic.class)
.withDriverContext(ProfinetDriverContext.class)
// Every incoming message is to be treated as a response.
.withParserArgs(true)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public class S7HSingleProtocolStackConfigurer<BASE_PACKET_CLASS extends Message>
private final Class<? extends ToIntFunction<ByteBuf>> packetSizeEstimatorClass;
private final Class<? extends Consumer<ByteBuf>> corruptPacketRemoverClass;
private final MessageToMessageCodec<ByteBuf, ByteBuf> encryptionHandler;
private final Object[] parserArgs;

private Plc4xProtocolBase<BASE_PACKET_CLASS> protocol = null;

Expand All @@ -77,7 +76,6 @@ public static <BPC extends Message> S7HSingleProtocolStackBuilder<BPC> builder(C
*/
S7HSingleProtocolStackConfigurer(Class<BASE_PACKET_CLASS> basePacketClass,
ByteOrder byteOrder,
Object[] parserArgs,
Class<? extends Plc4xProtocolBase<BASE_PACKET_CLASS>> protocol,
Class<? extends DriverContext> driverContextClass,
MessageInput<BASE_PACKET_CLASS> messageInput,
Expand All @@ -87,7 +85,6 @@ public static <BPC extends Message> S7HSingleProtocolStackBuilder<BPC> builder(C
MessageToMessageCodec<ByteBuf, ByteBuf> encryptionHandler) {
this.basePacketClass = basePacketClass;
this.byteOrder = byteOrder;
this.parserArgs = parserArgs;
this.protocolClass = protocol;
this.driverContextClass = driverContextClass;
this.messageInput = messageInput;
Expand All @@ -98,7 +95,7 @@ public static <BPC extends Message> S7HSingleProtocolStackBuilder<BPC> builder(C
}

private ChannelHandler getMessageCodec(PlcConnectionConfiguration configuration) {
return new GeneratedProtocolMessageCodec<>(basePacketClass, messageInput, messageOutput, byteOrder, parserArgs,
return new GeneratedProtocolMessageCodec<>(basePacketClass, messageInput, messageOutput, byteOrder,
packetSizeEstimatorClass != null ? configure(configuration, createInstance(packetSizeEstimatorClass)) : null,
corruptPacketRemoverClass != null ? configure(configuration, createInstance(corruptPacketRemoverClass)) : null);
}
Expand Down Expand Up @@ -154,7 +151,6 @@ public static final class S7HSingleProtocolStackBuilder<BASE_PACKET_CLASS extend
private final MessageOutput<BASE_PACKET_CLASS> messageOutput;
private Class<? extends DriverContext> driverContextClass;
private ByteOrder byteOrder = ByteOrder.BIG_ENDIAN;
private Object[] parserArgs;
private Class<? extends Plc4xProtocolBase<BASE_PACKET_CLASS>> protocol;
private Class<? extends ToIntFunction<ByteBuf>> packetSizeEstimator;
private Class<? extends Consumer<ByteBuf>> corruptPacketRemover;
Expand Down Expand Up @@ -186,11 +182,6 @@ public S7HSingleProtocolStackBuilder<BASE_PACKET_CLASS> littleEndian() {
return this;
}

public S7HSingleProtocolStackBuilder<BASE_PACKET_CLASS> withParserArgs(Object... parserArgs) {
this.parserArgs = parserArgs;
return this;
}

public S7HSingleProtocolStackBuilder<BASE_PACKET_CLASS> withProtocol(Class<? extends Plc4xProtocolBase<BASE_PACKET_CLASS>> protocol) {
this.protocol = protocol;
return this;
Expand All @@ -213,7 +204,7 @@ public S7HSingleProtocolStackBuilder<BASE_PACKET_CLASS> withEncryptionHandler(Me

public S7HSingleProtocolStackConfigurer<BASE_PACKET_CLASS> build() {
assert this.protocol != null;
return new S7HSingleProtocolStackConfigurer<>(basePacketClass, byteOrder, parserArgs, protocol,
return new S7HSingleProtocolStackConfigurer<>(basePacketClass, byteOrder, protocol,
driverContextClass, messageInput, messageOutput, packetSizeEstimator, corruptPacketRemover,
encryptionHandler);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,15 @@ public abstract class GeneratedDriverByteToMessageCodec<T extends Message> exten
private static final Logger LOGGER = LoggerFactory.getLogger(GeneratedDriverByteToMessageCodec.class);

private final ByteOrder byteOrder;
private final Object[] parserArgs;
private final MessageInput<T> messageInput;
private final MessageOutput<T> messageOutput;

protected GeneratedDriverByteToMessageCodec(MessageInput<T> messageInput, MessageOutput<T> messageOutput,
Class<T> outboundMessageType, ByteOrder byteOrder, Object[] parserArgs) {
Class<T> outboundMessageType, ByteOrder byteOrder) {
super(outboundMessageType);
this.messageInput = messageInput;
this.messageOutput = messageOutput;
this.byteOrder = byteOrder;
this.parserArgs = parserArgs;
}

@Override
Expand Down Expand Up @@ -84,7 +82,7 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf byteBuf, List<Object> o
ReadBuffer readBuffer = new ReadBufferByteBased(bytes, byteOrder);

// Parse the packet.
T packet = messageInput.parse(readBuffer, parserArgs);
T packet = messageInput.parse(readBuffer);

// Pass the packet to the pipeline.
out.add(packet);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public static <BPC extends Message> CustomProtocolStackBuilder<BPC> builder(Clas
}

private ChannelHandler getMessageCodec(PlcConnectionConfiguration configuration) {
return new GeneratedProtocolMessageCodec<>(basePacketClass, protocolIO.apply(configuration), byteOrder, parserArgs,
return new GeneratedProtocolMessageCodec<>(basePacketClass, protocolIO.apply(configuration), byteOrder,
packetSizeEstimator == null ? null : packetSizeEstimator.apply(configuration),
corruptPacketRemover == null ? null : corruptPacketRemover.apply(configuration));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ public GeneratedProtocolMessageCodec(
Class<BASE_PACKET_CLASS> basePacketClass,
MessageInput<BASE_PACKET_CLASS> messageInput,
ByteOrder byteOrder,
Object[] parserArgs,
ToIntFunction<ByteBuf> packetSizeEstimator,
Consumer<ByteBuf> corruptPackageRemover) {
super(messageInput, null, basePacketClass, byteOrder, parserArgs);
super(messageInput, null, basePacketClass, byteOrder);
this.packetSizeEstimator = packetSizeEstimator;
this.corruptPackageRemover = corruptPackageRemover;
}
Expand All @@ -47,10 +46,9 @@ public GeneratedProtocolMessageCodec(
MessageInput<BASE_PACKET_CLASS> messageInput,
MessageOutput<BASE_PACKET_CLASS> messageOutput,
ByteOrder byteOrder,
Object[] parserArgs,
ToIntFunction<ByteBuf> packetSizeEstimator,
Consumer<ByteBuf> corruptPackageRemover) {
super(messageInput, messageOutput, basePacketClass, byteOrder, parserArgs);
super(messageInput, messageOutput, basePacketClass, byteOrder);
this.packetSizeEstimator = packetSizeEstimator;
this.corruptPackageRemover = corruptPackageRemover;
}
Expand Down
Loading

0 comments on commit 5a2e560

Please sign in to comment.