From 40af90ad25b23b91e4f97a324375e5da2fd3de0a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 5 Oct 2026 19:24:13 -0500 Subject: [PATCH] [logger] Use the static ring buffer structure as its handle on ESP32 (#20129) --- esphome/components/logger/__init__.py | 10 +++++++ .../logger/task_log_buffer_esp32.cpp | 20 +++++--------- .../components/logger/task_log_buffer_esp32.h | 8 ++++-- tests/component_tests/logger/test_logger.py | 27 +++++++++++++++++++ 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/esphome/components/logger/__init__.py b/esphome/components/logger/__init__.py index d6b9bca38b..a966ba67ad 100644 --- a/esphome/components/logger/__init__.py +++ b/esphome/components/logger/__init__.py @@ -235,6 +235,15 @@ def warn_ram_log_strings(config: ConfigType) -> ConfigType: return config +def validate_task_log_buffer_alignment(value: int) -> int: + # ESP-IDF rejects a no-split ring buffer whose size is not a multiple of 4 + if CORE.is_esp32 and value % 4: + raise cv.Invalid( + f"{CONF_TASK_LOG_BUFFER_SIZE} must be a multiple of 4 on ESP32" + ) + return value + + def validate_wait_for_cdc(config: ConfigType) -> ConfigType: if config.get(CONF_WAIT_FOR_CDC) and config.get(CONF_HARDWARE_UART) != USB_CDC: raise cv.Invalid("wait_for_cdc requires hardware_uart: USB_CDC") @@ -282,6 +291,7 @@ CONFIG_SCHEMA = cv.All( max=32768, # Max: Depends on message sizes, typically ~300 messages with default size ), ), + validate_task_log_buffer_alignment, ), cv.SplitDefault( CONF_HARDWARE_UART, diff --git a/esphome/components/logger/task_log_buffer_esp32.cpp b/esphome/components/logger/task_log_buffer_esp32.cpp index cb97f5504f..75b567ff89 100644 --- a/esphome/components/logger/task_log_buffer_esp32.cpp +++ b/esphome/components/logger/task_log_buffer_esp32.cpp @@ -10,15 +10,7 @@ namespace esphome::logger { TaskLogBuffer::TaskLogBuffer() { // Create a static ring buffer with RINGBUF_TYPE_NOSPLIT for message integrity // Storage is a member array (embedded in Logger), no heap allocation needed - this->ring_buffer_ = - xRingbufferCreateStatic(sizeof(this->storage_), RINGBUF_TYPE_NOSPLIT, this->storage_, &this->structure_); -} - -TaskLogBuffer::~TaskLogBuffer() { - if (this->ring_buffer_ != nullptr) { - vRingbufferDelete(this->ring_buffer_); - this->ring_buffer_ = nullptr; - } + xRingbufferCreateStatic(sizeof(this->storage_), RINGBUF_TYPE_NOSPLIT, this->storage_, &this->structure_); } bool TaskLogBuffer::borrow_message_main_loop(LogMessage *&message, uint16_t &text_length) { @@ -27,7 +19,7 @@ bool TaskLogBuffer::borrow_message_main_loop(LogMessage *&message, uint16_t &tex } size_t item_size = 0; - void *received_item = xRingbufferReceive(ring_buffer_, &item_size, 0); + void *received_item = xRingbufferReceive(this->handle_(), &item_size, 0); if (received_item == nullptr) { return false; } @@ -44,7 +36,7 @@ void TaskLogBuffer::release_message_main_loop() { if (this->current_token_ == nullptr) { return; } - vRingbufferReturnItem(ring_buffer_, this->current_token_); + vRingbufferReturnItem(this->handle_(), this->current_token_); this->current_token_ = nullptr; // Update counter to mark all messages as processed last_processed_counter_ = message_counter_.load(std::memory_order_relaxed); @@ -71,7 +63,7 @@ bool TaskLogBuffer::send_message_thread_safe(uint8_t level, const char *tag, uin // Acquire memory directly from the ring buffer void *acquired_memory = nullptr; - BaseType_t result = xRingbufferSendAcquire(ring_buffer_, &acquired_memory, total_size, 0); + BaseType_t result = xRingbufferSendAcquire(this->handle_(), &acquired_memory, total_size, 0); if (result != pdTRUE || acquired_memory == nullptr) { return false; // Failed to acquire memory @@ -100,7 +92,7 @@ bool TaskLogBuffer::send_message_thread_safe(uint8_t level, const char *tag, uin // Handle unexpected formatting error if (ret <= 0) { - vRingbufferReturnItem(ring_buffer_, acquired_memory); + vRingbufferReturnItem(this->handle_(), acquired_memory); return false; } @@ -111,7 +103,7 @@ bool TaskLogBuffer::send_message_thread_safe(uint8_t level, const char *tag, uin msg->text_length = text_length; // Complete the send operation with the acquired memory - result = xRingbufferSendComplete(ring_buffer_, acquired_memory); + result = xRingbufferSendComplete(this->handle_(), acquired_memory); if (result != pdTRUE) { return false; // Failed to complete the message send diff --git a/esphome/components/logger/task_log_buffer_esp32.h b/esphome/components/logger/task_log_buffer_esp32.h index e819766795..0ffaa04493 100644 --- a/esphome/components/logger/task_log_buffer_esp32.h +++ b/esphome/components/logger/task_log_buffer_esp32.h @@ -47,7 +47,7 @@ class TaskLogBuffer { }; TaskLogBuffer(); - ~TaskLogBuffer(); + // No destructor: Logger is never destroyed // NOT thread-safe - borrow a message from the ring buffer, only call from main loop bool borrow_message_main_loop(LogMessage *&message, uint16_t &text_length); @@ -68,7 +68,11 @@ class TaskLogBuffer { static constexpr size_t size() { return ESPHOME_TASK_LOG_BUFFER_SIZE; } private: - RingbufHandle_t ring_buffer_{nullptr}; // FreeRTOS ring buffer handle + // xRingbufferCreateStatic() returns the static structure itself as the handle; it only + // returns NULL for a no-split size that is unaligned or under two item headers + static_assert(ESPHOME_TASK_LOG_BUFFER_SIZE % 4 == 0, "task_log_buffer_size must be a multiple of 4"); + RingbufHandle_t handle_() { return &this->structure_; } + StaticRingbuffer_t structure_; // Static structure for the ring buffer uint8_t storage_[ESPHOME_TASK_LOG_BUFFER_SIZE]; // Embedded in Logger (no separate heap allocation) diff --git a/tests/component_tests/logger/test_logger.py b/tests/component_tests/logger/test_logger.py index 199d67ff5c..20b99f718f 100644 --- a/tests/component_tests/logger/test_logger.py +++ b/tests/component_tests/logger/test_logger.py @@ -6,7 +6,11 @@ import re import pytest +from esphome.components.logger import validate_task_log_buffer_alignment +from esphome.config_validation import Invalid +from esphome.const import PlatformFramework from esphome.core import CORE +from tests.component_tests.types import SetCoreConfigCallable def test_logger_pre_setup_before_other_components(generate_main): @@ -111,3 +115,26 @@ def test_flash_log_strings_default_does_not_warn( generate_main("tests/component_tests/logger/test_logger.yaml") assert "esp8266_store_log_strings_in_flash" not in caplog.text + + +@pytest.mark.parametrize("value", [0, 640, 768, 32768]) +def test_task_log_buffer_size_aligned_on_esp32( + set_core_config: SetCoreConfigCallable, value: int +) -> None: + set_core_config(PlatformFramework.ESP32_IDF) + assert validate_task_log_buffer_alignment(value) == value + + +def test_task_log_buffer_size_unaligned_rejected_on_esp32( + set_core_config: SetCoreConfigCallable, +) -> None: + set_core_config(PlatformFramework.ESP32_IDF) + with pytest.raises(Invalid, match="multiple of 4"): + validate_task_log_buffer_alignment(641) + + +def test_task_log_buffer_size_unaligned_allowed_on_libretiny( + set_core_config: SetCoreConfigCallable, +) -> None: + set_core_config(PlatformFramework.BK72XX_ARDUINO) + assert validate_task_log_buffer_alignment(641) == 641