enwi / hueplusplus

A simple C++ library to control Philips Hue lights on Linux, MacOS, Windows, Espressif ESP32 SDK and Arduino. Full documentation at
https://enwi.github.io/hueplusplus/
GNU Lesser General Public License v3.0
55 stars 22 forks source link

Missing support for new Bluetooth (Gen4) lights #58

Closed LiSongMWO closed 4 years ago

LiSongMWO commented 4 years ago

I got a new Hue light yesterday with bluetooth support (gen4) with the identifier LTA001. This caused hueplusplus to crash on enumerating the lights with:

Oct 28 11:49:37 raspberrypi photon-hue-service[24287]: terminate called after throwing an instance of 'std::runtime_error'
Oct 28 11:49:37 raspberrypi photon-hue-service[24287]:   what():  Could not determine HueLight type!

This is caused by the string "LTA001" missing from the check here:

https://github.com/enwi/hueplusplus/blob/83d4883211e61a66867f5736a96c667652ce333e/hueplusplus/Hue.cpp#L277

Presumably there are other new lights that are also missing that I don't know of.

In general I believe that the approach of enumerating all product IDs here to determine their capabilities is going to cause this class on sins of omission to continue appearing as new products are created by Philips. Is there no way to query the lights to get their capabilities from the API so that a hard coded list like this isn't needed?

Jojo-1000 commented 4 years ago

There is, and it is implemented on the development branch along with a bunch of other changes.

You can look at #57 for more detail, TL;DR:

I would be especially interested in feedback on the color conversion. Maybe it needs to be rolled back to only affect XY and leave brightness alone.

enwi commented 4 years ago

@EmilyBjoerk Can you confirm that the development branch is working for you?

LiSongMWO commented 4 years ago

@enwi I'm using my fork that I've locked at an older commit with some of my fixes on. I don't currently have the time to rework my code to work with the breaking changes in the development branch. So unfortunately I haven't tested and will unlikely have the time to test. (I've applied a fix to my local fork for the light that I needed already).

enwi commented 4 years ago

Okay then I will close this issue.