[light] Keep the effect list in a flash table (#20204)

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
This commit is contained in:
J. Nick Koston
2026-10-09 15:55:09 -10:00
committed by GitHub
co-authored by pre-commit-ci-lite[bot]
parent 6b03d68fac
commit 41cc96bbb8
19 changed files with 207 additions and 80 deletions
+1
View File
@@ -34,6 +34,7 @@ from esphome.cpp_generator import ( # noqa: F401
extern_progmem_array,
get_variable,
get_variable_with_full_id,
is_static_pointer,
is_template,
new_Pvariable,
new_variable,
+1 -1
View File
@@ -760,7 +760,7 @@ message ListEntitiesLightResponse {
bool legacy_supports_color_temperature = 8 [deprecated=true];
float min_mireds = 9;
float max_mireds = 10;
repeated string effects = 11 [(container_pointer_no_template) = "FixedVector<const char *>"];
repeated string effects = 11 [(container_pointer_no_template) = "light::LightEffectNames"];
bool disabled_by_default = 13;
string icon = 14 [(field_ifdef) = "USE_ENTITY_ICON", (max_data_length) = 63];
EntityCategory entity_category = 15;
+1 -10
View File
@@ -644,16 +644,7 @@ uint16_t APIConnection::try_send_light_info(EntityBase *entity, APIConnection *c
msg.min_mireds = traits.get_min_mireds();
msg.max_mireds = traits.get_max_mireds();
}
FixedVector<const char *> effects_list;
if (light->supports_effects()) {
auto &light_effects = light->get_effects();
effects_list.init(light_effects.size() + 1);
effects_list.push_back("None");
for (auto *effect : light_effects) {
// c_str() is safe as effect names are null-terminated strings from codegen
effects_list.push_back(effect->get_name().c_str());
}
}
light::LightEffectNames effects_list(light->get_effects());
msg.effects = &effects_list;
return fill_and_encode_entity_info(light, msg, conn, remaining_size);
}
+4 -4
View File
@@ -688,8 +688,8 @@ uint8_t *ListEntitiesLightResponse::encode_msg(const void *self, uint8_t *__rest
if (uint32_t raw = float_to_raw(msg.max_mireds); raw != 0) [[likely]] {
pos = ProtoEncode::write_tag_and_fixed32(pos PROTO_ENCODE_DEBUG_ARG, 85, raw);
}
for (const char *it : *msg.effects) {
pos = ProtoEncode::encode_string_force(pos PROTO_ENCODE_DEBUG_ARG, 11, it, strlen(it));
for (const auto &it : *msg.effects) {
pos = ProtoEncode::encode_string_force(pos PROTO_ENCODE_DEBUG_ARG, 11, it);
}
pos = ProtoEncode::encode_bool(pos PROTO_ENCODE_DEBUG_ARG, 13, msg.disabled_by_default);
#ifdef USE_ENTITY_ICON
@@ -713,8 +713,8 @@ uint32_t ListEntitiesLightResponse::calc_size_msg(const void *self) {
size += ProtoSize::calc_float(1, msg.min_mireds);
size += ProtoSize::calc_float(1, msg.max_mireds);
if (!msg.effects->empty()) {
for (const char *it : *msg.effects) {
size += ProtoSize::calc_length_force(1, strlen(it));
for (const auto &it : *msg.effects) {
size += ProtoSize::calc_length_force(1, it.size());
}
}
size += ProtoSize::calc_bool(1, msg.disabled_by_default);
+1 -1
View File
@@ -1059,7 +1059,7 @@ class ListEntitiesLightResponse final : public InfoResponseProtoMessage {
const light::ColorModeMask *supported_color_modes{};
float min_mireds{0.0f};
float max_mireds{0.0f};
const FixedVector<const char *> *effects{};
const light::LightEffectNames *effects{};
static uint8_t *encode_msg(const void *self, uint8_t *pos PROTO_ENCODE_DEBUG_PARAM);
uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const {
return encode_msg(this, buffer.get_pos() PROTO_ENCODE_DEBUG_ARG);
@@ -15,6 +15,7 @@
#endif
#ifdef USE_LIGHT
#include "esphome/components/light/light_effect_names.h"
#include "esphome/components/light/light_traits.h"
#endif
+14 -2
View File
@@ -41,7 +41,7 @@ from esphome.core.entity_helpers import (
queue_entity_register,
setup_entity,
)
from esphome.cpp_generator import MockObjClass
from esphome.cpp_generator import MockObj, MockObjClass
import esphome.final_validate as fv
from esphome.types import ConfigType
@@ -71,6 +71,7 @@ from .types import ( # noqa: F401
ChannelColors,
ColorMode,
GammaTable,
LightEffect,
LightOutput,
LightState,
LightStateRTCState,
@@ -519,6 +520,17 @@ def validate_color_temperature_channels(value):
return value
def _effects_table(effects: list[MockObj]) -> MockObj:
"""Emit the flash table of a light's effect pointers; each must be a static address."""
return cg.shared_progmem_array(
"light_effects",
LightEffect.operator("ptr"),
cg.ArrayInitializer(*effects),
share=False,
constexpr=False,
)
@setup_entity("light")
async def setup_light_core_(light_var, config, output_var):
# All 8 legacy restore_mode values, and the restore_state key, are just different
@@ -586,7 +598,7 @@ async def setup_light_core_(light_var, config, output_var):
EFFECTS_REGISTRY, config.get(CONF_EFFECTS, [])
)
if effects:
cg.add(light_var.add_effects(effects))
cg.add(light_var.add_effects(_effects_table(effects), len(effects)))
for conf in config.get(CONF_ON_TURN_ON, []):
trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], light_var)
+1 -1
View File
@@ -147,7 +147,7 @@ void LightCall::perform() {
// EFFECT
StringRef effect_s;
if (this->effect_ == 0u) {
effect_s = StringRef::from_lit("None");
effect_s = EFFECT_NONE_REF;
} else {
effect_s = this->parent_->effects_[this->effect_ - 1]->get_name();
}
+2
View File
@@ -5,6 +5,8 @@
namespace esphome::light {
inline constexpr StringRef EFFECT_NONE_REF = StringRef::from_lit("None");
class LightState;
class LightEffect {
@@ -0,0 +1,44 @@
#pragma once
#include "esphome/core/helpers.h"
#include "esphome/core/string_ref.h"
#include "light_effect.h"
namespace esphome::light {
/// Effect names as the native API lists them: "None" first, then each effect; empty without effects.
/// Reads the light's effect table directly, so listing entities allocates nothing. Holds a reference:
/// it only lives for one synchronous encode, and by value costs flash on every copy.
class LightEffectNames {
public:
explicit LightEffectNames(const ConstVector<LightEffect *> &effects) : effects_(effects) {}
class Iterator {
public:
ESPHOME_ALWAYS_INLINE Iterator(const ConstVector<LightEffect *> &effects, size_t index)
: effects_(effects), index_(index) {}
ESPHOME_ALWAYS_INLINE StringRef operator*() const {
return this->index_ == 0 ? EFFECT_NONE_REF : this->effects_[this->index_ - 1]->get_name();
}
ESPHOME_ALWAYS_INLINE Iterator &operator++() {
++this->index_;
return *this;
}
ESPHOME_ALWAYS_INLINE bool operator!=(const Iterator &other) const { return this->index_ != other.index_; }
protected:
const ConstVector<LightEffect *> &effects_;
size_t index_;
};
ESPHOME_ALWAYS_INLINE Iterator begin() const { return {this->effects_, 0}; }
ESPHOME_ALWAYS_INLINE Iterator end() const {
return {this->effects_, this->effects_.empty() ? 0 : this->effects_.size() + 1};
}
ESPHOME_ALWAYS_INLINE bool empty() const { return this->effects_.empty(); }
protected:
const ConstVector<LightEffect *> &effects_;
};
} // namespace esphome::light
-12
View File
@@ -9,8 +9,6 @@
#include "light_output.h"
#include "transformers.h"
#include <limits>
namespace esphome::light {
ESPHOME_LOG_TAG(TAG, "light");
@@ -196,8 +194,6 @@ void LightState::publish_state() {
LightOutput *LightState::get_output() const { return this->output_; }
static constexpr auto EFFECT_NONE_REF = StringRef::from_lit("None");
StringRef LightState::get_effect_name() {
if (this->active_effect_index_ > 0) {
return this->effects_[this->active_effect_index_ - 1]->get_name();
@@ -218,11 +214,6 @@ void LightState::add_target_state_reached_listener(LightTargetStateReachedListen
this->target_state_reached_listeners_->push_back(listener);
}
void LightState::add_effects(const std::initializer_list<LightEffect *> &effects) {
// Called once from Python codegen during setup with all effects from YAML config
this->effects_ = effects;
}
void LightState::current_values_as_brightness(float *brightness) {
this->current_values.as_brightness(brightness);
*brightness = this->gamma_correct_lut(*brightness);
@@ -352,9 +343,6 @@ float LightState::gamma_uncorrect_lut(float value) const {
#endif // USE_LIGHT_GAMMA_LUT
void LightState::start_effect_(uint32_t effect_index) {
// An external add_effects() can exceed the codegen cap; ignore an index the uint16_t can't hold
if (effect_index > std::numeric_limits<uint16_t>::max())
return;
this->stop_effect_();
if (effect_index == 0)
return;
+7 -4
View File
@@ -217,10 +217,13 @@ class LightState : public EntityBase, public Component {
bool supports_effects() const { return !this->effects_.empty(); }
/// Get all effects for this light state.
const FixedVector<LightEffect *> &get_effects() const { return this->effects_; }
const ConstVector<LightEffect *> &get_effects() const { return this->effects_; }
/// Add effects for this light state.
void add_effects(const std::initializer_list<LightEffect *> &effects);
/// Codegen only: point at the generated table of this light's effects, which must outlive it.
/// Codegen caps the count at 65535, so an effect index always fits the uint16_t active index.
void add_effects(LightEffect *const *effects, size_t count) {
this->effects_ = ConstVector<LightEffect *>(effects, count);
}
/// Get the total number of effects available for this light.
size_t get_effect_count() const { return this->effects_.size(); }
@@ -354,7 +357,7 @@ class LightState : public EntityBase, public Component {
/// The currently active transformer for this light (transition/flash).
std::unique_ptr<LightTransformer> transformer_{nullptr};
/// List of effects for this light.
FixedVector<LightEffect *> effects_;
ConstVector<LightEffect *> effects_;
/// Object used to store the persisted values of the light.
ESPPreferenceObject rtc_;
+8
View File
@@ -4,9 +4,17 @@
#include <string>
#include "gpio.h"
#include "esphome/core/defines.h"
#include "esphome/core/time_64.h"
#include "esphome/core/time_conversion.h"
/// Address tables in ESP8266 flash must be constant-initialized: a runtime initializer would write to flash.
#if defined(USE_ESP8266) && defined(__GNUC__) && !defined(__clang__)
#define ESPHOME_FLASH_CONSTINIT constinit
#else
#define ESPHOME_FLASH_CONSTINIT
#endif
// Per-platform HAL bits (IRAM_ATTR / PROGMEM macros, in_isr_context(),
// inline yield/delay/micros/millis/millis_64 wrappers, ESP8266 progmem
// helpers) live next to each platform component as components/<platform>/hal.h
+41 -8
View File
@@ -10,6 +10,7 @@ from esphome.core import (
ID,
Define,
EnumValue,
EsphomeError,
HexInt,
Lambda,
Library,
@@ -447,13 +448,16 @@ class LineComment(Statement):
class ProgmemAssignmentExpression(AssignmentExpression):
__slots__ = ()
__slots__ = ("constexpr",)
def __init__(self, type_, name, rhs):
def __init__(self, type_, name, rhs, constexpr: bool = True):
super().__init__(type_, "", name, rhs)
self.constexpr = constexpr
def __str__(self):
return f"static constexpr {self.type} {self.name}[] PROGMEM = {self.rhs}"
if self.constexpr:
return f"static constexpr {self.type} {self.name}[] PROGMEM = {self.rhs}"
return f"ESPHOME_FLASH_CONSTINIT static {self.type} const {self.name}[] PROGMEM = {self.rhs}"
class StaticConstAssignmentExpression(AssignmentExpression):
@@ -493,20 +497,34 @@ def extern_progmem_array(
def shared_progmem_array(
name: str, type_: "MockObjClass", rhs: SafeExpType, *, share: bool = True
name: str,
type_: "MockObjClass",
rhs: SafeExpType,
*,
share: bool = True,
constexpr: bool = True,
) -> "MockObj":
"""Emit a global PROGMEM array once per distinct type and contents; later calls reuse it.
The array is ``static constexpr``, so elements must be constant expressions and lambdas
must be captureless. Its name is made unique against every config id and variable.
The array is ``static constexpr`` by default, so elements must be constant expressions and
lambdas must be captureless. Its name is made unique against every config id and variable.
``share=False`` always emits a new array, e.g. for lambdas that may keep static state.
``constexpr=False`` is for tables of generated object pointers: each top level variable
element must pass ``is_static_pointer`` or ``EsphomeError`` is raised.
"""
from esphome.config import iter_ids
from esphome.config_validation import RESERVED_IDS
arrays: dict[str, MockObj] = CORE.data.setdefault("shared_progmem_array", {})
rhs = safe_exp(rhs)
key = f"{type_} {rhs}"
if not constexpr and isinstance(rhs, ArrayInitializer):
for arg in rhs.args:
if isinstance(arg, MockObj) and not is_static_pointer(arg):
raise EsphomeError(
f"'{arg}' must be created with cg.new_Pvariable so its address is known "
"at compile time"
)
key = f"{type_} {rhs} {constexpr}"
if share and (array := arrays.get(key)) is not None:
return array
used = {str(i) for i, _ in iter_ids(CORE.config)}
@@ -514,7 +532,7 @@ def shared_progmem_array(
used |= set(RESERVED_IDS) | CORE.loaded_integrations
id_ = ID(ensure_unique_string(name, used), is_declaration=True, type=type_)
# Global, so any scope can use it; anything a lambda references is already declared.
CORE.add_global(ProgmemAssignmentExpression(type_, id_, rhs))
CORE.add_global(ProgmemAssignmentExpression(type_, id_, rhs, constexpr))
array = MockObj(id_, ".")
CORE.register_variable(id_, array)
if share:
@@ -703,6 +721,7 @@ def Pvariable(id_: ID, rhs: SafeExpType, type_: "MockObj" = None) -> "MockObj":
)
placement_new = CallExpression(f"new({id_.id}) {actual_type}", *call_expr.args)
CORE.add(ExpressionStatement(placement_new))
CORE.data.setdefault(_STATIC_POINTER_IDS, set()).add(id_.id)
else:
decl = VariableDeclarationExpression(id_.type, "*", id_, static=True)
CORE.add_global(decl)
@@ -712,6 +731,20 @@ def Pvariable(id_: ID, rhs: SafeExpType, type_: "MockObj" = None) -> "MockObj":
return obj
_STATIC_POINTER_IDS = "static_pointer_ids"
def is_static_pointer(obj: SafeExpType) -> bool:
"""True if ``obj`` is a Pvariable whose object was placement constructed in static storage.
Its pointer is then an address known at compile time and may appear in a ``constexpr=False``
PROGMEM table; a pointer assigned in ``setup()`` may not.
"""
return isinstance(obj, MockObj) and str(obj.base) in CORE.data.get(
_STATIC_POINTER_IDS, ()
)
def new_Pvariable(id_: ID, *args: SafeExpType) -> "MockObj":
"""Declare a new pointer variable in the code generation by calling it's constructor
with the given arguments.
@@ -3,6 +3,8 @@
#include "esphome/components/api/api_pb2.h"
#include "esphome/components/api/api_buffer.h"
#include "esphome/components/light/color_mode.h"
#include "esphome/components/light/light_effect.h"
#include "esphome/components/light/light_effect_names.h"
namespace esphome::api::benchmarks {
@@ -150,7 +152,17 @@ BENCHMARK(CalcAndEncode_ListEntitiesBinarySensorResponse);
// --- ListEntitiesLightResponse ---
static light::ColorModeMask light_color_modes;
static FixedVector<const char *> light_effects;
class BenchEffect : public light::LightEffect {
public:
explicit BenchEffect(const char *name) : LightEffect(name) {}
void apply() override {}
};
static BenchEffect rainbow_effect("Rainbow");
static BenchEffect strobe_effect("Strobe");
static light::LightEffect *const LIGHT_EFFECT_TABLE[] = {&rainbow_effect, &strobe_effect};
static const ConstVector<light::LightEffect *> light_effect_list(LIGHT_EFFECT_TABLE, 2);
static const light::LightEffectNames light_effects(light_effect_list);
static ListEntitiesLightResponse make_light_response() {
// Initialize static data on first call
@@ -158,10 +170,6 @@ static ListEntitiesLightResponse make_light_response() {
if (!initialized) {
light_color_modes.insert(light::ColorMode::RGB_WHITE);
light_color_modes.insert(light::ColorMode::COLOR_TEMPERATURE);
light_effects.init(3);
light_effects.push_back("None");
light_effects.push_back("Rainbow");
light_effects.push_back("Strobe");
initialized = true;
}
@@ -2,6 +2,14 @@
from collections.abc import Callable
from pathlib import Path
import re
import pytest
import esphome.codegen as cg
from esphome.components import light
from esphome.components.light import types as light_types
from esphome.core import ID, EsphomeError
def test_default_flash_length_and_empty_effects_are_not_emitted(
@@ -16,4 +24,20 @@ def test_default_flash_length_and_empty_effects_are_not_emitted(
assert "bare_light->set_flash_transition_length(" not in main_cpp
assert "bare_light->add_effects(" not in main_cpp
assert "fancy_light->set_flash_transition_length(500);" in main_cpp
assert "fancy_light->add_effects({" in main_cpp
call = re.search(r"fancy_light->add_effects\((\w+), (\d+)\);", main_cpp)
assert call is not None
# The effect pointers form a flash table; they are address constants, not constexpr
assert re.search(
rf"static light::LightEffect \* const {call.group(1)}\[\] PROGMEM = \{{[^}}]+\}};",
main_cpp,
)
def test_effect_assigned_in_setup_is_rejected() -> None:
"""A pointer assigned in setup() would make the flash table need dynamic init."""
dynamic_effect = cg.Pvariable(
ID("dynamic_effect", is_declaration=True, type=light_types.LightEffect),
cg.RawExpression("make_effect()"),
)
with pytest.raises(EsphomeError, match="dynamic_effect"):
light._effects_table([dynamic_effect])
+2 -1
View File
@@ -32,6 +32,7 @@ esphome:
- lambda: |-
const auto &effects = id(test_monochromatic_light).get_effects();
auto &same = id(test_monochromatic_light).get_effects();
const auto copy = id(test_monochromatic_light).get_effects(); // the view is copyable
uint32_t total = effects.size();
for (auto *effect : effects) {
ESP_LOGD("test", "Effect %s", effect->get_name().c_str());
@@ -39,7 +40,7 @@ esphome:
if (total > 0) {
ESP_LOGD("test", "First %s", effects.at(0)->get_name().c_str());
// Raw pointer iterators on purpose: external code binds std::find's result to `const auto *`
auto *it = std::find(same.begin(), same.end(), effects[0]);
auto *it = std::find(same.begin(), same.end(), copy[0]);
ESP_LOGD("test", "Index %d", (int) (it - same.begin()));
}
@@ -1,6 +1,5 @@
#include <gtest/gtest.h>
#include "esphome/components/light/light_effect.h"
#include "esphome/components/light/light_output.h"
#include "esphome/components/light/light_state.h"
@@ -18,36 +17,8 @@ class BrightnessOutput : public LightOutput {
void write_state(LightState *state) override {}
};
class NoopEffect : public LightEffect {
public:
using LightEffect::LightEffect;
void apply() override {}
};
// start_effect_() is where the uint32_t index is narrowed to the stored uint16_t.
class TestableLightState : public LightState {
public:
using LightState::LightState;
using LightState::start_effect_;
};
} // namespace
// add_effects() is public, so an external component can exceed the codegen cap on effect count;
// an index the uint16_t can't hold must be ignored rather than wrap onto another effect.
TEST(LightStateEffect, IndexAboveUint16IsIgnoredAndKeepsTheActiveEffect) {
BrightnessOutput output;
TestableLightState state(&output);
NoopEffect effect("Noop");
state.add_effects({&effect});
state.start_effect_(1);
ASSERT_EQ(state.get_current_effect_index(), 1u);
state.start_effect_(0x10000u); // unchecked narrowing wraps this to 0, which stops the effect
EXPECT_EQ(state.get_current_effect_index(), 1u);
}
// get_gamma_correct() reads the gamma codegen stores after the lookup table, rounded to two decimals.
TEST(LightStateGamma, ReadsTheGammaStoredWithTheTable) {
static constexpr GammaTable TABLE{{}, 280};
+41 -1
View File
@@ -4,7 +4,7 @@ import math
import pytest
from esphome import cpp_generator as cg, cpp_types as ct
from esphome.core import CORE, ID
from esphome.core import CORE, ID, EsphomeError
class TestExpressions:
@@ -829,6 +829,46 @@ class TestSharedProgmemArray:
assert str(a) != str(b)
assert sum("PROGMEM" in str(st) for st in CORE.global_statements) == 2
def test_is_static_pointer_only_for_placement_new(self) -> None:
CORE.config = {}
static = cg.new_Pvariable(ID("static_obj", is_declaration=True, type=ct.uint8))
dynamic = cg.Pvariable(
ID("dynamic_obj", is_declaration=True, type=ct.uint8),
cg.RawExpression("nullptr"),
)
assert cg.is_static_pointer(static)
assert not cg.is_static_pointer(dynamic)
def test_constexpr_and_const_arrays_are_not_shared(self) -> None:
CORE.config = {}
a = cg.shared_progmem_array("table", ct.uint8, [1], constexpr=False)
b = cg.shared_progmem_array("table", ct.uint8, [1])
assert str(a) != str(b)
def test_constexpr_false_rejects_dynamic_pointers(self) -> None:
CORE.config = {}
dynamic = cg.Pvariable(
ID("dynamic_obj", is_declaration=True, type=ct.uint8),
cg.RawExpression("nullptr"),
)
with pytest.raises(EsphomeError, match="dynamic_obj"):
cg.shared_progmem_array(
"table",
ct.uint8.operator("ptr"),
cg.ArrayInitializer(dynamic),
constexpr=False,
)
def test_constexpr_false_emits_a_const_array(self) -> None:
CORE.config = {}
cg.shared_progmem_array("table", ct.uint8, [1], constexpr=False)
(statement,) = (
str(st) for st in CORE.global_statements if "PROGMEM" in str(st)
)
assert statement.startswith(
"ESPHOME_FLASH_CONSTINIT static uint8_t const table[] PROGMEM = {1}"
)
def test_same_contents_different_type_are_separate(self) -> None:
CORE.config = {}
a = cg.shared_progmem_array("table", ct.uint8, [1])