Reserve through esphome.preferences, share the RTC layout with the crash handler, fold the redundant checks

esphome/components/esp8266/__init__.py

@@ -53,14 +53,14 @@ from .const import (
     KEY_SERIAL1_REQUIRED,
     KEY_SERIAL_REQUIRED,
     KEY_WAVEFORM_REQUIRED,
-    PREFERENCES_FLASH_WORDS,
-    PREFERENCES_FLASH_WORDS_WITHOUT_RESTORE,
     PREFERENCES_RTC_EBOOT_WORDS,
     PREFERENCES_RTC_NORMAL_WORDS,
     THROW_STUBS_HEADER,
+    check_preferences_fit,
     enable_serial,
     enable_serial1,
     esp8266_ns,
+    preferences_flash_words,
 )
 from .gpio import PinInitialState, add_pin_initial_states_array
 
@@ -471,16 +471,14 @@ async def to_code(config: ConfigType) -> None:
     # Other projects like Tasmota & ESPEasy also use this
     cg.add_build_flag("-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH")
 
-    # Saved-settings layout from const.py, read by preferences.cpp
     cg.add_define("ESP8266_PREFERENCES_RTC_NORMAL_WORDS", PREFERENCES_RTC_NORMAL_WORDS)
     cg.add_define("ESP8266_PREFERENCES_RTC_EBOOT_WORDS", PREFERENCES_RTC_EBOOT_WORDS)
+    cg.add_define(
+        "ESP8266_PREFERENCES_FLASH_WORDS",
+        preferences_flash_words(config[CONF_RESTORE_FROM_FLASH]),
+    )
     if config[CONF_RESTORE_FROM_FLASH]:
         cg.add_define("USE_ESP8266_PREFERENCES_FLASH")
-        cg.add_define("ESP8266_PREFERENCES_FLASH_WORDS", PREFERENCES_FLASH_WORDS)
-    else:
-        cg.add_define(
-            "ESP8266_PREFERENCES_FLASH_WORDS", PREFERENCES_FLASH_WORDS_WITHOUT_RESTORE
-        )
 
     if config[CONF_EARLY_PIN_INIT]:
         cg.add_define("USE_ESP8266_EARLY_PIN_INIT")
@@ -540,6 +538,7 @@ async def to_code(config: ConfigType) -> None:
     CORE.add_job(add_pin_initial_states_array)
     CORE.add_job(finalize_waveform_config)
     CORE.add_job(finalize_serial_config)
+    CORE.add_job(check_preferences_fit)
 
 
 @coroutine_with_priority(CoroPriority.WORKAROUNDS)

esphome/components/esp8266/const.py

@@ -32,86 +32,74 @@ KEY_DECODE_TOOLS = "decode_tools"
 # RTC regions, and the flash store is smaller when only explicit flash preferences use it.
 PREFERENCES_RTC_NORMAL_WORDS = 78
 PREFERENCES_RTC_EBOOT_WORDS = 32
+PREFERENCES_RTC_WORDS = PREFERENCES_RTC_NORMAL_WORDS + PREFERENCES_RTC_EBOOT_WORDS
 PREFERENCES_FLASH_WORDS = 128
 PREFERENCES_FLASH_WORDS_WITHOUT_RESTORE = 64
-
-
 KEY_PREFERENCES = "preferences"
-# Words of RTC memory open to saved values; the crash handler keeps the rest
-PREFERENCES_RTC_WORDS = PREFERENCES_RTC_NORMAL_WORDS + PREFERENCES_RTC_EBOOT_WORDS
+
+
+def preferences_flash_words(restore_from_flash: bool) -> int:
+    """Size of the flash store for this configuration."""
+    return (
+        PREFERENCES_FLASH_WORDS
+        if restore_from_flash
+        else PREFERENCES_FLASH_WORDS_WITHOUT_RESTORE
+    )
 
 
 def preference_words(value_bytes: int) -> int:
-    """Words a saved value takes, checksum word included."""
+    """Words a saved value takes, checksum word included (rtc_pref_bytes_to_words plus one)."""
     return (value_bytes + 3) // 4 + 1
 
 
 def reserve_preferences(count: int, value_bytes: int, in_flash: bool = False) -> None:
-    """Record saved values a component creates, so all of them are checked to fit; call from to_code."""
-    if not CORE.is_esp8266:
-        return
+    """Record saved values a component creates; check_preferences_fit() adds them all up."""
     reservations = CORE.data.setdefault(KEY_ESP8266, {}).setdefault(KEY_PREFERENCES, [])
-    if not reservations:
-        CORE.add_job(check_preferences_fit)
-    reservations.append((count, preference_words(value_bytes), in_flash))
+    reservations.extend([(preference_words(value_bytes), in_flash)] * count)
 
 
 def _rtc_values_can_split(values: list[int]) -> bool:
-    """Whether the values can be split between the two RTC regions, whatever order the device fills them in."""
+    """Whether some order fills the two RTC regions without a value spanning them."""
     total = sum(values)
+    if total <= PREFERENCES_RTC_NORMAL_WORDS:
+        return True
     # Bit i of reachable is set when some subset of the values sums to i
     reachable = 1
     for words in values:
         reachable |= reachable << words
-    return any(
-        reachable >> normal & 1 and total - normal <= PREFERENCES_RTC_EBOOT_WORDS
-        for normal in range(min(total, PREFERENCES_RTC_NORMAL_WORDS) + 1)
-    )
+    lowest = max(0, total - PREFERENCES_RTC_EBOOT_WORDS)
+    highest = min(total, PREFERENCES_RTC_NORMAL_WORDS)
+    return any(reachable >> normal & 1 for normal in range(lowest, highest + 1))
 
 
 @coroutine_with_priority(CoroPriority.FINAL)
 async def check_preferences_fit() -> None:
     """Fail the build when the reserved saved values cannot fit the space preferences.cpp has."""
