From edb2145aca99fb737619acbc24f099327a359d5f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 15:55:50 -1000 Subject: [PATCH 01/11] [light] Collapse 8 clamp-and-copy blocks in LightCall::validate_ into a loop Reorder FieldFlags so the eight [0.0, 1.0]-clamped float fields occupy bits 0-7 in the same order as they appear in LightCall and LightColorValues, and move color_temperature_ to the end of both structs. Under that layout the LightCall offset for clamp field i is `offsetof(LightCall, brightness_) + i * 4`, and the LightColorValues offset is exactly 12 bytes lower for every field. validate_() now iterates the active clamp bits in a small loop that computes these offsets from the bit position instead of expanding eight nearly identical inline blocks via macro. The field-name PROGMEM pointer is passed to clamp_and_log_if_invalid as `const LogString *const *`; progmem_read_ptr only runs on the cold (out-of-range) path, so the hot path performs no flash reads for the name. The eight invariants the loop relies on (flag-bit layout, field contiguity, and the constant 12-byte delta) are enforced by static_asserts so any future reshuffle fails loudly at compile time. Size deltas for the isolated light component build (vs dev): ESP32-IDF: -118 B code, +32 B PROGMEM name table = -86 B net ESP8266: -56 B code, +32 B PROGMEM name table = -24 B net 0 B RAM impact on both targets. --- esphome/components/light/light_call.cpp | 84 ++++++++++++++----- esphome/components/light/light_call.h | 43 ++++++---- esphome/components/light/light_color_values.h | 10 ++- 3 files changed, 100 insertions(+), 37 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index a749cd7305a..de8f892b04c 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -10,10 +10,19 @@ namespace esphome::light { static const char *const TAG = "light"; -// Helper functions to reduce code size for logging -static void clamp_and_log_if_invalid(const char *name, float &value, const LogString *param_name, float min = 0.0f, - float max = 1.0f) { +// Helper functions to reduce code size for logging. +// +// `param_name_progmem` is a pointer to a flash-resident `const LogString *` +// slot (e.g. into the FIELD_NAMES table below). We only dereference it via +// `progmem_read_ptr` on the cold path that actually emits the log message, +// so the hot path (value in range) performs no flash reads at all. On +// non-ESP8266 platforms `progmem_read_ptr` is a plain `*addr` inline, so +// there is no cost there either. +static void clamp_and_log_if_invalid(const char *name, float &value, const LogString *const *param_name_progmem, + float min = 0.0f, float max = 1.0f) { if (value < min || value > max) { + const auto *param_name = reinterpret_cast( + progmem_read_ptr(reinterpret_cast(param_name_progmem))); ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, LOG_STR_ARG(param_name), value, min, max); value = clamp(value, min, max); } @@ -277,25 +286,60 @@ LightColorValues LightCall::validate_() { if (this->has_state()) v.set_state(this->state_); - // clamp_and_log_if_invalid already clamps in-place, so assign directly - // to avoid redundant clamp code from the setter being inlined. -#define VALIDATE_AND_APPLY(field, name_str, ...) \ - if (this->has_##field()) { \ - clamp_and_log_if_invalid(name, this->field##_, LOG_STR(name_str), ##__VA_ARGS__); \ - v.field##_ = this->field##_; \ + // Clamp the eight [0.0, 1.0] fields and copy them from `this` into `v`. + // + // LightCall and LightColorValues both declare the same eight float fields in + // the same order (brightness_, color_brightness_, red_, green_, blue_, + // white_, cold_white_, warm_white_), and their corresponding flag bits are + // also 0-7 in that order. Under that layout the LightCall offset for field i + // is `offsetof(LightCall, brightness_) + i * 4`, and the LightColorValues + // offset is exactly 12 bytes lower (enforced by the static_asserts below). + // Iterating via bit-position arithmetic lets us collapse eight inlined + // clamp/copy blocks into a single loop. + static_assert(FLAG_HAS_BRIGHTNESS == 1u << 0, "clamp loop assumes bit 0"); + static_assert(FLAG_HAS_WARM_WHITE == 1u << 7, "clamp loop assumes bit 7"); + static_assert(offsetof(LightCall, warm_white_) - offsetof(LightCall, brightness_) == 7 * sizeof(float), + "LightCall clamp fields must be contiguous"); + static_assert(offsetof(LightColorValues, warm_white_) - offsetof(LightColorValues, brightness_) == 7 * sizeof(float), + "LightColorValues clamp fields must be contiguous"); + static_assert(offsetof(LightCall, brightness_) - offsetof(LightColorValues, brightness_) == 12, + "LightCall and LightColorValues clamp fields must have constant byte-offset delta"); + + static const LogString *const FIELD_NAMES[8] PROGMEM = { + LOG_STR("Brightness"), // FLAG_HAS_BRIGHTNESS (bit 0) + LOG_STR("Color brightness"), // FLAG_HAS_COLOR_BRIGHTNESS (bit 1) + LOG_STR("Red"), // FLAG_HAS_RED (bit 2) + LOG_STR("Green"), // FLAG_HAS_GREEN (bit 3) + LOG_STR("Blue"), // FLAG_HAS_BLUE (bit 4) + LOG_STR("White"), // FLAG_HAS_WHITE (bit 5) + LOG_STR("Cold white"), // FLAG_HAS_COLD_WHITE (bit 6) + LOG_STR("Warm white"), // FLAG_HAS_WARM_WHITE (bit 7) + }; + constexpr size_t SRC_BASE = offsetof(LightCall, brightness_); + constexpr size_t SRC_TO_DST_DELTA = SRC_BASE - offsetof(LightColorValues, brightness_); + + uint8_t active = this->flags_ & CLAMP_FLAGS_MASK; + if (active != 0) { + auto *self = reinterpret_cast(this); + auto *out = reinterpret_cast(&v); + for (uint8_t bit = 0; bit < 8; bit++) { + if (!(active & (1u << bit))) + continue; + const size_t src_off = SRC_BASE + bit * sizeof(float); + float &f = *reinterpret_cast(self + src_off); + clamp_and_log_if_invalid(name, f, &FIELD_NAMES[bit]); + *reinterpret_cast(out + src_off - SRC_TO_DST_DELTA) = f; + } } - VALIDATE_AND_APPLY(brightness, "Brightness") - VALIDATE_AND_APPLY(color_brightness, "Color brightness") - VALIDATE_AND_APPLY(red, "Red") - VALIDATE_AND_APPLY(green, "Green") - VALIDATE_AND_APPLY(blue, "Blue") - VALIDATE_AND_APPLY(white, "White") - VALIDATE_AND_APPLY(cold_white, "Cold white") - VALIDATE_AND_APPLY(warm_white, "Warm white") - VALIDATE_AND_APPLY(color_temperature, "Color temperature", traits.get_min_mireds(), traits.get_max_mireds()) - -#undef VALIDATE_AND_APPLY + // color_temperature uses a dynamic range from the light's traits and is + // handled separately. + if (this->has_color_temperature()) { + static const LogString *const CT_NAME PROGMEM = LOG_STR("Color temperature"); + clamp_and_log_if_invalid(name, this->color_temperature_, &CT_NAME, traits.get_min_mireds(), + traits.get_max_mireds()); + v.color_temperature_ = this->color_temperature_; + } v.normalize_color(); diff --git a/esphome/components/light/light_call.h b/esphome/components/light/light_call.h index 88d29bd3490..9f34297dcda 100644 --- a/esphome/components/light/light_call.h +++ b/esphome/components/light/light_call.h @@ -196,24 +196,31 @@ class LightCall { void transform_parameters_(const LightTraits &traits); // Bitfield flags - each flag indicates whether a corresponding value has been set. + // + // Bits 0-7 are the eight float fields that share the [0.0, 1.0] clamp range, + // in member declaration order. The validate_() clamp loop relies on this + // layout to index into LightCall/LightColorValues via bit-position arithmetic + // without a per-field offset table. Do not reorder without updating the + // static_asserts in light_call.cpp. enum FieldFlags : uint16_t { - FLAG_HAS_STATE = 1 << 0, - FLAG_HAS_TRANSITION = 1 << 1, - FLAG_HAS_FLASH = 1 << 2, - FLAG_HAS_EFFECT = 1 << 3, - FLAG_HAS_BRIGHTNESS = 1 << 4, - FLAG_HAS_COLOR_BRIGHTNESS = 1 << 5, - FLAG_HAS_RED = 1 << 6, - FLAG_HAS_GREEN = 1 << 7, - FLAG_HAS_BLUE = 1 << 8, - FLAG_HAS_WHITE = 1 << 9, - FLAG_HAS_COLOR_TEMPERATURE = 1 << 10, - FLAG_HAS_COLD_WHITE = 1 << 11, - FLAG_HAS_WARM_WHITE = 1 << 12, + FLAG_HAS_BRIGHTNESS = 1 << 0, + FLAG_HAS_COLOR_BRIGHTNESS = 1 << 1, + FLAG_HAS_RED = 1 << 2, + FLAG_HAS_GREEN = 1 << 3, + FLAG_HAS_BLUE = 1 << 4, + FLAG_HAS_WHITE = 1 << 5, + FLAG_HAS_COLD_WHITE = 1 << 6, + FLAG_HAS_WARM_WHITE = 1 << 7, + FLAG_HAS_COLOR_TEMPERATURE = 1 << 8, + FLAG_HAS_STATE = 1 << 9, + FLAG_HAS_TRANSITION = 1 << 10, + FLAG_HAS_FLASH = 1 << 11, + FLAG_HAS_EFFECT = 1 << 12, FLAG_HAS_COLOR_MODE = 1 << 13, FLAG_PUBLISH = 1 << 14, FLAG_SAVE = 1 << 15, }; + static constexpr uint16_t CLAMP_FLAGS_MASK = 0x00FFu; // bits 0-7 inline bool has_transition_() { return (this->flags_ & FLAG_HAS_TRANSITION) != 0; } inline bool has_flash_() { return (this->flags_ & FLAG_HAS_FLASH) != 0; } @@ -239,7 +246,13 @@ class LightCall { LightState *parent_; // Light state values - use flags_ to check if a value has been set. - // Group 4-byte aligned members first + // Group 4-byte aligned members first. + // + // The eight [0.0, 1.0]-clamped float fields (brightness_ ... warm_white_) + // are declared in the same order as their flag bits (0-7) and the matching + // fields in LightColorValues. validate_() exploits this to iterate them via + // bit-position arithmetic. color_temperature_ has a custom range and lives + // outside that block. uint32_t transition_length_; uint32_t flash_length_; uint32_t effect_; @@ -249,9 +262,9 @@ class LightCall { float green_; float blue_; float white_; - float color_temperature_; float cold_white_; float warm_white_; + float color_temperature_; // Smaller members at the end for better packing uint16_t flags_{FLAG_PUBLISH | FLAG_SAVE}; // Tracks which values are set diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index fa286a3941b..c520f4dc250 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -52,9 +52,9 @@ class LightColorValues { green_(1.0f), blue_(1.0f), white_(1.0f), - color_temperature_{0.0f}, cold_white_{1.0f}, warm_white_{1.0f}, + color_temperature_{0.0f}, color_mode_(ColorMode::UNKNOWN) {} LightColorValues(ColorMode color_mode, float state, float brightness, float color_brightness, float red, float green, @@ -287,6 +287,12 @@ class LightColorValues { friend class LightCall; protected: + // The eight [0.0, 1.0]-clamped float fields are declared in the same order + // as their flag bits (0-7) in LightCall::FieldFlags and the matching fields + // in LightCall. LightCall::validate_() exploits this layout to iterate and + // copy them via bit-position arithmetic with a constant delta of 12 bytes + // between matching LightCall and LightColorValues members. color_temperature_ + // has a different range and is placed after the clamp block. float state_; ///< ON / OFF, float for transition float brightness_; float color_brightness_; @@ -294,9 +300,9 @@ class LightColorValues { float green_; float blue_; float white_; - float color_temperature_; ///< Color Temperature in Mired float cold_white_; float warm_white_; + float color_temperature_; ///< Color Temperature in Mired ColorMode color_mode_; }; From cdde0abec7fcf63d1858b868463216b5691c2356 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 16:07:34 -1000 Subject: [PATCH 02/11] [light] Refine validate_ clamp loop: ctz iteration, per-field asserts, typed pointers Follow-up to edb2145a addressing three points: 1. Hoist the in-range check out of the logging helper. The loop now tests `value < 0.0f || value > 1.0f` inline and only calls the out-of-line log_out_of_range_and_clamp_ helper on the cold path. Hot path (value in range) skips the call8 and the register spill/reload around it, which matters because HA automations can drive perform() at high frequency. 2. Iterate only set bits with __builtin_ctz + (active & active-1). Common calls with one or two flags set now exit the loop after one or two iterations instead of always scanning all eight slots. 3. Replace the uint8_t* pointer arithmetic with typed float arrays aliasing &brightness_ in each struct. Per-field static_asserts (expanded via a local macro) now catch reorders of any single member in either struct, not just reorders at the endpoints. Compiles to the same machine code as the uint8_t* version. Size delta vs. prior commit (isolated light build): ESP32-IDF: validate_ +60 B, helper -29 B, net +31 B ESP8266: validate_ +72 B, helper -42 B, net +30 B Still a net win vs. dev on both targets (ESP32-IDF -91 B, ESP8266 -22 B). --- esphome/components/light/light_call.cpp | 98 +++++++++++++++---------- 1 file changed, 58 insertions(+), 40 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index de8f892b04c..df65883ed66 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -10,22 +10,17 @@ namespace esphome::light { static const char *const TAG = "light"; -// Helper functions to reduce code size for logging. -// -// `param_name_progmem` is a pointer to a flash-resident `const LogString *` -// slot (e.g. into the FIELD_NAMES table below). We only dereference it via -// `progmem_read_ptr` on the cold path that actually emits the log message, -// so the hot path (value in range) performs no flash reads at all. On -// non-ESP8266 platforms `progmem_read_ptr` is a plain `*addr` inline, so -// there is no cost there either. -static void clamp_and_log_if_invalid(const char *name, float &value, const LogString *const *param_name_progmem, - float min = 0.0f, float max = 1.0f) { - if (value < min || value > max) { - const auto *param_name = reinterpret_cast( - progmem_read_ptr(reinterpret_cast(param_name_progmem))); - ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, LOG_STR_ARG(param_name), value, min, max); - value = clamp(value, min, max); - } +// Cold-path helper: called only when the caller has already determined the +// value is out of range. Keeping the range check at the caller avoids the +// call-site spill/reload and prologue on the hot path (in-range). The +// `param_name_progmem` argument points into the FIELD_NAMES table in flash; +// `progmem_read_ptr` is a plain `*addr` inline on non-ESP8266 platforms. +static void log_out_of_range_and_clamp_(const char *name, float &value, const LogString *const *param_name_progmem, + float min, float max) { + const auto *param_name = + reinterpret_cast(progmem_read_ptr(reinterpret_cast(param_name_progmem))); + ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, LOG_STR_ARG(param_name), value, min, max); + value = clamp(value, min, max); } #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_WARN @@ -296,14 +291,31 @@ LightColorValues LightCall::validate_() { // offset is exactly 12 bytes lower (enforced by the static_asserts below). // Iterating via bit-position arithmetic lets us collapse eight inlined // clamp/copy blocks into a single loop. - static_assert(FLAG_HAS_BRIGHTNESS == 1u << 0, "clamp loop assumes bit 0"); - static_assert(FLAG_HAS_WARM_WHITE == 1u << 7, "clamp loop assumes bit 7"); - static_assert(offsetof(LightCall, warm_white_) - offsetof(LightCall, brightness_) == 7 * sizeof(float), - "LightCall clamp fields must be contiguous"); - static_assert(offsetof(LightColorValues, warm_white_) - offsetof(LightColorValues, brightness_) == 7 * sizeof(float), - "LightColorValues clamp fields must be contiguous"); - static_assert(offsetof(LightCall, brightness_) - offsetof(LightColorValues, brightness_) == 12, - "LightCall and LightColorValues clamp fields must have constant byte-offset delta"); + constexpr size_t SRC_BASE = offsetof(LightCall, brightness_); + constexpr size_t SRC_TO_DST_DELTA = SRC_BASE - offsetof(LightColorValues, brightness_); + + // Per-field layout assertions: each clamp field must sit at its bit-indexed + // slot in both LightCall and LightColorValues, with the same byte-offset + // delta. A reorder of any single field (in either struct) trips the assert + // pointing at that field, so failures name the exact member at fault. + // The one case these cannot catch is a synchronized reorder in both structs + // plus FIELD_NAMES — that would compile silently, but requires deliberate + // three-place changes by the refactorer. +#define ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(bit, flag_suffix, member) \ + static_assert(FLAG_HAS_##flag_suffix == 1u << (bit), "FLAG_HAS_" #flag_suffix " bit position"); \ + static_assert(offsetof(LightCall, member) == SRC_BASE + (bit) * sizeof(float), \ + "LightCall::" #member " must be at bit-indexed slot"); \ + static_assert(offsetof(LightColorValues, member) == SRC_BASE + (bit) * sizeof(float) - SRC_TO_DST_DELTA, \ + "LightColorValues::" #member " must match LightCall delta") + ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(0, BRIGHTNESS, brightness_); + ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(1, COLOR_BRIGHTNESS, color_brightness_); + ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(2, RED, red_); + ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(3, GREEN, green_); + ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(4, BLUE, blue_); + ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(5, WHITE, white_); + ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(6, COLD_WHITE, cold_white_); + ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(7, WARM_WHITE, warm_white_); +#undef ESPHOME_LIGHT_ASSERT_CLAMP_FIELD static const LogString *const FIELD_NAMES[8] PROGMEM = { LOG_STR("Brightness"), // FLAG_HAS_BRIGHTNESS (bit 0) @@ -315,29 +327,35 @@ LightColorValues LightCall::validate_() { LOG_STR("Cold white"), // FLAG_HAS_COLD_WHITE (bit 6) LOG_STR("Warm white"), // FLAG_HAS_WARM_WHITE (bit 7) }; - constexpr size_t SRC_BASE = offsetof(LightCall, brightness_); - constexpr size_t SRC_TO_DST_DELTA = SRC_BASE - offsetof(LightColorValues, brightness_); - uint8_t active = this->flags_ & CLAMP_FLAGS_MASK; - if (active != 0) { - auto *self = reinterpret_cast(this); - auto *out = reinterpret_cast(&v); - for (uint8_t bit = 0; bit < 8; bit++) { - if (!(active & (1u << bit))) - continue; - const size_t src_off = SRC_BASE + bit * sizeof(float); - float &f = *reinterpret_cast(self + src_off); - clamp_and_log_if_invalid(name, f, &FIELD_NAMES[bit]); - *reinterpret_cast(out + src_off - SRC_TO_DST_DELTA) = f; - } + // The static_asserts above guarantee the eight clampable floats are laid + // out consecutively starting at brightness_ in both structs, so we can + // treat `&brightness_` as the base of an 8-element float array and index + // by bit position directly. Iterate only the set bits via __builtin_ctz + + // clear-lowest-bit: HA can drive high-frequency automations through + // perform(), so the hot path runs in O(popcount) instead of always + // scanning all eight slots. The range check is inlined here (cold path + // is the out-of-line helper) so an in-range value skips the call entirely. + float *const src_fields = &this->brightness_; + float *const dst_fields = &v.brightness_; + unsigned active = this->flags_ & CLAMP_FLAGS_MASK; + while (active != 0) { + unsigned bit = __builtin_ctz(active); + active &= active - 1; // clear lowest set bit + float &value = src_fields[bit]; + if (value < 0.0f || value > 1.0f) + log_out_of_range_and_clamp_(name, value, &FIELD_NAMES[bit], 0.0f, 1.0f); + dst_fields[bit] = value; } // color_temperature uses a dynamic range from the light's traits and is // handled separately. if (this->has_color_temperature()) { static const LogString *const CT_NAME PROGMEM = LOG_STR("Color temperature"); - clamp_and_log_if_invalid(name, this->color_temperature_, &CT_NAME, traits.get_min_mireds(), - traits.get_max_mireds()); + const float ct_min = traits.get_min_mireds(); + const float ct_max = traits.get_max_mireds(); + if (this->color_temperature_ < ct_min || this->color_temperature_ > ct_max) + log_out_of_range_and_clamp_(name, this->color_temperature_, &CT_NAME, ct_min, ct_max); v.color_temperature_ = this->color_temperature_; } From 36881166a8868676a48cc23350cfb9916b9cee99 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 16:16:44 -1000 Subject: [PATCH 03/11] [light] Replace soft-float range check with union bit-cast + unsigned compare MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inlined `value < 0.0f || value > 1.0f` check in the clamp loop costs ~50 B per iteration on ESP8266 (two libgcc soft-float calls with register spills). IEEE 754 floats in [0.0f, 1.0f] have bit patterns in [0x00000000, 0x3F800000]; anything out of range — values > 1.0f, negatives (sign bit set → huge unsigned interpretation), NaN, Infinity — has a strictly larger unsigned interpretation. A single `pun.u > 0x3F800000u` covers every case. Using a union for the type-pun rather than memcpy/bit_cast because those don't optimize to a no-op on xtensa-gcc (same reason api/proto.h's float_to_raw() uses a union). The loop body is now two instructions for the range check: l32i a2, a9, 0 ; load raw u32 bgeu a10, a2, ... ; compare against pre-hoisted 1.0f bits Size delta vs. the prior float-compare form: ESP32-IDF: validate_ -16 B, net -12 B ESP8266: validate_ -20 B, net -20 B vs. dev baseline (isolated light build, matched funcs): ESP32-IDF: -103 B code, +32 B PROGMEM table = -71 B net ESP8266: -42 B code, +32 B PROGMEM table = -10 B net --- esphome/components/light/light_call.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index df65883ed66..045a4f29356 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -334,8 +334,16 @@ LightColorValues LightCall::validate_() { // by bit position directly. Iterate only the set bits via __builtin_ctz + // clear-lowest-bit: HA can drive high-frequency automations through // perform(), so the hot path runs in O(popcount) instead of always - // scanning all eight slots. The range check is inlined here (cold path - // is the out-of-line helper) so an in-range value skips the call entirely. + // scanning all eight slots. + // + // The range check is done on the IEEE 754 bit pattern as an unsigned int, + // not on the float itself. Values in [0.0f, 1.0f] have bits in + // [0x00000000, 0x3F800000]; anything greater (as unsigned) is out of range: + // values > 1.0f have a larger bit pattern, and negative values have the + // sign bit (0x80000000) set which makes their unsigned interpretation + // enormous. One unsigned compare replaces two soft-float __ltsf2/__gtsf2 + // calls on ESP8266 and is essentially free on targets with an FPU too. + constexpr uint32_t ONE_F_BITS = 0x3F800000u; // bit pattern of 1.0f float *const src_fields = &this->brightness_; float *const dst_fields = &v.brightness_; unsigned active = this->flags_ & CLAMP_FLAGS_MASK; @@ -343,7 +351,14 @@ LightColorValues LightCall::validate_() { unsigned bit = __builtin_ctz(active); active &= active - 1; // clear lowest set bit float &value = src_fields[bit]; - if (value < 0.0f || value > 1.0f) + // Union type-pun (GCC/Clang extension): bit_cast/memcpy don't optimize to + // a no-op on xtensa-gcc, same reasoning as api/proto.h float_to_raw(). + union { + float f; + uint32_t u; + } pun; + pun.f = value; + if (pun.u > ONE_F_BITS) log_out_of_range_and_clamp_(name, value, &FIELD_NAMES[bit], 0.0f, 1.0f); dst_fields[bit] = value; } From 8eb8b3dc0fed6d394b5ff418d316052664fbde77 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 16:27:09 -1000 Subject: [PATCH 04/11] [light] Promote bit-pattern clamp to LightColorValues setters and add layout asserts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move `float_out_of_unit_range()` / add `clamp_unit_float()` to light_color_values.h so the nine `set_*(float)` setters can use the unsigned bit-pattern clamp instead of `std::clamp(x, 0.0f, 1.0f)`. `std::clamp` expands to two soft-float `__ltsf2`/`__gtsf2` calls per invocation on ESP8266 — replacing it with a single unsigned compare saves code across every caller of these setters (StrobeLightEffect, the 11-arg LightColorValues constructor, external components). Split the cold-path helper: `log_value_out_of_range_()` now only logs, and each caller applies the clamp strategy appropriate to its range (`clamp_unit_float` for the 8-field loop, `std::clamp` for color temperature's runtime-variable range). Add layout/format assertions: - std::is_standard_layout_v on LightCall and LightColorValues so the offsetof arithmetic in the clamp loop is well-defined. - sizeof(float) == 4 and is_iec559 so the bit-pattern trick is valid. A direct __builtin_bit_cast check would be cleaner but is not available on the ESP8266 xtensa toolchain. Text-section delta vs. prior commit (isolated light build): ESP32-IDF: .flash.text 137760 -> 136988 (-772 B) ESP8266: .irom0.text 268440 -> 267912 (-528 B) --- esphome/components/light/light_call.cpp | 59 +++++++++--------- esphome/components/light/light_color_values.h | 62 ++++++++++++++++--- 2 files changed, 83 insertions(+), 38 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 045a4f29356..bc0b12aa773 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -1,4 +1,5 @@ #include +#include #include "light_call.h" #include "light_state.h" @@ -10,17 +11,19 @@ namespace esphome::light { static const char *const TAG = "light"; -// Cold-path helper: called only when the caller has already determined the -// value is out of range. Keeping the range check at the caller avoids the -// call-site spill/reload and prologue on the hot path (in-range). The -// `param_name_progmem` argument points into the FIELD_NAMES table in flash; -// `progmem_read_ptr` is a plain `*addr` inline on non-ESP8266 platforms. -static void log_out_of_range_and_clamp_(const char *name, float &value, const LogString *const *param_name_progmem, - float min, float max) { +// Cold-path logger: called only after the caller has determined `value` is +// out of range. Does not clamp — the caller handles that with the strategy +// appropriate to its range (bit-pattern clamp_unit_float for [0,1] on the +// hot path, std::clamp for arbitrary ranges like color_temperature). Keeping +// the range check at the caller avoids the call-site spill/reload and +// prologue when the value is in range. The `param_name_progmem` argument +// points into the FIELD_NAMES table in flash; `progmem_read_ptr` is a plain +// `*addr` inline on non-ESP8266 platforms. +static void log_value_out_of_range_(const char *name, float value, const LogString *const *param_name_progmem, + float min, float max) { const auto *param_name = reinterpret_cast(progmem_read_ptr(reinterpret_cast(param_name_progmem))); ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, LOG_STR_ARG(param_name), value, min, max); - value = clamp(value, min, max); } #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_WARN @@ -291,6 +294,15 @@ LightColorValues LightCall::validate_() { // offset is exactly 12 bytes lower (enforced by the static_asserts below). // Iterating via bit-position arithmetic lets us collapse eight inlined // clamp/copy blocks into a single loop. + // offsetof is only well-defined on standard-layout types (C++17 relaxed it + // slightly, but GCC still warns on non-standard-layout). Verify here rather + // than relying on diagnostics: a future change that adds a virtual base, a + // non-public data member mixed with public ones, or a derived-class data + // member would break the layout contract below. + static_assert(std::is_standard_layout_v, "LightCall must be standard-layout for offsetof arithmetic"); + static_assert(std::is_standard_layout_v, + "LightColorValues must be standard-layout for offsetof arithmetic"); + constexpr size_t SRC_BASE = offsetof(LightCall, brightness_); constexpr size_t SRC_TO_DST_DELTA = SRC_BASE - offsetof(LightColorValues, brightness_); @@ -335,15 +347,6 @@ LightColorValues LightCall::validate_() { // clear-lowest-bit: HA can drive high-frequency automations through // perform(), so the hot path runs in O(popcount) instead of always // scanning all eight slots. - // - // The range check is done on the IEEE 754 bit pattern as an unsigned int, - // not on the float itself. Values in [0.0f, 1.0f] have bits in - // [0x00000000, 0x3F800000]; anything greater (as unsigned) is out of range: - // values > 1.0f have a larger bit pattern, and negative values have the - // sign bit (0x80000000) set which makes their unsigned interpretation - // enormous. One unsigned compare replaces two soft-float __ltsf2/__gtsf2 - // calls on ESP8266 and is essentially free on targets with an FPU too. - constexpr uint32_t ONE_F_BITS = 0x3F800000u; // bit pattern of 1.0f float *const src_fields = &this->brightness_; float *const dst_fields = &v.brightness_; unsigned active = this->flags_ & CLAMP_FLAGS_MASK; @@ -351,26 +354,24 @@ LightColorValues LightCall::validate_() { unsigned bit = __builtin_ctz(active); active &= active - 1; // clear lowest set bit float &value = src_fields[bit]; - // Union type-pun (GCC/Clang extension): bit_cast/memcpy don't optimize to - // a no-op on xtensa-gcc, same reasoning as api/proto.h float_to_raw(). - union { - float f; - uint32_t u; - } pun; - pun.f = value; - if (pun.u > ONE_F_BITS) - log_out_of_range_and_clamp_(name, value, &FIELD_NAMES[bit], 0.0f, 1.0f); + if (float_out_of_unit_range(value)) { + log_value_out_of_range_(name, value, &FIELD_NAMES[bit], 0.0f, 1.0f); + value = clamp_unit_float(value); + } dst_fields[bit] = value; } // color_temperature uses a dynamic range from the light's traits and is - // handled separately. + // handled separately. No bit-pattern shortcut here because the range is + // runtime-variable. if (this->has_color_temperature()) { static const LogString *const CT_NAME PROGMEM = LOG_STR("Color temperature"); const float ct_min = traits.get_min_mireds(); const float ct_max = traits.get_max_mireds(); - if (this->color_temperature_ < ct_min || this->color_temperature_ > ct_max) - log_out_of_range_and_clamp_(name, this->color_temperature_, &CT_NAME, ct_min, ct_max); + if (this->color_temperature_ < ct_min || this->color_temperature_ > ct_max) { + log_value_out_of_range_(name, this->color_temperature_, &CT_NAME, ct_min, ct_max); + this->color_temperature_ = clamp(this->color_temperature_, ct_min, ct_max); + } v.color_temperature_ = this->color_temperature_; } diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index c520f4dc250..74af29fe59f 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -3,11 +3,55 @@ #include "esphome/core/helpers.h" #include "color_mode.h" #include +#include +#include namespace esphome::light { inline static uint8_t to_uint8_scale(float x) { return static_cast(roundf(x * 255.0f)); } +// IEEE 754 bit pattern of 1.0f. Floats in [0.0f, 1.0f] have unsigned bit +// pattern <= this value; negatives have the sign bit set (→ huge unsigned), +// values > 1.0f have a larger exponent, and NaN/Infinity also exceed this. +// Verify the platform actually provides IEEE 754 single-precision floats so +// the bit-pattern tricks below are well-defined. +static constexpr uint32_t ONE_F_BITS = 0x3F800000u; +// sizeof check + is_iec559 together pin the format to IEEE 754 single-precision, +// which fixes the bit pattern of 1.0f as 0x3F800000. A direct bit-cast check +// would be cleaner but __builtin_bit_cast is not available on the older xtensa +// toolchain used for ESP8266. +static_assert(sizeof(float) == sizeof(uint32_t), "float must be 32-bit for bit-pattern range checks"); +static_assert(std::numeric_limits::is_iec559, "IEEE 754 single-precision float required"); + +// Returns true iff `x` is outside [0.0f, 1.0f] via a single unsigned compare on +// its IEEE 754 bit pattern. Uses a union type-pun (GCC/Clang extension) because +// memcpy/bit_cast don't optimize to a no-op on xtensa-gcc (same reasoning as +// api/proto.h's float_to_raw). Replaces two soft-float __ltsf2/__gtsf2 calls +// with one `bltu` on ESP8266 and is free on FPU targets. +inline bool float_out_of_unit_range(float x) { + union { + float f; + uint32_t u; + } pun; + pun.f = x; + return pun.u > ONE_F_BITS; +} + +// Clamps `x` to [0.0f, 1.0f] with no floating-point compares. In-range values +// return via a single branch; out-of-range pick 0.0f for negatives (sign bit +// set) and 1.0f otherwise (> 1.0f, NaN, Infinity). Cheaper than std::clamp on +// ESP8266, which expands to two soft-float calls per invocation. +inline float clamp_unit_float(float x) { + union { + float f; + uint32_t u; + } pun; + pun.f = x; + if (pun.u <= ONE_F_BITS) + return x; + return (pun.u & 0x80000000u) ? 0.0f : 1.0f; +} + /** This class represents the color state for a light object. * * The representation of the color state is dependent on the active color mode. A color mode consists of multiple @@ -220,39 +264,39 @@ class LightColorValues { /// Get the binary true/false state of these light color values. bool is_on() const { return this->get_state() != 0.0f; } /// Set the state of these light color values. In range from 0.0 (off) to 1.0 (on) - void set_state(float state) { this->state_ = clamp(state, 0.0f, 1.0f); } + void set_state(float state) { this->state_ = clamp_unit_float(state); } /// Set the state of these light color values as a binary true/false. void set_state(bool state) { this->state_ = state ? 1.0f : 0.0f; } /// Get the brightness property of these light color values. In range 0.0 to 1.0 float get_brightness() const { return this->brightness_; } /// Set the brightness property of these light color values. In range 0.0 to 1.0 - void set_brightness(float brightness) { this->brightness_ = clamp(brightness, 0.0f, 1.0f); } + void set_brightness(float brightness) { this->brightness_ = clamp_unit_float(brightness); } /// Get the color brightness property of these light color values. In range 0.0 to 1.0 float get_color_brightness() const { return this->color_brightness_; } /// Set the color brightness property of these light color values. In range 0.0 to 1.0 - void set_color_brightness(float brightness) { this->color_brightness_ = clamp(brightness, 0.0f, 1.0f); } + void set_color_brightness(float brightness) { this->color_brightness_ = clamp_unit_float(brightness); } /// Get the red property of these light color values. In range 0.0 to 1.0 float get_red() const { return this->red_; } /// Set the red property of these light color values. In range 0.0 to 1.0 - void set_red(float red) { this->red_ = clamp(red, 0.0f, 1.0f); } + void set_red(float red) { this->red_ = clamp_unit_float(red); } /// Get the green property of these light color values. In range 0.0 to 1.0 float get_green() const { return this->green_; } /// Set the green property of these light color values. In range 0.0 to 1.0 - void set_green(float green) { this->green_ = clamp(green, 0.0f, 1.0f); } + void set_green(float green) { this->green_ = clamp_unit_float(green); } /// Get the blue property of these light color values. In range 0.0 to 1.0 float get_blue() const { return this->blue_; } /// Set the blue property of these light color values. In range 0.0 to 1.0 - void set_blue(float blue) { this->blue_ = clamp(blue, 0.0f, 1.0f); } + void set_blue(float blue) { this->blue_ = clamp_unit_float(blue); } /// Get the white property of these light color values. In range 0.0 to 1.0 float get_white() const { return white_; } /// Set the white property of these light color values. In range 0.0 to 1.0 - void set_white(float white) { this->white_ = clamp(white, 0.0f, 1.0f); } + void set_white(float white) { this->white_ = clamp_unit_float(white); } /// Get the color temperature property of these light color values in mired. float get_color_temperature() const { return this->color_temperature_; } @@ -277,12 +321,12 @@ class LightColorValues { /// Get the cold white property of these light color values. In range 0.0 to 1.0. float get_cold_white() const { return this->cold_white_; } /// Set the cold white property of these light color values. In range 0.0 to 1.0. - void set_cold_white(float cold_white) { this->cold_white_ = clamp(cold_white, 0.0f, 1.0f); } + void set_cold_white(float cold_white) { this->cold_white_ = clamp_unit_float(cold_white); } /// Get the warm white property of these light color values. In range 0.0 to 1.0. float get_warm_white() const { return this->warm_white_; } /// Set the warm white property of these light color values. In range 0.0 to 1.0. - void set_warm_white(float warm_white) { this->warm_white_ = clamp(warm_white, 0.0f, 1.0f); } + void set_warm_white(float warm_white) { this->warm_white_ = clamp_unit_float(warm_white); } friend class LightCall; From de44b8e859bf500721db4a4124785ac9a497bbc8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 16:31:50 -1000 Subject: [PATCH 05/11] [light] Tighten comments in validate_ clamp loop and LightColorValues helpers --- esphome/components/light/light_call.cpp | 54 +++++-------------- esphome/components/light/light_call.h | 17 ++---- esphome/components/light/light_color_values.h | 37 ++++--------- 3 files changed, 27 insertions(+), 81 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index bc0b12aa773..43fea14e4e6 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -11,14 +11,9 @@ namespace esphome::light { static const char *const TAG = "light"; -// Cold-path logger: called only after the caller has determined `value` is -// out of range. Does not clamp — the caller handles that with the strategy -// appropriate to its range (bit-pattern clamp_unit_float for [0,1] on the -// hot path, std::clamp for arbitrary ranges like color_temperature). Keeping -// the range check at the caller avoids the call-site spill/reload and -// prologue when the value is in range. The `param_name_progmem` argument -// points into the FIELD_NAMES table in flash; `progmem_read_ptr` is a plain -// `*addr` inline on non-ESP8266 platforms. +// Cold-path logger. Caller handles the clamp so the in-range hot path avoids +// the call-site spill/reload. `param_name_progmem` is a pointer into FIELD_NAMES +// in flash; the `progmem_read_ptr` is a no-op on non-ESP8266. static void log_value_out_of_range_(const char *name, float value, const LogString *const *param_name_progmem, float min, float max) { const auto *param_name = @@ -285,34 +280,16 @@ LightColorValues LightCall::validate_() { v.set_state(this->state_); // Clamp the eight [0.0, 1.0] fields and copy them from `this` into `v`. - // - // LightCall and LightColorValues both declare the same eight float fields in - // the same order (brightness_, color_brightness_, red_, green_, blue_, - // white_, cold_white_, warm_white_), and their corresponding flag bits are - // also 0-7 in that order. Under that layout the LightCall offset for field i - // is `offsetof(LightCall, brightness_) + i * 4`, and the LightColorValues - // offset is exactly 12 bytes lower (enforced by the static_asserts below). - // Iterating via bit-position arithmetic lets us collapse eight inlined - // clamp/copy blocks into a single loop. - // offsetof is only well-defined on standard-layout types (C++17 relaxed it - // slightly, but GCC still warns on non-standard-layout). Verify here rather - // than relying on diagnostics: a future change that adds a virtual base, a - // non-public data member mixed with public ones, or a derived-class data - // member would break the layout contract below. - static_assert(std::is_standard_layout_v, "LightCall must be standard-layout for offsetof arithmetic"); - static_assert(std::is_standard_layout_v, - "LightColorValues must be standard-layout for offsetof arithmetic"); + // Both structs declare the same fields in the same order as FieldFlags bits + // 0-7, with a constant byte-offset delta. The asserts below pin that layout + // so the loop can index by bit position; any single-field reorder trips the + // assert naming the field at fault. + static_assert(std::is_standard_layout_v, "LightCall must be standard-layout"); + static_assert(std::is_standard_layout_v, "LightColorValues must be standard-layout"); constexpr size_t SRC_BASE = offsetof(LightCall, brightness_); constexpr size_t SRC_TO_DST_DELTA = SRC_BASE - offsetof(LightColorValues, brightness_); - // Per-field layout assertions: each clamp field must sit at its bit-indexed - // slot in both LightCall and LightColorValues, with the same byte-offset - // delta. A reorder of any single field (in either struct) trips the assert - // pointing at that field, so failures name the exact member at fault. - // The one case these cannot catch is a synchronized reorder in both structs - // plus FIELD_NAMES — that would compile silently, but requires deliberate - // three-place changes by the refactorer. #define ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(bit, flag_suffix, member) \ static_assert(FLAG_HAS_##flag_suffix == 1u << (bit), "FLAG_HAS_" #flag_suffix " bit position"); \ static_assert(offsetof(LightCall, member) == SRC_BASE + (bit) * sizeof(float), \ @@ -340,13 +317,8 @@ LightColorValues LightCall::validate_() { LOG_STR("Warm white"), // FLAG_HAS_WARM_WHITE (bit 7) }; - // The static_asserts above guarantee the eight clampable floats are laid - // out consecutively starting at brightness_ in both structs, so we can - // treat `&brightness_` as the base of an 8-element float array and index - // by bit position directly. Iterate only the set bits via __builtin_ctz + - // clear-lowest-bit: HA can drive high-frequency automations through - // perform(), so the hot path runs in O(popcount) instead of always - // scanning all eight slots. + // Iterate only the set bits (ctz + clear-lowest) so the hot path is + // O(popcount) — HA can drive perform() at high frequency. float *const src_fields = &this->brightness_; float *const dst_fields = &v.brightness_; unsigned active = this->flags_ & CLAMP_FLAGS_MASK; @@ -361,9 +333,7 @@ LightColorValues LightCall::validate_() { dst_fields[bit] = value; } - // color_temperature uses a dynamic range from the light's traits and is - // handled separately. No bit-pattern shortcut here because the range is - // runtime-variable. + // color_temperature has a runtime range from traits — no bit-pattern shortcut. if (this->has_color_temperature()) { static const LogString *const CT_NAME PROGMEM = LOG_STR("Color temperature"); const float ct_min = traits.get_min_mireds(); diff --git a/esphome/components/light/light_call.h b/esphome/components/light/light_call.h index 9f34297dcda..01591143d3b 100644 --- a/esphome/components/light/light_call.h +++ b/esphome/components/light/light_call.h @@ -195,12 +195,9 @@ class LightCall { /// Some color modes also can be set using non-native parameters, transform those calls. void transform_parameters_(const LightTraits &traits); - // Bitfield flags - each flag indicates whether a corresponding value has been set. - // - // Bits 0-7 are the eight float fields that share the [0.0, 1.0] clamp range, - // in member declaration order. The validate_() clamp loop relies on this - // layout to index into LightCall/LightColorValues via bit-position arithmetic - // without a per-field offset table. Do not reorder without updating the + // Each flag indicates whether the corresponding value has been set. Bits 0-7 + // are the [0.0, 1.0] clamp fields; validate_() iterates them via bit-position + // arithmetic and asserts the layout — don't reorder without matching the // static_asserts in light_call.cpp. enum FieldFlags : uint16_t { FLAG_HAS_BRIGHTNESS = 1 << 0, @@ -246,13 +243,7 @@ class LightCall { LightState *parent_; // Light state values - use flags_ to check if a value has been set. - // Group 4-byte aligned members first. - // - // The eight [0.0, 1.0]-clamped float fields (brightness_ ... warm_white_) - // are declared in the same order as their flag bits (0-7) and the matching - // fields in LightColorValues. validate_() exploits this to iterate them via - // bit-position arithmetic. color_temperature_ has a custom range and lives - // outside that block. + // brightness_..warm_white_ match FieldFlags bits 0-7 in order (see validate_). uint32_t transition_length_; uint32_t flash_length_; uint32_t effect_; diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index 74af29fe59f..1c41d727165 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -10,24 +10,15 @@ namespace esphome::light { inline static uint8_t to_uint8_scale(float x) { return static_cast(roundf(x * 255.0f)); } -// IEEE 754 bit pattern of 1.0f. Floats in [0.0f, 1.0f] have unsigned bit -// pattern <= this value; negatives have the sign bit set (→ huge unsigned), -// values > 1.0f have a larger exponent, and NaN/Infinity also exceed this. -// Verify the platform actually provides IEEE 754 single-precision floats so -// the bit-pattern tricks below are well-defined. +// Bit pattern of 1.0f. Values in [0.0f, 1.0f] have bits <= this; out-of-range +// values (including negatives, whose sign bit makes their uint32 huge) exceed +// it. Lets a single unsigned compare replace two soft-float calls on ESP8266. static constexpr uint32_t ONE_F_BITS = 0x3F800000u; -// sizeof check + is_iec559 together pin the format to IEEE 754 single-precision, -// which fixes the bit pattern of 1.0f as 0x3F800000. A direct bit-cast check -// would be cleaner but __builtin_bit_cast is not available on the older xtensa -// toolchain used for ESP8266. -static_assert(sizeof(float) == sizeof(uint32_t), "float must be 32-bit for bit-pattern range checks"); -static_assert(std::numeric_limits::is_iec559, "IEEE 754 single-precision float required"); +static_assert(sizeof(float) == sizeof(uint32_t), "float must be 32-bit"); +static_assert(std::numeric_limits::is_iec559, "IEEE 754 float required"); -// Returns true iff `x` is outside [0.0f, 1.0f] via a single unsigned compare on -// its IEEE 754 bit pattern. Uses a union type-pun (GCC/Clang extension) because -// memcpy/bit_cast don't optimize to a no-op on xtensa-gcc (same reasoning as -// api/proto.h's float_to_raw). Replaces two soft-float __ltsf2/__gtsf2 calls -// with one `bltu` on ESP8266 and is free on FPU targets. +// Union type-pun (GCC/Clang extension): memcpy/bit_cast don't fold to a no-op +// on xtensa-gcc. Same reasoning as api/proto.h's float_to_raw(). inline bool float_out_of_unit_range(float x) { union { float f; @@ -37,10 +28,8 @@ inline bool float_out_of_unit_range(float x) { return pun.u > ONE_F_BITS; } -// Clamps `x` to [0.0f, 1.0f] with no floating-point compares. In-range values -// return via a single branch; out-of-range pick 0.0f for negatives (sign bit -// set) and 1.0f otherwise (> 1.0f, NaN, Infinity). Cheaper than std::clamp on -// ESP8266, which expands to two soft-float calls per invocation. +// Clamps to [0.0f, 1.0f] without float compares. Negatives (sign bit set) +// fold to 0.0f; everything else out of range (>1, NaN, Inf) folds to 1.0f. inline float clamp_unit_float(float x) { union { float f; @@ -331,12 +320,8 @@ class LightColorValues { friend class LightCall; protected: - // The eight [0.0, 1.0]-clamped float fields are declared in the same order - // as their flag bits (0-7) in LightCall::FieldFlags and the matching fields - // in LightCall. LightCall::validate_() exploits this layout to iterate and - // copy them via bit-position arithmetic with a constant delta of 12 bytes - // between matching LightCall and LightColorValues members. color_temperature_ - // has a different range and is placed after the clamp block. + // brightness_..warm_white_ match LightCall::FieldFlags bits 0-7 in order. + // LightCall::validate_() relies on this layout via static_asserts. float state_; ///< ON / OFF, float for transition float brightness_; float color_brightness_; From bdd1c413def2a3f164c378252f2570ba898cd342 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 16:38:44 -1000 Subject: [PATCH 06/11] [light] Eliminate static-local guard variables by hoisting names to PROGMEM_STRING_TABLE The PROGMEM_STRING_TABLE expansion is entirely constexpr-initialized, so it doesn't require the per-static thread-safe-init guard variables that the previous `static const LogString *const FIELD_NAMES[8] PROGMEM = { LOG_STR(...) }` form emitted inside validate_() (LOG_STR contains a GCC statement-expression, which is not a constant expression, so GCC fell back to dynamic init with an 8-byte guard). Combined the 8 clamp-field names and "Color temperature" into a single ValidateFieldNames table; index 8 is reserved for CT. log_value_out_of_range_() now takes the resolved `const LogString *` directly, so the helper no longer needs its internal progmem_read_ptr dereference. Impact vs. prior commit (isolated light build): ESP8266 .irom0.text: 267912 -> 267784 (-128 B) ESP8266 .bss guard vars on light symbols: 16 -> 0 (-16 B RAM) ESP32-IDF .flash.text: 136988 -> 136996 (+8 B, marginal) --- esphome/components/light/light_call.cpp | 41 +++++++++++++------------ 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 43fea14e4e6..ef344a9a742 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -12,12 +12,8 @@ namespace esphome::light { static const char *const TAG = "light"; // Cold-path logger. Caller handles the clamp so the in-range hot path avoids -// the call-site spill/reload. `param_name_progmem` is a pointer into FIELD_NAMES -// in flash; the `progmem_read_ptr` is a no-op on non-ESP8266. -static void log_value_out_of_range_(const char *name, float value, const LogString *const *param_name_progmem, - float min, float max) { - const auto *param_name = - reinterpret_cast(progmem_read_ptr(reinterpret_cast(param_name_progmem))); +// the call-site spill/reload around the out-of-line call. +static void log_value_out_of_range_(const char *name, float value, const LogString *param_name, float min, float max) { ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, LOG_STR_ARG(param_name), value, min, max); } @@ -59,6 +55,22 @@ static void log_invalid_parameter(const char *name, const LogString *message) { PROGMEM_STRING_TABLE(ColorModeHumanStrings, "Unknown", "On/Off", "Brightness", "White", "Color temperature", "Cold/warm white", "RGB", "RGBW", "RGB + color temperature", "RGB + cold/warm white"); +// Field names for validate_(). PROGMEM_STRING_TABLE uses constexpr init so no +// per-static guard variables in RAM (a plain LOG_STR array of static locals +// would need them because LOG_STR is a statement-expression on ESP8266). +// Indices 0-7 match FieldFlags bits 0-7; index 8 is color_temperature. +PROGMEM_STRING_TABLE(ValidateFieldNames, + "Brightness", // FLAG_HAS_BRIGHTNESS (bit 0) + "Color brightness", // FLAG_HAS_COLOR_BRIGHTNESS (bit 1) + "Red", // FLAG_HAS_RED (bit 2) + "Green", // FLAG_HAS_GREEN (bit 3) + "Blue", // FLAG_HAS_BLUE (bit 4) + "White", // FLAG_HAS_WHITE (bit 5) + "Cold white", // FLAG_HAS_COLD_WHITE (bit 6) + "Warm white", // FLAG_HAS_WARM_WHITE (bit 7) + "Color temperature"); +static constexpr uint8_t VALIDATE_CT_INDEX = 8; + static const LogString *color_mode_to_human(ColorMode color_mode) { return ColorModeHumanStrings::get_log_str(ColorModeBitPolicy::to_bit(color_mode), 0); } @@ -306,17 +318,6 @@ LightColorValues LightCall::validate_() { ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(7, WARM_WHITE, warm_white_); #undef ESPHOME_LIGHT_ASSERT_CLAMP_FIELD - static const LogString *const FIELD_NAMES[8] PROGMEM = { - LOG_STR("Brightness"), // FLAG_HAS_BRIGHTNESS (bit 0) - LOG_STR("Color brightness"), // FLAG_HAS_COLOR_BRIGHTNESS (bit 1) - LOG_STR("Red"), // FLAG_HAS_RED (bit 2) - LOG_STR("Green"), // FLAG_HAS_GREEN (bit 3) - LOG_STR("Blue"), // FLAG_HAS_BLUE (bit 4) - LOG_STR("White"), // FLAG_HAS_WHITE (bit 5) - LOG_STR("Cold white"), // FLAG_HAS_COLD_WHITE (bit 6) - LOG_STR("Warm white"), // FLAG_HAS_WARM_WHITE (bit 7) - }; - // Iterate only the set bits (ctz + clear-lowest) so the hot path is // O(popcount) — HA can drive perform() at high frequency. float *const src_fields = &this->brightness_; @@ -327,7 +328,7 @@ LightColorValues LightCall::validate_() { active &= active - 1; // clear lowest set bit float &value = src_fields[bit]; if (float_out_of_unit_range(value)) { - log_value_out_of_range_(name, value, &FIELD_NAMES[bit], 0.0f, 1.0f); + log_value_out_of_range_(name, value, ValidateFieldNames::get_log_str(bit, 0), 0.0f, 1.0f); value = clamp_unit_float(value); } dst_fields[bit] = value; @@ -335,11 +336,11 @@ LightColorValues LightCall::validate_() { // color_temperature has a runtime range from traits — no bit-pattern shortcut. if (this->has_color_temperature()) { - static const LogString *const CT_NAME PROGMEM = LOG_STR("Color temperature"); const float ct_min = traits.get_min_mireds(); const float ct_max = traits.get_max_mireds(); if (this->color_temperature_ < ct_min || this->color_temperature_ > ct_max) { - log_value_out_of_range_(name, this->color_temperature_, &CT_NAME, ct_min, ct_max); + log_value_out_of_range_(name, this->color_temperature_, ValidateFieldNames::get_log_str(VALIDATE_CT_INDEX, 0), + ct_min, ct_max); this->color_temperature_ = clamp(this->color_temperature_, ct_min, ct_max); } v.color_temperature_ = this->color_temperature_; From 8ef105767424419184be06795a767df2891b299c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 16:48:00 -1000 Subject: [PATCH 07/11] [light] Treat -0.0f as in range in float_out_of_unit_range -0.0f has bit pattern 0x80000000 which exceeds ONE_F_BITS as unsigned, so the check previously flagged it and emitted a spurious out-of-range warning. Add an explicit compare against NEG_ZERO_F_BITS (declared as a named constexpr for readability) so -0.0f takes the in-range fast path. clamp_unit_float() already returned 0.0f for it via the sign-bit branch, so behavior on clamp is unchanged. Caught by Copilot on PR review. --- esphome/components/light/light_color_values.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index 1c41d727165..7efea56dbbb 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -14,18 +14,23 @@ inline static uint8_t to_uint8_scale(float x) { return static_cast(roun // values (including negatives, whose sign bit makes their uint32 huge) exceed // it. Lets a single unsigned compare replace two soft-float calls on ESP8266. static constexpr uint32_t ONE_F_BITS = 0x3F800000u; +// Bit pattern of -0.0f: sign bit set, magnitude zero. Treated as in range. +static constexpr uint32_t NEG_ZERO_F_BITS = 0x80000000u; static_assert(sizeof(float) == sizeof(uint32_t), "float must be 32-bit"); static_assert(std::numeric_limits::is_iec559, "IEEE 754 float required"); // Union type-pun (GCC/Clang extension): memcpy/bit_cast don't fold to a no-op // on xtensa-gcc. Same reasoning as api/proto.h's float_to_raw(). +// -0.0f (bit pattern 0x80000000) exceeds ONE_F_BITS as unsigned but is +// numerically zero and clamps to 0.0f anyway — treat it as in range so we +// don't log a spurious out-of-range warning. inline bool float_out_of_unit_range(float x) { union { float f; uint32_t u; } pun; pun.f = x; - return pun.u > ONE_F_BITS; + return pun.u > ONE_F_BITS && pun.u != NEG_ZERO_F_BITS; } // Clamps to [0.0f, 1.0f] without float compares. Negatives (sign bit set) @@ -38,7 +43,7 @@ inline float clamp_unit_float(float x) { pun.f = x; if (pun.u <= ONE_F_BITS) return x; - return (pun.u & 0x80000000u) ? 0.0f : 1.0f; + return (pun.u & NEG_ZERO_F_BITS) ? 0.0f : 1.0f; // sign bit → negative → clamp to 0 } /** This class represents the color state for a light object. From 6491072a77d15752b25223f489015ba41e63105e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 16:54:43 -1000 Subject: [PATCH 08/11] [light] Alias clamp fields via anonymous-union float[8] to eliminate pointer UB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the previous `float *p = &this->brightness_; p[bit]` pattern (flagged by Copilot as pointer-into-scalar UB) with a shared anonymous union that exposes brightness_..warm_white_ as unit_fields_[8] in both LightCall and LightColorValues. validate_() now indexes the real array directly — defined behavior — while the named members stay accessible to getters/setters. Code generation is unchanged. The union is declared once via ESPHOME_LIGHT_UNIT_FIELDS_UNION() in light_color_values.h and expanded in both structs. The per-field offsetof static_asserts collapse to a single FieldFlags bit-layout assert since the union guarantees member ↔ array-index alignment. Caught by Copilot on PR review. --- esphome/components/light/light_call.cpp | 62 +++++-------------- esphome/components/light/light_call.h | 15 +---- esphome/components/light/light_color_values.h | 49 ++++++++------- 3 files changed, 44 insertions(+), 82 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index ef344a9a742..7edcd9c82b5 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -11,8 +11,8 @@ namespace esphome::light { static const char *const TAG = "light"; -// Cold-path logger. Caller handles the clamp so the in-range hot path avoids -// the call-site spill/reload around the out-of-line call. +// Cold-path logger; caller handles the clamp so the in-range hot path avoids +// the spill/reload around the call. static void log_value_out_of_range_(const char *name, float value, const LogString *param_name, float min, float max) { ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, LOG_STR_ARG(param_name), value, min, max); } @@ -55,20 +55,10 @@ static void log_invalid_parameter(const char *name, const LogString *message) { PROGMEM_STRING_TABLE(ColorModeHumanStrings, "Unknown", "On/Off", "Brightness", "White", "Color temperature", "Cold/warm white", "RGB", "RGBW", "RGB + color temperature", "RGB + cold/warm white"); -// Field names for validate_(). PROGMEM_STRING_TABLE uses constexpr init so no -// per-static guard variables in RAM (a plain LOG_STR array of static locals -// would need them because LOG_STR is a statement-expression on ESP8266). // Indices 0-7 match FieldFlags bits 0-7; index 8 is color_temperature. -PROGMEM_STRING_TABLE(ValidateFieldNames, - "Brightness", // FLAG_HAS_BRIGHTNESS (bit 0) - "Color brightness", // FLAG_HAS_COLOR_BRIGHTNESS (bit 1) - "Red", // FLAG_HAS_RED (bit 2) - "Green", // FLAG_HAS_GREEN (bit 3) - "Blue", // FLAG_HAS_BLUE (bit 4) - "White", // FLAG_HAS_WHITE (bit 5) - "Cold white", // FLAG_HAS_COLD_WHITE (bit 6) - "Warm white", // FLAG_HAS_WARM_WHITE (bit 7) - "Color temperature"); +// PROGMEM_STRING_TABLE is constexpr-init (no RAM guard variable). +PROGMEM_STRING_TABLE(ValidateFieldNames, "Brightness", "Color brightness", "Red", "Green", "Blue", "White", + "Cold white", "Warm white", "Color temperature"); static constexpr uint8_t VALIDATE_CT_INDEX = 8; static const LogString *color_mode_to_human(ColorMode color_mode) { @@ -291,50 +281,30 @@ LightColorValues LightCall::validate_() { if (this->has_state()) v.set_state(this->state_); - // Clamp the eight [0.0, 1.0] fields and copy them from `this` into `v`. - // Both structs declare the same fields in the same order as FieldFlags bits - // 0-7, with a constant byte-offset delta. The asserts below pin that layout - // so the loop can index by bit position; any single-field reorder trips the - // assert naming the field at fault. + // FieldFlags bits 0-7 must match unit_fields_ array indices; the union in + // both structs guarantees brightness_..warm_white_ alias unit_fields_[0..7]. static_assert(std::is_standard_layout_v, "LightCall must be standard-layout"); static_assert(std::is_standard_layout_v, "LightColorValues must be standard-layout"); + static_assert(FLAG_HAS_BRIGHTNESS == 1u << 0 && FLAG_HAS_COLOR_BRIGHTNESS == 1u << 1 && FLAG_HAS_RED == 1u << 2 && + FLAG_HAS_GREEN == 1u << 3 && FLAG_HAS_BLUE == 1u << 4 && FLAG_HAS_WHITE == 1u << 5 && + FLAG_HAS_COLD_WHITE == 1u << 6 && FLAG_HAS_WARM_WHITE == 1u << 7, + "FieldFlags bits 0-7 must match unit_fields_ indices"); - constexpr size_t SRC_BASE = offsetof(LightCall, brightness_); - constexpr size_t SRC_TO_DST_DELTA = SRC_BASE - offsetof(LightColorValues, brightness_); - -#define ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(bit, flag_suffix, member) \ - static_assert(FLAG_HAS_##flag_suffix == 1u << (bit), "FLAG_HAS_" #flag_suffix " bit position"); \ - static_assert(offsetof(LightCall, member) == SRC_BASE + (bit) * sizeof(float), \ - "LightCall::" #member " must be at bit-indexed slot"); \ - static_assert(offsetof(LightColorValues, member) == SRC_BASE + (bit) * sizeof(float) - SRC_TO_DST_DELTA, \ - "LightColorValues::" #member " must match LightCall delta") - ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(0, BRIGHTNESS, brightness_); - ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(1, COLOR_BRIGHTNESS, color_brightness_); - ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(2, RED, red_); - ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(3, GREEN, green_); - ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(4, BLUE, blue_); - ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(5, WHITE, white_); - ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(6, COLD_WHITE, cold_white_); - ESPHOME_LIGHT_ASSERT_CLAMP_FIELD(7, WARM_WHITE, warm_white_); -#undef ESPHOME_LIGHT_ASSERT_CLAMP_FIELD - - // Iterate only the set bits (ctz + clear-lowest) so the hot path is - // O(popcount) — HA can drive perform() at high frequency. - float *const src_fields = &this->brightness_; - float *const dst_fields = &v.brightness_; + // Iterate set bits only (ctz + clear-lowest) — HA can drive perform() + // at high frequency so the hot path is O(popcount). unsigned active = this->flags_ & CLAMP_FLAGS_MASK; while (active != 0) { unsigned bit = __builtin_ctz(active); active &= active - 1; // clear lowest set bit - float &value = src_fields[bit]; + float &value = this->unit_fields_[bit]; if (float_out_of_unit_range(value)) { log_value_out_of_range_(name, value, ValidateFieldNames::get_log_str(bit, 0), 0.0f, 1.0f); value = clamp_unit_float(value); } - dst_fields[bit] = value; + v.unit_fields_[bit] = value; } - // color_temperature has a runtime range from traits — no bit-pattern shortcut. + // color_temperature: runtime range from traits. if (this->has_color_temperature()) { const float ct_min = traits.get_min_mireds(); const float ct_max = traits.get_max_mireds(); diff --git a/esphome/components/light/light_call.h b/esphome/components/light/light_call.h index 01591143d3b..054f1b1571e 100644 --- a/esphome/components/light/light_call.h +++ b/esphome/components/light/light_call.h @@ -195,10 +195,7 @@ class LightCall { /// Some color modes also can be set using non-native parameters, transform those calls. void transform_parameters_(const LightTraits &traits); - // Each flag indicates whether the corresponding value has been set. Bits 0-7 - // are the [0.0, 1.0] clamp fields; validate_() iterates them via bit-position - // arithmetic and asserts the layout — don't reorder without matching the - // static_asserts in light_call.cpp. + // Bits 0-7 index unit_fields_[] in validate_(); don't reorder (asserts in light_call.cpp). enum FieldFlags : uint16_t { FLAG_HAS_BRIGHTNESS = 1 << 0, FLAG_HAS_COLOR_BRIGHTNESS = 1 << 1, @@ -243,18 +240,10 @@ class LightCall { LightState *parent_; // Light state values - use flags_ to check if a value has been set. - // brightness_..warm_white_ match FieldFlags bits 0-7 in order (see validate_). uint32_t transition_length_; uint32_t flash_length_; uint32_t effect_; - float brightness_; - float color_brightness_; - float red_; - float green_; - float blue_; - float white_; - float cold_white_; - float warm_white_; + ESPHOME_LIGHT_UNIT_FIELDS_UNION(); float color_temperature_; // Smaller members at the end for better packing diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index 7efea56dbbb..43b5e325b8a 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -10,20 +10,16 @@ namespace esphome::light { inline static uint8_t to_uint8_scale(float x) { return static_cast(roundf(x * 255.0f)); } -// Bit pattern of 1.0f. Values in [0.0f, 1.0f] have bits <= this; out-of-range -// values (including negatives, whose sign bit makes their uint32 huge) exceed -// it. Lets a single unsigned compare replace two soft-float calls on ESP8266. -static constexpr uint32_t ONE_F_BITS = 0x3F800000u; -// Bit pattern of -0.0f: sign bit set, magnitude zero. Treated as in range. -static constexpr uint32_t NEG_ZERO_F_BITS = 0x80000000u; +// IEEE 754 bit patterns. Values in [0.0f, 1.0f] have bits <= ONE_F_BITS; +// negatives have the sign bit set (→ huge unsigned). A single unsigned compare +// replaces two soft-float __ltsf2/__gtsf2 calls on ESP8266. +static constexpr uint32_t ONE_F_BITS = 0x3F800000u; // 1.0f +static constexpr uint32_t NEG_ZERO_F_BITS = 0x80000000u; // -0.0f / sign-bit mask static_assert(sizeof(float) == sizeof(uint32_t), "float must be 32-bit"); static_assert(std::numeric_limits::is_iec559, "IEEE 754 float required"); -// Union type-pun (GCC/Clang extension): memcpy/bit_cast don't fold to a no-op -// on xtensa-gcc. Same reasoning as api/proto.h's float_to_raw(). -// -0.0f (bit pattern 0x80000000) exceeds ONE_F_BITS as unsigned but is -// numerically zero and clamps to 0.0f anyway — treat it as in range so we -// don't log a spurious out-of-range warning. +// Union pun — memcpy/bit_cast don't fold on xtensa-gcc (see api/proto.h). +// -0.0f counts as in range (clamps to 0.0f anyway; don't log a false warning). inline bool float_out_of_unit_range(float x) { union { float f; @@ -33,8 +29,7 @@ inline bool float_out_of_unit_range(float x) { return pun.u > ONE_F_BITS && pun.u != NEG_ZERO_F_BITS; } -// Clamps to [0.0f, 1.0f] without float compares. Negatives (sign bit set) -// fold to 0.0f; everything else out of range (>1, NaN, Inf) folds to 1.0f. +// Clamps to [0.0f, 1.0f] without float compares. Negatives → 0; >1/NaN/Inf → 1. inline float clamp_unit_float(float x) { union { float f; @@ -46,6 +41,23 @@ inline float clamp_unit_float(float x) { return (pun.u & NEG_ZERO_F_BITS) ? 0.0f : 1.0f; // sign bit → negative → clamp to 0 } +// Shared anonymous union: eight unit-range floats alias unit_fields_[8] so +// LightCall::validate_() can iterate them as a real array. GCC/Clang ext. +#define ESPHOME_LIGHT_UNIT_FIELDS_UNION() \ + union { \ + struct { \ + float brightness_; \ + float color_brightness_; \ + float red_; \ + float green_; \ + float blue_; \ + float white_; \ + float cold_white_; \ + float warm_white_; \ + }; \ + float unit_fields_[8]; \ + } + /** This class represents the color state for a light object. * * The representation of the color state is dependent on the active color mode. A color mode consists of multiple @@ -325,17 +337,8 @@ class LightColorValues { friend class LightCall; protected: - // brightness_..warm_white_ match LightCall::FieldFlags bits 0-7 in order. - // LightCall::validate_() relies on this layout via static_asserts. float state_; ///< ON / OFF, float for transition - float brightness_; - float color_brightness_; - float red_; - float green_; - float blue_; - float white_; - float cold_white_; - float warm_white_; + ESPHOME_LIGHT_UNIT_FIELDS_UNION(); float color_temperature_; ///< Color Temperature in Mired ColorMode color_mode_; }; From 58885c2090cd0e7c337dfc49d79534067bff689c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 16:55:25 -1000 Subject: [PATCH 09/11] [light] Rename log_value_out_of_range_ to drop trailing underscore (clang-tidy) --- esphome/components/light/light_call.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 7edcd9c82b5..c9a5af52773 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -13,7 +13,7 @@ static const char *const TAG = "light"; // Cold-path logger; caller handles the clamp so the in-range hot path avoids // the spill/reload around the call. -static void log_value_out_of_range_(const char *name, float value, const LogString *param_name, float min, float max) { +static void log_value_out_of_range(const char *name, float value, const LogString *param_name, float min, float max) { ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, LOG_STR_ARG(param_name), value, min, max); } @@ -298,7 +298,7 @@ LightColorValues LightCall::validate_() { active &= active - 1; // clear lowest set bit float &value = this->unit_fields_[bit]; if (float_out_of_unit_range(value)) { - log_value_out_of_range_(name, value, ValidateFieldNames::get_log_str(bit, 0), 0.0f, 1.0f); + log_value_out_of_range(name, value, ValidateFieldNames::get_log_str(bit, 0), 0.0f, 1.0f); value = clamp_unit_float(value); } v.unit_fields_[bit] = value; @@ -309,8 +309,8 @@ LightColorValues LightCall::validate_() { const float ct_min = traits.get_min_mireds(); const float ct_max = traits.get_max_mireds(); if (this->color_temperature_ < ct_min || this->color_temperature_ > ct_max) { - log_value_out_of_range_(name, this->color_temperature_, ValidateFieldNames::get_log_str(VALIDATE_CT_INDEX, 0), - ct_min, ct_max); + log_value_out_of_range(name, this->color_temperature_, ValidateFieldNames::get_log_str(VALIDATE_CT_INDEX, 0), + ct_min, ct_max); this->color_temperature_ = clamp(this->color_temperature_, ct_min, ct_max); } v.color_temperature_ = this->color_temperature_; From 674fb1d3f6c2ded7f88fc2ed72d4556c58baff92 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 16:57:58 -1000 Subject: [PATCH 10/11] [light] Drop redundant is_standard_layout asserts (offsetof no longer used) Since validate_() now accesses unit_fields_[] directly via the union alias, there's no offsetof arithmetic to guard. The FieldFlags bit layout assert is all that's needed. --- esphome/components/light/light_call.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index c9a5af52773..7b28065e4ea 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -1,5 +1,4 @@ #include -#include #include "light_call.h" #include "light_state.h" @@ -281,10 +280,7 @@ LightColorValues LightCall::validate_() { if (this->has_state()) v.set_state(this->state_); - // FieldFlags bits 0-7 must match unit_fields_ array indices; the union in - // both structs guarantees brightness_..warm_white_ alias unit_fields_[0..7]. - static_assert(std::is_standard_layout_v, "LightCall must be standard-layout"); - static_assert(std::is_standard_layout_v, "LightColorValues must be standard-layout"); + // FieldFlags bits 0-7 must match unit_fields_ array indices. static_assert(FLAG_HAS_BRIGHTNESS == 1u << 0 && FLAG_HAS_COLOR_BRIGHTNESS == 1u << 1 && FLAG_HAS_RED == 1u << 2 && FLAG_HAS_GREEN == 1u << 3 && FLAG_HAS_BLUE == 1u << 4 && FLAG_HAS_WHITE == 1u << 5 && FLAG_HAS_COLD_WHITE == 1u << 6 && FLAG_HAS_WARM_WHITE == 1u << 7, From c9b6253d248998c5e83daf7abd20b5f892eb25a1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Apr 2026 17:01:11 -1000 Subject: [PATCH 11/11] [light] Fix stale comments on float_out_of_unit_range / clamp_unit_float --- esphome/components/light/light_color_values.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index 43b5e325b8a..5cafa9fe827 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -19,7 +19,7 @@ static_assert(sizeof(float) == sizeof(uint32_t), "float must be 32-bit"); static_assert(std::numeric_limits::is_iec559, "IEEE 754 float required"); // Union pun — memcpy/bit_cast don't fold on xtensa-gcc (see api/proto.h). -// -0.0f counts as in range (clamps to 0.0f anyway; don't log a false warning). +// -0.0f is numerically zero so it's reported in range (no warning, no clamp). inline bool float_out_of_unit_range(float x) { union { float f; @@ -29,7 +29,8 @@ inline bool float_out_of_unit_range(float x) { return pun.u > ONE_F_BITS && pun.u != NEG_ZERO_F_BITS; } -// Clamps to [0.0f, 1.0f] without float compares. Negatives → 0; >1/NaN/Inf → 1. +// Clamps to [0.0f, 1.0f] without float compares. Out of range: sign bit set +// (negatives, -NaN, -Inf) → 0.0f; sign bit clear (>1, +NaN, +Inf) → 1.0f. inline float clamp_unit_float(float x) { union { float f;