CANopen driver fixes.

splatch · splatch · commit 1e0884689874 · 2024-03-11T19:02:38.000+01:00
Signed-off-by: Łukasz Dywicki &lt;luke@code-house.org&gt;
diff --git a/plc4j/drivers/can/src/main/java/org/apache/plc4x/java/can/generic/transport/GenericCANFrameDataHandler.java b/plc4j/drivers/can/src/main/java/org/apache/plc4x/java/can/generic/transport/GenericCANFrameDataHandler.java
@@ -24,6 +24,12 @@
 import org.apache.plc4x.java.transport.can.CANTransport;
 import org.apache.plc4x.java.transport.can.FrameData;
 
+/**
+ * Generic CAN frame handler turn a wire level message @{@link FrameData} into a wrapper which
+ * does not have any specific other than node id and data.
+ *
+ * Because it is used by generic purpose driver it can not assume any semantics on message role.
+ */
 public class GenericCANFrameDataHandler implements CANTransport.FrameHandler<Message, GenericFrame> {
 
     private final Supplier<CANFrameBuilder<Message>> frameBuilder;
diff --git a/plc4j/drivers/can/src/main/java/org/apache/plc4x/java/can/generic/transport/GenericFrame.java b/plc4j/drivers/can/src/main/java/org/apache/plc4x/java/can/generic/transport/GenericFrame.java
@@ -18,11 +18,10 @@
  */
 package org.apache.plc4x.java.can.generic.transport;
 
-import org.apache.plc4x.java.spi.generation.Message;
-import org.apache.plc4x.java.spi.generation.SerializationException;
-import org.apache.plc4x.java.spi.generation.WriteBuffer;
-
-public class GenericFrame implements Message {
+/**
+ * Wrapper for wire level data.
+ */
+public class GenericFrame {
 
     private final int nodeId;
     private final byte[] data;
@@ -40,27 +39,4 @@ public byte[] getData() {
         return data;
     }
 
-    @Override
-    public void serialize(WriteBuffer writeBuffer) throws SerializationException {
-        // TODO: Is this correct?
-        writeBuffer.writeUnsignedShort("length", 8, (short) data.length);
-        writeBuffer.writeUnsignedInt("nodeId", 32, nodeId);
-        writeBuffer.writeByteArray("data", data);
-    }
-
-    @Override
-    public int getLengthInBytes() {
-        return 0;
-    }
-
-    @Override
-    public int getLengthInBits() {
-        return 0;
-    }
-
-    /*@Override
-    public MessageIO<? extends Message, ? extends Message> getMessageIO() {
-        return null;
-    }*/
-
 }
diff --git a/plc4j/drivers/can/src/test/java/org/apache/plc4x/java/can/generic/GenericCANDriverTest.java b/plc4j/drivers/can/src/test/java/org/apache/plc4x/java/can/generic/GenericCANDriverTest.java
@@ -18,18 +18,40 @@
  */
 package org.apache.plc4x.java.can.generic;
 
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufAllocator;
+import io.netty.buffer.ByteBufUtil;
+import io.netty.buffer.Unpooled;
+import io.netty.buffer.UnpooledDirectByteBuf;
+import io.netty.channel.Channel;
+import io.netty.channel.ChannelHandlerAdapter;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.channel.ChannelOutboundHandlerAdapter;
+import io.netty.channel.ChannelPromise;
+import io.netty.channel.embedded.EmbeddedChannel;
+import io.netty.channel.embedded.Plc4xEmbeddedChannel;
+import java.util.Optional;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.plc4x.java.DefaultPlcDriverManager;
 import org.apache.plc4x.java.api.PlcConnection;
 import org.apache.plc4x.java.api.exceptions.PlcConnectionException;
 import org.apache.plc4x.java.api.messages.PlcSubscriptionEvent;
+import org.apache.plc4x.java.spi.connection.ChannelExposingConnection;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.*;
 
+/**
+ * Test of generic can driver with virtual can transport.
+ *
+ * This test have additional role of confirming end to end behavior of driver and transport layer.
+ * The virtual can transport rely on netty channel/pipeline infrastructure, hence it does not have
+ * any backend such as memory queue. It simply converts message to a stream and expect it to be read.
+ */
 public class GenericCANDriverTest {
 
     @Test
@@ -44,20 +66,26 @@ void testConnection() throws PlcConnectionException {
     }
 
     @Test
-    @Disabled("This test requires working virtual CAN transport to be truly platform independent")
     void testSubscribeAndWrite() throws Exception {
-//        PlcConnection connection1 = new PlcDriverManager().getConnection("genericcan:socketcan://vcan0");
-//        PlcConnection connection2 = new PlcDriverManager().getConnection("genericcan:socketcan://vcan0");
-        PlcConnection connection1 = new DefaultPlcDriverManager().getConnection("genericcan:virtualcan://");
-        PlcConnection connection2 = connection1;
+        PlcConnection connection = new DefaultPlcDriverManager().getConnection("genericcan:virtualcan://");
+
+        Plc4xEmbeddedChannel subscribeChannel = null;
+        Plc4xEmbeddedChannel writeChannel = null;
+        if (connection instanceof ChannelExposingConnection) {
+            Channel channel = ((ChannelExposingConnection) connection).getChannel();
+            if (channel instanceof Plc4xEmbeddedChannel) {
+                subscribeChannel = (Plc4xEmbeddedChannel) channel;
+                writeChannel = (Plc4xEmbeddedChannel) channel;
+            }
+        }
 
         CountDownLatch latch = new CountDownLatch(1);
-        Byte tag1 = 0x55;
+        short tag1 = 0x55;
         short tag2 = 10;
         short tag3 = 50;
 
         final AtomicReference<PlcSubscriptionEvent> plcEvent = new AtomicReference<>();
-        connection1.subscriptionRequestBuilder()
+        connection.subscriptionRequestBuilder()
             .addEventTagAddress("tag1", "200:BYTE")
             .addEventTagAddress("tag2", "200:UNSIGNED8")
             .addEventTagAddress("tag3", "200:UNSIGNED8")
@@ -73,7 +101,7 @@ void testSubscribeAndWrite() throws Exception {
                 });
             });
 
-        connection2.writeRequestBuilder()
+        connection.writeRequestBuilder()
             .addTagAddress("f1", "200:BYTE", tag1)
             .addTagAddress("f2", "200:UNSIGNED8", tag2)
             .addTagAddress("f3", "200:UNSIGNED8", tag3)
@@ -83,10 +111,16 @@ void testSubscribeAndWrite() throws Exception {
                 }
             }).get();
 
