The problem

stop_action is not being called after max_duration on feedback cover with built-in endstops. The documentation states that the purpose of max_duration is protecting from dysfunctional endstops. That should include the built-in ones - I don't want the motors to burn down if the built-in endstops fail for any reason. In addition, there is no need for the relays to stay energized for days or weeks after the covers are fully open or fully closed. I did that previously with delays on open_action and close_action, but I suppose that is not ideal, and there should be no need for that with max_duration on the feedback cover.

Which version of ESPHome has the issue?

2022.9.1

What type of installation are you using?

Home Assistant Add-on

Which version of Home Assistant has the issue?

2022.9.5

What platform are you using?

ESP8266

Board

d1_mini

Component causing the issue

Feedback cover

Example YAML snippet

cover:
  - platform: feedback
    name: Left shade
    id: left_shade
    device_class: shade
    has_built_in_endstop: true
    assumed_state: true
    max_duration: 32s
    direction_change_wait_time: 1s
    open_action:
      - switch.turn_off: lmotor_direction
      - switch.turn_on: lmotor_enable
    open_duration: 25s
    close_action:
      - switch.turn_on: lmotor_direction
      - switch.turn_on: lmotor_enable
    close_duration: 25s
    stop_action:
      - switch.turn_off: lmotor_enable
      - switch.turn_off: lmotor_direction
      - logger.log: 
          format: "Left stopped"
          level: INFO

Anything in the logs that might be useful for us?

"Left stopped" log message is not being output after opening or closing and `max_duration` has elapsed.

Additional information

No response

This does not seem to work with the current based cover mode either. It only stops when current is below open_moving_current_threshold or close_moving_current_threshold but does not trigger on max_duration

If max_duration < open_duration, then the cover does stop after max_duration, but only if it has not yet reached the fully open state. (same with close) This means that max_duration doesn't do anything if the cover already reached the inferred fully open (closed) state while opening (closing).

E.g:

Working case

get the cover to 50%
set max_duration: 1s
command it to open --> relay turns on, then turns off after 1s

Broken case

get the cover to 100%
set max_duration: 1s
command it to open --> relay turns on, then stays on forever

Workaround

Not exactly equivalent to my max_duration, but IMO even better for time based covers (it will "overshoot" by 5s after reaching the limits)

cover:
  - platform: feedback
    name: ${friendly_name}
    id: "bedroom_roller_shutter"
    open_action:
      - switch.turn_on: open_cover_switch
    open_duration: 27s
    # max_duration: 30s # <-- broken https://github.com/esphome/issues/issues/3611
    close_action:
      - switch.turn_on: close_cover_switch
    close_duration: 26s
    has_built_in_endstop: true
    direction_change_wait_time: 200ms
    stop_action:
      - switch.turn_off: open_cover_switch
      - switch.turn_off: close_cover_switch

interval:
  # until max_duration is fixed
  - interval: 100ms
    then:
      - if:
          condition:
            for:
              time: 5s
              condition:
                or: 
                  - and:
                    - switch.is_on: close_cover_switch
                    - lambda: return id(bedroom_roller_shutter).position == COVER_CLOSED;
                  - and:
                    - switch.is_on: open_cover_switch
                    - lambda: return id(bedroom_roller_shutter).position == COVER_OPEN;
          then: 
            - cover.stop: bedroom_roller_shutter

@ianchi Could you have a look on that, would be amazing.

Hi @tracestep / @cheisig,

This was the intended behavior, perhaps the documentation is not well written. And the check on the config is what needs fixing, as it should not allow this parameter when there is no sensor (nor it to be lower than the open/close time).

The max_duration was intended to protect in the cases that the stop action is dependent on some kind of feedback from an endstop. If the signal never came, this threshold would prevent the cover from operating indefinitely, turning it into a time based one. But this only makes sense when the integration is in charge of the stop action.

But in the case when it has has_built_in_endstop, then all the stop operation is assumed to be fully handled by the cover itself, and the stop action is never called automatically (it is only there for the case of a manual stop command). It is the same behavior of the Time based cover.

In this last case ESPHome is not controlling the stop at all and it has no way of knowing if the stop has actually been requested or not, so if we were to include something like this, it would have to always trigger the stop action, most of the time a redundant second time, which could be a problem for many covers that only have a toggle switch to control them.

I'm inclined to send a fix in the checks of the config, to avoid these confusing possibilities.

Sorry but I might be an ignorant user, as from code perspective I might understand why max_duration is not supported. But from a users perspective this all seems unexpected. Please Let me explain that in 2 situations:

Situation 1 (a human example): as ESPHOME is replacing the human interface 😄

The sun is shining and the human want to keep it cool inside.
The human presses the down button
The human keeps pressing that button until the cover automatically reaches its build-in endstop
After an indefinite (its still human after all 😉) amount of time it stops pressing the button as it thinks the cover should be closed by now.
The human discovers the cover was stuck and motor was saved just in time.

Situation 2 (an automation example) ESPHOME is regularly used for automatons in which some of them are unattended and unsupervised.

A sensor detected a substantial amount of sunshine outside while the user is away from home
An automation triggers ESPHOME to lower the cover to keep it cool inside
ESPHOME commands the cover to go down until close_moving_current_threshold is met as a build-in endstop is installed.
This trigger does not happen because cover is stuck and motor remains drawing current.
ESPHOME should trigger on max_duration but as is designed does not.
After returning home the user discovers the cover is stuck ESPHOME is still commanding the cover to go down and the motor has burned out.

> Hi @tracestep / @cheisig,

This was the intended behavior, perhaps the documentation is not well written. And the check on the config is what needs fixing, as it should not allow this parameter when there is no sensor (nor it to be lower than the open/close time).

The max_duration was intended to protect in the cases that the stop action is dependent on some kind of feedback from an endstop. If the signal never came, this threshold would prevent the cover from operating indefinitely, turning it into a time based one. But this only makes sense when the integration is in charge of the stop action.

Hi @ianchi,

First of all, thanks a lot for this wonderful implementation! This fixes some issues I had with time_based_cover!

I understand your argument and it is absolutely valid, the signal does not show up and max_duration releases the relay of the ESP to safeguard the motor, humans and house.

But in the case when it has has_built_in_endstop, then all the stop operation is assumed to be fully handled by the cover itself, and the stop action is never called automatically (it is only there for the case of a manual stop command). It is the same behavior of the Time based cover.

Correct, if you think about motors with mechanical built-in-endstops, I agree, a malfuntion of these is very unlikely. And it can be assumed they will stop as they should. But the newer generation of motors are with electronical endstops, which could be affected of a malfunction. Nontheless, the ESP must be informed to "Open" the relay, either you have a current sensor or by a defined time.

CASE1: ESP with current sensor and motor with mechanical or electronical endstop Example Code: "Opening" 1.) The movement of the cover does only start if the current is above 0.5A

    id: open_binary_sensor
    **sensor_id: open_current_sensor** 
    **threshold: 0.5**
    filters:
      - delayed_off: 0.8s

_2. To have the relay of the ESP released we need to set "has_built_in_endstop" and "infer_endstop_frommovement" So the relay is released when the current is below 0.5A, correct @ianchi ?

cover:
  - platform: feedback
    name: "Feedback Based Cover"
    **has_built_in_endstop: true**
    **infer_endstop_from_movement: true**

CASE1: As long as the current sensor does fall below 0.5A the relay is released by infer_endstop_from_movement: true Problem: As @TimelessNL wrote, the current is not falling below threshold, the relay stays closed. --> max_duration would be useful

CASE2: ESP without current sensor and motor with mechanical or electronical endstop Example Code: "Opening" 1.) Everthing is assumed, means: The stop action is performed based on the time elapsed, so when open_duration is reached. With time_based_cover this caused issues when there was a delay for interlock, especially when human did stop and start the cover in between (before reaching the end)

 open_action:
      - switch.turn_on: open_cover_switch
    open_duration: 2.1min

