Merge remote-tracking branch 'origin/apply-publish-on-success' into bq25186-switch-tests

esphome/components/bq25186/bq25186_switch.h

@@ -0,0 +1,21 @@
+#pragma once
+
+#include "esphome/core/defines.h"
+#ifdef USE_BQ25186_PG_GPO
+
+#include "bq25186.h"
+#include "esphome/components/switch/switch.h"
+
+namespace esphome::bq25186 {
+
+class BQ25186PgGpoSwitch final : public switch_::Switch, public Parented<BQ25186Component> {
+ protected:
+  void write_state(bool state) override {
+    if (this->parent_->write_pg_gpo_level(state))
+      this->publish_state(state);
+  }
+};
+
+}  // namespace esphome::bq25186
+
+#endif  // USE_BQ25186_PG_GPO

esphome/components/bq25186/switch.py

@@ -3,16 +3,18 @@ from esphome.components import switch
 import esphome.config_validation as cv
 from esphome.types import ConfigType
 
-from . import BQ25186_CHILD_SCHEMA, CONF_BQ25186_ID
+from . import BQ25186_CHILD_SCHEMA, CONF_BQ25186_ID, bq25186_ns
 
 DEPENDENCIES = ["bq25186"]
 
 CONF_PG_GPO = "pg_gpo"
 
+BQ25186PgGpoSwitch = bq25186_ns.class_("BQ25186PgGpoSwitch", switch.Switch)
+
 CONFIG_SCHEMA = BQ25186_CHILD_SCHEMA.extend(
     {
         cv.Required(CONF_PG_GPO): switch.switch_schema(
-            switch.ApplySwitch, default_restore_mode="DISABLED"
+            BQ25186PgGpoSwitch, default_restore_mode="DISABLED"
         ),
     }
 )
@@ -22,9 +24,4 @@ async def to_code(config: ConfigType) -> None:
     cg.add_define("USE_BQ25186_PG_GPO")
     hub = await cg.get_variable(config[CONF_BQ25186_ID])
     switches = switch.sub_switches(config, parent=hub)
-    await switches(
-        CONF_PG_GPO,
-        hub.set_pg_gpo_switch,
-        apply="write_pg_gpo_level(state)",
-        publish="on_success",
-    )
+    await switches(CONF_PG_GPO, hub.set_pg_gpo_switch)

esphome/core/entity_helpers.py

@@ -633,7 +633,14 @@ class SubEntities:
         elif publish == "after":
             body = [call, apply_entity.publish]
         elif publish == "on_success":
-            body = [f"if ({expr}) {{ {apply_entity.publish} }}"]
+            # An esp_err_t or int would convert to bool silently and flip the meaning
+            body = [
+                (
+                    f"static_assert(std::is_same_v<decltype({expr}), bool>, "
+                    '"publish=\\"on_success\\" needs a hub call that returns bool");'
+                ),
+                f"if ({expr}) {{ {apply_entity.publish} }}",
+            ]
         elif publish == "momentary":
             acknowledge, condition = apply_entity.momentary
             body = [acknowledge, f"if ({condition}) {{ {call} }}"]

tests/components/bq25186/test_bq25186_pg_gpo.cpp

@@ -3,7 +3,7 @@
 #include <array>
 
 #include "esphome/components/bq25186/bq25186.h"
-#include "esphome/components/switch/switch.h"
+#include "esphome/components/bq25186/bq25186_switch.h"
 
 namespace esphome::bq25186::testing {
 
@@ -37,13 +37,6 @@ class RegisterBus final : public i2c::I2CBus {
   uint8_t pointer_{0};
 };
 
-// The body code generation emits for apply= with publish="on_success"
-static BQ25186Component *hub_under_test = nullptr;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
-static void apply_pg_gpo(switch_::Switch *entity, bool state) {
-  if (hub_under_test->write_pg_gpo_level(state))
-    entity->publish_state(state);
-}
-
 class BQ25186PgGpoTest : public ::testing::Test {
  protected:
   void SetUp() override {
@@ -51,7 +44,7 @@ class BQ25186PgGpoTest : public ::testing::Test {
     this->bus_.registers[BQ25186_REG_SYS_REG] = SYS_REG_OTHER_BITS;
     this->hub_.set_i2c_bus(&this->bus_);
     this->hub_.set_i2c_address(ADDRESS);
-    hub_under_test = &this->hub_;
+    this->switch_.set_parent(&this->hub_);
     this->switch_.set_restore_mode(switch_::SWITCH_RESTORE_DISABLED);
     this->hub_.set_pg_gpo_switch(&this->switch_);
   }
@@ -61,7 +54,7 @@ class BQ25186PgGpoTest : public ::testing::Test {
 
   RegisterBus bus_;
   BQ25186Component hub_;
-  switch_::ApplySwitch<apply_pg_gpo> switch_;
+  BQ25186PgGpoSwitch switch_;
 };
 
 TEST_F(BQ25186PgGpoTest, TurnOnSetsTheLevelAndPublishes) {

tests/unit_tests/core/test_entity_helpers.py

@@ -1716,6 +1716,8 @@ _SELECT_APPLY = entity_helpers.ApplyEntity(
             "on_success",
             (
                 "const char *option = entity->option_at(index);\n"
+                "static_assert(std::is_same_v<decltype(::hub->set_mode(option)), bool>, "
+                '"publish=\\"on_success\\" needs a hub call that returns bool");\n'
                 "if (::hub->set_mode(option)) { entity->publish_state(index); }"
             ),
         ),
@@ -1898,12 +1900,20 @@ async def test_sub_entities_apply_momentary(
         (
             _HUB,
             "write_level(state)",
-            "if (::hub->write_level(state)) { entity->publish_state(state); }",
+            (
+                "static_assert(std::is_same_v<decltype(::hub->write_level(state)), bool>, "
+                '"publish=\\"on_success\\" needs a hub call that returns bool");\n'
+                "if (::hub->write_level(state)) { entity->publish_state(state); }"
+            ),
         ),
         (
             None,
             "chip::write_level(state)",
-            "if (chip::write_level(state)) { entity->publish_state(state); }",
+            (
+                "static_assert(std::is_same_v<decltype(chip::write_level(state)), bool>, "
+                '"publish=\\"on_success\\" needs a hub call that returns bool");\n'
+                "if (chip::write_level(state)) { entity->publish_state(state); }"
+            ),
         ),
     ],
 )