diff --git a/esphome/cpp_generator.py b/esphome/cpp_generator.py index cf90b878e1..c622207dac 100644 --- a/esphome/cpp_generator.py +++ b/esphome/cpp_generator.py @@ -606,33 +606,43 @@ def Pvariable(id_: ID, rhs: SafeExpType, type_: "MockObj" = None) -> "MockObj": if isinstance(rhs, MockObj) and rhs.is_new_expr: # For 'new' allocations, use placement new into static storage # to avoid heap fragmentation on embedded devices. - the_type = id_.type + # + # Storage must be sized and aligned for the actual instantiated class, + # which may be a subclass of id_.type (e.g. `cv.declare_id(BaseClass)` + # combined with `SubClass.new()` — used by ili9xxx, waveshare_epaper, + # etc. to select a model-specific constructor). Using id_.type would + # run the base-class default constructor instead, silently losing any + # subclass initialization. Template args live on the CallExpression + # and are re-emitted below. + call_expr = rhs.base + assert isinstance(call_expr, CallExpression), ( + f"Expected CallExpression for placement new, got {type(call_expr)}" + ) + actual_type = rhs.new_type if rhs.new_type is not None else id_.type + if call_expr.template_args is not None: + actual_type = f"{actual_type}{call_expr.template_args}" + pointer_type = id_.type # Extract component namespace from type for memory analysis attribution - component_ns = _extract_component_ns(str(the_type)) + component_ns = _extract_component_ns(str(actual_type)) storage_name = f"{component_ns}__{id_.id}__pstorage" # Declare aligned byte array for the object storage CORE.add_global( RawStatement( - f"alignas({the_type}) static unsigned char {storage_name}[sizeof({the_type})];" + f"alignas({actual_type}) static unsigned char {storage_name}[sizeof({actual_type})];" ) ) + # Pointer declaration uses id_.type to preserve the declared base-class + # pointer type for downstream callers (polymorphism through base ptr). CORE.add_global( AssignmentExpression( - f"static {the_type}", + f"static {pointer_type}", "*const ", id_, - MockObj(f"reinterpret_cast<{the_type} *>({storage_name})"), + MockObj(f"reinterpret_cast<{pointer_type} *>({storage_name})"), ) ) - # Extract args from the CallExpression and rebuild as placement new. - # Template args are already encoded in the_type (e.g. GlobalsComponent), - # so we only pass the constructor args, not template_args. - call_expr = rhs.base - assert isinstance(call_expr, CallExpression), ( - f"Expected CallExpression for placement new, got {type(call_expr)}" - ) - placement_new = CallExpression(f"new({id_.id}) {the_type}", *call_expr.args) + placement_new = CallExpression(f"new({id_.id}) {actual_type}", *call_expr.args) CORE.add(ExpressionStatement(placement_new)) else: decl = VariableDeclarationExpression(id_.type, "*", id_, static=True) @@ -869,12 +879,16 @@ class MockObj(Expression): Mostly consists of magic methods that allow ESPHome's codegen syntax. """ - __slots__ = ("base", "op", "is_new_expr") + __slots__ = ("base", "op", "is_new_expr", "new_type") - def __init__(self, base, op=".", is_new_expr=False) -> None: + def __init__(self, base, op=".", is_new_expr=False, new_type=None) -> None: self.base = base self.op = op self.is_new_expr = is_new_expr + # For `is_new_expr=True` objects, `new_type` holds the class name being + # constructed (e.g. "ili9xxx::ILI9XXXST7789V"). Needed by Pvariable so + # placement new uses the actual subclass rather than id_.type. + self.new_type = new_type def __getattr__(self, attr: str) -> "MockObj": # prevent python dunder methods being replaced by mock objects @@ -889,7 +903,9 @@ class MockObj(Expression): def __call__(self, *args: SafeExpType) -> "MockObj": call = CallExpression(self.base, *args) - return MockObj(call, self.op, is_new_expr=self.is_new_expr) + return MockObj( + call, self.op, is_new_expr=self.is_new_expr, new_type=self.new_type + ) def __str__(self): return str(self.base) @@ -903,7 +919,7 @@ class MockObj(Expression): @property def new(self) -> "MockObj": - return MockObj(f"new {self.base}", "->", is_new_expr=True) + return MockObj(f"new {self.base}", "->", is_new_expr=True, new_type=self.base) def template(self, *args: SafeExpType) -> "MockObj": """Apply template parameters to this object.""" diff --git a/tests/component_tests/ili9xxx/__init__.py b/tests/component_tests/ili9xxx/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/component_tests/ili9xxx/config/ili9xxx_test.yaml b/tests/component_tests/ili9xxx/config/ili9xxx_test.yaml new file mode 100644 index 0000000000..bc6148b8d8 --- /dev/null +++ b/tests/component_tests/ili9xxx/config/ili9xxx_test.yaml @@ -0,0 +1,20 @@ +esphome: + name: test + +esp32: + board: esp32dev + framework: + type: arduino + +spi: + clk_pin: GPIO18 + mosi_pin: GPIO23 + +display: + - platform: ili9xxx + id: tft_display + model: ST7789V + cs_pin: GPIO5 + dc_pin: GPIO17 + reset_pin: GPIO16 + invert_colors: false diff --git a/tests/component_tests/ili9xxx/test_ili9xxx.py b/tests/component_tests/ili9xxx/test_ili9xxx.py new file mode 100644 index 0000000000..3919eb3823 --- /dev/null +++ b/tests/component_tests/ili9xxx/test_ili9xxx.py @@ -0,0 +1,31 @@ +"""Tests for the ili9xxx component.""" + +from __future__ import annotations + +from collections.abc import Callable +from pathlib import Path + + +def test_ili9xxx_placement_new_uses_model_subclass( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], +) -> None: + """Regression test for ili9xxx picking the right constructor under placement new. + + ili9xxx declares the ID as the base ``ILI9XXXDisplay`` but constructs a + model-specific subclass (e.g. ``ILI9XXXST7789V``) via ``MODELS[...].new()``. + Pvariable must emit placement new for the subclass — otherwise the base + default constructor runs and the panel is left with a null init sequence + and 0x0 dimensions, producing a silent blank screen. + """ + main_cpp = generate_main(component_config_path("ili9xxx_test.yaml")) + + # Storage is sized for the subclass so the full object fits. + assert "sizeof(ili9xxx::ILI9XXXST7789V)" in main_cpp + assert "alignas(ili9xxx::ILI9XXXST7789V)" in main_cpp + # Pointer is declared as the base type for polymorphism. + assert "static ili9xxx::ILI9XXXDisplay *const tft_display" in main_cpp + # Placement new runs the subclass constructor — this is the actual regression fix. + assert "new(tft_display) ili9xxx::ILI9XXXST7789V()" in main_cpp + # Base-class default constructor must NOT be used. + assert "new(tft_display) ili9xxx::ILI9XXXDisplay()" not in main_cpp