michielpost / Q42.HueApi

C# helper library to talk to the Philips Hue bridge
MIT License
412 stars 114 forks source link

Set Light State Using XY when streaming instead of RGBColor? #202

Closed d8ahazard closed 4 years ago

d8ahazard commented 4 years ago

Hey there, it's me again. :P

I've got a mix of Gen1 and Gen2 bulbs that I'm testing with, and I've noticed that the Gen1 bulb's colors can sometimes appear more washed-out than that of the G2 bulbs.

I'm thinking this is because I'm trying to set the color using the RGB value, versus the XY value, as per their doc.

I see you've got the converter in place for the gamut, but I don't see any reference to it when setting the state.

Is there a way I can properly convert the gamut for my light so that the color appearance is the same? Perhaps an overload for SetState() that allows a gamut input as well?

michielpost commented 4 years ago

I did start working on this some while a go, there's a feature/streaming-xy branch: https://github.com/Q42/Q42.HueApi/tree/feature/streaming-xy

It looked easy to implement at first, but there was something that made it a bit more complex. Not sure anymore what it was. I did not invest any more time in that feature because there's not much need for it.

Is there some way for you to work around it? Like converting the correct XY value for the model back to RGB?

d8ahazard commented 4 years ago

I thought about trying that. Right now I'm doing a refactor so I can actually get the gamut information for the different light types and calculate properly. once I'm finished with that, I'll let you know how it works out.

On Mon, Dec 9, 2019, 2:50 PM Michiel Post notifications@github.com wrote:

I did start working on this some while a go, there's a feature/streaming-xy branch: https://github.com/Q42/Q42.HueApi/tree/feature/streaming-xy

It looked easy to implement at first, but there was something that made it a bit more complex. Not sure anymore what it was. I did not invest any more time in that feature because there's not much need for it.

Is there some way for you to work around it? Like converting the correct XY value for the model back to RGB?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Q42/Q42.HueApi/issues/202?email_source=notifications&email_token=AAMO4NCWYBTQCMRJBEAO6VDQX2VP5A5CNFSM4JWFZFYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGKUVLY#issuecomment-563432111, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMO4NFZWLXQX7I224VQLS3QX2VP5ANCNFSM4JWFZFYA .

d8ahazard commented 4 years ago

So, on further investigation, I'm trying to convert from RGB to XY, but it appears as if HueColorConverter is an internal class? In lieu of allowing setting the state using XY, what about exposing the converter, and maybe adding another method to it that fetches the gamut based on the light model ID.

A rough, ugly approximation of a class to do this would be like so:

`private CIE1931Gamut GetLightGamut(string modelId) { /"""Gets the correct color gamut for the provided model id. Docs: http://www.developers.meethue.com/documentation/supported-lights """/ string[] modelsA = { "LST001", "LLC010", "LLC011", "LLC012", "LLC006", "LLC007", "LLC013"}; string[] modelsB = {"LCT001", "LCT007", "LCT002", "LCT003", "LLM001"}; string[] modelsC = {"LCT010", "LCT014", "LCT011", "LLC020", "LST002"}; if (Array.Exists(modelsA, element => element == modelId)) { return CIE1931Gamut.ModelTypeA; }

        if (Array.Exists(modelsB, element => element == modelId)) {
            return CIE1931Gamut.ModelTypeB;
        }

        if (Array.Exists(modelsC, element => element == modelId)) {
            return CIE1931Gamut.ModelTypeC;
        }

        return CIE1931Gamut.PhilipsWideGamut;
    }`
michielpost commented 4 years ago

Q42.HueApi.ColorConverters and Q42.HueApi.Entertainment 3.14.0 have been submitted to NuGet with your PR changes.

d8ahazard commented 4 years ago

I was thinking about this this morning after some much-needed sleep. I don't know if the conversion to RGB is correct, or if it the Gamut for the light needs to be passed.

I think to modify the value to the "right" RGB values for the light, we need to use a different color space. According to this, we should be using the Wide color space?

https://gist.github.com/popcorn245/30afa0f98eea1c2fd34d

michielpost commented 4 years ago

@d8ahazard I'm closing this issue now. Let me know if you need changes.