Merge pull request #2271 from ControlSystemStudio/pva_byteorder

kasemir · web-flow · commit c29754819a64 · 2022-05-24T16:31:33.000-04:00
PVA: Check/clarify handling of byte order
diff --git a/core/pva/src/main/java/org/epics/pva/client/ClientTCPHandler.java b/core/pva/src/main/java/org/epics/pva/client/ClientTCPHandler.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2019-2021 Oak Ridge National Laboratory.
+ * Copyright (c) 2019-2022 Oak Ridge National Laboratory.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -296,6 +296,12 @@ protected void onReceiverExited(final boolean running)
     @Override
     protected void handleControlMessage(final byte command, final ByteBuffer buffer) throws Exception
     {
+        // 0 byte magic
+        // 1 byte version
+        // 2 byte flags
+        // 3 byte command
+        // 4 int32 payload size
+        // 8 .. payload
         if (command == PVAHeader.CTRL_SET_BYTE_ORDER)
         {
             // First message received from server, remember its version
@@ -308,12 +314,23 @@ protected void handleControlMessage(final byte command, final ByteBuffer buffer)
             // configure it
             send_buffer.order(buffer.order());
 
-            logger.log(Level.FINE, () -> "Server Version " + server_version + " sets byte order to " + send_buffer.order());
-            // Payload indicates if the server will send messages in that same order,
+            if (connection_validated.get())
+                logger.log(Level.WARNING, () -> "Server Version " + server_version + " sets byte order to " + send_buffer.order() +
+                           " after connection has already been validated");
+            else
+                logger.log(Level.FINE, () -> "Server Version " + server_version + " sets byte order to " + send_buffer.order());
+            // Payload 'size' indicates if the server will send messages in that same order,
             // or might change order for each message.
             // We always adapt based on the flags of each received message,
             // so ignore.
             // send_buffer byte order is locked at this time, though.
+            final int hint = buffer.getInt(4);
+            if (hint == 0x00000000)
+                logger.log(Level.FINE, () -> "Server hints that it will send all messages in byte order " + send_buffer.order());
+            else if (hint == 0xFFFFFFFF)
+                logger.log(Level.FINE, () -> "Server hints that client needs to check each received messages for changing byte order");
+            else
+                logger.log(Level.WARNING, () -> String.format("Server sent SET_BYTE_ORDER hint 0x%08X, expecting 0x00000000 or 0xFFFFFFFF", hint));
         }
         else
             super.handleControlMessage(command, buffer);
diff --git a/core/pva/src/main/java/org/epics/pva/common/PVAHeader.java b/core/pva/src/main/java/org/epics/pva/common/PVAHeader.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2019-2021 Oak Ridge National Laboratory.
+ * Copyright (c) 2019-2022 Oak Ridge National Laboratory.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -124,7 +124,7 @@ public class PVAHeader
     public static final byte CMD_SUB_GET = 0x40;
 
 
-    /** Control message command to set byte order */
+    /** Control message command to set byte order, aka 'SetEndian' */
     public static final byte CTRL_SET_BYTE_ORDER = 2;
 
     /** Size of common PVA message header */
@@ -145,6 +145,7 @@ public class PVAHeader
      */
     public static void encodeMessageHeader(final ByteBuffer buffer, byte flags, final byte command, final int payload_size)
     {
+        // Indicate byte order within message header
         if (buffer.order() == ByteOrder.BIG_ENDIAN)
             flags |= FLAG_BIG_ENDIAN;
         else
@@ -168,6 +169,8 @@ public static int checkMessageAndGetSize(final ByteBuffer buffer, final boolean
         if (buffer.position() < PVAHeader.HEADER_SIZE)
             return PVAHeader.HEADER_SIZE;
 
+        // Byte order of message is irrelevant for
+        // parsing the initial set of bytes
         final byte magic = buffer.get(0);
         if (magic != PVAHeader.PVA_MAGIC)
             throw new Exception(String.format("Message lacks magic 0x%02X, got 0x%02X", PVAHeader.PVA_MAGIC, magic));
@@ -184,6 +187,9 @@ public static int checkMessageAndGetSize(final ByteBuffer buffer, final boolean
         if (is_server != expect_server)
                 throw new Exception(expect_server ? "Expected server message" : "Expected client message");
 
+        // With each received message, check the byte order
+        // and adjust buffer to read further content which usually
+        // contains data larger than byte-sized, so the order matters
         if ((flags & PVAHeader.FLAG_BIG_ENDIAN) == 0)
             buffer.order(ByteOrder.LITTLE_ENDIAN);
         else
diff --git a/core/pva/src/main/java/org/epics/pva/common/TCPHandler.java b/core/pva/src/main/java/org/epics/pva/common/TCPHandler.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2019-2021 Oak Ridge National Laboratory.
+ * Copyright (c) 2019-2022 Oak Ridge National Laboratory.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -12,6 +12,7 @@
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
 import java.nio.channels.SocketChannel;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ExecutorService;
@@ -117,6 +118,12 @@ public TCPHandler(final SocketChannel socket, final boolean client_mode)
         this.socket = socket;
         this.client_mode = client_mode;
 
+        // Receive buffer byte order is set based on header flag of each received message.
+        // Send buffer of server and client starts out with native byte order.
+        // For server, it stays that way.
+        // For client, order is updated during connection validation (PVAHeader.CTRL_SET_BYTE_ORDER)
+        send_buffer.order(ByteOrder.nativeOrder());
+
         // Start receiving data
         receive_thread = thread_pool.submit(this::receiver);
     }
@@ -376,7 +383,11 @@ private void handleMessage(final ByteBuffer buffer) throws Exception
             final boolean control = (flags & PVAHeader.FLAG_CONTROL) != 0;
             final byte command = buffer.get(3);
             // Move to start of potential payload
-            buffer.position(8);
+            if (buffer.limit() >= 8)
+                buffer.position(8);
+            else
+                logger.log(Level.SEVERE, Thread.currentThread().getName() + " received buffer with only " +
+                           buffer.limit() + " bytes:" + Hexdump.toHex(buffer));
             if (control)
                 handleControlMessage(command, buffer);
             else
diff --git a/core/pva/src/main/java/org/epics/pva/common/UDPHandler.java b/core/pva/src/main/java/org/epics/pva/common/UDPHandler.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2019 Oak Ridge National Laboratory.
+ * Copyright (c) 2019-2022 Oak Ridge National Laboratory.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -75,6 +75,9 @@ private void handleMessages(final InetSocketAddress from, final ByteBuffer buffe
 
             final byte version = buffer.get();
 
+            // After initial byte-sized header entries,
+            // adjust buffer byte order as indicated in message header
+            // to allow decoding of following data (size, payload)
             final byte flags = buffer.get();
             if ((flags & PVAHeader.FLAG_BIG_ENDIAN) != 0)
                 buffer.order(ByteOrder.BIG_ENDIAN);
diff --git a/core/pva/src/main/java/org/epics/pva/server/ServerTCPHandler.java b/core/pva/src/main/java/org/epics/pva/server/ServerTCPHandler.java
@@ -64,9 +64,15 @@ public ServerTCPHandler(final PVAServer server, final SocketChannel client) thro
         submit((version, buffer) ->
         {
             logger.log(Level.FINE, () -> "Set byte order " + buffer.order());
+            // Payload size is used as byte order hint
+            // 0xFFFFFFFF: Client needs to check each message for byte order
+            // 0x00000000: Server sends each message in the same byte order,
+            //             but there's still a valid byte order flag,
+            //             so client is free to keep checking each message
+            final int size_used_as_hint = 0x00000000;
             PVAHeader.encodeMessageHeader(buffer,
                     (byte) (PVAHeader.FLAG_CONTROL | PVAHeader.FLAG_SERVER),
-                    PVAHeader.CTRL_SET_BYTE_ORDER, 0);
+                    PVAHeader.CTRL_SET_BYTE_ORDER, size_used_as_hint);
         });
         // .. and requesting connection validation
         submit((version, buffer) ->
