Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PilotBuilder's _set_rgb overrides the 'c' and 'w' values I pass into the PilotBuilder's constructor. #96

Open
EdLaFave opened this issue Jan 26, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@EdLaFave
Copy link

EdLaFave commented Jan 26, 2022

I'm creating a PilotBuilder with the RGB values (255, 173, 0), 0 cold white, and 0 warm white. Then I'm passing the PilotBuilder instance into the turn_on method of the light.

It turns out the PilotBuilder constructor passes my RGB value to _set_rgb, which in turn calls rgb2rgbcw and uses the returned values to set cold white and warm white to non-zero values.

This appears to be very much intentional and so I'm assuming this isn't a bug and you're covering a use case I'm unaware of. Perhaps I'm using the PilotBuilder incorrectly to set the RGB values of my bulb. However, it seemingly would be very nice for the PilotBuilder to maintain the values I passed into the constructor.

This isn't a huge deal because I can work around this by doing this after I create the PilotBuilder:

pilot_builder.pilot_params["w"] = 0
pilot_builder.pilot_params["c"] = 0

@sbidy
Copy link
Owner

sbidy commented Jan 29, 2022

This should set the correct hue/saturation of the color based on the Wiz App color representation.
So, please use your suggested workaround for resting the cw values to get "native" RGB.

@sbidy sbidy self-assigned this Jan 29, 2022
@sbidy sbidy added the wontfix This will not be worked on label Jan 29, 2022
@bdraco
Copy link
Contributor

bdraco commented Feb 9, 2022

There is now functionality to set RGBWW/RGBW. Please use those in the future.

@sbidy sbidy added enhancement New feature or request and removed wontfix This will not be worked on labels Feb 9, 2022
@bdraco
Copy link
Contributor

bdraco commented Feb 11, 2022

This should be closable since there is an alternative option that will work

@brettonw
Copy link

brettonw commented Jun 15, 2022

TLDR: This behavior is by design.

I agree that it is confusing that trying to set an RGB color on the bulb engages the cw component. It took me a bit to wrap my head around it, but it's easiest to understand if you look at the wiz app and the color picker. You expect the bulb to be bright white when the picker is centered, which is (rgb = 0, 0, 0). So... this isn't a classic downward projection of a cube in HSV with black at (rgb = 0, 0, 0).

If you look at my comments in the rgb2rgbcw code, I documented why this happens both from a functional standpoint and from a hardware standpoint.

  1. The bulbs have 5 LEDs in them (RGB, WW, CW). However, the bulbs also have a fixed power budget that cannot support all the LEDs on at full brightness. It looks like only 3 LEDs can be lit at any given time. As a result, all of the color options must be reached with a combination of 2 colors and a white. The Wiz app uses the warmer of the two white LEDs, so I mimicked that behavior (that is why the bulb sets warm white to the cw value).
  2. The bulb behavior in the wiz app is discontinuous in an interesting way that lets you set both color and brightness, because just turning on the 3 RGB LEDs is not very bright. This is how we get white when the color picker is in the middle, and fully saturated color along the edges. See line 183 for an understanding here, but you can think of it like a cone in the HSV space, just the muffin top, so to speak.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants