From e2b3617df32cd4c3696eed1aac4ddfcc38aa42a6 Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Wed, 22 Oct 2025 01:08:40 -0700 Subject: [PATCH 01/12] [climate] Fix restore state for fan mode, preset, and swing mode (#11126) Co-authored-by: J. Nick Koston Co-authored-by: J. Nick Koston --- esphome/components/climate/climate.cpp | 60 ++++++++++++-------------- esphome/components/climate/climate.h | 1 + 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/esphome/components/climate/climate.cpp b/esphome/components/climate/climate.cpp index 87d03f78c5..19fe241729 100644 --- a/esphome/components/climate/climate.cpp +++ b/esphome/components/climate/climate.cpp @@ -524,13 +524,23 @@ ClimateCall ClimateDeviceRestoreState::to_call(Climate *climate) { if (traits.has_feature_flags(climate::CLIMATE_SUPPORTS_TARGET_HUMIDITY)) { call.set_target_humidity(this->target_humidity); } - if (traits.get_supports_fan_modes() || !traits.get_supported_custom_fan_modes().empty()) { + if (this->uses_custom_fan_mode) { + if (this->custom_fan_mode < traits.get_supported_custom_fan_modes().size()) { + call.fan_mode_.reset(); + call.custom_fan_mode_ = *std::next(traits.get_supported_custom_fan_modes().cbegin(), this->custom_fan_mode); + } + } else if (traits.supports_fan_mode(this->fan_mode)) { call.set_fan_mode(this->fan_mode); } - if (traits.get_supports_presets() || !traits.get_supported_custom_presets().empty()) { + if (this->uses_custom_preset) { + if (this->custom_preset < traits.get_supported_custom_presets().size()) { + call.preset_.reset(); + call.custom_preset_ = *std::next(traits.get_supported_custom_presets().cbegin(), this->custom_preset); + } + } else if (traits.supports_preset(this->preset)) { call.set_preset(this->preset); } - if (traits.get_supports_swing_modes()) { + if (traits.supports_swing_mode(this->swing_mode)) { call.set_swing_mode(this->swing_mode); } return call; @@ -549,41 +559,25 @@ void ClimateDeviceRestoreState::apply(Climate *climate) { if (traits.has_feature_flags(climate::CLIMATE_SUPPORTS_TARGET_HUMIDITY)) { climate->target_humidity = this->target_humidity; } - if (traits.get_supports_fan_modes() && !this->uses_custom_fan_mode) { + if (this->uses_custom_fan_mode) { + if (this->custom_fan_mode < traits.get_supported_custom_fan_modes().size()) { + climate->fan_mode.reset(); + climate->custom_fan_mode = *std::next(traits.get_supported_custom_fan_modes().cbegin(), this->custom_fan_mode); + } + } else if (traits.supports_fan_mode(this->fan_mode)) { climate->fan_mode = this->fan_mode; + climate->custom_fan_mode.reset(); } - if (!traits.get_supported_custom_fan_modes().empty() && this->uses_custom_fan_mode) { - // std::set has consistent order (lexicographic for strings) - const auto &modes = traits.get_supported_custom_fan_modes(); - if (custom_fan_mode < modes.size()) { - size_t i = 0; - for (const auto &mode : modes) { - if (i == this->custom_fan_mode) { - climate->custom_fan_mode = mode; - break; - } - i++; - } + if (this->uses_custom_preset) { + if (this->custom_preset < traits.get_supported_custom_presets().size()) { + climate->preset.reset(); + climate->custom_preset = *std::next(traits.get_supported_custom_presets().cbegin(), this->custom_preset); } - } - if (traits.get_supports_presets() && !this->uses_custom_preset) { + } else if (traits.supports_preset(this->preset)) { climate->preset = this->preset; + climate->custom_preset.reset(); } - if (!traits.get_supported_custom_presets().empty() && uses_custom_preset) { - // std::set has consistent order (lexicographic for strings) - const auto &presets = traits.get_supported_custom_presets(); - if (custom_preset < presets.size()) { - size_t i = 0; - for (const auto &preset : presets) { - if (i == this->custom_preset) { - climate->custom_preset = preset; - break; - } - i++; - } - } - } - if (traits.get_supports_swing_modes()) { + if (traits.supports_swing_mode(this->swing_mode)) { climate->swing_mode = this->swing_mode; } climate->publish_state(); diff --git a/esphome/components/climate/climate.h b/esphome/components/climate/climate.h index 495464c6a2..0c3e3ebe16 100644 --- a/esphome/components/climate/climate.h +++ b/esphome/components/climate/climate.h @@ -33,6 +33,7 @@ class Climate; class ClimateCall { public: explicit ClimateCall(Climate *parent) : parent_(parent) {} + friend struct ClimateDeviceRestoreState; /// Set the mode of the climate device. ClimateCall &set_mode(ClimateMode mode); From d37eb59fd733b3dd06b2e62409249e92bde5b7b8 Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Wed, 22 Oct 2025 01:22:33 -0700 Subject: [PATCH 02/12] [light] Eliminate dimming undershoot during addressable light transition (#11471) --- .../components/light/addressable_light.cpp | 61 ++++++++++--------- esphome/components/light/addressable_light.h | 1 - 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/esphome/components/light/addressable_light.cpp b/esphome/components/light/addressable_light.cpp index cd83015ecb..5cbdcb0e86 100644 --- a/esphome/components/light/addressable_light.cpp +++ b/esphome/components/light/addressable_light.cpp @@ -61,6 +61,10 @@ void AddressableLightTransformer::start() { this->target_color_ *= to_uint8_scale(end_values.get_brightness() * end_values.get_state()); } +inline constexpr uint8_t subtract_scaled_difference(uint8_t a, uint8_t b, int32_t scale) { + return uint8_t(int32_t(a) - (((int32_t(a) - int32_t(b)) * scale) / 256)); +} + optional AddressableLightTransformer::apply() { float smoothed_progress = LightTransformer::smoothed_progress(this->get_progress_()); @@ -74,38 +78,37 @@ optional AddressableLightTransformer::apply() { // all LEDs, we use the current state of each LED as the start. // We can't use a direct lerp smoothing here though - that would require creating a copy of the original - // state of each LED at the start of the transition. - // Instead, we "fake" the look of the LERP by using an exponential average over time and using - // dynamically-calculated alpha values to match the look. + // state of each LED at the start of the transition. Instead, we "fake" the look of lerp by calculating + // the delta between the current state and the target state, assuming that the delta represents the rest + // of the transition that was to be applied as of the previous transition step, and scaling the delta for + // what should be left after the current transition step. In this manner, the delta decays to zero as the + // transition progresses. + // + // Here's an example of how the algorithm progresses in discrete steps: + // + // At time = 0.00, 0% complete, 100% remaining, 100% will remain after this step, so the scale is 100% / 100% = 100%. + // At time = 0.10, 0% complete, 100% remaining, 90% will remain after this step, so the scale is 90% / 100% = 90%. + // At time = 0.20, 10% complete, 90% remaining, 80% will remain after this step, so the scale is 80% / 90% = 88.9%. + // At time = 0.50, 20% complete, 80% remaining, 50% will remain after this step, so the scale is 50% / 80% = 62.5%. + // At time = 0.90, 50% complete, 50% remaining, 10% will remain after this step, so the scale is 10% / 50% = 20%. + // At time = 0.91, 90% complete, 10% remaining, 9% will remain after this step, so the scale is 9% / 10% = 90%. + // At time = 1.00, 91% complete, 9% remaining, 0% will remain after this step, so the scale is 0% / 9% = 0%. + // + // Because the color values are quantized to 8 bit resolution after each step, the transition may appear + // non-linear when applying small deltas. - float denom = (1.0f - smoothed_progress); - float alpha = denom == 0.0f ? 1.0f : (smoothed_progress - this->last_transition_progress_) / denom; - - // We need to use a low-resolution alpha here which makes the transition set in only after ~half of the length - // We solve this by accumulating the fractional part of the alpha over time. - float alpha255 = alpha * 255.0f; - float alpha255int = floorf(alpha255); - float alpha255remainder = alpha255 - alpha255int; - - this->accumulated_alpha_ += alpha255remainder; - float alpha_add = floorf(this->accumulated_alpha_); - this->accumulated_alpha_ -= alpha_add; - - alpha255 += alpha_add; - alpha255 = clamp(alpha255, 0.0f, 255.0f); - auto alpha8 = static_cast(alpha255); - - if (alpha8 != 0) { - uint8_t inv_alpha8 = 255 - alpha8; - Color add = this->target_color_ * alpha8; - - for (auto led : this->light_) - led.set(add + led.get() * inv_alpha8); + if (smoothed_progress > this->last_transition_progress_ && this->last_transition_progress_ < 1.f) { + int32_t scale = int32_t(256.f * std::max((1.f - smoothed_progress) / (1.f - this->last_transition_progress_), 0.f)); + for (auto led : this->light_) { + led.set_rgbw(subtract_scaled_difference(this->target_color_.red, led.get_red(), scale), + subtract_scaled_difference(this->target_color_.green, led.get_green(), scale), + subtract_scaled_difference(this->target_color_.blue, led.get_blue(), scale), + subtract_scaled_difference(this->target_color_.white, led.get_white(), scale)); + } + this->last_transition_progress_ = smoothed_progress; + this->light_.schedule_show(); } - this->last_transition_progress_ = smoothed_progress; - this->light_.schedule_show(); - return {}; } diff --git a/esphome/components/light/addressable_light.h b/esphome/components/light/addressable_light.h index c8ed4897fa..393cc679bc 100644 --- a/esphome/components/light/addressable_light.h +++ b/esphome/components/light/addressable_light.h @@ -113,7 +113,6 @@ class AddressableLightTransformer : public LightTransformer { protected: AddressableLight &light_; float last_transition_progress_{0.0f}; - float accumulated_alpha_{0.0f}; Color target_color_{}; }; From 3fda73bcf251e334fd85a881c4c067fad6ffffa4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Oct 2025 00:05:06 -1000 Subject: [PATCH 03/12] bot review --- esphome/components/light/color_mode.h | 20 ++++++++++++-------- esphome/core/enum_bitmask.h | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 03132f54bf..77c5a13a6f 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -159,16 +159,13 @@ constexpr uint16_t CAPABILITY_BITMASKS[] = { compute_capability_bitmask(ColorCapability::RGB), // 1 << 5 }; -/// 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 +/// Convert a power-of-2 ColorCapability value to an array index +/// ColorCapability values: 1, 2, 4, 8, 16, 32 -> array indices: 0, 1, 2, 3, 4, 5 +inline int capability_to_index(ColorCapability capability) { 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); + return __builtin_ctz(cap_val); #else // Fallback for compilers without __builtin_ctz int index = 0; @@ -176,8 +173,15 @@ inline bool has_capability(const ColorModeMask &mask, ColorCapability capability cap_val >>= 1; ++index; } + return index; #endif - return (mask.get_mask() & CAPABILITY_BITMASKS[index]) != 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) +inline bool has_capability(const ColorModeMask &mask, ColorCapability capability) { + // Lookup the pre-computed bitmask for this capability and check intersection with our mask + return (mask.get_mask() & CAPABILITY_BITMASKS[capability_to_index(capability)]) != 0; } } // namespace light diff --git a/esphome/core/enum_bitmask.h b/esphome/core/enum_bitmask.h index b3112c610b..f9cda7ca2d 100644 --- a/esphome/core/enum_bitmask.h +++ b/esphome/core/enum_bitmask.h @@ -151,7 +151,7 @@ template class EnumBitmask { // 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 + static EnumType bit_to_enum(int bit); // Not constexpr: array indexing with runtime bounds checking bitmask_t mask_{0}; }; From 6edbb945295188f94950288a90a8dad45171464c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Oct 2025 00:06:14 -1000 Subject: [PATCH 04/12] [ci] Fix test detection for components with only variant tests (#11474) --- script/determine-jobs.py | 4 +- tests/script/test_determine_jobs.py | 144 ++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 2 deletions(-) diff --git a/script/determine-jobs.py b/script/determine-jobs.py index 7cdec959c7..ac384d74f1 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -336,7 +336,7 @@ def _component_has_tests(component: str) -> bool: Returns: True if the component has test YAML files """ - return bool(get_component_test_files(component)) + return bool(get_component_test_files(component, all_variants=True)) def _select_platform_by_preference( @@ -496,7 +496,7 @@ def detect_memory_impact_config( for component in sorted(changed_component_set): # Look for test files on preferred platforms - test_files = get_component_test_files(component) + test_files = get_component_test_files(component, all_variants=True) if not test_files: continue diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index 6095e86ea7..c9ccf53252 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -574,6 +574,105 @@ def test_main_filters_components_without_tests( assert output["memory_impact"]["should_run"] == "false" +def test_main_detects_components_with_variant_tests( + 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, + capsys: pytest.CaptureFixture[str], + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test that components with only variant test files (test-*.yaml) are detected. + + This test verifies the fix for components like improv_serial, ethernet, mdns, + improv_base, and safe_mode which only have variant test files (test-*.yaml) + instead of base test files (test.*.yaml). + """ + # Ensure we're not in GITHUB_ACTIONS mode for this test + monkeypatch.delenv("GITHUB_ACTIONS", raising=False) + + mock_should_run_integration_tests.return_value = False + mock_should_run_clang_tidy.return_value = False + mock_should_run_clang_format.return_value = False + mock_should_run_python_linters.return_value = False + + # Mock changed_files to return component files + mock_changed_files.return_value = [ + "esphome/components/improv_serial/improv_serial.cpp", + "esphome/components/ethernet/ethernet.cpp", + "esphome/components/no_tests/component.cpp", + ] + + # Create test directory structure + tests_dir = tmp_path / "tests" / "components" + + # improv_serial has only variant tests (like the real component) + improv_serial_dir = tests_dir / "improv_serial" + improv_serial_dir.mkdir(parents=True) + (improv_serial_dir / "test-uart0.esp32-idf.yaml").write_text("test: config") + (improv_serial_dir / "test-uart0.esp8266-ard.yaml").write_text("test: config") + (improv_serial_dir / "test-usb_cdc.esp32-s2-idf.yaml").write_text("test: config") + + # ethernet also has only variant tests + ethernet_dir = tests_dir / "ethernet" + ethernet_dir.mkdir(parents=True) + (ethernet_dir / "test-manual_ip.esp32-idf.yaml").write_text("test: config") + (ethernet_dir / "test-dhcp.esp32-idf.yaml").write_text("test: config") + + # no_tests component has no test files at all + no_tests_dir = tests_dir / "no_tests" + no_tests_dir.mkdir(parents=True) + + # Mock root_path to use tmp_path (need to patch both determine_jobs and helpers) + with ( + patch.object(determine_jobs, "root_path", str(tmp_path)), + patch.object(helpers, "root_path", str(tmp_path)), + patch("sys.argv", ["determine-jobs.py"]), + patch.object( + determine_jobs, + "get_changed_components", + return_value=["improv_serial", "ethernet", "no_tests"], + ), + 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: ( + ["improv_serial", "ethernet"] + if not deps + else ["improv_serial", "ethernet", "no_tests"] + ), + ), + patch.object(determine_jobs, "changed_files", return_value=[]), + ): + # Clear the cache since we're mocking root_path + determine_jobs._component_has_tests.cache_clear() + determine_jobs.main() + + # Check output + captured = capsys.readouterr() + output = json.loads(captured.out) + + # changed_components should have all components + assert set(output["changed_components"]) == { + "improv_serial", + "ethernet", + "no_tests", + } + # changed_components_with_tests should include components with variant tests + assert set(output["changed_components_with_tests"]) == {"improv_serial", "ethernet"} + # component_test_count should be 2 (improv_serial and ethernet) + assert output["component_test_count"] == 2 + # no_tests should be excluded since it has no test files + assert "no_tests" not in output["changed_components_with_tests"] + + # Tests for detect_memory_impact_config function @@ -785,6 +884,51 @@ def test_detect_memory_impact_config_skips_base_bus_components(tmp_path: Path) - assert "i2c" not in result["components"] +def test_detect_memory_impact_config_with_variant_tests(tmp_path: Path) -> None: + """Test memory impact detection for components with only variant test files. + + This verifies that memory impact analysis works correctly for components like + improv_serial, ethernet, mdns, etc. which only have variant test files + (test-*.yaml) instead of base test files (test.*.yaml). + """ + # Create test directory structure + tests_dir = tmp_path / "tests" / "components" + + # improv_serial with only variant tests + improv_serial_dir = tests_dir / "improv_serial" + improv_serial_dir.mkdir(parents=True) + (improv_serial_dir / "test-uart0.esp32-idf.yaml").write_text("test: improv") + (improv_serial_dir / "test-uart0.esp8266-ard.yaml").write_text("test: improv") + (improv_serial_dir / "test-usb_cdc.esp32-s2-idf.yaml").write_text("test: improv") + + # ethernet with only variant tests + ethernet_dir = tests_dir / "ethernet" + ethernet_dir.mkdir(parents=True) + (ethernet_dir / "test-manual_ip.esp32-idf.yaml").write_text("test: ethernet") + (ethernet_dir / "test-dhcp.esp32-c3-idf.yaml").write_text("test: ethernet") + + # Mock changed_files to return both components + with ( + patch.object(determine_jobs, "root_path", str(tmp_path)), + patch.object(helpers, "root_path", str(tmp_path)), + patch.object(determine_jobs, "changed_files") as mock_changed_files, + ): + mock_changed_files.return_value = [ + "esphome/components/improv_serial/improv_serial.cpp", + "esphome/components/ethernet/ethernet.cpp", + ] + determine_jobs._component_has_tests.cache_clear() + + result = determine_jobs.detect_memory_impact_config() + + # Should detect both components even though they only have variant tests + assert result["should_run"] == "true" + assert set(result["components"]) == {"improv_serial", "ethernet"} + # Both components support esp32-idf + assert result["platform"] == "esp32-idf" + assert result["use_merged_config"] == "true" + + # Tests for clang-tidy split mode logic From f592f79bcece7edb4810f09ebdb6e3f25267b398 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Oct 2025 07:30:27 -1000 Subject: [PATCH 05/12] [ci] Fix component splitter for components with only variant tests (#11476) --- script/split_components_for_ci.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/script/split_components_for_ci.py b/script/split_components_for_ci.py index 6ba2598eda..c58dfd218f 100755 --- a/script/split_components_for_ci.py +++ b/script/split_components_for_ci.py @@ -49,9 +49,9 @@ def has_test_files(component_name: str, tests_dir: Path) -> bool: tests_dir: Path to tests/components directory (unused, kept for compatibility) Returns: - True if the component has test.*.yaml files + True if the component has test.*.yaml or test-*.yaml files """ - return bool(get_component_test_files(component_name)) + return bool(get_component_test_files(component_name, all_variants=True)) def create_intelligent_batches( From 77141d3e83e1bb11107255423556373ba588d3ca Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 22 Oct 2025 14:28:18 -0400 Subject: [PATCH 06/12] [esp32] Set the location of the IDF component manager cache (#11467) --- esphome/components/esp32/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index cb6354cc74..48d11f46fa 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -877,6 +877,11 @@ async def to_code(config): for clean_var in ("IDF_PATH", "IDF_TOOLS_PATH"): os.environ.pop(clean_var, None) + # Set the location of the IDF component manager cache + os.environ["IDF_COMPONENT_CACHE_PATH"] = str( + CORE.relative_internal_path(".espressif") + ) + add_extra_script( "post", "post_build.py", From 92a812e154157ff83168bd9af4fd214831571d96 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Oct 2025 08:30:17 -1000 Subject: [PATCH 07/12] optimize --- esphome/core/enum_bitmask.h | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/esphome/core/enum_bitmask.h b/esphome/core/enum_bitmask.h index f9cda7ca2d..e1840a3ac0 100644 --- a/esphome/core/enum_bitmask.h +++ b/esphome/core/enum_bitmask.h @@ -92,6 +92,7 @@ template class EnumBitmask { /// Iterator support for range-based for loops and API encoding /// Iterates over set bits and converts bit positions to enum values + /// Optimization: removes bits from mask as we iterate class Iterator { public: using iterator_category = std::forward_iterator_tag; @@ -100,29 +101,29 @@ template class EnumBitmask { using pointer = const EnumType *; using reference = EnumType; - constexpr Iterator(bitmask_t mask, int bit) : mask_(mask), bit_(bit) { advance_to_next_set_bit_(); } + constexpr explicit Iterator(bitmask_t mask) : mask_(mask) {} - constexpr EnumType operator*() const { return bit_to_enum(bit_); } + constexpr EnumType operator*() const { + // Return enum for the first set bit + return bit_to_enum(find_next_set_bit(mask_, 0)); + } constexpr Iterator &operator++() { - ++bit_; - advance_to_next_set_bit_(); + // Clear the lowest set bit (Brian Kernighan's algorithm) + mask_ &= mask_ - 1; return *this; } - constexpr bool operator==(const Iterator &other) const { return bit_ == other.bit_; } + constexpr bool operator==(const Iterator &other) const { return mask_ == other.mask_; } 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); } + constexpr Iterator begin() const { return Iterator(mask_); } + constexpr Iterator end() const { return Iterator(0); } /// Get the raw bitmask value for optimized operations constexpr bitmask_t get_mask() const { return this->mask_; } From c70a3cf405a89a5a1b7f0300342c37763d526af6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Oct 2025 08:44:08 -1000 Subject: [PATCH 08/12] feedback --- esphome/components/light/color_mode.h | 14 ++-- .../{enum_bitmask.h => finite_set_mask.h} | 77 ++++++++++--------- 2 files changed, 46 insertions(+), 45 deletions(-) rename esphome/core/{enum_bitmask.h => finite_set_mask.h} (56%) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 77c5a13a6f..61f12f559c 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -1,7 +1,7 @@ #pragma once #include -#include "esphome/core/enum_bitmask.h" +#include "esphome/core/finite_set_mask.h" namespace esphome { namespace light { @@ -127,8 +127,8 @@ constexpr ColorMode COLOR_MODE_LOOKUP[COLOR_MODE_BITMASK_SIZE] = { ColorMode::RGB_COLD_WARM_WHITE, // bit 9 }; -// Type alias for ColorMode bitmask using generic EnumBitmask template -using ColorModeMask = EnumBitmask; +// Type alias for ColorMode bitmask using generic FiniteSetMask template +using ColorModeMask = FiniteSetMask; // Number of ColorCapability enum values constexpr int COLOR_CAPABILITY_COUNT = 6; @@ -190,7 +190,7 @@ inline bool has_capability(const ColorModeMask &mask, ColorCapability capability // 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), +// original template. Since FiniteSetMask 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. @@ -198,7 +198,7 @@ inline bool has_capability(const ColorModeMask &mask, ColorCapability capability /// Map ColorMode enum values to bit positions (0-9) /// Bit positions follow the enum declaration order template<> -constexpr int esphome::EnumBitmask::enum_to_bit( +constexpr int esphome::FiniteSetMask::value_to_bit( esphome::light::ColorMode mode) { // Linear search through COLOR_MODE_LOOKUP array // Compiler optimizes this to efficient code since array is constexpr @@ -212,8 +212,8 @@ constexpr int esphome::EnumBitmask -inline esphome::light::ColorMode esphome::EnumBitmask::bit_to_enum(int bit) { +inline esphome::light::ColorMode esphome::FiniteSetMask< + esphome::light::ColorMode, esphome::light::COLOR_MODE_BITMASK_SIZE>::bit_to_value(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/core/enum_bitmask.h b/esphome/core/finite_set_mask.h similarity index 56% rename from esphome/core/enum_bitmask.h rename to esphome/core/finite_set_mask.h index e1840a3ac0..e6e7564d4b 100644 --- a/esphome/core/enum_bitmask.h +++ b/esphome/core/finite_set_mask.h @@ -8,34 +8,35 @@ 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). +/// Generic bitmask for storing a finite set of discrete 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) +/// ValueType: The type to store (typically enum, but can be any discrete bounded type) /// 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 +/// - ValueType must have a bounded discrete range that maps to bit positions +/// - Specialization must provide value_to_bit() and bit_to_value() static methods +/// - MaxBits must be sufficient to hold all possible values /// /// Example usage: -/// using ClimateModeMask = EnumBitmask; +/// using ClimateModeMask = FiniteSetMask; /// ClimateModeMask modes({CLIMATE_MODE_HEAT, CLIMATE_MODE_COOL}); /// if (modes.count(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) +/// - esphome/components/light/color_mode.h (ColorMode enum 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 +/// - Iterator converts bit positions to actual values during traversal /// - All operations are constexpr-compatible for compile-time initialization -/// - Drop-in replacement for std::set with simpler API +/// - Drop-in replacement for std::set with simpler API +/// - Despite the name, works with any discrete bounded type, not just enums /// -template class EnumBitmask { +template class FiniteSetMask { public: // Automatic bitmask type selection based on MaxBits // ≤8 bits: uint8_t, ≤16 bits: uint16_t, otherwise: uint32_t @@ -43,38 +44,38 @@ template class EnumBitmask { typename std::conditional<(MaxBits <= 8), uint8_t, typename std::conditional<(MaxBits <= 16), uint16_t, uint32_t>::type>::type; - constexpr EnumBitmask() = default; + constexpr FiniteSetMask() = default; /// Construct from initializer list: {VALUE1, VALUE2, ...} - constexpr EnumBitmask(std::initializer_list values) { + constexpr FiniteSetMask(std::initializer_list values) { for (auto value : values) { this->insert(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 a single value to the set (std::set compatibility) + constexpr void insert(ValueType value) { this->mask_ |= (static_cast(1) << value_to_bit(value)); } - /// Add multiple enum values from initializer list - constexpr void insert(std::initializer_list values) { + /// Add multiple values from initializer list + constexpr void insert(std::initializer_list values) { for (auto value : values) { this->insert(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)); } + /// Remove a value from the set (std::set compatibility) + constexpr void erase(ValueType value) { this->mask_ &= ~(static_cast(1) << value_to_bit(value)); } /// Clear all values from the set constexpr void clear() { this->mask_ = 0; } - /// Check if the set contains a specific enum value (std::set compatibility) + /// Check if the set contains a specific 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; + constexpr size_t count(ValueType value) const { + return (this->mask_ & (static_cast(1) << value_to_bit(value))) != 0 ? 1 : 0; } - /// Count the number of enum values in the set + /// Count the number of 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 @@ -91,21 +92,21 @@ template class EnumBitmask { 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 + /// Iterates over set bits and converts bit positions to values /// Optimization: removes bits from mask as we iterate class Iterator { public: using iterator_category = std::forward_iterator_tag; - using value_type = EnumType; + using value_type = ValueType; using difference_type = std::ptrdiff_t; - using pointer = const EnumType *; - using reference = EnumType; + using pointer = const ValueType *; + using reference = ValueType; constexpr explicit Iterator(bitmask_t mask) : mask_(mask) {} - constexpr EnumType operator*() const { - // Return enum for the first set bit - return bit_to_enum(find_next_set_bit(mask_, 0)); + constexpr ValueType operator*() const { + // Return value for the first set bit + return bit_to_value(find_next_set_bit(mask_, 0)); } constexpr Iterator &operator++() { @@ -128,15 +129,15 @@ template class EnumBitmask { /// 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 + /// Check if a specific 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; + static constexpr bool mask_contains(bitmask_t mask, ValueType value) { + return (mask & (static_cast(1) << value_to_bit(value))) != 0; } - /// Get the first enum value from a raw bitmask + /// Get the first 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)); } + static constexpr ValueType first_value_from_mask(bitmask_t mask) { return bit_to_value(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 @@ -150,9 +151,9 @@ template class EnumBitmask { 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: array indexing with runtime bounds checking + // These convert between values and bit positions (0, 1, 2, ...) + static constexpr int value_to_bit(ValueType value); + static ValueType bit_to_value(int bit); // Not constexpr: array indexing with runtime bounds checking bitmask_t mask_{0}; }; From 753662feaab5b3df9c98dcf3eb6e9cdea4044964 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Oct 2025 08:47:18 -1000 Subject: [PATCH 09/12] preen --- esphome/components/light/color_mode.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 61f12f559c..71b79ea506 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -105,7 +105,7 @@ constexpr ColorModeHelper operator|(ColorModeHelper lhs, ColorMode rhs) { return static_cast(static_cast(lhs) | static_cast(rhs)); } -// Type alias for raw color mode bitmask values (retained for compatibility) +// Type alias for raw color mode bitmask values using color_mode_bitmask_t = uint16_t; // Number of ColorMode enum values From 02a8024e9499ae5404f6c34ce95fc025822ec7c7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Oct 2025 08:54:21 -1000 Subject: [PATCH 10/12] Update esphome/components/light/color_mode.h Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/components/light/color_mode.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 71b79ea506..1f64b22e82 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -159,8 +159,15 @@ constexpr uint16_t CAPABILITY_BITMASKS[] = { compute_capability_bitmask(ColorCapability::RGB), // 1 << 5 }; -/// Convert a power-of-2 ColorCapability value to an array index -/// ColorCapability values: 1, 2, 4, 8, 16, 32 -> array indices: 0, 1, 2, 3, 4, 5 +/** + * @brief Helper function to convert a power-of-2 ColorCapability value to an array index for CAPABILITY_BITMASKS lookup. + * + * This function maps ColorCapability values (1, 2, 4, 8, 16, 32) to array indices (0, 1, 2, 3, 4, 5). + * Used to index into the CAPABILITY_BITMASKS lookup table. + * + * @param capability A ColorCapability enum value (must be a power of 2). + * @return The corresponding array index (0-based). + */ inline int capability_to_index(ColorCapability capability) { uint8_t cap_val = static_cast(capability); #if defined(__GNUC__) || defined(__clang__) From a335aa0713b5d8905c4baa416639fbc0b51491fa Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Wed, 22 Oct 2025 18:56:11 +0000 Subject: [PATCH 11/12] [pre-commit.ci lite] apply automatic fixes --- esphome/components/light/color_mode.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 1f64b22e82..963c36c2a6 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -160,7 +160,8 @@ constexpr uint16_t CAPABILITY_BITMASKS[] = { }; /** - * @brief Helper function to convert a power-of-2 ColorCapability value to an array index for CAPABILITY_BITMASKS lookup. + * @brief Helper function to convert a power-of-2 ColorCapability value to an array index for CAPABILITY_BITMASKS + * lookup. * * This function maps ColorCapability values (1, 2, 4, 8, 16, 32) to array indices (0, 1, 2, 3, 4, 5). * Used to index into the CAPABILITY_BITMASKS lookup table. From bc7cc066a5630776c6b61b66a5385cd6781b8de7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Oct 2025 09:54:47 -1000 Subject: [PATCH 12/12] backmerge --- esphome/core/finite_set_mask.h | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/esphome/core/finite_set_mask.h b/esphome/core/finite_set_mask.h index e6e7564d4b..ab2454508f 100644 --- a/esphome/core/finite_set_mask.h +++ b/esphome/core/finite_set_mask.h @@ -17,17 +17,20 @@ namespace esphome { /// /// Requirements: /// - ValueType must have a bounded discrete range that maps to bit positions -/// - Specialization must provide value_to_bit() and bit_to_value() static methods +/// - For 1:1 mappings (contiguous enums starting at 0), no specialization needed +/// - For custom mappings (like ColorMode), specialize value_to_bit() and/or bit_to_value() /// - MaxBits must be sufficient to hold all possible values /// -/// Example usage: -/// using ClimateModeMask = FiniteSetMask; +/// Example usage (1:1 mapping - climate enums): +/// // For enums with contiguous values starting at 0, no specialization needed! +/// using ClimateModeMask = FiniteSetMask; /// ClimateModeMask modes({CLIMATE_MODE_HEAT, CLIMATE_MODE_COOL}); /// if (modes.count(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 enum example) +/// Example usage (custom mapping - ColorMode): +/// // For non-contiguous enums or custom mappings, specialize value_to_bit() and/or bit_to_value() +/// // See esphome/components/light/color_mode.h for complete example /// /// Design notes: /// - Uses compile-time type selection for optimal size (uint8_t/uint16_t/uint32_t) @@ -150,10 +153,11 @@ template class FiniteSetMask { } protected: - // Must be provided by template specialization - // These convert between values and bit positions (0, 1, 2, ...) - static constexpr int value_to_bit(ValueType value); - static ValueType bit_to_value(int bit); // Not constexpr: array indexing with runtime bounds checking + // Default implementations for 1:1 mapping (enum value = bit position) + // For enums with contiguous values starting at 0, these defaults work as-is. + // If you need custom mapping (like ColorMode), provide specializations. + static constexpr int value_to_bit(ValueType value) { return static_cast(value); } + static constexpr ValueType bit_to_value(int bit) { return static_cast(bit); } bitmask_t mask_{0}; };