From e27472b87db519abd0bb8cfdafb8ca0fb691ef40 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 12:35:13 -1000 Subject: [PATCH 01/18] fixes --- esphome/components/api/api.proto | 2 +- esphome/components/api/api_connection.cpp | 5 +- esphome/components/api/api_options.proto | 17 +++--- esphome/components/api/api_pb2.cpp | 14 ++--- esphome/components/api/api_pb2.h | 2 +- esphome/components/api/api_pb2_dump.cpp | 6 +-- esphome/components/light/color_mode.h | 2 + script/api_protobuf/api_protobuf.py | 65 ++++++----------------- 8 files changed, 38 insertions(+), 75 deletions(-) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index c64fc038d6..d202486cfa 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -506,7 +506,7 @@ message ListEntitiesLightResponse { string name = 3; reserved 4; // Deprecated: was string unique_id - repeated ColorMode supported_color_modes = 12 [(enum_as_bitmask) = true]; + repeated ColorMode supported_color_modes = 12 [(container_pointer_no_template) = "light::ColorModeMask"]; // next four supports_* are for legacy clients, newer clients should use color modes // Deprecated in API version 1.6 bool legacy_supports_brightness = 5 [deprecated=true]; diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 8be96c641b..c8a1d85ef1 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -476,9 +476,8 @@ uint16_t APIConnection::try_send_light_info(EntityBase *entity, APIConnection *c auto *light = static_cast(entity); ListEntitiesLightResponse msg; auto traits = light->get_traits(); - // msg.supported_color_modes is uint32_t, but get_mask() returns uint16_t - // The upper 16 bits are zero-extended during assignment (ColorMode only has 10 values) - msg.supported_color_modes = traits.get_supported_color_modes().get_mask(); + // Pass pointer to ColorModeMask so the iterator can encode actual ColorMode enum values + msg.supported_color_modes = &traits.get_supported_color_modes(); if (traits.supports_color_capability(light::ColorCapability::COLOR_TEMPERATURE) || traits.supports_color_capability(light::ColorCapability::COLD_WARM_WHITE)) { msg.min_mireds = traits.get_min_mireds(); diff --git a/esphome/components/api/api_options.proto b/esphome/components/api/api_options.proto index 450b5e83de..6b33408e2f 100644 --- a/esphome/components/api/api_options.proto +++ b/esphome/components/api/api_options.proto @@ -71,12 +71,13 @@ extend google.protobuf.FieldOptions { // and is ideal when the exact size is known before populating the array. optional bool fixed_vector = 50013 [default=false]; - // enum_as_bitmask: Encode repeated enum fields as a uint32_t bitmask - // When set on a repeated enum field, the field will be stored as a single uint32_t - // where each bit represents whether that enum value is present. This is ideal for - // enums with ≤32 values and eliminates all vector template instantiation overhead. - // The enum values should be sequential starting from 0. - // Encoding: bit N set means enum value N is present in the set. - // Example: {ColorMode::RGB, ColorMode::WHITE} → bitmask with bits 5 and 6 set - optional bool enum_as_bitmask = 50014 [default=false]; + // container_pointer_no_template: Use a non-template container type for repeated fields + // Similar to container_pointer, but for containers that don't take template parameters. + // The container type is used as-is without appending element type. + // The container must have: + // - begin() and end() methods returning iterators + // - empty() method + // Example: [(container_pointer_no_template) = "light::ColorModeMask"] + // generates: const light::ColorModeMask *supported_color_modes{}; + optional string container_pointer_no_template = 50014; } diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index c7b88bb312..37bcf5d8a0 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -471,10 +471,8 @@ void ListEntitiesLightResponse::encode(ProtoWriteBuffer buffer) const { buffer.encode_string(1, this->object_id_ref_); buffer.encode_fixed32(2, this->key); buffer.encode_string(3, this->name_ref_); - for (uint8_t bit = 0; bit < 32; bit++) { - if (this->supported_color_modes & (1U << bit)) { - buffer.encode_uint32(12, bit, true); - } + for (const auto &it : *this->supported_color_modes) { + buffer.encode_uint32(12, static_cast(it), true); } buffer.encode_float(9, this->min_mireds); buffer.encode_float(10, this->max_mireds); @@ -494,11 +492,9 @@ void ListEntitiesLightResponse::calculate_size(ProtoSize &size) const { size.add_length(1, this->object_id_ref_.size()); size.add_fixed32(1, this->key); size.add_length(1, this->name_ref_.size()); - if (this->supported_color_modes != 0) { - for (uint8_t bit = 0; bit < 32; bit++) { - if (this->supported_color_modes & (1U << bit)) { - size.add_uint32_force(1, static_cast(bit)); - } + if (!this->supported_color_modes->empty()) { + for (const auto &it : *this->supported_color_modes) { + size.add_uint32_force(1, static_cast(it)); } } size.add_float(1, this->min_mireds); diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 5b86b7f276..ed49498176 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -790,7 +790,7 @@ class ListEntitiesLightResponse final : public InfoResponseProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP const char *message_name() const override { return "list_entities_light_response"; } #endif - uint32_t supported_color_modes{}; + const light::ColorModeMask *supported_color_modes{}; float min_mireds{0.0f}; float max_mireds{0.0f}; std::vector effects{}; diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index 69143f50f8..e803125f53 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -913,9 +913,9 @@ void ListEntitiesLightResponse::dump_to(std::string &out) const { dump_field(out, "object_id", this->object_id_ref_); dump_field(out, "key", this->key); dump_field(out, "name", this->name_ref_); - char buffer[64]; - snprintf(buffer, sizeof(buffer), " supported_color_modes: 0x%08" PRIX32 "\n", this->supported_color_modes); - out.append(buffer); + for (const auto &it : *this->supported_color_modes) { + dump_field(out, "supported_color_modes", static_cast(it), 4); + } dump_field(out, "min_mireds", this->min_mireds); dump_field(out, "max_mireds", this->max_mireds); for (const auto &it : this->effects) { diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 77be58bb3b..1241d59627 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -132,6 +132,8 @@ class ColorModeMask { return count; } + constexpr bool empty() const { return this->mask_ == 0; } + /// Iterator support for API encoding class Iterator { public: diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 075efe88f9..2f83b0bd79 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -1415,11 +1415,15 @@ class RepeatedTypeInfo(TypeInfo): super().__init__(field) # Check if this is a pointer field by looking for container_pointer option self._container_type = get_field_opt(field, pb.container_pointer, "") - self._use_pointer = bool(self._container_type) + # Check for non-template container pointer + self._container_no_template = get_field_opt( + field, pb.container_pointer_no_template, "" + ) + self._use_pointer = bool(self._container_type) or bool( + self._container_no_template + ) # Check if this should use FixedVector instead of std::vector self._use_fixed_vector = get_field_opt(field, pb.fixed_vector, False) - # Check if this should be encoded as a bitmask - self._use_bitmask = get_field_opt(field, pb.enum_as_bitmask, False) # For repeated fields, we need to get the base type info # but we can't call create_field_type_info as it would cause recursion @@ -1436,15 +1440,18 @@ class RepeatedTypeInfo(TypeInfo): @property def cpp_type(self) -> str: - if self._use_bitmask: - # For bitmask fields, store as a single uint32_t - return "uint32_t" + if self._container_no_template: + # Non-template container: use type as-is without appending template parameters + return f"const {self._container_no_template}*" if self._use_pointer and self._container_type: # For pointer fields, use the specified container type - # If the container type already includes the element type (e.g., std::set) - # use it as-is, otherwise append the element type + # Two cases: + # 1. "std::set" - Full type with template params, use as-is + # 2. "std::set" - No <>, append the element type if "<" in self._container_type and ">" in self._container_type: + # Has template parameters specified, use as-is return f"const {self._container_type}*" + # No <> at all, append element type return f"const {self._container_type}<{self._ti.cpp_type}>*" if self._use_fixed_vector: return f"FixedVector<{self._ti.cpp_type}>" @@ -1471,11 +1478,6 @@ class RepeatedTypeInfo(TypeInfo): # Pointer fields don't support decoding if self._use_pointer: return None - if self._use_bitmask: - # Bitmask fields don't support decoding (only used for device->client messages) - raise RuntimeError( - f"enum_as_bitmask fields do not support decoding: {self.field_name}" - ) content = self._ti.decode_varint if content is None: return None @@ -1529,21 +1531,6 @@ class RepeatedTypeInfo(TypeInfo): @property def encode_content(self) -> str: - if self._use_bitmask: - # For bitmask fields, iterate through set bits and encode each enum value - # The bitmask is stored as uint32_t where bit N represents enum value N - # Note: We iterate through all 32 bits to support the full range of enum_as_bitmask - # (enums with up to 32 values). Specific uses may have fewer values, but the - # generated code is general-purpose. - assert isinstance(self._ti, EnumType), ( - "enum_as_bitmask only works with enum fields" - ) - o = "for (uint8_t bit = 0; bit < 32; bit++) {\n" - o += f" if (this->{self.field_name} & (1U << bit)) {{\n" - o += f" buffer.{self._ti.encode_func}({self.number}, bit, true);\n" - o += " }\n" - o += "}" - return o if self._use_pointer: # For pointer fields, just dereference (pointer should never be null in our use case) o = f"for (const auto &it : *this->{self.field_name}) {{\n" @@ -1563,13 +1550,6 @@ class RepeatedTypeInfo(TypeInfo): @property def dump_content(self) -> str: - if self._use_bitmask: - # For bitmask fields, dump the hex value of the bitmask - return ( - f"char buffer[64];\n" - f'snprintf(buffer, sizeof(buffer), " {self.field_name}: 0x%08" PRIX32 "\\n", this->{self.field_name});\n' - f"out.append(buffer);" - ) if self._use_pointer: # For pointer fields, dereference and use the existing helper return _generate_array_dump_content( @@ -1586,21 +1566,6 @@ class RepeatedTypeInfo(TypeInfo): # For repeated fields, we always need to pass force=True to the underlying type's calculation # This is because the encode method always sets force=true for repeated fields - if self._use_bitmask: - # For bitmask fields, iterate through set bits and calculate size - # Each set bit encodes one enum value (as varint) - # Note: We iterate through all 32 bits to support the full range of enum_as_bitmask - # (enums with up to 32 values). Specific uses may have fewer values, but the - # generated code is general-purpose. - o = f"if ({name} != 0) {{\n" - o += " for (uint8_t bit = 0; bit < 32; bit++) {\n" - o += f" if ({name} & (1U << bit)) {{\n" - o += f" {self._ti.get_size_calculation('bit', True)}\n" - o += " }\n" - o += " }\n" - o += "}" - return o - # Handle message types separately as they use a dedicated helper if isinstance(self._ti, MessageType): field_id_size = self._ti.calculate_field_id_size() From 3ef402ef6409c8abbee9ed2f60324e5d92ca9d90 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 12:38:02 -1000 Subject: [PATCH 02/18] cover --- tests/integration/test_light_calls.py | 29 +++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/integration/test_light_calls.py b/tests/integration/test_light_calls.py index af90ddbe86..152896ba88 100644 --- a/tests/integration/test_light_calls.py +++ b/tests/integration/test_light_calls.py @@ -8,6 +8,7 @@ import asyncio from typing import Any from aioesphomeapi import LightState +from aioesphomeapi.model import ColorMode import pytest from .types import APIClientConnectedFactory, RunCompiledFunction @@ -40,6 +41,34 @@ async def test_light_calls( rgbcw_light = next(light for light in lights if "RGBCW" in light.name) rgb_light = next(light for light in lights if "RGB Light" in light.name) + # Test color mode encoding: Verify supported_color_modes contains actual ColorMode enum values + # not bit positions. This is critical - the bug was encoding bit position 6 instead of + # ColorMode.RGB (value 35). + + # RGB light should support RGB mode (ColorMode.RGB = 35) + assert ColorMode.RGB in rgb_light.supported_color_modes, ( + f"RGB light missing RGB color mode. Got: {rgb_light.supported_color_modes}" + ) + # Verify it's the actual enum value, not a bit position + assert 35 in [mode.value for mode in rgb_light.supported_color_modes], ( + f"RGB light has wrong color mode values. Expected 35 (RGB), got: " + f"{[mode.value for mode in rgb_light.supported_color_modes]}" + ) + + # RGBCW light should support multiple modes including RGB_COLD_WARM_WHITE (value 51) + assert ColorMode.RGB_COLD_WARM_WHITE in rgbcw_light.supported_color_modes, ( + f"RGBCW light missing RGB_COLD_WARM_WHITE mode. Got: {rgbcw_light.supported_color_modes}" + ) + # Verify actual enum values + expected_rgbcw_modes = { + ColorMode.RGB_COLD_WARM_WHITE, # 51 + # May have other modes too + } + assert expected_rgbcw_modes.issubset(set(rgbcw_light.supported_color_modes)), ( + f"RGBCW light missing expected color modes. Got: " + f"{[f'{mode.name}={mode.value}' for mode in rgbcw_light.supported_color_modes]}" + ) + async def wait_for_state_change(key: int, timeout: float = 1.0) -> Any: """Wait for a state change for the given entity key.""" loop = asyncio.get_event_loop() From 89903929f3da55a46840ba82dcd9a3f623807a8f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:05:59 -1000 Subject: [PATCH 03/18] preen --- esphome/components/light/color_mode.h | 95 ++++++++++++++++++------- esphome/components/light/light_call.cpp | 63 +++++++--------- esphome/components/light/light_call.h | 4 +- 3 files changed, 96 insertions(+), 66 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 1241d59627..059996b740 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -104,6 +104,9 @@ constexpr ColorModeHelper operator|(ColorModeHelper lhs, ColorMode rhs) { return static_cast(static_cast(lhs) | static_cast(rhs)); } +// Type alias for raw color mode bitmask values +using color_mode_bitmask_t = uint16_t; + /// Bitmask for storing a set of ColorMode values efficiently. /// Replaces std::set to eliminate red-black tree overhead (~586 bytes). class ColorModeMask { @@ -143,7 +146,7 @@ class ColorModeMask { using pointer = const ColorMode *; using reference = ColorMode; - constexpr Iterator(uint16_t mask, int bit) : mask_(mask), bit_(bit) { advance_to_next_set_bit_(); } + 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_); } @@ -159,52 +162,92 @@ class ColorModeMask { private: constexpr void advance_to_next_set_bit_() { - while (bit_ < 16 && !(mask_ & (1 << bit_))) { + while (bit_ < MAX_BIT_INDEX && !(mask_ & (1 << bit_))) { ++bit_; } } - uint16_t mask_; + color_mode_bitmask_t mask_; int bit_; }; constexpr Iterator begin() const { return Iterator(mask_, 0); } - constexpr Iterator end() const { return Iterator(mask_, 16); } + constexpr Iterator end() const { return Iterator(mask_, MAX_BIT_INDEX); } /// Get the raw bitmask value for API encoding - constexpr uint16_t get_mask() const { return this->mask_; } + constexpr color_mode_bitmask_t get_mask() const { return this->mask_; } + + /// 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) { + // Find the position of the first set bit (least significant bit) + int bit = 0; + while (bit < MAX_BIT_INDEX && !(mask & (1 << bit))) { + ++bit; + } + return bit_to_mode(bit); + } + + /// 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; + } + + /// Build a bitmask of modes that match the given capability requirements + /// @param require_caps Capabilities that must be present in the mode + /// @param exclude_caps Capabilities that must not be present in the mode (for none case) + /// @return Raw bitmask value + static constexpr color_mode_bitmask_t build_mask_matching(uint8_t require_caps, uint8_t exclude_caps = 0) { + color_mode_bitmask_t mask = 0; + // Check each mode to see if it matches the requirements + // Skip UNKNOWN (bit 0), iterate through actual color modes (bits 1-9) + for (int bit = 1; bit < COLOR_MODE_COUNT; ++bit) { + ColorMode mode = bit_to_mode(bit); + uint8_t mode_val = static_cast(mode); + // Mode matches if it has all required caps and none of the excluded caps + if ((mode_val & require_caps) == require_caps && (exclude_caps == 0 || (mode_val & exclude_caps) == 0)) { + mask |= (1 << bit); + } + } + return mask; + } 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). - uint16_t mask_{0}; + color_mode_bitmask_t mask_{0}; + + // 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 /// Map ColorMode enum values to bit positions (0-9) static constexpr int mode_to_bit(ColorMode mode) { // Using switch instead of lookup table to avoid RAM usage on ESP8266 // The compiler optimizes this efficiently switch (mode) { - case ColorMode::UNKNOWN: + case ColorMode::UNKNOWN: // 0 return 0; - case ColorMode::ON_OFF: + case ColorMode::ON_OFF: // 1 return 1; - case ColorMode::BRIGHTNESS: + case ColorMode::BRIGHTNESS: // 3 return 2; - case ColorMode::WHITE: + case ColorMode::WHITE: // 7 return 3; - case ColorMode::COLOR_TEMPERATURE: + case ColorMode::COLOR_TEMPERATURE: // 11 return 4; - case ColorMode::COLD_WARM_WHITE: + case ColorMode::COLD_WARM_WHITE: // 19 return 5; - case ColorMode::RGB: + case ColorMode::RGB: // 35 return 6; - case ColorMode::RGB_WHITE: + case ColorMode::RGB_WHITE: // 39 return 7; - case ColorMode::RGB_COLOR_TEMPERATURE: + case ColorMode::RGB_COLOR_TEMPERATURE: // 47 return 8; - case ColorMode::RGB_COLD_WARM_WHITE: + case ColorMode::RGB_COLD_WARM_WHITE: // 51 return 9; default: return 0; @@ -215,25 +258,25 @@ class ColorModeMask { // Using switch instead of lookup table to avoid RAM usage on ESP8266 switch (bit) { case 0: - return ColorMode::UNKNOWN; + return ColorMode::UNKNOWN; // 0 case 1: - return ColorMode::ON_OFF; + return ColorMode::ON_OFF; // 1 case 2: - return ColorMode::BRIGHTNESS; + return ColorMode::BRIGHTNESS; // 3 case 3: - return ColorMode::WHITE; + return ColorMode::WHITE; // 7 case 4: - return ColorMode::COLOR_TEMPERATURE; + return ColorMode::COLOR_TEMPERATURE; // 11 case 5: - return ColorMode::COLD_WARM_WHITE; + return ColorMode::COLD_WARM_WHITE; // 19 case 6: - return ColorMode::RGB; + return ColorMode::RGB; // 35 case 7: - return ColorMode::RGB_WHITE; + return ColorMode::RGB_WHITE; // 39 case 8: - return ColorMode::RGB_COLOR_TEMPERATURE; + return ColorMode::RGB_COLOR_TEMPERATURE; // 47 case 9: - return ColorMode::RGB_COLD_WARM_WHITE; + return ColorMode::RGB_COLD_WARM_WHITE; // 51 default: return ColorMode::UNKNOWN; } diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 4e6251492d..fbc1b8f97d 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -425,20 +425,19 @@ ColorMode LightCall::compute_color_mode_() { // If no color mode is specified, we try to guess the color mode. This is needed for backward compatibility to // pre-colormode clients and automations, but also for the MQTT API, where HA doesn't let us know which color mode // was used for some reason. - ColorModeMask suitable_modes = this->get_suitable_color_modes_(); + // Compute intersection of suitable and supported modes using bitwise AND + color_mode_bitmask_t intersection = this->get_suitable_color_modes_mask_() & supported_modes.get_mask(); - // Don't change if the current mode is suitable. - if (suitable_modes.contains(current_mode)) { + // Don't change if the current mode is in the intersection (suitable AND supported) + if (ColorModeMask::mask_contains(intersection, current_mode)) { ESP_LOGI(TAG, "'%s': color mode not specified; retaining %s", this->parent_->get_name().c_str(), LOG_STR_ARG(color_mode_to_human(current_mode))); return current_mode; } // Use the preferred suitable mode. - for (auto mode : suitable_modes) { - if (!supported_modes.contains(mode)) - continue; - + if (intersection != 0) { + ColorMode mode = ColorModeMask::first_mode_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; @@ -451,7 +450,7 @@ ColorMode LightCall::compute_color_mode_() { LOG_STR_ARG(color_mode_to_human(color_mode))); return color_mode; } -ColorModeMask LightCall::get_suitable_color_modes_() { +color_mode_bitmask_t LightCall::get_suitable_color_modes_mask_() { bool has_white = this->has_white() && this->white_ > 0.0f; bool has_ct = this->has_color_temperature(); bool has_cwww = @@ -459,39 +458,27 @@ ColorModeMask LightCall::get_suitable_color_modes_() { bool has_rgb = (this->has_color_brightness() && this->color_brightness_ > 0.0f) || (this->has_red() || this->has_green() || this->has_blue()); -// Build key from flags: [rgb][cwww][ct][white] -#define KEY(white, ct, cwww, rgb) ((white) << 0 | (ct) << 1 | (cwww) << 2 | (rgb) << 3) + // Build required capabilities mask + uint8_t require_caps = static_cast(ColorCapability::ON_OFF | ColorCapability::BRIGHTNESS); + if (has_rgb) + require_caps |= static_cast(ColorCapability::RGB); + if (has_white) + require_caps |= static_cast(ColorCapability::WHITE); + if (has_ct) + require_caps |= static_cast(ColorCapability::COLOR_TEMPERATURE); + if (has_cwww) + require_caps |= static_cast(ColorCapability::COLD_WARM_WHITE); - uint8_t key = KEY(has_white, has_ct, has_cwww, has_rgb); - - switch (key) { - case KEY(true, false, false, false): // white only - return {ColorMode::WHITE, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE, - ColorMode::RGB_COLD_WARM_WHITE}; - case KEY(false, true, false, false): // ct only - return {ColorMode::COLOR_TEMPERATURE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE, - ColorMode::RGB_COLD_WARM_WHITE}; - case KEY(true, true, false, false): // white + ct - return {ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; - case KEY(false, false, true, false): // cwww only - return {ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLD_WARM_WHITE}; - case KEY(false, false, false, false): // none - return {ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE, ColorMode::RGB, - ColorMode::WHITE, ColorMode::COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE}; - case KEY(true, false, false, true): // rgb + white - return {ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; - case KEY(false, true, false, true): // rgb + ct - case KEY(true, true, false, true): // rgb + white + ct - return {ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; - case KEY(false, false, true, true): // rgb + cwww - return {ColorMode::RGB_COLD_WARM_WHITE}; - case KEY(false, false, false, true): // rgb only - return {ColorMode::RGB, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; - default: - return {}; // conflicting flags + // If no specific color parameters set, exclude modes with color capabilities + uint8_t exclude_caps = 0; + if (!has_rgb && !has_white && !has_ct && !has_cwww) { + // For "none" case, we want all modes but don't exclude anything + // Just require ON_OFF + BRIGHTNESS which all modes have + return ColorModeMask::build_mask_matching( + static_cast(ColorCapability::ON_OFF | ColorCapability::BRIGHTNESS), exclude_caps); } -#undef KEY + return ColorModeMask::build_mask_matching(require_caps, exclude_caps); } LightCall &LightCall::set_effect(const std::string &effect) { diff --git a/esphome/components/light/light_call.h b/esphome/components/light/light_call.h index e87ccd3efd..6931b58b9d 100644 --- a/esphome/components/light/light_call.h +++ b/esphome/components/light/light_call.h @@ -185,8 +185,8 @@ class LightCall { //// Compute the color mode that should be used for this call. ColorMode compute_color_mode_(); - /// Get potential color modes for this light call. - ColorModeMask get_suitable_color_modes_(); + /// Get potential color modes bitmask for this light call. + color_mode_bitmask_t get_suitable_color_modes_mask_(); /// Some color modes also can be set using non-native parameters, transform those calls. void transform_parameters_(); From 89c719d71d6e0a488dc05a889a162d8c51f45531 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:10:41 -1000 Subject: [PATCH 04/18] preen --- esphome/components/light/color_mode.h | 13 +++++++++++++ esphome/components/light/light_traits.h | 6 +----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 059996b740..c542984d1b 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -194,6 +194,19 @@ class ColorModeMask { 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 { + uint8_t cap_mask = static_cast(capability); + // Check each set bit to see if any mode has this capability + for (int bit = 1; bit < COLOR_MODE_COUNT; ++bit) { + if ((this->mask_ & (1 << bit)) && (static_cast(bit_to_mode(bit)) & cap_mask)) { + return true; + } + } + return false; + } + /// Build a bitmask of modes that match the given capability requirements /// @param require_caps Capabilities that must be present in the mode /// @param exclude_caps Capabilities that must not be present in the mode (for none case) diff --git a/esphome/components/light/light_traits.h b/esphome/components/light/light_traits.h index 0db028598c..c83d8ad2a9 100644 --- a/esphome/components/light/light_traits.h +++ b/esphome/components/light/light_traits.h @@ -28,11 +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 { - for (auto mode : this->supported_color_modes_) { - if (mode & color_capability) - return true; - } - return false; + return this->supported_color_modes_.has_capability(color_capability); } ESPDEPRECATED("get_supports_brightness() is deprecated, use color modes instead.", "v1.21") From acef2085d9fb1224df316271727ceaa1414bf332 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 18 Oct 2025 23:11:36 +0000 Subject: [PATCH 05/18] Bump aioesphomeapi from 42.1.0 to 42.2.0 (#11352) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 2f25905d94..ec7794c75a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ platformio==6.1.18 # When updating platformio, also update /docker/Dockerfile esptool==5.1.0 click==8.1.7 esphome-dashboard==20251013.0 -aioesphomeapi==42.1.0 +aioesphomeapi==42.2.0 zeroconf==0.148.0 puremagic==1.30 ruamel.yaml==0.18.15 # dashboard_import From ec8d8538f64e3b705012ccf70a8fc4ce708147d9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:12:48 -1000 Subject: [PATCH 06/18] preen --- esphome/components/light/color_mode.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index c542984d1b..85e7a18406 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -198,9 +198,9 @@ class ColorModeMask { /// Used for checking if a light supports a capability (e.g., BRIGHTNESS, RGB) bool has_capability(ColorCapability capability) const { uint8_t cap_mask = static_cast(capability); - // Check each set bit to see if any mode has this capability - for (int bit = 1; bit < COLOR_MODE_COUNT; ++bit) { - if ((this->mask_ & (1 << bit)) && (static_cast(bit_to_mode(bit)) & cap_mask)) { + // Iterate through each mode and check if it has the capability + for (auto mode : *this) { + if (static_cast(mode) & cap_mask) { return true; } } From 80fd51e198a17c74c0b53af5566c7f79a8e8e0be Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:14:05 -1000 Subject: [PATCH 07/18] 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 85e7a18406..4fc65ac5f0 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -219,7 +219,7 @@ class ColorModeMask { ColorMode mode = bit_to_mode(bit); uint8_t mode_val = static_cast(mode); // Mode matches if it has all required caps and none of the excluded caps - if ((mode_val & require_caps) == require_caps && (exclude_caps == 0 || (mode_val & exclude_caps) == 0)) { + if ((mode_val & require_caps) == require_caps && (mode_val & exclude_caps) == 0) { mask |= (1 << bit); } } From cc6b798f2ba69bd01c9591dd3f7d6a044449be94 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:15:47 -1000 Subject: [PATCH 08/18] overkill --- esphome/components/light/color_mode.h | 7 +++---- esphome/components/light/light_call.cpp | 11 +---------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 4fc65ac5f0..a91df200c9 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -209,17 +209,16 @@ class ColorModeMask { /// Build a bitmask of modes that match the given capability requirements /// @param require_caps Capabilities that must be present in the mode - /// @param exclude_caps Capabilities that must not be present in the mode (for none case) /// @return Raw bitmask value - static constexpr color_mode_bitmask_t build_mask_matching(uint8_t require_caps, uint8_t exclude_caps = 0) { + static constexpr color_mode_bitmask_t build_mask_matching(uint8_t require_caps) { color_mode_bitmask_t mask = 0; // Check each mode to see if it matches the requirements // Skip UNKNOWN (bit 0), iterate through actual color modes (bits 1-9) for (int bit = 1; bit < COLOR_MODE_COUNT; ++bit) { ColorMode mode = bit_to_mode(bit); uint8_t mode_val = static_cast(mode); - // Mode matches if it has all required caps and none of the excluded caps - if ((mode_val & require_caps) == require_caps && (mode_val & exclude_caps) == 0) { + // Mode matches if it has all required caps + if ((mode_val & require_caps) == require_caps) { mask |= (1 << bit); } } diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index fbc1b8f97d..036de98639 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -469,16 +469,7 @@ color_mode_bitmask_t LightCall::get_suitable_color_modes_mask_() { if (has_cwww) require_caps |= static_cast(ColorCapability::COLD_WARM_WHITE); - // If no specific color parameters set, exclude modes with color capabilities - uint8_t exclude_caps = 0; - if (!has_rgb && !has_white && !has_ct && !has_cwww) { - // For "none" case, we want all modes but don't exclude anything - // Just require ON_OFF + BRIGHTNESS which all modes have - return ColorModeMask::build_mask_matching( - static_cast(ColorCapability::ON_OFF | ColorCapability::BRIGHTNESS), exclude_caps); - } - - return ColorModeMask::build_mask_matching(require_caps, exclude_caps); + return ColorModeMask::build_mask_matching(require_caps); } LightCall &LightCall::set_effect(const std::string &effect) { From 44d3f355a5b4e72b8115dc8df3abbb981b9b0047 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:16:52 -1000 Subject: [PATCH 09/18] overkill --- esphome/components/light/color_mode.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index a91df200c9..a154397aea 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -215,8 +215,7 @@ class ColorModeMask { // Check each mode to see if it matches the requirements // Skip UNKNOWN (bit 0), iterate through actual color modes (bits 1-9) for (int bit = 1; bit < COLOR_MODE_COUNT; ++bit) { - ColorMode mode = bit_to_mode(bit); - uint8_t mode_val = static_cast(mode); + uint8_t mode_val = static_cast(bit_to_mode(bit)); // Mode matches if it has all required caps if ((mode_val & require_caps) == require_caps) { mask |= (1 << bit); From 1c8b60891c9a3b9b6c8b4e9c011ef43a2c8c5751 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:32:48 -1000 Subject: [PATCH 10/18] simplify --- esphome/components/light/color_mode.h | 168 +++++++++++++++----------- 1 file changed, 99 insertions(+), 69 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index a154397aea..c2b1a860ec 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -107,6 +107,97 @@ constexpr ColorModeHelper operator|(ColorModeHelper lhs, ColorMode rhs) { // Type alias for raw color mode bitmask values 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 + +/// Map ColorMode enum values to bit positions (0-9) +static constexpr int mode_to_bit(ColorMode mode) { + // Using switch instead of lookup table to avoid RAM usage on ESP8266 + // The compiler optimizes this efficiently + switch (mode) { + case ColorMode::UNKNOWN: // 0 + return 0; + case ColorMode::ON_OFF: // 1 + return 1; + case ColorMode::BRIGHTNESS: // 3 + return 2; + case ColorMode::WHITE: // 7 + return 3; + case ColorMode::COLOR_TEMPERATURE: // 11 + return 4; + case ColorMode::COLD_WARM_WHITE: // 19 + return 5; + case ColorMode::RGB: // 35 + return 6; + case ColorMode::RGB_WHITE: // 39 + return 7; + case ColorMode::RGB_COLOR_TEMPERATURE: // 47 + return 8; + case ColorMode::RGB_COLD_WARM_WHITE: // 51 + return 9; + default: + return 0; + } +} + +static constexpr ColorMode bit_to_mode(int bit) { + // Using switch instead of lookup table to avoid RAM usage on ESP8266 + switch (bit) { + case 0: + return ColorMode::UNKNOWN; // 0 + case 1: + return ColorMode::ON_OFF; // 1 + case 2: + return ColorMode::BRIGHTNESS; // 3 + case 3: + return ColorMode::WHITE; // 7 + case 4: + return ColorMode::COLOR_TEMPERATURE; // 11 + case 5: + return ColorMode::COLD_WARM_WHITE; // 19 + case 6: + return ColorMode::RGB; // 35 + case 7: + return ColorMode::RGB_WHITE; // 39 + case 8: + return ColorMode::RGB_COLOR_TEMPERATURE; // 47 + case 9: + return ColorMode::RGB_COLD_WARM_WHITE; // 51 + default: + return ColorMode::UNKNOWN; + } +} + +/// 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; + 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)); + if ((mode_val & cap_bit) != 0) { + mask |= (1 << bit); + } + } + 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[] = { + compute_capability_bitmask(ColorCapability::ON_OFF), // 1 << 0 + compute_capability_bitmask(ColorCapability::BRIGHTNESS), // 1 << 1 + compute_capability_bitmask(ColorCapability::WHITE), // 1 << 2 + compute_capability_bitmask(ColorCapability::COLOR_TEMPERATURE), // 1 << 3 + compute_capability_bitmask(ColorCapability::COLD_WARM_WHITE), // 1 << 4 + 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 { @@ -197,14 +288,15 @@ class ColorModeMask { /// 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 { - uint8_t cap_mask = static_cast(capability); - // Iterate through each mode and check if it has the capability - for (auto mode : *this) { - if (static_cast(mode) & cap_mask) { - return true; - } + // Convert capability bit to array index (log2 of the bit value) + uint8_t cap_bit = static_cast(capability); + // Count trailing zeros to get the bit position (0-5) + int index = 0; + while (index < COLOR_CAPABILITY_COUNT && !(cap_bit & (1 << index))) { + ++index; } - return false; + // Look up the pre-computed bitmask and check if any of our set bits match + return (index < COLOR_CAPABILITY_COUNT) && ((this->mask_ & CAPABILITY_BITMASKS[index]) != 0); } /// Build a bitmask of modes that match the given capability requirements @@ -230,68 +322,6 @@ class ColorModeMask { // 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}; - - // 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 - - /// Map ColorMode enum values to bit positions (0-9) - static constexpr int mode_to_bit(ColorMode mode) { - // Using switch instead of lookup table to avoid RAM usage on ESP8266 - // The compiler optimizes this efficiently - switch (mode) { - case ColorMode::UNKNOWN: // 0 - return 0; - case ColorMode::ON_OFF: // 1 - return 1; - case ColorMode::BRIGHTNESS: // 3 - return 2; - case ColorMode::WHITE: // 7 - return 3; - case ColorMode::COLOR_TEMPERATURE: // 11 - return 4; - case ColorMode::COLD_WARM_WHITE: // 19 - return 5; - case ColorMode::RGB: // 35 - return 6; - case ColorMode::RGB_WHITE: // 39 - return 7; - case ColorMode::RGB_COLOR_TEMPERATURE: // 47 - return 8; - case ColorMode::RGB_COLD_WARM_WHITE: // 51 - return 9; - default: - return 0; - } - } - - static constexpr ColorMode bit_to_mode(int bit) { - // Using switch instead of lookup table to avoid RAM usage on ESP8266 - switch (bit) { - case 0: - return ColorMode::UNKNOWN; // 0 - case 1: - return ColorMode::ON_OFF; // 1 - case 2: - return ColorMode::BRIGHTNESS; // 3 - case 3: - return ColorMode::WHITE; // 7 - case 4: - return ColorMode::COLOR_TEMPERATURE; // 11 - case 5: - return ColorMode::COLD_WARM_WHITE; // 19 - case 6: - return ColorMode::RGB; // 35 - case 7: - return ColorMode::RGB_WHITE; // 39 - case 8: - return ColorMode::RGB_COLOR_TEMPERATURE; // 47 - case 9: - return ColorMode::RGB_COLD_WARM_WHITE; // 51 - default: - return ColorMode::UNKNOWN; - } - } }; } // namespace light From 8545b5231bf6e6b7927ba219bd0b09810fc5cced Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:38:45 -1000 Subject: [PATCH 11/18] preen --- esphome/components/light/color_mode.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index c2b1a860ec..7c7239a5af 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -288,15 +288,16 @@ class ColorModeMask { /// 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 { - // Convert capability bit to array index (log2 of the bit value) - uint8_t cap_bit = static_cast(capability); - // Count trailing zeros to get the bit position (0-5) + // 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); int index = 0; - while (index < COLOR_CAPABILITY_COUNT && !(cap_bit & (1 << index))) { + while (cap_val > 1) { + cap_val >>= 1; ++index; } - // Look up the pre-computed bitmask and check if any of our set bits match - return (index < COLOR_CAPABILITY_COUNT) && ((this->mask_ & CAPABILITY_BITMASKS[index]) != 0); + return (this->mask_ & CAPABILITY_BITMASKS[index]) != 0; } /// Build a bitmask of modes that match the given capability requirements From a249c9c28290b54e0f143711c97b09ba4f5cdade Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:46:49 -1000 Subject: [PATCH 12/18] preen --- esphome/components/light/color_mode.h | 24 +++------- esphome/components/light/light_call.cpp | 58 ++++++++++++++++++++----- 2 files changed, 54 insertions(+), 28 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 7c7239a5af..97e61e2a1c 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -213,6 +213,13 @@ class ColorModeMask { 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 { @@ -300,23 +307,6 @@ class ColorModeMask { return (this->mask_ & CAPABILITY_BITMASKS[index]) != 0; } - /// Build a bitmask of modes that match the given capability requirements - /// @param require_caps Capabilities that must be present in the mode - /// @return Raw bitmask value - static constexpr color_mode_bitmask_t build_mask_matching(uint8_t require_caps) { - color_mode_bitmask_t mask = 0; - // Check each mode to see if it matches the requirements - // Skip UNKNOWN (bit 0), iterate through actual color modes (bits 1-9) - for (int bit = 1; bit < COLOR_MODE_COUNT; ++bit) { - uint8_t mode_val = static_cast(bit_to_mode(bit)); - // Mode matches if it has all required caps - if ((mode_val & require_caps) == require_caps) { - mask |= (1 << bit); - } - } - return mask; - } - 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. diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 036de98639..f209f26005 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -458,18 +458,54 @@ color_mode_bitmask_t LightCall::get_suitable_color_modes_mask_() { bool has_rgb = (this->has_color_brightness() && this->color_brightness_ > 0.0f) || (this->has_red() || this->has_green() || this->has_blue()); - // Build required capabilities mask - uint8_t require_caps = static_cast(ColorCapability::ON_OFF | ColorCapability::BRIGHTNESS); - if (has_rgb) - require_caps |= static_cast(ColorCapability::RGB); - if (has_white) - require_caps |= static_cast(ColorCapability::WHITE); - if (has_ct) - require_caps |= static_cast(ColorCapability::COLOR_TEMPERATURE); - if (has_cwww) - require_caps |= static_cast(ColorCapability::COLD_WARM_WHITE); + // Build key from flags: [rgb][cwww][ct][white] +#define KEY(white, ct, cwww, rgb) ((white) << 0 | (ct) << 1 | (cwww) << 2 | (rgb) << 3) - return ColorModeMask::build_mask_matching(require_caps); + uint8_t key = KEY(has_white, has_ct, has_cwww, has_rgb); + + // Build bitmask from suitable ColorModes + ColorModeMask suitable; + + switch (key) { + case KEY(true, false, false, false): // white only + suitable.add({ColorMode::WHITE, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, + ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLD_WARM_WHITE}); + break; + case KEY(false, true, false, false): // ct only + suitable.add({ColorMode::COLOR_TEMPERATURE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE, + ColorMode::RGB_COLD_WARM_WHITE}); + break; + case KEY(true, true, false, false): // white + ct + suitable.add({ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}); + break; + case KEY(false, false, true, false): // cwww only + suitable.add({ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLD_WARM_WHITE}); + break; + case KEY(false, false, false, false): // none + suitable.add({ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE, + ColorMode::RGB, ColorMode::WHITE, ColorMode::COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE}); + break; + case KEY(true, false, false, true): // rgb + white + suitable.add({ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}); + break; + case KEY(false, true, false, true): // rgb + ct + case KEY(true, true, false, true): // rgb + white + ct + suitable.add({ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}); + break; + case KEY(false, false, true, true): // rgb + cwww + suitable.add(ColorMode::RGB_COLD_WARM_WHITE); + break; + case KEY(false, false, false, true): // rgb only + suitable.add( + {ColorMode::RGB, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}); + break; + default: + break; // conflicting flags - return empty mask + } + +#undef KEY + + return suitable.get_mask(); } LightCall &LightCall::set_effect(const std::string &effect) { From 2cdfd04204403151f74f47d56ff585e2401bbe05 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:53:05 -1000 Subject: [PATCH 13/18] dry --- esphome/components/light/color_mode.h | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 97e61e2a1c..70d940ec54 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -259,11 +259,7 @@ class ColorModeMask { constexpr bool operator!=(const Iterator &other) const { return !(*this == other); } private: - constexpr void advance_to_next_set_bit_() { - while (bit_ < MAX_BIT_INDEX && !(mask_ & (1 << bit_))) { - ++bit_; - } - } + constexpr void advance_to_next_set_bit_() { bit_ = ColorModeMask::find_next_set_bit(mask_, bit_); } color_mode_bitmask_t mask_; int bit_; @@ -275,15 +271,20 @@ class ColorModeMask { /// Get the raw bitmask value for API encoding constexpr color_mode_bitmask_t get_mask() const { return this->mask_; } - /// 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) { - // Find the position of the first set bit (least significant bit) - int bit = 0; + /// 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_to_mode(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 From f2d01ecd6c7e47896d09dee4534dd0d112e6165c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 13:58:52 -1000 Subject: [PATCH 14/18] dry --- esphome/components/light/color_mode.h | 76 +++++++++------------------ 1 file changed, 26 insertions(+), 50 deletions(-) diff --git a/esphome/components/light/color_mode.h b/esphome/components/light/color_mode.h index 70d940ec54..1583bde4d3 100644 --- a/esphome/components/light/color_mode.h +++ b/esphome/components/light/color_mode.h @@ -111,62 +111,38 @@ using color_mode_bitmask_t = uint16_t; 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 +// 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] = { + 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 +}; + /// Map ColorMode enum values to bit positions (0-9) +/// Bit positions follow the enum declaration order static constexpr int mode_to_bit(ColorMode mode) { - // Using switch instead of lookup table to avoid RAM usage on ESP8266 - // The compiler optimizes this efficiently - switch (mode) { - case ColorMode::UNKNOWN: // 0 - return 0; - case ColorMode::ON_OFF: // 1 - return 1; - case ColorMode::BRIGHTNESS: // 3 - return 2; - case ColorMode::WHITE: // 7 - return 3; - case ColorMode::COLOR_TEMPERATURE: // 11 - return 4; - case ColorMode::COLD_WARM_WHITE: // 19 - return 5; - case ColorMode::RGB: // 35 - return 6; - case ColorMode::RGB_WHITE: // 39 - return 7; - case ColorMode::RGB_COLOR_TEMPERATURE: // 47 - return 8; - case ColorMode::RGB_COLD_WARM_WHITE: // 51 - return 9; - default: - return 0; + // 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; } +/// Map bit positions (0-9) to ColorMode enum values +/// Bit positions follow the enum declaration order static constexpr ColorMode bit_to_mode(int bit) { - // Using switch instead of lookup table to avoid RAM usage on ESP8266 - switch (bit) { - case 0: - return ColorMode::UNKNOWN; // 0 - case 1: - return ColorMode::ON_OFF; // 1 - case 2: - return ColorMode::BRIGHTNESS; // 3 - case 3: - return ColorMode::WHITE; // 7 - case 4: - return ColorMode::COLOR_TEMPERATURE; // 11 - case 5: - return ColorMode::COLD_WARM_WHITE; // 19 - case 6: - return ColorMode::RGB; // 35 - case 7: - return ColorMode::RGB_WHITE; // 39 - case 8: - return ColorMode::RGB_COLOR_TEMPERATURE; // 47 - case 9: - return ColorMode::RGB_COLD_WARM_WHITE; // 51 - default: - return ColorMode::UNKNOWN; - } + // Direct lookup in COLOR_MODES array + return (bit >= 0 && bit < COLOR_MODE_COUNT) ? COLOR_MODES[bit] : ColorMode::UNKNOWN; } /// Helper to compute capability bitmask at compile time From 764428870d7a2520adcc2e93f419c99af01a4a3e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 14:11:23 -1000 Subject: [PATCH 15/18] reduce diff --- esphome/components/light/light_call.cpp | 46 +++++++++---------------- esphome/components/light/light_call.h | 2 +- 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index f209f26005..5ca2f24d0e 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -426,7 +426,8 @@ ColorMode LightCall::compute_color_mode_() { // pre-colormode clients and automations, but also for the MQTT API, where HA doesn't let us know which color mode // was used for some reason. // Compute intersection of suitable and supported modes using bitwise AND - color_mode_bitmask_t intersection = this->get_suitable_color_modes_mask_() & supported_modes.get_mask(); + ColorModeMask suitable = this->get_suitable_color_modes_(); + color_mode_bitmask_t intersection = suitable.get_mask() & supported_modes.get_mask(); // Don't change if the current mode is in the intersection (suitable AND supported) if (ColorModeMask::mask_contains(intersection, current_mode)) { @@ -450,7 +451,7 @@ ColorMode LightCall::compute_color_mode_() { LOG_STR_ARG(color_mode_to_human(color_mode))); return color_mode; } -color_mode_bitmask_t LightCall::get_suitable_color_modes_mask_() { +ColorModeMask LightCall::get_suitable_color_modes_() { bool has_white = this->has_white() && this->white_ > 0.0f; bool has_ct = this->has_color_temperature(); bool has_cwww = @@ -463,49 +464,34 @@ color_mode_bitmask_t LightCall::get_suitable_color_modes_mask_() { uint8_t key = KEY(has_white, has_ct, has_cwww, has_rgb); - // Build bitmask from suitable ColorModes - ColorModeMask suitable; - switch (key) { case KEY(true, false, false, false): // white only - suitable.add({ColorMode::WHITE, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, - ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLD_WARM_WHITE}); - break; + return {ColorMode::WHITE, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE, + ColorMode::RGB_COLD_WARM_WHITE}; case KEY(false, true, false, false): // ct only - suitable.add({ColorMode::COLOR_TEMPERATURE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE, - ColorMode::RGB_COLD_WARM_WHITE}); - break; + return {ColorMode::COLOR_TEMPERATURE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE, + ColorMode::RGB_COLD_WARM_WHITE}; case KEY(true, true, false, false): // white + ct - suitable.add({ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}); - break; + return {ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; case KEY(false, false, true, false): // cwww only - suitable.add({ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLD_WARM_WHITE}); - break; + return {ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLD_WARM_WHITE}; case KEY(false, false, false, false): // none - suitable.add({ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE, - ColorMode::RGB, ColorMode::WHITE, ColorMode::COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE}); - break; + return {ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE, ColorMode::RGB, + ColorMode::WHITE, ColorMode::COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE}; case KEY(true, false, false, true): // rgb + white - suitable.add({ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}); - break; + return {ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; case KEY(false, true, false, true): // rgb + ct case KEY(true, true, false, true): // rgb + white + ct - suitable.add({ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}); - break; + return {ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; case KEY(false, false, true, true): // rgb + cwww - suitable.add(ColorMode::RGB_COLD_WARM_WHITE); - break; + return {ColorMode::RGB_COLD_WARM_WHITE}; case KEY(false, false, false, true): // rgb only - suitable.add( - {ColorMode::RGB, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}); - break; + return {ColorMode::RGB, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; default: - break; // conflicting flags - return empty mask + return {}; // conflicting flags } #undef KEY - - return suitable.get_mask(); } LightCall &LightCall::set_effect(const std::string &effect) { diff --git a/esphome/components/light/light_call.h b/esphome/components/light/light_call.h index 6931b58b9d..f34feadefe 100644 --- a/esphome/components/light/light_call.h +++ b/esphome/components/light/light_call.h @@ -186,7 +186,7 @@ class LightCall { //// Compute the color mode that should be used for this call. ColorMode compute_color_mode_(); /// Get potential color modes bitmask for this light call. - color_mode_bitmask_t get_suitable_color_modes_mask_(); + ColorModeMask get_suitable_color_modes_(); /// Some color modes also can be set using non-native parameters, transform those calls. void transform_parameters_(); From 32eb43fd02ae743fda3e2b5cc251cba663010d1b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 14:14:48 -1000 Subject: [PATCH 16/18] preen --- esphome/components/light/light_call.cpp | 39 +++++++++++++++---------- esphome/components/light/light_call.h | 2 +- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 5ca2f24d0e..89910d851b 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -426,8 +426,7 @@ ColorMode LightCall::compute_color_mode_() { // pre-colormode clients and automations, but also for the MQTT API, where HA doesn't let us know which color mode // was used for some reason. // Compute intersection of suitable and supported modes using bitwise AND - ColorModeMask suitable = this->get_suitable_color_modes_(); - color_mode_bitmask_t intersection = suitable.get_mask() & supported_modes.get_mask(); + color_mode_bitmask_t intersection = this->get_suitable_color_modes_() & supported_modes.get_mask(); // Don't change if the current mode is in the intersection (suitable AND supported) if (ColorModeMask::mask_contains(intersection, current_mode)) { @@ -451,7 +450,7 @@ ColorMode LightCall::compute_color_mode_() { LOG_STR_ARG(color_mode_to_human(color_mode))); return color_mode; } -ColorModeMask LightCall::get_suitable_color_modes_() { +color_mode_bitmask_t LightCall::get_suitable_color_modes_() { bool has_white = this->has_white() && this->white_ > 0.0f; bool has_ct = this->has_color_temperature(); bool has_cwww = @@ -466,29 +465,37 @@ ColorModeMask LightCall::get_suitable_color_modes_() { switch (key) { case KEY(true, false, false, false): // white only - return {ColorMode::WHITE, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE, - ColorMode::RGB_COLD_WARM_WHITE}; + return ColorModeMask({ColorMode::WHITE, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, + ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLD_WARM_WHITE}) + .get_mask(); case KEY(false, true, false, false): // ct only - return {ColorMode::COLOR_TEMPERATURE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE, - ColorMode::RGB_COLD_WARM_WHITE}; + return ColorModeMask({ColorMode::COLOR_TEMPERATURE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE, + ColorMode::RGB_COLD_WARM_WHITE}) + .get_mask(); case KEY(true, true, false, false): // white + ct - return {ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; + return ColorModeMask( + {ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}) + .get_mask(); case KEY(false, false, true, false): // cwww only - return {ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLD_WARM_WHITE}; + return ColorModeMask({ColorMode::COLD_WARM_WHITE, ColorMode::RGB_COLD_WARM_WHITE}).get_mask(); case KEY(false, false, false, false): // none - return {ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE, ColorMode::RGB, - ColorMode::WHITE, ColorMode::COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE}; + return ColorModeMask({ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE, + ColorMode::RGB, ColorMode::WHITE, ColorMode::COLOR_TEMPERATURE, ColorMode::COLD_WARM_WHITE}) + .get_mask(); case KEY(true, false, false, true): // rgb + white - return {ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; + return ColorModeMask({ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}) + .get_mask(); case KEY(false, true, false, true): // rgb + ct case KEY(true, true, false, true): // rgb + white + ct - return {ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; + return ColorModeMask({ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}).get_mask(); case KEY(false, false, true, true): // rgb + cwww - return {ColorMode::RGB_COLD_WARM_WHITE}; + return ColorModeMask({ColorMode::RGB_COLD_WARM_WHITE}).get_mask(); case KEY(false, false, false, true): // rgb only - return {ColorMode::RGB, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, ColorMode::RGB_COLD_WARM_WHITE}; + return ColorModeMask({ColorMode::RGB, ColorMode::RGB_WHITE, ColorMode::RGB_COLOR_TEMPERATURE, + ColorMode::RGB_COLD_WARM_WHITE}) + .get_mask(); default: - return {}; // conflicting flags + return 0; // conflicting flags } #undef KEY diff --git a/esphome/components/light/light_call.h b/esphome/components/light/light_call.h index f34feadefe..e25b26731f 100644 --- a/esphome/components/light/light_call.h +++ b/esphome/components/light/light_call.h @@ -186,7 +186,7 @@ class LightCall { //// Compute the color mode that should be used for this call. ColorMode compute_color_mode_(); /// Get potential color modes bitmask for this light call. - ColorModeMask get_suitable_color_modes_(); + color_mode_bitmask_t get_suitable_color_modes_(); /// Some color modes also can be set using non-native parameters, transform those calls. void transform_parameters_(); From 1381db37adc2b6ed5c984b64d51a0e21b1e091ce Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 14:18:17 -1000 Subject: [PATCH 17/18] preen --- esphome/components/light/light_call.cpp | 4 ++-- esphome/components/light/light_call.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 89910d851b..af193e1f11 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -426,7 +426,7 @@ ColorMode LightCall::compute_color_mode_() { // pre-colormode clients and automations, but also for the MQTT API, where HA doesn't let us know which color mode // was used for some reason. // Compute intersection of suitable and supported modes using bitwise AND - color_mode_bitmask_t intersection = this->get_suitable_color_modes_() & supported_modes.get_mask(); + color_mode_bitmask_t intersection = this->get_suitable_color_modes_mask_() & supported_modes.get_mask(); // Don't change if the current mode is in the intersection (suitable AND supported) if (ColorModeMask::mask_contains(intersection, current_mode)) { @@ -450,7 +450,7 @@ ColorMode LightCall::compute_color_mode_() { LOG_STR_ARG(color_mode_to_human(color_mode))); return color_mode; } -color_mode_bitmask_t LightCall::get_suitable_color_modes_() { +color_mode_bitmask_t LightCall::get_suitable_color_modes_mask_() { bool has_white = this->has_white() && this->white_ > 0.0f; bool has_ct = this->has_color_temperature(); bool has_cwww = diff --git a/esphome/components/light/light_call.h b/esphome/components/light/light_call.h index e25b26731f..6931b58b9d 100644 --- a/esphome/components/light/light_call.h +++ b/esphome/components/light/light_call.h @@ -186,7 +186,7 @@ class LightCall { //// Compute the color mode that should be used for this call. ColorMode compute_color_mode_(); /// Get potential color modes bitmask for this light call. - color_mode_bitmask_t get_suitable_color_modes_(); + color_mode_bitmask_t get_suitable_color_modes_mask_(); /// Some color modes also can be set using non-native parameters, transform those calls. void transform_parameters_(); From 437dd503ca0f4bd346407a9c6995b20e46cfd2ae Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 14:21:52 -1000 Subject: [PATCH 18/18] more cover --- tests/integration/test_light_calls.py | 57 ++++++++++++++++----------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/tests/integration/test_light_calls.py b/tests/integration/test_light_calls.py index 152896ba88..0eaf5af91b 100644 --- a/tests/integration/test_light_calls.py +++ b/tests/integration/test_light_calls.py @@ -36,37 +36,50 @@ async def test_light_calls( # Get the light entities entities = await client.list_entities_services() lights = [e for e in entities[0] if e.object_id.startswith("test_")] - assert len(lights) >= 2 # Should have RGBCW and RGB lights + assert len(lights) >= 3 # Should have RGBCW, RGB, and Binary lights rgbcw_light = next(light for light in lights if "RGBCW" in light.name) rgb_light = next(light for light in lights if "RGB Light" in light.name) + binary_light = next(light for light in lights if "Binary" in light.name) # Test color mode encoding: Verify supported_color_modes contains actual ColorMode enum values - # not bit positions. This is critical - the bug was encoding bit position 6 instead of - # ColorMode.RGB (value 35). + # not bit positions. This is critical - the iterator must convert bit positions to actual + # ColorMode enum values for API encoding. - # RGB light should support RGB mode (ColorMode.RGB = 35) - assert ColorMode.RGB in rgb_light.supported_color_modes, ( - f"RGB light missing RGB color mode. Got: {rgb_light.supported_color_modes}" - ) - # Verify it's the actual enum value, not a bit position - assert 35 in [mode.value for mode in rgb_light.supported_color_modes], ( - f"RGB light has wrong color mode values. Expected 35 (RGB), got: " - f"{[mode.value for mode in rgb_light.supported_color_modes]}" - ) - - # RGBCW light should support multiple modes including RGB_COLD_WARM_WHITE (value 51) + # RGBCW light (rgbww platform) should support RGB_COLD_WARM_WHITE mode assert ColorMode.RGB_COLD_WARM_WHITE in rgbcw_light.supported_color_modes, ( f"RGBCW light missing RGB_COLD_WARM_WHITE mode. Got: {rgbcw_light.supported_color_modes}" ) - # Verify actual enum values - expected_rgbcw_modes = { - ColorMode.RGB_COLD_WARM_WHITE, # 51 - # May have other modes too - } - assert expected_rgbcw_modes.issubset(set(rgbcw_light.supported_color_modes)), ( - f"RGBCW light missing expected color modes. Got: " - f"{[f'{mode.name}={mode.value}' for mode in rgbcw_light.supported_color_modes]}" + # Verify it's the actual enum value, not bit position + assert ColorMode.RGB_COLD_WARM_WHITE.value in [ + mode.value for mode in rgbcw_light.supported_color_modes + ], ( + f"RGBCW light has wrong color mode values. Expected {ColorMode.RGB_COLD_WARM_WHITE.value} " + f"(RGB_COLD_WARM_WHITE), got: {[mode.value for mode in rgbcw_light.supported_color_modes]}" + ) + + # RGB light should support RGB mode + assert ColorMode.RGB in rgb_light.supported_color_modes, ( + f"RGB light missing RGB color mode. Got: {rgb_light.supported_color_modes}" + ) + # Verify it's the actual enum value, not bit position + assert ColorMode.RGB.value in [ + mode.value for mode in rgb_light.supported_color_modes + ], ( + f"RGB light has wrong color mode values. Expected {ColorMode.RGB.value} (RGB), got: " + f"{[mode.value for mode in rgb_light.supported_color_modes]}" + ) + + # Binary light (on/off only) should support ON_OFF mode + assert ColorMode.ON_OFF in binary_light.supported_color_modes, ( + f"Binary light missing ON_OFF color mode. Got: {binary_light.supported_color_modes}" + ) + # Verify it's the actual enum value, not bit position + assert ColorMode.ON_OFF.value in [ + mode.value for mode in binary_light.supported_color_modes + ], ( + f"Binary light has wrong color mode values. Expected {ColorMode.ON_OFF.value} (ON_OFF), got: " + f"{[mode.value for mode in binary_light.supported_color_modes]}" ) async def wait_for_state_change(key: int, timeout: float = 1.0) -> Any: