From 0b77cb1d1692f036db2f99cc73940cc16b7302f3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 15 May 2025 04:36:28 -0500 Subject: [PATCH 1/4] Logger Recursion Guard per Task on ESP32 (#8765) --- esphome/components/logger/__init__.py | 1 + esphome/components/logger/logger.cpp | 47 +++++++++++---------- esphome/components/logger/logger.h | 60 ++++++++++++++++++++++++--- 3 files changed, 82 insertions(+), 26 deletions(-) diff --git a/esphome/components/logger/__init__.py b/esphome/components/logger/__init__.py index 01e75a424d..4698c1d9f1 100644 --- a/esphome/components/logger/__init__.py +++ b/esphome/components/logger/__init__.py @@ -254,6 +254,7 @@ async def to_code(config): config[CONF_TX_BUFFER_SIZE], ) if CORE.is_esp32: + cg.add(log.create_pthread_key()) task_log_buffer_size = config[CONF_TASK_LOG_BUFFER_SIZE] if task_log_buffer_size > 0: cg.add_define("USE_ESPHOME_TASK_LOG_BUFFER") diff --git a/esphome/components/logger/logger.cpp b/esphome/components/logger/logger.cpp index 812a7cc16d..0ad909cb07 100644 --- a/esphome/components/logger/logger.cpp +++ b/esphome/components/logger/logger.cpp @@ -14,20 +14,27 @@ namespace logger { static const char *const TAG = "logger"; #ifdef USE_ESP32 -// Implementation for ESP32 (multi-core with atomic support) -// Main thread: synchronous logging with direct buffer access -// Other threads: console output with stack buffer, callbacks via async buffer +// Implementation for ESP32 (multi-task platform with task-specific tracking) +// Main task always uses direct buffer access for console output and callbacks +// Other tasks: +// - With task log buffer: stack buffer for console output, async buffer for callbacks +// - Without task log buffer: only console output, no callbacks void HOT Logger::log_vprintf_(int level, const char *tag, int line, const char *format, va_list args) { // NOLINT - if (level > this->level_for(tag) || recursion_guard_.load(std::memory_order_relaxed)) + if (level > this->level_for(tag)) return; - recursion_guard_.store(true, std::memory_order_relaxed); TaskHandle_t current_task = xTaskGetCurrentTaskHandle(); + bool is_main_task = (current_task == main_task_); - // For main task: call log_message_to_buffer_and_send_ which does console and callback logging - if (current_task == main_task_) { + // Check and set recursion guard - uses pthread TLS for per-task state + if (this->check_and_set_task_log_recursion_(is_main_task)) { + return; // Recursion detected + } + + // Main task uses the shared buffer for efficiency + if (is_main_task) { this->log_message_to_buffer_and_send_(level, tag, line, format, args); - recursion_guard_.store(false, std::memory_order_release); + this->reset_task_log_recursion_(is_main_task); return; } @@ -51,23 +58,21 @@ void HOT Logger::log_vprintf_(int level, const char *tag, int line, const char * } #endif // USE_ESPHOME_TASK_LOG_BUFFER - recursion_guard_.store(false, std::memory_order_release); + // Reset the recursion guard for this task + this->reset_task_log_recursion_(is_main_task); } -#endif // USE_ESP32 - -#ifndef USE_ESP32 -// Implementation for platforms that do not support atomic operations -// or have to consider logging in other tasks +#else +// Implementation for all other platforms void HOT Logger::log_vprintf_(int level, const char *tag, int line, const char *format, va_list args) { // NOLINT - if (level > this->level_for(tag) || recursion_guard_) + if (level > this->level_for(tag) || global_recursion_guard_) return; - recursion_guard_ = true; + global_recursion_guard_ = true; // Format and send to both console and callbacks this->log_message_to_buffer_and_send_(level, tag, line, format, args); - recursion_guard_ = false; + global_recursion_guard_ = false; } #endif // !USE_ESP32 @@ -76,10 +81,10 @@ void HOT Logger::log_vprintf_(int level, const char *tag, int line, const char * // Note: USE_STORE_LOG_STR_IN_FLASH is only defined for ESP8266. void Logger::log_vprintf_(int level, const char *tag, int line, const __FlashStringHelper *format, va_list args) { // NOLINT - if (level > this->level_for(tag) || recursion_guard_) + if (level > this->level_for(tag) || global_recursion_guard_) return; - recursion_guard_ = true; + global_recursion_guard_ = true; this->tx_buffer_at_ = 0; // Copy format string from progmem @@ -91,7 +96,7 @@ void Logger::log_vprintf_(int level, const char *tag, int line, const __FlashStr // Buffer full from copying format if (this->tx_buffer_at_ >= this->tx_buffer_size_) { - recursion_guard_ = false; // Make sure to reset the recursion guard before returning + global_recursion_guard_ = false; // Make sure to reset the recursion guard before returning return; } @@ -107,7 +112,7 @@ void Logger::log_vprintf_(int level, const char *tag, int line, const __FlashStr } this->call_log_callbacks_(level, tag, this->tx_buffer_ + msg_start); - recursion_guard_ = false; + global_recursion_guard_ = false; } #endif // USE_STORE_LOG_STR_IN_FLASH diff --git a/esphome/components/logger/logger.h b/esphome/components/logger/logger.h index 8619cc0992..5c53c4d40c 100644 --- a/esphome/components/logger/logger.h +++ b/esphome/components/logger/logger.h @@ -3,7 +3,7 @@ #include #include #ifdef USE_ESP32 -#include +#include #endif #include "esphome/core/automation.h" #include "esphome/core/component.h" @@ -84,6 +84,23 @@ enum UARTSelection { }; #endif // USE_ESP32 || USE_ESP8266 || USE_RP2040 || USE_LIBRETINY +/** + * @brief Logger component for all ESPHome logging. + * + * This class implements a multi-platform logging system with protection against recursion. + * + * Recursion Protection Strategy: + * - On ESP32: Uses task-specific recursion guards + * * Main task: Uses a dedicated boolean member variable for efficiency + * * Other tasks: Uses pthread TLS with a dynamically allocated key for task-specific state + * - On other platforms: Uses a simple global recursion guard + * + * We use pthread TLS via pthread_key_create to create a unique key for storing + * task-specific recursion state, which: + * 1. Efficiently handles multiple tasks without locks or mutexes + * 2. Works with ESP-IDF's pthread implementation that uses a linked list for TLS variables + * 3. Avoids the limitations of the fixed FreeRTOS task local storage slots + */ class Logger : public Component { public: explicit Logger(uint32_t baud_rate, size_t tx_buffer_size); @@ -102,6 +119,9 @@ class Logger : public Component { #ifdef USE_ESP_IDF uart_port_t get_uart_num() const { return uart_num_; } #endif +#ifdef USE_ESP32 + void create_pthread_key() { pthread_key_create(&log_recursion_key_, nullptr); } +#endif #if defined(USE_ESP32) || defined(USE_ESP8266) || defined(USE_RP2040) || defined(USE_LIBRETINY) void set_uart_selection(UARTSelection uart_selection) { uart_ = uart_selection; } /// Get the UART used by the logger. @@ -222,18 +242,22 @@ class Logger : public Component { std::map log_levels_{}; CallbackManager log_callback_{}; int current_level_{ESPHOME_LOG_LEVEL_VERY_VERBOSE}; -#ifdef USE_ESP32 - std::atomic recursion_guard_{false}; #ifdef USE_ESPHOME_TASK_LOG_BUFFER std::unique_ptr log_buffer_; // Will be initialized with init_log_buffer #endif +#ifdef USE_ESP32 + // Task-specific recursion guards: + // - Main task uses a dedicated member variable for efficiency + // - Other tasks use pthread TLS with a dynamically created key via pthread_key_create + bool main_task_recursion_guard_{false}; + pthread_key_t log_recursion_key_; #else - bool recursion_guard_{false}; + bool global_recursion_guard_{false}; // Simple global recursion guard for single-task platforms #endif - void *main_task_ = nullptr; CallbackManager level_callback_{}; #if defined(USE_ESP32) || defined(USE_LIBRETINY) + void *main_task_ = nullptr; // Only used for thread name identification const char *HOT get_thread_name_() { TaskHandle_t current_task = xTaskGetCurrentTaskHandle(); if (current_task == main_task_) { @@ -248,6 +272,32 @@ class Logger : public Component { } #endif +#ifdef USE_ESP32 + inline bool HOT check_and_set_task_log_recursion_(bool is_main_task) { + if (is_main_task) { + const bool was_recursive = main_task_recursion_guard_; + main_task_recursion_guard_ = true; + return was_recursive; + } + + intptr_t current = (intptr_t) pthread_getspecific(log_recursion_key_); + if (current != 0) + return true; + + pthread_setspecific(log_recursion_key_, (void *) 1); + return false; + } + + inline void HOT reset_task_log_recursion_(bool is_main_task) { + if (is_main_task) { + main_task_recursion_guard_ = false; + return; + } + + pthread_setspecific(log_recursion_key_, (void *) 0); + } +#endif + inline void HOT write_header_to_buffer_(int level, const char *tag, int line, const char *thread_name, char *buffer, int *buffer_at, int buffer_size) { // Format header From 88edddf07a6f471fa19d43d35330beff6d184762 Mon Sep 17 00:00:00 2001 From: Thomas Rupprecht Date: Thu, 15 May 2025 11:45:07 +0200 Subject: [PATCH 2/4] [log] improve/refactor `log` (#8708) --- esphome/__main__.py | 30 +++++++++-------- esphome/config.py | 26 +++++++-------- esphome/log.py | 34 +++++++++---------- esphome/mqtt.py | 4 +-- esphome/wizard.py | 80 +++++++++++++++++++++++++-------------------- 5 files changed, 91 insertions(+), 83 deletions(-) diff --git a/esphome/__main__.py b/esphome/__main__.py index c78eda7e12..9f638456e6 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -43,7 +43,7 @@ from esphome.const import ( ) from esphome.core import CORE, EsphomeError, coroutine from esphome.helpers import get_bool_env, indent, is_ip_address -from esphome.log import Fore, color, setup_log +from esphome.log import AnsiFore, color, setup_log from esphome.util import ( get_serial_ports, list_yaml_files, @@ -83,7 +83,7 @@ def choose_prompt(options, purpose: str = None): raise ValueError break except ValueError: - safe_print(color(Fore.RED, f"Invalid option: '{opt}'")) + safe_print(color(AnsiFore.RED, f"Invalid option: '{opt}'")) return options[opt - 1][1] @@ -596,30 +596,30 @@ def command_update_all(args): click.echo(f"{half_line}{middle_text}{half_line}") for f in files: - print(f"Updating {color(Fore.CYAN, f)}") + print(f"Updating {color(AnsiFore.CYAN, f)}") print("-" * twidth) print() rc = run_external_process( "esphome", "--dashboard", "run", f, "--no-logs", "--device", "OTA" ) if rc == 0: - print_bar(f"[{color(Fore.BOLD_GREEN, 'SUCCESS')}] {f}") + print_bar(f"[{color(AnsiFore.BOLD_GREEN, 'SUCCESS')}] {f}") success[f] = True else: - print_bar(f"[{color(Fore.BOLD_RED, 'ERROR')}] {f}") + print_bar(f"[{color(AnsiFore.BOLD_RED, 'ERROR')}] {f}") success[f] = False print() print() print() - print_bar(f"[{color(Fore.BOLD_WHITE, 'SUMMARY')}]") + print_bar(f"[{color(AnsiFore.BOLD_WHITE, 'SUMMARY')}]") failed = 0 for f in files: if success[f]: - print(f" - {f}: {color(Fore.GREEN, 'SUCCESS')}") + print(f" - {f}: {color(AnsiFore.GREEN, 'SUCCESS')}") else: - print(f" - {f}: {color(Fore.BOLD_RED, 'FAILED')}") + print(f" - {f}: {color(AnsiFore.BOLD_RED, 'FAILED')}") failed += 1 return failed @@ -645,7 +645,7 @@ def command_rename(args, config): if c not in ALLOWED_NAME_CHARS: print( color( - Fore.BOLD_RED, + AnsiFore.BOLD_RED, f"'{c}' is an invalid character for names. Valid characters are: " f"{ALLOWED_NAME_CHARS} (lowercase, no spaces)", ) @@ -658,7 +658,9 @@ def command_rename(args, config): yaml = yaml_util.load_yaml(CORE.config_path) if CONF_ESPHOME not in yaml or CONF_NAME not in yaml[CONF_ESPHOME]: print( - color(Fore.BOLD_RED, "Complex YAML files cannot be automatically renamed.") + color( + AnsiFore.BOLD_RED, "Complex YAML files cannot be automatically renamed." + ) ) return 1 old_name = yaml[CONF_ESPHOME][CONF_NAME] @@ -681,7 +683,7 @@ def command_rename(args, config): ) > 1 ): - print(color(Fore.BOLD_RED, "Too many matches in YAML to safely rename")) + print(color(AnsiFore.BOLD_RED, "Too many matches in YAML to safely rename")) return 1 new_raw = re.sub( @@ -693,7 +695,7 @@ def command_rename(args, config): new_path = os.path.join(CORE.config_dir, args.name + ".yaml") print( - f"Updating {color(Fore.CYAN, CORE.config_path)} to {color(Fore.CYAN, new_path)}" + f"Updating {color(AnsiFore.CYAN, CORE.config_path)} to {color(AnsiFore.CYAN, new_path)}" ) print() @@ -702,7 +704,7 @@ def command_rename(args, config): rc = run_external_process("esphome", "config", new_path) if rc != 0: - print(color(Fore.BOLD_RED, "Rename failed. Reverting changes.")) + print(color(AnsiFore.BOLD_RED, "Rename failed. Reverting changes.")) os.remove(new_path) return 1 @@ -728,7 +730,7 @@ def command_rename(args, config): if CORE.config_path != new_path: os.remove(CORE.config_path) - print(color(Fore.BOLD_GREEN, "SUCCESS")) + print(color(AnsiFore.BOLD_GREEN, "SUCCESS")) print() return 0 diff --git a/esphome/config.py b/esphome/config.py index 09ee2a8f9b..4b26b33c78 100644 --- a/esphome/config.py +++ b/esphome/config.py @@ -28,7 +28,7 @@ import esphome.core.config as core_config import esphome.final_validate as fv from esphome.helpers import indent from esphome.loader import ComponentManifest, get_component, get_platform -from esphome.log import Fore, color +from esphome.log import AnsiFore, color from esphome.types import ConfigFragmentType, ConfigType from esphome.util import OrderedDict, safe_print from esphome.voluptuous_schema import ExtraKeysInvalid @@ -959,7 +959,7 @@ def line_info(config, path, highlight=True): if obj: mark = obj.start_mark source = f"[source {mark.document}:{mark.line + 1}]" - return color(Fore.CYAN, source) + return color(AnsiFore.CYAN, source) return "None" @@ -983,7 +983,7 @@ def dump_dict( if at_root: error = config.get_error_for_path(path) if error is not None: - ret += f"\n{color(Fore.BOLD_RED, _format_vol_invalid(error, config))}\n" + ret += f"\n{color(AnsiFore.BOLD_RED, _format_vol_invalid(error, config))}\n" if isinstance(conf, (list, tuple)): multiline = True @@ -995,11 +995,11 @@ def dump_dict( path_ = path + [i] error = config.get_error_for_path(path_) if error is not None: - ret += f"\n{color(Fore.BOLD_RED, _format_vol_invalid(error, config))}\n" + ret += f"\n{color(AnsiFore.BOLD_RED, _format_vol_invalid(error, config))}\n" sep = "- " if config.is_in_error_path(path_): - sep = color(Fore.RED, sep) + sep = color(AnsiFore.RED, sep) msg, _ = dump_dict(config, path_, at_root=False) msg = indent(msg) inf = line_info(config, path_, highlight=config.is_in_error_path(path_)) @@ -1018,11 +1018,11 @@ def dump_dict( path_ = path + [k] error = config.get_error_for_path(path_) if error is not None: - ret += f"\n{color(Fore.BOLD_RED, _format_vol_invalid(error, config))}\n" + ret += f"\n{color(AnsiFore.BOLD_RED, _format_vol_invalid(error, config))}\n" st = f"{k}: " if config.is_in_error_path(path_): - st = color(Fore.RED, st) + st = color(AnsiFore.RED, st) msg, m = dump_dict(config, path_, at_root=False) inf = line_info(config, path_, highlight=config.is_in_error_path(path_)) @@ -1044,7 +1044,7 @@ def dump_dict( if len(conf) > 80: conf = f"|-\n{indent(conf)}" error = config.get_error_for_path(path) - col = Fore.BOLD_RED if error else Fore.KEEP + col = AnsiFore.BOLD_RED if error else AnsiFore.KEEP ret += color(col, str(conf)) elif isinstance(conf, core.Lambda): if is_secret(conf): @@ -1052,13 +1052,13 @@ def dump_dict( conf = f"!lambda |-\n{indent(str(conf.value))}" error = config.get_error_for_path(path) - col = Fore.BOLD_RED if error else Fore.KEEP + col = AnsiFore.BOLD_RED if error else AnsiFore.KEEP ret += color(col, conf) elif conf is None: pass else: error = config.get_error_for_path(path) - col = Fore.BOLD_RED if error else Fore.KEEP + col = AnsiFore.BOLD_RED if error else AnsiFore.KEEP ret += color(col, str(conf)) multiline = "\n" in ret @@ -1100,13 +1100,13 @@ def read_config(command_line_substitutions): if not CORE.verbose: res = strip_default_ids(res) - safe_print(color(Fore.BOLD_RED, "Failed config")) + safe_print(color(AnsiFore.BOLD_RED, "Failed config")) safe_print("") for path, domain in res.output_paths: if not res.is_in_error_path(path): continue - errstr = color(Fore.BOLD_RED, f"{domain}:") + errstr = color(AnsiFore.BOLD_RED, f"{domain}:") errline = line_info(res, path) if errline: errstr += f" {errline}" @@ -1121,7 +1121,7 @@ def read_config(command_line_substitutions): safe_print(indent("\n".join(split_dump[:i]))) for err in res.errors: - safe_print(color(Fore.BOLD_RED, err.msg)) + safe_print(color(AnsiFore.BOLD_RED, err.msg)) safe_print("") return None diff --git a/esphome/log.py b/esphome/log.py index 516f27be45..7e69a2fef8 100644 --- a/esphome/log.py +++ b/esphome/log.py @@ -1,9 +1,10 @@ +from enum import Enum import logging from esphome.core import CORE -class AnsiFore: +class AnsiFore(Enum): KEEP = "" BLACK = "\033[30m" RED = "\033[31m" @@ -26,7 +27,7 @@ class AnsiFore: BOLD_RESET = "\033[1;39m" -class AnsiStyle: +class AnsiStyle(Enum): BRIGHT = "\033[1m" BOLD = "\033[1m" DIM = "\033[2m" @@ -35,16 +36,10 @@ class AnsiStyle: RESET_ALL = "\033[0m" -Fore = AnsiFore() -Style = AnsiStyle() - - -def color(col: str, msg: str, reset: bool = True) -> bool: - if col and not col.startswith("\033["): - raise ValueError("Color must be value from esphome.log.Fore") - s = str(col) + msg +def color(col: AnsiFore, msg: str, reset: bool = True) -> str: + s = col.value + msg if reset and col: - s += str(Style.RESET_ALL) + s += AnsiStyle.RESET_ALL.value return s @@ -54,20 +49,21 @@ class ESPHomeLogFormatter(logging.Formatter): fmt += "%(levelname)s %(message)s" super().__init__(fmt=fmt, style="%") - def format(self, record): + # @override + def format(self, record: logging.LogRecord) -> str: formatted = super().format(record) prefix = { - "DEBUG": Fore.CYAN, - "INFO": Fore.GREEN, - "WARNING": Fore.YELLOW, - "ERROR": Fore.RED, - "CRITICAL": Fore.RED, + "DEBUG": AnsiFore.CYAN.value, + "INFO": AnsiFore.GREEN.value, + "WARNING": AnsiFore.YELLOW.value, + "ERROR": AnsiFore.RED.value, + "CRITICAL": AnsiFore.RED.value, }.get(record.levelname, "") - return f"{prefix}{formatted}{Style.RESET_ALL}" + return f"{prefix}{formatted}{AnsiStyle.RESET_ALL.value}" def setup_log( - log_level=logging.INFO, + log_level: int = logging.INFO, include_timestamp: bool = False, ) -> None: import colorama diff --git a/esphome/mqtt.py b/esphome/mqtt.py index 2403a4a1d9..a420b5ba7f 100644 --- a/esphome/mqtt.py +++ b/esphome/mqtt.py @@ -28,7 +28,7 @@ from esphome.const import ( ) from esphome.core import CORE, EsphomeError from esphome.helpers import get_int_env, get_str_env -from esphome.log import Fore, color +from esphome.log import AnsiFore, color from esphome.util import safe_print _LOGGER = logging.getLogger(__name__) @@ -291,7 +291,7 @@ def get_fingerprint(config): sha1 = hashlib.sha1(cert_der).hexdigest() - safe_print(f"SHA1 Fingerprint: {color(Fore.CYAN, sha1)}") + safe_print(f"SHA1 Fingerprint: {color(AnsiFore.CYAN, sha1)}") safe_print( f"Copy the string above into mqtt.ssl_fingerprints section of {CORE.config_path}" ) diff --git a/esphome/wizard.py b/esphome/wizard.py index 8c5bd07e1f..ca987304e2 100644 --- a/esphome/wizard.py +++ b/esphome/wizard.py @@ -9,7 +9,7 @@ import esphome.config_validation as cv from esphome.const import ALLOWED_NAME_CHARS, ENV_QUICKWIZARD from esphome.core import CORE from esphome.helpers import get_bool_env, write_file -from esphome.log import Fore, color +from esphome.log import AnsiFore, color from esphome.storage_json import StorageJSON, ext_storage_path from esphome.util import safe_input, safe_print @@ -219,7 +219,7 @@ def wizard_write(path, **kwargs): elif board in rtl87xx_boards.BOARDS: platform = "RTL87XX" else: - safe_print(color(Fore.RED, f'The board "{board}" is unknown.')) + safe_print(color(AnsiFore.RED, f'The board "{board}" is unknown.')) return False kwargs["platform"] = platform hardware = kwargs["platform"] @@ -274,12 +274,12 @@ def wizard(path): if not path.endswith(".yaml") and not path.endswith(".yml"): safe_print( - f"Please make your configuration file {color(Fore.CYAN, path)} have the extension .yaml or .yml" + f"Please make your configuration file {color(AnsiFore.CYAN, path)} have the extension .yaml or .yml" ) return 1 if os.path.exists(path): safe_print( - f"Uh oh, it seems like {color(Fore.CYAN, path)} already exists, please delete that file first or chose another configuration file." + f"Uh oh, it seems like {color(AnsiFore.CYAN, path)} already exists, please delete that file first or chose another configuration file." ) return 2 @@ -298,17 +298,19 @@ def wizard(path): sleep(3.0) safe_print() safe_print_step(1, CORE_BIG) - safe_print(f"First up, please choose a {color(Fore.GREEN, 'name')} for your node.") + safe_print( + f"First up, please choose a {color(AnsiFore.GREEN, 'name')} for your node." + ) safe_print( "It should be a unique name that can be used to identify the device later." ) sleep(1) safe_print( - f"For example, I like calling the node in my living room {color(Fore.BOLD_WHITE, 'livingroom')}." + f"For example, I like calling the node in my living room {color(AnsiFore.BOLD_WHITE, 'livingroom')}." ) safe_print() sleep(1) - name = safe_input(color(Fore.BOLD_WHITE, "(name): ")) + name = safe_input(color(AnsiFore.BOLD_WHITE, "(name): ")) while True: try: @@ -317,7 +319,7 @@ def wizard(path): except vol.Invalid: safe_print( color( - Fore.RED, + AnsiFore.RED, f'Oh noes, "{name}" isn\'t a valid name. Names can only ' f"include numbers, lower-case letters and hyphens. ", ) @@ -325,11 +327,13 @@ def wizard(path): name = strip_accents(name).lower().replace(" ", "-") name = strip_accents(name).lower().replace("_", "-") name = "".join(c for c in name if c in ALLOWED_NAME_CHARS) - safe_print(f'Shall I use "{color(Fore.CYAN, name)}" as the name instead?') + safe_print( + f'Shall I use "{color(AnsiFore.CYAN, name)}" as the name instead?' + ) sleep(0.5) name = default_input("(name [{}]): ", name) - safe_print(f'Great! Your node is now called "{color(Fore.CYAN, name)}".') + safe_print(f'Great! Your node is now called "{color(AnsiFore.CYAN, name)}".') sleep(1) safe_print_step(2, ESP_BIG) safe_print( @@ -346,7 +350,7 @@ def wizard(path): sleep(0.5) safe_print() platform = safe_input( - color(Fore.BOLD_WHITE, f"({'/'.join(wizard_platforms)}): ") + color(AnsiFore.BOLD_WHITE, f"({'/'.join(wizard_platforms)}): ") ) try: platform = vol.All(vol.Upper, vol.Any(*wizard_platforms))(platform.upper()) @@ -355,7 +359,9 @@ def wizard(path): safe_print( f'Unfortunately, I can\'t find an espressif microcontroller called "{platform}". Please try again.' ) - safe_print(f"Thanks! You've chosen {color(Fore.CYAN, platform)} as your platform.") + safe_print( + f"Thanks! You've chosen {color(AnsiFore.CYAN, platform)} as your platform." + ) safe_print() sleep(1) @@ -376,27 +382,29 @@ def wizard(path): else: raise NotImplementedError("Unknown platform!") - safe_print(f"Next, I need to know what {color(Fore.GREEN, 'board')} you're using.") + safe_print( + f"Next, I need to know what {color(AnsiFore.GREEN, 'board')} you're using." + ) sleep(0.5) - safe_print(f"Please go to {color(Fore.GREEN, board_link)} and choose a board.") + safe_print(f"Please go to {color(AnsiFore.GREEN, board_link)} and choose a board.") if platform == "ESP32": - safe_print(f"(Type {color(Fore.GREEN, 'esp01_1m')} for Sonoff devices)") + safe_print(f"(Type {color(AnsiFore.GREEN, 'esp01_1m')} for Sonoff devices)") safe_print() # Don't sleep because user needs to copy link if platform == "ESP32": - safe_print(f'For example "{color(Fore.BOLD_WHITE, "nodemcu-32s")}".') + safe_print(f'For example "{color(AnsiFore.BOLD_WHITE, "nodemcu-32s")}".') boards_list = esp32_boards.BOARDS.items() elif platform == "ESP8266": - safe_print(f'For example "{color(Fore.BOLD_WHITE, "nodemcuv2")}".') + safe_print(f'For example "{color(AnsiFore.BOLD_WHITE, "nodemcuv2")}".') boards_list = esp8266_boards.BOARDS.items() elif platform == "BK72XX": - safe_print(f'For example "{color(Fore.BOLD_WHITE, "cb2s")}".') + safe_print(f'For example "{color(AnsiFore.BOLD_WHITE, "cb2s")}".') boards_list = bk72xx_boards.BOARDS.items() elif platform == "RTL87XX": - safe_print(f'For example "{color(Fore.BOLD_WHITE, "wr3")}".') + safe_print(f'For example "{color(AnsiFore.BOLD_WHITE, "wr3")}".') boards_list = rtl87xx_boards.BOARDS.items() elif platform == "RP2040": - safe_print(f'For example "{color(Fore.BOLD_WHITE, "rpipicow")}".') + safe_print(f'For example "{color(AnsiFore.BOLD_WHITE, "rpipicow")}".') boards_list = rp2040_boards.BOARDS.items() else: @@ -409,19 +417,21 @@ def wizard(path): boards.append(board_id) while True: - board = safe_input(color(Fore.BOLD_WHITE, "(board): ")) + board = safe_input(color(AnsiFore.BOLD_WHITE, "(board): ")) try: board = vol.All(vol.Lower, vol.Any(*boards))(board) break except vol.Invalid: safe_print( - color(Fore.RED, f'Sorry, I don\'t think the board "{board}" exists.') + color( + AnsiFore.RED, f'Sorry, I don\'t think the board "{board}" exists.' + ) ) safe_print() sleep(0.25) safe_print() - safe_print(f"Way to go! You've chosen {color(Fore.CYAN, board)} as your board.") + safe_print(f"Way to go! You've chosen {color(AnsiFore.CYAN, board)} as your board.") safe_print() sleep(1) @@ -432,19 +442,19 @@ def wizard(path): safe_print() sleep(1) safe_print( - f"First, what's the {color(Fore.GREEN, 'SSID')} (the name) of the WiFi network {name} should connect to?" + f"First, what's the {color(AnsiFore.GREEN, 'SSID')} (the name) of the WiFi network {name} should connect to?" ) sleep(1.5) - safe_print(f'For example "{color(Fore.BOLD_WHITE, "Abraham Linksys")}".') + safe_print(f'For example "{color(AnsiFore.BOLD_WHITE, "Abraham Linksys")}".') while True: - ssid = safe_input(color(Fore.BOLD_WHITE, "(ssid): ")) + ssid = safe_input(color(AnsiFore.BOLD_WHITE, "(ssid): ")) try: ssid = cv.ssid(ssid) break except vol.Invalid: safe_print( color( - Fore.RED, + AnsiFore.RED, f'Unfortunately, "{ssid}" doesn\'t seem to be a valid SSID. Please try again.', ) ) @@ -452,18 +462,18 @@ def wizard(path): sleep(1) safe_print( - f'Thank you very much! You\'ve just chosen "{color(Fore.CYAN, ssid)}" as your SSID.' + f'Thank you very much! You\'ve just chosen "{color(AnsiFore.CYAN, ssid)}" as your SSID.' ) safe_print() sleep(0.75) safe_print( - f"Now please state the {color(Fore.GREEN, 'password')} of the WiFi network so that I can connect to it (Leave empty for no password)" + f"Now please state the {color(AnsiFore.GREEN, 'password')} of the WiFi network so that I can connect to it (Leave empty for no password)" ) safe_print() - safe_print(f'For example "{color(Fore.BOLD_WHITE, "PASSWORD42")}"') + safe_print(f'For example "{color(AnsiFore.BOLD_WHITE, "PASSWORD42")}"') sleep(0.5) - psk = safe_input(color(Fore.BOLD_WHITE, "(PSK): ")) + psk = safe_input(color(AnsiFore.BOLD_WHITE, "(PSK): ")) safe_print( "Perfect! WiFi is now set up (you can create static IPs and so on later)." ) @@ -475,12 +485,12 @@ def wizard(path): "(over the air) and integrates into Home Assistant with a native API." ) safe_print( - f"This can be insecure if you do not trust the WiFi network. Do you want to set a {color(Fore.GREEN, 'password')} for connecting to this ESP?" + f"This can be insecure if you do not trust the WiFi network. Do you want to set a {color(AnsiFore.GREEN, 'password')} for connecting to this ESP?" ) safe_print() sleep(0.25) safe_print("Press ENTER for no password") - password = safe_input(color(Fore.BOLD_WHITE, "(password): ")) + password = safe_input(color(AnsiFore.BOLD_WHITE, "(password): ")) else: ssid, password, psk = "", "", "" @@ -497,8 +507,8 @@ def wizard(path): safe_print() safe_print( - color(Fore.CYAN, "DONE! I've now written a new configuration file to ") - + color(Fore.BOLD_CYAN, path) + color(AnsiFore.CYAN, "DONE! I've now written a new configuration file to ") + + color(AnsiFore.BOLD_CYAN, path) ) safe_print() safe_print("Next steps:") From 4761ffe0235e295df9a5c07247cf7da78150d36d Mon Sep 17 00:00:00 2001 From: Thomas Rupprecht Date: Thu, 15 May 2025 12:07:41 +0200 Subject: [PATCH 3/4] [gps] update lib, improve code/tests/config (#8768) --- CODEOWNERS | 2 +- esphome/components/gps/__init__.py | 53 +++++++++++++++++++++--------- esphome/components/gps/gps.cpp | 50 ++++++++++++++++++---------- esphome/components/gps/gps.h | 31 ++++++++--------- platformio.ini | 2 +- tests/components/gps/common.yaml | 14 ++++++++ 6 files changed, 102 insertions(+), 50 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index ddd0494a3c..a6e08f225d 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -169,7 +169,7 @@ esphome/components/gp2y1010au0f/* @zry98 esphome/components/gp8403/* @jesserockz esphome/components/gpio/* @esphome/core esphome/components/gpio/one_wire/* @ssieb -esphome/components/gps/* @coogle +esphome/components/gps/* @coogle @ximex esphome/components/graph/* @synco esphome/components/graphical_display_menu/* @MrMDavidson esphome/components/gree/* @orestismers diff --git a/esphome/components/gps/__init__.py b/esphome/components/gps/__init__.py index 88e6f0fd9b..7ccd82dec3 100644 --- a/esphome/components/gps/__init__.py +++ b/esphome/components/gps/__init__.py @@ -9,23 +9,32 @@ from esphome.const import ( CONF_LONGITUDE, CONF_SATELLITES, CONF_SPEED, + DEVICE_CLASS_SPEED, STATE_CLASS_MEASUREMENT, UNIT_DEGREES, UNIT_KILOMETER_PER_HOUR, UNIT_METER, ) +CONF_GPS_ID = "gps_id" +CONF_HDOP = "hdop" + +ICON_ALTIMETER = "mdi:altimeter" +ICON_COMPASS = "mdi:compass" +ICON_LATITUDE = "mdi:latitude" +ICON_LONGITUDE = "mdi:longitude" +ICON_SATELLITE = "mdi:satellite-variant" +ICON_SPEEDOMETER = "mdi:speedometer" + DEPENDENCIES = ["uart"] AUTO_LOAD = ["sensor"] -CODEOWNERS = ["@coogle"] +CODEOWNERS = ["@coogle", "@ximex"] gps_ns = cg.esphome_ns.namespace("gps") GPS = gps_ns.class_("GPS", cg.Component, uart.UARTDevice) GPSListener = gps_ns.class_("GPSListener") -CONF_GPS_ID = "gps_id" -CONF_HDOP = "hdop" MULTI_CONF = True CONFIG_SCHEMA = cv.All( cv.Schema( @@ -33,25 +42,37 @@ CONFIG_SCHEMA = cv.All( cv.GenerateID(): cv.declare_id(GPS), cv.Optional(CONF_LATITUDE): sensor.sensor_schema( unit_of_measurement=UNIT_DEGREES, + icon=ICON_LATITUDE, accuracy_decimals=6, + state_class=STATE_CLASS_MEASUREMENT, ), cv.Optional(CONF_LONGITUDE): sensor.sensor_schema( unit_of_measurement=UNIT_DEGREES, + icon=ICON_LONGITUDE, accuracy_decimals=6, + state_class=STATE_CLASS_MEASUREMENT, ), cv.Optional(CONF_SPEED): sensor.sensor_schema( unit_of_measurement=UNIT_KILOMETER_PER_HOUR, + icon=ICON_SPEEDOMETER, accuracy_decimals=3, + device_class=DEVICE_CLASS_SPEED, + state_class=STATE_CLASS_MEASUREMENT, ), cv.Optional(CONF_COURSE): sensor.sensor_schema( unit_of_measurement=UNIT_DEGREES, + icon=ICON_COMPASS, accuracy_decimals=2, + state_class=STATE_CLASS_MEASUREMENT, ), cv.Optional(CONF_ALTITUDE): sensor.sensor_schema( unit_of_measurement=UNIT_METER, + icon=ICON_ALTIMETER, accuracy_decimals=2, + state_class=STATE_CLASS_MEASUREMENT, ), cv.Optional(CONF_SATELLITES): sensor.sensor_schema( + icon=ICON_SATELLITE, accuracy_decimals=0, state_class=STATE_CLASS_MEASUREMENT, ), @@ -73,28 +94,28 @@ async def to_code(config): await cg.register_component(var, config) await uart.register_uart_device(var, config) - if CONF_LATITUDE in config: - sens = await sensor.new_sensor(config[CONF_LATITUDE]) + if latitude_config := config.get(CONF_LATITUDE): + sens = await sensor.new_sensor(latitude_config) cg.add(var.set_latitude_sensor(sens)) - if CONF_LONGITUDE in config: - sens = await sensor.new_sensor(config[CONF_LONGITUDE]) + if longitude_config := config.get(CONF_LONGITUDE): + sens = await sensor.new_sensor(longitude_config) cg.add(var.set_longitude_sensor(sens)) - if CONF_SPEED in config: - sens = await sensor.new_sensor(config[CONF_SPEED]) + if speed_config := config.get(CONF_SPEED): + sens = await sensor.new_sensor(speed_config) cg.add(var.set_speed_sensor(sens)) - if CONF_COURSE in config: - sens = await sensor.new_sensor(config[CONF_COURSE]) + if course_config := config.get(CONF_COURSE): + sens = await sensor.new_sensor(course_config) cg.add(var.set_course_sensor(sens)) - if CONF_ALTITUDE in config: - sens = await sensor.new_sensor(config[CONF_ALTITUDE]) + if altitude_config := config.get(CONF_ALTITUDE): + sens = await sensor.new_sensor(altitude_config) cg.add(var.set_altitude_sensor(sens)) - if CONF_SATELLITES in config: - sens = await sensor.new_sensor(config[CONF_SATELLITES]) + if satellites_config := config.get(CONF_SATELLITES): + sens = await sensor.new_sensor(satellites_config) cg.add(var.set_satellites_sensor(sens)) if hdop_config := config.get(CONF_HDOP): @@ -102,4 +123,4 @@ async def to_code(config): cg.add(var.set_hdop_sensor(sens)) # https://platformio.org/lib/show/1655/TinyGPSPlus - cg.add_library("mikalhart/TinyGPSPlus", "1.0.2") + cg.add_library("mikalhart/TinyGPSPlus", "1.1.0") diff --git a/esphome/components/gps/gps.cpp b/esphome/components/gps/gps.cpp index e54afdb07e..9dcb351b39 100644 --- a/esphome/components/gps/gps.cpp +++ b/esphome/components/gps/gps.cpp @@ -10,6 +10,17 @@ static const char *const TAG = "gps"; TinyGPSPlus &GPSListener::get_tiny_gps() { return this->parent_->get_tiny_gps(); } +void GPS::dump_config() { + ESP_LOGCONFIG(TAG, "GPS:"); + LOG_SENSOR(" ", "Latitude", this->latitude_sensor_); + LOG_SENSOR(" ", "Longitude", this->longitude_sensor_); + LOG_SENSOR(" ", "Speed", this->speed_sensor_); + LOG_SENSOR(" ", "Course", this->course_sensor_); + LOG_SENSOR(" ", "Altitude", this->altitude_sensor_); + LOG_SENSOR(" ", "Satellites", this->satellites_sensor_); + LOG_SENSOR(" ", "HDOP", this->hdop_sensor_); +} + void GPS::update() { if (this->latitude_sensor_ != nullptr) this->latitude_sensor_->publish_state(this->latitude_); @@ -34,40 +45,45 @@ void GPS::update() { } void GPS::loop() { - while (this->available() && !this->has_time_) { + while (this->available() > 0 && !this->has_time_) { if (this->tiny_gps_.encode(this->read())) { - if (tiny_gps_.location.isUpdated()) { - this->latitude_ = tiny_gps_.location.lat(); - this->longitude_ = tiny_gps_.location.lng(); + if (this->tiny_gps_.location.isUpdated()) { + this->latitude_ = this->tiny_gps_.location.lat(); + this->longitude_ = this->tiny_gps_.location.lng(); ESP_LOGD(TAG, "Location:"); - ESP_LOGD(TAG, " Lat: %f", this->latitude_); - ESP_LOGD(TAG, " Lon: %f", this->longitude_); + ESP_LOGD(TAG, " Lat: %.6f °", this->latitude_); + ESP_LOGD(TAG, " Lon: %.6f °", this->longitude_); } - if (tiny_gps_.speed.isUpdated()) { - this->speed_ = tiny_gps_.speed.kmph(); + if (this->tiny_gps_.speed.isUpdated()) { + this->speed_ = this->tiny_gps_.speed.kmph(); ESP_LOGD(TAG, "Speed: %.3f km/h", this->speed_); } - if (tiny_gps_.course.isUpdated()) { - this->course_ = tiny_gps_.course.deg(); + + if (this->tiny_gps_.course.isUpdated()) { + this->course_ = this->tiny_gps_.course.deg(); ESP_LOGD(TAG, "Course: %.2f °", this->course_); } - if (tiny_gps_.altitude.isUpdated()) { - this->altitude_ = tiny_gps_.altitude.meters(); + + if (this->tiny_gps_.altitude.isUpdated()) { + this->altitude_ = this->tiny_gps_.altitude.meters(); ESP_LOGD(TAG, "Altitude: %.2f m", this->altitude_); } - if (tiny_gps_.satellites.isUpdated()) { - this->satellites_ = tiny_gps_.satellites.value(); + + if (this->tiny_gps_.satellites.isUpdated()) { + this->satellites_ = this->tiny_gps_.satellites.value(); ESP_LOGD(TAG, "Satellites: %d", this->satellites_); } - if (tiny_gps_.hdop.isUpdated()) { - this->hdop_ = tiny_gps_.hdop.hdop(); + + if (this->tiny_gps_.hdop.isUpdated()) { + this->hdop_ = this->tiny_gps_.hdop.hdop(); ESP_LOGD(TAG, "HDOP: %.3f", this->hdop_); } - for (auto *listener : this->listeners_) + for (auto *listener : this->listeners_) { listener->on_update(this->tiny_gps_); + } } } } diff --git a/esphome/components/gps/gps.h b/esphome/components/gps/gps.h index a400820738..7bc23ed1e0 100644 --- a/esphome/components/gps/gps.h +++ b/esphome/components/gps/gps.h @@ -5,7 +5,7 @@ #include "esphome/core/component.h" #include "esphome/components/uart/uart.h" #include "esphome/components/sensor/sensor.h" -#include +#include #include @@ -27,13 +27,13 @@ class GPSListener { class GPS : public PollingComponent, public uart::UARTDevice { public: - void set_latitude_sensor(sensor::Sensor *latitude_sensor) { latitude_sensor_ = latitude_sensor; } - void set_longitude_sensor(sensor::Sensor *longitude_sensor) { longitude_sensor_ = longitude_sensor; } - void set_speed_sensor(sensor::Sensor *speed_sensor) { speed_sensor_ = speed_sensor; } - void set_course_sensor(sensor::Sensor *course_sensor) { course_sensor_ = course_sensor; } - void set_altitude_sensor(sensor::Sensor *altitude_sensor) { altitude_sensor_ = altitude_sensor; } - void set_satellites_sensor(sensor::Sensor *satellites_sensor) { satellites_sensor_ = satellites_sensor; } - void set_hdop_sensor(sensor::Sensor *hdop_sensor) { hdop_sensor_ = hdop_sensor; } + void set_latitude_sensor(sensor::Sensor *latitude_sensor) { this->latitude_sensor_ = latitude_sensor; } + void set_longitude_sensor(sensor::Sensor *longitude_sensor) { this->longitude_sensor_ = longitude_sensor; } + void set_speed_sensor(sensor::Sensor *speed_sensor) { this->speed_sensor_ = speed_sensor; } + void set_course_sensor(sensor::Sensor *course_sensor) { this->course_sensor_ = course_sensor; } + void set_altitude_sensor(sensor::Sensor *altitude_sensor) { this->altitude_sensor_ = altitude_sensor; } + void set_satellites_sensor(sensor::Sensor *satellites_sensor) { this->satellites_sensor_ = satellites_sensor; } + void set_hdop_sensor(sensor::Sensor *hdop_sensor) { this->hdop_sensor_ = hdop_sensor; } void register_listener(GPSListener *listener) { listener->parent_ = this; @@ -41,19 +41,20 @@ class GPS : public PollingComponent, public uart::UARTDevice { } float get_setup_priority() const override { return setup_priority::HARDWARE; } + void dump_config() override; void loop() override; void update() override; TinyGPSPlus &get_tiny_gps() { return this->tiny_gps_; } protected: - float latitude_ = NAN; - float longitude_ = NAN; - float speed_ = NAN; - float course_ = NAN; - float altitude_ = NAN; - int satellites_ = 0; - double hdop_ = NAN; + float latitude_{NAN}; + float longitude_{NAN}; + float speed_{NAN}; + float course_{NAN}; + float altitude_{NAN}; + uint16_t satellites_{0}; + float hdop_{NAN}; sensor::Sensor *latitude_sensor_{nullptr}; sensor::Sensor *longitude_sensor_{nullptr}; diff --git a/platformio.ini b/platformio.ini index ccfd52c3ca..292188c6fa 100644 --- a/platformio.ini +++ b/platformio.ini @@ -64,7 +64,7 @@ lib_deps = heman/AsyncMqttClient-esphome@1.0.0 ; mqtt esphome/ESPAsyncWebServer-esphome@3.3.0 ; web_server_base fastled/FastLED@3.9.16 ; fastled_base - mikalhart/TinyGPSPlus@1.0.2 ; gps + mikalhart/TinyGPSPlus@1.1.0 ; gps freekode/TM1651@1.0.1 ; tm1651 glmnet/Dsmr@0.7 ; dsmr rweather/Crypto@0.4.0 ; dsmr diff --git a/tests/components/gps/common.yaml b/tests/components/gps/common.yaml index fc8228c909..53dc67e457 100644 --- a/tests/components/gps/common.yaml +++ b/tests/components/gps/common.yaml @@ -6,6 +6,20 @@ uart: parity: EVEN gps: + latitude: + name: "Latitude" + longitude: + name: "Longitude" + altitude: + name: "Altitude" + speed: + name: "Speed" + course: + name: "Course" + satellites: + name: "Satellites" + hdop: + name: "HDOP" time: - platform: gps From eebdc9c38fc69ef611dd2fc1929a4b052ba4695d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 15 May 2025 10:51:50 -0500 Subject: [PATCH 4/4] Fix ESP32 console logging corruption and message loss in multi-task environments These changes enhance ESPHome's logging system on ESP32 multi-task environments: 1. **Emergency Console Logging**: - Added fallback console logging when the task log buffer is full or disabled - Ensures critical messages are still visible even when the ring buffer fails 2. **Improved Console Output**: - Messages successfully sent to the ring buffer now also display on the console - Ensures consistent console output for all log messages regardless of source 3. **Optimized Resource Usage**: - Release ring buffer messages earlier after transferring to tx_buffer - Reduces contention for the shared log buffer in multi-task environments 1. **Stack Memory Efficiency**: - No longer need to allocate stack memory for console output when ring buffer is available - Only uses stack memory for emergency fallback cases, reducing stack usage in normal operation 2. **Console Output Integrity**: - Prevents console output corruption that could occur with concurrent writes from multiple tasks - Serializes all console output through the main loop when possible 3. **Message Ordering**: - Messages from different tasks may appear slightly out of order due to async delivery to main loop - This trade-off is preferable to corrupted console output from concurrent writes These improvements provide more reliable logging behavior, particularly under memory constraints or high logging volume, while maintaining thread safety and minimizing resource contention. --- esphome/components/logger/logger.cpp | 44 +++++++++++++++++++--------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/esphome/components/logger/logger.cpp b/esphome/components/logger/logger.cpp index 0ad909cb07..014f7e3dec 100644 --- a/esphome/components/logger/logger.cpp +++ b/esphome/components/logger/logger.cpp @@ -16,9 +16,14 @@ static const char *const TAG = "logger"; #ifdef USE_ESP32 // Implementation for ESP32 (multi-task platform with task-specific tracking) // Main task always uses direct buffer access for console output and callbacks -// Other tasks: -// - With task log buffer: stack buffer for console output, async buffer for callbacks -// - Without task log buffer: only console output, no callbacks +// +// For non-main tasks: +// - WITH task log buffer: Prefer sending to ring buffer for async processing +// - Avoids allocating stack memory for console output in normal operation +// - Prevents console corruption from concurrent writes by multiple tasks +// - Messages are serialized through main loop for proper console output +// - Fallback to emergency console logging only if ring buffer is full +// - WITHOUT task log buffer: Only emergency console output, no callbacks void HOT Logger::log_vprintf_(int level, const char *tag, int line, const char *format, va_list args) { // NOLINT if (level > this->level_for(tag)) return; @@ -38,8 +43,18 @@ void HOT Logger::log_vprintf_(int level, const char *tag, int line, const char * return; } - // For non-main tasks: use stack-allocated buffer only for console output - if (this->baud_rate_ > 0) { // If logging is enabled, write to console + bool message_sent = false; +#ifdef USE_ESPHOME_TASK_LOG_BUFFER + // For non-main tasks, queue the message for callbacks - but only if we have any callbacks registered + message_sent = this->log_buffer_->send_message_thread_safe(static_cast(level), tag, + static_cast(line), current_task, format, args); +#endif // USE_ESPHOME_TASK_LOG_BUFFER + + // Emergency console logging for non-main tasks when ring buffer is full or disabled + // This is a fallback mechanism to ensure critical log messages are visible + // Note: This may cause interleaved/corrupted console output if multiple tasks + // log simultaneously, but it's better than losing important messages entirely + if (!message_sent && this->baud_rate_ > 0) { // If logging is enabled, write to console // Maximum size for console log messages (includes null terminator) static const size_t MAX_CONSOLE_LOG_MSG_SIZE = 144; char console_buffer[MAX_CONSOLE_LOG_MSG_SIZE]; // MUST be stack allocated for thread safety @@ -49,15 +64,6 @@ void HOT Logger::log_vprintf_(int level, const char *tag, int line, const char * this->write_msg_(console_buffer); } -#ifdef USE_ESPHOME_TASK_LOG_BUFFER - // For non-main tasks, queue the message for callbacks - but only if we have any callbacks registered - if (this->log_callback_.size() > 0) { - // This will be processed in the main loop - this->log_buffer_->send_message_thread_safe(static_cast(level), tag, static_cast(line), - current_task, format, args); - } -#endif // USE_ESPHOME_TASK_LOG_BUFFER - // Reset the recursion guard for this task this->reset_task_log_recursion_(is_main_task); } @@ -184,7 +190,17 @@ void Logger::loop() { this->write_footer_to_buffer_(this->tx_buffer_, &this->tx_buffer_at_, this->tx_buffer_size_); this->tx_buffer_[this->tx_buffer_at_] = '\0'; this->call_log_callbacks_(message->level, message->tag, this->tx_buffer_); + // At this point all the data we need from message has been transferred to the tx_buffer + // so we can release the message to allow other tasks to use it as soon as possible. this->log_buffer_->release_message_main_loop(received_token); + + // Write to console from the main loop to prevent corruption from concurrent writes + // This ensures all log messages appear on the console in a clean, serialized manner + // Note: Messages may appear slightly out of order due to async processing, but + // this is preferred over corrupted/interleaved console output + if (this->baud_rate_ > 0) { + this->write_msg_(this->tx_buffer_); + } } } #endif