From 80dc93d6cea6d69317e3c4b6b7de8fad360c082b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 5 Oct 2026 19:24:31 -0500 Subject: [PATCH] [core] [select] Keep select options in flash with a shared ConstVector (#20117) --- esphome/components/api/api.proto | 2 +- esphome/components/api/api_connection.cpp | 4 +- esphome/components/api/api_pb2.h | 2 +- esphome/components/api/api_pb2_includes.h | 1 + esphome/components/select/__init__.py | 4 +- esphome/components/select/select_traits.cpp | 15 ++- esphome/components/select/select_traits.h | 18 +++- esphome/core/helpers.h | 79 +++++++++++++-- .../select/config/select_options.yaml | 26 +++++ .../select/test_select_options.py | 27 ++++++ tests/components/core/const_vector_test.cpp | 96 +++++++++++++++++++ .../components/select/select_traits_test.cpp | 83 ++++++++++++++++ 12 files changed, 336 insertions(+), 21 deletions(-) create mode 100644 tests/component_tests/select/config/select_options.yaml create mode 100644 tests/component_tests/select/test_select_options.py create mode 100644 tests/components/core/const_vector_test.cpp create mode 100644 tests/components/select/select_traits_test.cpp diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 687dc1ca95..a03bf5fc1a 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -1439,7 +1439,7 @@ message ListEntitiesSelectResponse { reserved 4; // Deprecated: was string unique_id string icon = 5 [(field_ifdef) = "USE_ENTITY_ICON", (max_data_length) = 63]; - repeated string options = 6 [(container_pointer_no_template) = "FixedVector"]; + repeated string options = 6 [(container_pointer_no_template) = "std::span"]; bool disabled_by_default = 7; EntityCategory entity_category = 8; uint32 device_id = 9 [(field_ifdef) = "USE_DEVICES"]; diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 0207bc14b9..e50bf2f722 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -990,7 +990,9 @@ uint16_t APIConnection::try_send_select_state(EntityBase *entity, APIConnection uint16_t APIConnection::try_send_select_info(EntityBase *entity, APIConnection *conn, uint32_t remaining_size) { auto *select = static_cast(entity); ListEntitiesSelectResponse msg; - msg.options = &select->traits.get_options(); + const auto &opts = select->traits.get_options(); + const std::span options(opts.data(), opts.size()); + msg.options = &options; return fill_and_encode_entity_info(select, msg, conn, remaining_size); } void APIConnection::on_select_command_request(const SelectCommandRequest &msg) { diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index dbf0fb49a1..b4e3d4c55b 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1909,7 +1909,7 @@ class ListEntitiesSelectResponse final : public InfoResponseProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP const LogString *message_name() const override { return LOG_STR("list_entities_select_response"); } #endif - const FixedVector *options{}; + const std::span *options{}; static uint8_t *encode_msg(const void *self, ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM); uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { return encode_msg(this, buffer PROTO_ENCODE_DEBUG_ARG); diff --git a/esphome/components/api/api_pb2_includes.h b/esphome/components/api/api_pb2_includes.h index 70ba579fcc..833e6529e2 100644 --- a/esphome/components/api/api_pb2_includes.h +++ b/esphome/components/api/api_pb2_includes.h @@ -28,6 +28,7 @@ // Standard library includes that might be needed #include +#include #include #include diff --git a/esphome/components/select/__init__.py b/esphome/components/select/__init__.py index fec88d2bfd..fa266fc81f 100644 --- a/esphome/components/select/__init__.py +++ b/esphome/components/select/__init__.py @@ -95,7 +95,9 @@ def select_schema( @setup_entity("select") async def setup_select_core_(var, config, *, options: list[str]): - cg.add(var.traits.set_options(options)) + if options: + table = cg.shared_progmem_array("select_options", cg.const_char_ptr, options) + cg.add(var.traits.set_options_static(table, len(options))) for conf in config.get(CONF_ON_VALUE, []): trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) diff --git a/esphome/components/select/select_traits.cpp b/esphome/components/select/select_traits.cpp index 67a5118646..23d1f47cce 100644 --- a/esphome/components/select/select_traits.cpp +++ b/esphome/components/select/select_traits.cpp @@ -2,13 +2,18 @@ namespace esphome::select { -void SelectTraits::set_options(const std::initializer_list &options) { this->options_ = options; } +// Runtime option lists are copied, since the argument may not outlive the select; one +// out of line copy keeps a single instance of the copy code. +void SelectTraits::set_options_copy_(const char *const *options, size_t count) { + this->options_.assign_copy(options, count); +} + +void SelectTraits::set_options(const std::initializer_list &options) { + this->set_options_copy_(options.begin(), options.size()); +} void SelectTraits::set_options(const FixedVector &options) { - this->options_.init(options.size()); - for (const auto &opt : options) { - this->options_.push_back(opt); - } + this->set_options_copy_(options.begin(), options.size()); } } // namespace esphome::select diff --git a/esphome/components/select/select_traits.h b/esphome/components/select/select_traits.h index e1b261bc96..47997ac7f2 100644 --- a/esphome/components/select/select_traits.h +++ b/esphome/components/select/select_traits.h @@ -5,14 +5,28 @@ namespace esphome::select { +/// Option strings: a shared codegen table, or a copy of a runtime list. +using SelectOptions = ConstVector; + class SelectTraits { public: + SelectTraits() = default; + SelectTraits(const SelectTraits &) = delete; + SelectTraits &operator=(const SelectTraits &) = delete; + + /// Codegen only: points at a table that outlives the select. Call before any runtime set_options; + /// it does not free a previous copy (generated setup() runs before any lambda or automation). + void set_options_static(const char *const *options, size_t count) { this->options_.assign_static(options, count); } + /// Runtime lists: the pointer list is copied, as before; the strings must still outlive the select. + void set_options(const SelectOptions &options) { this->set_options_copy_(options.data(), options.size()); } void set_options(const std::initializer_list &options); void set_options(const FixedVector &options); - const FixedVector &get_options() const { return this->options_; } + const SelectOptions &get_options() const { return this->options_; } protected: - FixedVector options_; + void set_options_copy_(const char *const *options, size_t count); + + SelectOptions options_; }; } // namespace esphome::select diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 6d00e18799..b88a9d70e9 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -129,23 +129,82 @@ template<> constexpr int64_t byteswap(int64_t n) { return __builtin_bswap64(n); /// @name Container utilities ///@{ -/// Lightweight read-only view over a const array stored in RODATA (will typically be in flash memory) -/// Avoids copying data from flash to RAM by keeping a pointer to the flash data. -/// Similar to std::span but with minimal overhead for embedded systems. - -template class ConstVector { +/// Lightweight read-only view over a const array stored in RODATA (will typically be in flash memory). +/// Iterators are raw pointers like FixedVector. With Owning = true it can also hold a heap copy it +/// owns (see the specialization below); the default view never frees and has no extra cost. +template class ConstVector { public: + using value_type = T; + + constexpr ConstVector() = default; constexpr ConstVector(const T *data, size_t size) : data_(data), size_(size) {} - const constexpr T &operator[](size_t i) const { return data_[i]; } - constexpr size_t size() const { return size_; } - constexpr bool empty() const { return size_ == 0; } + const T *begin() const { return this->data_; } + const T *end() const { return this->data_ + this->size_; } + const T *data() const { return this->data_; } + constexpr size_t size() const { return this->size_; } + constexpr bool empty() const { return this->size_ == 0; } + const constexpr T &operator[](size_t i) const { return this->data_[i]; } + const T &at(size_t i) const { return this->data_[i]; } protected: - const T *data_; - size_t size_; + const T *data_{nullptr}; + size_t size_{0}; }; +/// Owning variant: a codegen table that outlives it, or a heap copy of a runtime list it owns. +/// Ownership is the top bit of the size; it is not copyable, so a copy can never outlive the owner. +/// Elements must be whole words so ESP8266 can read a codegen table from flash. +template class ConstVector { + static_assert(std::is_trivially_copyable_v && sizeof(T) % sizeof(uint32_t) == 0, + "ConstVector elements must be whole words so ESP8266 can read them from flash"); + + public: + using value_type = T; + + constexpr ConstVector() = default; + constexpr ConstVector(const T *data, size_t size) : data_(data), size_(size) {} + ConstVector(const ConstVector &) = delete; + ConstVector &operator=(const ConstVector &) = delete; + ~ConstVector() { this->release_(); } + + const T *begin() const { return this->data_; } + const T *end() const { return this->data_ + this->size(); } + const T *data() const { return this->data_; } + size_t size() const { return this->size_ & ~OWNED_BIT; } + bool empty() const { return this->size() == 0; } + const T &operator[](size_t index) const { return this->data_[index]; } + const T &at(size_t index) const { return this->data_[index]; } + + /// Codegen only: call before any runtime copy; it does not free a previous owned copy + /// (generated setup() runs before any lambda or automation can call set_options). + void assign_static(const T *data, size_t size) { + this->data_ = data; + this->size_ = size; + } + /// Copies the list into a heap array this owns, freeing a previous owned copy. + void assign_copy(const T *data, size_t size) { + auto *table = new T[size]; // NOLINT(cppcoreguidelines-owning-memory) + std::copy(data, data + size, table); + this->release_(); + this->data_ = table; + this->size_ = size | OWNED_BIT; + } + + protected: + static constexpr size_t OWNED_BIT = size_t{1} << (sizeof(size_t) * 8 - 1); + + void release_() { + if (this->size_ & OWNED_BIT) + delete[] this->data_; // NOLINT(cppcoreguidelines-owning-memory) + } + + const T *data_{nullptr}; + size_t size_{0}; // top bit set when data_ is an owned heap copy +}; +static_assert(sizeof(ConstVector) == 2 * sizeof(void *), + "ConstVector must stay a pointer and a size"); + /// Small buffer optimization - stores data inline when small, heap-allocates for large data /// This avoids heap fragmentation for common small allocations while supporting arbitrary sizes. /// Memory management is encapsulated - callers just use set() and data(). diff --git a/tests/component_tests/select/config/select_options.yaml b/tests/component_tests/select/config/select_options.yaml new file mode 100644 index 0000000000..474fc7cec4 --- /dev/null +++ b/tests/component_tests/select/config/select_options.yaml @@ -0,0 +1,26 @@ +esphome: + name: test + +esp8266: + board: esp01_1m + +select: + - platform: template + id: fan_a + name: Fan A + optimistic: true + options: [low, medium, high] + - platform: template + id: fan_b + name: Fan B + optimistic: true + options: [low, medium, high] + - platform: template + id: mode + name: Mode + optimistic: true + options: [auto, manual] + - platform: copy + id: fan_copy + name: Fan Copy + source_id: fan_a diff --git a/tests/component_tests/select/test_select_options.py b/tests/component_tests/select/test_select_options.py new file mode 100644 index 0000000000..0fe70f7f2c --- /dev/null +++ b/tests/component_tests/select/test_select_options.py @@ -0,0 +1,27 @@ +"""Select options are shared PROGMEM tables of option pointers.""" + +from collections.abc import Callable +from pathlib import Path +import re + + +def test_select_options_use_shared_tables( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], +) -> None: + main_cpp = generate_main(component_config_path("select_options.yaml")) + + calls = { + var: table + for var, table, _ in re.findall( + r"(\w+)->traits\.set_options_static\((\w+), (\d+)\);", main_cpp + ) + } + assert set(calls) == {"fan_a", "fan_b", "mode"} + assert calls["fan_a"] == calls["fan_b"] != calls["mode"] + assert ( + f'static constexpr const char * {calls["fan_a"]}[] PROGMEM = {{"low", "medium", "high"}};' + in main_cpp + ) + # The copy select takes its options from the source at setup + assert "fan_copy->traits.set_options_static(" not in main_cpp diff --git a/tests/components/core/const_vector_test.cpp b/tests/components/core/const_vector_test.cpp new file mode 100644 index 0000000000..aae067fb0f --- /dev/null +++ b/tests/components/core/const_vector_test.cpp @@ -0,0 +1,96 @@ +#include + +#include +#include + +#include "esphome/core/helpers.h" + +namespace esphome::testing { + +static constexpr const char *const TABLE[] = {"a", "b", "c"}; + +// Exposes the owned flag, which is protected. +class ProbeVector : public ConstVector { + public: + using ConstVector::ConstVector; + bool owned() const { return (this->size_ & OWNED_BIT) != 0; } +}; + +TEST(ConstVector, StaticTableIsViewedNotOwned) { + ProbeVector list; + EXPECT_TRUE(list.empty()); + list.assign_static(TABLE, 3); + EXPECT_EQ(list.data(), TABLE); + EXPECT_EQ(list.size(), 3U); + EXPECT_FALSE(list.owned()); + EXPECT_STREQ(list[1], "b"); + EXPECT_STREQ(list.at(2), "c"); +} + +TEST(ConstVector, CopyIsOwnedAndSizeMasksTheFlag) { + ProbeVector list; + list.assign_copy(TABLE, 3); + EXPECT_NE(list.data(), TABLE); + EXPECT_TRUE(list.owned()); + EXPECT_EQ(list.size(), 3U); + EXPECT_STREQ(list[0], "a"); + + const char *const next[] = {"x", "y"}; + list.assign_copy(next, 2); // frees the previous copy + EXPECT_TRUE(list.owned()); + EXPECT_EQ(list.size(), 2U); + EXPECT_STREQ(list[1], "y"); +} + +TEST(ConstVector, StaticThenRuntimeCopiesNeverFreeTheTable) { + ProbeVector list; + list.assign_static(TABLE, 3); + EXPECT_FALSE(list.owned()); + list.assign_copy(TABLE, 2); // the static table is not owned, so nothing is freed + EXPECT_TRUE(list.owned()); + EXPECT_NE(list.data(), TABLE); + const char *const next[] = {"x"}; + list.assign_copy(next, 1); // frees the previous copy + EXPECT_EQ(list.size(), 1U); + EXPECT_STREQ(list[0], "x"); + EXPECT_STREQ(TABLE[0], "a"); +} + +TEST(ConstVector, EmptyCopyIsEmptyAndFreedOnNextSet) { + ProbeVector list; + list.assign_copy(TABLE, 3); + list.assign_copy(TABLE, 0); + EXPECT_TRUE(list.empty()); + list.assign_copy(TABLE, 2); // frees the empty copy + EXPECT_EQ(list.size(), 2U); +} + +TEST(ConstVector, OwningVariantIsNotCopyable) { + static_assert(!std::is_copy_constructible_v>); + static_assert(!std::is_copy_assignable_v>); +} + +TEST(ConstVector, CopyFromItsOwnStorage) { + ProbeVector list; + list.assign_copy(TABLE, 3); + list.assign_copy(list.data(), list.size()); + EXPECT_EQ(list.size(), 3U); + EXPECT_STREQ(list[2], "c"); +} + +TEST(ConstVector, PlainViewHasNoOwnershipCost) { + static_assert(std::is_trivially_copyable_v>); + static_assert(std::is_trivially_destructible_v>); + ConstVector list(TABLE, 3); + EXPECT_EQ(list.size(), 3U); + EXPECT_STREQ(list[2], "c"); +} + +TEST(ConstVector, IteratorsAreRawPointers) { + ConstVector list(TABLE, 3); + static_assert(std::is_same_v); + const auto *it = std::find(list.begin(), list.end(), TABLE[1]); + EXPECT_EQ(it - list.begin(), 1); +} + +} // namespace esphome::testing diff --git a/tests/components/select/select_traits_test.cpp b/tests/components/select/select_traits_test.cpp new file mode 100644 index 0000000000..2280dc203d --- /dev/null +++ b/tests/components/select/select_traits_test.cpp @@ -0,0 +1,83 @@ +#include + +#include +#include + +#include "esphome/components/select/select_traits.h" + +namespace esphome::select::testing { + +static constexpr const char *const OPTIONS[] = {"low", "medium", "high"}; + +TEST(SelectTraits, ViewsTheTableWithoutCopying) { + SelectTraits traits; + EXPECT_TRUE(traits.get_options().empty()); + traits.set_options_static(OPTIONS, 3); + const auto &options = traits.get_options(); + EXPECT_EQ(options.size(), 3U); + EXPECT_FALSE(options.empty()); + EXPECT_EQ(options.data(), OPTIONS); + EXPECT_STREQ(options[1], "medium"); + EXPECT_STREQ(options.at(2), "high"); + std::vector seen; + for (const char *option : options) + seen.emplace_back(option); + EXPECT_EQ(seen, (std::vector{"low", "medium", "high"})); +} + +TEST(SelectTraits, RuntimeListsAreCopied) { + SelectTraits traits; + traits.set_options({"a", "b"}); + EXPECT_EQ(traits.get_options().size(), 2U); + EXPECT_STREQ(traits.get_options()[1], "b"); + + FixedVector list; + list.init(3); + list.push_back("x"); + list.push_back("y"); + list.push_back("z"); + traits.set_options(list); + EXPECT_NE(traits.get_options().data(), list.begin()); + EXPECT_EQ(traits.get_options().size(), 3U); + EXPECT_STREQ(traits.get_options().at(2), "z"); + + // A later runtime list replaces the earlier copy + traits.set_options({"only"}); + EXPECT_EQ(traits.get_options().size(), 1U); + EXPECT_STREQ(traits.get_options()[0], "only"); +} + +TEST(SelectTraits, CopyingAnotherSelectSurvivesItsNextRuntimeList) { + SelectTraits source; + source.set_options({"a", "b"}); + SelectTraits copy; + copy.set_options(source.get_options()); + EXPECT_NE(copy.get_options().data(), source.get_options().data()); + + source.set_options({"c"}); + ASSERT_EQ(copy.get_options().size(), 2U); + EXPECT_STREQ(copy.get_options()[0], "a"); + EXPECT_STREQ(copy.get_options()[1], "b"); +} + +TEST(SelectTraits, StaticTablesAreNeverOwned) { + SelectTraits traits; + traits.set_options_static(OPTIONS, 3); + EXPECT_EQ(traits.get_options().data(), OPTIONS); + EXPECT_EQ(traits.get_options().size(), 3U); + // A runtime list after a static one copies and leaves the static table alone + traits.set_options({"x"}); + EXPECT_NE(traits.get_options().data(), OPTIONS); + EXPECT_EQ(traits.get_options().size(), 1U); + EXPECT_STREQ(OPTIONS[0], "low"); +} + +TEST(SelectTraits, CopyOfItsOwnOptionsStaysValid) { + SelectTraits traits; + traits.set_options({"a", "b"}); + traits.set_options(traits.get_options()); + ASSERT_EQ(traits.get_options().size(), 2U); + EXPECT_STREQ(traits.get_options()[1], "b"); +} + +} // namespace esphome::select::testing