sbidy / pywizlight

A python connector for WiZ devices
MIT License
463 stars 79 forks source link

Add push update support #103

Closed bdraco closed 2 years ago

bdraco commented 2 years ago

This could use some additional testing with the Home Assistant PR as I'm out of ways to try to get this to break it. Also a bit tired as its late here, but it seems like its good to go.

I tested this on MacOS and Linux with 2 Home Assistant installs, and iOS device, and and Android device all getting the updates and while it was a bit chatty, it was still near instant at reporting state changes.

It does have to listen on port 38900 so only one Home Assistant instance per IP can get the push updates (its using a singleton to coordinate all the bulbs from the single listener), but most people don't run multiple Home Assistant installs on the same server so thats probably not an issue, but I did test that it falls back to polling if it can't listen on port 38900

sbidy commented 2 years ago

Ok, thank you! I'll take it an test it out in my enviroment to have a addtional test case. Mabye the pytest should be extended with some unit test too.

bdraco commented 2 years ago

Mabye the pytest should be extended with some unit test too.

I ordered 7 different devices types which are showing up this week. After I'm able to do some testing with them, I'm planning on working on a mock_push_bulb and mock_poll_bulb (when it has to fallback) pytest fixtures.

In the mean time, I'll work on getting the firmware version exported to Home Assistant's device registry so if there are any differences between firmware versions it will be easier to catalog. I'll try to do a turn of that later today, and then it would be good to get a new PyPI published so I can clear the outstanding PRs on the Home Assistant side.

sbidy commented 2 years ago

I'll try to make a release in the next 2h

sbidy commented 2 years ago

Release done with version 0.5.1

bdraco commented 2 years ago

Thanks https://github.com/sbidy/pywizlight/pull/104 should be ready for merge and release as well when you have a moment.

I'll do another PR to up the test coverage once more of the test devices show

sbidy commented 2 years ago

Merged - do you need a release for that?

bdraco commented 2 years ago

Yes

sbidy commented 2 years ago

Yes

Done - 0.5.2

bdraco commented 2 years ago

Thanks. Updated https://github.com/home-assistant/core/pull/66017 with it