[mqtt][tradfri][webthing] Adapt to ColorUtil changes in core (#14810)

* [mqtt][tradfri][webthing] Adapt to ColorUtil changes in core

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>

bundles/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/values/ColorValue.java

@@ -28,6 +28,7 @@ import org.openhab.core.library.types.PercentType;
 import org.openhab.core.library.types.StringType;
 import org.openhab.core.types.Command;
 import org.openhab.core.types.UnDefType;
+import org.openhab.core.util.ColorUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -144,14 +145,12 @@ public class ColorValue extends Value {
                 return String.format(formatPattern, hsbState.getHue().intValue(), hsbState.getSaturation().intValue(),
                         hsbState.getBrightness().intValue());
             case RGB:
-                PercentType[] rgb = hsbState.toRGB();
+                int[] rgb = ColorUtil.hsbToRgb(hsbState);
-                return String.format(formatPattern, rgb[0].toBigDecimal().multiply(factor).intValue(),
+                return String.format(formatPattern, rgb[0], rgb[1], rgb[2]);
-                        rgb[1].toBigDecimal().multiply(factor).intValue(),
-                        rgb[2].toBigDecimal().multiply(factor).intValue());
             case XYY:
-                PercentType[] xyY = hsbState.toXY();
+                double[] xyY = ColorUtil.hsbToXY(hsbState);
-                return String.format(Locale.ROOT, formatPattern, xyY[0].floatValue() / 100.0f,
+                return String.format(Locale.ROOT, formatPattern, xyY[0], xyY[1],
-                        xyY[1].floatValue() / 100.0f, hsbState.getBrightness().floatValue());
+                        hsbState.getBrightness().doubleValue());
             default:
                 throw new NotSupportedException(String.format("Non supported color mode: {}", this.colorMode));
         }

bundles/org.openhab.binding.mqtt.generic/src/test/java/org/openhab/binding/mqtt/generic/ChannelStateTests.java

@@ -14,6 +14,7 @@ package org.openhab.binding.mqtt.generic;
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.number.IsCloseTo.closeTo;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.ArgumentMatchers.any;
@@ -49,11 +50,13 @@ import org.openhab.binding.mqtt.generic.values.PercentageValue;
 import org.openhab.binding.mqtt.generic.values.TextValue;
 import org.openhab.core.io.transport.mqtt.MqttBrokerConnection;
 import org.openhab.core.library.types.HSBType;
+import org.openhab.core.library.types.PercentType;
 import org.openhab.core.library.types.RawType;
 import org.openhab.core.library.types.StringType;
 import org.openhab.core.library.unit.Units;
 import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.types.Command;
+import org.openhab.core.util.ColorUtil;
 
 /**
  * Tests the {@link ChannelState} class.
@@ -247,7 +250,7 @@ public class ChannelStateTests {
 
         c.processMessage("state", "ON".getBytes()); // Normal on state
         assertThat(value.getChannelState().toString(), is("0,0,10"));
-        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("25,25,25"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("26,26,26"));
 
         c.processMessage("state", "FOFF".getBytes()); // Custom off state
         assertThat(value.getChannelState().toString(), is("0,0,0"));
@@ -255,15 +258,14 @@ public class ChannelStateTests {
 
         c.processMessage("state", "10".getBytes()); // Brightness only
         assertThat(value.getChannelState().toString(), is("0,0,10"));
-        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("25,25,25"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("26,26,26"));
 
         HSBType t = HSBType.fromRGB(12, 18, 231);
 
         c.processMessage("state", "12,18,231".getBytes());
         assertThat(value.getChannelState(), is(t)); // HSB
-        // rgb -> hsv -> rgb is quite lossy
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("12,18,231"));
-        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("11,18,232"));
+        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), "%3$d,%2$d,%1$d"), is("231,18,12"));
-        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), "%3$d,%2$d,%1$d"), is("232,18,11"));
     }
 
     @Test
@@ -295,25 +297,39 @@ public class ChannelStateTests {
         ChannelState c = spy(new ChannelState(config, channelUIDMock, value, channelStateUpdateListenerMock));
         c.start(connectionMock, mock(ScheduledExecutorService.class), 100);
 
+        // incoming messages
         c.processMessage("state", "ON".getBytes()); // Normal on state
         assertThat(value.getChannelState().toString(), is("0,0,10"));
-        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0.322700,0.329000,10.00"));
 
         c.processMessage("state", "FOFF".getBytes()); // Custom off state
-        assertThat(value.getChannelState().toString(), is("0,0,0"));
+        // note we don't care what color value is currently stored, just that brightness is off
-        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0.000000,0.000000,0.00"));
+        assertThat(((HSBType) value.getChannelState()).getBrightness(), is(PercentType.ZERO));
 
         c.processMessage("state", "10".getBytes()); // Brightness only
         assertThat(value.getChannelState().toString(), is("0,0,10"));
-        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0.322700,0.329000,10.00"));
-
-        HSBType t = HSBType.fromXY(0.3f, 0.6f);
 
+        HSBType t = ColorUtil.xyToHsb(new double[] { 0.3f, 0.6f });
         c.processMessage("state", "0.3,0.6,100".getBytes());
-        assertThat(value.getChannelState(), is(t)); // HSB
+        assertTrue(((HSBType) value.getChannelState()).closeTo(t, 0.001)); // HSB
-        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), null), is("0.298700,0.601500,100.00"));
+
-        assertThat(value.getMQTTpublishValue((Command) value.getChannelState(), "%3$.1f,%2$.4f,%1$.4f"),
+        // outgoing messages
-                is("100.0,0.6015,0.2987"));
+        // these use the 0.3,0.6,100 from above, but care more about proper formatting of the outgoing message
+        // than about the precise value (since color conversions have happened)
+        assertCloseTo(value.getMQTTpublishValue((Command) value.getChannelState(), null), "0.300000,0.600000,100.00");
+        assertCloseTo(value.getMQTTpublishValue((Command) value.getChannelState(), "%3$.1f,%2$.2f,%1$.2f"),
+                "100.0,0.60,0.30");
+    }
+
+    // also ensures the string elements are the same _length_, i.e. the correct precision for each element
+    private void assertCloseTo(String aString, String bString) {
+        String[] aElements = aString.split(",");
+        String[] bElements = bString.split(",");
+        double[] a = Arrays.stream(aElements).mapToDouble(Double::parseDouble).toArray();
+        double[] b = Arrays.stream(bElements).mapToDouble(Double::parseDouble).toArray();
+        for (int i = 0; i < a.length; i++) {
+            assertThat(aElements[i].length(), is(bElements[i].length()));
+            assertThat(a[i], closeTo(b[i], 0.002));
+        }
     }
 
     @Test

bundles/org.openhab.binding.mqtt.homeassistant/src/test/java/org/openhab/binding/mqtt/homeassistant/internal/component/AbstractComponentTests.java

@@ -41,6 +41,7 @@ import org.openhab.binding.mqtt.homeassistant.internal.HaID;
 import org.openhab.binding.mqtt.homeassistant.internal.HandlerConfiguration;
 import org.openhab.binding.mqtt.homeassistant.internal.config.dto.AbstractChannelConfiguration;
 import org.openhab.binding.mqtt.homeassistant.internal.handler.HomeAssistantThingHandler;
+import org.openhab.core.library.types.HSBType;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatusInfo;
 import org.openhab.core.thing.binding.ThingHandlerCallback;
@@ -173,7 +174,12 @@ public abstract class AbstractComponentTests extends AbstractHomeAssistantTests
     @SuppressWarnings("null")
     protected static void assertState(AbstractComponent<@NonNull ? extends AbstractChannelConfiguration> component,
             String channelId, State state) {
-        assertThat(component.getChannel(channelId).getState().getCache().getChannelState(), is(state));
+        State actualState = component.getChannel(channelId).getState().getCache().getChannelState();
+        if ((actualState instanceof HSBType actualHsb) && (state instanceof HSBType stateHsb)) {
+            assertThat(actualHsb.closeTo(stateHsb, 0.01), is(true));
+        } else {
+            assertThat(actualState, is(state));
+        }
     }
 
     protected void spyOnChannelUpdates(AbstractComponent<@NonNull ? extends AbstractChannelConfiguration> component,

bundles/org.openhab.binding.mqtt.homeassistant/src/test/java/org/openhab/binding/mqtt/homeassistant/internal/component/DefaultSchemaLightTests.java

@@ -137,7 +137,7 @@ public class DefaultSchemaLightTests extends AbstractComponentTests {
 
         // Brightness commands should route to the correct topic, converted to RGB
         sendCommand(component, Light.COLOR_CHANNEL_ID, new PercentType(50));
-        assertPublished("zigbee2mqtt/light/set/rgb", "127,127,127");
+        assertPublished("zigbee2mqtt/light/set/rgb", "128,128,128");
 
         // OnOff commands should route to the correct topic
         sendCommand(component, Light.COLOR_CHANNEL_ID, OnOffType.OFF);

bundles/org.openhab.binding.tradfri/src/test/java/org/openhab/binding/tradfri/internal/TradfriColorTest.java

@@ -36,7 +36,7 @@ public class TradfriColorTest {
         HSBType hsbType = color.getHSB();
         assertNotNull(hsbType);
         assertEquals(312, hsbType.getHue().intValue());
-        assertEquals(92, hsbType.getSaturation().intValue());
+        assertEquals(91, hsbType.getSaturation().intValue());
         assertEquals(100, hsbType.getBrightness().intValue());
     }
 
@@ -48,7 +48,7 @@ public class TradfriColorTest {
         assertEquals(84, (int) color.brightness);
         HSBType hsbType = color.getHSB();
         assertNotNull(hsbType);
-        assertEquals(93, hsbType.getHue().intValue());
+        assertEquals(92, hsbType.getHue().intValue());
         assertEquals(65, hsbType.getSaturation().intValue());
         assertEquals(34, hsbType.getBrightness().intValue());
     }
@@ -61,7 +61,7 @@ public class TradfriColorTest {
         assertEquals(1, (int) color.brightness);
         HSBType hsbType = color.getHSB();
         assertNotNull(hsbType);
-        assertEquals(93, hsbType.getHue().intValue());
+        assertEquals(92, hsbType.getHue().intValue());
         assertEquals(65, hsbType.getSaturation().intValue());
         assertEquals(1, hsbType.getBrightness().intValue());
     }
@@ -75,7 +75,7 @@ public class TradfriColorTest {
         HSBType hsbType = color.getHSB();
         assertNotNull(hsbType);
         assertEquals(156, hsbType.getHue().intValue());
-        assertEquals(77, hsbType.getSaturation().intValue());
+        assertEquals(76, hsbType.getSaturation().intValue());
         assertEquals(72, hsbType.getBrightness().intValue());
     }
 

bundles/org.openhab.binding.webthing/src/test/java/org/openhab/binding/webthing/internal/link/WebthingChannelLinkTest.java

@@ -12,7 +12,7 @@
  */
 package org.openhab.binding.webthing.internal.link;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
 import java.net.URI;
@@ -185,7 +185,12 @@ public class WebthingChannelLinkTest {
         message.data = Map.of(propertyName, initialValue);
         websocketConnectionFactory.webSocketRef.get().sendToClient(message);
 
-        assertEquals(initialState, testWebthingThingHandler.itemState.get(channelUID));
+        Command actualState = testWebthingThingHandler.itemState.get(channelUID);
+        if ((actualState instanceof HSBType actualHsb) && (initialState instanceof HSBType initialStateHsb)) {
+            assertTrue(actualHsb.closeTo(initialStateHsb, 0.01));
+        } else {
+            assertEquals(initialState, actualState);
+        }
 
         ChannelToPropertyLink.establish(testWebthingThingHandler, channel, webthing, propertyName);
         testWebthingThingHandler.listeners.get(channelUID).onItemStateChanged(channelUID, updatedState);