Revisit data buffer handling

Fix issue with change to wait_for_buffer_ready() which could end up looping
while waiting for experiment data.  Also simplify the block handling in
capture_experiment().

driver/panda_device.h

@@ -31,4 +31,7 @@ struct panda_block {
 #define PANDA_COMPLETION_DMA        4
 #define PANDA_COMPLETION_OVERRUN    8
 
-#define PANDA_GET_START_TS _IOR('P', 4, struct timespec64)
+/* Returns timestamp associated with the start of capture, created on rising
+ * edge of capture enable.  To ensure a non-zero timestamp this should not be
+ * called until data has been returned from the data stream. */
+#define PANDA_GET_START_TS  _IOR('P', 4, struct timespec64)

server/buffer.c

@@ -25,11 +25,11 @@ struct capture_buffer {
     bool released_once;     // Used to wait for first data
 
     void *buffer;           // Base of captured data buffer
+
     /* Capture and buffer cycle counting are used to manage connections without
      * having to keep track of clients.  If the client and buffer capture_cycle
      * don't agree then the client has been reset, and the buffer_cycle is used
      * to check whether the client's buffer has been overwritten. */
-
     unsigned int capture_cycle; // Counts data capture cycles
     unsigned int buffer_cycle;  // Counts buffer cycles, for overrun detection
 
@@ -303,17 +303,15 @@ static bool wait_for_buffer_ready(
      * with or until the deadline expires. */
     while (true)
     {
-        while (!buffer->released_once && !buffer->shutdown)
-            pwait_deadline(&buffer->mutex, &buffer->signal, &deadline);
-
         if (buffer->shutdown)
             /* Shutdown forced. */
             return false;
-        if (buffer->state != STATE_IDLE  &&
+        else if (buffer->state != STATE_IDLE  &&
             buffer->capture_cycle == reader->capture_cycle)
-            /* New capture cycle ready for us. */
-            return true;
-        if (!pwait_deadline(&buffer->mutex, &buffer->signal, &deadline))
+            /* New capture cycle ready for us, but only return success once
+             * we've started to receive data. */
+            return buffer->released_once;
+        else if (!pwait_deadline(&buffer->mutex, &buffer->signal, &deadline))
             /* Timeout detected. */
             return false;
     }

server/data_server.c

@@ -129,23 +129,25 @@ static void capture_experiment(void)
         do
             count = hw_read_streamed_data(block, DATA_BLOCK_SIZE, &at_eof);
         while (data_thread_running  &&  count == 0  &&  !at_eof);
-        if (count > 0)
+
+        /* Do our best to capture a timestamp before releasing the first block
+         * of the experiment.  If the experiment is empty this return an empty
+         * timestamp, that's how it goes. */
+        if (!ts_captured)
         {
-            if (!ts_captured)
-            {
-                hw_get_start_ts(&pcap_start_ts);
-                ts_captured = true;
-            }
-            release_write_block(data_buffer, count);
+            hw_get_start_ts(&pcap_start_ts);
+            ts_captured = true;
         }
 
+        /* Unconditionally release the write block here.  Either we have data to
+         * send (exited loop above with count==0), or we're at the end of the
+         * experiment, in which case an empty block is harmless. */
+        release_write_block(data_buffer, count);
+
         total_bytes += count;
         experiment_sample_count = total_bytes / sample_length;
     }
 
-    if (!ts_captured)
-        release_write_block(data_buffer, 0);
-
     completion_code = hw_read_streamed_completion();
 
     end_write(data_buffer);

server/hardware.c

@@ -45,15 +45,15 @@ struct register_fields {
         32 - BLOCK_REGISTER_BITS - BLOCK_INSTANCE_BITS - BLOCK_TYPE_BITS;
 };
 
-// This has to be bigger or same size than the linux kernel structure with the
-// same name.
-// We are using this to convert to a stardard timespec in a way that we are
-// compatible with 32-bit and 64-bit architectures, however, we will not
-// need it when we update to a newer glibc (which contains __timespec64)
+/* This has to be bigger or same size than the linux kernel structure with the
+ * same name.
+ * We are using this to convert to a stardard timespec in a way that we are
+ * compatible with 32-bit and 64-bit architectures, however, we will not
+ * need it when we update to a newer glibc (which contains __timespec64) */
 struct timespec64 {
     __time64_t tv_sec;  /* Seconds */
     uint32_t tv_nsec;   /* Nanoseconds */
-    uint32_t :32;       /* padding */
+    uint32_t : 32;      /* padding */
 };