mirror of
https://github.com/esphome/esphome.git
synced 2026-09-03 19:46:02 +00:00
Address review feedback
- Remove dead DEFAULT_SLOT_COUNT constant from host header - Fix misleading BSS comments (Logger is heap-allocated via new_Pvariable) - Add comment explaining Zephyr BUF_WORD_COUNT formula
This commit is contained in:
@@ -332,7 +332,7 @@ async def to_code(config: ConfigType) -> None:
|
||||
tx_buffer_size = config[CONF_TX_BUFFER_SIZE]
|
||||
cg.add_define("ESPHOME_LOGGER_TX_BUFFER_SIZE", tx_buffer_size)
|
||||
# Determine task log buffer size. The buffer is a direct member of Logger
|
||||
# (lives in BSS), so it's available from program start with no heap allocation.
|
||||
# (no separate heap allocation).
|
||||
task_log_buffer_size = 0
|
||||
if CORE.is_esp32 or CORE.is_libretiny or CORE.is_nrf52:
|
||||
task_log_buffer_size = config[CONF_TASK_LOG_BUFFER_SIZE]
|
||||
|
||||
@@ -369,7 +369,7 @@ class Logger final : public Component {
|
||||
// Large buffers placed last to keep frequently-accessed member offsets small
|
||||
char tx_buffer_[ESPHOME_LOGGER_TX_BUFFER_SIZE + 1]; // +1 for null terminator
|
||||
#ifdef USE_ESPHOME_TASK_LOG_BUFFER
|
||||
logger::TaskLogBuffer log_buffer_; // Task log buffer storage in BSS (no heap allocation)
|
||||
logger::TaskLogBuffer log_buffer_; // Embedded in Logger (no separate heap allocation)
|
||||
#endif
|
||||
|
||||
// --- get_thread_name_ overloads (per-platform) ---
|
||||
|
||||
@@ -9,7 +9,7 @@ namespace esphome::logger {
|
||||
|
||||
TaskLogBuffer::TaskLogBuffer() {
|
||||
// Create a static ring buffer with RINGBUF_TYPE_NOSPLIT for message integrity
|
||||
// Storage is a member array (lives in BSS), no heap allocation needed
|
||||
// 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_);
|
||||
}
|
||||
|
||||
@@ -70,7 +70,7 @@ class TaskLogBuffer {
|
||||
private:
|
||||
RingbufHandle_t ring_buffer_{nullptr}; // FreeRTOS ring buffer handle
|
||||
StaticRingbuffer_t structure_; // Static structure for the ring buffer
|
||||
uint8_t storage_[ESPHOME_TASK_LOG_BUFFER_SIZE]; // Buffer storage in BSS
|
||||
uint8_t storage_[ESPHOME_TASK_LOG_BUFFER_SIZE]; // Embedded in Logger (no separate heap allocation)
|
||||
|
||||
// Atomic counter for message tracking (only differences matter)
|
||||
std::atomic<uint16_t> message_counter_{0}; // Incremented when messages are committed
|
||||
|
||||
@@ -49,9 +49,6 @@ namespace esphome::logger {
|
||||
*/
|
||||
class TaskLogBuffer {
|
||||
public:
|
||||
// Default number of message slots - host has plenty of memory
|
||||
static constexpr size_t DEFAULT_SLOT_COUNT = 64;
|
||||
|
||||
// Structure for a log message (fixed size for lock-free operation)
|
||||
struct LogMessage {
|
||||
// Size constants
|
||||
@@ -103,7 +100,7 @@ class TaskLogBuffer {
|
||||
// Commit a slot after writing (thread-safe)
|
||||
void commit_write_slot_(int slot_index);
|
||||
|
||||
LogMessage slots_[ESPHOME_TASK_LOG_BUFFER_SIZE]; // Pre-allocated message slots in BSS
|
||||
LogMessage slots_[ESPHOME_TASK_LOG_BUFFER_SIZE]; // Embedded in Logger (no separate heap allocation)
|
||||
|
||||
// Lock-free indices using atomics
|
||||
// - reserve_index_: Next slot to reserve (producers CAS this to claim slots)
|
||||
|
||||
@@ -9,7 +9,7 @@ namespace esphome::logger {
|
||||
|
||||
TaskLogBuffer::TaskLogBuffer() {
|
||||
// Create mutex for thread-safe access
|
||||
// Storage is a member array (lives in BSS), no heap allocation needed
|
||||
// Storage is a member array (embedded in Logger), no heap allocation needed
|
||||
this->mutex_ = xSemaphoreCreateMutex();
|
||||
}
|
||||
|
||||
|
||||
@@ -86,7 +86,7 @@ class TaskLogBuffer {
|
||||
// Calculate available contiguous space at write position
|
||||
size_t available_contiguous_space() const;
|
||||
|
||||
uint8_t storage_[ESPHOME_TASK_LOG_BUFFER_SIZE]; // Buffer storage in BSS
|
||||
uint8_t storage_[ESPHOME_TASK_LOG_BUFFER_SIZE]; // Embedded in Logger (no separate heap allocation)
|
||||
size_t head_{0}; // Write position
|
||||
size_t tail_{0}; // Read position
|
||||
|
||||
|
||||
@@ -18,7 +18,7 @@ static inline uint32_t get_wlen(const mpsc_pbuf_generic *item) {
|
||||
}
|
||||
|
||||
TaskLogBuffer::TaskLogBuffer() {
|
||||
// Storage is a member array (lives in BSS), no heap allocation needed
|
||||
// Storage is a member array (embedded in Logger), no heap allocation needed
|
||||
this->mpsc_config_.buf = this->buf_storage_;
|
||||
this->mpsc_config_.size = BUF_WORD_COUNT;
|
||||
this->mpsc_config_.flags = MPSC_PBUF_MODE_OVERWRITE;
|
||||
|
||||
@@ -53,8 +53,9 @@ class TaskLogBuffer {
|
||||
const char *format, va_list args);
|
||||
|
||||
protected:
|
||||
// Round up byte size to 32-bit word count for mpsc_pbuf alignment requirement
|
||||
static constexpr size_t BUF_WORD_COUNT = (ESPHOME_TASK_LOG_BUFFER_SIZE + 3) / sizeof(uint32_t);
|
||||
uint32_t buf_storage_[BUF_WORD_COUNT]; // Buffer storage in BSS
|
||||
uint32_t buf_storage_[BUF_WORD_COUNT]; // Embedded in Logger (no separate heap allocation)
|
||||
mpsc_pbuf_buffer_config mpsc_config_{};
|
||||
mpsc_pbuf_buffer log_buffer_{};
|
||||
const mpsc_pbuf_generic *current_token_{};
|
||||
|
||||
Reference in New Issue
Block a user