align the CAN FD handling

the buffer allocation logic when creating frames did not consider the FD_FLAG_FD_FRAME flag when deciding how big of a buffer to allocate. SocketCAN ignores flags (which are an FD feature) if the written buffer is not exactly as big as FD's MTU. fixes #52
pschichtel · Dec 16, 2023 · 2aa22c7 · 2aa22c7
1 parent 024a852
commit 2aa22c7
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 9 deletions.
diff --git a/core-test/src/test/java/tel/schich/javacan/test/RawCanSocketTest.java b/core-test/src/test/java/tel/schich/javacan/test/RawCanSocketTest.java
@@ -32,12 +32,12 @@
 
 import java.nio.ByteBuffer;
 import java.time.Duration;
+import java.util.Arrays;
 
 import static java.time.Duration.ofMillis;
 import static java.time.Duration.ofSeconds;
 import static org.junit.jupiter.api.Assertions.*;
-import static tel.schich.javacan.CanFrame.FD_NO_FLAGS;
-import static tel.schich.javacan.CanFrame.MAX_FD_DATA_LENGTH;
+import static tel.schich.javacan.CanFrame.*;
 import static tel.schich.javacan.CanSocketOptions.*;
 import static tel.schich.javacan.test.CanTestHelper.CAN_INTERFACE;
 
@@ -232,7 +232,7 @@ void testFDFrame() throws Exception {
 
             // more than 8 data bytes
             byte[] data = {0x00, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x22, 0x22, 0x22, 0x22};
-            final CanFrame input = CanFrame.create(0x7ED, FD_NO_FLAGS, data);
+            final CanFrame input = CanFrame.create(0x7ED, FD_FLAG_FD_FRAME, data);
             CanTestHelper.sendFrameViaUtils(CAN_INTERFACE, input);
             Thread.sleep(50);
             final CanFrame output = socket.read();
@@ -241,6 +241,46 @@ void testFDFrame() throws Exception {
         }
     }
 
+    @Test
+    void testSmallFDFrame() throws Exception {
+        byte[] data = new byte[]{0x00, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11};
+        try (final RawCanChannel writer = CanChannels.newRawChannel()) {
+            writer.bind(CAN_INTERFACE);
+            writer.setOption(FD_FRAMES, true);
+            writer.configureBlocking(false);
+
+            try (final RawCanChannel reader = CanChannels.newRawChannel()) {
+                reader.bind(CAN_INTERFACE);
+                reader.setOption(FD_FRAMES, true);
+                reader.configureBlocking(false);
+
+                final CanFrame input = CanFrame.create(0x7ED, FD_FLAG_FD_FRAME, data);
+                writer.write(input);
+                Thread.sleep(50);
+                final CanFrame output = reader.read();
+
+                assertEquals(input, output, "what comes in should come out with JavaCAN");
+            }
+        }
+
+        try (final RawCanChannel reader = CanChannels.newRawChannel()) {
+            reader.bind(CAN_INTERFACE);
+            reader.setOption(FD_FRAMES, true);
+            reader.configureBlocking(false);
+
+            final byte[] actualData;
+            actualData = new byte[MAX_FD_DATA_LENGTH];
+            Arrays.fill(actualData, (byte) 0);
+            System.arraycopy(data, 0, actualData, 0, data.length);
+            final CanFrame input = CanFrame.create(0x7ED, FD_FLAG_FD_FRAME, actualData);
+            CanTestHelper.sendFrameViaUtils(CAN_INTERFACE, input);
+            Thread.sleep(50);
+            final CanFrame output = reader.read();
+
+            assertEquals(input, output, "what comes in should come out with cansend");
+        }
+    }
+
     @Test
     void testBufferReuseWithNonZeroBase() {
         byte[] data = new byte[MAX_FD_DATA_LENGTH];

diff --git a/core/src/main/java/tel/schich/javacan/CanFrame.java b/core/src/main/java/tel/schich/javacan/CanFrame.java
@@ -69,8 +69,10 @@ public class CanFrame {
     public static final int MAX_FD_DATA_LENGTH = 64;
 
     private static final int OFFSET_ID = 0;
-    private static final int OFFSET_DATA_LENGTH = OFFSET_ID + Integer.BYTES;
-    private static final int OFFSET_FLAGS = OFFSET_DATA_LENGTH + 1;
+    private static final int SIZE_ID = Integer.BYTES;
+    private static final int OFFSET_DATA_LENGTH = OFFSET_ID + SIZE_ID;
+    private static final int SIZE_DATA_LENGTH = 1;
+    private static final int OFFSET_FLAGS = OFFSET_DATA_LENGTH + SIZE_DATA_LENGTH;
     private static final int OFFSET_DATA = HEADER_LENGTH;
 
     private final ByteBuffer buffer;
@@ -211,7 +213,7 @@ public void getData(byte[] dest, int offset, int length) {
      * @return true if this frame is an FD frame
      */
     public boolean isFDFrame() {
-        return (getFlags() & FD_FLAG_FD_FRAME) == FD_FLAG_FD_FRAME || getDataLength() > MAX_DATA_LENGTH;
+        return isFDFrame(getFlags(), getDataLength());
     }
 
     /**
@@ -403,10 +405,10 @@ private static CanFrame create(int id, boolean extended, byte flags, byte[] data
      */
     public static CanFrame createRaw(int id, byte flags, byte[] data, int offset, int length) {
         int bufSize;
-        if (data.length <= CanFrame.MAX_DATA_LENGTH) {
-            bufSize = RawCanChannel.MTU;
-        } else {
+        if (isFDFrame(flags, data.length)) {
             bufSize = RawCanChannel.FD_MTU;
+        } else {
+            bufSize = RawCanChannel.MTU;
         }
         ByteBuffer buffer = JavaCAN.allocateOrdered(bufSize);
         buffer.putInt(id)
@@ -457,4 +459,8 @@ public static CanFrame createUnsafe(ByteBuffer buffer) {
         }
         return new CanFrame(buffer);
     }
+
+    private static boolean isFDFrame(byte flags, int dataLength) {
+        return (flags & FD_FLAG_FD_FRAME) == FD_FLAG_FD_FRAME || dataLength > MAX_DATA_LENGTH;
+    }
 }