raulgbcr / lednetwf_ble

Home Assistant custom integration for LEDnetWF devices
MIT License
24 stars 2 forks source link

Add support for live status updates #6

Closed 8none1 closed 11 months ago

8none1 commented 11 months ago

This PR adds support for live, near-real-time updates from the LED controller. As remote control buttons are pressed the colour changes, colour mode and/or selected effect is sent back in to Home Assistant and reflected in the UI.

Supported updates:

I've tested this on my devices at home and as far as I can tell it all works fine. But, it could probably use some more testing before a release is published.

Additionally I have made some changes to the way in which devices are shown when discovered. The user can no longer choose to give the device a name during discovery. Instead the name is generated automatically from the name presented by the LED controller. e.g. LEDnetWF0011DEADBEEF. If the user wants to rename the device to something more appropriate they can still do this from the normal HA UI. This somewhat simplifies the discovery and addition of new devices and brings things more in line with what LED BLE does.

I'd like to land this change before adding the repo to HACS.

8none1 commented 11 months ago

Oh, something worth mentioning... you only get updates once the device is connected over BLE. If the connection times out updates are dropped until it reconnects. On first addition of a new device the connection is dropped once it has been added, and so you need to turn the light on (or off) via the HA UI for a new connection to be established and updates to work.

raulgbcr commented 11 months ago

Looks really great! Good Job!!!

Not really understanding the sleep on the update function, shouldn't HA process that on its own already? Also the stop_notify() on the disconnect function should not be necessary yeah, but doesn't really hurt at the moment I guess, feel free to remove it if you wish

Another thing we may have to look up for the future is the per device config of effects, right now all the "suported" features of a device are defined as a 1 element array on lednetwf.py since this comes from https://github.com/dave-code-ruiz/elkbledom which supports various models of devices, every one with its own characteristics and capabilities. We could just drop that entirely and only just support 1 model or leave it so it can support other models in the future if anything changes, which means that we should map all the effects to an array to be used on each model of device. Not sure if its really worth it tbh.

8none1 commented 11 months ago

Not really understanding the sleep on the update function, shouldn't HA process that on its own already? It's a hack :)

If we send the "initial packet" that causes the LED controller to send back a status packet which then means the UI is syncronised with the light. But... if you send the initial packet as part of the start up (which HA does) then as soon as it has completed we disconnect and the notification is not received, and so the current state isn't updated.

What we should really do is wait until the callback has triggered before disconnecting. But I couldn't work out how to do. So I hacked it.

Also the stop_notify() on the disconnect function should not be necessary yeah, but doesn't really hurt at the moment I guess, feel free to remove it if you wish

I'll play with it in the future, for now it's OK I think.

Another thing we may have to look up for the future is the per device config of effects, right now all the "suported" features of a device are defined as a 1 element array on lednetwf.py since this comes from https://github.com/dave-code-ruiz/elkbledom which supports various models of devices, every one with its own characteristics and capabilities. We could just drop that entirely and only just support 1 model or leave it so it can support other models in the future if anything changes, which means that we should map all the effects to an array to be used on each model of device. Not sure if its really worth it tbh.

Yeah, agreed. I don't know of any other models at the moment. I have three lights and they are all exactly the same, but I have seen on Aliexpress there are similar looking lights with different remotes. Maybe they are a later model? Easy enough to change in the future. Perhaps when we get in to HACS more people will try it and report back.

Thanks for the review!

raulgbcr commented 11 months ago

If we send the "initial packet" that causes the LED controller to send back a status packet which then means the UI is syncronised with the light. But... if you send the initial packet as part of the start up (which HA does) then as soon as it has completed we disconnect and the notification is not received, and so the current state isn't updated.

What we should really do is wait until the callback has triggered before disconnecting. But I couldn't work out how to do. So I hacked it.

Ohh, good one! :_)

Yeah, agreed. I don't know of any other models at the moment. I have three lights and they are all exactly the same, but I have seen on Aliexpress there are similar looking lights with different remotes. Maybe they are a later model? Easy enough to change in the future. Perhaps when we get in to HACS more people will try it and report back.

Feel free to ping me if you need anything with that!! I really don't know how much to say thanks for the work you put on this, I just put this together rather quickly for personal use and really didn't expected it to move it this quickly on my own, so thanks again ;_;

8none1 commented 11 months ago

just put this together rather quickly for personal use

Well, I did the reverse engineering hoping that I could add it to HA, but I couldn't work out how to do it, so you helped me get further than I could on my own. The dream of open source! :tada: