diff --git a/components/esp_ringbuf/ringbuf.c b/components/esp_ringbuf/ringbuf.c index d69d96fee0..f205f9afad 100644 --- a/components/esp_ringbuf/ringbuf.c +++ b/components/esp_ringbuf/ringbuf.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -629,8 +629,21 @@ static void prvReturnItemDefault(Ringbuffer_t *pxRingbuffer, uint8_t *pucItem) * freed or items with dummy data should be skipped over */ pxCurHeader = (ItemHeader_t *)pxRingbuffer->pucFree; + + // allow a single advancement either when buffer is FULL (to release the oldest block) + // or when buffer is EMPTY (to skip a FREE block sitting at the head) + BaseType_t allow_advance_free_pointer = + ((pxRingbuffer->uxRingbufferFlags & rbBUFFER_FULL_FLAG) ? pdTRUE : pdFALSE) || + ((pxRingbuffer->xItemsWaiting == 0) ? pdTRUE : pdFALSE); + BaseType_t freed_any = pdFALSE; + //Skip over Items that have already been freed or are dummy items - while (((pxCurHeader->uxItemFlags & rbITEM_FREE_FLAG) || (pxCurHeader->uxItemFlags & rbITEM_DUMMY_DATA_FLAG)) && pxRingbuffer->pucFree != pxRingbuffer->pucRead) { + while (((pxCurHeader->uxItemFlags & (rbITEM_FREE_FLAG | rbITEM_DUMMY_DATA_FLAG)) != 0) && + ((pxRingbuffer->pucFree != pxRingbuffer->pucRead) || allow_advance_free_pointer)) { + + allow_advance_free_pointer = pdFALSE; + freed_any = pdTRUE; + if (pxCurHeader->uxItemFlags & rbITEM_DUMMY_DATA_FLAG) { pxCurHeader->uxItemFlags |= rbITEM_FREE_FLAG; //Mark as freed (not strictly necessary but adds redundancy) pxRingbuffer->pucFree = pxRingbuffer->pucHead; //Wrap around due to dummy data @@ -648,14 +661,9 @@ static void prvReturnItemDefault(Ringbuffer_t *pxRingbuffer, uint8_t *pucItem) pxCurHeader = (ItemHeader_t *)pxRingbuffer->pucFree; //Update header to point to item } - //Check if the buffer full flag should be reset - if (pxRingbuffer->uxRingbufferFlags & rbBUFFER_FULL_FLAG) { - if (pxRingbuffer->pucFree != pxRingbuffer->pucAcquire) { - pxRingbuffer->uxRingbufferFlags &= ~rbBUFFER_FULL_FLAG; - } else if (pxRingbuffer->pucFree == pxRingbuffer->pucAcquire && pxRingbuffer->pucFree == pxRingbuffer->pucRead) { - //Special case where a full buffer is completely freed in one go - pxRingbuffer->uxRingbufferFlags &= ~rbBUFFER_FULL_FLAG; - } + // clear FULL only if we actually freed something + if ((pxRingbuffer->uxRingbufferFlags & rbBUFFER_FULL_FLAG) && freed_any) { + pxRingbuffer->uxRingbufferFlags &= ~rbBUFFER_FULL_FLAG; } } @@ -1356,12 +1364,80 @@ void xRingbufferPrintInfo(RingbufHandle_t xRingbuffer) { Ringbuffer_t *pxRingbuffer = (Ringbuffer_t *)xRingbuffer; configASSERT(pxRingbuffer); - printf("Rb size:%" PRId32 "\tfree: %" PRId32 "\trptr: %" PRId32 "\tfreeptr: %" PRId32 "\twptr: %" PRId32 ", aptr: %" PRId32 "\n", - (int32_t)pxRingbuffer->xSize, (int32_t)prvGetFreeSize(pxRingbuffer), + + uint32_t RingbufferFlags = pxRingbuffer->uxRingbufferFlags; + printf("RingBuffer Size: %" PRId32 ", FreeSize: %" PRId32 "\n" + " Read: %" PRId32 ", Free: %" PRId32 ", Write: %" PRId32 ", Acquire: %" PRId32 ", Waiting: %" PRId32 ", Flags: 0x%" PRIx32 " [", + (int32_t)pxRingbuffer->xSize, + (int32_t)prvGetFreeSize(pxRingbuffer), (int32_t)(pxRingbuffer->pucRead - pxRingbuffer->pucHead), (int32_t)(pxRingbuffer->pucFree - pxRingbuffer->pucHead), (int32_t)(pxRingbuffer->pucWrite - pxRingbuffer->pucHead), - (int32_t)(pxRingbuffer->pucAcquire - pxRingbuffer->pucHead)); + (int32_t)(pxRingbuffer->pucAcquire - pxRingbuffer->pucHead), + (int32_t)pxRingbuffer->xItemsWaiting, + RingbufferFlags); + + if (RingbufferFlags) { + if (RingbufferFlags & rbALLOW_SPLIT_FLAG) { + printf(" [ALLOW_SPLIT]"); + } + if (RingbufferFlags & rbBYTE_BUFFER_FLAG) { + printf(" [BYTE_BUFFER]"); + } + if (RingbufferFlags & rbBUFFER_FULL_FLAG) { + printf(" [FULL]"); + } + if (RingbufferFlags & rbBUFFER_STATIC_FLAG) { + printf(" [STATIC]"); + } + if (RingbufferFlags & rbUSING_QUEUE_SET) { + printf(" [USING_QUEUE_SET]"); + } + } + printf(" ]\n Items:\n"); + + uint8_t *Head = pxRingbuffer->pucHead; + uint8_t *ptr = Head; + size_t max_steps = (pxRingbuffer->xSize / rbHEADER_SIZE) + 4; + for (size_t steps = 0; ptr < pxRingbuffer->pucTail && steps < max_steps; ++steps) { + unsigned offset = ptr - Head; + ItemHeader_t *hdr = (ItemHeader_t *)ptr; + unsigned ItemFlags = hdr->uxItemFlags; + unsigned ItemLen = hdr->xItemLen; + printf(" [%4u] Size: %u Flags: 0x%x [", offset, ItemLen, ItemFlags); + if (ItemLen > pxRingbuffer->xMaxItemSize && !(ItemFlags & rbITEM_DUMMY_DATA_FLAG)) { + printf(" [INVALID HEADER or UNUSED SPACE]\n"); + break; + } + if (ItemFlags) { + if (ItemFlags & rbITEM_FREE_FLAG) { + printf(" [FREE]"); + } + if (ItemFlags & rbITEM_DUMMY_DATA_FLAG) { + printf(" [DUMMY]"); + } + if (ItemFlags & rbITEM_SPLIT_FLAG) { + printf(" [SPLIT]"); + } + if (ItemFlags & rbITEM_WRITTEN_FLAG) { + printf(" [WRITTEN]"); + } + } + printf(" ]\n"); + + if (ItemFlags & rbITEM_DUMMY_DATA_FLAG) { + ptr = Head; + } else { + size_t step = rbHEADER_SIZE + rbALIGN_SIZE(ItemLen); + if (step == 0) { + break; + } + ptr += step; + } + if (ptr == Head) { + break; // full circle + } + } } BaseType_t xRingbufferGetStaticBuffer(RingbufHandle_t xRingbuffer, uint8_t **ppucRingbufferStorage, StaticRingbuffer_t **ppxStaticRingbuffer) diff --git a/components/esp_ringbuf/test_apps/main/test_ringbuf_common.c b/components/esp_ringbuf/test_apps/main/test_ringbuf_common.c index bff4d73f00..028671dbe0 100644 --- a/components/esp_ringbuf/test_apps/main/test_ringbuf_common.c +++ b/components/esp_ringbuf/test_apps/main/test_ringbuf_common.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -12,6 +12,7 @@ #include "sdkconfig.h" #include #include +#include #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "freertos/queue.h" @@ -1096,12 +1097,12 @@ TEST_CASE("Test no-split buffers can receive items if the acquire pointer wraps } // At this point, the buffer should be full and the acquire pointer must have wrapped around - UBaseType_t *uxFree = NULL; - UBaseType_t *uxRead = NULL; - UBaseType_t *uxWrite = NULL; - UBaseType_t *uxAcquire = NULL; - vRingbufferGetInfo(buffer_handle, uxFree, uxRead, uxWrite, uxAcquire, &items_waiting); - TEST_ASSERT_EQUAL(uxAcquire, uxWrite); + xRingbufferPrintInfo(buffer_handle); + UBaseType_t uFree, uRead, uWrite, uAcquire; + vRingbufferGetInfo(buffer_handle, &uFree, &uRead, &uWrite, &uAcquire, &items_waiting); + TEST_ASSERT_EQUAL_UINT32(uAcquire, uWrite); + TEST_ASSERT_EQUAL_UINT32(uAcquire, uRead); + TEST_ASSERT_EQUAL_UINT32(uAcquire, uFree); TEST_ASSERT_EQUAL(0U, xRingbufferGetCurFreeSize(buffer_handle)); // Send the items out-of-order to the buffer. Verify that the items are not received until the first item is sent. @@ -1124,3 +1125,196 @@ TEST_CASE("Test no-split buffers can receive items if the acquire pointer wraps // Cleanup vRingbufferDelete(buffer_handle); } + +static void acquire_all_items(RingbufHandle_t buffer_handle, void **items, int capacity) +{ + // Acquire all items with SendAcquire + for (int i = 0; i < capacity; i++) { + printf("Acquire %d item\n", i); + TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendAcquire(buffer_handle, &items[i], MEDIUM_ITEM_SIZE, 0)); + TEST_ASSERT_NOT_NULL(items[i]); + } +} + +static void write_and_send_all_items(RingbufHandle_t buffer_handle, void **items, int capacity, uint8_t data[]) +{ + // Write to all acquired items and send them with SendComplete + for (int i = 0; i < capacity; i++) { + printf("Write and Send %d item\n", i); + memset(items[i], data[i], MEDIUM_ITEM_SIZE); + TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendComplete(buffer_handle, items[i])); + } +} + +static void receive_all_items(RingbufHandle_t buffer_handle, void **rx_items, int capacity) +{ + // Receive all items + for (int i = 0; i < capacity; i++) { + printf("Receive %d item\n", i); + size_t item_size; + rx_items[i] = xRingbufferReceive(buffer_handle, &item_size, 0); + TEST_ASSERT_NOT_NULL(rx_items[i]); + TEST_ASSERT_EQUAL(MEDIUM_ITEM_SIZE, item_size); + } +} + +static void read_and_return_item(RingbufHandle_t buffer_handle, void **rx_items, int index, uint8_t data) +{ + TEST_ASSERT_NOT_NULL(rx_items[index]); + TEST_ASSERT_EQUAL(((uint8_t *)rx_items[index])[0], data); + vRingbufferReturnItem(buffer_handle, rx_items[index]); +} + +static void read_received_all_items(RingbufHandle_t buffer_handle, void **rx_items, int capacity, uint8_t data[]) +{ + for (int i = 0; i < capacity; i++) { + printf("Read and return %d item\n", i); + read_and_return_item(buffer_handle, rx_items, i, data[i]); + } +} + +TEST_CASE("Test no-split buffers returning oldest item when full frees one slot", "[esp_ringbuf][linux]") +{ + // Create buffer + RingbufHandle_t buffer_handle = xRingbufferCreate(BUFFER_SIZE, RINGBUF_TYPE_NOSPLIT); + TEST_ASSERT_MESSAGE(buffer_handle != NULL, "Failed to create ring buffer"); + const int capacity = BUFFER_SIZE / (ITEM_HDR_SIZE + MEDIUM_ITEM_SIZE); + TEST_ASSERT_EQUAL(4, capacity); + void *rx_items[capacity]; + uint8_t data[] = {10, 11, 12, 13}; + + acquire_all_items(buffer_handle, rx_items, capacity); + write_and_send_all_items(buffer_handle, rx_items, capacity, data); + receive_all_items(buffer_handle, rx_items, capacity); + + const int free_idx = 0; + printf("-- Read and returning only %d item\n", free_idx); + TEST_ASSERT_EQUAL(((uint8_t *)rx_items[free_idx])[0], data[free_idx]); + vRingbufferReturnItem(buffer_handle, rx_items[free_idx]); + xRingbufferPrintInfo(buffer_handle); + + printf("-- Verify that only the first item is returned.\n"); + TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendAcquire(buffer_handle, &rx_items[free_idx], MEDIUM_ITEM_SIZE, 0)); + xRingbufferPrintInfo(buffer_handle); + data[free_idx] = 20; + memset(rx_items[free_idx], data[free_idx], MEDIUM_ITEM_SIZE); + TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendComplete(buffer_handle, rx_items[free_idx])); + + printf("-- Attempting to acquire a second item should fail, as expected.\n"); + TEST_ASSERT_EQUAL(pdFALSE, xRingbufferSendAcquire(buffer_handle, &rx_items[free_idx], MEDIUM_ITEM_SIZE, 0)); + + printf("-- Receive the first item second time, making it available for reading\n"); + size_t item_size; + rx_items[free_idx] = xRingbufferReceive(buffer_handle, &item_size, 0); + TEST_ASSERT_NOT_NULL(rx_items[free_idx]); + TEST_ASSERT_EQUAL(((uint8_t *)rx_items[free_idx])[0], data[free_idx]); + + printf("-- Read and return all items\n"); + read_received_all_items(buffer_handle, rx_items, capacity, data); + xRingbufferPrintInfo(buffer_handle); + + printf("-- Repeat the process: Acquire -> Write -> Send -> Receive -> Read -> Return\n"); + for (int i = 0; i < capacity; i++) { + data[i] = 100 + i; + } + acquire_all_items(buffer_handle, rx_items, capacity); + write_and_send_all_items(buffer_handle, rx_items, capacity, data); + receive_all_items(buffer_handle, rx_items, capacity); + read_received_all_items(buffer_handle, rx_items, capacity, data); + + // Cleanup + vRingbufferDelete(buffer_handle); +} + +TEST_CASE("Test no-split buffers returning non-oldest item when full does not free slot", "[esp_ringbuf][linux]") +{ + // Create buffer + RingbufHandle_t buffer_handle = xRingbufferCreate(BUFFER_SIZE, RINGBUF_TYPE_NOSPLIT); + TEST_ASSERT_MESSAGE(buffer_handle != NULL, "Failed to create ring buffer"); + const int capacity = BUFFER_SIZE / (ITEM_HDR_SIZE + MEDIUM_ITEM_SIZE); + TEST_ASSERT_EQUAL(4, capacity); + void *rx_items[capacity]; + uint8_t data[] = {10, 11, 12, 13}; + + acquire_all_items(buffer_handle, rx_items, capacity); + write_and_send_all_items(buffer_handle, rx_items, capacity, data); + receive_all_items(buffer_handle, rx_items, capacity); + + const int free_idx = 1; + printf("-- Read and returning only %d item\n", free_idx); + TEST_ASSERT_EQUAL(((uint8_t *)rx_items[free_idx])[0], data[free_idx]); + vRingbufferReturnItem(buffer_handle, rx_items[free_idx]); + + printf("-- Verify that only the second item is returned. Can not acquire the second item\n"); + TEST_ASSERT_EQUAL(pdFALSE, xRingbufferSendAcquire(buffer_handle, &rx_items[free_idx], MEDIUM_ITEM_SIZE, 0)); + + xRingbufferPrintInfo(buffer_handle); + printf("-- Read and return the rest of the items\n"); + for (int i = 0; i < capacity; i++) { + if (i != free_idx) { + read_and_return_item(buffer_handle, rx_items, i, data[i]); + } + } + xRingbufferPrintInfo(buffer_handle); + + printf("-- Repeat the process: Acquire -> Write -> Send -> Receive -> Read -> Return\n"); + for (int i = 0; i < capacity; i++) { + data[i] = 100 + i; + } + acquire_all_items(buffer_handle, rx_items, capacity); + write_and_send_all_items(buffer_handle, rx_items, capacity, data); + receive_all_items(buffer_handle, rx_items, capacity); + read_received_all_items(buffer_handle, rx_items, capacity, data); + + // Cleanup + vRingbufferDelete(buffer_handle); +} + +TEST_CASE("Test no-split full buffer becomes empty when oldest is returned last", "[esp_ringbuf][linux]") +{ + RingbufHandle_t buffer_handle = xRingbufferCreate(BUFFER_SIZE, RINGBUF_TYPE_NOSPLIT); + TEST_ASSERT_MESSAGE(buffer_handle != NULL, "Failed to create ring buffer"); + const int capacity = BUFFER_SIZE / (ITEM_HDR_SIZE + MEDIUM_ITEM_SIZE); + TEST_ASSERT_EQUAL(4, capacity); + uint8_t data[] = {10, 11, 12, 13}; + + // 1) Acquire, write, complete: buffer becomes FULL + void *rx_items[capacity]; + acquire_all_items(buffer_handle, rx_items, capacity); + write_and_send_all_items(buffer_handle, rx_items, capacity, data); + TEST_ASSERT_EQUAL(0U, xRingbufferGetCurFreeSize(buffer_handle)); // FULL + + // 2) Receive all items but DON'T return yet (hold them) + receive_all_items(buffer_handle, rx_items, capacity); + UBaseType_t items_waiting_after_recv; + vRingbufferGetInfo(buffer_handle, NULL, NULL, NULL, NULL, &items_waiting_after_recv); + TEST_ASSERT_EQUAL(0, items_waiting_after_recv); + TEST_ASSERT_EQUAL(0U, xRingbufferGetCurFreeSize(buffer_handle)); // still FULL + + // 3) Return non-oldest first — still no space (FULL must remain) + for (int i = 1; i < capacity; ++i) { + read_and_return_item(buffer_handle, rx_items, i, data[i]); + } + void *tmp; + TEST_ASSERT_EQUAL(pdFALSE, xRingbufferSendAcquire(buffer_handle, &tmp, MEDIUM_ITEM_SIZE, 0)); + TEST_ASSERT_NULL(tmp); + + // 4) Return the oldest last — now FULL must clear, buffer becomes truly empty + xRingbufferPrintInfo(buffer_handle); + read_and_return_item(buffer_handle, rx_items, 0, data[0]); + xRingbufferPrintInfo(buffer_handle); + + // Verify: pointers collapse, not FULL anymore, new acquires succeed + UBaseType_t uFree, uRead, uWrite, uAcquire, waiting; + vRingbufferGetInfo(buffer_handle, &uFree, &uRead, &uWrite, &uAcquire, &waiting); + TEST_ASSERT_EQUAL_UINT32(waiting, 0); + TEST_ASSERT_EQUAL_UINT32(uFree, uRead); + TEST_ASSERT_EQUAL_UINT32(uFree, uAcquire); + + TEST_ASSERT_GREATER_THAN_UINT32(0, xRingbufferGetCurFreeSize(buffer_handle)); // FULL cleared + + void *slot; + TEST_ASSERT_EQUAL(pdTRUE, xRingbufferSendAcquire(buffer_handle, &slot, MEDIUM_ITEM_SIZE, 0)); + TEST_ASSERT_NOT_NULL(slot); + vRingbufferDelete(buffer_handle); +}