+        // copy outbound message to inbound queue to confirm that transport API works and subscription
+        // is properly matched against incoming message
+        ByteBuf outgoing = writeChannel.flushOutbound().readOutbound();
+        subscribeChannel.writeInbound(Unpooled.wrappedBuffer(outgoing));
+        subscribeChannel.flushInbound();
+
         latch.await();
 
         PlcSubscriptionEvent event = plcEvent.get();
-        assertEquals(tag1, event.getByte("tag1"));
+        assertEquals(tag1, (short) event.getByte("tag1"));
         assertEquals(tag2, event.getShort("tag2"));
         assertEquals(tag3, event.getShort("tag3"));
 
diff --git a/plc4j/drivers/canopen/src/main/generated/org/apache/plc4x/java/canopen/readwrite/CANOpenDataType.java b/plc4j/drivers/canopen/src/main/generated/org/apache/plc4x/java/canopen/readwrite/CANOpenDataType.java
@@ -45,8 +45,8 @@ public enum CANOpenDataType {
   INTEGER64((long) 16L, (String) "LINT", (short) 64),
   REAL32((long) 17L, (String) "REAL", (short) 32),
   REAL64((long) 18L, (String) "LREAL", (short) 64),
-  RECORD((long) 19L, (String) "BYTE", (short) 8),
-  OCTET_STRING((long) 20L, (String) "BYTE", (short) 8),
+  RECORD((long) 19L, (String) "RAW_BYTE_ARRAY", (short) 8),
+  OCTET_STRING((long) 20L, (String) "RAW_BYTE_ARRAY", (short) 8),
   VISIBLE_STRING((long) 21L, (String) "CHAR", (short) 8),
   UNICODE_STRING((long) 22L, (String) "WCHAR", (short) 16),
   TIME_OF_DAY((long) 23L, (String) "TIME_OF_DAY", (short) 48),
diff --git a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/values/PlcRawByteArray.java b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/values/PlcRawByteArray.java
@@ -43,12 +43,13 @@ public PlcValueType getPlcValueType() {
     }
 
     @Override
-    public String toString() {
-        return Hex.encodeHexString(value);
+    public byte[] getRaw() {
+        return value;
     }
 
-    public byte[] getBytes() {
-        return value;
+    @Override
+    public String toString() {
+        return Hex.encodeHexString(value);
     }
 
     @Override
diff --git a/plc4j/transports/socketcan/src/main/java/org/apache/plc4x/java/transport/socketcan/SocketCANTransport.java b/plc4j/transports/socketcan/src/main/java/org/apache/plc4x/java/transport/socketcan/SocketCANTransport.java
@@ -26,6 +26,7 @@
 import org.apache.plc4x.java.spi.configuration.PlcConnectionConfiguration;
 import org.apache.plc4x.java.api.exceptions.PlcRuntimeException;
 import org.apache.plc4x.java.socketcan.readwrite.SocketCANFrame;
+import org.apache.plc4x.java.spi.configuration.PlcTransportConfiguration;
 import org.apache.plc4x.java.spi.connection.ChannelFactory;
 import org.apache.plc4x.java.spi.generation.*;
 import org.apache.plc4x.java.transport.can.CANFrameBuilder;
@@ -59,6 +60,14 @@ public ToIntFunction<ByteBuf> getEstimator() {
         return (buff) -> 16;
     }
 
+    public static class EmptyConfiguration implements PlcTransportConfiguration {}
+
+    @Override
+    public Class<? extends PlcTransportConfiguration> getTransportConfigType() {
+        return EmptyConfiguration.class;
+    }
+
+
     @Override
     public MessageInput<SocketCANFrame> getMessageInput(PlcConnectionConfiguration cfg) {
         return SocketCANFrame::staticParse;
diff --git a/plc4j/transports/virtualcan/src/main/java/org/apache/plc4x/java/transport/virtualcan/VirtualCANFrame.java b/plc4j/transports/virtualcan/src/main/java/org/apache/plc4x/java/transport/virtualcan/VirtualCANFrame.java
@@ -18,12 +18,36 @@
  */
 package org.apache.plc4x.java.transport.virtualcan;
 
+import static org.apache.plc4x.java.spi.codegen.fields.FieldWriterFactory.*;
+import static org.apache.plc4x.java.spi.codegen.fields.FieldReaderFactory.*;
+import static org.apache.plc4x.java.spi.codegen.io.DataWriterFactory.*;
+
 import org.apache.commons.codec.binary.Hex;
+import org.apache.plc4x.java.spi.codegen.WithOption;
+import org.apache.plc4x.java.spi.generation.ByteOrder;
 import org.apache.plc4x.java.spi.generation.Message;
+import org.apache.plc4x.java.spi.generation.MessageInput;
+import org.apache.plc4x.java.spi.generation.ParseException;
+import org.apache.plc4x.java.spi.generation.ReadBuffer;
 import org.apache.plc4x.java.spi.generation.SerializationException;
 import org.apache.plc4x.java.spi.generation.WriteBuffer;
 
 public class VirtualCANFrame implements Message {
+
+    public final static MessageInput<VirtualCANFrame> PARSER = new MessageInput<VirtualCANFrame>() {
+
+        @Override
+        public VirtualCANFrame parse(ReadBuffer io, Object... args) throws ParseException {
+            WithOption withOption = WithOption.WithByteOrder(ByteOrder.LITTLE_ENDIAN);
+
+            short length = io.readUnsignedShort("length", 8, withOption);
+            int nodeId = io.readInt("nodeId", 32, withOption);
+            byte[] data = io.readByteArray("data", length, withOption);
+
+            return new VirtualCANFrame(nodeId, data);
+        }
+    };
+
     private final int nodeId;
     private final byte[] data;
 
@@ -42,19 +66,22 @@ public byte[] getData() {
 
     @Override
     public void serialize(WriteBuffer writeBuffer) throws SerializationException {
-        writeBuffer.writeUnsignedShort("length", 8, (short) data.length);
-        writeBuffer.writeUnsignedInt("nodeId", 32, nodeId);
-        writeBuffer.writeByteArray("data", data);
+        WithOption withOption = WithOption.WithByteOrder(ByteOrder.LITTLE_ENDIAN);
+
+        writeSimpleField("length", (short) data.length, writeUnsignedShort(writeBuffer, 8), withOption);
+        writeSimpleField("nodeId", nodeId, writeSignedInt(writeBuffer, 32), withOption);
+        writeByteArrayField("data", data, writeByteArray(writeBuffer, 8), withOption);
     }
 
+
     @Override
     public int getLengthInBytes() {
-        return 0;
+        return 1 + 4 + data.length;
     }
 
     @Override
     public int getLengthInBits() {
-        return 0;
+        return getLengthInBytes() * 8;
     }
 
     public String toString() {
diff --git a/plc4j/transports/virtualcan/src/main/java/org/apache/plc4x/java/transport/virtualcan/VirtualCANTransport.java b/plc4j/transports/virtualcan/src/main/java/org/apache/plc4x/java/transport/virtualcan/VirtualCANTransport.java
@@ -21,12 +21,12 @@
 import io.netty.buffer.ByteBuf;
 import org.apache.plc4x.java.spi.configuration.PlcConnectionConfiguration;
 import org.apache.plc4x.java.api.exceptions.PlcRuntimeException;
+import org.apache.plc4x.java.spi.configuration.PlcTransportConfiguration;
 import org.apache.plc4x.java.spi.generation.*;
 import org.apache.plc4x.java.transport.can.CANFrameBuilder;
 import org.apache.plc4x.java.transport.can.CANTransport;
 import org.apache.plc4x.java.transport.can.FrameData;
 import org.apache.plc4x.java.transport.test.TestTransport;
-import org.apache.plc4x.java.transport.virtualcan.io.VirtualCANFrameIO;
 
 import java.util.function.Function;
 import java.util.function.ToIntFunction;
@@ -45,14 +45,21 @@ public String getTransportName() {
 
     @Override
     public ToIntFunction<ByteBuf> getEstimator() {
-        return value -> value.getShort(value.readerIndex());
+        return ByteBuf::readableBytes;
     }
 
     @Override
     public Class<VirtualCANFrame> getMessageType() {
         return VirtualCANFrame.class;
     }
 
+    public static class EmptyConfiguration implements PlcTransportConfiguration {}
+
+    @Override
+    public Class<? extends PlcTransportConfiguration> getTransportConfigType() {
+        return EmptyConfiguration.class;
+    }
+
     @Override
     public CANFrameBuilder<VirtualCANFrame> getTransportFrameBuilder() {
         return new CANFrameBuilder<>() {
@@ -110,7 +117,7 @@ public byte[] getData() {
 
     @Override
     public MessageInput<VirtualCANFrame> getMessageInput(PlcConnectionConfiguration configuration) {
-        return new VirtualCANFrameIO();
+        return VirtualCANFrame.PARSER;
     }
 
 }
diff --git a/plc4j/transports/virtualcan/src/main/java/org/apache/plc4x/java/transport/virtualcan/io/VirtualCANFrameIO.java b/plc4j/transports/virtualcan/src/main/java/org/apache/plc4x/java/transport/virtualcan/io/VirtualCANFrameIO.java
diff --git a/protocols/canopen/src/main/resources/protocols/can/canopen.mspec b/protocols/canopen/src/main/resources/protocols/can/canopen.mspec
@@ -262,8 +262,8 @@
     [REAL64      ['64', '"LREAL"'         ] ]
 
     // compound/complex types
-    [RECORD           [ '8', '"BYTE"'       ] ]
-    [OCTET_STRING     [ '8', '"BYTE"'       ] ]
+    [RECORD           [ '8', '"RAW_BYTE_ARRAY"' ] ]
+    [OCTET_STRING     [ '8', '"RAW_BYTE_ARRAY"' ] ]
     [VISIBLE_STRING   [ '8', '"CHAR"'       ] ]
     [UNICODE_STRING   ['16', '"WCHAR"'      ] ]
     [TIME_OF_DAY      ['48', '"TIME_OF_DAY"'] ]
@@ -330,7 +330,7 @@
             [simple float 64 value]
         ]
         ['RECORD' List(int 32 size)
-            [array byte value length 'size']
+            [array byte   value count 'size']
         ]
         ['OCTET_STRING' STRING
            [simple vstring 'size' value]

Original file line number	Diff line number	Diff line change
`@@ -43,12 +43,13 @@ public PlcValueType getPlcValueType() {`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`@Override`
`46`		`- public String toString() {`
`47`		`- return Hex.encodeHexString(value);`
	`46`	`+ public byte[] getRaw() {`
	`47`	`+ return value;`
`48`	`48`	`}`
`49`	`49`
`50`		`- public byte[] getBytes() {`
`51`		`- return value;`
	`50`	`+ @Override`
	`51`	`+ public String toString() {`
	`52`	`+ return Hex.encodeHexString(value);`
`52`	`53`	`}`
`53`	`54`
`54`	`55`	`@Override`