Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
erincinci opened this issue Apr 20, 2021 · 17 comments
Closed

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

erincinci opened this issue Apr 20, 2021 · 17 comments

Comments

@erincinci
Copy link

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 erincinci changed the title Longer Timeout for Doorbell Pressed Event [Feature] Longer Timeout for Doorbell Pressed Event Apr 20, 2021
@erincinci
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

matijse commented Apr 20, 2021

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
Copy link
Author

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.

@erincinci erincinci changed the title [Feature] Longer Timeout for Doorbell Pressed Event [Feature] Configurable off-delay for Doorbell Pressed Event Apr 20, 2021
@skank01
Copy link

skank01 commented Apr 20, 2021

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

@erincinci
Copy link
Author

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
Copy link
Owner

matijse commented Apr 20, 2021

@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
Copy link
Author

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
Copy link
Contributor

@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
Copy link
Contributor

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

@MaxWinterstein
Copy link
Contributor

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
Copy link

Snuffy2 commented Apr 21, 2021

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

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

image

@MaxWinterstein
Copy link
Contributor

Fingers crossed, update is available ✌️

@erincinci
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants