[LightModel] Add RGBW and RGBCW "no brightness" modes

[andrewfg]

Until now the LightModel used to support just three RGB modes as follows:

1. DEFAULT (aka RGB)
2. RGB_NO_BRIGHTNESS
3. RGBW
4. RGBCW

The RGB_NO_BRIGHTNESS is a variant of the default RGB mode in which the color components only act on the HS-part of HSB values. Until now there were no equivalent such variants for RGBW or RGBCW modes. This PR adds support for two additional modes as follows:

5. RGB_W_NO_BRIGHTNESS (this is to RGBW what RGB_NO_BRIGHTNESS is to RGB)
6. RGB_C_W_NO_BRIGHTNESS (this is to RGBCW what RGB_NO_BRIGHTNESS is to RGB)

This PR adds new functionality only. There is no impact on bindings already using the existing code.

This PR also changes the criterion for the light to be ON from brightness >= minimum to brightness > minimum. This is to align with OH best practice as described here

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

[Copilot]

Pull request overview

Adds two new “no brightness” RGB data modes to LightModel so RGBW and RGBCW lights can ignore the brightness component in HSB-derived RGB calculations, analogous to the existing RGB_NO_BRIGHTNESS mode.

Changes:

• Extends RgbDataType with RGB_W_NO_BRIGHTNESS and RGB_C_W_NO_BRIGHTNESS.
• Introduces helper Set groupings to simplify “no brightness” and “has white channel(s)” condition checks.
• Updates getRGBx() and setRGBx() logic to apply “no brightness” behavior across the new modes.

Suppressed comments (1)

bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java:1075

• setRGBx() Javadoc still describes the "brightness not changed" behavior only for RGB_NO_BRIGHTNESS, but the implementation now applies it to RGB_W_NO_BRIGHTNESS and RGB_C_W_NO_BRIGHTNESS via NO_BRIGHTNESS_TYPES. Update the documentation so it matches the new behavior.

        if (rgbxParameter.length < 3 || (RGB_W_TYPES.contains(rgbDataType) && rgbxParameter.length < 4)
                || (RGB_C_W_TYPES.contains(rgbDataType) && rgbxParameter.length < 5)) {
            throw new IllegalArgumentException("Too few arguments in RGBx array");

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (4)

bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java:766

• The Javadoc list is not properly closed (<ul> is opened but the closing tag is also <ul>). This breaks generated Javadoc HTML and makes the comment harder to read.

This issue also appears on line 1055 of the same file.

     * <ul>
     * <li>'RGB_NO_BRIGHTNESS', 'RGB_W_NO_BRIGHTNESS', 'RGB_C_W_NO_BRIGHTNESS': The return result does not depend on
     * the current brightness. In other words the values only relate to the 'HS' part of the {@link HSBType} state.
     * Note: this means that in this case a round trip of setRGBx() followed by getRGBx() will NOT necessarily contain
     * identical values, although the RGB ratios will certainly be the same.</li>
     *
     * <li>All other values of {@link #rgbDataType}: The return result depends on the current brightness. In other
     * words the values relate to all the 'HSB' parts of the {@link HSBType} state.</li>
     * <ul>

bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java:1064

• The Javadoc list is not properly closed (<ul> is opened but the closing tag is also <ul>). This breaks generated Javadoc HTML and makes the comment harder to read.

     * <ul>
     * <li>'RGB_NO_BRIGHTNESS', 'RGB_W_NO_BRIGHTNESS', 'RGB_C_W_NO_BRIGHTNESS': The set value does not affect the
     * current brightness. In other words the values only relate to the 'HS' part of the {@link HSBType} state. Note:
     * this means that in this case a round trip of 'setRGBx()' followed by 'getRGBx()' will NOT necessarily contain
     * identical values, although the RGB ratios will certainly be the same.</li>
     *
     * <li>All other values of {@link #rgbDataType}: both [255,0,0] and [127.5,0,0] change the color to RED and the
     * former changes the brightness to 100 percent, whereas the latter changes it to 50 percent. In other words the
     * values relate to all the 'HSB' parts of the {@link HSBType} state.</li>
     * <ul>

bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java:749

• getOnOff(..) now treats brightness equal to minimumOnBrightness as OFF (>), but internal state handling (zHandleBrightness) uses >= to treat the same brightness as ON. This makes the derived on/off state inconsistent at the threshold (e.g., brightness 1.0 with default minimumOnBrightness), and can flip ON→OFF unexpectedly.

    public synchronized @Nullable OnOffType getOnOff(boolean forceChannelVisible) {
        return (!lightCapabilities.supportsColor() && !lightCapabilities.supportsBrightness()) || forceChannelVisible
                ? OnOffType.from(cachedHSB.getBrightness().doubleValue() > minimumOnBrightness)
                : null;

bundles/org.openhab.core/src/main/java/org/openhab/core/util/LightModel.java:245

• RGB_C_W_NO_BRIGHTNESS uses a plain block comment (/* ... */) while the surrounding enum constants use Javadoc (/** ... */). If Javadoc generation/checkstyle is in use, this can create an inconsistency or warning and the description won’t show up in generated docs.

        RGB_C_W,
        /** as RGB_W but ignores brightness (i.e. only HS parts of HSBType) */
        RGB_W_NO_BRIGHTNESS,
        /* as RGB_C_W but ignores brightness (i.e. only HS parts of HSBType) */
        RGB_C_W_NO_BRIGHTNESS

[wborn]

Thanks for adding the RGBW and RGBCW no-brightness modes. I found two remaining paths in WHITE_ONLY mode where these modes still depend on or modify brightness. The change allowing a zero minimum brightness also leaves two existing tests failing.

Please see the inline comments below.

[wborn]

Thanks for addressing the previous feedback. The implementation changes now appear to handle the no-brightness modes correctly in WHITE_ONLY, and the minimum-brightness tests have been updated.

I found two remaining gaps in the newly added tests. In their current form, the tests would still pass with the previous buggy implementation, so they do not yet protect these fixes against regression.

Please see the inline comments below.

[wborn]

Thanks for addressing the feedback. The updated tests now exercise the actual regression cases and would fail against the previous implementation.

Everything looks good to me now. 👍