[light] Tighten comments in validate_ clamp loop and LightColorValues helpers

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>, "LightCall must be standard-layout for offsetof arithmetic");
-  static_assert(std::is_standard_layout_v<LightColorValues>,
-                "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>, "LightCall must be standard-layout");
+  static_assert(std::is_standard_layout_v<LightColorValues>, "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();

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_;

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<uint8_t>(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<float>::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<float>::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_;