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

Pair each click key with its detector class in one table, add the define
and create the detectors in one pass, register every test callback
through one helper, and point the skill at the in-tree example instead
of copying it.

.agents/skills/automations/SKILL.md

@@ -123,10 +123,17 @@ Both live in `esphome/core/helpers.h` and accept `std::function`, lambdas and fo
 their templatized `add()` method. `LazyCallbackManager` is a drop-in replacement for `CallbackManager` that defers allocation until the first
 callback is added. Prefer it for entity-level callbacks where most instances have no subscribers.
 
+A filter that keeps state between calls (timing, edges) still needs no Trigger class: give each automation a
+small object with constant initializers, created from its `CONF_TRIGGER_ID` before the automations are built,
+and let `when` ask it, e.g. `ApplyCall("{}->check(state, {}, {})", ((CONF_TRIGGER_ID, ...), ...))`. An `ID`
+value in an `ApplyCall` is written as `::id`, so a callback parameter cannot hide it. `on_click` in
+`esphome/components/binary_sensor/__init__.py` is the in-tree example; once a second component needs this,
+move the object creation into `CallbackAutomation`.
+
 ## Triggers: Trigger class method
 
-Use `build_automation()` with a `Trigger<Ts...>` subclass only when the forwarder needs **mutable state
-beyond a single `Automation*` pointer** (e.g. edge detection tracking previous state, timing logic).
+Use `build_automation()` with a `Trigger<Ts...>` subclass only when the state must be set up from the
+parent (in the constructor or in `setup()`), or the trigger needs its own timers.
 
 **Python:**
 ```python

esphome/components/binary_sensor/__init__.py

@@ -134,10 +134,8 @@ BinarySensorInitiallyOff = binary_sensor_ns.class_(
 BinarySensorPtr = BinarySensor.operator("ptr")
 
 # Triggers
-ClickTrigger = binary_sensor_ns.class_("ClickTrigger", automation.Trigger.template())
-DoubleClickTrigger = binary_sensor_ns.class_(
-    "DoubleClickTrigger", automation.Trigger.template()
-)
+ClickDetector = binary_sensor_ns.class_("ClickDetector")
+DoubleClickDetector = binary_sensor_ns.class_("DoubleClickDetector")
 MultiClickTrigger = binary_sensor_ns.class_(
     "MultiClickTrigger", automation.Trigger.template(), cg.Component
 )
@@ -450,6 +448,23 @@ def validate_click_timing(value):
     return value
 
 
+def _click_automation_schema(detector: MockObjClass) -> cv.All:
+    return cv.All(
+        automation.validate_automation(
+            {
+                cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(detector),
+                cv.Optional(
+                    CONF_MIN_LENGTH, default="50ms"
+                ): cv.positive_time_period_milliseconds,
+                cv.Optional(
+                    CONF_MAX_LENGTH, default="350ms"
+                ): cv.positive_time_period_milliseconds,
+            }
+        ),
+        validate_click_timing,
+    )
+
+
 def validate_publish_initial_state(value):
     value = cv.boolean(value)
     _LOGGER.warning(
@@ -482,35 +497,9 @@ _BINARY_SENSOR_SCHEMA = (
             ): validate_filters,
             cv.Optional(CONF_ON_PRESS): automation.validate_automation({}),
             cv.Optional(CONF_ON_RELEASE): automation.validate_automation({}),
-            cv.Optional(CONF_ON_CLICK): cv.All(
-                automation.validate_automation(
-                    {
-                        cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(ClickTrigger),
-                        cv.Optional(
-                            CONF_MIN_LENGTH, default="50ms"
-                        ): cv.positive_time_period_milliseconds,
-                        cv.Optional(
-                            CONF_MAX_LENGTH, default="350ms"
-                        ): cv.positive_time_period_milliseconds,
-                    }
-                ),
-                validate_click_timing,
-            ),
-            cv.Optional(CONF_ON_DOUBLE_CLICK): cv.All(
-                automation.validate_automation(
-                    {
-                        cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(
-                            DoubleClickTrigger
-                        ),
-                        cv.Optional(
-                            CONF_MIN_LENGTH, default="50ms"
-                        ): cv.positive_time_period_milliseconds,
-                        cv.Optional(
-                            CONF_MAX_LENGTH, default="350ms"
-                        ): cv.positive_time_period_milliseconds,
-                    }
-                ),
-                validate_click_timing,
+            cv.Optional(CONF_ON_CLICK): _click_automation_schema(ClickDetector),
+            cv.Optional(CONF_ON_DOUBLE_CLICK): _click_automation_schema(
+                DoubleClickDetector
             ),
             cv.Optional(CONF_ON_MULTI_CLICK): automation.validate_automation(
                 {
@@ -564,6 +553,11 @@ def binary_sensor_schema(
     return _BINARY_SENSOR_SCHEMA.extend(schema)
 
 
+_CLICK_DETECTORS = (
+    (CONF_ON_CLICK, ClickDetector),
+    (CONF_ON_DOUBLE_CLICK, DoubleClickDetector),
+)
+
 _CALLBACK_AUTOMATIONS = (
     automation.CallbackAutomation(
         CONF_ON_PRESS,
@@ -586,32 +580,40 @@ _CALLBACK_AUTOMATIONS = (
         "add_full_state_callback",
         [(cg.optional.template(bool), "x_previous"), (cg.optional.template(bool), "x")],
     ),
+    *(
+        automation.CallbackAutomation(
+            conf_key,
+            "add_on_state_callback",
+            params=[(bool, "state")],
+            forward=[],
+            when=automation.ApplyCall(
+                "{}->check(state, {}, {})",
+                (
+                    (CONF_TRIGGER_ID, detector.operator("ptr")),
+                    (CONF_MIN_LENGTH, cg.uint32),
+                    (CONF_MAX_LENGTH, cg.uint32),
+                ),
+            ),
+        )
+        for conf_key, detector in _CLICK_DETECTORS
+    ),
 )
 
 
 @coroutine_with_priority(CoroPriority.AUTOMATION)
 async def _build_binary_sensor_automations(var, config):
+    # The detectors must exist before the automations that name them are built
+    if clicks := [c for key, _ in _CLICK_DETECTORS for c in config.get(key, ())]:
+        cg.add_define("USE_BINARY_SENSOR_CLICK_TRIGGER")
+    for conf in clicks:
+        cg.new_Pvariable(conf[CONF_TRIGGER_ID])
     await automation.build_callback_automations(var, config, _CALLBACK_AUTOMATIONS)
 
-    if config.get(CONF_ON_CLICK) or config.get(CONF_ON_DOUBLE_CLICK):
-        cg.add_define("USE_BINARY_SENSOR_CLICK_TRIGGER")
     if config.get(CONF_ON_MULTI_CLICK):
         cg.add_define("USE_BINARY_SENSOR_MULTI_CLICK_TRIGGER")
     if config.get(CONF_ON_RAW_VALUE):
         cg.add_define("USE_BINARY_SENSOR_RAW_CALLBACK")
 
-    for conf in config.get(CONF_ON_CLICK, []):
-        trigger = cg.new_Pvariable(
-            conf[CONF_TRIGGER_ID], var, conf[CONF_MIN_LENGTH], conf[CONF_MAX_LENGTH]
-        )
-        await automation.build_automation(trigger, [], conf)
-
-    for conf in config.get(CONF_ON_DOUBLE_CLICK, []):
-        trigger = cg.new_Pvariable(
-            conf[CONF_TRIGGER_ID], var, conf[CONF_MIN_LENGTH], conf[CONF_MAX_LENGTH]
-        )
-        await automation.build_automation(trigger, [], conf)
-
     for conf in config.get(CONF_ON_MULTI_CLICK, []):
         timings = [
             cg.StructInitializer(

esphome/components/binary_sensor/automation.cpp

@@ -136,6 +136,28 @@ bool match_interval(uint32_t min_length, uint32_t max_length, uint32_t length) {
     return length >= min_length && length <= max_length;
   }
 }
+
+bool ClickDetector::check(bool state, uint32_t min_length, uint32_t max_length) {
+  if (state) {
+    this->start_time_ = millis();
+    return false;
+  }
+  return match_interval(min_length, max_length, millis() - this->start_time_);
+}
+
+bool DoubleClickDetector::check(bool state, uint32_t min_length, uint32_t max_length) {
+  const uint32_t now = millis();
+  if (state && this->start_time_ != 0 && this->end_time_ != 0 &&
+      match_interval(min_length, max_length, this->end_time_ - this->start_time_) &&
+      match_interval(min_length, max_length, now - this->end_time_)) {
+    this->start_time_ = 0;
+    this->end_time_ = 0;
+    return true;
+  }
+  this->start_time_ = this->end_time_;
+  this->end_time_ = now;
+  return false;
+}
 #endif  // USE_BINARY_SENSOR_CLICK_TRIGGER
 
 }  // namespace esphome::binary_sensor

esphome/components/binary_sensor/automation.h

@@ -24,54 +24,25 @@ static_assert(std::is_same_v<decltype(MultiClickTriggerEvent::state), uint32_t>
 
 bool match_interval(uint32_t min_length, uint32_t max_length, uint32_t length);
 
-class ClickTrigger final : public Trigger<> {
+/// Times the presses of one on_click automation.
+class ClickDetector {
  public:
-  explicit ClickTrigger(BinarySensor *parent, uint32_t min_length, uint32_t max_length)
-      : min_length_(min_length), max_length_(max_length) {
-    parent->add_on_state_callback([this](bool state) {
-      if (state) {
-        this->start_time_ = millis();
-      } else {
-        const uint32_t length = millis() - this->start_time_;
-        if (match_interval(this->min_length_, this->max_length_, length))
-          this->trigger();
-      }
-    });
-  }
+  /// Whether `state` ends a press of min_length to max_length ms; a max_length of 0 means no maximum.
+  bool check(bool state, uint32_t min_length, uint32_t max_length);
 
  protected:
-  uint32_t start_time_{0};  /// The millis() time when the click started.
-  uint32_t min_length_;     /// Minimum length of click. 0 means no minimum.
-  uint32_t max_length_;     /// Maximum length of click. 0 means no maximum.
+  uint32_t start_time_{0};  ///< The millis() time when the press started.
 };
 
-class DoubleClickTrigger final : public Trigger<> {
+/// Times the presses of one on_double_click automation.
+class DoubleClickDetector {
  public:
-  explicit DoubleClickTrigger(BinarySensor *parent, uint32_t min_length, uint32_t max_length)
-      : min_length_(min_length), max_length_(max_length) {
-    parent->add_on_state_callback([this](bool state) {
-      const uint32_t now = millis();
-
-      if (state && this->start_time_ != 0 && this->end_time_ != 0) {
-        if (match_interval(this->min_length_, this->max_length_, this->end_time_ - this->start_time_) &&
-            match_interval(this->min_length_, this->max_length_, now - this->end_time_)) {
-          this->trigger();
-          this->start_time_ = 0;
-          this->end_time_ = 0;
-          return;
-        }
-      }
-
-      this->start_time_ = this->end_time_;
-      this->end_time_ = now;
-    });
-  }
+  /// Whether `state` is the second press, with the first press and the gap both in the window.
+  bool check(bool state, uint32_t min_length, uint32_t max_length);
 
  protected:
-  uint32_t start_time_{0};
-  uint32_t end_time_{0};
-  uint32_t min_length_;  /// Minimum length of click. 0 means no minimum.
-  uint32_t max_length_;  /// Maximum length of click. 0 means no maximum.
+  uint32_t start_time_{0};  ///< The millis() time of the edge before the last one.
+  uint32_t end_time_{0};    ///< The millis() time of the last edge.
 };
 
 class MultiClickTrigger final : public Trigger<>, public Component {

esphome/components/feedback/cover.py

@@ -27,13 +27,13 @@ CONF_ACCELERATION_WAIT_TIME = "acceleration_wait_time"
 CONF_OVERSHOOT_DURATION = "overshoot_duration"
 CONF_OBSTACLE_ROLLBACK = "obstacle_rollback"
 
-feedback_ns = cg.esphome_ns.namespace("feedback")
-FeedbackCover = feedback_ns.class_("FeedbackCover", cover.Cover, cg.Component)
+endstop_ns = cg.esphome_ns.namespace("feedback")
+FeedbackCover = endstop_ns.class_("FeedbackCover", cover.Cover, cg.Component)
 
 
 def validate_infer_endstop(config: ConfigType) -> ConfigType:
-    if config[CONF_INFER_ENDSTOP_FROM_MOVEMENT]:
-        if not config[CONF_HAS_BUILT_IN_ENDSTOP]:
+    if config[CONF_INFER_ENDSTOP_FROM_MOVEMENT] is True:
+        if config[CONF_HAS_BUILT_IN_ENDSTOP] is False:
             raise cv.Invalid(
                 f"{CONF_INFER_ENDSTOP_FROM_MOVEMENT} can only be set if {CONF_HAS_BUILT_IN_ENDSTOP} is also set"
             )
@@ -119,52 +119,58 @@ async def to_code(config: ConfigType) -> None:
     var = await cover.new_cover(config)
     await cg.register_component(var, config)
 
+    # STOP
     await automation.build_automation(
         var.get_stop_trigger(), [], config[CONF_STOP_ACTION]
     )
+
+    # OPEN
     await automation.build_automation(
         var.get_open_trigger(), [], config[CONF_OPEN_ACTION]
     )
+    cg.add(var.set_open_duration(config[CONF_OPEN_DURATION]))
+    await cg.add_variable(var.set_open_endstop, config.get(CONF_OPEN_ENDSTOP))
+    await cg.add_variable(var.set_open_sensor, config.get(CONF_OPEN_SENSOR))
+    await cg.add_variable(
+        var.set_open_obstacle_sensor, config.get(CONF_OPEN_OBSTACLE_SENSOR)
+    )
+
+    # CLOSE
     await automation.build_automation(
         var.get_close_trigger(), [], config[CONF_CLOSE_ACTION]
     )
-    cg.add(var.set_open_duration(config[CONF_OPEN_DURATION]))
     cg.add(var.set_close_duration(config[CONF_CLOSE_DURATION]))
-
-    if CONF_OPEN_ENDSTOP in config or CONF_CLOSE_ENDSTOP in config:
-        cg.add_define("USE_FEEDBACK_COVER_ENDSTOP")
-        await cg.add_variable(var.set_open_endstop, config.get(CONF_OPEN_ENDSTOP))
-        await cg.add_variable(var.set_close_endstop, config.get(CONF_CLOSE_ENDSTOP))
-    # validation requires both movement sensors or neither
-    if CONF_OPEN_SENSOR in config:
-        cg.add_define("USE_FEEDBACK_COVER_MOVEMENT")
-        await cg.add_variable(var.set_open_sensor, config[CONF_OPEN_SENSOR])
-        await cg.add_variable(var.set_close_sensor, config[CONF_CLOSE_SENSOR])
-        cg.add(var.set_infer_endstop(config[CONF_INFER_ENDSTOP_FROM_MOVEMENT]))
-    if CONF_OPEN_OBSTACLE_SENSOR in config or CONF_CLOSE_OBSTACLE_SENSOR in config:
-        cg.add_define("USE_FEEDBACK_COVER_OBSTACLE")
-        await cg.add_variable(
-            var.set_open_obstacle_sensor, config.get(CONF_OPEN_OBSTACLE_SENSOR)
-        )
-        await cg.add_variable(
-            var.set_close_obstacle_sensor, config.get(CONF_CLOSE_OBSTACLE_SENSOR)
-        )
-        cg.add(var.set_obstacle_rollback(config[CONF_OBSTACLE_ROLLBACK]))
-
-    cg.add_optional(var.set_max_duration, config.get(CONF_MAX_DURATION))
-    cg.add(var.set_has_built_in_endstop(config[CONF_HAS_BUILT_IN_ENDSTOP]))
-    assumed_state = config.get(CONF_ASSUMED_STATE)
-    if assumed_state is None:
-        assumed_state = not (
-            (CONF_CLOSE_ENDSTOP in config and CONF_OPEN_ENDSTOP in config)
-            or config[CONF_INFER_ENDSTOP_FROM_MOVEMENT]
-        )
-    cg.add(var.set_assumed_state(assumed_state))
-    cg.add(var.set_update_interval(config[CONF_UPDATE_INTERVAL]))
-    cg.add_optional(
-        var.set_direction_change_waittime, config.get(CONF_DIRECTION_CHANGE_WAIT_TIME)
+    await cg.add_variable(var.set_close_endstop, config.get(CONF_CLOSE_ENDSTOP))
+    await cg.add_variable(var.set_close_sensor, config.get(CONF_CLOSE_SENSOR))
+    await cg.add_variable(
+        var.set_close_obstacle_sensor, config.get(CONF_CLOSE_OBSTACLE_SENSOR)
     )
+
+    # OTHER
+    cg.add_optional(var.set_max_duration, config.get(CONF_MAX_DURATION))
+
+    cg.add(var.set_has_built_in_endstop(config[CONF_HAS_BUILT_IN_ENDSTOP]))
+
+    if CONF_ASSUMED_STATE in config:
+        cg.add(var.set_assumed_state(config[CONF_ASSUMED_STATE]))
+    else:
+        cg.add(
+            var.set_assumed_state(
+                not (
+                    (CONF_CLOSE_ENDSTOP in config and CONF_OPEN_ENDSTOP in config)
+                    or config[CONF_INFER_ENDSTOP_FROM_MOVEMENT]
+                )
+            )
+        )
+
+    cg.add(var.set_update_interval(config[CONF_UPDATE_INTERVAL]))
+    cg.add(var.set_infer_endstop(config[CONF_INFER_ENDSTOP_FROM_MOVEMENT]))
+    if CONF_DIRECTION_CHANGE_WAIT_TIME in config:
+        cg.add(
+            var.set_direction_change_waittime(config[CONF_DIRECTION_CHANGE_WAIT_TIME])
+        )
     cg.add(var.set_acceleration_wait_time(config[CONF_ACCELERATION_WAIT_TIME]))
     if (overshoot := config[CONF_OVERSHOOT_DURATION]).total_milliseconds:
         cg.add_define("USE_FEEDBACK_COVER_OVERSHOOT")
         cg.add(var.set_overshoot_duration(overshoot))
+    cg.add(var.set_obstacle_rollback(config[CONF_OBSTACLE_ROLLBACK]))

esphome/components/feedback/feedback_cover.cpp

@@ -22,15 +22,15 @@ void FeedbackCover::setup() {
   }
   this->current_operation = COVER_OPERATION_IDLE;
 
-#ifdef USE_FEEDBACK_COVER_ENDSTOP
+#ifdef USE_BINARY_SENSOR
+  // if available, get position from endstop sensors
   if (this->open_endstop_ != nullptr && this->open_endstop_->state) {
     this->position = COVER_OPEN;
   } else if (this->close_endstop_ != nullptr && this->close_endstop_->state) {
     this->position = COVER_CLOSED;
   }
-#endif
 
-#ifdef USE_FEEDBACK_COVER_MOVEMENT
+  // if available, get moving state from sensors
   if (this->open_feedback_ != nullptr && this->open_feedback_->state) {
     this->current_operation = COVER_OPERATION_OPENING;
   } else if (this->close_feedback_ != nullptr && this->close_feedback_->state) {
@@ -51,36 +51,30 @@ CoverTraits FeedbackCover::get_traits() {
 }
 
 void FeedbackCover::dump_config() {
-  LOG_COVER("", "Feedback Cover", this);
-  ESP_LOGCONFIG(TAG,
-                "  Open Duration: %.1fs\n"
-                "  Close Duration: %.1fs",
-                this->open_duration_ / 1e3f, this->close_duration_ / 1e3f);
-#ifdef USE_FEEDBACK_COVER_ENDSTOP
+  LOG_COVER("", "Endstop Cover", this);
+  ESP_LOGCONFIG(TAG, "  Open Duration: %.1fs", this->open_duration_ / 1e3f);
+#ifdef USE_BINARY_SENSOR
   LOG_BINARY_SENSOR("  ", "Open Endstop", this->open_endstop_);
-  LOG_BINARY_SENSOR("  ", "Close Endstop", this->close_endstop_);
-#endif
-#ifdef USE_FEEDBACK_COVER_MOVEMENT
   LOG_BINARY_SENSOR("  ", "Open Feedback", this->open_feedback_);
-  LOG_BINARY_SENSOR("  ", "Close Feedback", this->close_feedback_);
-#endif
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
   LOG_BINARY_SENSOR("  ", "Open Obstacle", this->open_obstacle_);
+#endif
+  ESP_LOGCONFIG(TAG, "  Close Duration: %.1fs", this->close_duration_ / 1e3f);
+#ifdef USE_BINARY_SENSOR
+  LOG_BINARY_SENSOR("  ", "Close Endstop", this->close_endstop_);
+  LOG_BINARY_SENSOR("  ", "Close Feedback", this->close_feedback_);
   LOG_BINARY_SENSOR("  ", "Close Obstacle", this->close_obstacle_);
 #endif
   if (this->has_built_in_endstop_) {
     ESP_LOGCONFIG(TAG, "  Has builtin endstop: YES");
   }
-#ifdef USE_FEEDBACK_COVER_MOVEMENT
   if (this->infer_endstop_) {
     ESP_LOGCONFIG(TAG, "  Infer endstop from movement: YES");
   }
-#endif
   if (this->max_duration_ < UINT32_MAX) {
     ESP_LOGCONFIG(TAG, "  Max Duration: %.1fs", this->max_duration_ / 1e3f);
   }
-  if (this->direction_change_waittime_ != UINT32_MAX) {
-    ESP_LOGCONFIG(TAG, "  Direction change wait time: %.1fs", this->direction_change_waittime_ / 1e3f);
+  if (this->direction_change_waittime_.has_value()) {
+    ESP_LOGCONFIG(TAG, "  Direction change wait time: %.1fs", *this->direction_change_waittime_ / 1e3f);
   }
   if (this->acceleration_wait_time_) {
     ESP_LOGCONFIG(TAG, "  Acceleration wait time: %.1fs", this->acceleration_wait_time_ / 1e3f);
@@ -90,51 +84,60 @@ void FeedbackCover::dump_config() {
     ESP_LOGCONFIG(TAG, "  Overshoot duration: %.1fs", this->overshoot_duration_ / 1e3f);
   }
 #endif
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
-  if (this->obstacle_rollback_) {
+#ifdef USE_BINARY_SENSOR
+  if (this->obstacle_rollback_ && (this->open_obstacle_ != nullptr || this->close_obstacle_ != nullptr)) {
     ESP_LOGCONFIG(TAG, "  Obstacle rollback: %.1f%%", this->obstacle_rollback_ * 100);
   }
 #endif
 }
 
-#ifdef USE_FEEDBACK_COVER_MOVEMENT
+#ifdef USE_BINARY_SENSOR
+
 void FeedbackCover::set_open_sensor(binary_sensor::BinarySensor *open_feedback) {
   this->open_feedback_ = open_feedback;
-  open_feedback->add_on_state_callback([this](bool state) { this->on_feedback_(true, state); });
+
+  // setup callbacks to react to sensor changes
+  open_feedback->add_on_state_callback([this](bool state) {
+    ESP_LOGD(TAG, "'%s' - Open feedback '%s'.", LOG_STR_ARG(this->get_log_name()),
+             state ? LOG_STR_LITERAL("STARTED") : LOG_STR_LITERAL("ENDED"));
+    this->recompute_position_();
+    if (!state && this->infer_endstop_ && this->current_trigger_operation_ == COVER_OPERATION_OPENING) {
+      this->endstop_reached_(true);
+    }
+    this->set_current_operation_(state ? COVER_OPERATION_OPENING : COVER_OPERATION_IDLE, false);
+  });
 }
 
 void FeedbackCover::set_close_sensor(binary_sensor::BinarySensor *close_feedback) {
   this->close_feedback_ = close_feedback;
-  close_feedback->add_on_state_callback([this](bool state) { this->on_feedback_(false, state); });
+
+  close_feedback->add_on_state_callback([this](bool state) {
+    ESP_LOGD(TAG, "'%s' - Close feedback '%s'.", LOG_STR_ARG(this->get_log_name()),
+             state ? LOG_STR_LITERAL("STARTED") : LOG_STR_LITERAL("ENDED"));
+    this->recompute_position_();
+    if (!state && this->infer_endstop_ && this->current_trigger_operation_ == COVER_OPERATION_CLOSING) {
+      this->endstop_reached_(false);
+    }
+
+    this->set_current_operation_(state ? COVER_OPERATION_CLOSING : COVER_OPERATION_IDLE, false);
+  });
 }
 
-void FeedbackCover::on_feedback_(bool open, bool state) {
-  const CoverOperation operation = open ? COVER_OPERATION_OPENING : COVER_OPERATION_CLOSING;
-  ESP_LOGD(TAG, "'%s' - %s feedback '%s'.", LOG_STR_ARG(this->get_log_name()),
-           open ? LOG_STR_LITERAL("Open") : LOG_STR_LITERAL("Close"),
-           state ? LOG_STR_LITERAL("STARTED") : LOG_STR_LITERAL("ENDED"));
-  this->recompute_position_();
-  if (!state && this->infer_endstop_ && this->current_trigger_operation_ == operation) {
-    this->endstop_reached_(open);
-  }
-  this->set_current_operation_(state ? operation : COVER_OPERATION_IDLE, false);
-}
-#endif
-
-#ifdef USE_FEEDBACK_COVER_ENDSTOP
 void FeedbackCover::set_open_endstop(binary_sensor::BinarySensor *open_endstop) {
   this->open_endstop_ = open_endstop;
   open_endstop->add_on_state_callback([this](bool state) {
-    if (state)
+    if (state) {
       this->endstop_reached_(true);
+    }
   });
 }
 
 void FeedbackCover::set_close_endstop(binary_sensor::BinarySensor *close_endstop) {
   this->close_endstop_ = close_endstop;
   close_endstop->add_on_state_callback([this](bool state) {
-    if (state)
+    if (state) {
       this->endstop_reached_(false);
+    }
   });
 }
 #endif
@@ -144,22 +147,22 @@ void FeedbackCover::endstop_reached_(bool open_endstop) {
 
   this->position = open_endstop ? COVER_OPEN : COVER_CLOSED;
 
-  // Only when moving towards it; the cover may be returning from just past it
+  // only act if endstop activated while moving in the right direction, in case we are coming back
+  // from a position slightly past the endpoint
   if (this->current_trigger_operation_ == (open_endstop ? COVER_OPERATION_OPENING : COVER_OPERATION_CLOSING)) {
     float dur = (now - this->start_dir_time_) / 1e3f;
     ESP_LOGD(TAG, "'%s' - %s endstop reached. Took %.1fs.", LOG_STR_ARG(this->get_log_name()),
              open_endstop ? LOG_STR_LITERAL("Open") : LOG_STR_LITERAL("Close"), dur);
 
+    // if there is no external mechanism, stop the cover
     if (!this->has_built_in_endstop_) {
       this->start_direction_(COVER_OPERATION_IDLE);
     } else {
       this->set_current_operation_(COVER_OPERATION_IDLE, true);
     }
-    // Without movement sensors the stop above already published
-    if (!this->has_movement_sensors_())
-      return;
   }
 
+  // always sync position and publish
   this->publish_state();
   this->last_publish_time_ = now;
 }
@@ -167,54 +170,69 @@ void FeedbackCover::endstop_reached_(bool open_endstop) {
 void FeedbackCover::set_current_operation_(cover::CoverOperation operation, bool is_triggered) {
   if (is_triggered) {
     this->current_trigger_operation_ = operation;
-    // With movement sensors they set the actual operation
-    if (this->has_movement_sensors_())
-      return;
   }
 
-  const uint32_t now = App.get_loop_component_start_time();
-  this->current_operation = operation;
-  if (operation != COVER_OPERATION_IDLE)
-    this->enable_loop();
-  this->start_dir_time_ = this->last_recompute_time_ = now;
-  this->publish_state();
-  this->last_publish_time_ = now;
-}
+  // if it is setting the actual operation (not triggered one) or
+  // if we don't have moving sensor, we operate in optimistic mode, assuming actions take place immediately
+  // thus, triggered operation always sets current operation.
+  // otherwise, current operation comes from sensor, and may differ from requested operation
+  // this might be from delays or complex actions, or because the movement was not trigger by the component
+  // but initiated externally
 
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
-void FeedbackCover::set_open_obstacle_sensor(binary_sensor::BinarySensor *open_obstacle) {
-  this->open_obstacle_ = open_obstacle;
-  open_obstacle->add_on_state_callback([this](bool state) { this->on_obstacle_(true, state); });
+#ifdef USE_BINARY_SENSOR
+  if (!is_triggered || (this->open_feedback_ == nullptr || this->close_feedback_ == nullptr))
+#endif
+  {
+    const uint32_t now = App.get_loop_component_start_time();
+    this->current_operation = operation;
+    this->start_dir_time_ = this->last_recompute_time_ = now;
+    this->publish_state();
+    this->last_publish_time_ = now;
+  }
 }
 
+#ifdef USE_BINARY_SENSOR
 void FeedbackCover::set_close_obstacle_sensor(binary_sensor::BinarySensor *close_obstacle) {
   this->close_obstacle_ = close_obstacle;
-  close_obstacle->add_on_state_callback([this](bool state) { this->on_obstacle_(false, state); });
+
+  close_obstacle->add_on_state_callback([this](bool state) {
+    if (state && (this->current_operation == COVER_OPERATION_CLOSING ||
+                  this->current_trigger_operation_ == COVER_OPERATION_CLOSING)) {
+      ESP_LOGD(TAG, "'%s' - Close obstacle detected.", LOG_STR_ARG(this->get_log_name()));
+      this->start_direction_(COVER_OPERATION_IDLE);
+
+      if (this->obstacle_rollback_) {
+        this->target_position_ = clamp(this->position + this->obstacle_rollback_, COVER_CLOSED, COVER_OPEN);
+        this->start_direction_(COVER_OPERATION_OPENING);
+      }
+    }
+  });
 }
 
-void FeedbackCover::on_obstacle_(bool open, bool state) {
-  const CoverOperation operation = open ? COVER_OPERATION_OPENING : COVER_OPERATION_CLOSING;
-  if (!state || (this->current_operation != operation && this->current_trigger_operation_ != operation))
-    return;
-  ESP_LOGD(TAG, "'%s' - %s obstacle detected.", LOG_STR_ARG(this->get_log_name()),
-           open ? LOG_STR_LITERAL("Open") : LOG_STR_LITERAL("Close"));
-  this->start_direction_(COVER_OPERATION_IDLE);
+void FeedbackCover::set_open_obstacle_sensor(binary_sensor::BinarySensor *open_obstacle) {
+  this->open_obstacle_ = open_obstacle;
 
-  if (this->obstacle_rollback_) {
-    const float rollback = open ? -this->obstacle_rollback_ : this->obstacle_rollback_;
-    this->target_position_ = clamp(this->position + rollback, COVER_CLOSED, COVER_OPEN);
-    this->start_direction_(open ? COVER_OPERATION_CLOSING : COVER_OPERATION_OPENING);
-  }
+  open_obstacle->add_on_state_callback([this](bool state) {
+    if (state && (this->current_operation == COVER_OPERATION_OPENING ||
+                  this->current_trigger_operation_ == COVER_OPERATION_OPENING)) {
+      ESP_LOGD(TAG, "'%s' - Open obstacle detected.", LOG_STR_ARG(this->get_log_name()));
+      this->start_direction_(COVER_OPERATION_IDLE);
+
+      if (this->obstacle_rollback_) {
+        this->target_position_ = clamp(this->position - this->obstacle_rollback_, COVER_CLOSED, COVER_OPEN);
+        this->start_direction_(COVER_OPERATION_CLOSING);
+      }
+    }
+  });
 }
 #endif
 
 void FeedbackCover::loop() {
-  if (this->current_operation == COVER_OPERATION_IDLE) {
-    this->disable_loop();
+  if (this->current_operation == COVER_OPERATION_IDLE)
     return;
-  }
   const uint32_t now = App.get_loop_component_start_time();
 
+  // Recompute position every loop cycle
   this->recompute_position_();
 
   // if we initiated the move, check if we reached position or max time
@@ -246,7 +264,9 @@ void FeedbackCover::loop() {
     }
   }
 
-  // Publish intermediate positions without saving them
+  // update current position at requested interval, regardless of who started the movement
+  // so that we also update UI if there was an external movement
+  // don't save intermediate positions
   if (now - this->last_publish_time_ > this->update_interval_) {
     this->publish_state(false);
     this->last_publish_time_ = now;
@@ -254,9 +274,11 @@ void FeedbackCover::loop() {
 }
 
 void FeedbackCover::control(const CoverCall &call) {
+  // stop action logic
   if (call.get_stop()) {
     this->start_direction_(COVER_OPERATION_IDLE);
   } else if (call.get_toggle().has_value()) {
+    // toggle action logic: OPEN - STOP - CLOSE
     if (this->current_trigger_operation_ != COVER_OPERATION_IDLE) {
       this->start_direction_(COVER_OPERATION_IDLE);
     } else {
@@ -272,15 +294,28 @@ void FeedbackCover::control(const CoverCall &call) {
     auto pos_opt = call.get_position();
     if (!pos_opt.has_value())
       return;
+    // go to position action
     auto pos = *pos_opt;
     if (pos == this->position) {
-      // Resend in case the assumed state is wrong
-      if (this->has_built_in_endstop_ &&
-          ((pos == COVER_OPEN && !this->has_endstop_(true)) || (pos == COVER_CLOSED && !this->has_endstop_(false)))) {
+      // already at target,
+
+      // for covers with built in end stop, if we don't have sensors we should send the command again
+      // to make sure the assumed state is not wrong
+      if (this->has_built_in_endstop_ && ((pos == COVER_OPEN
+#ifdef USE_BINARY_SENSOR
+                                           && this->open_endstop_ == nullptr
+#endif
+                                           && !this->infer_endstop_) ||
+                                          (pos == COVER_CLOSED
+#ifdef USE_BINARY_SENSOR
+                                           && this->close_endstop_ == nullptr
+#endif
+                                           && !this->infer_endstop_))) {
         this->target_position_ = pos;
         this->start_direction_(pos == COVER_CLOSED ? COVER_OPERATION_CLOSING : COVER_OPERATION_OPENING);
       } else if (this->current_operation != COVER_OPERATION_IDLE ||
                  this->current_trigger_operation_ != COVER_OPERATION_IDLE) {
+        // if we are moving, stop
         this->start_direction_(COVER_OPERATION_IDLE);
       }
     } else {
@@ -291,7 +326,7 @@ void FeedbackCover::control(const CoverCall &call) {
 }
 
 void FeedbackCover::stop_prev_trigger_() {
-  if (this->direction_change_waittime_ != UINT32_MAX) {
+  if (this->direction_change_waittime_.has_value()) {
     this->cancel_timeout(DIRECTION_CHANGE_TIMEOUT_ID);
   }
   if (this->prev_command_trigger_ != nullptr) {
@@ -301,6 +336,9 @@ void FeedbackCover::stop_prev_trigger_() {
 }
 
 bool FeedbackCover::is_at_target_() const {
+  // if initiated externally, current operation might be different from
+  // operation that was triggered, thus evaluate position against what was asked
+
   switch (this->current_trigger_operation_) {
     case COVER_OPERATION_OPENING:
       return this->position >= this->target_position_;
@@ -312,10 +350,10 @@ bool FeedbackCover::is_at_target_() const {
       return true;
   }
 }
-
 void FeedbackCover::start_direction_(CoverOperation dir) {
   Trigger<> *trig;
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
+
+#ifdef USE_BINARY_SENSOR
   binary_sensor::BinarySensor *obstacle{nullptr};
 #endif
 
@@ -331,14 +369,14 @@ void FeedbackCover::start_direction_(CoverOperation dir) {
     case COVER_OPERATION_OPENING:
       this->last_operation_ = dir;
       trig = &this->open_trigger_;
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
+#ifdef USE_BINARY_SENSOR
       obstacle = this->open_obstacle_;
 #endif
       break;
     case COVER_OPERATION_CLOSING:
       this->last_operation_ = dir;
       trig = &this->close_trigger_;
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
+#ifdef USE_BINARY_SENSOR
       obstacle = this->close_obstacle_;
 #endif
       break;
@@ -348,8 +386,9 @@ void FeedbackCover::start_direction_(CoverOperation dir) {
 
   this->stop_prev_trigger_();
 
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
-  // An obstacle already present aborts the start
+#ifdef USE_BINARY_SENSOR
+  // check if there is an obstacle to start the new operation -> abort without any change
+  // the case when an obstacle appears while moving is handled in the callback
   if (obstacle != nullptr && obstacle->state) {
     ESP_LOGD(TAG, "'%s' - %s obstacle detected. Action not started.", LOG_STR_ARG(this->get_log_name()),
              dir == COVER_OPERATION_OPENING ? LOG_STR_LITERAL("Open") : LOG_STR_LITERAL("Close"));
@@ -357,12 +396,14 @@ void FeedbackCover::start_direction_(CoverOperation dir) {
   }
 #endif
 
-  if (this->direction_change_waittime_ != UINT32_MAX && dir != COVER_OPERATION_IDLE &&
+  // if we are moving and need to move in the opposite direction
+  // check if we have a wait time
+  if (this->direction_change_waittime_.has_value() && dir != COVER_OPERATION_IDLE &&
       this->current_operation != COVER_OPERATION_IDLE && dir != this->current_operation) {
+    const uint32_t waittime = *this->direction_change_waittime_;
     ESP_LOGD(TAG, "'%s' - Reversing direction.", LOG_STR_ARG(this->get_log_name()));
     this->start_direction_(COVER_OPERATION_IDLE);
-    this->set_timeout(DIRECTION_CHANGE_TIMEOUT_ID, this->direction_change_waittime_,
-                      [this, dir]() { this->start_direction_(dir); });
+    this->set_timeout(DIRECTION_CHANGE_TIMEOUT_ID, waittime, [this, dir]() { this->start_direction_(dir); });
   } else {
     this->set_current_operation_(dir, true);
     this->prev_command_trigger_ = trig;
@@ -384,24 +425,42 @@ void FeedbackCover::recompute_position_() {
   float min_pos;
   float max_pos;
 
-  // With an endstop the estimate stops short of the end; the endstop callback sets it
+  // endstop sensors update position from their callbacks, and sets the fully open/close value
+  // If we have endstop, estimation never reaches the fully open/closed state.
+  // but if movement continues past corresponding endstop (inertia), keep the fully open/close state
+
   switch (this->current_operation) {
     case COVER_OPERATION_OPENING:
       dir = 1.0f;
       action_dur = this->open_duration_;
       min_pos = COVER_CLOSED;
-      max_pos = this->has_endstop_(true) && this->position < COVER_OPEN ? 0.99f : COVER_OPEN;
+      max_pos = (
+#ifdef USE_BINARY_SENSOR
+                    this->open_endstop_ != nullptr ||
+#endif
+                    this->infer_endstop_) &&
+                        this->position < COVER_OPEN
+                    ? 0.99f
+                    : COVER_OPEN;
       break;
     case COVER_OPERATION_CLOSING:
       dir = -1.0f;
       action_dur = this->close_duration_;
-      min_pos = this->has_endstop_(false) && this->position > COVER_CLOSED ? 0.01f : COVER_CLOSED;
+      min_pos = (
+#ifdef USE_BINARY_SENSOR
+                    this->close_endstop_ != nullptr ||
+#endif
+                    this->infer_endstop_) &&
+                        this->position > COVER_CLOSED
+                    ? 0.01f
+                    : COVER_CLOSED;
       max_pos = COVER_OPEN;
       break;
     default:
       return;
   }
 
+  // check if we have an acceleration_wait_time, and remove from position computation
   if (now - this->start_dir_time_ > this->acceleration_wait_time_) {
     uint32_t accel_end_time = this->start_dir_time_ + this->acceleration_wait_time_;
     uint32_t effective_start;

esphome/components/feedback/feedback_cover.h

@@ -20,19 +20,13 @@ class FeedbackCover final : public cover::Cover, public Component {
   Trigger<> *get_close_trigger() { return &this->close_trigger_; }
   Trigger<> *get_stop_trigger() { return &this->stop_trigger_; }
 
-#ifdef USE_FEEDBACK_COVER_ENDSTOP
+#ifdef USE_BINARY_SENSOR
   void set_open_endstop(binary_sensor::BinarySensor *open_endstop);
-  void set_close_endstop(binary_sensor::BinarySensor *close_endstop);
-#endif
-#ifdef USE_FEEDBACK_COVER_MOVEMENT
   void set_open_sensor(binary_sensor::BinarySensor *open_feedback);
-  void set_close_sensor(binary_sensor::BinarySensor *close_feedback);
-  void set_infer_endstop(bool infer_endstop) { this->infer_endstop_ = infer_endstop; }
-#endif
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
   void set_open_obstacle_sensor(binary_sensor::BinarySensor *open_obstacle);
+  void set_close_endstop(binary_sensor::BinarySensor *close_endstop);
+  void set_close_sensor(binary_sensor::BinarySensor *close_feedback);
   void set_close_obstacle_sensor(binary_sensor::BinarySensor *close_obstacle);
-  void set_obstacle_rollback(float obstacle_rollback) { this->obstacle_rollback_ = obstacle_rollback; }
 #endif
   void set_open_duration(uint32_t duration) { this->open_duration_ = duration; }
   void set_close_duration(uint32_t duration) { this->close_duration_ = duration; }
@@ -42,7 +36,9 @@ class FeedbackCover final : public cover::Cover, public Component {
 #ifdef USE_FEEDBACK_COVER_OVERSHOOT
   void set_overshoot_duration(uint32_t overshoot_duration) { this->overshoot_duration_ = overshoot_duration; }
 #endif
+  void set_obstacle_rollback(float obstacle_rollback) { this->obstacle_rollback_ = obstacle_rollback; }
   void set_update_interval(uint32_t interval) { this->update_interval_ = interval; }
+  void set_infer_endstop(bool infer_endstop) { this->infer_endstop_ = infer_endstop; }
   void set_direction_change_waittime(uint32_t waittime) { this->direction_change_waittime_ = waittime; }
   void set_acceleration_wait_time(uint32_t waittime) { this->acceleration_wait_time_ = waittime; }
 
@@ -57,43 +53,15 @@ class FeedbackCover final : public cover::Cover, public Component {
   void endstop_reached_(bool open_endstop);
   void recompute_position_();
   void set_current_operation_(cover::CoverOperation operation, bool is_triggered);
-#ifdef USE_FEEDBACK_COVER_MOVEMENT
-  void on_feedback_(bool open, bool state);
-#endif
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
-  void on_obstacle_(bool open, bool state);
-#endif
 
-  bool has_endstop_(bool open) const {
-#ifdef USE_FEEDBACK_COVER_ENDSTOP
-    if ((open ? this->open_endstop_ : this->close_endstop_) != nullptr)
-      return true;
-#endif
-#ifdef USE_FEEDBACK_COVER_MOVEMENT
-    return this->infer_endstop_;
-#else
-    return false;
-#endif
-  }
-  bool has_movement_sensors_() const {
-#ifdef USE_FEEDBACK_COVER_MOVEMENT
-    return this->open_feedback_ != nullptr && this->close_feedback_ != nullptr;
-#else
-    return false;
-#endif
-  }
-
-#ifdef USE_FEEDBACK_COVER_ENDSTOP
+#ifdef USE_BINARY_SENSOR
   binary_sensor::BinarySensor *open_endstop_{nullptr};
   binary_sensor::BinarySensor *close_endstop_{nullptr};
-#endif
-#ifdef USE_FEEDBACK_COVER_MOVEMENT
   binary_sensor::BinarySensor *open_feedback_{nullptr};
   binary_sensor::BinarySensor *close_feedback_{nullptr};
-#endif
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
   binary_sensor::BinarySensor *open_obstacle_{nullptr};
   binary_sensor::BinarySensor *close_obstacle_{nullptr};
+
 #endif
   Trigger<> open_trigger_;
   Trigger<> close_trigger_;
@@ -105,13 +73,12 @@ class FeedbackCover final : public cover::Cover, public Component {
 #ifdef USE_FEEDBACK_COVER_OVERSHOOT
   uint32_t overshoot_duration_{0};
 #endif
-  uint32_t direction_change_waittime_{UINT32_MAX};
+  optional<uint32_t> direction_change_waittime_{};
   uint32_t acceleration_wait_time_{0};
   bool has_built_in_endstop_{false};
   bool assumed_state_{false};
-#ifdef USE_FEEDBACK_COVER_MOVEMENT
   bool infer_endstop_{false};
-#endif
+  float obstacle_rollback_{0};
 
   cover::CoverOperation last_operation_{cover::COVER_OPERATION_OPENING};
   cover::CoverOperation current_trigger_operation_{cover::COVER_OPERATION_IDLE};
@@ -123,9 +90,6 @@ class FeedbackCover final : public cover::Cover, public Component {
   uint32_t start_overshoot_time_{0};
 #endif
   float target_position_{0};
-#ifdef USE_FEEDBACK_COVER_OBSTACLE
-  float obstacle_rollback_{0};
-#endif
   uint32_t update_interval_{1000};
 };
 

esphome/core/defines.h

@@ -109,9 +109,6 @@
 #define USE_EZSP_PROXY_TAP
 #define USE_FAN
 #define USE_FONT
-#define USE_FEEDBACK_COVER_ENDSTOP
-#define USE_FEEDBACK_COVER_MOVEMENT
-#define USE_FEEDBACK_COVER_OBSTACLE
 #define USE_FEEDBACK_COVER_OVERSHOOT
 #define USE_GPIO_BINARY_SENSOR_INTERRUPT
 #define USE_GPIO_SWITCH_INTERLOCK

tests/component_tests/binary_sensor/test_click.py

@@ -8,18 +8,24 @@ from esphome.core import CORE
 
 
 def test_click_triggers(generate_main: Callable[[str | Path], str]) -> None:
-    """Each trigger gets its timing in milliseconds, defaults 50 and 350, and its own automation."""
+    """Each trigger gets a detector and a callback with its timing in milliseconds, defaults 50 and 350."""
     main_cpp = generate_main("tests/component_tests/binary_sensor/test_click.yaml")
 
-    triggers = re.findall(
-        r"new\((\w+)\) binary_sensor::(Click|DoubleClick)Trigger\(button_a, (\d+), (\d+)\);",
-        main_cpp,
+    kinds = dict(
+        re.findall(
+            r"new\((\w+)\) binary_sensor::(Click|DoubleClick)Detector\(\);", main_cpp
+        )
     )
-    assert [trigger[1:] for trigger in triggers] == [
+    calls = re.findall(r"if \(!\(::(\w+)->check\(state, (\d+), (\d+)\)\)\)", main_cpp)
+    assert [(kinds[detector_id], *timing) for detector_id, *timing in calls] == [
         ("Click", "50", "350"),
         ("Click", "1000", "3000"),
         ("DoubleClick", "20", "400"),
     ]
-    for trigger_id, *_ in triggers:
-        assert f"Automation<>({trigger_id});" in main_cpp
+    assert main_cpp.count("button_a->add_on_state_callback([](") == 3
+    # Only the automation with a delay keeps an Automation object
+    assert re.findall(r"new\((\w+)\) Automation<>\(\);", main_cpp) == [
+        "automation_id_2"
+    ]
+    assert "::automation_id_2->trigger();" in main_cpp
     assert "USE_BINARY_SENSOR_CLICK_TRIGGER" in {define.name for define in CORE.defines}

tests/component_tests/binary_sensor/test_click.yaml

@@ -21,7 +21,9 @@ binary_sensor:
           - delay: 1s
           - logger.log: a long click done
     on_double_click:
-      - min_length: 20ms
+      # Named like the callback parameter, which must not hide it
+      - trigger_id: state
+        min_length: 20ms
         max_length: 400ms
         then:
           - logger.log: a double click

tests/components/binary_sensor/click_trigger_test.cpp

@@ -1,16 +1,15 @@
 #include <gtest/gtest.h>
-#include <memory>
+#include <functional>
+#include <utility>
 
 #include "esphome/components/binary_sensor/automation.h"
 #include "esphome/components/binary_sensor/binary_sensor.h"
 #include "esphome/components/binary_sensor/filter.h"
-#include "esphome/core/automation.h"
-#include "esphome/core/base_automation.h"
 #include "esphome/core/hal.h"
 
 namespace esphome::binary_sensor::testing {
 
-// The triggers read the real millis(), so every wait is at least 80 ms away from a window edge
+// The click detectors read the real millis(), so every wait is at least 80 ms away from a window edge
 static constexpr uint32_t MIN_MS = 100;
 static constexpr uint32_t MAX_MS = 300;
 static constexpr uint32_t SHORT_MS = 20;
@@ -24,18 +23,24 @@ class ClickTriggerTest : public ::testing::Test {
   void SetUp() override {
     fired = 0;
     this->sensor_.publish_state(false);
-    this->automation_.add_action(&this->action_);
   }
 
   // As codegen builds on_click and on_double_click
   void add_click_(uint32_t min_length = MIN_MS, uint32_t max_length = MAX_MS, BinarySensor *sensor = nullptr) {
-    this->click_ = std::make_unique<ClickTrigger>(sensor != nullptr ? sensor : &this->sensor_, min_length, max_length);
-    this->click_->set_automation_parent(&this->automation_);
+    this->watch_(this->click_, min_length, max_length, sensor != nullptr ? *sensor : this->sensor_);
   }
-  void add_double_click_() {
-    this->double_click_ = std::make_unique<DoubleClickTrigger>(&this->sensor_, MIN_MS, MAX_MS);
-    this->double_click_->set_automation_parent(&this->automation_);
+  void add_double_click_(std::function<void()> on_fire = count_fire) {
+    this->watch_(this->double_click_, MIN_MS, MAX_MS, this->sensor_, std::move(on_fire));
   }
+  template<typename D>
+  void watch_(D &detector, uint32_t min_length, uint32_t max_length, BinarySensor &sensor,
+              std::function<void()> on_fire = count_fire) {
+    sensor.add_on_state_callback([&detector, min_length, max_length, on_fire = std::move(on_fire)](bool state) {
+      if (detector.check(state, min_length, max_length))
+        on_fire();
+    });
+  }
+  static void count_fire() { fired++; }
 
   void press_for_(uint32_t length) {
     this->sensor_.publish_state(true);
@@ -44,10 +49,8 @@ class ClickTriggerTest : public ::testing::Test {
   }
 
   BinarySensor sensor_;
-  StatelessLambdaAction<> action_{[]() { fired++; }};
-  Automation<> automation_;
-  std::unique_ptr<ClickTrigger> click_;
-  std::unique_ptr<DoubleClickTrigger> double_click_;
+  ClickDetector click_;
+  DoubleClickDetector double_click_;
 };
 
 TEST(ClickMatchInterval, BoundsAreInclusive) {
@@ -198,4 +201,23 @@ TEST_F(ClickTriggerTest, DoubleClickDoesNotFireTwiceWithoutReset) {
   EXPECT_EQ(fired, 1);
 }
 
+TEST_F(ClickTriggerTest, DoubleClickKeepsPressPublishedByItsActions) {
+  // The times reset before the actions run, so a press the first action publishes is kept
+  this->add_double_click_([this]() {
+    if (++fired == 1) {
+      this->sensor_.publish_state(false);
+      this->sensor_.publish_state(true);
+    }
+  });
+  this->press_for_(INSIDE_MS);
+  delay(INSIDE_MS);
+  this->sensor_.publish_state(true);
+  EXPECT_EQ(fired, 1);
+  delay(INSIDE_MS);
+  this->sensor_.publish_state(false);
+  delay(INSIDE_MS);
+  this->sensor_.publish_state(true);
+  EXPECT_EQ(fired, 2);
+}
+
 }  // namespace esphome::binary_sensor::testing

tests/components/feedback/test-minimal.esp8266-ard.yaml

@@ -1,13 +0,0 @@
-# No endstop, movement or obstacle sensors, so their code is compiled out
-cover:
-  - platform: feedback
-    id: feedback_minimal
-    name: Minimal Feedback Cover
-    open_action:
-      - logger.log: open
-    open_duration: 10s
-    close_action:
-      - logger.log: close
-    close_duration: 10s
-    stop_action:
-      - logger.log: stop

tests/integration/fixtures/feedback_cover_loop_wake.yaml

@@ -1,69 +0,0 @@
-esphome:
-  name: feedback-loop-wake
-
-host:
-api:
-  batch_delay: 0ms
-logger:
-  level: DEBUG
-
-# Stand in for a motor started outside ESPHome
-switch:
-  - platform: template
-    name: "External Open"
-    id: external_open
-    optimistic: true
-  - platform: template
-    name: "External Close"
-    id: external_close
-    optimistic: true
-
-binary_sensor:
-  - platform: template
-    id: open_moving
-    internal: true
-    condition:
-      switch.is_on: external_open
-  - platform: template
-    id: close_moving
-    internal: true
-    condition:
-      switch.is_on: external_close
-
-cover:
-  - platform: feedback
-    name: "Time Cover"
-    open_action:
-      - logger.log: "time_cover open"
-    close_action:
-      - logger.log: "time_cover close"
-    stop_action:
-      - logger.log: "time_cover stop"
-    open_duration: 2s
-    close_duration: 2s
-
-  - platform: feedback
-    name: "Max Duration Cover"
-    open_action:
-      - logger.log: "max_cover open"
-    close_action:
-      - logger.log: "max_cover close"
-    stop_action:
-      - logger.log: "max_cover stop"
-    open_duration: 3s
-    close_duration: 3s
-    max_duration: 1s
-
-  - platform: feedback
-    name: "Sensor Cover"
-    open_action:
-      - logger.log: "sensor_cover open"
-    close_action:
-      - logger.log: "sensor_cover close"
-    stop_action:
-      - logger.log: "sensor_cover stop"
-    open_duration: 4s
-    close_duration: 4s
-    open_sensor: open_moving
-    close_sensor: close_moving
-    update_interval: 200ms

tests/integration/test_feedback_cover_loop_wake.py

@@ -1,100 +0,0 @@
-"""Integration test that every way a feedback cover starts moving wakes its idle loop."""
-
-from __future__ import annotations
-
-import asyncio
-from collections.abc import Awaitable
-
-from aioesphomeapi import CoverInfo, CoverOperation, CoverState, EntityState, SwitchInfo
-import pytest
-
-from .log_utils import LineWaiter
-from .state_utils import StateWaiter, require_entity, subscribe_after_initial_states
-from .types import APIClientConnectedFactory, RunCompiledFunction
-
-
-def _expect_idle(waiter: StateWaiter, cover: CoverInfo) -> Awaitable[EntityState]:
-    return waiter.expect(
-        lambda s: (
-            isinstance(s, CoverState)
-            and s.key == cover.key
-            and s.current_operation == CoverOperation.IDLE
-        ),
-        label=f"{cover.name} IDLE",
-    )
-
-
-@pytest.mark.asyncio
-async def test_feedback_cover_loop_wake(
-    yaml_config: str,
-    run_compiled: RunCompiledFunction,
-    api_client_connected: APIClientConnectedFactory,
-) -> None:
-    """Test that target position, max_duration and movement sensors still work with the loop idled."""
-    lines = LineWaiter()
-    max_stopped = lines.expect("Max duration reached")
-    async with (
-        run_compiled(yaml_config, line_callback=lines.callback),
-        api_client_connected() as client,
-    ):
-        sensor_states: list[CoverState] = []
-        entities, _ = await client.list_entities_services()
-        time_cover = require_entity(entities, "time_cover", CoverInfo)
-        max_cover = require_entity(entities, "max_duration_cover", CoverInfo)
-        sensor_cover = require_entity(entities, "sensor_cover", CoverInfo)
-
-        def on_state(state: EntityState) -> None:
-            if isinstance(state, CoverState) and state.key == sensor_cover.key:
-                sensor_states.append(state)
-
-        sub = await subscribe_after_initial_states(client, entities, on_state=on_state)
-        waiter = sub.waiter
-
-        # Positions are restored from the previous run, so pick targets away from them
-        start = sub.initial_states[time_cover.key].position
-        target = 0.2 if start >= 0.5 else 0.8
-
-        # Only loop() stops a time-only cover at its target
-        done = _expect_idle(waiter, time_cover)
-        client.cover_command(key=time_cover.key, position=target)
-        state = await done
-        assert state.position == pytest.approx(target, abs=0.05)
-
-        # Stop mid move, then a new command must move it again and stop at the target
-        stopped = _expect_idle(waiter, time_cover)
-        client.cover_command(key=time_cover.key, position=start)
-        await asyncio.sleep(0.3)
-        client.cover_command(key=time_cover.key, stop=True)
-        mid = (await stopped).position
-        assert mid != pytest.approx(target, abs=0.05)
-        done = _expect_idle(waiter, time_cover)
-        client.cover_command(key=time_cover.key, position=target)
-        state = await done
-        assert state.position == pytest.approx(target, abs=0.05)
-
-        # 1s max_duration against a 3s travel
-        max_start = sub.initial_states[max_cover.key].position
-        max_target = 0.0 if max_start >= 0.5 else 1.0
-        done = _expect_idle(waiter, max_cover)
-        client.cover_command(key=max_cover.key, position=max_target)
-        state = await done
-        await asyncio.wait_for(max_stopped, 1.0)
-        assert abs(state.position - max_start) == pytest.approx(1 / 3, abs=0.1)
-
-        # Moved by the movement sensor alone, no command
-        sensor_start = sub.initial_states[sensor_cover.key].position
-        switch_name = "external_close" if sensor_start >= 0.5 else "external_open"
-        switch = require_entity(entities, switch_name, SwitchInfo)
-        sensor_states.clear()
-        client.switch_command(switch.key, True)
-        await asyncio.sleep(1.2)
-        settled = _expect_idle(waiter, sensor_cover)
-        client.switch_command(switch.key, False)
-        await settled
-        moving = {
-            round(s.position, 3)
-            for s in sensor_states
-            if s.current_operation != CoverOperation.IDLE
-        }
-        # One state from the sensor callback, the rest are loop() updates every 200ms
-        assert len(moving) >= 4, sensor_states