diff --git a/esphome/components/logger/logger.cpp b/esphome/components/logger/logger.cpp index f10efc0070..14862389ca 100644 --- a/esphome/components/logger/logger.cpp +++ b/esphome/components/logger/logger.cpp @@ -177,7 +177,8 @@ void Logger::init_log_buffer(size_t total_buffer_size) { // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) - allocated once, never freed this->log_buffer_ = new logger::TaskLogBuffer(total_buffer_size); -#if defined(USE_ESP32) || defined(USE_LIBRETINY) || (defined(USE_ZEPHYR) && !defined(USE_LOGGER_USB_CDC)) +// Zephyr needs loop working to check when CDC port is open +#if !(defined(USE_ZEPHYR) || defined(USE_LOGGER_USB_CDC)) // Start with loop disabled when using task buffer (unless using USB CDC on ESP32) // The loop will be enabled automatically when messages arrive this->disable_loop_when_buffer_empty_(); @@ -199,26 +200,19 @@ void Logger::process_messages_() { // Process any buffered messages when available if (this->log_buffer_->has_messages()) { logger::TaskLogBuffer::LogMessage *message; - const char *text; -#ifdef USE_HOST - while (this->log_buffer_->get_message_main_loop(&message)) -#elif defined(USE_ESP32) || defined(USE_LIBRETINY) || defined(USE_ZEPHYR) - while (this->log_buffer_->borrow_message_main_loop(&message, &text)) -#endif - { -#ifdef USE_HOST - text = message->text; -#endif + uint16_t text_length; + while (this->log_buffer_->borrow_message_main_loop(message, text_length)) { const char *thread_name = message->thread_name[0] != '\0' ? message->thread_name : nullptr; LogBuffer buf{this->tx_buffer_, this->tx_buffer_size_}; - this->format_buffered_message_and_notify_(message->level, message->tag, message->line, thread_name, text, - message->text_length, buf); + this->format_buffered_message_and_notify_(message->level, message->tag, message->line, thread_name, + message->text_data(), text_length, buf); // Release the message to allow other tasks to use it as soon as possible this->log_buffer_->release_message_main_loop(); this->write_log_buffer_to_console_(buf); } } -#if defined(USE_ESP32) || defined(USE_LIBRETINY) || (defined(USE_ZEPHYR) && !defined(USE_LOGGER_USB_CDC)) +// Zephyr needs loop working to check when CDC port is open +#if !(defined(USE_ZEPHYR) || defined(USE_LOGGER_USB_CDC)) else { // No messages to process, disable loop if appropriate // This reduces overhead when there's no async logging activity diff --git a/esphome/components/logger/logger.h b/esphome/components/logger/logger.h index 5ce6a5cd80..fe3b226e3b 100644 --- a/esphome/components/logger/logger.h +++ b/esphome/components/logger/logger.h @@ -417,7 +417,8 @@ class Logger : public Component { inline RecursionGuard make_non_main_task_guard_() { return RecursionGuard(non_main_task_recursion_guard_); } #endif -#if defined(USE_ESP32) || defined(USE_LIBRETINY) || (defined(USE_ZEPHYR) && !defined(USE_LOGGER_USB_CDC)) +// Zephyr needs loop working to check when CDC port is open +#if defined(USE_ESPHOME_TASK_LOG_BUFFER) && !(defined(USE_ZEPHYR) || defined(USE_LOGGER_USB_CDC)) // Disable loop when task buffer is empty (with USB CDC check on ESP32) inline void disable_loop_when_buffer_empty_() { // Thread safety note: This is safe even if another task calls enable_loop_soon_any_context() diff --git a/esphome/components/logger/task_log_buffer_esp32.cpp b/esphome/components/logger/task_log_buffer_esp32.cpp index d092d5f0e4..921ef16eee 100644 --- a/esphome/components/logger/task_log_buffer_esp32.cpp +++ b/esphome/components/logger/task_log_buffer_esp32.cpp @@ -31,8 +31,8 @@ TaskLogBuffer::~TaskLogBuffer() { } } -bool TaskLogBuffer::borrow_message_main_loop(LogMessage **message, const char **text) { - if (message == nullptr || text == nullptr || this->current_token_) { +bool TaskLogBuffer::borrow_message_main_loop(LogMessage *&message, uint16_t &text_length) { + if (this->current_token_) { return false; } @@ -44,7 +44,7 @@ bool TaskLogBuffer::borrow_message_main_loop(LogMessage **message, const char ** LogMessage *msg = static_cast(received_item); *message = msg; - *text = msg->text_data(); + text_length = msg->text_length; this->current_token_ = received_item; return true; diff --git a/esphome/components/logger/task_log_buffer_esp32.h b/esphome/components/logger/task_log_buffer_esp32.h index f837814d3c..88d72eacfc 100644 --- a/esphome/components/logger/task_log_buffer_esp32.h +++ b/esphome/components/logger/task_log_buffer_esp32.h @@ -52,7 +52,7 @@ class TaskLogBuffer { ~TaskLogBuffer(); // NOT thread-safe - borrow a message from the ring buffer, only call from main loop - bool borrow_message_main_loop(LogMessage **message, const char **text); + bool borrow_message_main_loop(LogMessage *&message, uint16_t &text_length); // NOT thread-safe - release a message buffer and update the counter, only call from main loop void release_message_main_loop(); diff --git a/esphome/components/logger/task_log_buffer_host.cpp b/esphome/components/logger/task_log_buffer_host.cpp index c98c062b75..c2ab009db4 100644 --- a/esphome/components/logger/task_log_buffer_host.cpp +++ b/esphome/components/logger/task_log_buffer_host.cpp @@ -115,11 +115,7 @@ bool TaskLogBuffer::send_message_thread_safe(uint8_t level, const char *tag, uin return true; } -bool TaskLogBuffer::get_message_main_loop(LogMessage **message) { - if (message == nullptr) { - return false; - } - +bool TaskLogBuffer::borrow_message_main_loop(LogMessage *&message, uint16_t &text_length) { size_t current_read = this->read_index_.load(std::memory_order_relaxed); size_t current_write = this->write_index_.load(std::memory_order_acquire); @@ -134,7 +130,8 @@ bool TaskLogBuffer::get_message_main_loop(LogMessage **message) { return false; } - *message = &msg; + message = &msg; + text_length = msg.text_length; return true; } diff --git a/esphome/components/logger/task_log_buffer_host.h b/esphome/components/logger/task_log_buffer_host.h index c9de381cd8..31bded9c9f 100644 --- a/esphome/components/logger/task_log_buffer_host.h +++ b/esphome/components/logger/task_log_buffer_host.h @@ -21,12 +21,12 @@ namespace esphome::logger { * * Threading Model: Multi-Producer Single-Consumer (MPSC) * - Multiple threads can safely call send_message_thread_safe() concurrently - * - Only the main loop thread calls get_message_main_loop() and release_message_main_loop() + * - Only the main loop thread calls borrow_message_main_loop() and release_message_main_loop() * * Producers (multiple threads) Consumer (main loop only) * │ │ * ▼ ▼ - * acquire_write_slot_() get_message_main_loop() + * acquire_write_slot_() bool borrow_message_main_loop() * CAS on reserve_index_ read write_index_ * │ check ready flag * ▼ │ @@ -79,7 +79,7 @@ class TaskLogBuffer { // NOT thread-safe - get next message from buffer, only call from main loop // Returns true if a message was retrieved, false if buffer is empty - bool get_message_main_loop(LogMessage **message); + bool borrow_message_main_loop(LogMessage *&message, uint16_t &text_length); // NOT thread-safe - release the message after processing, only call from main loop void release_message_main_loop(); diff --git a/esphome/components/logger/task_log_buffer_libretiny.cpp b/esphome/components/logger/task_log_buffer_libretiny.cpp index d4662bd907..5969f6fb40 100644 --- a/esphome/components/logger/task_log_buffer_libretiny.cpp +++ b/esphome/components/logger/task_log_buffer_libretiny.cpp @@ -47,11 +47,7 @@ size_t TaskLogBuffer::available_contiguous_space() const { } } -bool TaskLogBuffer::borrow_message_main_loop(LogMessage **message, const char **text) { - if (message == nullptr || text == nullptr) { - return false; - } - +bool TaskLogBuffer::borrow_message_main_loop(LogMessage *&message, uint16_t &text_length) { // Check if buffer was initialized successfully if (this->mutex_ == nullptr || this->storage_ == nullptr) { return false; @@ -77,8 +73,8 @@ bool TaskLogBuffer::borrow_message_main_loop(LogMessage **message, const char ** this->tail_ = 0; msg = reinterpret_cast(this->storage_); } - *message = msg; - *text = msg->text_data(); + message = msg; + text_length = msg->text_length; this->current_message_size_ = message_total_size(msg->text_length); // Keep mutex held until release_message_main_loop() diff --git a/esphome/components/logger/task_log_buffer_libretiny.h b/esphome/components/logger/task_log_buffer_libretiny.h index 8a5cc87bb3..c065065fe7 100644 --- a/esphome/components/logger/task_log_buffer_libretiny.h +++ b/esphome/components/logger/task_log_buffer_libretiny.h @@ -64,7 +64,7 @@ class TaskLogBuffer { ~TaskLogBuffer(); // NOT thread-safe - borrow a message from the buffer, only call from main loop - bool borrow_message_main_loop(LogMessage **message, const char **text); + bool borrow_message_main_loop(LogMessage *&message, uint16_t &text_length); // NOT thread-safe - release a message buffer, only call from main loop void release_message_main_loop(); diff --git a/esphome/components/logger/task_log_buffer_zephyr.cpp b/esphome/components/logger/task_log_buffer_zephyr.cpp index 174f98d92c..7a21af1ef1 100644 --- a/esphome/components/logger/task_log_buffer_zephyr.cpp +++ b/esphome/components/logger/task_log_buffer_zephyr.cpp @@ -8,10 +8,13 @@ namespace esphome::logger { __thread bool non_main_task_recursion_guard_; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -static inline uint32_t get_wlen(const mpsc_pbuf_generic *item) { - auto *msg = reinterpret_cast(item); +static inline uint32_t total_size_in_32bit_words(uint16_t text_length) { // Calculate total size in 32-bit words needed (header + text length + null terminator + 3(4 bytes alignment) - return (sizeof(TaskLogBuffer::LogMessage) + msg->text_length + 1 + 3) / sizeof(uint32_t); + return (sizeof(TaskLogBuffer::LogMessage) + text_length + 1 + 3) / sizeof(uint32_t); +} + +static inline uint32_t get_wlen(const mpsc_pbuf_generic *item) { + return total_size_in_32bit_words(reinterpret_cast(item)->text_length); } TaskLogBuffer::TaskLogBuffer(size_t total_buffer_size) { @@ -42,8 +45,7 @@ bool TaskLogBuffer::send_message_thread_safe(uint8_t level, const char *tag, uin // Calculate actual text length (capped to maximum size) static constexpr size_t MAX_TEXT_SIZE = 255; size_t text_length = (static_cast(ret) > MAX_TEXT_SIZE) ? MAX_TEXT_SIZE : ret; - // Calculate total size in 32-bit words needed (header + text length + null terminator + 3(4 bytes alignment) - size_t total_size = (sizeof(LogMessage) + text_length + 1 + 3) / sizeof(uint32_t); + size_t total_size = total_size_in_32bit_words(text_length); auto *msg = reinterpret_cast(mpsc_pbuf_alloc(&this->log_buffer_, total_size, K_NO_WAIT)); if (nullptr == msg) { return false; @@ -76,7 +78,7 @@ bool TaskLogBuffer::send_message_thread_safe(uint8_t level, const char *tag, uin return true; } -bool TaskLogBuffer::borrow_message_main_loop(LogMessage **message, const char **text) { +bool TaskLogBuffer::borrow_message_main_loop(LogMessage *&message, uint16_t &text_length) { if (this->current_token_) { return false; } @@ -88,13 +90,12 @@ bool TaskLogBuffer::borrow_message_main_loop(LogMessage **message, const char ** } // we claimed buffer alraedy const_cast is safe here - *message = const_cast(reinterpret_cast(this->current_token_)); - - *text = (*message)->text_data(); + message = const_cast(reinterpret_cast(this->current_token_)); + text_length = message->text_length; // Remove trailing newlines - while ((*message)->text_length > 0 && (*text)[(*message)->text_length - 1] == '\n') { - (*message)->text_length--; + while (text_length > 0 && message->text_data()[text_length - 1] == '\n') { + text_length--; } return true; diff --git a/esphome/components/logger/task_log_buffer_zephyr.h b/esphome/components/logger/task_log_buffer_zephyr.h index c7e3ae5599..f5487a1c97 100644 --- a/esphome/components/logger/task_log_buffer_zephyr.h +++ b/esphome/components/logger/task_log_buffer_zephyr.h @@ -32,8 +32,6 @@ class TaskLogBuffer { // Methods for accessing message contents inline char *text_data() { return reinterpret_cast(this) + sizeof(LogMessage); } - - inline const char *text_data() const { return reinterpret_cast(this) + sizeof(LogMessage); } }; // Constructor that takes a total buffer size explicit TaskLogBuffer(size_t total_buffer_size); @@ -46,7 +44,7 @@ class TaskLogBuffer { inline size_t size() const { return this->mpsc_config_.size * sizeof(uint32_t); } // NOT thread-safe - borrow a message from the ring buffer, only call from main loop - bool borrow_message_main_loop(LogMessage **message, const char **text); + bool borrow_message_main_loop(LogMessage *&message, uint16_t &text_length); // NOT thread-safe - release a message buffer and update the counter, only call from main loop void release_message_main_loop();