Merge remote-tracking branch 'origin/dev' into HEAD

# Conflicts:
#	esphome/automation.py

esphome/components/display/__init__.py

@@ -29,6 +29,7 @@ from esphome.const import (
     SCHEDULER_DONT_RUN,
 )
 from esphome.core import CORE, ID, CoroPriority, coroutine_with_priority
+from esphome.cpp_generator import MockObj, TemplateArgsType
 from esphome.final_validate import full_config, get_declaring_config
 from esphome.types import ConfigType
 
@@ -43,7 +44,6 @@ DisplayBuffer = display_ns.class_("DisplayBuffer", Display)
 DisplayPage = display_ns.class_("DisplayPage")
 DisplayPagePtr = DisplayPage.operator("ptr")
 DisplayRef = Display.operator("ref")
-DisplayPageShowAction = display_ns.class_("DisplayPageShowAction", automation.Action)
 
 CONF_ON_PAGE_CHANGE = "on_page_change"
 CONF_SHOW_TEST_CARD = "show_test_card"
@@ -346,7 +346,7 @@ async def add_writer(
 
 @automation.register_action(
     "display.page.show",
-    DisplayPageShowAction,
+    automation.ApplyAction,
     maybe_simple_id(
         {
             cv.Required(CONF_ID): cv.templatable(cv.use_id(DisplayPage)),
@@ -354,16 +354,25 @@ async def add_writer(
     ),
     synchronous=True,
 )
-async def display_page_show_to_code(config, action_id, template_arg, args):
-    var = cg.new_Pvariable(action_id, template_arg)
-    if isinstance(config[CONF_ID], core.Lambda):
-        template_ = await cg.templatable(config[CONF_ID], args, DisplayPagePtr)
-        cg.add(var.set_page(template_))
+async def display_page_show_to_code(
+    config: ConfigType,
+    action_id: ID,
+    template_arg: cg.TemplateArguments,
+    args: TemplateArgsType,
+) -> MockObj:
+    page = config[CONF_ID]
+    if isinstance(page, core.Lambda):
+        # Called with the trigger's by-value args, never reduced, so the body may modify them
+        lamb = await cg.process_lambda(page, args, return_type=DisplayPagePtr)
+        statements = [
+            f"{DisplayPagePtr} esphome__page = {automation.immediate_lambda_call(lamb)};",
+            "if (esphome__page != nullptr) {",
+            "  esphome__page->show();",
+            "}",
+        ]
     else:
-        paren = await cg.get_variable(config[CONF_ID])
-        template_ = await cg.templatable(paren, args, DisplayPagePtr)
-        cg.add(var.set_page(template_))
-    return var
+        statements = [f"{automation.parent_ref(await cg.get_variable(page))}->show();"]
+    return automation.apply_statements_action(action_id, template_arg, args, statements)
 
 
 # The id is the display itself, so it cannot be a lambda; a plain use_id rejects one clearly.

esphome/components/display/display.h

@@ -895,18 +895,6 @@ class DisplayPage final {
 inline void Display::show_next_page() { this->page_->show_next(); }
 inline void Display::show_prev_page() { this->page_->show_prev(); }
 
-template<typename... Ts> class DisplayPageShowAction final : public Action<Ts...> {
- public:
-  TEMPLATABLE_VALUE(DisplayPage *, page)
-
-  void play(const Ts &...x) override {
-    auto *page = this->page_.value(x...);
-    if (page != nullptr) {
-      page->show();
-    }
-  }
-};
-
 const LogString *text_align_to_string(TextAlign textalign);
 
 }  // namespace esphome::display

tests/component_tests/display/test_display_page_show.py

@@ -10,29 +10,48 @@ def main_cpp(generate_main: Callable[[str], str]) -> str:
     return generate_main("tests/component_tests/display/config/page_show.yaml")
 
 
+def _function_body(main_cpp: str, signature: str) -> str:
+    return main_cpp.split(f"{signature} {{\n", 1)[1].split("\n}\n", 1)[0]
+
+
 def test_page_show_constant_page(main_cpp: str) -> None:
-    """A page id is wrapped in a lambda that returns the page."""
-    assert (
-        "new(display_displaypageshowaction_id_2) display::DisplayPageShowAction<>();\n"
-        "display_displaypageshowaction_id_2->set_page([]() -> display::DisplayPage * {\n"
-        "    return page1;\n"
-        "});"
-    ) in main_cpp
+    """A page id is shown directly, with no action object."""
+    assert "::page1->show();" in main_cpp
+    assert "DisplayPageShowAction" not in main_cpp
 
 
 def test_page_show_lambda_page(main_cpp: str) -> None:
-    """A page lambda is stored on the action."""
-    # The user lambda carries #line directives; match the block without them
-    code = "\n".join(
-        line
-        for line in main_cpp.splitlines()
-        if not line.lstrip().startswith(("#line", "// ESPHOME_LINE_DIRECTIVE_RESET"))
+    """A page lambda is called once and its page shown only when it is not null.
+
+    The check is an if block, never an early return, so the actions after it still run.
+    """
+    body = _function_body(main_cpp, "static void esphome__applyaction_id__fn()")
+    assert body.startswith(
+        "  {\n"
+        "  display::DisplayPage * esphome__page = []() -> display::DisplayPage * {\n"
     )
+    assert "    return page2;\n" in body
     assert (
-        "display_displaypageshowaction_id->set_page([]() -> display::DisplayPage * {\n"
-        "    return page2;\n"
+        "  }();\n"
+        "  if (esphome__page != nullptr) {\n"
+        "    esphome__page->show();\n"
+        "  }\n"
+        "  }\n"
+        "  {\n"
+        "  ::page1->show();\n"
+        "  }"
+    ) in body
+    assert "return;" not in body
+
+
+def test_page_show_callback_automation_is_direct(main_cpp: str) -> None:
+    """A button press calls the generated function, with no Automation object."""
+    assert (
+        "show_page_button->add_on_press_callback([]() -> void {\n"
+        "    esphome__applyaction_id__fn();\n"
         "});"
-    ) in code
+    ) in main_cpp
+    assert "Automation<>" not in main_cpp
 
 
 @pytest.mark.parametrize(
@@ -43,9 +62,15 @@ def test_page_show_lambda_page(main_cpp: str) -> None:
     ],
 )
 def test_page_show_lambda_takes_args_by_value(main_cpp: str, body: str) -> None:
-    """The page lambda gets the trigger args by value, so it may modify them."""
+    """The page lambda gets the trigger args by value, so it may modify them.
+
+    It is called rather than reduced, so even a single return modifies only its own copy.
+    """
     lambda_start = main_cpp.index(f"    {body}\n")
     head = main_cpp.rindex("[", 0, lambda_start)
     assert main_cpp[head:lambda_start].startswith(
         "[](int index) -> display::DisplayPage * {\n"
     )
+    assert (
+        f"    {body}\n    // ESPHOME_LINE_DIRECTIVE_RESET\n  }}(index);\n" in main_cpp
+    )