matijse / eufy-ha-mqtt-bridge

Receive Eufy alerts and thumbnails in Home Assistant via MQTT
124 stars 25 forks source link

[Feature] Configurable `off-delay` for Doorbell Pressed Event #62

Closed erincinci closed 3 years ago

erincinci commented 3 years ago

Hey, great work on the integration, completes the missing features from the other unofficial Eufy integration, thanks for that.

Also, it's great to have the doorbell pressed event so that we can use it in multiple automation/scenarios (like showing the last doorbell image on doorbell pressed). Would be much better if we would be able to configure the timeout in seconds for the doorbell event, or at least to set it to longer by default since right now it goes back to the off state quite quickly (in 1-2 seconds). Thanks!

erincinci commented 3 years ago

I'm not sure if this can be solvable without an update on the integration itself, probably through a custom configuration on the MQTT binary_sensor (Official Docs). Would appreciate your input on the topic.

erincinci commented 3 years ago

Small update, tried overriding off_delay from customization config, but doesn't seem to work (even though override is seen in the entity). Screenshot 2021-04-20 at 15 57 51

matijse commented 3 years ago

The off_delay is set when "auto configuring" the MQTT entities in Home Assistant. This happens every time HA restarts, so probably your changes are overwritten again then... You might try to add a completely different entity that listens to the correct topics.

But it would also be pretty easy to make this configurable... I can probably add it soon...

Just out of curiosity, why would you want the delay to be longer? I implemented it like this because I imagined people would use it as a trigger (trigger when state changes from "clear" to "motion"), so it doesn't really matter how long it stays on "motion"...

What use case do you have?

erincinci commented 3 years ago

Thanks for the fast response @matijse, my use-case is I show the last screenshot from my doorbell on my wallpanel in a conditional lovelace panel, so when the doorbell pressed trigger switches back to off quickly there's no reasonable time to show the person on the door in the wallpanel tablet.

skank01 commented 3 years ago

Thanks for the fast response @matijse, my use-case is I show the last screenshot from my doorbell on my wallpanel in a conditional lovelace panel, so when the doorbell pressed trigger switches back to off quickly there's no reasonable time to show the person on the door in the wallpanel tablet.

Sounds more like https://github.com/matijse/eufy-ha-mqtt-bridge/issues/23 In the end, we just want a valid , true picture of the one who is pressing the doorbell. For now, this aint always working no matter what

erincinci commented 3 years ago

Thanks for the fast response @matijse, my use-case is I show the last screenshot from my doorbell on my wallpanel in a conditional lovelace panel, so when the doorbell pressed trigger switches back to off quickly there's no reasonable time to show the person on the door in the wallpanel tablet.

Sounds more like #23 In the end, we just want a valid , true picture of the one who is pressing the doorbell. For now, this aint always working no matter what

@skank01 I don't agree those two are the same, even if the last image is fixed, right now we have no way of configuring off-delay to show the last image on a conditional card.

matijse commented 3 years ago

@erincinci Nice solution! Makes sense to do it this way. Since it was very easy to implement, I just created a release that has this option. (Building now...).

See here on how to configure this if you use the Docker image: https://github.com/matijse/eufy-ha-mqtt-bridge#configuration.

For the add-on, I think maybe some extra configuration option needs to be made available, if so @MaxWinterstein can you check this?

@skank01 This has nothing to do with the images itself, that is a separate problem, which is harder to solve... Time is limited unfortunately :(

erincinci commented 3 years ago

Thanks a lot @matijse, appreciate the fast implementation! @MaxWinterstein looking forward to the add-on updates as well, let me know if I can be of any help there.

MaxWinterstein commented 3 years ago

@MaxWinterstein looking forward to the add-on updates as well, let me know if I can be of any help there.

@matijse and I found a nice way to automate nearly everything on these 'simple updates'. Just one manually approve from my side and it is done. But you are welcome to test it now :) (might need to manually reload repositories via the three dots in the upper right as there is some caching involved)

MaxWinterstein commented 3 years ago

Update: Did not see the new config option, should have read all notifications before 🙄 Will try do implement asap 👍

MaxWinterstein commented 3 years ago

Update2: Should be possible to configure now. I am in a little hurry, so just quick tested. please provide feedback if it works. (sorry for the multiple comments, but I hate that edits on comments won't send out separate emails. often information got lost that way...)

Snuffy2 commented 3 years ago

After the update to 1.16 in the Home Assistant Supervisor Add On, the Add On won't load unless you add the new option to the coniguration.

home_assistant: off_delay: 5

Without it, this error appears in the Supervisor Logs: 21-04-21 01:28:36 ERROR (MainThread) [supervisor.addons.addon] Add-on f1c878cb_eufy-ha-mqtt-bridge has invalid options: Missing option 'home_assistant' in root in Eufy Home Assistant MQTT Bridge (f1c878cb_eufy-ha-mqtt-bridge). Got {'eufy': {'username': 'eufy_name', 'password': 'eufy_pass'}, 'mqtt': {'url': 'mqtt://homeassistant.local:1883', 'username': 'mqtt_name', 'password': 'mqtt_pass', 'keepalive': 60}, 'log_level': 'info', 'eval': ''}

erincinci commented 3 years ago

Just updated the add-on and verified the feature is working (I added the config manually) 🎉 Thanks a lot folks! I set the off-delay to 30 seconds, FYI for screenshot.

@MaxWinterstein do we need to set the default config to include new configuration for fixing @Snuffy2 's problem? https://github.com/MaxWinterstein/homeassistant-addons/blob/main/eufy-ha-mqtt-bridge/config.json#L16

Screenshot 2021-04-21 at 08 26 01

MaxWinterstein commented 3 years ago

After the update to 1.16 in the Home Assistant Supervisor Add On, the Add On won't load unless you add the new option to the coniguration.

home_assistant: off_delay: 5

Without it, this error appears in the Supervisor Logs: 21-04-21 01:28:36 ERROR (MainThread) [supervisor.addons.addon] Add-on f1c878cb_eufy-ha-mqtt-bridge has invalid options: Missing option 'home_assistant' in root in Eufy Home Assistant MQTT Bridge (f1c878cb_eufy-ha-mqtt-bridge). Got {'eufy': {'username': 'eufy_name', 'password': 'eufy_pass'}, 'mqtt': {'url': 'mqtt://homeassistant.local:1883', 'username': 'mqtt_name', 'password': 'mqtt_pass', 'keepalive': 60}, 'log_level': 'info', 'eval': ''}

D'oh >.<

@MaxWinterstein do we need to set the default config to include new configuration for fixing @Snuffy2 's problem? https://github.com/MaxWinterstein/homeassistant-addons/blob/main/eufy-ha-mqtt-bridge/config.json#L16

Yes, seems so. I thought the documentation https://developers.home-assistant.io/docs/add-ons/configuration/#options--schema allowes for optional values, but this seems to be not working for dicts.


Release v1.16.1 is currently backing, should be there in around 30 minutes. As I am on the run I started the build synchronous to the update release, you should wait a little bit before hitting the update button. Will give feedback here, too.

MaxWinterstein commented 3 years ago

@Snuffy2 Just add it manually for now, this should also work:

image

MaxWinterstein commented 3 years ago

Fingers crossed, update is available :v:

erincinci commented 3 years ago

Thanks @MaxWinterstein working for me. But probably somebody without the config should test the update sequence.