[Bug]: (S7-light) Issue when reading Db Addresses which are undefined

[frankpuppa]

What happened?

Hello,
I noticed that when reading a DB variable using the S7-light protocol, the library throws a NullPointerException if the variable is not defined in the DB.
Here is an example of the error:

	at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315)
	at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320)
	at java.base/java.util.concurrent.CompletableFuture.uniHandle(CompletableFuture.java:936)
	at java.base/java.util.concurrent.CompletableFuture$UniHandle.tryFire(CompletableFuture.java:911)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
	at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2179)
	at org.apache.plc4x.java.s7light.readwrite.protocol.S7ProtocolLogic.lambda$14(S7ProtocolLogic.java:212)
	at java.base/java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:863)
	at java.base/java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:841)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
	at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2179)
	at org.apache.plc4x.java.s7light.readwrite.protocol.S7ProtocolLogic.lambda$26(S7ProtocolLogic.java:518)
	at java.base/java.util.function.Consumer.lambda$andThen$0(Consumer.java:65)
	at org.apache.plc4x.java.spi.Plc4xNettyWrapper.decode(Plc4xNettyWrapper.java:194)
	at io.netty.handler.codec.MessageToMessageCodec$1.decode(MessageToMessageCodec.java:67)
	at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:91)
	at io.netty.handler.codec.MessageToMessageCodec.channelRead(MessageToMessageCodec.java:120)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:107)
	at io.netty.handler.codec.MessageToMessageCodec.channelRead(MessageToMessageCodec.java:120)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318)
	at io.netty.handler.codec.ByteToMessageCodec.channelRead(ByteToMessageCodec.java:103)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868)
	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:796)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:732)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:658)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.lang.NullPointerException: Cannot invoke "org.apache.plc4x.java.api.value.PlcValue.getRaw()" because "plcValue" is null
	at org.apache.plc4x.java.s7light.readwrite.optimizer.S7BlockReadOptimizer.processReadResponses(S7BlockReadOptimizer.java:342)
	at org.apache.plc4x.java.spi.optimizer.BaseOptimizer.lambda$1(BaseOptimizer.java:112)
	at org.apache.plc4x.java.spi.optimizer.BaseOptimizer.lambda$8(BaseOptimizer.java:205)
	at java.base/java.util.concurrent.CompletableFuture.uniHandle(CompletableFuture.java:934)

Unfortunately, this is not ideal, especially when reading multiple variables at once (such as when polling a device periodically), since the entire request fails. A better approach would be to populate the response with an empty value and let the code handle the result based on the PlcResponseCode of the tag.

I’ve made an attempt to solve this issue, and it works well in the tests I’ve run so far. You can review the changes here: commit
I am willing to open a PR if this looks fine to you.

Version

v0.13.0

Programming Languages

• plc4c
• plc4go
• plc4j
• plc4net
• plc4py

Protocols

• AB-Ethernet
• ADS /AMS
• BACnet/IP
• C-Bus
• CANopen
• EtherNet/IP
• Firmata
• IEC-69870
• KNXnet/IP
• Modbus
• OPC-UA
• Profinet
• S7
• S7-light

[chrisdutz]

Hmmm .... ok ... interesting ... I should have a look.
But having a look at your commit, probably it should not return a PlcValue for that item and instead return a PlcStatus code ... I'll investigate.

[frankpuppa]

Hi Chris,
The change is only to fill the PlcValue with zero and fix the null pointer exception. The status code (PlcResponseCode) remains unchanged. For this specific case is (PlcResponseCode.INVALID_ADDRESS). But yeah, I am sure there is a better way to handle this. Perhaps without allocating the buffer.