All committers have signed the CLA.

👋Hello thorrak, we appreciate your contribution to this project!

Generated by 🚫dangerJS againstde332e1

One downside to this fix (and with the Zigbee standard in general) is that there is necessarily a slight loss of information when mapping from a 0-255 saturation range to a 0-254 saturation range. This means that the color value known to Zigbee will always differ from any cached value set in code for colors with 255 saturation.

For example - if I set a color of #FF0000 in code (0H, 255S, 255V), it will get converted to #FF0101 (0H, 254S, 255V).

Inpractice, this may not be as much of an issue, as any color received via Zigbee will already have a maximum saturation value of 254 per the standard, but any color set from code will not be similarly limited.

I have no idea how to handle this other than potentially to log a warning to the console, but in my PR this error will just silently occur.

On a hunch I tested what happens when the hue was set to something that mapped to 255, and - sure enough - saw similar behavior:

[ 57201][E][ZBCDL.cpp:178] setLight(): Failed to set light hue: 0x87: Invalid value

I went ahead and applied the same fix for hue as I did for saturation.

I expect the same issue here as for saturation, unfortunately - any colors that would map to a hue of 255 (if set directly within code) will end up being clamped to a hue of 254. As this is a limitation of the Zigbee standard, I don't know if there is any way to correct for this, however.

@thorrak This is what cluster specification says:

Test Results

 76 files   76 suites   13m 9s ⏱️
 38 tests  38 ✅ 0 💤 0 ❌
241 runs  241 ✅ 0 💤 0 ❌

Results for commitde332e1.

♻️ This comment has been updated with latest results.

Memory usage test (comparing PR against master branch)

The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.

@thorrak This is what cluster specification says

Is your preference then to scale rather than to truncate?Hue254 = (uint8/t) (hue255 / 255 * 254)?

Given we’re dropping one value of 256, I would expect this would just have the effect of doing the following (but with floating point math):

Hue254 = (Hue255>=128) ? (Hue255-1) : Hue255

If either of those are the way you would want to go (compression rather than truncation) I’m guessing we would probably need to expand the value received from the Zigbee network as well. My guess - having not reviewed the code for values received - is that the 0-254 values received from the network aredirectly mapped to the 0-255 space used by the ColorFormat library on the ESP rather than being expanded.

I have not reviewed any of the code dealing with received values however.

I will check and let you know what solution would be the best.