sinricpro / esp8266-esp32-sdk

Library for https://sinric.pro - simple way to connect your device to Alexa, Google Home, SmartThings and cloud
https://sinric.pro
228 stars 122 forks source link

RGB_LED_Stripe_5050.ino example incorrectly works with Google Home #220

Closed shur1k closed 2 years ago

shur1k commented 2 years ago

RGB_LED_Stripe_5050.ino example incorrectly works with Google Home. Some of near white colours are handled by Sinric as a color temperature change, not the colour itself. So, we need to add the color temperature change handler here:

.............
bool onColorTemperature(const String &deviceId, int &colorTemperature) {
  switch (colorTemperature) {
    case 2000:
      device_state.color.r = 255;
      device_state.color.g = 138;
      device_state.color.b = 18;
      break;
    case 2700:
      device_state.color.r = 255;
      device_state.color.g = 169;
      device_state.color.b = 87;
      break;
    case 3000:
      device_state.color.r = 255;
      device_state.color.g = 180;
      device_state.color.b = 107;
      break;
    case 5000:
      device_state.color.r = 255;
      device_state.color.g = 228;
      device_state.color.b = 206;
      break;
    case 5500:
      device_state.color.r = 255;
      device_state.color.g = 236;
      device_state.color.b = 224;
      break;
    case 6000:
      device_state.color.r = 255;
      device_state.color.g = 243;
      device_state.color.b = 239;
      break;
    case 6500:
      device_state.color.r = 255;
      device_state.color.g = 249;
      device_state.color.b = 253;
      break;
    case 7500:
      device_state.color.r = 235;
      device_state.color.g = 238;
      device_state.color.b = 255;
      break;
    case 9000:
      device_state.color.r = 214;
      device_state.color.g = 225;
      device_state.color.b = 255;
      break;
    default:
      device_state.color.r = 255;
      device_state.color.g = 255;
      device_state.color.b = 255;
      break;
  }
  setStripe(); 
  return true;
}
.........
myLight.onColorTemperature(onColorTemperature);   // set Color temperature callback

Can't create the PR unfortunately.

sivar2311 commented 2 years ago

Hello @shur1k shur1k! Thank you for your contribution. I will take this as an opportunity to revise the entire example, since other functions have not yet been implemented.

For the sake of completeness: Alexa uses the color temperatures 2200, 2700, 4000, 5000 and 7000. So I am still missing the RGB values for 2200, 4000 and 7000.

Can you give me these values or your source for converting the color temperature to RGB values?

I found a source for converting color temperatures to RGB values and it looks like it uses the same values as you.

sivar2311 commented 2 years ago

I have finished the new example.

It would be very nice if you could test it before I add it to the library.

shur1k commented 2 years ago

Thank you for a quick response @sivar2311 . Yep, I took the values from the link you've posted. I'll test the new example tomorrow and ;et you know. Thank you once again!

sivar2311 commented 2 years ago

Hello!

I have changed the example once again.

Please check with the latest update.

shur1k commented 2 years ago

Hello. Thank you. Sorry, but I have comments. #include <ESP8266WiFi.h> - i think this is obsolete. I believe this import is already done inside the SinricPro library. And you also have a redundant comment // Change baudrate to your need for serial log (it's added twice)

shur1k commented 2 years ago

And the #include <Arduino.h> import can be removed also. I removed the mentioned imports and everything looks good. And seems to work correctly. At least with Google Home.

sivar2311 commented 2 years ago

Hello and thanks for the testing and feedback.

Yes the (respectively) required WiFi library is already included by the SDK. But it doesn't hurt because it will be included only once. Removing the WiFi library would tend to confuse most users.

I will adjust the comment regarding the baud rate.

The include statement for Arduino.h is necessary if you don't work with ArduinoIDE but e.g. with PlatformIO.

shur1k commented 2 years ago

Thanks for the explanation @sivar2311 !

sivar2311 commented 2 years ago

The example was changed: a2bd364