[feedback] Support for overshoot_duration (#13186)

Co-authored-by: J. Nick Koston <nick@koston.org>
Co-authored-by: J. Nick Koston <nick@home-assistant.io>

esphome/components/feedback/cover.py

@@ -24,6 +24,7 @@ CONF_HAS_BUILT_IN_ENDSTOP = "has_built_in_endstop"
 CONF_INFER_ENDSTOP_FROM_MOVEMENT = "infer_endstop_from_movement"
 CONF_DIRECTION_CHANGE_WAIT_TIME = "direction_change_wait_time"
 CONF_ACCELERATION_WAIT_TIME = "acceleration_wait_time"
+CONF_OVERSHOOT_DURATION = "overshoot_duration"
 CONF_OBSTACLE_ROLLBACK = "obstacle_rollback"
 
 endstop_ns = cg.esphome_ns.namespace("feedback")
@@ -82,6 +83,9 @@ CONFIG_FEEDBACK_COVER_BASE_SCHEMA = (
             cv.Optional(
                 CONF_ACCELERATION_WAIT_TIME, "0s"
             ): cv.positive_time_period_milliseconds,
+            cv.Optional(
+                CONF_OVERSHOOT_DURATION, "0s"
+            ): cv.positive_time_period_milliseconds,
             cv.Optional(CONF_OBSTACLE_ROLLBACK, default="10%"): cv.percentage,
         },
     )
@@ -89,10 +93,25 @@ CONFIG_FEEDBACK_COVER_BASE_SCHEMA = (
 )
 
 
+def validate_overshoot(config: ConfigType) -> ConfigType:
+    if not config[CONF_OVERSHOOT_DURATION].total_milliseconds:
+        return config
+    if config[CONF_HAS_BUILT_IN_ENDSTOP]:
+        raise cv.Invalid(
+            f"{CONF_OVERSHOOT_DURATION} has no effect with {CONF_HAS_BUILT_IN_ENDSTOP}"
+        )
+    if CONF_OPEN_ENDSTOP in config and CONF_CLOSE_ENDSTOP in config:
+        raise cv.Invalid(
+            f"{CONF_OVERSHOOT_DURATION} has no effect when both endstop sensors are supplied"
+        )
+    return config
+
+
 CONFIG_SCHEMA = cv.All(
     CONFIG_FEEDBACK_COVER_BASE_SCHEMA,
     cv.has_none_or_all_keys(CONF_OPEN_SENSOR, CONF_CLOSE_SENSOR),
     validate_infer_endstop,
+    validate_overshoot,
 )
 
 
@@ -160,4 +179,7 @@ async def to_code(config: ConfigType) -> None:
             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

@@ -79,6 +79,11 @@ void FeedbackCover::dump_config() {
   if (this->acceleration_wait_time_) {
     ESP_LOGCONFIG(TAG, "  Acceleration wait time: %.1fs", this->acceleration_wait_time_ / 1e3f);
   }
+#ifdef USE_FEEDBACK_COVER_OVERSHOOT
+  if (this->overshoot_duration_) {
+    ESP_LOGCONFIG(TAG, "  Overshoot duration: %.1fs", this->overshoot_duration_ / 1e3f);
+  }
+#endif
 #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);
@@ -234,10 +239,22 @@ void FeedbackCover::loop() {
   // (stoping from endstop sensor is handled in callback)
   if (this->current_trigger_operation_ != COVER_OPERATION_IDLE) {
     if (this->is_at_target_()) {
-      if (this->has_built_in_endstop_ &&
-          (this->target_position_ == COVER_OPEN || this->target_position_ == COVER_CLOSED)) {
+      const bool at_end = this->target_position_ == COVER_OPEN || this->target_position_ == COVER_CLOSED;
+      if (this->has_built_in_endstop_ && at_end) {
         // Don't trigger stop, let the cover stop by itself.
         this->set_current_operation_(COVER_OPERATION_IDLE, true);
+#ifdef USE_FEEDBACK_COVER_OVERSHOOT
+      } else if (at_end && this->overshoot_duration_) {
+        // The define is global, so a cover without the option still lands here with a zero duration
+        // Keep driving past the end position for a while so accumulated timing error is squeezed out
+        if (!this->start_overshoot_time_) {
+          this->start_overshoot_time_ = now;
+        } else if (now - this->start_overshoot_time_ > this->overshoot_duration_ ||
+                   now - this->start_dir_time_ > this->max_duration_) {
+          ESP_LOGD(TAG, "'%s' - Overshoot duration reached. Stopping cover.", this->name_.c_str());
+          this->start_direction_(COVER_OPERATION_IDLE);
+        }
+#endif
       } else {
         this->start_direction_(COVER_OPERATION_IDLE);
       }
@@ -340,6 +357,11 @@ void FeedbackCover::start_direction_(CoverOperation dir) {
   binary_sensor::BinarySensor *obstacle{nullptr};
 #endif
 
+#ifdef USE_FEEDBACK_COVER_OVERSHOOT
+  // Any new command ends a running overshoot
+  this->start_overshoot_time_ = 0;
+#endif
+
   switch (dir) {
     case COVER_OPERATION_IDLE:
       trig = &this->stop_trigger_;

esphome/components/feedback/feedback_cover.h

@@ -33,6 +33,9 @@ class FeedbackCover final : public cover::Cover, public Component {
   void set_has_built_in_endstop(bool value) { this->has_built_in_endstop_ = value; }
   void set_assumed_state(bool value) { this->assumed_state_ = value; }
   void set_max_duration(uint32_t max_duration) { this->max_duration_ = max_duration; }
+#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; }
@@ -67,6 +70,9 @@ class FeedbackCover final : public cover::Cover, public Component {
   uint32_t open_duration_{0};
   uint32_t close_duration_{0};
   uint32_t max_duration_{UINT32_MAX};
+#ifdef USE_FEEDBACK_COVER_OVERSHOOT
+  uint32_t overshoot_duration_{0};
+#endif
   optional<uint32_t> direction_change_waittime_{};
   uint32_t acceleration_wait_time_{0};
   bool has_built_in_endstop_{false};
@@ -80,6 +86,9 @@ class FeedbackCover final : public cover::Cover, public Component {
   uint32_t last_recompute_time_{0};
   uint32_t start_dir_time_{0};
   uint32_t last_publish_time_{0};
+#ifdef USE_FEEDBACK_COVER_OVERSHOOT
+  uint32_t start_overshoot_time_{0};
+#endif
   float target_position_{0};
   uint32_t update_interval_{1000};
 };

esphome/core/defines.h

@@ -73,6 +73,7 @@
 #define USE_ESP_NOW_HOSTED
 #define USE_EVENT
 #define USE_FAN
+#define USE_FEEDBACK_COVER_OVERSHOOT
 #define USE_GPIO_BINARY_SENSOR_INTERRUPT
 #define USE_GPIO_SWITCH_INTERLOCK
 #define USE_GRAPH

tests/components/feedback/common.yaml

@@ -37,3 +37,16 @@ cover:
       - logger.log: Close Action
     stop_action:
       - logger.log: Stop Action
+  # Time-only cover with overshoot next to one without: the define is global, so both paths compile
+  - platform: feedback
+    name: Feedback Cover Plain
+    id: feedback_plain
+    open_duration: 10s
+    close_duration: 10s
+    overshoot_duration: 2s
+    open_action:
+      - logger.log: Plain Open
+    close_action:
+      - logger.log: Plain Close
+    stop_action:
+      - logger.log: Plain Stop