[Neopixel] HSV2RGBW fix

[chromoxdor]

Just when I thought that ESPEasy has been running smoothly for quite a while now, I stumbled across this... 🙂

The colourwheel conversion didn't work as expected, because the original code wasn't adapted correctly.

[TD-er]

So this was the only bug? (and line 426 of course)
Why change the defined values ? Are their int values '1' and '2' somewhere else used as integer values? (or stored in settings)

[chromoxdor]

I only changed these values to match the changed order of the rgbw values
To use the RGB scheme for the order is actually a bit misleading in my opinion...
(huh.. now i realized i only had to change the order of the order.. nevermind. hard to explain :) )

Are their int values '1' and '2' somewhere else used as integer values? (or stored in settings)

Good question... I don't think so. Probably unnecessary.

[chromoxdor]

we could also use following to save space. I asked a "friend" to optimize it 😬 (i tested it and it works)

void HSV2RGBW(float H, float S, float I, int rgbw[4]) {
    constexpr float d = 3.14159f / 180.0f, a = 120.0f * d, z = 240.0f * d, c = 60.0f * d;
    H = fmod(H, 360) * d;
    S = fmax(0, fmin(S / 100, 1));
    I = fmax(0, fmin(I / 100, 1));
    int o = (H < a) ? 0 : (H < z ? (H -= a, 1) : (H -= z, 2));
    float x = cosf(H), y = cosf(c - H);
    int r = S * I * 85 * (1 + x / y), g = S * I * 85 * (1 + (1 - x / y)), b = 0;
    rgbw[3] = 255 * (1 - S) * I;
    if (!o) rgbw[0] = r, rgbw[1] = g, rgbw[2] = b;
    else if (o == 1) rgbw[0] = b, rgbw[1] = r, rgbw[2] = g;
    else rgbw[0] = g, rgbw[1] = b, rgbw[2] = r;
}

[TD-er]

Not sure if the second suggested code is better as it is much harder to read and I don't like those comma separated assignment oneliners.

[TD-er]

As you can see, removing the defines is not making it better as you literally everywhere have to comment what you mean by it.
So the define is better here as it makes the code better readable.

[tonhuisman]

Some time ago I copied an existing (supposed to do a better conversion) RGB2HSV function to Misc.cpp, so I'm a tad surprised that it's not doing the correct conversion, as nobody mentioned this as an issue, so far... 🤔

Good that it's now being fixed. 😸

[chromoxdor]

as nobody mentioned this as an issue, so far... 🤔

I guess it is something that is rarely used by anyone.

I'm a tad surprised that it's not doing the correct conversion

It almost did, but as it is not a 1:1 copy, there was a bit of a mix-up. 🙂

As you can see, removing the defines is not making it better as you literally everywhere have to comment what you mean by it.

You're right, and I agree that the compressed version doesn't really help in terms of readability.

[chromoxdor]

But i don´t know how i undo the second commit... 🤷‍♂️

[tonhuisman]

But i don´t know how i undo the second commit... 🤷‍♂️

With this few changes, I usually just re-edit the file(s) and commit that.
Or you can use a GUI tool for your local git repo, and revert your commit (most of these tools have features like that), then commit the changes.