Merge branch 'fix/ringbuf_receives_item_not_yet_sent_v5.0' into 'release/v5.0'

fix(esp_ringbuf): Fixed a bug where in a no-split buffer received items prematurely (v5.0)

See merge request espressif/esp-idf!33652

components/esp_ringbuf/ringbuf.c

@@ -510,6 +510,13 @@ static BaseType_t prvCheckItemAvail(Ringbuffer_t *pxRingbuffer)
         return pdFALSE;     //Byte buffers do not allow multiple retrievals before return
     }
     if ((pxRingbuffer->xItemsWaiting > 0) && ((pxRingbuffer->pucRead != pxRingbuffer->pucWrite) || (pxRingbuffer->uxRingbufferFlags & rbBUFFER_FULL_FLAG))) {
+        // If the ring buffer is a no-split buffer, the read pointer must point to an item that has been written to.
+        if ((pxRingbuffer->uxRingbufferFlags & (rbBYTE_BUFFER_FLAG | rbALLOW_SPLIT_FLAG)) == 0) {
+            ItemHeader_t *pxHeader = (ItemHeader_t *)pxRingbuffer->pucRead;
+            if ((pxHeader->uxItemFlags & rbITEM_WRITTEN_FLAG) == 0) {
+                return pdFALSE;
+            }
+        }
         return pdTRUE;      //Items/data available for retrieval
     } else {
         return pdFALSE;     //No items/data available for retrieval
@@ -926,9 +933,6 @@ BaseType_t xRingbufferSendAcquire(RingbufHandle_t xRingbuffer, void **ppvItem, s
     if (xItemSize > pxRingbuffer->xMaxItemSize) {
         return pdFALSE;     //Data will never ever fit in the queue.
     }
-    if ((pxRingbuffer->uxRingbufferFlags & rbBYTE_BUFFER_FLAG) && xItemSize == 0) {
-        return pdTRUE;      //Sending 0 bytes to byte buffer has no effect
-    }
 
     //Attempt to send an item
     BaseType_t xReturn = pdFALSE;

components/esp_ringbuf/test/test_ringbuf.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -804,6 +804,8 @@ TEST_CASE("Test ring buffer ISR", "[esp_ringbuf]")
  * tested.
  */
 
+#if !CONFIG_FREERTOS_UNICORE
+
 #define SRAND_SEED                      3   //Arbitrarily chosen srand() seed
 #define SMP_TEST_ITERATIONS             4
 
@@ -1018,6 +1020,7 @@ TEST_CASE("Test static ring buffer SMP", "[esp_ringbuf]")
     cleanup();
 }
 #endif
+#endif //!CONFIG_FREERTOS_UNICORE
 
 #if !CONFIG_RINGBUF_PLACE_FUNCTIONS_INTO_FLASH && !CONFIG_RINGBUF_PLACE_ISR_FUNCTIONS_INTO_FLASH
 /* -------------------------- Test ring buffer IRAM ------------------------- */
@@ -1027,7 +1030,7 @@ static IRAM_ATTR __attribute__((noinline)) bool iram_ringbuf_test(void)
     bool result = true;
     uint8_t item[4];
     size_t item_size;
-    RingbufHandle_t handle = xRingbufferCreate(CONT_DATA_TEST_BUFF_LEN, RINGBUF_TYPE_NOSPLIT);
+    RingbufHandle_t handle = xRingbufferCreate(BUFFER_SIZE, RINGBUF_TYPE_NOSPLIT);
     result = result && (handle != NULL);
     spi_flash_guard_get()->start(); // Disables flash cache
 
@@ -1046,3 +1049,80 @@ TEST_CASE("Test ringbuffer functions work with flash cache disabled", "[esp_ring
     TEST_ASSERT( iram_ringbuf_test() );
 }
 #endif /* !CONFIG_RINGBUF_PLACE_FUNCTIONS_INTO_FLASH && !CONFIG_RINGBUF_PLACE_ISR_FUNCTIONS_INTO_FLASH */
+
+/* ---------------------------- Test no-split ring buffer SendAquire and SendComplete ---------------------------
+ * The following test case tests the SendAquire and SendComplete functions of the no-split ring buffer.
+ *
+ * The test case will do the following...
+ * 1) Create a no-split ring buffer.
+ * 2) Acquire space on the buffer to send an item.
+ * 3) Send the item to the buffer.
+ * 4) Verify that the item is received correctly.
+ * 5) Acquire space on the buffer until the buffer is full.
+ * 6) Send the items out-of-order to the buffer.
+ * 7) Verify that the items are not received until the first item is sent.
+ * 8) Send the first item.
+ * 9) Verify that the items are received in the correct order.
+ */
+TEST_CASE("Test no-split buffers always receive items in order", "[esp_ringbuf]")
+{
+    // Create buffer
+    RingbufHandle_t buffer_handle = xRingbufferCreate(BUFFER_SIZE, RINGBUF_TYPE_NOSPLIT);
+    TEST_ASSERT_MESSAGE(buffer_handle != NULL, "Failed to create ring buffer");
+
+    // Acquire space on the buffer to send an item and write to the item
+    void *item1;
+    TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendAcquire(buffer_handle, &item1, MEDIUM_ITEM_SIZE, TIMEOUT_TICKS));
+    *(uint32_t *)item1 = 0x123;
+
+    // Send the item to the buffer
+    TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendComplete(buffer_handle, item1));
+
+    // Verify that the item is received correctly
+    size_t item_size;
+    uint32_t *received_item = xRingbufferReceive(buffer_handle, &item_size, TIMEOUT_TICKS);
+    TEST_ASSERT_NOT_NULL(received_item);
+    TEST_ASSERT_EQUAL(item_size, MEDIUM_ITEM_SIZE);
+    TEST_ASSERT_EQUAL(*(uint32_t *)received_item, 0x123);
+
+    // Return the space to the buffer after receiving the item
+    vRingbufferReturnItem(buffer_handle, received_item);
+
+    // At this point, the buffer should be empty
+    UBaseType_t items_waiting;
+    vRingbufferGetInfo(buffer_handle, NULL, NULL, NULL, NULL, &items_waiting);
+    TEST_ASSERT_MESSAGE(items_waiting == 0, "Incorrect items waiting");
+
+    // Acquire space on the buffer until the buffer is full
+#define MAX_NUM_ITEMS ( BUFFER_SIZE / ( MEDIUM_ITEM_SIZE + ITEM_HDR_SIZE ) )
+    void *items[MAX_NUM_ITEMS];
+    for (int i = 0; i < MAX_NUM_ITEMS; i++) {
+        TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendAcquire(buffer_handle, &items[i], MEDIUM_ITEM_SIZE, TIMEOUT_TICKS));
+        TEST_ASSERT_NOT_NULL(items[i]);
+        *(uint32_t *)items[i] = (0x100 + i);
+    }
+
+    // Verify that the buffer is full by attempting to acquire space for another item
+    void *another_item;
+    TEST_ASSERT_EQUAL(pdFALSE, xRingbufferSendAcquire(buffer_handle, &another_item, MEDIUM_ITEM_SIZE, TIMEOUT_TICKS));
+
+    // Send the items out-of-order to the buffer. Verify that the items are not received until the first item is sent.
+    // In this case, we send the items in the reverse order until the first item is sent.
+    for (int i = MAX_NUM_ITEMS - 1; i > 0; i--) {
+        TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendComplete(buffer_handle, items[i]));
+        TEST_ASSERT_NULL(xRingbufferReceive(buffer_handle, &item_size, 0));
+    }
+
+    // Send the first item
+    TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendComplete(buffer_handle, items[0]));
+
+    // Verify that the items are received in the correct order
+    for (int i = 0; i < MAX_NUM_ITEMS; i++) {
+        received_item = xRingbufferReceive(buffer_handle, &item_size, TIMEOUT_TICKS);
+        TEST_ASSERT_EQUAL(*(uint32_t *)received_item, (0x100 + i));
+        vRingbufferReturnItem(buffer_handle, received_item);
+    }
+
+    // Cleanup
+    vRingbufferDelete(buffer_handle);
+}

components/freertos/test/integration/tasks/test_vTaskSuspendAll_xTaskResumeAll.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -487,6 +487,9 @@ static void test_pended_running_task(void *arg)
         TEST_ASSERT_EQUAL(true, has_run[i]);
     }
 
+    // Short delay to let the tasks be suspended
+    vTaskDelay(10);
+
     // Clean up the interrupt and tasks
     deregister_intr_cb();
     for (int i = 0; i < TEST_PENDED_NUM_BLOCKED_TASKS; i++) {
@@ -506,6 +509,8 @@ TEST_CASE("Test xTaskResumeAll resumes pended tasks", "[freertos]")
         TEST_ASSERT_EQUAL(pdTRUE, xTaskCreatePinnedToCore(test_pended_running_task, "susp", 2048, (void *)xTaskGetCurrentTaskHandle(), UNITY_FREERTOS_PRIORITY + 1, &susp_tsk_hdl, i));
         // Wait for to be notified to test completion
         ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
+        // Short delay to let the task be suspended
+        vTaskDelay(10);
         vTaskDelete(susp_tsk_hdl);
     }
     // Add a short delay to allow the idle task to free any remaining task memory