Thanks for the wonderful fix in Feedback cover of that @ianchi Issue on time based: [Time-Based Cover - calculate interlock_wait_time to open/close duration + add max_duration]

CASE2: IMHO, a max_duration is not neccessary as long as "has_built_in_endstop" is not set to "true", nontheless it would not harm to have it implemented.

In this last case ESPHome is not controlling the stop at all and it has no way of knowing if the stop has actually been requested or not, so if we were to include something like this, it would have to always trigger the stop action, most of the time a redundant second time, which could be a problem for many covers that only have a toggle switch to control them.

Indeed this is valid for the ESP without any sensors, but as mentioned in above CASE1, it would be a good idea to have max_duration working in case "has_built_in_endstop" is set to "true". Why would that cause any issue with the _toggle__ switch motors? When "has_built_in_endstop" is active, currently max_duration is completely useless, isn't it?

BR Chris

There are many cases described (some not relevant to this component, i.e. using close_moving_current_threshold which does not exist in feedback cover) with different situations.

But from all of them I get that the underlying hardware configuration is VERY different from mine, which was the one that prompted me to create this integration. I tried to make it as generic as possible but obviously it doesn't accommodate every alternative.

From the examples, I see the common factor of having built in endstops but some cases show some feedback sensor and some none.

In the case of no feedback sensor (like the one from the original issue), we are in a simple time based cover. Setting has_built_in_endstop tells that we want to track state by time but not to trigger the stop action. It is the whole purpose of the option.

But it seems that what it is actually wanted is to ALWAYS trigger the stop action no matter what. Why set the option then? This can be achieved by letting the component issue the stop after the open/close time.

Or if the motor switch needs to always be released after some amount of time, why not simply tell that to the switch? No very specific behavior is needed to be added to the cover component.


switch:
  - platform: gpio
    pin: 25
    id: lmotor_enable

    on_turn_on:
    - delay: 10000ms # name your max time
    - switch.turn_off: lmotor_enable

But in order to be of better help, first I'd like to broadly understand the hardware (the ESPHome controlled part, the built-in one and how they interact).

There are many cases described (some not relevant to this component, i.e. using close_moving_current_threshold which does not exist in feedback cover) with different situations.

Excuse me, I falsely assumed the feedback cover used an identical max_duration approach to that of the current based cover as the behavior is the same and I was wrong.

But it seems that what it is actually wanted is to ALWAYS trigger the stop action no matter what

No? where do you get that from? The expected behavior is when the cover is going and max_duration is elapsed it should preform the stop_action.

In the case detailed in the opening post of this issue, the configuration acts as a simple time based cover. There is no sensor. There is no way of knowing if the cover is actually moving.

So there are only two alternatives:

You NEVER issue the stop action (and leave it to the external hardware)
you ALWAYS issue the stop action (after some fixed time)

There’s simply no information to take an intermediate (conditional) path.

The first alternative is achieved by setting has_built_in_endstop (that is exactly the purpose of the option). The other by not setting it.

So, again, if the desired behavior is to issue the stop action always after some time, this can already be achieved.

So, again, if the desired behavior is to issue the stop action always after some time, this can already be achieved.

No not always, when the cover is already stopped the stop option must not be called.

In the case detailed in the opening post of this issue, the configuration acts as a simple time based cover. There is no sensor. There is no way of knowing if the cover is actually moving.

So there are only two alternatives:

You NEVER issue the stop action (and leave it to the external hardware)

you ALWAYS issue the stop action (after some fixed time)

There’s simply no information to take an intermediate (conditional) path.

The first alternative is achieved by setting has_built_in_endstop (that is exactly the purpose of the option). The other by not setting it.

So, again, if the desired behavior is to issue the stop action always after some time, this can already be achieved.

@ianchi Hi again, after some time now First: I am super happy that you created feedback_cover, because it solves a lot of issues with the delay when starting stopping via time_based_cover

I finally had the time to implement the feedback_cover as an alternative to time_based_cover

In ESPHome Version: 2023.07.01 I thought the "max_duration" was working as intended or at least as I require it. I updated my HA and ESPHome and suddently the behaviour of my ESP changed and stopped working.

Documentation says: The documentation of the parameter "maxduration" says: "Max duration, to protect from faulty endstops" and _"maxduration (Optional, Time): The maximum duration the cover should be opening or closing. Useful for protecting from dysfunctional endstops. Requires internal, builtin or inferred endstops."

So my Case suddenly stopped working, while it did before (currently I restore a snapshot to find out if I mixed something up), did you change something?

My case: 1.) Roller Shutter on my windows I have some "rolladen" in english Roller Shutter? (looks like that: Rolladen

2.) Motor with Endstops but without "feedback" to any e.g. ESP These do have a motor with build-in-endstop which will stop the motor when the end-position is reached. Something like that: Motor

My idea of implementation: Your feedback cover as an time-based-cover as it does the math for delays much better

1.) Open and Close duration are used to drive the roller to the position I want (e.g. 20% or 60% or any else) 2.) has_built_in_endstop takes care that the motor doesn't stop when open- or close_duration has been reached _Reasons:

As we are "optimistic" only, we cannot be sure that the roller is already fully closed or fully opened
As I have an endstop (mechanically in my motor), the motor will stop by itself (while my relay is still closed on the ESP)_

3.) max_duration should now take care to action a "stop command" to release my relay on the ESP board While the motor is now stoppped, I like to have the ESP relay released again as well (prevent for welding relays)

Obviously we have a different idea of max_duration. Would you be able to help me understanding what your intention of max_duration is and what it should do as per your design?

Okay, just a heads up... More difficult than I thought (and honestly I am wondering if my previous config had the max_duration inside at all...)

But I played around with "on_open and on_closed" Cover On Open

The idea was to simply add additional 5 seconds after the cover reaches either fully closed or fully open. Issue: Every Time when the "Stop Button" is pressed these states are reached as well. Means as well, if I press stop followed by "up", it stops after 5 seconds... So the following was not sufficient:

    on_open: 
        - logger.log: "Cover is Open!"
        - lambda: |-
            if (id(cover_kitchen).current_operation == CoverOperation::COVER_OPERATION_IDLE) {
            ESP_LOGD("custom", "Cover is idle in on_open");
            }
        # - delay: ${max_duration_delta}sec
        # - switch.turn_off: relay_kitchen_up
        # - delay: 100ms
        # - switch.turn_off: relay_kitchen_down
        - script.execute: script_save_changes
    on_closed: 
        - logger.log: "Cover is Closed!"
        - lambda: |-
            if (id(cover_kitchen).current_operation == CoverOperation::COVER_OPERATION_IDLE) {
            ESP_LOGD("custom", "Cover is idle in on_closed");
            }
        # - delay: ${max_duration_delta}sec
        # - switch.turn_off: relay_kitchen_up
        # - delay: 100ms
        # - switch.turn_off: relay_kitchen_down
        - script.execute: script_save_changes

I decided to rethink and refining my requirement: 1.) close_duration & open_duration should only be used for calculating when positioning --> We will set "has_built_in_endstop: True" 2.) max_duration is as per design currently not used when no feedback is provided (e.g. measuring thru current or feedback by reed contact) 3.) To stop the relay, the stopp command should be issued a few seconds later than close duration or open duration

So if anyone has the same requirement than me (and is not able to develop in code, same as me), here is what does the job finally: Solution was: "wait_until"

substitutions:
  device_name: 12v_shift_relay
  friendly_name: 12VTestRollade

  #wifi domain to use, standard =  .local
  domain: .local

  #max_duration(in seconds)=the extra time to wait until fully open state is reached
  release_relay_delta: 10sec
  cover_kitchen: Küche
  #Open Duration only indication for positioning not stopping at fully open / closed
  cover_kitchen_open_duration: 20.05sec
  cover_kitchen_close_duration: 20.00sec