-    restore_from_flash = CORE.config[KEY_ESP8266].get(CONF_RESTORE_FROM_FLASH, False)
-    flash_store = (
-        PREFERENCES_FLASH_WORDS
-        if restore_from_flash
-        else PREFERENCES_FLASH_WORDS_WITHOUT_RESTORE
+    restore_from_flash = CORE.config[KEY_ESP8266][CONF_RESTORE_FROM_FLASH]
+    flash_store = preferences_flash_words(restore_from_flash)
+    reservations = CORE.data[KEY_ESP8266].get(KEY_PREFERENCES, [])
+    flash_words = sum(
+        w for w, in_flash in reservations if in_flash or restore_from_flash
     )
-    rtc_words = flash_words = 0
-    rtc_values: list[int] = []
-    for count, words, in_flash in CORE.data[KEY_ESP8266][KEY_PREFERENCES]:
-        if in_flash or restore_from_flash:
-            if words > flash_store:
-                raise EsphomeError(
-                    f"A saved value of {words} words does not fit the ESP8266 flash store of {flash_store} words"
-                )
-            flash_words += count * words
-        else:
-            # A value cannot span the two RTC regions, so the larger one bounds it
-            if words > PREFERENCES_RTC_NORMAL_WORDS:
-                raise EsphomeError(
-                    f"A saved value of {words} words does not fit an ESP8266 RTC region of "
-                    f"{PREFERENCES_RTC_NORMAL_WORDS} words"
-                )
-            rtc_words += count * words
-            rtc_values.extend([words] * count)
+    rtc_values = [
+        w for w, in_flash in reservations if not (in_flash or restore_from_flash)
+    ]
     if flash_words > flash_store:
         raise EsphomeError(
             f"Saved values need {flash_words} words but the ESP8266 flash store has {flash_store}; "
             "use fewer or smaller saved values"
         )
-    if rtc_words > PREFERENCES_RTC_WORDS or not _rtc_values_can_split(rtc_values):
+    if not _rtc_values_can_split(rtc_values):
         raise EsphomeError(
-            f"Saved values need {rtc_words} words but ESP8266 RTC memory has {PREFERENCES_RTC_WORDS} in regions of "
-            f"{PREFERENCES_RTC_NORMAL_WORDS} and {PREFERENCES_RTC_EBOOT_WORDS} that a value cannot span; "
-            "use fewer or smaller saved values, or set restore_from_flash: true"
+            f"Saved values need {sum(rtc_values)} words but ESP8266 RTC memory has {PREFERENCES_RTC_WORDS} in "
+            f"regions of {PREFERENCES_RTC_NORMAL_WORDS} and {PREFERENCES_RTC_EBOOT_WORDS} that a value cannot "
+            "span; use fewer or smaller saved values, or set restore_from_flash: true"
         )
-    if rtc_words > PREFERENCES_RTC_NORMAL_WORDS:
+    if sum(rtc_values) > PREFERENCES_RTC_NORMAL_WORDS:
         _LOGGER.warning(
-            "Saved values need %d of the %d words of ESP8266 RTC memory; values that straddle the two regions "
-            "and saved settings that are not counted here may not all fit",
-            rtc_words,
+            "Saved values reserve %d of %d words of ESP8266 RTC memory; the fit depends on setup order, and "
+            "saved settings that do not reserve are not counted",
+            sum(rtc_values),
             PREFERENCES_RTC_WORDS,
         )
 

esphome/components/esp8266/crash_handler.cpp

@@ -4,6 +4,7 @@
 #ifdef USE_ESP8266_CRASH_HANDLER
 
 #include "crash_handler.h"
+#include "rtc_layout.h"
 #include "esphome/core/log.h"
 
 #include <cinttypes>
@@ -46,11 +47,8 @@ static inline bool IRAM_ATTR is_code_addr(uint32_t val) {
 // Recover the actual code address from a windowed-ABI return address on the stack.
 static inline uint32_t IRAM_ATTR recover_code_addr(uint32_t val) { return (val & XTENSA_ADDR_MASK) | XTENSA_CODE_BASE; }
 
-// RTC user memory layout for crash backtrace data.
-// User-accessible RTC memory: blocks 64-191 (each block = 4 bytes).
-// We use blocks 174-191 (last 18 blocks, 72 bytes) to minimize conflicts.
+// Crash data lives in the last esphome::esp8266::RTC_CRASH_WORDS of user RTC memory (rtc_layout.h).
 // Store 16 raw candidates, filter to real return addresses at log time.
-static constexpr uint8_t RTC_CRASH_BASE = 174;
 static constexpr size_t MAX_BACKTRACE = 16;
 
 // Magic word packs sentinel, version, and count into one uint32_t:
@@ -78,7 +76,7 @@ struct RtcCrashData {
   uint32_t backtrace[MAX_BACKTRACE];
   uint32_t epc1;  // Fault PC, used to filter duplicates
 };
-static_assert(sizeof(RtcCrashData) == 72, "RtcCrashData must fit in 18 RTC blocks");
+static_assert(sizeof(RtcCrashData) == esphome::esp8266::RTC_CRASH_WORDS * 4, "RtcCrashData must fill its RTC words");
 
 namespace esphome::esp8266 {
 
@@ -129,7 +127,7 @@ static const LogString *get_reset_reason(uint32_t reason) {
 // Returns the number of valid backtrace entries (0 if no data found).
 static uint8_t read_rtc_backtrace(uint32_t *backtrace, size_t max_entries) {
   RtcCrashData rtc_data;
-  if (!system_rtc_mem_read(RTC_CRASH_BASE, &rtc_data, sizeof(rtc_data)))
+  if (!system_rtc_mem_read(esphome::esp8266::RTC_CRASH_BASE, &rtc_data, sizeof(rtc_data)))
     return 0;
   uint32_t magic = rtc_data.magic;
   if ((magic & CRASH_SENTINEL_MASK) != CRASH_SENTINEL || (magic & CRASH_VERSION_MASK) != CRASH_VERSION)
@@ -233,7 +231,7 @@ extern "C" void IRAM_ATTR custom_crash_callback(struct rst_info *rst_info, uint3
   data.epc1 = epc1;
   data.magic = CRASH_SENTINEL | CRASH_VERSION | count;
 
-  system_rtc_mem_write(RTC_CRASH_BASE, &data, sizeof(data));
+  system_rtc_mem_write(esphome::esp8266::RTC_CRASH_BASE, &data, sizeof(data));
 }
 
 #endif  // USE_ESP8266_CRASH_HANDLER

esphome/components/esp8266/preferences.cpp

@@ -6,6 +6,7 @@ extern "C" {
 }
 
 #include "preferences.h"
+#include "rtc_layout.h"
 #include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 #include "esphome/core/preferences_rtc.h"
@@ -17,7 +18,7 @@ namespace esphome::esp8266 {
 ESPHOME_LOG_TAG(TAG, "preferences");
 
 static constexpr uint32_t ESP_RTC_USER_MEM_START = 0x60001200;
-static constexpr uint32_t ESP_RTC_USER_MEM_SIZE_WORDS = 128;
+static constexpr uint32_t ESP_RTC_USER_MEM_SIZE_WORDS = RTC_USER_MEM_WORDS;
 static constexpr uint32_t ESP_RTC_USER_MEM_SIZE_BYTES = ESP_RTC_USER_MEM_SIZE_WORDS * 4;
 
 // RTC memory layout (sizes come from const.py through codegen):
@@ -27,8 +28,7 @@ static constexpr uint32_t ESP_RTC_USER_MEM_SIZE_BYTES = ESP_RTC_USER_MEM_SIZE_WO
 static constexpr uint32_t RTC_EBOOT_REGION_WORDS = ESP8266_PREFERENCES_RTC_EBOOT_WORDS;
 static constexpr uint32_t RTC_NORMAL_REGION_WORDS = ESP8266_PREFERENCES_RTC_NORMAL_WORDS;
 static constexpr uint32_t PREF_TOTAL_WORDS = RTC_EBOOT_REGION_WORDS + RTC_NORMAL_REGION_WORDS;  // 110
-static constexpr uint32_t RTC_CRASH_HANDLER_WORDS = 18;  // Words 110-127, see crash_handler.cpp
-static_assert(PREF_TOTAL_WORDS + RTC_CRASH_HANDLER_WORDS <= ESP_RTC_USER_MEM_SIZE_WORDS,
+static_assert(PREF_TOTAL_WORDS <= RTC_CRASH_BASE - RTC_SYSTEM_USER_BASE,
               "preference regions overlap the crash handler's RTC words");
 
 // Maximum preference size in words (limited by uint8_t length_words field)

esphome/components/esp8266/rtc_layout.h

@@ -0,0 +1,18 @@
+#pragma once
+
+#ifdef USE_ESP8266
+
+#include <cstdint>
+
+namespace esphome::esp8266 {
+
+// User RTC memory is system blocks 64-191, one 4-byte word each. preferences.cpp owns the first
+// words, crash_handler.cpp the last; const.py hands preferences.cpp its region sizes.
+static constexpr uint32_t RTC_SYSTEM_USER_BASE = 64;
+static constexpr uint32_t RTC_USER_MEM_WORDS = 128;
+static constexpr uint32_t RTC_CRASH_WORDS = 18;
+static constexpr uint32_t RTC_CRASH_BASE = RTC_SYSTEM_USER_BASE + RTC_USER_MEM_WORDS - RTC_CRASH_WORDS;
+
+}  // namespace esphome::esp8266
+
+#endif  // USE_ESP8266

esphome/preferences.py

@@ -102,3 +102,11 @@ def is_in_flash(value: str) -> bool:
     if not in_flash:
         request_rtc_storage()
     return in_flash
+
+
+def reserve_preferences(count: int, value_bytes: int, in_flash: bool = False) -> None:
+    """Reserve saved values from to_code so a platform with a small store can check they all fit."""
+    if CORE.is_esp8266:
+        from esphome.components.esp8266.const import reserve_preferences as reserve
+
+        reserve(count, value_bytes, in_flash)

tests/unit_tests/components/esp8266/test_preferences.py

@@ -4,6 +4,7 @@ import asyncio
 
 import pytest
 
+from esphome import preferences
 from esphome.components.esp8266.const import (
     KEY_ESP8266,
     KEY_PREFERENCES,
@@ -35,35 +36,27 @@ def test_preference_words_rounds_up_and_adds_the_checksum(
     assert preference_words(value_bytes) == words
 
 
-def test_reservations_from_several_components_add_up() -> None:
-    reserve_preferences(2, 94)  # 50 words
-    reserve_preferences(2, 94)  # another component, 100 in total
+@pytest.mark.parametrize(
+    ("restore_from_flash", "in_flash", "fitting", "match"),
+    [
+        # 25 words each: 3 in the 78-word region and 1 in the 32-word one
+        (False, False, 4, "RTC memory has 110"),
+        # 125 of 128 flash words
+        (True, False, 5, "flash store has 128"),
+        (True, True, 5, "flash store has 128"),
+        # explicit flash values without restore_from_flash use the 64-word store
+        (False, True, 2, "flash store has 64"),
+    ],
+)
+def test_reservations_add_up_against_the_store_they_land_in(
+    restore_from_flash: bool, in_flash: bool, fitting: int, match: str
+) -> None:
+    CORE.config[KEY_ESP8266]["restore_from_flash"] = restore_from_flash
+    reserve_preferences(fitting - 1, 94, in_flash)  # one component
+    reserve_preferences(1, 94, in_flash)  # another component
     _check()
-    reserve_preferences(1, 94)  # 125 > 110
-    with pytest.raises(EsphomeError, match="RTC memory has 110"):
-        _check()
-
-
-def test_a_value_cannot_exceed_the_larger_rtc_region() -> None:
-    reserve_preferences(1, 400)
-    with pytest.raises(EsphomeError, match="RTC region"):
-        _check()
-
-
-def test_flash_store_follows_restore_from_flash() -> None:
-    CORE.config[KEY_ESP8266]["restore_from_flash"] = True
-    reserve_preferences(5, 94)  # 125 of 128 words
-    _check()
-    reserve_preferences(1, 94)
-    with pytest.raises(EsphomeError, match="flash store has 128"):
-        _check()
-
-
-def test_explicit_flash_values_use_the_small_store_without_restore() -> None:
-    reserve_preferences(2, 94, in_flash=True)  # 50 of 64 words
-    _check()
-    reserve_preferences(1, 94, in_flash=True)
-    with pytest.raises(EsphomeError, match="flash store has 64"):
+    reserve_preferences(1, 94, in_flash)
+    with pytest.raises(EsphomeError, match=match):
         _check()
 
 
@@ -74,6 +67,12 @@ def test_values_that_cannot_split_between_the_regions_fail() -> None:
         _check()
 
 
+def test_a_value_larger_than_either_region_fails() -> None:
+    reserve_preferences(1, 400)
+    with pytest.raises(EsphomeError, match="cannot span"):
+        _check()
+
+
 def test_a_split_that_works_in_some_order_passes_with_a_warning(
     caplog: pytest.LogCaptureFixture,
 ) -> None:
@@ -81,16 +80,25 @@ def test_a_split_that_works_in_some_order_passes_with_a_warning(
     reserve_preferences(1, 308)
     reserve_preferences(1, 124)
     _check()
-    assert "may not all fit" in caplog.text
+    assert "depends on setup order" in caplog.text
 
 
-def test_the_check_runs_from_the_scheduled_job() -> None:
+def test_nothing_reserved_passes_quietly(caplog: pytest.LogCaptureFixture) -> None:
+    _check()
+    assert caplog.text == ""
+
+
+def test_the_check_runs_as_a_scheduled_job() -> None:
     reserve_preferences(3, 400)
-    with pytest.raises(EsphomeError, match="RTC region"):
+    CORE.add_job(check_preferences_fit)
+    with pytest.raises(EsphomeError, match="cannot span"):
         CORE.flush_tasks()
 
 
-def test_other_platforms_reserve_nothing() -> None:
+def test_components_reserve_through_esphome_preferences() -> None:
+    preferences.reserve_preferences(2, 94, in_flash=True)
+    assert CORE.data[KEY_ESP8266][KEY_PREFERENCES] == [(25, True), (25, True)]
     CORE.data[KEY_CORE] = {KEY_TARGET_PLATFORM: "esp32"}
-    reserve_preferences(100, 400)
+    CORE.data[KEY_ESP8266] = {}
+    preferences.reserve_preferences(2, 94)
     assert KEY_PREFERENCES not in CORE.data[KEY_ESP8266]