search

maximvelichko / pyvera

A python library to control devices via the Vera hub
GNU General Public License v2.0
26 stars 31 forks source link

Add support for polling 'alert' data from state data #110

Closed brandond closed 5 years ago

brandond commented 5 years ago

This is the only way to detect some things like code use (which code was used to unlock the door) and PIN pad lockouts due to excessive failed attempts.

pavoni commented 5 years ago

This all looks good to me, although I found this code fragile and hard to test.

I think at the time I had an HA framework that allowed me to run test versions of this code live - but I no longer have that because I'm not as active with HA as I was. Afraid I don't have the time to rebuilt the test environment at the moment.

So am happy to merge if you're confidant that it works and is well tested (and you're willing to sort out issues if we break anything)

jwater7 commented 5 years ago

@pavoni If it were me, I'd say merge in https://github.com/pavoni/pyvera/pull/93 and then to minimize regressions we can have new PRs write some unit tests to cover all their new stuff (and use that as a model for all new changes since you're limited now on the project).

pavoni commented 5 years ago

@brandond I've merged in the long standing lock PR as suggested by @jwater7, has resulted in a small conflict - could you rebase and resolve? Sorry to have had this in such a low volume repo.

Also let me know your thoughts on testing, and then we can perhaps merge / plan how to release.

brandond commented 5 years ago

I agree that this codebase is kinda hard to wrangle. The style is weird and it uses a worker thread where native py3 code would probably go async instead. I was considering running it through a formatter and flake8, but figured I'd try to get the actual thing I wanted in first rather than just show up unannounced with a huge patch.

I'm glad to try to contribute some tests if possible. I have a VeraEdge with two locks on it, so that's all I was able to test locally by actually banging on the API.

brandond commented 5 years ago

FWIW, what I was hoping to accomplish here is to be able to send a notification in HA when someone inputs a bad PIN. I discovered that (on my Kwikset locks at least) the PIN failed alert doesn't actually fire until you've failed three times AND it finishes beeping loudly for about 10 seconds and goes into temporary pad lockout.

However, if all you're looking at is the lu_sdata api, the devicedata field is still empty (because the actual device status hasn't changed - it's still locked) so no callbacks were fired. You have to actually see that the dataversion changed and then call the status api and parse the alerts to reveal that something happened.

pavoni commented 5 years ago

Thanks @brandond, what you've done is well written - you clearly know what you're doing, and great to hear you have managed to test it in a live environment.

Unless @jwater7 disagrees - I'll also merge this and release a new version. I'll find way to check it still work with my lights dimmers etc.

And would be great to have some decent tests - but I think it's quite a big undertaking!

jwater7 commented 5 years ago

Sounds great! Nice work

pavoni commented 5 years ago

Now released in 0.3.1.