From 78ffeb30fb9f3cb578fd17d8ed852c139696f191 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 17:55:13 -1000 Subject: [PATCH 01/11] [binary_sensor] Optimize MultiClickTrigger with FixedVector (#11453) Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com> --- esphome/components/binary_sensor/automation.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/esphome/components/binary_sensor/automation.h b/esphome/components/binary_sensor/automation.h index b46436dc41..0bc7b9acb3 100644 --- a/esphome/components/binary_sensor/automation.h +++ b/esphome/components/binary_sensor/automation.h @@ -2,11 +2,11 @@ #include #include -#include #include "esphome/core/component.h" #include "esphome/core/automation.h" #include "esphome/core/hal.h" +#include "esphome/core/helpers.h" #include "esphome/components/binary_sensor/binary_sensor.h" namespace esphome { @@ -92,8 +92,8 @@ class DoubleClickTrigger : public Trigger<> { class MultiClickTrigger : public Trigger<>, public Component { public: - explicit MultiClickTrigger(BinarySensor *parent, std::vector timing) - : parent_(parent), timing_(std::move(timing)) {} + explicit MultiClickTrigger(BinarySensor *parent, std::initializer_list timing) + : parent_(parent), timing_(timing) {} void setup() override { this->last_state_ = this->parent_->get_state_default(false); @@ -115,7 +115,7 @@ class MultiClickTrigger : public Trigger<>, public Component { void trigger_(); BinarySensor *parent_; - std::vector timing_; + FixedVector timing_; uint32_t invalid_cooldown_{1000}; optional at_index_{}; bool last_state_{false}; From e3aaf6a1440d066429b8b0465538f6f10b873a33 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 17:55:46 -1000 Subject: [PATCH 02/11] [wifi] Test multiple stas in wifi compile tests (#11460) --- tests/components/wifi/common.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/components/wifi/common.yaml b/tests/components/wifi/common.yaml index 343d44b177..af27f85092 100644 --- a/tests/components/wifi/common.yaml +++ b/tests/components/wifi/common.yaml @@ -12,5 +12,8 @@ esphome: - logger.log: "Failed to connect to WiFi!" wifi: - ssid: MySSID - password: password1 + networks: + - ssid: MySSID + password: password1 + - ssid: MySSID2 + password: password2 From 0de79ba29144a38ba4de245990dc033cc0cddd1c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 17:57:18 -1000 Subject: [PATCH 03/11] [event] Replace std::set with FixedVector for event type storage (#11463) --- esphome/components/event/event.cpp | 13 ++++++++++--- esphome/components/event/event.h | 7 +++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/esphome/components/event/event.cpp b/esphome/components/event/event.cpp index d27b3b378e..20549ad0a5 100644 --- a/esphome/components/event/event.cpp +++ b/esphome/components/event/event.cpp @@ -8,12 +8,19 @@ namespace event { static const char *const TAG = "event"; void Event::trigger(const std::string &event_type) { - auto found = types_.find(event_type); - if (found == types_.end()) { + // Linear search - faster than std::set for small datasets (1-5 items typical) + const std::string *found = nullptr; + for (const auto &type : this->types_) { + if (type == event_type) { + found = &type; + break; + } + } + if (found == nullptr) { ESP_LOGE(TAG, "'%s': invalid event type for trigger(): %s", this->get_name().c_str(), event_type.c_str()); return; } - last_event_type = &(*found); + last_event_type = found; ESP_LOGD(TAG, "'%s' Triggered event '%s'", this->get_name().c_str(), last_event_type->c_str()); this->event_callback_.call(event_type); } diff --git a/esphome/components/event/event.h b/esphome/components/event/event.h index a90c8ebe05..2f6267a200 100644 --- a/esphome/components/event/event.h +++ b/esphome/components/event/event.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include "esphome/core/component.h" @@ -26,13 +25,13 @@ class Event : public EntityBase, public EntityBase_DeviceClass { const std::string *last_event_type; void trigger(const std::string &event_type); - void set_event_types(const std::set &event_types) { this->types_ = event_types; } - std::set get_event_types() const { return this->types_; } + void set_event_types(const std::initializer_list &event_types) { this->types_ = event_types; } + const FixedVector &get_event_types() const { return this->types_; } void add_on_event_callback(std::function &&callback); protected: CallbackManager event_callback_; - std::set types_; + FixedVector types_; }; } // namespace event From 5b15827009a1269df71754b850295580ad90e0eb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 17:58:40 -1000 Subject: [PATCH 04/11] [CI] Fix component detection when core files change in determine-jobs (#11461) --- script/determine-jobs.py | 24 ++++++------ tests/script/test_determine_jobs.py | 57 +++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 11 deletions(-) diff --git a/script/determine-jobs.py b/script/determine-jobs.py index 6651553ce7..7cdec959c7 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -606,21 +606,23 @@ def main() -> None: # [list]: Changed components (already includes dependencies) changed_components_result = get_changed_components() + # Always analyze component files, even if core files changed + # This is needed for component testing and memory impact analysis + changed = changed_files(args.branch) + component_files = [f for f in changed if filter_component_and_test_files(f)] + + directly_changed_components = get_components_with_dependencies( + component_files, False + ) + if changed_components_result is None: # Core files changed - will trigger full clang-tidy scan - # No specific components to test - changed_components = [] - directly_changed_components = [] + # But we still need to track changed components for testing and memory analysis + changed_components = get_components_with_dependencies(component_files, True) is_core_change = True else: - # Get both directly changed and all changed (with dependencies) - changed = changed_files(args.branch) - component_files = [f for f in changed if filter_component_and_test_files(f)] - - directly_changed_components = get_components_with_dependencies( - component_files, False - ) - changed_components = get_components_with_dependencies(component_files, True) + # Use the result from get_changed_components() which includes dependencies + changed_components = changed_components_result is_core_change = False # Filter to only components that have test files diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index a859b3c24d..6095e86ea7 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -910,3 +910,60 @@ def test_clang_tidy_mode_targeted_scan( output = json.loads(captured.out) assert output["clang_tidy_mode"] == expected_mode + + +def test_main_core_files_changed_still_detects_components( + mock_should_run_integration_tests: Mock, + mock_should_run_clang_tidy: Mock, + mock_should_run_clang_format: Mock, + mock_should_run_python_linters: Mock, + mock_changed_files: Mock, + mock_determine_cpp_unit_tests: Mock, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test that component changes are detected even when core files change.""" + monkeypatch.delenv("GITHUB_ACTIONS", raising=False) + + mock_should_run_integration_tests.return_value = True + mock_should_run_clang_tidy.return_value = True + mock_should_run_clang_format.return_value = True + mock_should_run_python_linters.return_value = True + mock_determine_cpp_unit_tests.return_value = (True, []) + + mock_changed_files.return_value = [ + "esphome/core/helpers.h", + "esphome/components/select/select_traits.h", + "esphome/components/select/select_traits.cpp", + "esphome/components/api/api.proto", + ] + + with ( + patch("sys.argv", ["determine-jobs.py"]), + patch.object(determine_jobs, "_is_clang_tidy_full_scan", return_value=False), + patch.object(determine_jobs, "get_changed_components", return_value=None), + patch.object( + determine_jobs, + "filter_component_and_test_files", + side_effect=lambda f: f.startswith("esphome/components/"), + ), + patch.object( + determine_jobs, + "get_components_with_dependencies", + side_effect=lambda files, deps: ( + ["select", "api"] + if not deps + else ["select", "api", "bluetooth_proxy", "logger"] + ), + ), + ): + determine_jobs.main() + + captured = capsys.readouterr() + output = json.loads(captured.out) + + assert output["clang_tidy"] is True + assert output["clang_tidy_mode"] == "split" + assert "select" in output["changed_components"] + assert "api" in output["changed_components"] + assert len(output["changed_components"]) > 0 From 146b067d629ee0401fee030815ef7eaf89fe8e06 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 17:59:39 -1000 Subject: [PATCH 05/11] [light] Add compile test for addressable lights (#11465) --- tests/components/light/common.yaml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/components/light/common.yaml b/tests/components/light/common.yaml index f807014065..247fc19aba 100644 --- a/tests/components/light/common.yaml +++ b/tests/components/light/common.yaml @@ -17,6 +17,20 @@ esphome: relative_brightness: 5% brightness_limits: max_brightness: 90% + - light.turn_on: + id: test_addressable_transition + brightness: 50% + red: 100% + green: 0% + blue: 0% + transition_length: 500ms + - light.turn_on: + id: test_addressable_transition + brightness: 100% + red: 0% + green: 100% + blue: 0% + transition_length: 1s light: - platform: binary @@ -163,3 +177,9 @@ light: blue: 0% duration: 1s transition_length: 500ms + - platform: partition + id: test_addressable_transition + name: Addressable Transition Test + default_transition_length: 1s + segments: + - single_light_id: test_rgb_light From e1c851cab88a408d9df25cd924caffd8db263f67 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 19:23:10 -1000 Subject: [PATCH 06/11] [wifi] Optimize WiFi network storage with FixedVector (#11458) Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com> --- esphome/components/wifi/__init__.py | 19 ++++++++++------- esphome/components/wifi/wifi_component.cpp | 2 ++ esphome/components/wifi/wifi_component.h | 3 ++- esphome/core/helpers.h | 24 ++++++++++++++++------ 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/esphome/components/wifi/__init__.py b/esphome/components/wifi/__init__.py index 494470cb48..29d33bfc76 100644 --- a/esphome/components/wifi/__init__.py +++ b/esphome/components/wifi/__init__.py @@ -378,14 +378,19 @@ async def to_code(config): # Track if any network uses Enterprise authentication has_eap = False - def add_sta(ap, network): - ip_config = network.get(CONF_MANUAL_IP, config.get(CONF_MANUAL_IP)) - cg.add(var.add_sta(wifi_network(network, ap, ip_config))) + # Initialize FixedVector with the count of networks + networks = config.get(CONF_NETWORKS, []) + if networks: + cg.add(var.init_sta(len(networks))) - for network in config.get(CONF_NETWORKS, []): - if CONF_EAP in network: - has_eap = True - cg.with_local_variable(network[CONF_ID], WiFiAP(), add_sta, network) + def add_sta(ap: cg.MockObj, network: dict) -> None: + ip_config = network.get(CONF_MANUAL_IP, config.get(CONF_MANUAL_IP)) + cg.add(var.add_sta(wifi_network(network, ap, ip_config))) + + for network in networks: + if CONF_EAP in network: + has_eap = True + cg.with_local_variable(network[CONF_ID], WiFiAP(), add_sta, network) if CONF_AP in config: conf = config[CONF_AP] diff --git a/esphome/components/wifi/wifi_component.cpp b/esphome/components/wifi/wifi_component.cpp index c89384d742..b278e5a386 100644 --- a/esphome/components/wifi/wifi_component.cpp +++ b/esphome/components/wifi/wifi_component.cpp @@ -330,9 +330,11 @@ float WiFiComponent::get_loop_priority() const { return 10.0f; // before other loop components } +void WiFiComponent::init_sta(size_t count) { this->sta_.init(count); } void WiFiComponent::add_sta(const WiFiAP &ap) { this->sta_.push_back(ap); } void WiFiComponent::set_sta(const WiFiAP &ap) { this->clear_sta(); + this->init_sta(1); this->add_sta(ap); } void WiFiComponent::clear_sta() { this->sta_.clear(); } diff --git a/esphome/components/wifi/wifi_component.h b/esphome/components/wifi/wifi_component.h index 10aa82a065..42f78dbfac 100644 --- a/esphome/components/wifi/wifi_component.h +++ b/esphome/components/wifi/wifi_component.h @@ -219,6 +219,7 @@ class WiFiComponent : public Component { void set_sta(const WiFiAP &ap); WiFiAP get_sta() { return this->selected_ap_; } + void init_sta(size_t count); void add_sta(const WiFiAP &ap); void clear_sta(); @@ -393,7 +394,7 @@ class WiFiComponent : public Component { #endif std::string use_address_; - std::vector sta_; + FixedVector sta_; std::vector sta_priorities_; wifi_scan_vector_t scan_result_; WiFiAP selected_ap_; diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 234d2a7d7d..9b0591c9c5 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -194,12 +194,8 @@ template class FixedVector { size_ = 0; } - public: - FixedVector() = default; - - /// Constructor from initializer list - allocates exact size needed - /// This enables brace initialization: FixedVector v = {1, 2, 3}; - FixedVector(std::initializer_list init_list) { + // Helper to assign from initializer list (shared by constructor and assignment operator) + void assign_from_initializer_list_(std::initializer_list init_list) { init(init_list.size()); size_t idx = 0; for (const auto &item : init_list) { @@ -209,6 +205,13 @@ template class FixedVector { size_ = init_list.size(); } + public: + FixedVector() = default; + + /// Constructor from initializer list - allocates exact size needed + /// This enables brace initialization: FixedVector v = {1, 2, 3}; + FixedVector(std::initializer_list init_list) { assign_from_initializer_list_(init_list); } + ~FixedVector() { cleanup_(); } // Disable copy operations (avoid accidental expensive copies) @@ -234,6 +237,15 @@ template class FixedVector { return *this; } + /// Assignment from initializer list - avoids temporary and move overhead + /// This enables: FixedVector v; v = {1, 2, 3}; + FixedVector &operator=(std::initializer_list init_list) { + cleanup_(); + reset_(); + assign_from_initializer_list_(init_list); + return *this; + } + // Allocate capacity - can be called multiple times to reinit void init(size_t n) { cleanup_(); From 777e73fd041ecc67539e339fe1ad1c5d4bfe4af2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 21:54:44 -1000 Subject: [PATCH 07/11] Extract ColorModeMask into EnumBitmask helper --- esphome/components/light/color_mode.h | 212 +++++++----------------- esphome/components/light/light_call.cpp | 2 +- esphome/components/light/light_traits.h | 2 +- esphome/core/enum_bitmask.h | 155 +++++++++++++++++ 4 files changed, 216 insertions(+), 155 deletions(-) create mode 100644 esphome/core/enum_bitmask.h diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index a26f917167..9c6a4d147b 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -1,6 +1,7 @@ #pragma once #include +#include "esphome/core/enum_bitmask.h" namespace esphome { namespace light { @@ -104,16 +105,16 @@ constexpr ColorModeHelper operator|(ColorModeHelper lhs, ColorMode rhs) { return static_cast(static_cast(lhs) | static_cast(rhs)); } -// Type alias for raw color mode bitmask values +// Type alias for raw color mode bitmask values (retained for compatibility) using color_mode_bitmask_t = uint16_t; -// Constants for ColorMode count and bit range -static constexpr int COLOR_MODE_COUNT = 10; // UNKNOWN through RGB_COLD_WARM_WHITE -static constexpr int MAX_BIT_INDEX = sizeof(color_mode_bitmask_t) * 8; // Number of bits in bitmask type +// Number of ColorMode enum values +constexpr int COLOR_MODE_BITMASK_SIZE = 10; -// Compile-time array of all ColorMode values in declaration order -// Bit positions (0-9) map directly to enum declaration order -static constexpr ColorMode COLOR_MODES[COLOR_MODE_COUNT] = { +// Shared lookup table for ColorMode bit mapping +// This array defines the canonical order of color modes (bit 0-9) +// Declared early so it can be used by constexpr functions +constexpr ColorMode COLOR_MODE_LOOKUP[COLOR_MODE_BITMASK_SIZE] = { ColorMode::UNKNOWN, // bit 0 ColorMode::ON_OFF, // bit 1 ColorMode::BRIGHTNESS, // bit 2 @@ -126,33 +127,20 @@ static constexpr ColorMode COLOR_MODES[COLOR_MODE_COUNT] = { ColorMode::RGB_COLD_WARM_WHITE, // bit 9 }; -/// Map ColorMode enum values to bit positions (0-9) -/// Bit positions follow the enum declaration order -static constexpr int mode_to_bit(ColorMode mode) { - // Linear search through COLOR_MODES array - // Compiler optimizes this to efficient code since array is constexpr - for (int i = 0; i < COLOR_MODE_COUNT; ++i) { - if (COLOR_MODES[i] == mode) - return i; - } - return 0; -} +// Type alias for ColorMode bitmask using generic EnumBitmask template +using ColorModeMask = EnumBitmask; -/// Map bit positions (0-9) to ColorMode enum values -/// Bit positions follow the enum declaration order -static constexpr ColorMode bit_to_mode(int bit) { - // Direct lookup in COLOR_MODES array - return (bit >= 0 && bit < COLOR_MODE_COUNT) ? COLOR_MODES[bit] : ColorMode::UNKNOWN; -} +// Number of ColorCapability enum values +constexpr int COLOR_CAPABILITY_COUNT = 6; /// Helper to compute capability bitmask at compile time -static constexpr color_mode_bitmask_t compute_capability_bitmask(ColorCapability capability) { - color_mode_bitmask_t mask = 0; +constexpr uint16_t compute_capability_bitmask(ColorCapability capability) { + uint16_t mask = 0; uint8_t cap_bit = static_cast(capability); // Check each ColorMode to see if it has this capability - for (int bit = 0; bit < COLOR_MODE_COUNT; ++bit) { - uint8_t mode_val = static_cast(bit_to_mode(bit)); + for (int bit = 0; bit < COLOR_MODE_BITMASK_SIZE; ++bit) { + uint8_t mode_val = static_cast(COLOR_MODE_LOOKUP[bit]); if ((mode_val & cap_bit) != 0) { mask |= (1 << bit); } @@ -160,12 +148,9 @@ static constexpr color_mode_bitmask_t compute_capability_bitmask(ColorCapability return mask; } -// Number of ColorCapability enum values -static constexpr int COLOR_CAPABILITY_COUNT = 6; - /// Compile-time lookup table mapping ColorCapability to bitmask /// This array is computed at compile time using constexpr -static constexpr color_mode_bitmask_t CAPABILITY_BITMASKS[] = { +constexpr uint16_t CAPABILITY_BITMASKS[] = { compute_capability_bitmask(ColorCapability::ON_OFF), // 1 << 0 compute_capability_bitmask(ColorCapability::BRIGHTNESS), // 1 << 1 compute_capability_bitmask(ColorCapability::WHITE), // 1 << 2 @@ -174,130 +159,51 @@ static constexpr color_mode_bitmask_t CAPABILITY_BITMASKS[] = { compute_capability_bitmask(ColorCapability::RGB), // 1 << 5 }; -/// Bitmask for storing a set of ColorMode values efficiently. -/// Replaces std::set to eliminate red-black tree overhead (~586 bytes). -class ColorModeMask { - public: - constexpr ColorModeMask() = default; - - /// Support initializer list syntax: {ColorMode::RGB, ColorMode::WHITE} - constexpr ColorModeMask(std::initializer_list modes) { - for (auto mode : modes) { - this->add(mode); - } - } - - constexpr void add(ColorMode mode) { this->mask_ |= (1 << mode_to_bit(mode)); } - - /// Add multiple modes at once using initializer list - constexpr void add(std::initializer_list modes) { - for (auto mode : modes) { - this->add(mode); - } - } - - constexpr bool contains(ColorMode mode) const { return (this->mask_ & (1 << mode_to_bit(mode))) != 0; } - - constexpr size_t size() const { - // Count set bits using Brian Kernighan's algorithm - // More efficient for sparse bitmasks (typical case: 2-4 modes out of 10) - uint16_t n = this->mask_; - size_t count = 0; - while (n) { - n &= n - 1; // Clear the least significant set bit - count++; - } - return count; - } - - constexpr bool empty() const { return this->mask_ == 0; } - - /// Iterator support for API encoding - class Iterator { - public: - using iterator_category = std::forward_iterator_tag; - using value_type = ColorMode; - using difference_type = std::ptrdiff_t; - using pointer = const ColorMode *; - using reference = ColorMode; - - constexpr Iterator(color_mode_bitmask_t mask, int bit) : mask_(mask), bit_(bit) { advance_to_next_set_bit_(); } - - constexpr ColorMode operator*() const { return bit_to_mode(bit_); } - - constexpr Iterator &operator++() { - ++bit_; - advance_to_next_set_bit_(); - return *this; - } - - constexpr bool operator==(const Iterator &other) const { return bit_ == other.bit_; } - - constexpr bool operator!=(const Iterator &other) const { return !(*this == other); } - - private: - constexpr void advance_to_next_set_bit_() { bit_ = ColorModeMask::find_next_set_bit(mask_, bit_); } - - color_mode_bitmask_t mask_; - int bit_; - }; - - constexpr Iterator begin() const { return Iterator(mask_, 0); } - constexpr Iterator end() const { return Iterator(mask_, MAX_BIT_INDEX); } - - /// Get the raw bitmask value for API encoding - constexpr color_mode_bitmask_t get_mask() const { return this->mask_; } - - /// Find the next set bit in a bitmask starting from a given position - /// Returns the bit position, or MAX_BIT_INDEX if no more bits are set - static constexpr int find_next_set_bit(color_mode_bitmask_t mask, int start_bit) { - int bit = start_bit; - while (bit < MAX_BIT_INDEX && !(mask & (1 << bit))) { - ++bit; - } - return bit; - } - - /// Find the first set bit in a bitmask and return the corresponding ColorMode - /// Used for optimizing compute_color_mode_() intersection logic - static constexpr ColorMode first_mode_from_mask(color_mode_bitmask_t mask) { - return bit_to_mode(find_next_set_bit(mask, 0)); - } - - /// Check if a ColorMode is present in a raw bitmask value - /// Useful for checking intersection results without creating a temporary ColorModeMask - static constexpr bool mask_contains(color_mode_bitmask_t mask, ColorMode mode) { - return (mask & (1 << mode_to_bit(mode))) != 0; - } - - /// Check if any mode in the bitmask has a specific capability - /// Used for checking if a light supports a capability (e.g., BRIGHTNESS, RGB) - bool has_capability(ColorCapability capability) const { - // Lookup the pre-computed bitmask for this capability and check intersection with our mask - // ColorCapability values: 1, 2, 4, 8, 16, 32 -> array indices: 0, 1, 2, 3, 4, 5 - // We need to convert the power-of-2 value to an index - uint8_t cap_val = static_cast(capability); +/// Check if any mode in the bitmask has a specific capability +/// Used for checking if a light supports a capability (e.g., BRIGHTNESS, RGB) +inline bool has_capability(const ColorModeMask &mask, ColorCapability capability) { + // Lookup the pre-computed bitmask for this capability and check intersection with our mask + // ColorCapability values: 1, 2, 4, 8, 16, 32 -> array indices: 0, 1, 2, 3, 4, 5 + // We need to convert the power-of-2 value to an index + uint8_t cap_val = static_cast(capability); #if defined(__GNUC__) || defined(__clang__) - // Use compiler intrinsic for efficient bit position lookup (O(1) vs O(log n)) - int index = __builtin_ctz(cap_val); + // Use compiler intrinsic for efficient bit position lookup (O(1) vs O(log n)) + int index = __builtin_ctz(cap_val); #else - // Fallback for compilers without __builtin_ctz - int index = 0; - while (cap_val > 1) { - cap_val >>= 1; - ++index; - } -#endif - return (this->mask_ & CAPABILITY_BITMASKS[index]) != 0; + // Fallback for compilers without __builtin_ctz + int index = 0; + while (cap_val > 1) { + cap_val >>= 1; + ++index; } - - private: - // Using uint16_t instead of uint32_t for more efficient iteration (fewer bits to scan). - // Currently only 10 ColorMode values exist, so 16 bits is sufficient. - // Can be changed to uint32_t if more than 16 color modes are needed in the future. - // Note: Due to struct padding, uint16_t and uint32_t result in same LightTraits size (12 bytes). - color_mode_bitmask_t mask_{0}; -}; +#endif + return (mask.get_mask() & CAPABILITY_BITMASKS[index]) != 0; +} } // namespace light } // namespace esphome + +// Template specializations for ColorMode must be in global namespace + +/// Map ColorMode enum values to bit positions (0-9) +/// Bit positions follow the enum declaration order +template<> +constexpr int esphome::EnumBitmask::enum_to_bit( + esphome::light::ColorMode mode) { + // Linear search through COLOR_MODE_LOOKUP array + // Compiler optimizes this to efficient code since array is constexpr + for (int i = 0; i < esphome::light::COLOR_MODE_BITMASK_SIZE; ++i) { + if (esphome::light::COLOR_MODE_LOOKUP[i] == mode) + return i; + } + return 0; +} + +/// Map bit positions (0-9) to ColorMode enum values +/// Bit positions follow the enum declaration order +template<> +inline esphome::light::ColorMode esphome::EnumBitmask::bit_to_enum(int bit) { + return (bit >= 0 && bit < esphome::light::COLOR_MODE_BITMASK_SIZE) ? esphome::light::COLOR_MODE_LOOKUP[bit] + : esphome::light::ColorMode::UNKNOWN; +} diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index af193e1f11..26d14d7bb4 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -437,7 +437,7 @@ ColorMode LightCall::compute_color_mode_() { // Use the preferred suitable mode. if (intersection != 0) { - ColorMode mode = ColorModeMask::first_mode_from_mask(intersection); + ColorMode mode = ColorModeMask::first_value_from_mask(intersection); ESP_LOGI(TAG, "'%s': color mode not specified; using %s", this->parent_->get_name().c_str(), LOG_STR_ARG(color_mode_to_human(mode))); return mode; diff --git a/esphome/components/light/light_traits.h b/esphome/components/light/light_traits.h index 4532edca83..9dec9fb577 100644 --- a/esphome/components/light/light_traits.h +++ b/esphome/components/light/light_traits.h @@ -28,7 +28,7 @@ class LightTraits { bool supports_color_mode(ColorMode color_mode) const { return this->supported_color_modes_.contains(color_mode); } bool supports_color_capability(ColorCapability color_capability) const { - return this->supported_color_modes_.has_capability(color_capability); + return has_capability(this->supported_color_modes_, color_capability); } float get_min_mireds() const { return this->min_mireds_; } diff --git a/esphome/core/enum_bitmask.h b/esphome/core/enum_bitmask.h new file mode 100644 index 0000000000..4c29c7047e --- /dev/null +++ b/esphome/core/enum_bitmask.h @@ -0,0 +1,155 @@ +#pragma once + +#include +#include +#include +#include +#include + +namespace esphome { + +/// Generic bitmask for storing a set of enum values efficiently. +/// Replaces std::set to eliminate red-black tree overhead (~586 bytes per instantiation). +/// +/// Template parameters: +/// EnumType: The enum type to store (must be uint8_t-based) +/// MaxBits: Maximum number of bits needed (auto-selects uint8_t/uint16_t/uint32_t) +/// +/// Requirements: +/// - EnumType must be an enum with sequential values starting from 0 +/// - Specialization must provide enum_to_bit() and bit_to_enum() static methods +/// - MaxBits must be sufficient to hold all enum values +/// +/// Example usage: +/// using ClimateModeMask = EnumBitmask; +/// ClimateModeMask modes({CLIMATE_MODE_HEAT, CLIMATE_MODE_COOL}); +/// if (modes.contains(CLIMATE_MODE_HEAT)) { ... } +/// for (auto mode : modes) { ... } // Iterate over set bits +/// +/// Design notes: +/// - Uses compile-time type selection for optimal size (uint8_t/uint16_t/uint32_t) +/// - Iterator converts bit positions to actual enum values during traversal +/// - All operations are constexpr-compatible for compile-time initialization +/// - Drop-in replacement for std::set with simpler API +/// +template class EnumBitmask { + public: + // Automatic bitmask type selection based on MaxBits + // ≤8 bits: uint8_t, ≤16 bits: uint16_t, otherwise: uint32_t + using bitmask_t = + typename std::conditional<(MaxBits <= 8), uint8_t, + typename std::conditional<(MaxBits <= 16), uint16_t, uint32_t>::type>::type; + + constexpr EnumBitmask() = default; + + /// Construct from initializer list: {VALUE1, VALUE2, ...} + constexpr EnumBitmask(std::initializer_list values) { + for (auto value : values) { + this->add(value); + } + } + + /// Add a single enum value to the set + constexpr void add(EnumType value) { this->mask_ |= (static_cast(1) << enum_to_bit(value)); } + + /// Add multiple enum values from initializer list + constexpr void add(std::initializer_list values) { + for (auto value : values) { + this->add(value); + } + } + + /// Remove an enum value from the set + constexpr void remove(EnumType value) { this->mask_ &= ~(static_cast(1) << enum_to_bit(value)); } + + /// Clear all values from the set + constexpr void clear() { this->mask_ = 0; } + + /// Check if the set contains a specific enum value + constexpr bool contains(EnumType value) const { + return (this->mask_ & (static_cast(1) << enum_to_bit(value))) != 0; + } + + /// Count the number of enum values in the set + constexpr size_t size() const { + // Brian Kernighan's algorithm - efficient for sparse bitmasks + // Typical case: 2-4 modes out of 10 possible + bitmask_t n = this->mask_; + size_t count = 0; + while (n) { + n &= n - 1; // Clear the least significant set bit + count++; + } + return count; + } + + /// Check if the set is empty + constexpr bool empty() const { return this->mask_ == 0; } + + /// Iterator support for range-based for loops and API encoding + /// Iterates over set bits and converts bit positions to enum values + class Iterator { + public: + using iterator_category = std::forward_iterator_tag; + using value_type = EnumType; + using difference_type = std::ptrdiff_t; + using pointer = const EnumType *; + using reference = EnumType; + + constexpr Iterator(bitmask_t mask, int bit) : mask_(mask), bit_(bit) { advance_to_next_set_bit_(); } + + constexpr EnumType operator*() const { return bit_to_enum(bit_); } + + constexpr Iterator &operator++() { + ++bit_; + advance_to_next_set_bit_(); + return *this; + } + + constexpr bool operator==(const Iterator &other) const { return bit_ == other.bit_; } + + constexpr bool operator!=(const Iterator &other) const { return !(*this == other); } + + private: + constexpr void advance_to_next_set_bit_() { bit_ = find_next_set_bit(mask_, bit_); } + + bitmask_t mask_; + int bit_; + }; + + constexpr Iterator begin() const { return Iterator(mask_, 0); } + constexpr Iterator end() const { return Iterator(mask_, MaxBits); } + + /// Get the raw bitmask value for optimized operations + constexpr bitmask_t get_mask() const { return this->mask_; } + + /// Check if a specific enum value is present in a raw bitmask + /// Useful for checking intersection results without creating temporary objects + static constexpr bool mask_contains(bitmask_t mask, EnumType value) { + return (mask & (static_cast(1) << enum_to_bit(value))) != 0; + } + + /// Get the first enum value from a raw bitmask + /// Used for optimizing intersection logic (e.g., "pick first suitable mode") + static constexpr EnumType first_value_from_mask(bitmask_t mask) { return bit_to_enum(find_next_set_bit(mask, 0)); } + + /// Find the next set bit in a bitmask starting from a given position + /// Returns the bit position, or MaxBits if no more bits are set + static constexpr int find_next_set_bit(bitmask_t mask, int start_bit) { + int bit = start_bit; + while (bit < MaxBits && !(mask & (static_cast(1) << bit))) { + ++bit; + } + return bit; + } + + protected: + // Must be provided by template specialization + // These convert between enum values and bit positions (0, 1, 2, ...) + static constexpr int enum_to_bit(EnumType value); + static EnumType bit_to_enum(int bit); // Not constexpr due to static array limitation in C++20 + + bitmask_t mask_{0}; +}; + +} // namespace esphome From c6711fc354d200bd37558367720c20ebefb5d542 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 22:19:07 -1000 Subject: [PATCH 08/11] adjust --- esphome/components/light/color_mode.h | 6 ++++++ esphome/core/enum_bitmask.h | 3 +++ 2 files changed, 9 insertions(+) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 9c6a4d147b..03132f54bf 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -184,6 +184,12 @@ inline bool has_capability(const ColorModeMask &mask, ColorCapability capability } // namespace esphome // Template specializations for ColorMode must be in global namespace +// +// C++ requires template specializations to be declared in the same namespace as the +// original template. Since EnumBitmask is in the esphome namespace (not esphome::light), +// we must provide these specializations at global scope with fully-qualified names. +// +// These specializations define how ColorMode enum values map to/from bit positions. /// Map ColorMode enum values to bit positions (0-9) /// Bit positions follow the enum declaration order diff --git a/esphome/core/enum_bitmask.h b/esphome/core/enum_bitmask.h index 4c29c7047e..fdbd0c50cc 100644 --- a/esphome/core/enum_bitmask.h +++ b/esphome/core/enum_bitmask.h @@ -26,6 +26,9 @@ namespace esphome { /// if (modes.contains(CLIMATE_MODE_HEAT)) { ... } /// for (auto mode : modes) { ... } // Iterate over set bits /// +/// For complete usage examples with template specializations, see: +/// - esphome/components/light/color_mode.h (ColorMode example) +/// /// Design notes: /// - Uses compile-time type selection for optimal size (uint8_t/uint16_t/uint32_t) /// - Iterator converts bit positions to actual enum values during traversal From 1119b4e11e21636f02e123053ad043054fae2319 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 22:23:37 -1000 Subject: [PATCH 09/11] [core] Add std::set compatibility aliases to EnumBitmask - Add insert() as alias for add() - Add erase() as alias for remove() - Add count() as alias for contains() - Makes EnumBitmask a true drop-in replacement for std::set - Update documentation to reflect compatibility --- enum_templates.md | 200 +++++++++++++++++++++++++++ esphome/core/enum_bitmask.h | 9 ++ extract_color_mode_mask_helper_pr.md | 98 +++++++++++++ 3 files changed, 307 insertions(+) create mode 100644 enum_templates.md create mode 100644 extract_color_mode_mask_helper_pr.md diff --git a/enum_templates.md b/enum_templates.md new file mode 100644 index 0000000000..175f8d0b89 --- /dev/null +++ b/enum_templates.md @@ -0,0 +1,200 @@ +# EnumBitmask Pattern Documentation + +## Overview + +`EnumBitmask` from `esphome/core/enum_bitmask.h` provides a memory-efficient replacement for `std::set` when storing sets of enum values. + +## When to Use + +Use `EnumBitmask` instead of `std::set` when: +- Storing sets of enum values (e.g., supported modes, capabilities) +- Enum has ≤32 distinct values +- Memory efficiency is important (saves ~586 bytes per `std::set` instance) + +## Benefits + +- **Memory Savings**: Eliminates red-black tree overhead (~586 bytes per instance) +- **Compact Storage**: 1-4 bytes depending on enum count (uint8_t/uint16_t/uint32_t) +- **Constexpr-Compatible**: Supports compile-time initialization +- **Efficient Iteration**: Only visits set bits, not all possible enum values +- **Range-Based Loops**: `for (auto value : mask)` works seamlessly + +## Requirements + +1. Enum must have sequential values (or use a lookup table for mapping) +2. Maximum 32 enum values (uint32_t bitmask limitation) +3. Must provide template specializations for `enum_to_bit()` and `bit_to_enum()` + +## Basic Usage Example + +```cpp +// Bad - red-black tree overhead (~586 bytes) +std::set supported_modes; +supported_modes.insert(ColorMode::RGB); +supported_modes.insert(ColorMode::WHITE); +if (supported_modes.count(ColorMode::RGB)) { ... } + +// Good - compact bitmask storage (2-4 bytes) +ColorModeMask supported_modes({ColorMode::RGB, ColorMode::WHITE}); +if (supported_modes.contains(ColorMode::RGB)) { ... } +for (auto mode : supported_modes) { ... } // Iterate over set values +``` + +## Implementation Pattern + +### 1. Define the Lookup Table + +If enum values aren't sequential from 0, create a lookup table: + +```cpp +// In your component header (e.g., esphome/components/light/color_mode.h) +constexpr ColorMode COLOR_MODE_LOOKUP[10] = { + ColorMode::UNKNOWN, // bit 0 + ColorMode::ON_OFF, // bit 1 + ColorMode::BRIGHTNESS, // bit 2 + ColorMode::WHITE, // bit 3 + ColorMode::COLOR_TEMPERATURE, // bit 4 + ColorMode::COLD_WARM_WHITE, // bit 5 + ColorMode::RGB, // bit 6 + ColorMode::RGB_WHITE, // bit 7 + ColorMode::RGB_COLOR_TEMPERATURE, // bit 8 + ColorMode::RGB_COLD_WARM_WHITE, // bit 9 +}; +``` + +### 2. Create Type Alias + +```cpp +constexpr int COLOR_MODE_BITMASK_SIZE = 10; +using ColorModeMask = EnumBitmask; +``` + +### 3. Provide Template Specializations + +**IMPORTANT**: Specializations must be in the **global namespace** (C++ requirement). Place them at the end of your header file, outside your component namespace. + +```cpp +// At end of header, outside namespace esphome::light +// Template specializations for ColorMode must be in global namespace +// +// C++ requires template specializations to be declared in the same namespace as the +// original template. Since EnumBitmask is in the esphome namespace (not esphome::light), +// we must provide these specializations at global scope with fully-qualified names. +// +// These specializations define how ColorMode enum values map to/from bit positions. + +/// Map ColorMode enum values to bit positions (0-9) +template<> +constexpr int esphome::EnumBitmask::enum_to_bit( + esphome::light::ColorMode mode) { + // Map enum value to bit position (0-9) + for (int i = 0; i < esphome::light::COLOR_MODE_BITMASK_SIZE; ++i) { + if (esphome::light::COLOR_MODE_LOOKUP[i] == mode) + return i; + } + return 0; // Unknown values map to bit 0 (typically reserved for UNKNOWN/NONE) +} + +/// Map bit positions (0-9) to ColorMode enum values +template<> +inline esphome::light::ColorMode esphome::EnumBitmask::bit_to_enum(int bit) { + return (bit >= 0 && bit < esphome::light::COLOR_MODE_BITMASK_SIZE) + ? esphome::light::COLOR_MODE_LOOKUP[bit] + : esphome::light::ColorMode::UNKNOWN; +} +``` + +### Error Handling in enum_to_bit() + +The implementation returns bit 0 for unknown enum values: +```cpp +return 0; // Unknown values map to bit 0 +``` + +This means an unknown ColorMode maps to the same bit as `ColorMode::UNKNOWN`. This is acceptable because: +- Compile-time failure occurs if using invalid enum values +- `ColorMode::UNKNOWN` at bit 0 is semantically correct +- Runtime misuse is prevented by type safety + +## API Compatibility with std::set + +EnumBitmask provides both modern `.contains()` / `.add()` / `.remove()` methods and std::set-compatible aliases for drop-in replacement: + +| Operation | std::set | EnumBitmask | Notes | +|-----------|----------|-------------|-------| +| Add value | `.insert(value)` | `.insert(value)` or `.add(value)` | Both work | +| Check membership | `.count(value)` | `.count(value)` or `.contains(value)` | Both work | +| Remove value | `.erase(value)` | `.erase(value)` or `.remove(value)` | Both work | +| Count elements | `.size()` | `.size()` | Same | +| Check empty | `.empty()` | `.empty()` | Same | +| Clear all | `.clear()` | `.clear()` | Same | +| Iterate | `for (auto v : set)` | `for (auto v : mask)` | Same | + +**Drop-in replacement**: You can use either the std::set-compatible methods (`.insert()`, `.count()`, `.erase()`) or the more explicit methods (`.add()`, `.contains()`, `.remove()`). + +## Complete Usage Example + +See `esphome/components/light/color_mode.h` for a complete real-world implementation showing: +- Lookup table definition +- Type aliases +- Template specializations +- Helper functions using the bitmask + +## Common Patterns + +### Compile-Time Initialization + +```cpp +// Constexpr-compatible for compile-time initialization +constexpr ColorModeMask DEFAULT_MODES({ColorMode::ON_OFF, ColorMode::BRIGHTNESS}); +``` + +### Adding Multiple Values + +```cpp +ColorModeMask modes; +modes.add({ColorMode::RGB, ColorMode::WHITE, ColorMode::COLOR_TEMPERATURE}); +``` + +### Checking and Iterating + +```cpp +if (modes.contains(ColorMode::RGB)) { + // RGB mode is supported +} + +for (auto mode : modes) { + // Process each supported mode + ESP_LOGD(TAG, "Supported mode: %d", static_cast(mode)); +} +``` + +### Working with Raw Bitmask Values + +```cpp +// Get raw bitmask for bitwise operations +auto mask = modes.get_mask(); + +// Check if raw bitmask contains a value +if (ColorModeMask::mask_contains(mask, ColorMode::RGB)) { ... } + +// Get first value from raw bitmask +auto first = ColorModeMask::first_value_from_mask(mask); +``` + +## Detection of Opportunities + +Look for these patterns in existing code: +- `std::set` with small enum sets (≤32 values) +- Components storing "supported modes" or "capabilities" +- Red-black tree code (`rb_tree`, `_Rb_tree`) in compiler output +- Flash size increases when adding enum set storage + +## When NOT to Use + +- Enum has >32 distinct values (bitmask limitation) +- Need to store arbitrary runtime-determined integer values (not enum values) +- Enum values are sparse or non-sequential and lookup table would be impractical +- Code readability matters more than memory savings (niche single-use components) diff --git a/esphome/core/enum_bitmask.h b/esphome/core/enum_bitmask.h index fdbd0c50cc..d5d531763e 100644 --- a/esphome/core/enum_bitmask.h +++ b/esphome/core/enum_bitmask.h @@ -62,9 +62,15 @@ template class EnumBitmask { } } + /// std::set compatibility: insert() is an alias for add() + constexpr void insert(EnumType value) { this->add(value); } + /// Remove an enum value from the set constexpr void remove(EnumType value) { this->mask_ &= ~(static_cast(1) << enum_to_bit(value)); } + /// std::set compatibility: erase() is an alias for remove() + constexpr void erase(EnumType value) { this->remove(value); } + /// Clear all values from the set constexpr void clear() { this->mask_ = 0; } @@ -73,6 +79,9 @@ template class EnumBitmask { return (this->mask_ & (static_cast(1) << enum_to_bit(value))) != 0; } + /// std::set compatibility: count() returns 1 if present, 0 if not (same as std::set for unique elements) + constexpr size_t count(EnumType value) const { return this->contains(value) ? 1 : 0; } + /// Count the number of enum values in the set constexpr size_t size() const { // Brian Kernighan's algorithm - efficient for sparse bitmasks diff --git a/extract_color_mode_mask_helper_pr.md b/extract_color_mode_mask_helper_pr.md new file mode 100644 index 0000000000..6a4d98a5f8 --- /dev/null +++ b/extract_color_mode_mask_helper_pr.md @@ -0,0 +1,98 @@ +# What does this implement/fix? + +This PR extracts the `ColorModeMask` implementation from the light component into a generic `EnumBitmask` template helper in `esphome/core/enum_bitmask.h`. This refactoring enables code reuse across other components (e.g., climate, fan) that need efficient enum set storage without STL container overhead. + +## Key Benefits + +- **Code Reuse**: Generic template can be used by any component needing enum bitmask storage (climate, fan, cover, etc.) +- **Memory Efficiency**: Replaces `std::set` with compact bitmask storage (~586 bytes saved per instance) +- **Zero-cost Abstraction**: Maintains same performance characteristics with cleaner, more maintainable code +- **Flash Savings**: 16 bytes reduction on ESP8266 in initial testing + +## Technical Changes + +1. **New Generic Template** (`esphome/core/enum_bitmask.h`): + - `EnumBitmask` template class + - Auto-selects optimal storage type (uint8_t/uint16_t/uint32_t) based on MaxBits + - Provides iterator support, initializer list construction, and static utility methods + - Requires specialization of `enum_to_bit()` and `bit_to_enum()` for each enum type + +2. **std::set Compatibility**: + - Provides both modern API (`.contains()`, `.add()`, `.remove()`) and std::set-compatible aliases (`.count()`, `.insert()`, `.erase()`) + - True drop-in replacement - existing code using `.insert()` and `.count()` works unchanged + +3. **Light Component Refactoring** (`esphome/components/light/color_mode.h`): + - Replaced custom `ColorModeMask` class with `using ColorModeMask = EnumBitmask` + - Single shared `COLOR_MODE_LOOKUP` array eliminates code duplication + - Template specializations provide enum↔bit mapping + - Moved `has_capability()` to namespace-level function for cleaner API + +4. **Updated Call Sites**: + - `light_call.cpp`: Uses `ColorModeMask::first_value_from_mask()` and `ColorModeMask::mask_contains()` static methods + - `light_traits.h`: Uses namespace-level `has_capability()` function + - No changes required to other light component files (drop-in replacement) + +## Design Rationale + +The generic template follows the same pattern as the original `ColorModeMask` but makes it reusable: +- Constexpr-compatible for compile-time initialization +- Iterator support for range-based for loops and API encoding +- Static methods for working with raw bitmask values (for bitwise operation results) +- Protected specialization interface ensures type safety + +This establishes a pattern that can be applied to other components: +- Climate modes/presets (upcoming PR) +- Fan modes +- Cover operations +- Any component with small enum sets (≤32 values) + +## Types of changes + +- [x] Code quality improvements to existing code or addition of tests + +**Related issue or feature (if applicable):** + +- Part of ongoing memory optimization effort for embedded platforms + +**Pull request in [esphome-docs](https://github.com/esphome/esphome-docs) with documentation (if applicable):** + +- N/A (internal refactoring, no user-facing changes) + +## Test Environment + +- [x] ESP32 +- [x] ESP32 IDF +- [x] ESP8266 +- [ ] RP2040 +- [ ] BK72xx +- [ ] RTL87xx +- [ ] nRF52840 + +## Example entry for `config.yaml`: + +```yaml +# No config changes required - internal refactoring only +# All existing light configurations continue to work unchanged + +light: + - platform: rgb + id: test_rgb_light + name: "Test RGB Light" + red: red_output + green: green_output + blue: blue_output +``` + +## Checklist: + - [x] The code change is tested and works locally. + - [x] Tests have been added to verify that the new code works (under `tests/` folder). + +If user exposed functionality or configuration variables are added/changed: + - [ ] Documentation added/updated in [esphome-docs](https://github.com/esphome/esphome-docs). + +## Additional Notes + +- **Zero functional changes**: This is a pure refactoring with identical runtime behavior +- **Binary size impact**: Slight improvement on ESP8266 (16 bytes flash reduction) +- **Future work**: Will apply this pattern to climate component in follow-up PR +- **Test coverage**: All modified code covered by existing light component tests From f8f967b25c0304ef00cd26f313eb16bc5306034a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 22:25:57 -1000 Subject: [PATCH 10/11] wi --- enum_templates.md | 200 --------------------------- esphome/core/enum_bitmask.h | 32 ++--- extract_color_mode_mask_helper_pr.md | 98 ------------- 3 files changed, 12 insertions(+), 318 deletions(-) delete mode 100644 enum_templates.md delete mode 100644 extract_color_mode_mask_helper_pr.md diff --git a/enum_templates.md b/enum_templates.md deleted file mode 100644 index 175f8d0b89..0000000000 --- a/enum_templates.md +++ /dev/null @@ -1,200 +0,0 @@ -# EnumBitmask Pattern Documentation - -## Overview - -`EnumBitmask` from `esphome/core/enum_bitmask.h` provides a memory-efficient replacement for `std::set` when storing sets of enum values. - -## When to Use - -Use `EnumBitmask` instead of `std::set` when: -- Storing sets of enum values (e.g., supported modes, capabilities) -- Enum has ≤32 distinct values -- Memory efficiency is important (saves ~586 bytes per `std::set` instance) - -## Benefits - -- **Memory Savings**: Eliminates red-black tree overhead (~586 bytes per instance) -- **Compact Storage**: 1-4 bytes depending on enum count (uint8_t/uint16_t/uint32_t) -- **Constexpr-Compatible**: Supports compile-time initialization -- **Efficient Iteration**: Only visits set bits, not all possible enum values -- **Range-Based Loops**: `for (auto value : mask)` works seamlessly - -## Requirements - -1. Enum must have sequential values (or use a lookup table for mapping) -2. Maximum 32 enum values (uint32_t bitmask limitation) -3. Must provide template specializations for `enum_to_bit()` and `bit_to_enum()` - -## Basic Usage Example - -```cpp -// Bad - red-black tree overhead (~586 bytes) -std::set supported_modes; -supported_modes.insert(ColorMode::RGB); -supported_modes.insert(ColorMode::WHITE); -if (supported_modes.count(ColorMode::RGB)) { ... } - -// Good - compact bitmask storage (2-4 bytes) -ColorModeMask supported_modes({ColorMode::RGB, ColorMode::WHITE}); -if (supported_modes.contains(ColorMode::RGB)) { ... } -for (auto mode : supported_modes) { ... } // Iterate over set values -``` - -## Implementation Pattern - -### 1. Define the Lookup Table - -If enum values aren't sequential from 0, create a lookup table: - -```cpp -// In your component header (e.g., esphome/components/light/color_mode.h) -constexpr ColorMode COLOR_MODE_LOOKUP[10] = { - ColorMode::UNKNOWN, // bit 0 - ColorMode::ON_OFF, // bit 1 - ColorMode::BRIGHTNESS, // bit 2 - ColorMode::WHITE, // bit 3 - ColorMode::COLOR_TEMPERATURE, // bit 4 - ColorMode::COLD_WARM_WHITE, // bit 5 - ColorMode::RGB, // bit 6 - ColorMode::RGB_WHITE, // bit 7 - ColorMode::RGB_COLOR_TEMPERATURE, // bit 8 - ColorMode::RGB_COLD_WARM_WHITE, // bit 9 -}; -``` - -### 2. Create Type Alias - -```cpp -constexpr int COLOR_MODE_BITMASK_SIZE = 10; -using ColorModeMask = EnumBitmask; -``` - -### 3. Provide Template Specializations - -**IMPORTANT**: Specializations must be in the **global namespace** (C++ requirement). Place them at the end of your header file, outside your component namespace. - -```cpp -// At end of header, outside namespace esphome::light -// Template specializations for ColorMode must be in global namespace -// -// C++ requires template specializations to be declared in the same namespace as the -// original template. Since EnumBitmask is in the esphome namespace (not esphome::light), -// we must provide these specializations at global scope with fully-qualified names. -// -// These specializations define how ColorMode enum values map to/from bit positions. - -/// Map ColorMode enum values to bit positions (0-9) -template<> -constexpr int esphome::EnumBitmask::enum_to_bit( - esphome::light::ColorMode mode) { - // Map enum value to bit position (0-9) - for (int i = 0; i < esphome::light::COLOR_MODE_BITMASK_SIZE; ++i) { - if (esphome::light::COLOR_MODE_LOOKUP[i] == mode) - return i; - } - return 0; // Unknown values map to bit 0 (typically reserved for UNKNOWN/NONE) -} - -/// Map bit positions (0-9) to ColorMode enum values -template<> -inline esphome::light::ColorMode esphome::EnumBitmask::bit_to_enum(int bit) { - return (bit >= 0 && bit < esphome::light::COLOR_MODE_BITMASK_SIZE) - ? esphome::light::COLOR_MODE_LOOKUP[bit] - : esphome::light::ColorMode::UNKNOWN; -} -``` - -### Error Handling in enum_to_bit() - -The implementation returns bit 0 for unknown enum values: -```cpp -return 0; // Unknown values map to bit 0 -``` - -This means an unknown ColorMode maps to the same bit as `ColorMode::UNKNOWN`. This is acceptable because: -- Compile-time failure occurs if using invalid enum values -- `ColorMode::UNKNOWN` at bit 0 is semantically correct -- Runtime misuse is prevented by type safety - -## API Compatibility with std::set - -EnumBitmask provides both modern `.contains()` / `.add()` / `.remove()` methods and std::set-compatible aliases for drop-in replacement: - -| Operation | std::set | EnumBitmask | Notes | -|-----------|----------|-------------|-------| -| Add value | `.insert(value)` | `.insert(value)` or `.add(value)` | Both work | -| Check membership | `.count(value)` | `.count(value)` or `.contains(value)` | Both work | -| Remove value | `.erase(value)` | `.erase(value)` or `.remove(value)` | Both work | -| Count elements | `.size()` | `.size()` | Same | -| Check empty | `.empty()` | `.empty()` | Same | -| Clear all | `.clear()` | `.clear()` | Same | -| Iterate | `for (auto v : set)` | `for (auto v : mask)` | Same | - -**Drop-in replacement**: You can use either the std::set-compatible methods (`.insert()`, `.count()`, `.erase()`) or the more explicit methods (`.add()`, `.contains()`, `.remove()`). - -## Complete Usage Example - -See `esphome/components/light/color_mode.h` for a complete real-world implementation showing: -- Lookup table definition -- Type aliases -- Template specializations -- Helper functions using the bitmask - -## Common Patterns - -### Compile-Time Initialization - -```cpp -// Constexpr-compatible for compile-time initialization -constexpr ColorModeMask DEFAULT_MODES({ColorMode::ON_OFF, ColorMode::BRIGHTNESS}); -``` - -### Adding Multiple Values - -```cpp -ColorModeMask modes; -modes.add({ColorMode::RGB, ColorMode::WHITE, ColorMode::COLOR_TEMPERATURE}); -``` - -### Checking and Iterating - -```cpp -if (modes.contains(ColorMode::RGB)) { - // RGB mode is supported -} - -for (auto mode : modes) { - // Process each supported mode - ESP_LOGD(TAG, "Supported mode: %d", static_cast(mode)); -} -``` - -### Working with Raw Bitmask Values - -```cpp -// Get raw bitmask for bitwise operations -auto mask = modes.get_mask(); - -// Check if raw bitmask contains a value -if (ColorModeMask::mask_contains(mask, ColorMode::RGB)) { ... } - -// Get first value from raw bitmask -auto first = ColorModeMask::first_value_from_mask(mask); -``` - -## Detection of Opportunities - -Look for these patterns in existing code: -- `std::set` with small enum sets (≤32 values) -- Components storing "supported modes" or "capabilities" -- Red-black tree code (`rb_tree`, `_Rb_tree`) in compiler output -- Flash size increases when adding enum set storage - -## When NOT to Use - -- Enum has >32 distinct values (bitmask limitation) -- Need to store arbitrary runtime-determined integer values (not enum values) -- Enum values are sparse or non-sequential and lookup table would be impractical -- Code readability matters more than memory savings (niche single-use components) diff --git a/esphome/core/enum_bitmask.h b/esphome/core/enum_bitmask.h index d5d531763e..b3112c610b 100644 --- a/esphome/core/enum_bitmask.h +++ b/esphome/core/enum_bitmask.h @@ -23,7 +23,7 @@ namespace esphome { /// Example usage: /// using ClimateModeMask = EnumBitmask; /// ClimateModeMask modes({CLIMATE_MODE_HEAT, CLIMATE_MODE_COOL}); -/// if (modes.contains(CLIMATE_MODE_HEAT)) { ... } +/// if (modes.count(CLIMATE_MODE_HEAT)) { ... } /// for (auto mode : modes) { ... } // Iterate over set bits /// /// For complete usage examples with template specializations, see: @@ -48,40 +48,32 @@ template class EnumBitmask { /// Construct from initializer list: {VALUE1, VALUE2, ...} constexpr EnumBitmask(std::initializer_list values) { for (auto value : values) { - this->add(value); + this->insert(value); } } - /// Add a single enum value to the set - constexpr void add(EnumType value) { this->mask_ |= (static_cast(1) << enum_to_bit(value)); } + /// Add a single enum value to the set (std::set compatibility) + constexpr void insert(EnumType value) { this->mask_ |= (static_cast(1) << enum_to_bit(value)); } /// Add multiple enum values from initializer list - constexpr void add(std::initializer_list values) { + constexpr void insert(std::initializer_list values) { for (auto value : values) { - this->add(value); + this->insert(value); } } - /// std::set compatibility: insert() is an alias for add() - constexpr void insert(EnumType value) { this->add(value); } - - /// Remove an enum value from the set - constexpr void remove(EnumType value) { this->mask_ &= ~(static_cast(1) << enum_to_bit(value)); } - - /// std::set compatibility: erase() is an alias for remove() - constexpr void erase(EnumType value) { this->remove(value); } + /// Remove an enum value from the set (std::set compatibility) + constexpr void erase(EnumType value) { this->mask_ &= ~(static_cast(1) << enum_to_bit(value)); } /// Clear all values from the set constexpr void clear() { this->mask_ = 0; } - /// Check if the set contains a specific enum value - constexpr bool contains(EnumType value) const { - return (this->mask_ & (static_cast(1) << enum_to_bit(value))) != 0; + /// Check if the set contains a specific enum value (std::set compatibility) + /// Returns 1 if present, 0 if not (same as std::set for unique elements) + constexpr size_t count(EnumType value) const { + return (this->mask_ & (static_cast(1) << enum_to_bit(value))) != 0 ? 1 : 0; } - /// std::set compatibility: count() returns 1 if present, 0 if not (same as std::set for unique elements) - constexpr size_t count(EnumType value) const { return this->contains(value) ? 1 : 0; } - /// Count the number of enum values in the set constexpr size_t size() const { // Brian Kernighan's algorithm - efficient for sparse bitmasks diff --git a/extract_color_mode_mask_helper_pr.md b/extract_color_mode_mask_helper_pr.md deleted file mode 100644 index 6a4d98a5f8..0000000000 --- a/extract_color_mode_mask_helper_pr.md +++ /dev/null @@ -1,98 +0,0 @@ -# What does this implement/fix? - -This PR extracts the `ColorModeMask` implementation from the light component into a generic `EnumBitmask` template helper in `esphome/core/enum_bitmask.h`. This refactoring enables code reuse across other components (e.g., climate, fan) that need efficient enum set storage without STL container overhead. - -## Key Benefits - -- **Code Reuse**: Generic template can be used by any component needing enum bitmask storage (climate, fan, cover, etc.) -- **Memory Efficiency**: Replaces `std::set` with compact bitmask storage (~586 bytes saved per instance) -- **Zero-cost Abstraction**: Maintains same performance characteristics with cleaner, more maintainable code -- **Flash Savings**: 16 bytes reduction on ESP8266 in initial testing - -## Technical Changes - -1. **New Generic Template** (`esphome/core/enum_bitmask.h`): - - `EnumBitmask` template class - - Auto-selects optimal storage type (uint8_t/uint16_t/uint32_t) based on MaxBits - - Provides iterator support, initializer list construction, and static utility methods - - Requires specialization of `enum_to_bit()` and `bit_to_enum()` for each enum type - -2. **std::set Compatibility**: - - Provides both modern API (`.contains()`, `.add()`, `.remove()`) and std::set-compatible aliases (`.count()`, `.insert()`, `.erase()`) - - True drop-in replacement - existing code using `.insert()` and `.count()` works unchanged - -3. **Light Component Refactoring** (`esphome/components/light/color_mode.h`): - - Replaced custom `ColorModeMask` class with `using ColorModeMask = EnumBitmask` - - Single shared `COLOR_MODE_LOOKUP` array eliminates code duplication - - Template specializations provide enum↔bit mapping - - Moved `has_capability()` to namespace-level function for cleaner API - -4. **Updated Call Sites**: - - `light_call.cpp`: Uses `ColorModeMask::first_value_from_mask()` and `ColorModeMask::mask_contains()` static methods - - `light_traits.h`: Uses namespace-level `has_capability()` function - - No changes required to other light component files (drop-in replacement) - -## Design Rationale - -The generic template follows the same pattern as the original `ColorModeMask` but makes it reusable: -- Constexpr-compatible for compile-time initialization -- Iterator support for range-based for loops and API encoding -- Static methods for working with raw bitmask values (for bitwise operation results) -- Protected specialization interface ensures type safety - -This establishes a pattern that can be applied to other components: -- Climate modes/presets (upcoming PR) -- Fan modes -- Cover operations -- Any component with small enum sets (≤32 values) - -## Types of changes - -- [x] Code quality improvements to existing code or addition of tests - -**Related issue or feature (if applicable):** - -- Part of ongoing memory optimization effort for embedded platforms - -**Pull request in [esphome-docs](https://github.com/esphome/esphome-docs) with documentation (if applicable):** - -- N/A (internal refactoring, no user-facing changes) - -## Test Environment - -- [x] ESP32 -- [x] ESP32 IDF -- [x] ESP8266 -- [ ] RP2040 -- [ ] BK72xx -- [ ] RTL87xx -- [ ] nRF52840 - -## Example entry for `config.yaml`: - -```yaml -# No config changes required - internal refactoring only -# All existing light configurations continue to work unchanged - -light: - - platform: rgb - id: test_rgb_light - name: "Test RGB Light" - red: red_output - green: green_output - blue: blue_output -``` - -## Checklist: - - [x] The code change is tested and works locally. - - [x] Tests have been added to verify that the new code works (under `tests/` folder). - -If user exposed functionality or configuration variables are added/changed: - - [ ] Documentation added/updated in [esphome-docs](https://github.com/esphome/esphome-docs). - -## Additional Notes - -- **Zero functional changes**: This is a pure refactoring with identical runtime behavior -- **Binary size impact**: Slight improvement on ESP8266 (16 bytes flash reduction) -- **Future work**: Will apply this pattern to climate component in follow-up PR -- **Test coverage**: All modified code covered by existing light component tests From 9d1ceba18f9246162a8e3b8de5219877e46d3da0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 21 Oct 2025 22:28:59 -1000 Subject: [PATCH 11/11] [core] Use std::set API for EnumBitmask - Replace .contains()/.add()/.remove() with .count()/.insert()/.erase() - Makes EnumBitmask a true drop-in replacement for std::set - Update all usages in light component --- esphome/components/light/light_traits.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/light/light_traits.h b/esphome/components/light/light_traits.h index 9dec9fb577..294b0cad1d 100644 --- a/esphome/components/light/light_traits.h +++ b/esphome/components/light/light_traits.h @@ -26,7 +26,7 @@ class LightTraits { this->supported_color_modes_ = ColorModeMask(modes); } - bool supports_color_mode(ColorMode color_mode) const { return this->supported_color_modes_.contains(color_mode); } + bool supports_color_mode(ColorMode color_mode) const { return this->supported_color_modes_.count(color_mode) > 0; } bool supports_color_capability(ColorCapability color_capability) const { return has_capability(this->supported_color_modes_, color_capability); }