Update ZigbeeColorDimmableLight to clamp color hue and saturation to 0-254 (Fixes #11527)

[thorrak]

Description of Change

The Zigbee standard requires color saturation in the range of 0-254, but espRgbColorToHsvColor returns saturations in the range of 0-255. As referenced in #11527 when a full saturation color is set, the following error is received:

[  6303][E][ZigbeeColorDimmableLight.cpp:180] setLight(): Failed to set light saturation: 0x87: Invalid value

This PR clamps the range of the saturation used in setLight() to 0-254 (reducing it to 254 from 255 when full saturation is set), eliminating the "Invalid value" error.

Tests scenarios

I have tested this change on my XIAO ESP32-C6

Related links

Fixes #11527

[CLAassistant]

All committers have signed the CLA.

[github-actions]

Warnings

⚠️

Some issues found for the commit messages in this PR: the commit message "Clamp Zigbee color saturation to 0-254": probably contains Jira ticket reference (0-254). Please remove Jira tickets from commit messages. summary looks empty type/action looks empty the commit message "Clamp hue to 0-254 for Zigbee color lights": probably contains Jira ticket reference (0-254). Please remove Jira tickets from commit messages. summary looks empty type/action looks empty the commit message "Use std::min instead of ternary operator": summary looks empty type/action looks empty Please fix these commit messages - here are some basic tips: follow Conventional Commits style correct format of commit message should be: <type/action>(<scope/component>): <summary>, for example fix(esp32): Fixed startup timeout issue allowed types are: change,ci,docs,feat,fix,refactor,remove,revert,test sufficiently descriptive message summary should be between 10 to 72 characters and start with upper case letter avoid Jira references in commit messages (unavailable/irrelevant for our customers) TIP: Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

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

---

📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

---

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

---

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against 020aedd

[thorrak]

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).

In practice, 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.

[thorrak]

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.

[P-R-O-C-H-Y]

@thorrak This is what cluster specification says:

[github-actions]

Test Results

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

Results for commit 020aedd.

[github-actions]

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.

Memory	FLASH [bytes]		FLASH [%]		RAM [bytes]		RAM [%]
Target	DEC	INC	DEC	INC	DEC	INC	DEC	INC
ESP32S3	0	0	0.00	0.00	0	0	0.00	0.00
ESP32S2	0	0	0.00	0.00	0	0	0.00	0.00
ESP32C3	0	0	0.00	0.00	0	0	0.00	0.00
ESP32C6	0	0	0.00	0.00	0	0	0.00	0.00
ESP32H2	0	0	0.00	0.00	0	0	0.00	0.00
ESP32	0	0	0.00	0.00	0	0	0.00	0.00

Click to expand the detailed deltas report [usage change in BYTES]

Target	ESP32S3		ESP32S2		ESP32C3		ESP32C6		ESP32H2		ESP32
Example	FLASH	RAM	FLASH	RAM	FLASH	RAM	FLASH	RAM	FLASH	RAM	FLASH	RAM
libraries/Zigbee/examples/Zigbee_Analog_Input_Output	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Color_Dimmer_Switch	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Electrical_AC_Sensor	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Electrical_AC_Sensor_MultiPhase	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Gateway	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_On_Off_Switch	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Range_Extender	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Thermostat	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Binary_Input	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_CarbonDioxide_Sensor	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Color_Dimmable_Light	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Contact_Switch	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Dimmable_Light	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Electrical_DC_Sensor	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Illuminance_Sensor	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_OTA_Client	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Occupancy_Sensor	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_On_Off_Light	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_On_Off_MultiSwitch	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_PM25_Sensor	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Power_Outlet	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Pressure_Flow_Sensor	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Scan_Networks	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Temp_Hum_Sensor_Sleepy	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Temperature_Sensor	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Vibration_Sensor	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Wind_Speed_Sensor	-	-	-	-	-	-	-	-	-	-	-	-
libraries/Zigbee/examples/Zigbee_Window_Covering	-	-	-	-	-	-	-	-	-	-	-	-

[thorrak]

@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

[thorrak]

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.

[P-R-O-C-H-Y]

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

[P-R-O-C-H-Y]

LGTM. As the Hue we use is 0-255 it can be just like this truncating the 255 to 254.