diff --git a/esphome/core/defines.h b/esphome/core/defines.h index bfb019d7ae..49b9583be3 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -157,9 +157,17 @@ #define USE_OUTPUT_FLOAT_POWER_SCALING #define USE_POWER_SUPPLY #define USE_PREFERENCES_SYNC_EVERY_LOOP -// Only defined by key-lookup preference backends (esp32, libretiny, host, zephyr); -// slot-based platforms (esp8266, rp2040) never set it in generated builds +// Only defined by key-lookup preference backends; the slot-based platforms +// (esp8266, rp2040) never set it in generated builds, and their preferences +// managers do not provide load_from_key(), so the PreferencesKeyLookupContract +// assert would fail their clang-tidy environments. Written as a deny-list so +// the no-platform analysis configuration (whose Preferences stub provides +// load_from_key()) keeps covering the key-lookup code paths, and so a future +// slot-based platform fails the assert loudly instead of silently losing +// analysis coverage. +#if !defined(USE_ESP8266) && !defined(USE_RP2) #define USE_PREFERENCE_KEY_LOOKUP +#endif #define USE_PROVISIONING #define USE_QR_CODE #define USE_SAFE_MODE_BOOT_IS_GOOD_ON_SHUTDOWN diff --git a/esphome/core/preference_backend.h b/esphome/core/preference_backend.h index b9bb9a0252..0622376fca 100644 --- a/esphome/core/preference_backend.h +++ b/esphome/core/preference_backend.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include "esphome/core/defines.h" @@ -30,6 +31,15 @@ namespace esphome { +// The PreferenceBackend method surface, asserted on the alias each platform +// header binds. save() persists len bytes; load() fills dest only when the +// stored data exists and matches len. Both report success as their return. +template +concept PreferenceBackendContract = requires(T backend, const uint8_t *src, uint8_t *dest, size_t len) { + { backend.save(src, len) } -> std::same_as; + { backend.load(dest, len) } -> std::same_as; +}; + #if !defined(USE_ESP32) && !defined(USE_ESP8266) && !defined(USE_RP2) && !defined(USE_LIBRETINY) && \ !defined(USE_HOST) && !(defined(USE_ZEPHYR) && defined(CONFIG_SETTINGS)) // Stub for static analysis when no platform is defined. @@ -40,6 +50,8 @@ struct PreferenceBackend { #endif using ESPPreferenceBackend = PreferenceBackend; +static_assert(PreferenceBackendContract, + "The platform's preference backend is missing part of the PreferenceBackend surface"); class ESPPreferenceObject { public: @@ -68,6 +80,39 @@ class ESPPreferenceObject { PreferenceBackend *backend_{nullptr}; }; +// The preferences manager method surface, asserted in esphome/core/preferences.h +// on the ESPPreferences alias each platform's preferences.h binds through +// DECLARE_PREFERENCE_ALIASES. Semantics beyond the signatures: +// - make_preference: the two-argument form applies the platform's historic +// default storage; in_flash=false may fall back to flash where the platform +// has no faster storage. +// - sync: commit pending writes to flash, true on success. +// - reset: forget unsaved changes and re-initialize the permanent storage +// (usually followed by a restart), true on success. +// The template forms are what component call sites use; PreferencesMixin +// supplies them, but the derived class's non-template overloads hide them +// unless it also declares `using PreferencesMixin::make_preference;`, so +// the concept pins those too. +template +concept PreferencesContract = requires(T prefs, size_t len, uint32_t type, bool in_flash) { + { prefs.make_preference(len, type, in_flash) } -> std::same_as; + { prefs.make_preference(len, type) } -> std::same_as; + { prefs.template make_preference(type, in_flash) } -> std::same_as; + { prefs.template make_preference(type) } -> std::same_as; + { prefs.sync() } -> std::same_as; + { prefs.reset() } -> std::same_as; +}; + +// Key-lookup platforms additionally provide load_from_key(), a one-shot read +// of a stored preference by key that migrate_preference() relies on; see the +// key-lookup note at the top of this file. Not part of PreferencesContract, +// so it is asserted in preferences.h only where USE_PREFERENCE_KEY_LOOKUP +// is set. +template +concept PreferencesKeyLookupContract = requires(T prefs, uint32_t type, uint8_t *data, size_t len) { + { prefs.load_from_key(type, data, len) } -> std::same_as; +}; + /// CRTP mixin providing type-safe template make_preference() helpers. /// Platform preferences classes inherit this to avoid duplicating these templates. template class PreferencesMixin { diff --git a/esphome/core/preferences.h b/esphome/core/preferences.h index d24d51164a..cfeddebda7 100644 --- a/esphome/core/preferences.h +++ b/esphome/core/preferences.h @@ -45,8 +45,18 @@ extern ESPPreferences *global_preferences; // NOLINT(cppcoreguidelines-avoid-no } // namespace esphome #endif +namespace esphome { +static_assert(PreferencesContract, + "The platform's preferences manager is missing part of the ESPPreferences surface " + "(esphome/core/preference_backend.h)"); +} // namespace esphome + #ifdef USE_PREFERENCE_KEY_LOOKUP namespace esphome { +static_assert(PreferencesKeyLookupContract, + "This platform emits USE_PREFERENCE_KEY_LOOKUP but its preferences manager does not provide " + "load_from_key() (esphome/core/preference_backend.h)"); + /// Copy preference data stored under old_key into new_pref (created for new_key) if the keys /// differ and new_pref has no data yet. scratch must hold at least size bytes. /// Returns true when scratch holds the entity's current data (loaded or just migrated). diff --git a/tests/component_tests/preferences/__init__.py b/tests/component_tests/preferences/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/component_tests/preferences/config/bk72xx.yaml b/tests/component_tests/preferences/config/bk72xx.yaml new file mode 100644 index 0000000000..9ea4154bca --- /dev/null +++ b/tests/component_tests/preferences/config/bk72xx.yaml @@ -0,0 +1,5 @@ +esphome: + name: preftest + +bk72xx: + board: generic-bk7252 diff --git a/tests/component_tests/preferences/config/esp32.yaml b/tests/component_tests/preferences/config/esp32.yaml new file mode 100644 index 0000000000..586979d7b6 --- /dev/null +++ b/tests/component_tests/preferences/config/esp32.yaml @@ -0,0 +1,5 @@ +esphome: + name: preftest + +esp32: + board: esp32dev diff --git a/tests/component_tests/preferences/config/esp8266.yaml b/tests/component_tests/preferences/config/esp8266.yaml new file mode 100644 index 0000000000..b8a1035159 --- /dev/null +++ b/tests/component_tests/preferences/config/esp8266.yaml @@ -0,0 +1,5 @@ +esphome: + name: preftest + +esp8266: + board: esp01_1m diff --git a/tests/component_tests/preferences/config/host.yaml b/tests/component_tests/preferences/config/host.yaml new file mode 100644 index 0000000000..047f8693a8 --- /dev/null +++ b/tests/component_tests/preferences/config/host.yaml @@ -0,0 +1,4 @@ +esphome: + name: preftest + +host: diff --git a/tests/component_tests/preferences/config/nrf52.yaml b/tests/component_tests/preferences/config/nrf52.yaml new file mode 100644 index 0000000000..00892addb5 --- /dev/null +++ b/tests/component_tests/preferences/config/nrf52.yaml @@ -0,0 +1,6 @@ +esphome: + name: preftest + +nrf52: + board: adafruit_itsybitsy_nrf52840 + bootloader: adafruit_nrf52_sd140_v6 diff --git a/tests/component_tests/preferences/config/rp2.yaml b/tests/component_tests/preferences/config/rp2.yaml new file mode 100644 index 0000000000..d57b96a54e --- /dev/null +++ b/tests/component_tests/preferences/config/rp2.yaml @@ -0,0 +1,5 @@ +esphome: + name: preftest + +rp2: + board: rpipicow diff --git a/tests/component_tests/preferences/test_key_lookup_gate.py b/tests/component_tests/preferences/test_key_lookup_gate.py new file mode 100644 index 0000000000..7726113aab --- /dev/null +++ b/tests/component_tests/preferences/test_key_lookup_gate.py @@ -0,0 +1,39 @@ +"""Every preferences platform either emits USE_PREFERENCE_KEY_LOOKUP from +codegen (key-lookup backends) or must not (slot-based backends, whose managers +have no load_from_key()). Run each platform's real codegen and assert the +emission, mirroring the split the deny-list in esphome/core/defines.h assumes +for static analysis. + +The fixtures cover every distinct preferences backend today: ln882x and +rtl87xx route through libretiny (bk72xx stands in for the family), rp2040 is +an alias of rp2, and nrf52 exercises zephyr. A seventh backend needs a new +fixture here.""" + +from collections.abc import Callable +from pathlib import Path + +import pytest + +from esphome.core import CORE + + +@pytest.mark.parametrize( + ("fixture", "emits"), + [ + ("esp32.yaml", True), + ("bk72xx.yaml", True), # libretiny + ("host.yaml", True), + ("nrf52.yaml", True), # zephyr + ("esp8266.yaml", False), + ("rp2.yaml", False), + ], +) +def test_key_lookup_define_matches_the_platform_backend( + fixture: str, + emits: bool, + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], +) -> None: + generate_main(component_config_path(fixture)) + defines = {define.name for define in CORE.defines} + assert ("USE_PREFERENCE_KEY_LOOKUP" in defines) is emits diff --git a/tests/components/core/test_preference_contract.cpp b/tests/components/core/test_preference_contract.cpp new file mode 100644 index 0000000000..f0833a5929 --- /dev/null +++ b/tests/components/core/test_preference_contract.cpp @@ -0,0 +1,91 @@ +// Pins the preferences contract concepts so the surface they enforce cannot +// drift unnoticed: a minimal conforming type must satisfy each concept, and a +// type missing a method or returning the wrong type must not. + +#include + +#include "esphome/core/preference_backend.h" + +namespace esphome::core::testing { + +struct MinimalBackend { + bool save(const uint8_t *, size_t) { return true; } + bool load(uint8_t *, size_t) { return true; } +}; +static_assert(PreferenceBackendContract); + +struct BackendMissingLoad { + bool save(const uint8_t *, size_t) { return true; } +}; +static_assert(!PreferenceBackendContract); + +struct BackendWrongReturn { + void save(const uint8_t *, size_t) {} + bool load(uint8_t *, size_t) { return true; } +}; +static_assert(!PreferenceBackendContract); + +struct MinimalPreferences : public PreferencesMixin { + using PreferencesMixin::make_preference; + ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } + ESPPreferenceObject make_preference(size_t, uint32_t) { return {}; } + bool sync() { return true; } + bool reset() { return true; } +}; +static_assert(PreferencesContract); + +struct PreferencesMissingTwoArgForm : public PreferencesMixin { + using PreferencesMixin::make_preference; + ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } + bool sync() { return true; } + bool reset() { return true; } +}; +static_assert(!PreferencesContract); + +struct PreferencesMissingReset : public PreferencesMixin { + using PreferencesMixin::make_preference; + ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } + ESPPreferenceObject make_preference(size_t, uint32_t) { return {}; } + bool sync() { return true; } +}; +static_assert(!PreferencesContract); + +struct PreferencesWrongSyncReturn : public PreferencesMixin { + using PreferencesMixin::make_preference; + ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } + ESPPreferenceObject make_preference(size_t, uint32_t) { return {}; } + void sync() {} + bool reset() { return true; } +}; +static_assert(!PreferencesContract); + +// Forgot `using PreferencesMixin::make_preference;`, so the derived +// overloads hide the template forms (see the PreferencesContract note in +// preference_backend.h); the concept must reject the class. +struct PreferencesForgotUsingDeclaration : public PreferencesMixin { + ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } + ESPPreferenceObject make_preference(size_t, uint32_t) { return {}; } + bool sync() { return true; } + bool reset() { return true; } +}; +static_assert(!PreferencesContract); + +struct MinimalKeyLookup { + bool load_from_key(uint32_t, uint8_t *, size_t) { return true; } +}; +static_assert(PreferencesKeyLookupContract); + +struct KeyLookupMissingMethod {}; +static_assert(!PreferencesKeyLookupContract); + +TEST(PreferenceContract, NullBackendRefusesBothOperations) { + // ESPPreferenceObject forwards to whichever backend the platform binds; a + // default-constructed object has no backend and must refuse both operations + // instead of crashing. + ESPPreferenceObject without_backend; + uint32_t value = 42; + EXPECT_FALSE(without_backend.save(&value)); + EXPECT_FALSE(without_backend.load(&value)); +} + +} // namespace esphome::core::testing