sbidy / pywizlight

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

Three channel RGB not displaying correctly #90

Open coolmule0 opened 3 years ago

coolmule0 commented 3 years ago

Using an RGB with three distinct non-zero values does not show up correctly on the bulb.

For example using the command light.turn_on(PilotBuilder(rgb = (128, 128, 255)))

coolmule0 commented 3 years ago

The values of state.get_rgb() always return one of the rgb channels as zero when using the app and 'custom color'. I guess this is to be expected for a bulb that cannot show greys and blacks, hence lacking a channel from the HSV spectrum (only HV).

However, state.get_rgb() will return the exact rgb sent from the API, even if it is not shown correctly.

I am wondering how best to convert the 3 channel rgb into its nearest 2-channel rgb?

nalf3in commented 2 years ago

I'm really not a expert in any of that color stuff so take this with a grain of salt, but I'm pretty sure I found the bug !

In the PilotBuilder class(bulb.py), the _set_rgb method, needs to convert the 3 channel rgb into its nearest 2-channel rgb (while still accounting for the white led) like mentionned by @coolmule0. This seems to be already taken care of by the rgb2rgbcw(values) function but the code only use the return value for cw (rgb_out is also returned and never used). Using rgb_out to set the rgb values sent to the bulb fixes the problem.

Example of the fixed _set_rgb method:

import asyncio
from typing import Tuple

from pywizlight.rgbcw import hs2rgbcw, rgb2rgbcw
from pywizlight import wizlight, PilotBuilder, discovery

def custom_set_rgb(self, values: Tuple[float, float, float]) -> None:
    """Set the RGB color state of the bulb."""

    # Setup the tuples for the RGB values
    red, green, blue = values

    if not (0 <= red < 256):
        raise ValueError("Red is not in range between 0-255.")
    if not (0 <= green < 256):
        raise ValueError("Green is not in range between 0-255.")
    if not (0 <= blue < 256):
        raise ValueError("Blue is not in range between 0-255.")

    # We change the red, green, blue values to account for the separate white light
    rgb_out, cw = rgb2rgbcw(values)
    red,green,blue = rgb_out

    self.pilot_params["r"] = red
    self.pilot_params["g"] = green
    self.pilot_params["b"] = blue

    # No CW because of full RGB color
    if cw is not None:
        # Use the existing set_warm_white function to set the CW values
        self._set_warm_white(cw)

        # The next line is commented because the mobile app never seems to set the
        # cold white value with it's own color picker and it looks better to me without it 

        #self._set_cold_white(cw)

async def main():

    r,g,b = 128,128,255

    light = wizlight("192.168.1.20")

    builder = PilotBuilder()
    builder._set_rgb = custom_set_rgb
    builder._set_rgb(builder, (r, g, b))

    await light.turn_on(builder)
    # Uncomment the following for a direct comparison with the current code
    #await light.turn_on(PilotBuilder(rgb= (r,g,b)))

    state = await light.updateState()
    # Print the values returned by the bulb
    red, green, blue = state.get_rgb()
    print(f"red {red}, green {green}, blue {blue}")
    print(f"Warm white: {state.get_warm_white()}")
    print(f"Cold white: {state.get_cold_white()}")
    print(f"Color temp: {state.get_colortemp()}")
    print(f"Extended white range: {state.get_extended_white_range()}")
    print(f"Brightness: {state.get_brightness()}")

loop = asyncio.get_event_loop()
loop.run_until_complete(main())

I also commented self._set_cold_white(cw) because the mobile app seems to only be using warm_white with it's color picker and the result seems better to me but I could be wrong

sbidy commented 2 years ago

Seems to be true. Thank you for the hint. I'll test it and made the change for the next version.

P.S.: Don't hesitate to open a PR if you find a bug next time 😉

rgomezjnr commented 2 years ago

Hello pywizlight team, just wanted to share that I updated pywizlight to 0.5.6 and I'm still seeing somewhat off bulb colors with various RGB values. Is it possible 7cdfc12 hasn't fixed this issue that @pierre-guillot and I are experiencing?

PonchoPowers commented 2 years ago

@rgomezjnr rgomezjnr It should be noted that the bulbs don't accurately translate RGB values into what you might expect, is the colour out a bit or a lot?

brettonw commented 2 years ago

@coolmule0 @BonnieSoftware To help you understand what is happening, I suggest you look at the color wheel in the wiz app. If you are used to specifying RGB colors for pixels in a paint program, or for colors in CSS, it is counter-intuitive that RGB(0,0,0) is white, and the color you specified is a desaturated version of blue towards white with no R or G at all. In this app, the RGB color you give is accurately projected to the top surface of the HSV cone, not computed as just the combination of the RGB vectors in a color cube (https://commons.wikimedia.org/wiki/File:HSL_color_solid_dblcone.png). I can't find a good thing on Google in short notice, but if you took the RGB cube and rotated it so that the black to white vector was sticking straight up, then looked straight down on it from the white side, that is what you are getting.