switch:
#Relay by ShiftRegister (id only, so only internal)
#Even if not required, in case motors are correctly wired, adding interlock to always make sure only 1 Relay is driving, especially in Web Server
  - platform: gpio
    id: relay_kitchen_up
    interlock: &interlock_group3 [relay_kitchen_up, relay_kitchen_down]
    restore_mode: always off
    pin:
      sn74hc595: sn74hc595_hub
      # Use pin number 4
      number: 4
      inverted: false

  - platform: gpio
    id: relay_kitchen_down
    interlock: *interlock_group3
    restore_mode: always off
    pin:
      sn74hc595: sn74hc595_hub
      # Use pin number 5
      number: 5
      inverted: false

#https://github.com/esphome/feature-requests/issues/778#issuecomment-989277084
cover:
  #Cover Kitchen UP  #Relay 4
  #Cover Kitchen DOWN  #Relay 5
  - platform: feedback
    name: "${friendly_name} ${cover_kitchen}"
    id: cover_kitchen
    assumed_state: true
    direction_change_wait_time: 250ms
    acceleration_wait_time: 50ms
    has_built_in_endstop: True
    update_interval: 1s
    open_duration: ${cover_kitchen_open_duration}
    close_duration: ${cover_kitchen_close_duration}
    #max_duration: 22sec #not intended to be used without feedback sensor - https://github.com/esphome/issues/issues/3611
    open_action:
      - switch.turn_on: relay_kitchen_up
      - wait_until:
            condition:
              lambda: 'return (id(cover_kitchen).position == COVER_OPEN);'
            timeout: 25sec
      - delay: ${release_relay_delta}
      - cover.stop: cover_kitchen

    close_action:
      - switch.turn_on: relay_kitchen_down
      - wait_until:
            condition:
              lambda: 'return (id(cover_kitchen).position == COVER_CLOSED);'
            timeout: 25sec
      - delay: ${release_relay_delta}
      - cover.stop: cover_kitchen

    stop_action:
      - switch.turn_off: relay_kitchen_up
      - delay: 100ms #for shift register a delay, avoiding being to fast
      - switch.turn_off: relay_kitchen_down
    on_open: 
        - logger.log: "Cover is Open!"
        - script.execute: script_save_changes
    on_closed: 
        - logger.log: "Cover is Closed!"
        - script.execute: script_save_changes

If someone ever decides to use the same cheap board as I do: Cheap X16 Board with ESP12f

The wiring should be done like that: Wiring X16 Relay

The LOG component is actually killing the board every few seconds. To avoid that, here my working config with WebServer but without logging.

esp8266:
  board: esp12e
  early_pin_init: false
  restore_from_flash: true
  framework:
    version: latest
    platform_version: "4.2.1"

#Logger must be at least debug (default)
#debug:

logger:
  level: warn
  esp8266_store_log_strings_in_flash: true
  tx_buffer_size: 32
  hardware_uart: UART1
  baud_rate: 0
  #enable for serial debugging
  #baud_rate: 115200

# uart:
#   #not required, but left in for reference
#   - id: soft_uart
#     tx_pin: ${low_io16}
#     rx_pin: none
#     baud_rate: 115200

#Enable Home Assistant API
api:
  password: !secret api_pass2
  #no reboot in case HA is down - can require full reboot when HA is back
  reboot_timeout: 0s
ota:
  password: !secret ota_pass

wifi:
  #https://esphome.io/components/wifi.html#power-save-mode
  power_save_mode: none
  #No reboot if Wifi is missing - can require full reboot when wifi is back
  reboot_timeout: 0s
  domain: "${domain}"

  networks:
    - ssid: !secret wifi_ssid
      password: !secret wifi_pass
    - ssid: !secret wifi_ssid_eg
      password: !secret wifi_pass_eg

    #https://esphome.io/components/wifi.html#connecting-to-multiple-networks
    #Instead using a AP, using a Rescue Network, so you can span a AP 
    #with your mobile and ESP is connecting to the same
    - ssid: "rescue"
      password: "safemysoul"

#Enable Web server
web_server:
  port: 80
  #version: 1
  log: false

@ianchi: If you every think you would like to implement the "max_duration" or lets call it "stop_duration", I would still love it 👍

esphome / issues

Stop action not being called after max_duration on feedback cover with built-in endstops #3611