From 64127632f5aef589966e66f7538940860ca71e90 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 24 Sep 2026 13:11:50 +0100 Subject: [PATCH] [core][logger] Keep ESP8266 log strings in flash and deprecate the RAM option (#19577) --- esphome/components/logger/__init__.py | 21 ++++++++++++--- esphome/components/logger/log_buffer.h | 10 +++---- esphome/components/logger/logger.cpp | 7 +++-- esphome/components/logger/logger.h | 16 +++++------ esphome/components/mqtt/mqtt_lock.cpp | 2 +- esphome/components/mqtt/mqtt_sensor.cpp | 2 +- .../prometheus/prometheus_handler.cpp | 6 ++++- esphome/components/script/script.cpp | 2 +- esphome/components/script/script.h | 2 +- esphome/core/log.cpp | 2 +- esphome/core/log.h | 11 ++++---- esphome/core/progmem.h | 11 ++------ tests/component_tests/logger/test_logger.py | 27 +++++++++++++++++++ .../logger/test_logger_ram_log_strings.yaml | 9 +++++++ 14 files changed, 87 insertions(+), 41 deletions(-) create mode 100644 tests/component_tests/logger/test_logger_ram_log_strings.yaml diff --git a/esphome/components/logger/__init__.py b/esphome/components/logger/__init__.py index 138db75ad1..eb1f34565a 100644 --- a/esphome/components/logger/__init__.py +++ b/esphome/components/logger/__init__.py @@ -1,3 +1,4 @@ +import logging import re from typing import Any @@ -63,6 +64,8 @@ from esphome.core import CORE, ID, CoroPriority, Lambda, coroutine_with_priority from esphome.cpp_generator import MockObj, TemplateArgsType from esphome.types import ConfigType +_LOGGER = logging.getLogger(__name__) + CODEOWNERS = ["@esphome/core"] logger_ns = cg.esphome_ns.namespace("logger") LOG_LEVELS = { @@ -105,6 +108,7 @@ DEFAULT = "DEFAULT" CONF_INITIAL_LEVEL = "initial_level" CONF_LOGGER_ID = "logger_id" +CONF_ESP8266_STORE_LOG_STRINGS_IN_FLASH = "esp8266_store_log_strings_in_flash" CONF_RUNTIME_TAG_LEVELS = "runtime_tag_levels" CONF_TASK_LOG_BUFFER_SIZE = "task_log_buffer_size" CONF_WAIT_FOR_CDC = "wait_for_cdc" @@ -219,6 +223,18 @@ def validate_initial_no_higher_than_global(config: ConfigType) -> ConfigType: return config +def warn_ram_log_strings(config: ConfigType) -> ConfigType: + # Remove before 2027.4.0 + if config.get(CONF_ESP8266_STORE_LOG_STRINGS_IN_FLASH) is False: + _LOGGER.warning( + "'%s: false' is ignored and will be rejected in 2027.4.0. Log format strings " + "always stay in flash now; copying them into RAM gave no speed gain and the " + "lost RAM caused crashes. Remove the option", + CONF_ESP8266_STORE_LOG_STRINGS_IN_FLASH, + ) + return config + + 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") @@ -232,7 +248,6 @@ LoggerMessageTrigger = logger_ns.class_( ) -CONF_ESP8266_STORE_LOG_STRINGS_IN_FLASH = "esp8266_store_log_strings_in_flash" CONFIG_SCHEMA = cv.All( cv.Schema( { @@ -332,6 +347,7 @@ CONFIG_SCHEMA = cv.All( validate_local_no_higher_than_global, validate_initial_no_higher_than_global, validate_wait_for_cdc, + warn_ram_log_strings, ) @@ -450,9 +466,6 @@ async def _late_logger_init(config: ConfigType) -> None: cg.add_build_flag("-DCORE_DEBUG_LEVEL=5") if CORE.is_esp32 and is_at_least_very_verbose: cg.add_build_flag("-DENABLE_I2C_DEBUG_BUFFER") - if config.get(CONF_ESP8266_STORE_LOG_STRINGS_IN_FLASH): - cg.add_build_flag("-DUSE_STORE_LOG_STR_IN_FLASH") - if CORE.is_esp32: if config[CONF_HARDWARE_UART] == USB_CDC: add_idf_sdkconfig_option("CONFIG_ESP_CONSOLE_USB_CDC", True) diff --git a/esphome/components/logger/log_buffer.h b/esphome/components/logger/log_buffer.h index 067ce04114..14121d3dd9 100644 --- a/esphome/components/logger/log_buffer.h +++ b/esphome/components/logger/log_buffer.h @@ -105,9 +105,9 @@ struct LogBuffer { this->format_vsnprintf_(format, args); this->finalize_(); } -#ifdef USE_STORE_LOG_STR_IN_FLASH - void HOT format_body_P(PGM_P format, va_list args) { - this->format_vsnprintf_P_(format, args); +#ifdef USE_ESP8266 + void HOT format_body_p(PGM_P format, va_list args) { + this->format_vsnprintf_p_(format, args); this->finalize_(); } #endif @@ -158,8 +158,8 @@ struct LogBuffer { return; this->process_vsnprintf_result_(vsnprintf(this->current_(), this->remaining_(), format, args)); } -#ifdef USE_STORE_LOG_STR_IN_FLASH - void format_vsnprintf_P_(PGM_P format, va_list args) { +#ifdef USE_ESP8266 + void format_vsnprintf_p_(PGM_P format, va_list args) { if (this->full_()) return; this->process_vsnprintf_result_(vsnprintf_P(this->current_(), this->remaining_(), format, args)); diff --git a/esphome/components/logger/logger.cpp b/esphome/components/logger/logger.cpp index bfc005070e..757ba6771e 100644 --- a/esphome/components/logger/logger.cpp +++ b/esphome/components/logger/logger.cpp @@ -127,9 +127,8 @@ void HOT Logger::log_vprintf_(uint8_t level, const char *tag, int line, const ch } #endif // USE_ESP32 || USE_HOST || USE_LIBRETINY || USE_ZEPHYR -#ifdef USE_STORE_LOG_STR_IN_FLASH -// Implementation for ESP8266 with flash string support. -// Note: USE_STORE_LOG_STR_IN_FLASH is only defined for ESP8266. +#ifdef USE_ESP8266 +// ESP8266 keeps log format strings in flash. // // This function handles format strings stored in flash memory (PROGMEM) to save RAM. // Uses vsnprintf_P to read the format string directly from flash without copying to RAM. @@ -141,7 +140,7 @@ void Logger::log_vprintf_(uint8_t level, const char *tag, int line, const __Flas this->log_message_to_buffer_and_send_(global_recursion_guard_, level, tag, line, format, args, nullptr); } -#endif // USE_STORE_LOG_STR_IN_FLASH +#endif // USE_ESP8266 inline uint8_t Logger::level_for(const char *tag) { #ifdef USE_LOGGER_RUNTIME_TAG_LEVELS diff --git a/esphome/components/logger/logger.h b/esphome/components/logger/logger.h index ae55f4145a..6bdd5910f6 100644 --- a/esphome/components/logger/logger.h +++ b/esphome/components/logger/logger.h @@ -200,9 +200,9 @@ class Logger final : public Component { float get_setup_priority() const override { return setup_priority::BUS + 500.0f; } void log_vprintf_(uint8_t level, const char *tag, int line, const char *format, va_list args); // NOLINT -#ifdef USE_STORE_LOG_STR_IN_FLASH - void log_vprintf_(uint8_t level, const char *tag, int line, const __FlashStringHelper *format, - va_list args); // NOLINT +#ifdef USE_ESP8266 + // NOLINTNEXTLINE(readability-identifier-naming) + void log_vprintf_(uint8_t level, const char *tag, int line, const __FlashStringHelper *format, va_list args); #endif protected: @@ -244,14 +244,14 @@ class Logger final : public Component { buf.format_body(format, args); } -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 // Format a log message with flash string format and write it to a buffer with header, footer, and null terminator // ESP8266-only (single-task), thread_name is always nullptr - inline void HOT format_log_to_buffer_with_terminator_P_(uint8_t level, const char *tag, int line, + inline void HOT format_log_to_buffer_with_terminator_p_(uint8_t level, const char *tag, int line, const __FlashStringHelper *format, va_list args, LogBuffer &buf) { buf.write_header(level, tag, line, nullptr); - buf.format_body_P(reinterpret_cast(format), args); + buf.format_body_p(reinterpret_cast(format), args); } #endif @@ -283,9 +283,9 @@ class Logger final : public Component { FormatType format, va_list args, const char *thread_name) { RecursionGuard guard(recursion_guard); LogBuffer buf{this->tx_buffer_, ESPHOME_LOGGER_TX_BUFFER_SIZE}; -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 if constexpr (std::is_same_v) { - this->format_log_to_buffer_with_terminator_P_(level, tag, line, format, args, buf); + this->format_log_to_buffer_with_terminator_p_(level, tag, line, format, args, buf); } else #endif { diff --git a/esphome/components/mqtt/mqtt_lock.cpp b/esphome/components/mqtt/mqtt_lock.cpp index 7920187f92..d555358573 100644 --- a/esphome/components/mqtt/mqtt_lock.cpp +++ b/esphome/components/mqtt/mqtt_lock.cpp @@ -50,7 +50,7 @@ bool MQTTLockComponent::send_initial_state() { return this->publish_state(); } bool MQTTLockComponent::publish_state() { char topic_buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 char buf[LOCK_STATE_STR_SIZE]; strncpy_P(buf, (PGM_P) lock_state_to_string(this->lock_->state), sizeof(buf) - 1); buf[sizeof(buf) - 1] = '\0'; diff --git a/esphome/components/mqtt/mqtt_sensor.cpp b/esphome/components/mqtt/mqtt_sensor.cpp index 1c0625d1c9..0fa30ef937 100644 --- a/esphome/components/mqtt/mqtt_sensor.cpp +++ b/esphome/components/mqtt/mqtt_sensor.cpp @@ -59,7 +59,7 @@ void MQTTSensorComponent::send_discovery(JsonObject root, mqtt::SendDiscoveryCon root[MQTT_FORCE_UPDATE] = true; if (this->sensor_->get_state_class() != STATE_CLASS_NONE) { -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 root[MQTT_STATE_CLASS] = (const __FlashStringHelper *) state_class_to_string(this->sensor_->get_state_class()); #else root[MQTT_STATE_CLASS] = LOG_STR_ARG(state_class_to_string(this->sensor_->get_state_class())); diff --git a/esphome/components/prometheus/prometheus_handler.cpp b/esphome/components/prometheus/prometheus_handler.cpp index 0412d8a842..26b7c6ea80 100644 --- a/esphome/components/prometheus/prometheus_handler.cpp +++ b/esphome/components/prometheus/prometheus_handler.cpp @@ -903,7 +903,7 @@ void PrometheusHandler::valve_row_(AsyncResponseStream *stream, valve::Valve *ob stream->print(ESPHOME_F("\",name=\"")); stream->print(relabel_name_(obj).c_str()); stream->print(ESPHOME_F("\",operation=\"")); -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 stream->print((const __FlashStringHelper *) valve::valve_operation_to_str(obj->current_operation)); #else stream->print((const char *) valve::valve_operation_to_str(obj->current_operation)); @@ -947,7 +947,11 @@ void PrometheusHandler::climate_setting_row_(AsyncResponseStream *stream, climat stream->print(ESPHOME_F("\",category=\"")); stream->print(setting.c_str()); stream->print(ESPHOME_F("\",setting_value=\"")); +#ifdef USE_ESP8266 + stream->print((const __FlashStringHelper *) setting_value); +#else stream->print(LOG_STR_ARG(setting_value)); +#endif stream->print(ESPHOME_F("\"} ")); stream->print(ESPHOME_F("1.0")); stream->print(ESPHOME_F("\n")); diff --git a/esphome/components/script/script.cpp b/esphome/components/script/script.cpp index 61bca5bc28..6a35684da8 100644 --- a/esphome/components/script/script.cpp +++ b/esphome/components/script/script.cpp @@ -5,7 +5,7 @@ namespace esphome::script { static const char *const TAG = "script"; -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 void ScriptLogger::esp_log_(int level, int line, const __FlashStringHelper *format, const char *param) { esp_log_printf_(level, TAG, line, format, param); } diff --git a/esphome/components/script/script.h b/esphome/components/script/script.h index 63d0ff7cb3..5e66899953 100644 --- a/esphome/components/script/script.h +++ b/esphome/components/script/script.h @@ -13,7 +13,7 @@ namespace esphome::script { class ScriptLogger { protected: -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 void esp_logw_(int line, const __FlashStringHelper *format, const char *param) { esp_log_(ESPHOME_LOG_LEVEL_WARN, line, format, param); } diff --git a/esphome/core/log.cpp b/esphome/core/log.cpp index 9fcddfeff6..2f1d568758 100644 --- a/esphome/core/log.cpp +++ b/esphome/core/log.cpp @@ -36,7 +36,7 @@ void HOT esp_log_printf_(int level, const char *tag, int line, const char *forma #endif } -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 void HOT esp_log_printf_(int level, const char *tag, int line, const __FlashStringHelper *format, ...) { #ifdef USE_LOGGER ESPHOME_DEBUG_ASSERT(logger::global_logger != nullptr); diff --git a/esphome/core/log.h b/esphome/core/log.h index 14d24412ef..362ed83b91 100644 --- a/esphome/core/log.h +++ b/esphome/core/log.h @@ -16,7 +16,7 @@ #include #include -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 #include "WString.h" #endif @@ -63,7 +63,8 @@ namespace esphome { void esp_log_printf_(int level, const char *tag, int line, const char *format, ...) // NOLINT __attribute__((format(printf, 4, 5))); -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 +// NOLINTNEXTLINE(readability-identifier-naming) void esp_log_printf_(int level, const char *tag, int line, const __FlashStringHelper *format, ...); #endif void esp_log_vprintf_(int level, const char *tag, int line, const char *format, va_list args); // NOLINT @@ -71,7 +72,7 @@ void esp_log_vprintf_(int level, const char *tag, int line, const char *format, int esp_idf_log_vprintf_(const char *format, va_list args); // NOLINT #endif -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 #define ESPHOME_LOG_FORMAT(format) F(format) #else #define ESPHOME_LOG_FORMAT(format) format @@ -172,7 +173,7 @@ int esp_idf_log_vprintf_(const char *format, va_list args); // NOLINT // Helper class that identifies strings that may be stored in flash storage (similar to Arduino's __FlashStringHelper) struct LogString; -#ifdef USE_STORE_LOG_STR_IN_FLASH +#ifdef USE_ESP8266 #include @@ -181,7 +182,7 @@ struct LogString; #define LOG_STR(s) (reinterpret_cast(PSTR(s))) #define LOG_STR_LITERAL(s) LOG_STR_ARG(LOG_STR(s)) -#else // !USE_STORE_LOG_STR_IN_FLASH +#else // !USE_ESP8266 #define LOG_STR(s) (reinterpret_cast(s)) #define LOG_STR_ARG(s) (reinterpret_cast(s)) diff --git a/esphome/core/progmem.h b/esphome/core/progmem.h index 28440fe838..54fc42dde1 100644 --- a/esphome/core/progmem.h +++ b/esphome/core/progmem.h @@ -27,17 +27,10 @@ #define ESPHOME_strlen_P strlen_P // Type for pointers to PROGMEM strings (for use with ESPHOME_F return values) using ProgmemStr = const __FlashStringHelper *; -// Storage class for PROGMEM_STRING_TABLE data. Mirrors the logger's choice of -// LOG_STR_ARG: when LOG_STR_ARG treats the LogString as PROGMEM (PGM_P), the -// table data must actually be in flash; when LOG_STR_ARG treats it as a plain -// const char* (assumes RAM), the table data must live in RAM or non-logger -// consumers (ArduinoJson, Print, MQTT publish) crash on unaligned flash reads. -#ifdef USE_STORE_LOG_STR_IN_FLASH +// Storage class for PROGMEM_STRING_TABLE data; LOG_STR_ARG treats a LogString as PGM_P +// on ESP8266, so the table data must be in flash to match. #define ESPHOME_PROGMEM_STRING_TABLE_STORAGE PROGMEM #else -#define ESPHOME_PROGMEM_STRING_TABLE_STORAGE -#endif -#else #define ESPHOME_F(string_literal) (string_literal) #define ESPHOME_PGM_P const char * #define ESPHOME_PSTR(s) (s) diff --git a/tests/component_tests/logger/test_logger.py b/tests/component_tests/logger/test_logger.py index 4ce30afb94..199d67ff5c 100644 --- a/tests/component_tests/logger/test_logger.py +++ b/tests/component_tests/logger/test_logger.py @@ -1,7 +1,13 @@ """Tests for the logger component.""" +from collections.abc import Callable +from pathlib import Path import re +import pytest + +from esphome.core import CORE + def test_logger_pre_setup_before_other_components(generate_main): """Logger::pre_setup() must be called before any other component is created. @@ -84,3 +90,24 @@ def test_libretiny_uart0_is_emitted(generate_main): ) assert "set_uart_selection(logger::UART_SELECTION_UART0);" in main_cpp + + +def test_ram_log_strings_is_deprecated( + generate_main: Callable[[str | Path], str], + caplog: pytest.LogCaptureFixture, +) -> None: + """`false` is accepted but ignored: no RAM build flag, and a warning with the removal version.""" + generate_main("tests/component_tests/logger/test_logger_ram_log_strings.yaml") + + assert not any("STORE_LOG_STR" in flag for flag in CORE.build_flags) + assert "esp8266_store_log_strings_in_flash: false' is ignored" in caplog.text + assert "2027.4.0" in caplog.text + + +def test_flash_log_strings_default_does_not_warn( + generate_main: Callable[[str | Path], str], + caplog: pytest.LogCaptureFixture, +) -> None: + generate_main("tests/component_tests/logger/test_logger.yaml") + + assert "esp8266_store_log_strings_in_flash" not in caplog.text diff --git a/tests/component_tests/logger/test_logger_ram_log_strings.yaml b/tests/component_tests/logger/test_logger_ram_log_strings.yaml new file mode 100644 index 0000000000..ef6a8e2da1 --- /dev/null +++ b/tests/component_tests/logger/test_logger_ram_log_strings.yaml @@ -0,0 +1,9 @@ +--- +esphome: + name: test + +esp8266: + board: d1_mini_lite + +logger: + esp8266_store_log_strings_in_flash: false