[hue] Fix edge cases for broken lights (API v2)

[andrewfg]

Resolves #15995

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

[andrewfg]

@jlaur here is the promised PR

It probably makes most sense if you review the test cases first, to make sure that they do address all of your actual edge cases.

Note: we had discussed adding a patch method to the thing handler for the case of things that don't report a minimum dimming level. However this is not feasible since such a patch would also add a a minimum dimming level to an On/Off only lamp, which would cause a brightness channel to wrongly be created when such a thing is discovered.

[jlaur]

This looks like exactly the same fix as provided in 52ea4f2 except that you use minimum dimming level rather than 100. Correct?

[andrewfg]

This looks like exactly the same fix as provided in 52ea4f2 except that you use minimum dimming level rather than 100. Correct?

Yes indeed. So it matches the Hue app.

[jlaur]

@andrewfg - the fix looks good to me. Besides the change of 100 to minimum dimming in comparison with my original fix, any brightness outside of the [minimum dimming : 100] interval will now be corrected to minimum dimming. Just to be sure, is that intentional? It does make some sense, just double-checking.

Regarding the test coverage, can I kindly ask you to bring back the original unit tests provided in 52ea4f2? They are more concise and readable without the distraction of deserialization which is already covered elsewhere. They also use the constraint-based assertion model for readability. I have adapted them for testing the new version of the fix:

/**
 * Copyright (c) 2010-2023 Contributors to the openHAB project
 *
 * See the NOTICE file(s) distributed with this work for additional
 * information.
 *
 * This program and the accompanying materials are made available under the
 * terms of the Eclipse Public License 2.0 which is available at
 * http://www.eclipse.org/legal/epl-2.0
 *
 * SPDX-License-Identifier: EPL-2.0
 */
package org.openhab.binding.hue.internal.clip2;

import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;

import java.math.BigDecimal;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.junit.jupiter.api.Test;
import org.openhab.binding.hue.internal.api.dto.clip2.Dimming;
import org.openhab.binding.hue.internal.api.dto.clip2.OnState;
import org.openhab.binding.hue.internal.api.dto.clip2.Resource;
import org.openhab.binding.hue.internal.api.dto.clip2.enums.ResourceType;
import org.openhab.core.library.types.PercentType;
import org.openhab.core.types.UnDefType;

/**
 * Tests for {@link Resource}.
 * 
 * @author Jacob Laursen - Initial contribution
 */
@NonNullByDefault
public class ResourceTest {

    @Test
    void getBrightnessStateWhenDimmingMissingReturnNull() {
        assertThat(createLightResource(true, null).getBrightnessState(), is(equalTo(UnDefType.NULL)));
    }

    @Test
    void getBrightnessStateWhenOnAndDimming75ReturnBrightness75() {
        assertThat(createLightResource(true, 75.0).getBrightnessState(), is(equalTo(new PercentType(75))));
    }

    @Test
    void getBrightnessStateWhenOnAndDimming125ReturnBrightness100() {
        assertThat(createLightResource(true, 125.0).getBrightnessState(), is(equalTo(new PercentType(100))));
    }

    @Test
    void getBrightnessStateWhenOffAndDimming100ReturnBrightness0() {
        assertThat(createLightResource(false, 100.0).getBrightnessState(), is(equalTo(new PercentType(0))));
    }

    @Test
    void getBrightnessStateWhenOnStateMissingAndDimming0ReturnMinimumBrightness0() {
        assertThat(createLightResource(null, 0.0).getBrightnessState(),
                is(equalTo(new PercentType(new BigDecimal(Dimming.DEFAULT_MINIMUM_DIMMIMG_LEVEL)))));
    }

    @Test
    void getBrightnessStateWhenOnStateMissingAndDimming100ReturnBrightness100() {
        assertThat(createLightResource(null, 100.0).getBrightnessState(), is(equalTo(new PercentType(100))));
    }

    @Test
    void getBrightnessStateWhenOnStateMissingAndDimmingMinus1ReturnMinimumBrightness() {
        assertThat(createLightResource(null, -1.0).getBrightnessState(),
                is(equalTo(new PercentType(new BigDecimal(Dimming.DEFAULT_MINIMUM_DIMMIMG_LEVEL)))));
    }

    @Test
    void getBrightnessStateWhenOnAndDimmingMinus1ReturnMinimumBrightness() {
        assertThat(createLightResource(true, -1.0).getBrightnessState(),
                is(equalTo(new PercentType(new BigDecimal(Dimming.DEFAULT_MINIMUM_DIMMIMG_LEVEL)))));
    }

    @Test
    void getBrightnessStateWhenOnAndDimming0ReturnMinimumBrightness() {
        assertThat(createLightResource(true, 0.0).getBrightnessState(),
                is(equalTo(new PercentType(new BigDecimal(Dimming.DEFAULT_MINIMUM_DIMMIMG_LEVEL)))));
    }

    @Test
    void getBrightnessStateWhenOnAndDimming0ReturnCustomMinimumBrightness() {
        assertThat(createLightResource(true, 0.0, 2.0).getBrightnessState(), is(equalTo(new PercentType(2))));
    }

    private Resource createLightResource(@Nullable Boolean on, @Nullable Double brightness) {
        return createLightResource(on, brightness, null);
    }

    private Resource createLightResource(@Nullable Boolean on, @Nullable Double brightness,
            @Nullable Double minimumDimmingLevel) {
        Resource resource = new Resource(ResourceType.LIGHT);

        if (on != null) {
            OnState onState = new OnState();
            onState.setOn(on);
            resource.setOnState(onState);
        }

        if (brightness != null) {
            Dimming dimming = new Dimming();
            dimming.setBrightness(brightness);

            if (minimumDimmingLevel != null) {
                dimming.setMinimumDimmingLevel(minimumDimmingLevel);
            }

            resource.setDimming(dimming);
        }

        return resource;
    }
}

[andrewfg]

any brightness outside of the [minimum dimming : 100] interval will now be corrected to minimum dimming

Actually anything above 100 is reduced to 100

[andrewfg]

bring back the original unit tests

Ok. I will do that.

[jlaur]

any brightness outside of the [minimum dimming : 100] interval will now be corrected to minimum dimming

Actually anything above 100 is reduced to 100

Yes, of course. 🙂 That was also the case previously, whereas now brightness 0 will be corrected to at least 0.5.

[jlaur]

Thanks for the changes. Two minor comments, otherwise LGTM.

[jlaur]

LGTM