konnected-io / konnected-esphome

ESPHome firmware configuration and recipes for Konnected devices
Other
54 stars 37 forks source link

Garage Door: "open" and "stop" should not push the button if the door is closed #13

Closed ImSorryButWho closed 7 months ago

ImSorryButWho commented 1 year ago

In the Google Assistant ecosystem (and possibly other, similar ecosystems), opening a garage door requires a PIN, while closing or stopping it does not. This makes sense: opening a door is a more security-sensitive operation than opening it.

In the current implementation, "closing" or "stopping" the door when it's closed will actually open it, bypassing any security restrictions and potentially causing errors. Consider, for example, a naïve automation to close the door at 10:00 PM. The most straightforward implementation would cause the door to open if it's already closed!

Of course, this can be worked around at another layer -- in Home Assistant, for example, you could wrap this cover with another template cover adding additional logic. But it seems to make more sense to handle the logic on the controller, to reduce complexity for users and prevent errors.

See this Reddit thread for an affected user.

tonytaylor85 commented 1 year ago

I was affected by this. I'm new to ESPHome, so I copied packages/garage-door-cover-wired-and-range.yaml into Home Assistant's ESPHome config page and got it working. I'm commenting so my use case gets covered/caught.

I'm using a wired sensor (closed contact = closed door) and the range sensor, and I had to invert the wired sensor.

The stop button also could use some logic. My opener doesn't have a deterministic "stop" function. I set mine up to only button.press if the range and wired sensors disagree.

cjust2006 commented 1 year ago

I was the poster in the above Reddit thread. This explanation is exactly right. Any command will toggle door action, regardless of state. Telling the door to open, close, or stop will send a button press regardless.

tonytaylor85 commented 1 year ago

I sent a support email, turns out it's the home assistant SERVICE that checks the current status. I was using the home assistant DEVICE, which is the esphome button. I haven't tested this yet, but I do remember the dashboard card ignoring the state.

tonytaylor85 commented 1 year ago

here's my yaml, yours could be different if you're not using a wired sensor. don't forget to comment out the GARAGE_DOOR_PACKAGE you're replacing. for me it was

# 3. WIRED & RANGE SENSORS - Include this line.
# - packages/garage-door-cover-wired-and-range.yaml
cover:
  - platform: template
    name: $garage_door_cover_name
    device_class: garage
    lambda: |-
      if (id(garage_door_wired_sensor).state || id(garage_door_range_sensor).state) {
        return COVER_OPEN;
      } else {
        return COVER_CLOSED;
      }
    open_action:
      - if:
          condition:
            and:
              - binary_sensor.is_off: garage_door_wired_sensor
              - binary_sensor.is_off: garage_door_range_sensor
          then:
            - button.press: garage_door_opener_button
    close_action:
      - if:
          condition:
            and:
              - binary_sensor.is_on: garage_door_wired_sensor
              - binary_sensor.is_on: garage_door_range_sensor
          then:
            - light.turn_on:
               id: warning_led
               effect: strobe
            - light.turn_on:
               id: warning_beep
               effect: strobe
            - delay: $garage_door_close_warning_duration
            - button.press: garage_door_opener_button
            - light.turn_off: warning_led
            - light.turn_off: warning_beep
    stop_action:
      - if:
          condition:
            and:
              - binary_sensor.is_on: garage_door_wired_sensor
              - binary_sensor.is_off: garage_door_range_sensor
          then:
            - button.press: garage_door_opener_button
      - if:
          condition:
            and:
              - binary_sensor.is_off: garage_door_wired_sensor
              - binary_sensor.is_on: garage_door_range_sensor
          then:
            - button.press: garage_door_opener_button
cjust2006 commented 1 year ago

Hey many thanks for that YAML! I'm extremely new to HA, but if I understand correctly, I need to enable the templates integration to use this, correct? And I'm not using a wired sensor, just the laser range sensor.

I'll get the template integration enabled and see how it goes. Thanks again!

On Fri, Jul 21, 2023 at 3:16 PM tonytaylor85 @.***> wrote:

here's my yaml, yours could be different if you're not using a wired sensor. don't forget to comment out the GARAGE_DOOR_PACKAGE you're replacing. for me it was # 3. WIRED & RANGE SENSORS - Include this line.

- packages/garage-door-cover-wired-and-range.yaml

`cover:

  • platform: template name: $garage_door_cover_name device_class: garage lambda: |- if (id(garage_door_wired_sensor).state || id(garage_door_range_sensor).state) { return COVER_OPEN; } else { return COVER_CLOSED; } open_action:
    • if: condition: and:
    • binary_sensor.is_off: garage_door_wired_sensor
    • binary_sensor.is_off: garage_door_range_sensor then:
    • button.press: garage_door_opener_button close_action:
    • if: condition: and:
    • binary_sensor.is_on: garage_door_wired_sensor
    • binary_sensor.is_on: garage_door_range_sensor then:
    • light.turn_on: id: warning_led effect: strobe
    • light.turn_on: id: warning_beep effect: strobe
    • delay: $garage_door_close_warning_duration
    • button.press: garage_door_opener_button
    • light.turn_off: warning_led
    • light.turn_off: warning_beep stop_action:
    • if: condition: and:
    • binary_sensor.is_on: garage_door_wired_sensor
    • binary_sensor.is_off: garage_door_range_sensor then:
    • button.press: garage_door_opener_button
    • if: condition: and:
    • binary_sensor.is_off: garage_door_wired_sensor
    • binary_sensor.is_on: garage_door_range_sensor then:
    • button.press: garage_door_opener_button`

— Reply to this email directly, view it on GitHub https://github.com/konnected-io/konnected-esphome/issues/13#issuecomment-1646193405, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALPAFZFIVRAMFTGRU5T3HLTXRLPSJANCNFSM6AAAAAA2GFGCBE . You are receiving this because you commented.Message ID: @.***>

tonytaylor85 commented 1 year ago

I am using the ESPHome integration, and the YAML goes in there. I don't have any template integration.

tonytaylor85 commented 10 months ago

Updated ESPHome today, and the wired sensor has been renamed here. You'll have to replace all references to garage_door_wired_sensor to garage_door_input.

Exact error for the search engines- Failed config

binary_sensor.unknown: [source /config/esphome/garage-door-esp.yaml:214]

Source for extension of ID 'garage_door_wired_sensor' was not found.

heythisisnate commented 7 months ago

This has been fixed in https://github.com/konnected-io/konnected-esphome/commit/cd9f33d90152110a78c6cf626ea5a769c9de7a69!