From 5214ab06e6122897f3c62b97c4065ff9234ca37b Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Wed, 27 Nov 2024 13:53:04 +0000 Subject: [PATCH 01/35] [govee] Fix synchronization of brightness Signed-off-by: AndrewFG --- .../binding/govee/internal/GoveeHandler.java | 222 ++++++++++-------- .../GoveeSerializeGoveeHandlerTest.java | 10 +- 2 files changed, 122 insertions(+), 110 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 6c8f51b1ab069..543854c4713a2 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -24,6 +24,7 @@ import org.openhab.binding.govee.internal.model.Color; import org.openhab.binding.govee.internal.model.ColorData; import org.openhab.binding.govee.internal.model.EmptyValueQueryStatusData; +import org.openhab.binding.govee.internal.model.GenericGoveeData; import org.openhab.binding.govee.internal.model.GenericGoveeMsg; import org.openhab.binding.govee.internal.model.GenericGoveeRequest; import org.openhab.binding.govee.internal.model.StatusResponse; @@ -88,10 +89,8 @@ public class GoveeHandler extends BaseThingHandler { private CommunicationManager communicationManager; - private int lastOnOff; - private int lastBrightness; private HSBType lastColor = new HSBType(); - private int lastColorTempInKelvin = COLOR_TEMPERATURE_MIN_VALUE.intValue(); + private int lastKelvin = COLOR_TEMPERATURE_MIN_VALUE.intValue(); /** * This thing related job thingRefreshSender triggers an update to the Govee device. @@ -153,43 +152,41 @@ public void dispose() { @Override public void handleCommand(ChannelUID channelUID, Command command) { try { + logger.debug("handleCommand({}, {})", channelUID, command); if (command instanceof RefreshType) { // we are refreshing all channels at once, as we get all information at the same time triggerDeviceStatusRefresh(); - logger.debug("Triggering Refresh"); } else { - logger.debug("Channel ID {} type {}", channelUID.getId(), command.getClass()); switch (channelUID.getId()) { case CHANNEL_COLOR: - if (command instanceof HSBType hsbCommand) { - int[] rgb = ColorUtil.hsbToRgb(hsbCommand); - sendColor(new Color(rgb[0], rgb[1], rgb[2])); - } else if (command instanceof PercentType percent) { - sendBrightness(percent.intValue()); - } else if (command instanceof OnOffType onOffCommand) { - sendOnOff(onOffCommand); + Command doCommand = command; + if (doCommand instanceof HSBType hsb) { + sendColor(hsb); + doCommand = hsb.getBrightness(); // fall through + } + if (doCommand instanceof PercentType percent) { + sendBrightness(percent); + doCommand = OnOffType.from(percent.intValue() > 0); // fall through + } + if (doCommand instanceof OnOffType onOff) { + sendOnOff(onOff); } break; + case CHANNEL_COLOR_TEMPERATURE: if (command instanceof PercentType percent) { - logger.debug("COLOR_TEMPERATURE: Color Temperature change with Percent Type {}", command); - Double colorTemp = (COLOR_TEMPERATURE_MIN_VALUE + percent.intValue() - * (COLOR_TEMPERATURE_MAX_VALUE - COLOR_TEMPERATURE_MIN_VALUE) / 100.0); - lastColorTempInKelvin = colorTemp.intValue(); - logger.debug("lastColorTempInKelvin {}", lastColorTempInKelvin); - sendColorTemp(lastColorTempInKelvin); + sendKelvin(percentToKelvin(percent)); } break; + case CHANNEL_COLOR_TEMPERATURE_ABS: - if (command instanceof QuantityType quantity) { - logger.debug("Color Temperature Absolute change with Percent Type {}", command); - lastColorTempInKelvin = quantity.intValue(); - logger.debug("COLOR_TEMPERATURE_ABS: lastColorTempInKelvin {}", lastColorTempInKelvin); - int lastColorTempInPercent = ((Double) ((lastColorTempInKelvin - - COLOR_TEMPERATURE_MIN_VALUE) - / (COLOR_TEMPERATURE_MAX_VALUE - COLOR_TEMPERATURE_MIN_VALUE) * 100.0)).intValue(); - logger.debug("computed lastColorTempInPercent {}", lastColorTempInPercent); - sendColorTemp(lastColorTempInKelvin); + if (command instanceof QuantityType genericQuantity) { + QuantityType kelvin = genericQuantity.toInvertibleUnit(Units.KELVIN); + if (kelvin == null) { + logger.warn("handleCommand() invalid QuantityType:{}", genericQuantity); + break; + } + sendKelvin(kelvin.intValue()); } break; } @@ -203,62 +200,73 @@ public void handleCommand(ChannelUID channelUID, Command command) { } /** - * Initiate a refresh to our thing devicee - * + * Initiate a refresh to our thing device */ private void triggerDeviceStatusRefresh() throws IOException { logger.debug("trigger Refresh Status of device {}", thing.getLabel()); - GenericGoveeRequest lightQuery = new GenericGoveeRequest( - new GenericGoveeMsg("devStatus", new EmptyValueQueryStatusData())); - communicationManager.sendRequest(this, lightQuery); + GenericGoveeData data = new EmptyValueQueryStatusData(); + GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("devStatus", data)); + communicationManager.sendRequest(this, request); } - public void sendColor(Color color) throws IOException { - lastColor = ColorUtil.rgbToHsb(new int[] { color.r(), color.g(), color.b() }); - - GenericGoveeRequest lightColor = new GenericGoveeRequest( - new GenericGoveeMsg("colorwc", new ColorData(color, 0))); - communicationManager.sendRequest(this, lightColor); + /** + * Send the normalized RGB color parameters. + */ + public void sendColor(HSBType color) throws IOException { + logger.debug("sendColor({})", color); + int[] normalRGB = ColorUtil.hsbToRgb(new HSBType(color.getHue(), color.getSaturation(), PercentType.HUNDRED)); + GenericGoveeData data = new ColorData(new Color(normalRGB[0], normalRGB[1], normalRGB[2]), 0); + GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("colorwc", data)); + communicationManager.sendRequest(this, request); + lastColor = color; } - public void sendBrightness(int brightness) throws IOException { - lastBrightness = brightness; - GenericGoveeRequest lightBrightness = new GenericGoveeRequest( - new GenericGoveeMsg("brightness", new ValueIntData(brightness))); - communicationManager.sendRequest(this, lightBrightness); + /** + * Send the brightness parameter. + */ + public void sendBrightness(PercentType brightness) throws IOException { + logger.debug("sendBrightness({})", brightness); + GenericGoveeData data = new ValueIntData(brightness.intValue()); + GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("brightness", data)); + communicationManager.sendRequest(this, request); + lastColor = new HSBType(lastColor.getHue(), lastColor.getSaturation(), brightness); } - private void sendOnOff(OnOffType switchValue) throws IOException { - lastOnOff = (switchValue == OnOffType.ON) ? 1 : 0; - GenericGoveeRequest switchLight = new GenericGoveeRequest( - new GenericGoveeMsg("turn", new ValueIntData(lastOnOff))); - communicationManager.sendRequest(this, switchLight); + /** + * Send the on-off parameter. + */ + private void sendOnOff(OnOffType onOff) throws IOException { + logger.debug("sendOnOff({})", onOff); + GenericGoveeData data = new ValueIntData(onOff == OnOffType.ON ? 1 : 0); + GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("turn", data)); + communicationManager.sendRequest(this, request); } - private void sendColorTemp(int colorTemp) throws IOException { - lastColorTempInKelvin = colorTemp; - logger.debug("sendColorTemp {}", colorTemp); - GenericGoveeRequest lightColor = new GenericGoveeRequest( - new GenericGoveeMsg("colorwc", new ColorData(new Color(0, 0, 0), colorTemp))); - communicationManager.sendRequest(this, lightColor); + /** + * Set the color temperature (Kelvin) parameter. + */ + private void sendKelvin(int kelvin) throws IOException { + logger.debug("sendKelvin({})", kelvin); + GenericGoveeData data = new ColorData(new Color(0, 0, 0), kelvin); + GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("colorwc", data)); + communicationManager.sendRequest(this, request); + lastKelvin = kelvin; } /** - * Creates a Color state by using the last color information from lastColor - * The brightness is overwritten either by the provided lastBrightness - * or if lastOnOff = 0 (off) then the brightness is set 0 + * Build an {@link HSBType} from the given normalized {@link Color} record, brightness, and on state. * - * @see #lastColor - * @see #lastBrightness - * @see #lastOnOff + * @param normalColor record containing the lamp's normalized RGB parameters (0..255) + * @param brightness the lamp brightness in range 0..100 + * @param on the lamp state * - * @return the computed state + * @return the HSB presentation state */ - private HSBType getColorState(Color color, int brightness) { - PercentType computedBrightness = lastOnOff == 0 ? new PercentType(0) : new PercentType(brightness); - int[] rgb = { color.r(), color.g(), color.b() }; - HSBType hsb = ColorUtil.rgbToHsb(rgb); - return new HSBType(hsb.getHue(), hsb.getSaturation(), computedBrightness); + private static HSBType buildHSB(Color normalColor, int brightness, boolean on) { + int[] normalizedRGB = { normalColor.r(), normalColor.g(), normalColor.b() }; + HSBType normalizedHSB = ColorUtil.rgbToHsb(normalizedRGB); + PercentType brightnessParam = on ? new PercentType(brightness) : PercentType.ZERO; + return new HSBType(normalizedHSB.getHue(), normalizedHSB.getSaturation(), brightnessParam); } void handleIncomingStatus(String response) { @@ -285,46 +293,54 @@ public void updateDeviceState(@Nullable StatusResponse message) { } logger.trace("Receiving Device State"); - int newOnOff = message.msg().data().onOff(); - logger.trace("newOnOff = {}", newOnOff); - int newBrightness = message.msg().data().brightness(); - logger.trace("newBrightness = {}", newBrightness); - Color newColor = message.msg().data().color(); - logger.trace("newColor = {}", newColor); - int newColorTempInKelvin = message.msg().data().colorTemInKelvin(); - logger.trace("newColorTempInKelvin = {}", newColorTempInKelvin); - - newColorTempInKelvin = (newColorTempInKelvin < COLOR_TEMPERATURE_MIN_VALUE) - ? COLOR_TEMPERATURE_MIN_VALUE.intValue() - : newColorTempInKelvin; - newColorTempInKelvin = (newColorTempInKelvin > COLOR_TEMPERATURE_MAX_VALUE) - ? COLOR_TEMPERATURE_MAX_VALUE.intValue() - : newColorTempInKelvin; - - int newColorTempInPercent = ((Double) ((newColorTempInKelvin - COLOR_TEMPERATURE_MIN_VALUE) - / (COLOR_TEMPERATURE_MAX_VALUE - COLOR_TEMPERATURE_MIN_VALUE) * 100.0)).intValue(); - - HSBType adaptedColor = getColorState(newColor, newBrightness); - - logger.trace("HSB old: {} vs adaptedColor: {}", lastColor, adaptedColor); - // avoid noise by only updating if the value has changed on the device - if (!adaptedColor.equals(lastColor)) { - logger.trace("UPDATING HSB old: {} != {}", lastColor, adaptedColor); - updateState(CHANNEL_COLOR, adaptedColor); + + boolean on = message.msg().data().onOff() == 1; + logger.trace("on:{}", on); + + int brightness = message.msg().data().brightness(); + logger.trace("brightness:{}", brightness); + + Color normalRGB = message.msg().data().color(); + logger.trace("normalRGB:{}", normalRGB); + + int kelvin = message.msg().data().colorTemInKelvin(); + logger.trace("kelvin:{}", kelvin); + + HSBType color = buildHSB(normalRGB, brightness, on); + + kelvin = Math.min(COLOR_TEMPERATURE_MAX_VALUE.intValue(), + Math.max(COLOR_TEMPERATURE_MIN_VALUE.intValue(), kelvin)); + + logger.trace("Comparing color old:{} to new:{}", lastColor, color); + if (!color.equals(lastColor)) { + logger.trace("Updating color old:{} to new:{}", lastColor, color); + updateState(CHANNEL_COLOR, color); + lastColor = color; } - // avoid noise by only updating if the value has changed on the device - logger.trace("Color-Temperature Status: old: {} K {}% vs new: {} K", lastColorTempInKelvin, - newColorTempInPercent, newColorTempInKelvin); - if (newColorTempInKelvin != lastColorTempInKelvin) { - logger.trace("Color-Temperature Status: old: {} K {}% vs new: {} K", lastColorTempInKelvin, - newColorTempInPercent, newColorTempInKelvin); - updateState(CHANNEL_COLOR_TEMPERATURE_ABS, new QuantityType<>(lastColorTempInKelvin, Units.KELVIN)); - updateState(CHANNEL_COLOR_TEMPERATURE, new PercentType(newColorTempInPercent)); + logger.trace("Comparing color temperature old:{} to new:{}", lastKelvin, kelvin); + if (kelvin != lastKelvin) { + logger.trace("Updating color temperature old:{} to new:{}", lastKelvin, kelvin); + updateState(CHANNEL_COLOR_TEMPERATURE_ABS, QuantityType.valueOf(kelvin, Units.KELVIN)); + updateState(CHANNEL_COLOR_TEMPERATURE, kelvinToPercent(kelvin)); + lastKelvin = kelvin; } + } - lastOnOff = newOnOff; - lastColor = adaptedColor; - lastBrightness = newBrightness; + /** + * Convert PercentType to Kelvin. + */ + private static int percentToKelvin(PercentType percent) { + return (int) Math + .round((((COLOR_TEMPERATURE_MAX_VALUE - COLOR_TEMPERATURE_MIN_VALUE) * percent.doubleValue() / 100.0) + + COLOR_TEMPERATURE_MIN_VALUE)); + } + + /** + * Convert Kelvin to PercentType. + */ + private static PercentType kelvinToPercent(int kelvin) { + return new PercentType((int) Math.round((kelvin - COLOR_TEMPERATURE_MIN_VALUE) * 100.0 + / (COLOR_TEMPERATURE_MAX_VALUE - COLOR_TEMPERATURE_MIN_VALUE))); } } diff --git a/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java b/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java index cb92a7478062f..6781f8ca32a5b 100644 --- a/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java +++ b/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java @@ -12,12 +12,8 @@ */ package org.openhab.binding.govee.internal; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; import java.util.Arrays; import java.util.List; @@ -135,7 +131,7 @@ public void testInvalidResponseMessage() { verify(callback).stateUpdated( new ChannelUID(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR_TEMPERATURE_ABS), - getState(2000, Units.KELVIN)); + getState(9000, Units.KELVIN)); } finally { handler.dispose(); } From d5daa92183f1db1d3356258c25f4237bd7b27a7e Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Wed, 27 Nov 2024 14:08:26 +0000 Subject: [PATCH 02/35] rectacoring Signed-off-by: AndrewFG --- .../org/openhab/binding/govee/internal/GoveeHandler.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 543854c4713a2..4af4f0ec1e8dc 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -263,10 +263,10 @@ private void sendKelvin(int kelvin) throws IOException { * @return the HSB presentation state */ private static HSBType buildHSB(Color normalColor, int brightness, boolean on) { - int[] normalizedRGB = { normalColor.r(), normalColor.g(), normalColor.b() }; - HSBType normalizedHSB = ColorUtil.rgbToHsb(normalizedRGB); - PercentType brightnessParam = on ? new PercentType(brightness) : PercentType.ZERO; - return new HSBType(normalizedHSB.getHue(), normalizedHSB.getSaturation(), brightnessParam); + int[] normalRGB = { normalColor.r(), normalColor.g(), normalColor.b() }; + HSBType normalHSB = ColorUtil.rgbToHsb(normalRGB); + PercentType brightnessPercent = on ? new PercentType(brightness) : PercentType.ZERO; + return new HSBType(normalHSB.getHue(), normalHSB.getSaturation(), brightnessPercent); } void handleIncomingStatus(String response) { From 7d49ffd9cf82d1de9c74404f4ccccc0f64e5caac Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Wed, 27 Nov 2024 19:42:07 +0000 Subject: [PATCH 03/35] improve update speed, add property Signed-off-by: AndrewFG --- .../binding/govee/internal/GoveeHandler.java | 64 +++++++++++-------- .../resources/OH-INF/i18n/govee.properties | 1 + 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 4af4f0ec1e8dc..9f31f3cc5821f 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -41,6 +41,7 @@ import org.openhab.core.thing.binding.BaseThingHandler; import org.openhab.core.types.Command; import org.openhab.core.types.RefreshType; +import org.openhab.core.types.UnDefType; import org.openhab.core.util.ColorUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -89,8 +90,9 @@ public class GoveeHandler extends BaseThingHandler { private CommunicationManager communicationManager; + private OnOffType lastOn = OnOffType.OFF; private HSBType lastColor = new HSBType(); - private int lastKelvin = COLOR_TEMPERATURE_MIN_VALUE.intValue(); + private int lastKelvin; /** * This thing related job thingRefreshSender triggers an update to the Govee device. @@ -159,23 +161,31 @@ public void handleCommand(ChannelUID channelUID, Command command) { } else { switch (channelUID.getId()) { case CHANNEL_COLOR: + boolean triggerRefresh = false; Command doCommand = command; if (doCommand instanceof HSBType hsb) { sendColor(hsb); + triggerRefresh = true; doCommand = hsb.getBrightness(); // fall through } if (doCommand instanceof PercentType percent) { sendBrightness(percent); + triggerRefresh = true; doCommand = OnOffType.from(percent.intValue() > 0); // fall through } if (doCommand instanceof OnOffType onOff) { sendOnOff(onOff); + triggerRefresh = true; + } + if (triggerRefresh) { + triggerDeviceStatusRefresh(); } break; case CHANNEL_COLOR_TEMPERATURE: if (command instanceof PercentType percent) { sendKelvin(percentToKelvin(percent)); + triggerDeviceStatusRefresh(); } break; @@ -187,6 +197,7 @@ public void handleCommand(ChannelUID channelUID, Command command) { break; } sendKelvin(kelvin.intValue()); + triggerDeviceStatusRefresh(); } break; } @@ -218,7 +229,6 @@ public void sendColor(HSBType color) throws IOException { GenericGoveeData data = new ColorData(new Color(normalRGB[0], normalRGB[1], normalRGB[2]), 0); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("colorwc", data)); communicationManager.sendRequest(this, request); - lastColor = color; } /** @@ -229,7 +239,6 @@ public void sendBrightness(PercentType brightness) throws IOException { GenericGoveeData data = new ValueIntData(brightness.intValue()); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("brightness", data)); communicationManager.sendRequest(this, request); - lastColor = new HSBType(lastColor.getHue(), lastColor.getSaturation(), brightness); } /** @@ -250,23 +259,23 @@ private void sendKelvin(int kelvin) throws IOException { GenericGoveeData data = new ColorData(new Color(0, 0, 0), kelvin); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("colorwc", data)); communicationManager.sendRequest(this, request); - lastKelvin = kelvin; } /** - * Build an {@link HSBType} from the given normalized {@link Color} record, brightness, and on state. + * Build an {@link HSBType} from the given normalized {@link Color} RGB parameters, brightness, and on-off state + * parameters. If the on parameter is true then use the brightness parameter, otherwise use a brightness of zero. * - * @param normalColor record containing the lamp's normalized RGB parameters (0..255) - * @param brightness the lamp brightness in range 0..100 - * @param on the lamp state + * @param normalRgbParams record containing the lamp's normalized RGB parameters (0..255) + * @param brightnessParam the lamp brightness in range 0..100 + * @param onParam the lamp on-off state * - * @return the HSB presentation state + * @return the respective HSBType */ - private static HSBType buildHSB(Color normalColor, int brightness, boolean on) { - int[] normalRGB = { normalColor.r(), normalColor.g(), normalColor.b() }; - HSBType normalHSB = ColorUtil.rgbToHsb(normalRGB); - PercentType brightnessPercent = on ? new PercentType(brightness) : PercentType.ZERO; - return new HSBType(normalHSB.getHue(), normalHSB.getSaturation(), brightnessPercent); + private static HSBType buildHSB(Color normalRgbParams, int brightnessParam, boolean onParam) { + HSBType normalColor = ColorUtil + .rgbToHsb(new int[] { normalRgbParams.r(), normalRgbParams.g(), normalRgbParams.b() }); + PercentType brightness = onParam ? new PercentType(brightnessParam) : PercentType.ZERO; + return new HSBType(normalColor.getHue(), normalColor.getSaturation(), brightness); } void handleIncomingStatus(String response) { @@ -294,7 +303,7 @@ public void updateDeviceState(@Nullable StatusResponse message) { logger.trace("Receiving Device State"); - boolean on = message.msg().data().onOff() == 1; + OnOffType on = OnOffType.from(message.msg().data().onOff() == 1); logger.trace("on:{}", on); int brightness = message.msg().data().brightness(); @@ -306,23 +315,28 @@ public void updateDeviceState(@Nullable StatusResponse message) { int kelvin = message.msg().data().colorTemInKelvin(); logger.trace("kelvin:{}", kelvin); - HSBType color = buildHSB(normalRGB, brightness, on); - - kelvin = Math.min(COLOR_TEMPERATURE_MAX_VALUE.intValue(), - Math.max(COLOR_TEMPERATURE_MIN_VALUE.intValue(), kelvin)); + HSBType color = buildHSB(normalRGB, brightness, true); - logger.trace("Comparing color old:{} to new:{}", lastColor, color); - if (!color.equals(lastColor)) { - logger.trace("Updating color old:{} to new:{}", lastColor, color); - updateState(CHANNEL_COLOR, color); + logger.trace("Comparing color old:{} to new:{}, on state old:{} to new:{}", lastColor, color, lastOn, on); + if ((on != lastOn) || !color.equals(lastColor)) { + logger.trace("Updating color old:{} to new:{}, on state old:{} to new:{}", lastColor, color, lastOn, on); + updateState(CHANNEL_COLOR, buildHSB(normalRGB, brightness, on == OnOffType.ON)); + lastOn = on; lastColor = color; } logger.trace("Comparing color temperature old:{} to new:{}", lastKelvin, kelvin); if (kelvin != lastKelvin) { logger.trace("Updating color temperature old:{} to new:{}", lastKelvin, kelvin); - updateState(CHANNEL_COLOR_TEMPERATURE_ABS, QuantityType.valueOf(kelvin, Units.KELVIN)); - updateState(CHANNEL_COLOR_TEMPERATURE, kelvinToPercent(kelvin)); + if (kelvin != 0) { + kelvin = Math.min(COLOR_TEMPERATURE_MAX_VALUE.intValue(), + Math.max(COLOR_TEMPERATURE_MIN_VALUE.intValue(), kelvin)); + updateState(CHANNEL_COLOR_TEMPERATURE, kelvinToPercent(kelvin)); + updateState(CHANNEL_COLOR_TEMPERATURE_ABS, QuantityType.valueOf(kelvin, Units.KELVIN)); + } else { + updateState(CHANNEL_COLOR_TEMPERATURE, UnDefType.UNDEF); + updateState(CHANNEL_COLOR_TEMPERATURE_ABS, UnDefType.UNDEF); + } lastKelvin = kelvin; } } diff --git a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties index b0a015db28f82..73b203dbfdc00 100644 --- a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties +++ b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties @@ -74,6 +74,7 @@ discovery.govee-light.H6059 = H6059 RGBWW Night Light for Kids discovery.govee-light.H618F = H618F RGBIC LED Strip Lights discovery.govee-light.H618E = H618E LED Strip Lights 22m discovery.govee-light.H6168 = H6168 TV LED Backlight +discovery.govee-light.H60A1 = H60A1 Smart Ceiling Light # thing status descriptions From 4dd0420910aacaf4e63810405e6ea8633d0e3a69 Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Wed, 27 Nov 2024 23:43:13 +0000 Subject: [PATCH 04/35] add new device labels, and sort them Signed-off-by: AndrewFG --- .../resources/OH-INF/i18n/govee.properties | 116 +++++++++++++----- 1 file changed, 86 insertions(+), 30 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties index 73b203dbfdc00..2fd545a61a011 100644 --- a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties +++ b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties @@ -15,66 +15,122 @@ thing-type.config.govee-light.refreshInterval.description = The amount of time t # product names -discovery.govee-light.H619Z = H619Z RGBIC Pro LED Strip Lights +discovery.govee-light.H6042 = H6042 Govee TV Light Bar #2 +discovery.govee-light.H6043 = H6043 Govee TV Light Bars #2 discovery.govee-light.H6046 = H6046 RGBIC TV Light Bars discovery.govee-light.H6047 = H6047 RGBIC Gaming Light Bars with Smart Controller +discovery.govee-light.H6051 = H6051 Aura - Smart Table Lamp +discovery.govee-light.H6052 = H6052 Govee Table Lamp +discovery.govee-light.H6056 = H6056 H6056 Flow Plus +discovery.govee-light.H6059 = H6059 RGBWW Night Light for Kids discovery.govee-light.H6061 = H6061 Glide Hexa LED Panels discovery.govee-light.H6062 = H6062 Glide Wall Light +discovery.govee-light.H6063 = H6063 Gaming Wall Light discovery.govee-light.H6065 = H6065 Glide RGBIC Y Lights discovery.govee-light.H6066 = H6066 Glide Hexa Pro LED Panel discovery.govee-light.H6067 = H6067 Glide Triangle Light Panels +discovery.govee-light.H606A = H606A Glide Hexa Light Panel Ultra discovery.govee-light.H6072 = H6072 RGBICWW Corner Floor Lamp -discovery.govee-light.H6076 = H6076 RGBICW Smart Corner Floor Lamp discovery.govee-light.H6073 = H6073 LED Floor Lamp +discovery.govee-light.H6076 = H6076 RGBICW Smart Corner Floor Lamp discovery.govee-light.H6078 = H6078 Cylinder Floor Lamp +discovery.govee-light.H607C = H607C Floor Lamp #2 discovery.govee-light.H6087 = H6087 RGBIC Smart Wall Sconces +discovery.govee-light.H6088 = H6088 RGBIC Cube Wall Sconces +discovery.govee-light.H608A = H608A String Downlights 5M +discovery.govee-light.H608B = H608B String Downlights 3M +discovery.govee-light.H608C = H608C String Downlights 2M +discovery.govee-light.H608D = H608D String Downlights 10M +discovery.govee-light.H60A0 = H60A0 Ceiling Light +discovery.govee-light.H60A1 = H60A1 Smart Ceiling Light +discovery.govee-light.H610A = H610A Glide Lively Wall Lights +discovery.govee-light.H610B = H610B Music Wall Lights +discovery.govee-light.H6110 = H6110 2x5M Multicolor with Alexa +discovery.govee-light.H6117 = H6117 Dream Color LED Strip Light 10M +discovery.govee-light.H6141 = H6141 5M Smart Multicolor Strip Light +discovery.govee-light.H6143 = H6143 5M Strip Light +discovery.govee-light.H6144 = H6144 2x5M Strip Light +discovery.govee-light.H6159 = H6159 RGB Light Strip +discovery.govee-light.H615A = H615A 5M Light Strip with Alexa +discovery.govee-light.H615B = H615B 10M Light Strip with Alexa +discovery.govee-light.H615C = H615C 15M Light Strip with Alexa +discovery.govee-light.H615D = H615D 20M Light Strip with Alexa +discovery.govee-light.H615E = H615E 30M Light Strip with Alexa +discovery.govee-light.H6163 = H6163 Dreamcolor LED Strip Light 5M +discovery.govee-light.H6167 = H6167 TV Backlight 2.4M +discovery.govee-light.H6168 = H6168 TV Backlight 2x0.7M+2x1.2M +discovery.govee-light.H616C = H616C Outdoor Strip 10M +discovery.govee-light.H616D = H616D Outdoor Strip 2x7.5M +discovery.govee-light.H616E = H616E Outdoor Strip 2x10M +discovery.govee-light.H6172 = H6172 Outdoor LED Strip 10m discovery.govee-light.H6173 = H6173 RGBIC Outdoor Strip Lights -discovery.govee-light.H619A = H619A RGBIC Strip Lights With Protective Coating 5M -discovery.govee-light.H619B = H619B RGBIC LED Strip Lights With Protective Coating -discovery.govee-light.H619C = H619C LED Strip Lights With Protective Coating -discovery.govee-light.H619D = H619D RGBIC PRO LED Strip Lights -discovery.govee-light.H619E = H619E RGBIC LED Strip Lights With Protective Coating -discovery.govee-light.H61A0 = H61A0 RGBIC Neon Rope Light 1M +discovery.govee-light.H6175 = H6175 RGBIC Outdoor Strip Lights 10M +discovery.govee-light.H6176 = H6176 RGBIC Outdoor Strip Lights 30M +discovery.govee-light.H6182 = H6182 WiFi Multicolor TV Strip Light +discovery.govee-light.H618A = H618A RGBIC Basic LED Strip Lights 5M +discovery.govee-light.H618C = H618C RGBIC Basic LED Strip Lights 5M +discovery.govee-light.H618E = H618E LED Strip Lights 22m +discovery.govee-light.H618F = H618F RGBIC LED Strip Lights +discovery.govee-light.H619A = H619A Strip Lights With Protective Coating 5M +discovery.govee-light.H619B = H619B Strip Lights With Protective Coating 7.5M +discovery.govee-light.H619C = H619C Strip Lights With Protective Coating with Alexa 10M +discovery.govee-light.H619D = H619D PRO LED Strip Lights with Alexa 2x7.5M +discovery.govee-light.H619E = H619E Strip Lights With Protective Coating with Alexa 2x10M +discovery.govee-light.H619Z = H619Z Pro LED Strip Lights 3M +discovery.govee-light.H61A0 = H61A0 RGBIC Neon Rope Light 3M discovery.govee-light.H61A1 = H61A1 RGBIC Neon Rope Light 2M discovery.govee-light.H61A2 = H61A2 RGBIC Neon Rope Light 5M -discovery.govee-light.H61A3 = H61A3 RGBIC Neon Rope Light +discovery.govee-light.H61A3 = H61A3 RGBIC Neon Rope Light 4M +discovery.govee-light.H61A5 = H61A5 Neon LED Strip Light 10M +discovery.govee-light.H61A8 = H61A8 Neon Rope Light 10M +discovery.govee-light.H61A9 = H61A8 Neon Rope Light 20M +discovery.govee-light.H61B1 = H61B1 Strip Light with Cover 5M +discovery.govee-light.H61B2 = H61B2 RGBIC Neon TV Backlight 3M +discovery.govee-light.H61BA = H61BA LED Strip Light 5M +discovery.govee-light.H61BC = H61BC LED Strip Light 10M +discovery.govee-light.H61BE = H61BE LED Strip Light 2x10M +discovery.govee-light.H61C2 = H61C2 Neon LED Strip Light 2M +discovery.govee-light.H61C3 = H61C2 Neon LED Strip Light 3M +discovery.govee-light.H61C5 = H61C2 Neon LED Strip Light 5M discovery.govee-light.H61D3 = H61D3 Neon Rope Light 2 3m discovery.govee-light.H61D5 = H61D5 Neon Rope Light 2 5m -discovery.govee-light.H61A5 = H61A5 Neon LED Strip Light 10 -discovery.govee-light.H61A8 = H61A8 Neon Rope Light 10 -discovery.govee-light.H618A = H618A RGBIC Basic LED Strip Lights 5M -discovery.govee-light.H618C = H618C RGBIC Basic LED Strip Lights 5M -discovery.govee-light.H6117 = H6117 Dream Color LED Strip Light 10M -discovery.govee-light.H6159 = H6159 RGB Light Strip -discovery.govee-light.H615E = H615E LED Strip Lights 30M -discovery.govee-light.H6163 = H6163 Dreamcolor LED Strip Light 5M -discovery.govee-light.H610A = H610A Glide Lively Wall Lights -discovery.govee-light.H610B = H610B Music Wall Lights -discovery.govee-light.H6172 = H6172 Outdoor LED Strip 10m -discovery.govee-light.H61B2 = H61B2 RGBIC Neon TV Backlight +discovery.govee-light.H61E0 = H61E0 LED Strip Light M1 discovery.govee-light.H61E1 = H61E1 LED Strip Light M1 discovery.govee-light.H7012 = H7012 Warm White Outdoor String Lights discovery.govee-light.H7013 = H7013 Warm White Outdoor String Lights discovery.govee-light.H7021 = H7021 RGBIC Warm White Smart Outdoor String discovery.govee-light.H7028 = H7028 Lynx Dream LED-Bulb String +discovery.govee-light.H7033 = H7033 LED-Bulb String Lights discovery.govee-light.H7041 = H7041 LED Outdoor Bulb String Lights discovery.govee-light.H7042 = H7042 LED Outdoor Bulb String Lights -discovery.govee-light.H705A = H705A Permanent Outdoor Lights 30M -discovery.govee-light.H705B = H705B Permanent Outdoor Lights 15M discovery.govee-light.H7050 = H7050 Outdoor Ground Lights 11M discovery.govee-light.H7051 = H7051 Outdoor Ground Lights 15M +discovery.govee-light.H7052 = H7052 Outdoor Ground Lights 15M +discovery.govee-light.H7053 = H7052 Outdoor Ground Lights 30M discovery.govee-light.H7055 = H7055 Pathway Light +discovery.govee-light.H705A = H705A Permanent Outdoor Lights 30M +discovery.govee-light.H705B = H705B Permanent Outdoor Lights 15M +discovery.govee-light.H705C = H705C Permanent Outdoor Lights 45M +discovery.govee-light.H705D = H705D Permanent Outdoor Lights #2 15M +discovery.govee-light.H705E = H705E Permanent Outdoor Lights #2 30M +discovery.govee-light.H705F = H705F Permanent Outdoor Lights #2 45M discovery.govee-light.H7060 = H7060 LED Flood Lights (2-Pack) discovery.govee-light.H7061 = H7061 LED Flood Lights (4-Pack) discovery.govee-light.H7062 = H7062 LED Flood Lights (6-Pack) +discovery.govee-light.H7063 = H7063 Outdoor Flood Lights discovery.govee-light.H7065 = H7065 Outdoor Spot Lights -discovery.govee-light.H6051 = H6051 Aura - Smart Table Lamp -discovery.govee-light.H6056 = H6056 H6056 Flow Plus -discovery.govee-light.H6059 = H6059 RGBWW Night Light for Kids -discovery.govee-light.H618F = H618F RGBIC LED Strip Lights -discovery.govee-light.H618E = H618E LED Strip Lights 22m -discovery.govee-light.H6168 = H6168 TV LED Backlight -discovery.govee-light.H60A1 = H60A1 Smart Ceiling Light +discovery.govee-light.H7066 = H7066 Outdoor Spot Lights +discovery.govee-light.H706A = H706A Permanent Outdoor Lights Pro 30M +discovery.govee-light.H706B = H706B Permanent Outdoor Lights Pro 45M +discovery.govee-light.H706C = H706C Permanent Outdoor Lights Pro 60M +discovery.govee-light.H7075 = H7075 Outdoor Wall Light +discovery.govee-light.H70B1 = H70B1 520 LED Curtain Lights +discovery.govee-light.H70BC = H70BC 400 LED Curtain Lights +discovery.govee-light.H70C1 = H70C1 RGBIC String Light 10M +discovery.govee-light.H70C2 = H70C2 RGBIC String Light 20M +discovery.govee-light.H805A = H805A Permanent Outdoor Lights Elite 30M +discovery.govee-light.H805B = H805B Permanent Outdoor Lights Elite 15M +discovery.govee-light.H805C = H805C Permanent Outdoor Lights Elite 45M # thing status descriptions From 8aa0c9634acdcf21c6db6fd69704bc8a4bdd29f8 Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Thu, 28 Nov 2024 13:47:07 +0000 Subject: [PATCH 05/35] read me Signed-off-by: AndrewFG --- bundles/org.openhab.binding.govee/README.md | 138 ++++++++++++++------ 1 file changed, 101 insertions(+), 37 deletions(-) diff --git a/bundles/org.openhab.binding.govee/README.md b/bundles/org.openhab.binding.govee/README.md index 459f17aa5f333..0c0fc9dfafa5a 100644 --- a/bundles/org.openhab.binding.govee/README.md +++ b/bundles/org.openhab.binding.govee/README.md @@ -20,68 +20,132 @@ While Govee provides probably more than a hundred different lights, only the fol Here is a list of the supported devices (the ones marked with * have been tested by the author) -- H619Z RGBIC Pro LED Strip Lights +- H6042 Govee TV Light Bar #2 +- H6043 Govee TV Light Bars #2 - H6046 RGBIC TV Light Bars - H6047 RGBIC Gaming Light Bars with Smart Controller -- H6061 Glide Hexa LED Panels (*) +- H6051 Aura - Smart Table Lamp +- H6052 Govee Table Lamp +- H6056 H6056 Flow Plus +- H6059 RGBWW Night Light for Kids +- H6061 Glide Hexa LED Panels - H6062 Glide Wall Light +- H6063 Gaming Wall Light - H6065 Glide RGBIC Y Lights - H6066 Glide Hexa Pro LED Panel -- H6067 Glide Triangle Light Panels (*) -- H6072 RGBICWW Corner Floor Lamp (*) -- H6076 RGBICW Smart Corner Floor Lamp (*) +- H6067 Glide Triangle Light Panels +- H606A Glide Hexa Light Panel Ultra +- H6072 RGBICWW Corner Floor Lamp - H6073 LED Floor Lamp +- H6076 RGBICW Smart Corner Floor Lamp - H6078 Cylinder Floor Lamp +- H607C Floor Lamp #2 - H6087 RGBIC Smart Wall Sconces -- H6173 RGBIC Outdoor Strip Lights -- H619A RGBIC Strip Lights With Protective Coating 5M -- H619B RGBIC LED Strip Lights With Protective Coating -- H619C LED Strip Lights With Protective Coating -- H619D RGBIC PRO LED Strip Lights -- H619E RGBIC LED Strip Lights With Protective Coating -- H61A0 RGBIC Neon Rope Light 1M -- H61A1 RGBIC Neon Rope Light 2M -- H61A2 RGBIC Neon Rope Light 5M -- H61A3 RGBIC Neon Rope Light -- H61C5 RGBIC LED Neon Rope Lights for Desks (*) -- H61D3 Neon Rope Light 2 3M (*) -- H61D5 Neon Rope Light 2 5M (*) -- H61A5 Neon LED Strip Light 10 -- H61A8Neon Neon Rope Light 10 -- H618A RGBIC Basic LED Strip Lights 5M -- H618C RGBIC Basic LED Strip Lights 5M -- H6117 Dream Color LED Strip Light 10M -- H6159 RGB Light Strip (*) -- H615E LED Strip Lights 30M -- H6163 Dreamcolor LED Strip Light 5M +- H6088 RGBIC Cube Wall Sconces +- H608A String Downlights 5M +- H608B String Downlights 3M +- H608C String Downlights 2M +- H608D String Downlights 10M +- H60A0 Ceiling Light +- H60A1 Smart Ceiling Light - H610A Glide Lively Wall Lights - H610B Music Wall Lights +- H6110 2x5M Multicolor with Alexa +- H6117 Dream Color LED Strip Light 10M +- H6141 5M Smart Multicolor Strip Light +- H6143 5M Strip Light +- H6144 2x5M Strip Light +- H6159 RGB Light Strip +- H615A 5M Light Strip with Alexa +- H615B 10M Light Strip with Alexa +- H615C 15M Light Strip with Alexa +- H615D 20M Light Strip with Alexa +- H615E 30M Light Strip with Alexa +- H6163 Dreamcolor LED Strip Light 5M +- H6167 TV Backlight 2.4M +- H6168 TV Backlight 2x0.7M+2x1.2M +- H616C Outdoor Strip 10M +- H616D Outdoor Strip 2x7.5M +- H616E Outdoor Strip 2x10M - H6172 Outdoor LED Strip 10m -- H61B2 RGBIC Neon TV Backlight +- H6173 RGBIC Outdoor Strip Lights +- H6175 RGBIC Outdoor Strip Lights 10M +- H6176 RGBIC Outdoor Strip Lights 30M +- H6182 WiFi Multicolor TV Strip Light +- H618A RGBIC Basic LED Strip Lights 5M +- H618C RGBIC Basic LED Strip Lights 5M +- H618E LED Strip Lights 22m +- H618F RGBIC LED Strip Lights +- H619A Strip Lights With Protective Coating 5M +- H619B Strip Lights With Protective Coating 7.5M +- H619C Strip Lights With Protective Coating with Alexa 10M +- H619D PRO LED Strip Lights with Alexa 2x7.5M +- H619E Strip Lights With Protective Coating with Alexa 2x10M +- H619Z Pro LED Strip Lights 3M +- H61A0 RGBIC Neon Rope Light 3M +- H61A1 RGBIC Neon Rope Light 2M +- H61A2 RGBIC Neon Rope Light 5M +- H61A3 RGBIC Neon Rope Light 4M +- H61A5 Neon LED Strip Light 10M +- H61A8 Neon Rope Light 10M +- H61A8 Neon Rope Light 20M +- H61B1 Strip Light with Cover 5M +- H61B2 RGBIC Neon TV Backlight 3M +- H61BA LED Strip Light 5M +- H61BC LED Strip Light 10M +- H61BE LED Strip Light 2x10M +- H61C2 Neon LED Strip Light 2M +- H61C2 Neon LED Strip Light 3M +- H61C2 Neon LED Strip Light 5M +- H61D3 Neon Rope Light 2 3m +- H61D5 Neon Rope Light 2 5m +- H61E0 LED Strip Light M1 - H61E1 LED Strip Light M1 - H7012 Warm White Outdoor String Lights - H7013 Warm White Outdoor String Lights - H7021 RGBIC Warm White Smart Outdoor String - H7028 Lynx Dream LED-Bulb String +- H7033 LED-Bulb String Lights - H7041 LED Outdoor Bulb String Lights - H7042 LED Outdoor Bulb String Lights -- H705A Permanent Outdoor Lights 30M -- H705B Permanent Outdoor Lights 15M - H7050 Outdoor Ground Lights 11M - H7051 Outdoor Ground Lights 15M +- H7052 Outdoor Ground Lights 15M +- H7052 Outdoor Ground Lights 30M - H7055 Pathway Light +- H705A Permanent Outdoor Lights 30M +- H705B Permanent Outdoor Lights 15M +- H705C Permanent Outdoor Lights 45M +- H705D Permanent Outdoor Lights #2 15M +- H705E Permanent Outdoor Lights #2 30M +- H705F Permanent Outdoor Lights #2 45M - H7060 LED Flood Lights (2-Pack) - H7061 LED Flood Lights (4-Pack) - H7062 LED Flood Lights (6-Pack) +- H7063 Outdoor Flood Lights - H7065 Outdoor Spot Lights -- H70C1 Govee Christmas String Lights 10m (*) -- H70C2 Govee Christmas String Lights 20m (*) -- H6051 Aura - Smart Table Lamp -- H6056 H6056 Flow Plus -- H6059 RGBWW Night Light for Kids -- H618F RGBIC LED Strip Lights -- H618E LED Strip Lights 22m -- H6168 TV LED Backlight +- H7066 Outdoor Spot Lights +- H706A Permanent Outdoor Lights Pro 30M +- H706B Permanent Outdoor Lights Pro 45M +- H706C Permanent Outdoor Lights Pro 60M +- H7075 Outdoor Wall Light +- H70B1 520 LED Curtain Lights +- H70BC 400 LED Curtain Lights +- H70C1 RGBIC String Light 10M +- H70C2 RGBIC String Light 20M +- H805A Permanent Outdoor Lights Elite 30M +- H805B Permanent Outdoor Lights Elite 15M +- H805C Permanent Outdoor Lights Elite 45M + +## Firewall + +Govee devices communicate via multicast and unicast messages over the LAN. +So you must ensure that any firewall on your OpenHAB server is configured to pass the following traffic: + +- Multicast UDP on 239.255.255.250 port 4001 +- Incoming unicast UDP on port 4002 +- Outgoing unicast UDP on port 4003 + ## Discovery Discovery is done by scanning the devices in the Thing section. From dcd6aeb837d5cb8ba713cd805955b9b4c0489bf0 Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Thu, 28 Nov 2024 13:48:17 +0000 Subject: [PATCH 06/35] fix host name resolution Signed-off-by: AndrewFG --- .../govee/internal/CommunicationManager.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index c8cdee364b464..a7f1b5c05971c 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -19,6 +19,7 @@ import java.net.InetSocketAddress; import java.net.MulticastSocket; import java.net.NetworkInterface; +import java.net.UnknownHostException; import java.time.Instant; import java.util.HashMap; import java.util.Map; @@ -71,9 +72,20 @@ public interface DiscoveryResultReceiver { public CommunicationManager() { } + /** + * Get the resolved IP address from the given host name + */ + private static String ipAddressFrom(String host) { + try { + return InetAddress.getByName(host).getHostAddress(); + } catch (UnknownHostException e) { + } + return host; + } + public void registerHandler(GoveeHandler handler) { synchronized (thingHandlers) { - thingHandlers.put(handler.getHostname(), handler); + thingHandlers.put(ipAddressFrom(handler.getHostname()), handler); if (receiverThread == null) { receiverThread = new StatusReceiver(); receiverThread.start(); @@ -83,7 +95,7 @@ public void registerHandler(GoveeHandler handler) { public void unregisterHandler(GoveeHandler handler) { synchronized (thingHandlers) { - thingHandlers.remove(handler.getHostname()); + thingHandlers.remove(ipAddressFrom(handler.getHostname())); if (thingHandlers.isEmpty()) { StatusReceiver receiver = receiverThread; if (receiver != null) { @@ -102,7 +114,7 @@ public void sendRequest(GoveeHandler handler, GenericGoveeRequest request) throw final byte[] data = message.getBytes(); final InetAddress address = InetAddress.getByName(hostname); DatagramPacket packet = new DatagramPacket(data, data.length, address, REQUEST_PORT); - logger.trace("Sending {} to {}", message, hostname); + logger.trace("Sending request to {} with content = {}", address.getHostAddress(), message); socket.send(packet); socket.close(); } @@ -212,7 +224,7 @@ public void run() { String response = new String(packet.getData(), packet.getOffset(), packet.getLength()); String deviceIPAddress = packet.getAddress().toString().replace("/", ""); - logger.trace("Response from {} = {}", deviceIPAddress, response); + logger.trace("Received response from {} with content = {}", deviceIPAddress, response); final DiscoveryResultReceiver discoveryReceiver; synchronized (this) { @@ -240,15 +252,15 @@ public void run() { handler = thingHandlers.get(deviceIPAddress); } if (handler == null) { - logger.warn("thing Handler for {} couldn't be found.", deviceIPAddress); + logger.warn("Handler not found for {}", deviceIPAddress); } else { - logger.debug("processing status updates for thing {} ", handler.getThing().getLabel()); + logger.debug("Processing response for {}", deviceIPAddress); handler.handleIncomingStatus(response); } } } } catch (IOException e) { - logger.warn("exception when receiving status packet", e); + logger.warn("Exception when receiving status packet", e); // as we haven't received a packet we also don't know where it should have come from // hence, we don't know which thing put offline. // a way to monitor this would be to keep track in a list, which device answers we expect From ab4a18ac0505e8294fda98102bd1c478953f39b8 Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Thu, 28 Nov 2024 13:48:35 +0000 Subject: [PATCH 07/35] logging tweaks Signed-off-by: AndrewFG --- .../binding/govee/internal/GoveeHandler.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 9f31f3cc5821f..5994a8bee0886 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -214,7 +214,7 @@ public void handleCommand(ChannelUID channelUID, Command command) { * Initiate a refresh to our thing device */ private void triggerDeviceStatusRefresh() throws IOException { - logger.debug("trigger Refresh Status of device {}", thing.getLabel()); + logger.debug("triggerDeviceStatusRefresh() on {}", thing.getUID()); GenericGoveeData data = new EmptyValueQueryStatusData(); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("devStatus", data)); communicationManager.sendRequest(this, request); @@ -224,7 +224,7 @@ private void triggerDeviceStatusRefresh() throws IOException { * Send the normalized RGB color parameters. */ public void sendColor(HSBType color) throws IOException { - logger.debug("sendColor({})", color); + logger.debug("sendColor({}) to {}", color, thing.getUID()); int[] normalRGB = ColorUtil.hsbToRgb(new HSBType(color.getHue(), color.getSaturation(), PercentType.HUNDRED)); GenericGoveeData data = new ColorData(new Color(normalRGB[0], normalRGB[1], normalRGB[2]), 0); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("colorwc", data)); @@ -235,7 +235,7 @@ public void sendColor(HSBType color) throws IOException { * Send the brightness parameter. */ public void sendBrightness(PercentType brightness) throws IOException { - logger.debug("sendBrightness({})", brightness); + logger.debug("sendBrightness({}) to {}", brightness, thing.getUID()); GenericGoveeData data = new ValueIntData(brightness.intValue()); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("brightness", data)); communicationManager.sendRequest(this, request); @@ -245,7 +245,7 @@ public void sendBrightness(PercentType brightness) throws IOException { * Send the on-off parameter. */ private void sendOnOff(OnOffType onOff) throws IOException { - logger.debug("sendOnOff({})", onOff); + logger.debug("sendOnOff({}) to {}", onOff, thing.getUID()); GenericGoveeData data = new ValueIntData(onOff == OnOffType.ON ? 1 : 0); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("turn", data)); communicationManager.sendRequest(this, request); @@ -255,7 +255,7 @@ private void sendOnOff(OnOffType onOff) throws IOException { * Set the color temperature (Kelvin) parameter. */ private void sendKelvin(int kelvin) throws IOException { - logger.debug("sendKelvin({})", kelvin); + logger.debug("sendKelvin({}) to {}", kelvin, thing.getUID()); GenericGoveeData data = new ColorData(new Color(0, 0, 0), kelvin); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("colorwc", data)); communicationManager.sendRequest(this, request); @@ -301,33 +301,33 @@ public void updateDeviceState(@Nullable StatusResponse message) { return; } - logger.trace("Receiving Device State"); + logger.debug("updateDeviceState() for {}", thing.getUID()); OnOffType on = OnOffType.from(message.msg().data().onOff() == 1); - logger.trace("on:{}", on); + logger.trace("- on:{}", on); int brightness = message.msg().data().brightness(); - logger.trace("brightness:{}", brightness); + logger.trace("- brightness:{}", brightness); Color normalRGB = message.msg().data().color(); - logger.trace("normalRGB:{}", normalRGB); + logger.trace("- normalRGB:{}", normalRGB); int kelvin = message.msg().data().colorTemInKelvin(); - logger.trace("kelvin:{}", kelvin); + logger.trace("- kelvin:{}", kelvin); HSBType color = buildHSB(normalRGB, brightness, true); - logger.trace("Comparing color old:{} to new:{}, on state old:{} to new:{}", lastColor, color, lastOn, on); + logger.trace("Compare color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastOn, on); if ((on != lastOn) || !color.equals(lastColor)) { - logger.trace("Updating color old:{} to new:{}, on state old:{} to new:{}", lastColor, color, lastOn, on); + logger.trace("Update color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastOn, on); updateState(CHANNEL_COLOR, buildHSB(normalRGB, brightness, on == OnOffType.ON)); lastOn = on; lastColor = color; } - logger.trace("Comparing color temperature old:{} to new:{}", lastKelvin, kelvin); + logger.trace("Compare color temperature old:{} to new:{}", lastKelvin, kelvin); if (kelvin != lastKelvin) { - logger.trace("Updating color temperature old:{} to new:{}", lastKelvin, kelvin); + logger.trace("Update color temperature old:{} to new:{}", lastKelvin, kelvin); if (kelvin != 0) { kelvin = Math.min(COLOR_TEMPERATURE_MAX_VALUE.intValue(), Math.max(COLOR_TEMPERATURE_MIN_VALUE.intValue(), kelvin)); From 0acd7db41ddac94f7057911dc9aa4fdc22ca330a Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Thu, 28 Nov 2024 15:57:28 +0000 Subject: [PATCH 08/35] support configurable color temperature ranges Signed-off-by: AndrewFG --- bundles/org.openhab.binding.govee/README.md | 12 +-- .../govee/internal/GoveeConfiguration.java | 4 + .../binding/govee/internal/GoveeHandler.java | 38 ++++++--- .../govee/internal/GoveeHandlerFactory.java | 9 +- .../GoveeStateDescriptionProvider.java | 83 +++++++++++++++++++ .../main/resources/OH-INF/config/config.xml | 10 +++ .../resources/OH-INF/i18n/govee.properties | 24 ++++++ .../resources/OH-INF/thing/thing-types.xml | 20 ++--- .../resources/OH-INF/update/instructions.xml | 14 ++++ .../govee/internal/GoveeHandlerMock.java | 10 +-- .../GoveeSerializeGoveeHandlerTest.java | 5 +- 11 files changed, 189 insertions(+), 40 deletions(-) create mode 100644 bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeStateDescriptionProvider.java create mode 100644 bundles/org.openhab.binding.govee/src/main/resources/OH-INF/update/instructions.xml diff --git a/bundles/org.openhab.binding.govee/README.md b/bundles/org.openhab.binding.govee/README.md index 0c0fc9dfafa5a..1320a1a3c6255 100644 --- a/bundles/org.openhab.binding.govee/README.md +++ b/bundles/org.openhab.binding.govee/README.md @@ -172,11 +172,13 @@ arp -a | grep "MAC_ADDRESS" ### `govee-light` Thing Configuration -| Name | Type | Description | Default | Required | Advanced | -|-----------------|---------|---------------------------------------|---------|----------|----------| -| hostname | text | Hostname or IP address of the device | N/A | yes | no | -| macAddress | text | MAC address of the device | N/A | yes | no | -| refreshInterval | integer | Interval the device is polled in sec. | 5 | no | yes | +| Name | Type | Description | Default | Required | Advanced | +|-----------------|---------|------------------------------------------------------------------|---------|----------|----------| +| hostname | text | Hostname or IP address of the device | N/A | yes | no | +| macAddress | text | MAC address of the device | N/A | yes | no | +| refreshInterval | integer | Interval the device is polled in sec. | 5 | no | yes | +| minKelvin | integer | The minimum color temperature that the light supports in Kelvin. | N/A | no | yes | +| maxKelvin | integer | The maximum color temperature that the light supports in Kelvin. | N/A | no | yes | ## Channels diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeConfiguration.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeConfiguration.java index 23932e8440da3..fe2d0d6a8d197 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeConfiguration.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeConfiguration.java @@ -13,6 +13,7 @@ package org.openhab.binding.govee.internal; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; /** * The {@link GoveeConfiguration} contains thing values that are used by the Thing Handler @@ -24,4 +25,7 @@ public class GoveeConfiguration { public String hostname = ""; public int refreshInterval = 5; // in seconds + + public @Nullable Integer minKelvin; + public @Nullable Integer maxKelvin; } diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 5994a8bee0886..aaf84110563db 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -15,6 +15,7 @@ import static org.openhab.binding.govee.internal.GoveeBindingConstants.*; import java.io.IOException; +import java.util.Objects; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -88,12 +89,16 @@ public class GoveeHandler extends BaseThingHandler { private ScheduledFuture triggerStatusJob; // send device status update job private GoveeConfiguration goveeConfiguration = new GoveeConfiguration(); - private CommunicationManager communicationManager; + private final CommunicationManager communicationManager; + private final GoveeStateDescriptionProvider stateDescriptionProvider; private OnOffType lastOn = OnOffType.OFF; private HSBType lastColor = new HSBType(); private int lastKelvin; + private int minKelvin; + private int maxKelvin; + /** * This thing related job thingRefreshSender triggers an update to the Govee device. * The device sends it back to the common port and the response is @@ -110,9 +115,11 @@ public class GoveeHandler extends BaseThingHandler { } }; - public GoveeHandler(Thing thing, CommunicationManager communicationManager) { + public GoveeHandler(Thing thing, CommunicationManager communicationManager, + GoveeStateDescriptionProvider stateDescriptionProvider) { super(thing); this.communicationManager = communicationManager; + this.stateDescriptionProvider = stateDescriptionProvider; } public String getHostname() { @@ -129,11 +136,22 @@ public void initialize() { "@text/offline.configuration-error.ip-address.missing"); return; } + + minKelvin = Objects.requireNonNullElse(goveeConfiguration.minKelvin, COLOR_TEMPERATURE_MIN_VALUE.intValue()); + maxKelvin = Objects.requireNonNullElse(goveeConfiguration.maxKelvin, COLOR_TEMPERATURE_MAX_VALUE.intValue()); + if ((minKelvin < COLOR_TEMPERATURE_MIN_VALUE) || (maxKelvin > COLOR_TEMPERATURE_MAX_VALUE) + || (minKelvin >= maxKelvin)) { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, + "@text/offline.configuration-error.invalid-color-temperature-range"); + return; + } + stateDescriptionProvider.setMinMaxKelvin(new ChannelUID(thing.getUID(), CHANNEL_COLOR_TEMPERATURE_ABS), + minKelvin, maxKelvin); + updateStatus(ThingStatus.UNKNOWN); communicationManager.registerHandler(this); if (triggerStatusJob == null) { logger.debug("Starting refresh trigger job for thing {} ", thing.getLabel()); - triggerStatusJob = executorService.scheduleWithFixedDelay(thingRefreshSender, 100, goveeConfiguration.refreshInterval * 1000L, TimeUnit.MILLISECONDS); } @@ -329,8 +347,7 @@ public void updateDeviceState(@Nullable StatusResponse message) { if (kelvin != lastKelvin) { logger.trace("Update color temperature old:{} to new:{}", lastKelvin, kelvin); if (kelvin != 0) { - kelvin = Math.min(COLOR_TEMPERATURE_MAX_VALUE.intValue(), - Math.max(COLOR_TEMPERATURE_MIN_VALUE.intValue(), kelvin)); + kelvin = Math.round(Math.min(maxKelvin, Math.max(minKelvin, kelvin))); updateState(CHANNEL_COLOR_TEMPERATURE, kelvinToPercent(kelvin)); updateState(CHANNEL_COLOR_TEMPERATURE_ABS, QuantityType.valueOf(kelvin, Units.KELVIN)); } else { @@ -344,17 +361,14 @@ public void updateDeviceState(@Nullable StatusResponse message) { /** * Convert PercentType to Kelvin. */ - private static int percentToKelvin(PercentType percent) { - return (int) Math - .round((((COLOR_TEMPERATURE_MAX_VALUE - COLOR_TEMPERATURE_MIN_VALUE) * percent.doubleValue() / 100.0) - + COLOR_TEMPERATURE_MIN_VALUE)); + private int percentToKelvin(PercentType percent) { + return (int) Math.round((((maxKelvin - minKelvin) * percent.doubleValue() / 100.0) + minKelvin)); } /** * Convert Kelvin to PercentType. */ - private static PercentType kelvinToPercent(int kelvin) { - return new PercentType((int) Math.round((kelvin - COLOR_TEMPERATURE_MIN_VALUE) * 100.0 - / (COLOR_TEMPERATURE_MAX_VALUE - COLOR_TEMPERATURE_MIN_VALUE))); + private PercentType kelvinToPercent(int kelvin) { + return new PercentType((int) Math.round((kelvin - minKelvin) * 100.0 / (maxKelvin - minKelvin))); } } diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandlerFactory.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandlerFactory.java index 907c3d41426d9..63a539ff8011a 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandlerFactory.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandlerFactory.java @@ -38,11 +38,14 @@ public class GoveeHandlerFactory extends BaseThingHandlerFactory { private static final Set SUPPORTED_THING_TYPES_UIDS = Set.of(THING_TYPE_LIGHT); - private CommunicationManager communicationManager; + private final CommunicationManager communicationManager; + private final GoveeStateDescriptionProvider stateDescriptionProvider; @Activate - public GoveeHandlerFactory(@Reference CommunicationManager communicationManager) { + public GoveeHandlerFactory(@Reference CommunicationManager communicationManager, + @Reference GoveeStateDescriptionProvider stateDescriptionProvider) { this.communicationManager = communicationManager; + this.stateDescriptionProvider = stateDescriptionProvider; } @Override @@ -55,7 +58,7 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) { ThingTypeUID thingTypeUID = thing.getThingTypeUID(); if (THING_TYPE_LIGHT.equals(thingTypeUID)) { - return new GoveeHandler(thing, communicationManager); + return new GoveeHandler(thing, communicationManager, stateDescriptionProvider); } return null; diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeStateDescriptionProvider.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeStateDescriptionProvider.java new file mode 100644 index 0000000000000..b35e3e8ef4e6f --- /dev/null +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeStateDescriptionProvider.java @@ -0,0 +1,83 @@ +/** + * Copyright (c) 2010-2024 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.govee.internal; + +import java.math.BigDecimal; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.events.EventPublisher; +import org.openhab.core.thing.Channel; +import org.openhab.core.thing.ChannelUID; +import org.openhab.core.thing.binding.BaseDynamicStateDescriptionProvider; +import org.openhab.core.thing.events.ThingEventFactory; +import org.openhab.core.thing.i18n.ChannelTypeI18nLocalizationService; +import org.openhab.core.thing.link.ItemChannelLinkRegistry; +import org.openhab.core.thing.type.DynamicStateDescriptionProvider; +import org.openhab.core.types.StateDescription; +import org.openhab.core.types.StateDescriptionFragment; +import org.openhab.core.types.StateDescriptionFragmentBuilder; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; + +/** + * The {@link GoveeStateDescriptionProvider} provides state descriptions for different color temperature ranges. + * + * @author Andrew Fiddian-Green - Initial contribution + * + */ +@NonNullByDefault +@Component(service = { DynamicStateDescriptionProvider.class, GoveeStateDescriptionProvider.class }) +public class GoveeStateDescriptionProvider extends BaseDynamicStateDescriptionProvider { + + private final Map stateDescriptionFragments = new ConcurrentHashMap<>(); + + @Activate + public GoveeStateDescriptionProvider(final @Reference EventPublisher eventPublisher, + final @Reference ItemChannelLinkRegistry itemChannelLinkRegistry, + final @Reference ChannelTypeI18nLocalizationService channelTypeI18nLocalizationService) { + this.eventPublisher = eventPublisher; + this.itemChannelLinkRegistry = itemChannelLinkRegistry; + this.channelTypeI18nLocalizationService = channelTypeI18nLocalizationService; + } + + @Override + public @Nullable StateDescription getStateDescription(Channel channel, @Nullable StateDescription original, + @Nullable Locale locale) { + StateDescriptionFragment stateDescriptionFragment = stateDescriptionFragments.get(channel.getUID()); + return stateDescriptionFragment != null ? stateDescriptionFragment.toStateDescription() + : super.getStateDescription(channel, original, locale); + } + + /** + * Set the state description minimum and maximum values and pattern in Kelvin for the given channel UID + */ + public void setMinMaxKelvin(ChannelUID channelUID, long minKelvin, long maxKelvin) { + StateDescriptionFragment oldStateDescriptionFragment = stateDescriptionFragments.get(channelUID); + StateDescriptionFragment newStateDescriptionFragment = StateDescriptionFragmentBuilder.create() + .withMinimum(BigDecimal.valueOf(minKelvin)).withMaximum(BigDecimal.valueOf(maxKelvin)) + .withStep(BigDecimal.valueOf(100)).withPattern("%.0f K").build(); + if (!newStateDescriptionFragment.equals(oldStateDescriptionFragment)) { + stateDescriptionFragments.put(channelUID, newStateDescriptionFragment); + ItemChannelLinkRegistry itemChannelLinkRegistry = this.itemChannelLinkRegistry; + postEvent(ThingEventFactory.createChannelDescriptionChangedEvent(channelUID, + itemChannelLinkRegistry != null ? itemChannelLinkRegistry.getLinkedItemNames(channelUID) : Set.of(), + newStateDescriptionFragment, oldStateDescriptionFragment)); + } + } +} diff --git a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/config/config.xml b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/config/config.xml index e153efa4d8067..ba8d9a650d948 100644 --- a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/config/config.xml +++ b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/config/config.xml @@ -19,6 +19,16 @@ The amount of time that passes until the device is refreshed (in seconds) 2 + + + The minimum color temperature that the light supports (in Kelvin) + true + + + + The maximum color temperature that the light supports (in Kelvin) + true + diff --git a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties index 2fd545a61a011..fcced7de99c1d 100644 --- a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties +++ b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties @@ -1,5 +1,28 @@ # add-on +addon.govee.name = Govee Lan-API Binding +addon.govee.description = This is the binding for handling Govee Lights via the LAN-API interface. + +# thing types + +thing-type.govee.govee-light.label = Govee Light +thing-type.govee.govee-light.description = Govee light controllable via LAN API + +# thing types config + +thing-type.config.govee.govee-light.hostname.label = Hostname/IP Address +thing-type.config.govee.govee-light.hostname.description = Hostname or IP address of the device +thing-type.config.govee.govee-light.macAddress.label = MAC Address +thing-type.config.govee.govee-light.macAddress.description = MAC Address of the device +thing-type.config.govee.govee-light.maxKelvin.label = Maximum Color Temperature +thing-type.config.govee.govee-light.maxKelvin.description = The maximum color temperature that the light supports (in Kelvin) +thing-type.config.govee.govee-light.minKelvin.label = Minimum Color Temperature +thing-type.config.govee.govee-light.minKelvin.description = The minimum color temperature that the light supports (in Kelvin) +thing-type.config.govee.govee-light.refreshInterval.label = Light Refresh Interval +thing-type.config.govee.govee-light.refreshInterval.description = The amount of time that passes until the device is refreshed (in seconds) + +# add-on + addon.name = Govee Binding addon.description = This is the binding for handling Govee Lights via the LAN-API interface. @@ -137,3 +160,4 @@ discovery.govee-light.H805C = H805C Permanent Outdoor Lights Elite 45M offline.communication-error.could-not-query-device = Could not control/query device at IP address {0} offline.configuration-error.ip-address.missing = IP address is missing offline.communication-error.empty-response = Empty response received +offline.configuration-error.invalid-color-temperature-range = Invalid color temperature range diff --git a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/thing/thing-types.xml b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/thing/thing-types.xml index 1a49746b3158a..b96fbc8d57fc4 100644 --- a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/thing/thing-types.xml +++ b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/thing/thing-types.xml @@ -11,22 +11,14 @@ - + - - + + 1 + - - Number:Temperature - - Controls the color temperature of the light in Kelvin - ColorLight - - Control - ColorTemperature - - - + + diff --git a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/update/instructions.xml b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/update/instructions.xml new file mode 100644 index 0000000000000..9317395b6a8de --- /dev/null +++ b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/update/instructions.xml @@ -0,0 +1,14 @@ + + + + + + + system:color-temperature-abs + + + + + diff --git a/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeHandlerMock.java b/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeHandlerMock.java index 7f048867a118f..f3374b2453ea8 100644 --- a/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeHandlerMock.java +++ b/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeHandlerMock.java @@ -12,8 +12,7 @@ */ package org.openhab.binding.govee.internal; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.doAnswer; import java.util.concurrent.ScheduledExecutorService; @@ -26,14 +25,15 @@ /** * The {@link GoveeHandlerMock} is responsible for mocking {@link GoveeHandler} - * + * * @author Leo Siepel - Initial contribution */ @NonNullByDefault public class GoveeHandlerMock extends GoveeHandler { - public GoveeHandlerMock(Thing thing, CommunicationManager communicationManager) { - super(thing, communicationManager); + public GoveeHandlerMock(Thing thing, CommunicationManager communicationManager, + GoveeStateDescriptionProvider stateDescriptionProvider) { + super(thing, communicationManager, stateDescriptionProvider); executorService = Mockito.mock(ScheduledExecutorService.class); doAnswer((InvocationOnMock invocation) -> { diff --git a/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java b/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java index 6781f8ca32a5b..46565a1a66a68 100644 --- a/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java +++ b/bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java @@ -83,7 +83,10 @@ private static Channel mockChannel(final ThingUID thingId, final String channelI private static GoveeHandlerMock createAndInitHandler(final ThingHandlerCallback callback, final Thing thing) { CommunicationManager communicationManager = mock(CommunicationManager.class); - final GoveeHandlerMock handler = spy(new GoveeHandlerMock(thing, communicationManager)); + GoveeStateDescriptionProvider stateDescriptionProvider = mock(GoveeStateDescriptionProvider.class); + + final GoveeHandlerMock handler = spy( + new GoveeHandlerMock(thing, communicationManager, stateDescriptionProvider)); handler.setCallback(callback); handler.initialize(); From 4c0ca10f03bfd379599891f574584e954bd2f40a Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Thu, 28 Nov 2024 17:21:30 +0000 Subject: [PATCH 09/35] refactoring Signed-off-by: AndrewFG --- .../binding/govee/internal/GoveeHandler.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index aaf84110563db..a88767492dbfb 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -92,7 +92,7 @@ public class GoveeHandler extends BaseThingHandler { private final CommunicationManager communicationManager; private final GoveeStateDescriptionProvider stateDescriptionProvider; - private OnOffType lastOn = OnOffType.OFF; + private OnOffType lastSwitch = OnOffType.OFF; private HSBType lastColor = new HSBType(); private int lastKelvin; @@ -321,8 +321,8 @@ public void updateDeviceState(@Nullable StatusResponse message) { logger.debug("updateDeviceState() for {}", thing.getUID()); - OnOffType on = OnOffType.from(message.msg().data().onOff() == 1); - logger.trace("- on:{}", on); + OnOffType switchValue = OnOffType.from(message.msg().data().onOff() == 1); + logger.trace("- switch:{}", switchValue); int brightness = message.msg().data().brightness(); logger.trace("- brightness:{}", brightness); @@ -335,11 +335,11 @@ public void updateDeviceState(@Nullable StatusResponse message) { HSBType color = buildHSB(normalRGB, brightness, true); - logger.trace("Compare color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastOn, on); - if ((on != lastOn) || !color.equals(lastColor)) { - logger.trace("Update color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastOn, on); - updateState(CHANNEL_COLOR, buildHSB(normalRGB, brightness, on == OnOffType.ON)); - lastOn = on; + logger.trace("Compare color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastSwitch, switchValue); + if ((switchValue != lastSwitch) || !color.equals(lastColor)) { + logger.trace("Update color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastSwitch, switchValue); + updateState(CHANNEL_COLOR, buildHSB(normalRGB, brightness, switchValue == OnOffType.ON)); + lastSwitch = switchValue; lastColor = color; } From 88afd3df3d6b46ac35390eb7805a3483b181be0c Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Thu, 28 Nov 2024 18:52:46 +0000 Subject: [PATCH 10/35] trigger refresh on separate thread Signed-off-by: AndrewFG --- .../binding/govee/internal/GoveeHandler.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index a88767492dbfb..072f4676ffa09 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -196,14 +196,14 @@ public void handleCommand(ChannelUID channelUID, Command command) { triggerRefresh = true; } if (triggerRefresh) { - triggerDeviceStatusRefresh(); + scheduler.submit(thingRefreshSender); } break; case CHANNEL_COLOR_TEMPERATURE: if (command instanceof PercentType percent) { sendKelvin(percentToKelvin(percent)); - triggerDeviceStatusRefresh(); + scheduler.submit(thingRefreshSender); } break; @@ -215,7 +215,7 @@ public void handleCommand(ChannelUID channelUID, Command command) { break; } sendKelvin(kelvin.intValue()); - triggerDeviceStatusRefresh(); + scheduler.submit(thingRefreshSender); } break; } @@ -321,8 +321,8 @@ public void updateDeviceState(@Nullable StatusResponse message) { logger.debug("updateDeviceState() for {}", thing.getUID()); - OnOffType switchValue = OnOffType.from(message.msg().data().onOff() == 1); - logger.trace("- switch:{}", switchValue); + OnOffType sw = OnOffType.from(message.msg().data().onOff() == 1); + logger.trace("- switch:{}", sw); int brightness = message.msg().data().brightness(); logger.trace("- brightness:{}", brightness); @@ -335,11 +335,11 @@ public void updateDeviceState(@Nullable StatusResponse message) { HSBType color = buildHSB(normalRGB, brightness, true); - logger.trace("Compare color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastSwitch, switchValue); - if ((switchValue != lastSwitch) || !color.equals(lastColor)) { - logger.trace("Update color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastSwitch, switchValue); - updateState(CHANNEL_COLOR, buildHSB(normalRGB, brightness, switchValue == OnOffType.ON)); - lastSwitch = switchValue; + logger.trace("Compare color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastSwitch, sw); + if ((sw != lastSwitch) || !color.equals(lastColor)) { + logger.trace("Update color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastSwitch, sw); + updateState(CHANNEL_COLOR, buildHSB(normalRGB, brightness, sw == OnOffType.ON)); + lastSwitch = sw; lastColor = color; } From 7d581b9460441916ef5b08b24856f78a76e229f2 Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Thu, 28 Nov 2024 20:08:42 +0000 Subject: [PATCH 11/35] add inter command sleep Signed-off-by: AndrewFG --- .../binding/govee/internal/GoveeHandler.java | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 072f4676ffa09..6cb0f993c0f8b 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -99,6 +99,8 @@ public class GoveeHandler extends BaseThingHandler { private int minKelvin; private int maxKelvin; + private static final int INTER_COMMAND_SLEEP_MILLISEC = 100; + /** * This thing related job thingRefreshSender triggers an update to the Govee device. * The device sends it back to the common port and the response is @@ -184,26 +186,32 @@ public void handleCommand(ChannelUID channelUID, Command command) { if (doCommand instanceof HSBType hsb) { sendColor(hsb); triggerRefresh = true; - doCommand = hsb.getBrightness(); // fall through + doCommand = hsb.getBrightness(); + Thread.sleep(INTER_COMMAND_SLEEP_MILLISEC); + // fall through } if (doCommand instanceof PercentType percent) { sendBrightness(percent); triggerRefresh = true; - doCommand = OnOffType.from(percent.intValue() > 0); // fall through + doCommand = OnOffType.from(percent.intValue() > 0); + Thread.sleep(INTER_COMMAND_SLEEP_MILLISEC); + // fall through } if (doCommand instanceof OnOffType onOff) { sendOnOff(onOff); triggerRefresh = true; + Thread.sleep(INTER_COMMAND_SLEEP_MILLISEC); } if (triggerRefresh) { - scheduler.submit(thingRefreshSender); + triggerDeviceStatusRefresh(); } break; case CHANNEL_COLOR_TEMPERATURE: if (command instanceof PercentType percent) { sendKelvin(percentToKelvin(percent)); - scheduler.submit(thingRefreshSender); + Thread.sleep(INTER_COMMAND_SLEEP_MILLISEC); + triggerDeviceStatusRefresh(); } break; @@ -215,7 +223,8 @@ public void handleCommand(ChannelUID channelUID, Command command) { break; } sendKelvin(kelvin.intValue()); - scheduler.submit(thingRefreshSender); + Thread.sleep(INTER_COMMAND_SLEEP_MILLISEC); + triggerDeviceStatusRefresh(); } break; } @@ -225,6 +234,8 @@ public void handleCommand(ChannelUID channelUID, Command command) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "@text/offline.communication-error.could-not-query-device [\"" + goveeConfiguration.hostname + "\"]"); + } catch (InterruptedException e) { + logger.debug("Inter command sleep was interrupted"); } } From 1590fd63d24e48729c11c7f6b0f26763eeec9316 Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Fri, 29 Nov 2024 14:07:40 +0000 Subject: [PATCH 12/35] tweak logging Signed-off-by: AndrewFG --- .../openhab/binding/govee/internal/CommunicationManager.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index a7f1b5c05971c..546d4798968df 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -114,7 +114,7 @@ public void sendRequest(GoveeHandler handler, GenericGoveeRequest request) throw final byte[] data = message.getBytes(); final InetAddress address = InetAddress.getByName(hostname); DatagramPacket packet = new DatagramPacket(data, data.length, address, REQUEST_PORT); - logger.trace("Sending request to {} with content = {}", address.getHostAddress(), message); + logger.trace("Sending request to {} on {} with content = {}", hostname, address.getHostAddress(), message); socket.send(packet); socket.close(); } @@ -254,7 +254,8 @@ public void run() { if (handler == null) { logger.warn("Handler not found for {}", deviceIPAddress); } else { - logger.debug("Processing response for {}", deviceIPAddress); + logger.debug("Processing response for {} on {}", handler.getHostname(), + deviceIPAddress); handler.handleIncomingStatus(response); } } From 190520f74ed817e52f049bd2a63b953d840d5536 Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Fri, 29 Nov 2024 14:08:16 +0000 Subject: [PATCH 13/35] gnarly thread synchronization code Signed-off-by: AndrewFG --- .../binding/govee/internal/GoveeHandler.java | 143 +++++++++++------- 1 file changed, 87 insertions(+), 56 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 6cb0f993c0f8b..3a0ac26581c44 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -15,7 +15,10 @@ import static org.openhab.binding.govee.internal.GoveeBindingConstants.*; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -99,7 +102,7 @@ public class GoveeHandler extends BaseThingHandler { private int minKelvin; private int maxKelvin; - private static final int INTER_COMMAND_SLEEP_MILLISEC = 100; + private static final int INTER_COMMAND_DELAY_MILLISEC = 100; /** * This thing related job thingRefreshSender triggers an update to the Govee device. @@ -173,69 +176,97 @@ public void dispose() { @Override public void handleCommand(ChannelUID channelUID, Command command) { - try { - logger.debug("handleCommand({}, {})", channelUID, command); - if (command instanceof RefreshType) { - // we are refreshing all channels at once, as we get all information at the same time - triggerDeviceStatusRefresh(); - } else { - switch (channelUID.getId()) { - case CHANNEL_COLOR: - boolean triggerRefresh = false; - Command doCommand = command; - if (doCommand instanceof HSBType hsb) { + logger.debug("handleCommand({}, {})", channelUID, command); + + List> communicationTasks = new ArrayList<>(); + int nextCommTaskScheduleTime = 0; + + if (command instanceof RefreshType) { + // we are refreshing all channels at once, as we get all information at the same time + nextCommTaskScheduleTime++; // trigger refresh + } else { + switch (channelUID.getId()) { + case CHANNEL_COLOR: + Command doCommand = command; + if (doCommand instanceof HSBType hsb) { + communicationTasks.add(scheduler.schedule(() -> { sendColor(hsb); - triggerRefresh = true; - doCommand = hsb.getBrightness(); - Thread.sleep(INTER_COMMAND_SLEEP_MILLISEC); - // fall through - } - if (doCommand instanceof PercentType percent) { + return true; + }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); + nextCommTaskScheduleTime += INTER_COMMAND_DELAY_MILLISEC; + doCommand = hsb.getBrightness(); // fall through + } + if (doCommand instanceof PercentType percent) { + communicationTasks.add(scheduler.schedule(() -> { sendBrightness(percent); - triggerRefresh = true; - doCommand = OnOffType.from(percent.intValue() > 0); - Thread.sleep(INTER_COMMAND_SLEEP_MILLISEC); - // fall through - } - if (doCommand instanceof OnOffType onOff) { + return true; + }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); + nextCommTaskScheduleTime += INTER_COMMAND_DELAY_MILLISEC; + doCommand = OnOffType.from(percent.intValue() > 0); // fall through + } + if (doCommand instanceof OnOffType onOff) { + communicationTasks.add(scheduler.schedule(() -> { sendOnOff(onOff); - triggerRefresh = true; - Thread.sleep(INTER_COMMAND_SLEEP_MILLISEC); - } - if (triggerRefresh) { - triggerDeviceStatusRefresh(); - } - break; - - case CHANNEL_COLOR_TEMPERATURE: - if (command instanceof PercentType percent) { + return true; + }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); + nextCommTaskScheduleTime += INTER_COMMAND_DELAY_MILLISEC; + } + break; + + case CHANNEL_COLOR_TEMPERATURE: + if (command instanceof PercentType percent) { + communicationTasks.add(scheduler.schedule(() -> { sendKelvin(percentToKelvin(percent)); - Thread.sleep(INTER_COMMAND_SLEEP_MILLISEC); - triggerDeviceStatusRefresh(); + return true; + }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); + nextCommTaskScheduleTime += INTER_COMMAND_DELAY_MILLISEC; + } + break; + + case CHANNEL_COLOR_TEMPERATURE_ABS: + if (command instanceof QuantityType genericQuantity) { + QuantityType kelvin = genericQuantity.toInvertibleUnit(Units.KELVIN); + if (kelvin == null) { + logger.warn("handleCommand() invalid QuantityType:{}", genericQuantity); + break; } - break; - - case CHANNEL_COLOR_TEMPERATURE_ABS: - if (command instanceof QuantityType genericQuantity) { - QuantityType kelvin = genericQuantity.toInvertibleUnit(Units.KELVIN); - if (kelvin == null) { - logger.warn("handleCommand() invalid QuantityType:{}", genericQuantity); - break; - } + communicationTasks.add(scheduler.schedule(() -> { sendKelvin(kelvin.intValue()); - Thread.sleep(INTER_COMMAND_SLEEP_MILLISEC); - triggerDeviceStatusRefresh(); + return true; + }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); + nextCommTaskScheduleTime += INTER_COMMAND_DELAY_MILLISEC; + } + break; + } + + if (nextCommTaskScheduleTime > 0) { + communicationTasks.add(scheduler.schedule(() -> { + triggerDeviceStatusRefresh(); + return true; + }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); + + scheduler.schedule(() -> { + boolean communicationError = false; + for (ScheduledFuture communicationTask : communicationTasks) { + try { + communicationTask.get(); + } catch (InterruptedException e) { + logger.debug("Communication task interrupted"); + break; + } catch (ExecutionException e) { + communicationError = true; + break; } - break; - } + } + if (!communicationError) { + updateStatus(ThingStatus.ONLINE); + } else { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, + "@text/offline.communication-error.could-not-query-device [\"" + + goveeConfiguration.hostname + "\"]"); + } + }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS); } - updateStatus(ThingStatus.ONLINE); - } catch (IOException e) { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, - "@text/offline.communication-error.could-not-query-device [\"" + goveeConfiguration.hostname - + "\"]"); - } catch (InterruptedException e) { - logger.debug("Inter command sleep was interrupted"); } } From 3f26176cca014ad13caf6416a8afced807335b37 Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Fri, 29 Nov 2024 14:34:08 +0000 Subject: [PATCH 14/35] restore * tags Signed-off-by: AndrewFG --- bundles/org.openhab.binding.govee/README.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/bundles/org.openhab.binding.govee/README.md b/bundles/org.openhab.binding.govee/README.md index 1320a1a3c6255..4ebb4a95aa560 100644 --- a/bundles/org.openhab.binding.govee/README.md +++ b/bundles/org.openhab.binding.govee/README.md @@ -28,16 +28,16 @@ Here is a list of the supported devices (the ones marked with * have been tested - H6052 Govee Table Lamp - H6056 H6056 Flow Plus - H6059 RGBWW Night Light for Kids -- H6061 Glide Hexa LED Panels +- H6061 Glide Hexa LED Panels (*) - H6062 Glide Wall Light - H6063 Gaming Wall Light - H6065 Glide RGBIC Y Lights - H6066 Glide Hexa Pro LED Panel -- H6067 Glide Triangle Light Panels +- H6067 Glide Triangle Light Panels (*) - H606A Glide Hexa Light Panel Ultra -- H6072 RGBICWW Corner Floor Lamp +- H6072 RGBICWW Corner Floor Lamp (*) - H6073 LED Floor Lamp -- H6076 RGBICW Smart Corner Floor Lamp +- H6076 RGBICW Smart Corner Floor Lamp (*) - H6078 Cylinder Floor Lamp - H607C Floor Lamp #2 - H6087 RGBIC Smart Wall Sconces @@ -46,7 +46,7 @@ Here is a list of the supported devices (the ones marked with * have been tested - H608B String Downlights 3M - H608C String Downlights 2M - H608D String Downlights 10M -- H60A0 Ceiling Light +- H60A0 Ceiling Light (*) - H60A1 Smart Ceiling Light - H610A Glide Lively Wall Lights - H610B Music Wall Lights @@ -55,7 +55,7 @@ Here is a list of the supported devices (the ones marked with * have been tested - H6141 5M Smart Multicolor Strip Light - H6143 5M Strip Light - H6144 2x5M Strip Light -- H6159 RGB Light Strip +- H6159 RGB Light Strip (*) - H615A 5M Light Strip with Alexa - H615B 10M Light Strip with Alexa - H615C 15M Light Strip with Alexa @@ -97,8 +97,8 @@ Here is a list of the supported devices (the ones marked with * have been tested - H61C2 Neon LED Strip Light 2M - H61C2 Neon LED Strip Light 3M - H61C2 Neon LED Strip Light 5M -- H61D3 Neon Rope Light 2 3m -- H61D5 Neon Rope Light 2 5m +- H61D3 Neon Rope Light 2 3m (*) +- H61D5 Neon Rope Light 2 5m (*) - H61E0 LED Strip Light M1 - H61E1 LED Strip Light M1 - H7012 Warm White Outdoor String Lights @@ -131,8 +131,8 @@ Here is a list of the supported devices (the ones marked with * have been tested - H7075 Outdoor Wall Light - H70B1 520 LED Curtain Lights - H70BC 400 LED Curtain Lights -- H70C1 RGBIC String Light 10M -- H70C2 RGBIC String Light 20M +- H70C1 RGBIC String Light 10M (*) +- H70C2 RGBIC String Light 20M (*) - H805A Permanent Outdoor Lights Elite 30M - H805B Permanent Outdoor Lights Elite 15M - H805C Permanent Outdoor Lights Elite 45M From d3a3bf53bb35bed506c9afe2537c8c0f180843f9 Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Fri, 29 Nov 2024 14:46:56 +0000 Subject: [PATCH 15/35] avoid potential threadlock Signed-off-by: AndrewFG --- .../java/org/openhab/binding/govee/internal/GoveeHandler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 3a0ac26581c44..f9d753ad3b315 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -244,6 +244,7 @@ public void handleCommand(ChannelUID channelUID, Command command) { triggerDeviceStatusRefresh(); return true; }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); + nextCommTaskScheduleTime += 5; scheduler.schedule(() -> { boolean communicationError = false; From 28bbd1da698c8a9514ac6998f07e8558a4f36f8d Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Fri, 29 Nov 2024 18:09:32 +0000 Subject: [PATCH 16/35] improved thread synchronization Signed-off-by: AndrewFG --- .../binding/govee/internal/GoveeHandler.java | 118 ++++++++++-------- 1 file changed, 66 insertions(+), 52 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index f9d753ad3b315..4f279093205ac 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -17,8 +17,9 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.ListIterator; import java.util.Objects; -import java.util.concurrent.ExecutionException; +import java.util.concurrent.Callable; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -175,51 +176,48 @@ public void dispose() { } @Override - public void handleCommand(ChannelUID channelUID, Command command) { + public void handleCommand(ChannelUID channelUID, Command commandParam) { + Command command = commandParam; + logger.debug("handleCommand({}, {})", channelUID, command); - List> communicationTasks = new ArrayList<>(); - int nextCommTaskScheduleTime = 0; + List> communicationTasks = new ArrayList<>(); + boolean mustRefresh = false; if (command instanceof RefreshType) { // we are refreshing all channels at once, as we get all information at the same time - nextCommTaskScheduleTime++; // trigger refresh + mustRefresh = true; } else { switch (channelUID.getId()) { case CHANNEL_COLOR: - Command doCommand = command; - if (doCommand instanceof HSBType hsb) { - communicationTasks.add(scheduler.schedule(() -> { + if (command instanceof HSBType hsb) { + communicationTasks.add(() -> { sendColor(hsb); return true; - }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); - nextCommTaskScheduleTime += INTER_COMMAND_DELAY_MILLISEC; - doCommand = hsb.getBrightness(); // fall through + }); + command = hsb.getBrightness(); // fall through } - if (doCommand instanceof PercentType percent) { - communicationTasks.add(scheduler.schedule(() -> { + if (command instanceof PercentType percent) { + communicationTasks.add(() -> { sendBrightness(percent); return true; - }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); - nextCommTaskScheduleTime += INTER_COMMAND_DELAY_MILLISEC; - doCommand = OnOffType.from(percent.intValue() > 0); // fall through + }); + command = OnOffType.from(percent.intValue() > 0); // fall through } - if (doCommand instanceof OnOffType onOff) { - communicationTasks.add(scheduler.schedule(() -> { + if (command instanceof OnOffType onOff) { + communicationTasks.add(() -> { sendOnOff(onOff); return true; - }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); - nextCommTaskScheduleTime += INTER_COMMAND_DELAY_MILLISEC; + }); } break; case CHANNEL_COLOR_TEMPERATURE: if (command instanceof PercentType percent) { - communicationTasks.add(scheduler.schedule(() -> { + communicationTasks.add(() -> { sendKelvin(percentToKelvin(percent)); return true; - }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); - nextCommTaskScheduleTime += INTER_COMMAND_DELAY_MILLISEC; + }); } break; @@ -230,45 +228,61 @@ public void handleCommand(ChannelUID channelUID, Command command) { logger.warn("handleCommand() invalid QuantityType:{}", genericQuantity); break; } - communicationTasks.add(scheduler.schedule(() -> { + communicationTasks.add(() -> { sendKelvin(kelvin.intValue()); return true; - }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); - nextCommTaskScheduleTime += INTER_COMMAND_DELAY_MILLISEC; + }); } break; } - if (nextCommTaskScheduleTime > 0) { - communicationTasks.add(scheduler.schedule(() -> { + if (mustRefresh || !communicationTasks.isEmpty()) { + communicationTasks.add(() -> { triggerDeviceStatusRefresh(); return true; - }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS)); - nextCommTaskScheduleTime += 5; - - scheduler.schedule(() -> { - boolean communicationError = false; - for (ScheduledFuture communicationTask : communicationTasks) { - try { - communicationTask.get(); - } catch (InterruptedException e) { - logger.debug("Communication task interrupted"); - break; - } catch (ExecutionException e) { - communicationError = true; - break; - } - } - if (!communicationError) { - updateStatus(ThingStatus.ONLINE); - } else { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, - "@text/offline.communication-error.could-not-query-device [\"" - + goveeConfiguration.hostname + "\"]"); - } - }, nextCommTaskScheduleTime, TimeUnit.MILLISECONDS); + }); + } + + if (!communicationTasks.isEmpty()) { + scheduler.submit(() -> runCallableTasks(communicationTasks)); + } + } + } + + /** + * Iterate over the given list of {@link Callable} tasks and run them with a delay between each. Reason for the + * delay is that Govee lamps cannot process multiple commands in short succession. Update the thing status depending + * on whether any of the tasks encountered an exception. + * + * @param callables a list of Callable tasks (which shall not be empty) + */ + @SuppressWarnings("null") + private void runCallableTasks(List> callables) { + boolean exception = false; + ListIterator> tasks = callables.listIterator(); + while (true) { // don't check hasNext() (we do it below) + try { + tasks.next().call(); + } catch (Exception e) { + exception = true; + break; + } + if (!tasks.hasNext()) { + break; + } + try { + Thread.sleep(INTER_COMMAND_DELAY_MILLISEC); + } catch (InterruptedException e) { + return; } } + if (!exception) { + updateStatus(ThingStatus.ONLINE); + } else { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, + "@text/offline.communication-error.could-not-query-device [\"" + goveeConfiguration.hostname + + "\"]"); + } } /** From b9f5384046241e0d8f339cc6f82505389f003bdf Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Sat, 30 Nov 2024 16:46:57 +0000 Subject: [PATCH 17/35] add (*) tag Signed-off-by: AndrewFG --- bundles/org.openhab.binding.govee/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.openhab.binding.govee/README.md b/bundles/org.openhab.binding.govee/README.md index 4ebb4a95aa560..f01ec0e4482a4 100644 --- a/bundles/org.openhab.binding.govee/README.md +++ b/bundles/org.openhab.binding.govee/README.md @@ -56,7 +56,7 @@ Here is a list of the supported devices (the ones marked with * have been tested - H6143 5M Strip Light - H6144 2x5M Strip Light - H6159 RGB Light Strip (*) -- H615A 5M Light Strip with Alexa +- H615A 5M Light Strip with Alexa (*) - H615B 10M Light Strip with Alexa - H615C 15M Light Strip with Alexa - H615D 20M Light Strip with Alexa From fcd032ccd9854a09cad37737c2306ed836f738be Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Sat, 30 Nov 2024 16:47:49 +0000 Subject: [PATCH 18/35] improve logging; add properties Signed-off-by: AndrewFG --- .../govee/internal/CommunicationManager.java | 5 ++-- .../govee/internal/GoveeBindingConstants.java | 3 ++ .../binding/govee/internal/GoveeHandler.java | 28 +++++++++++-------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index 546d4798968df..557135a53d0d2 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -114,7 +114,8 @@ public void sendRequest(GoveeHandler handler, GenericGoveeRequest request) throw final byte[] data = message.getBytes(); final InetAddress address = InetAddress.getByName(hostname); DatagramPacket packet = new DatagramPacket(data, data.length, address, REQUEST_PORT); - logger.trace("Sending request to {} on {} with content = {}", hostname, address.getHostAddress(), message); + logger.trace("Sending request to {} on {} with content = {}", handler.getThing().getUID(), + address.getHostAddress(), message); socket.send(packet); socket.close(); } @@ -254,7 +255,7 @@ public void run() { if (handler == null) { logger.warn("Handler not found for {}", deviceIPAddress); } else { - logger.debug("Processing response for {} on {}", handler.getHostname(), + logger.debug("Processing response for {} on {}", handler.getThing().getUID(), deviceIPAddress); handler.handleIncomingStatus(response); } diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeBindingConstants.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeBindingConstants.java index 35ca13bb647b1..e99c670b202c5 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeBindingConstants.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeBindingConstants.java @@ -44,4 +44,7 @@ public class GoveeBindingConstants { // Limit values of channels public static final Double COLOR_TEMPERATURE_MIN_VALUE = 2000.0; public static final Double COLOR_TEMPERATURE_MAX_VALUE = 9000.0; + + public static final String PROPERTY_COLOR_TEMPERATURE_MIN = "minKelvin"; + public static final String PROPERTY_COLOR_TEMPERATURE_MAX = "maxKelvin"; } diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 4f279093205ac..8e45613166a18 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -34,6 +34,7 @@ import org.openhab.binding.govee.internal.model.GenericGoveeRequest; import org.openhab.binding.govee.internal.model.StatusResponse; import org.openhab.binding.govee.internal.model.ValueIntData; +import org.openhab.core.library.types.DecimalType; import org.openhab.core.library.types.HSBType; import org.openhab.core.library.types.OnOffType; import org.openhab.core.library.types.PercentType; @@ -151,6 +152,8 @@ public void initialize() { "@text/offline.configuration-error.invalid-color-temperature-range"); return; } + thing.setProperty(PROPERTY_COLOR_TEMPERATURE_MIN, Integer.toString(minKelvin)); + thing.setProperty(PROPERTY_COLOR_TEMPERATURE_MAX, Integer.toString(maxKelvin)); stateDescriptionProvider.setMinMaxKelvin(new ChannelUID(thing.getUID(), CHANNEL_COLOR_TEMPERATURE_ABS), minKelvin, maxKelvin); @@ -232,6 +235,11 @@ public void handleCommand(ChannelUID channelUID, Command commandParam) { sendKelvin(kelvin.intValue()); return true; }); + } else if (command instanceof DecimalType kelvin) { + communicationTasks.add(() -> { + sendKelvin(kelvin.intValue()); + return true; + }); } break; } @@ -289,7 +297,7 @@ private void runCallableTasks(List> callables) { * Initiate a refresh to our thing device */ private void triggerDeviceStatusRefresh() throws IOException { - logger.debug("triggerDeviceStatusRefresh() on {}", thing.getUID()); + logger.debug("triggerDeviceStatusRefresh() to {}", thing.getUID()); GenericGoveeData data = new EmptyValueQueryStatusData(); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("devStatus", data)); communicationManager.sendRequest(this, request); @@ -379,30 +387,26 @@ public void updateDeviceState(@Nullable StatusResponse message) { logger.debug("updateDeviceState() for {}", thing.getUID()); OnOffType sw = OnOffType.from(message.msg().data().onOff() == 1); - logger.trace("- switch:{}", sw); - int brightness = message.msg().data().brightness(); - logger.trace("- brightness:{}", brightness); - Color normalRGB = message.msg().data().color(); - logger.trace("- normalRGB:{}", normalRGB); - int kelvin = message.msg().data().colorTemInKelvin(); - logger.trace("- kelvin:{}", kelvin); + + logger.trace("Update values: switch:{}, brightness:{}, normalRGB:{}, kelvin:{}", sw, brightness, normalRGB, + kelvin); HSBType color = buildHSB(normalRGB, brightness, true); - logger.trace("Compare color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastSwitch, sw); + logger.trace("Compare hsb old:{} to new:{}, switch old:{} to new:{}", lastColor, color, lastSwitch, sw); if ((sw != lastSwitch) || !color.equals(lastColor)) { - logger.trace("Update color old:{} to new:{}, on-state old:{} to new:{}", lastColor, color, lastSwitch, sw); + logger.trace("Update hsb old:{} to new:{}, switch old:{} to new:{}", lastColor, color, lastSwitch, sw); updateState(CHANNEL_COLOR, buildHSB(normalRGB, brightness, sw == OnOffType.ON)); lastSwitch = sw; lastColor = color; } - logger.trace("Compare color temperature old:{} to new:{}", lastKelvin, kelvin); + logger.trace("Compare kelvin old:{} to new:{}", lastKelvin, kelvin); if (kelvin != lastKelvin) { - logger.trace("Update color temperature old:{} to new:{}", lastKelvin, kelvin); + logger.trace("Update kelvin old:{} to new:{}", lastKelvin, kelvin); if (kelvin != 0) { kelvin = Math.round(Math.min(maxKelvin, Math.max(minKelvin, kelvin))); updateState(CHANNEL_COLOR_TEMPERATURE, kelvinToPercent(kelvin)); From 46809de2296febe8f15ed099ca390f81d97975ea Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Tue, 3 Dec 2024 18:26:01 +0000 Subject: [PATCH 19/35] fix discovery on multi head / multi protocol machines Signed-off-by: AndrewFG --- .../govee/internal/CommunicationManager.java | 58 +++++++++++-------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index 557135a53d0d2..c82aba3736654 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -15,14 +15,20 @@ import java.io.IOException; import java.net.DatagramPacket; import java.net.DatagramSocket; +import java.net.Inet4Address; import java.net.InetAddress; import java.net.InetSocketAddress; -import java.net.MulticastSocket; import java.net.NetworkInterface; +import java.net.StandardProtocolFamily; +import java.net.StandardSocketOptions; import java.net.UnknownHostException; +import java.nio.ByteBuffer; +import java.nio.channels.DatagramChannel; import java.time.Instant; +import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -138,24 +144,27 @@ public void runDiscoveryForInterface(NetworkInterface intf, DiscoveryResultRecei activeReceiver.setDiscoveryResultsReceiver(receiver); } - final InetAddress broadcastAddress = InetAddress.getByName(DISCOVERY_MULTICAST_ADDRESS); - final InetSocketAddress socketAddress = new InetSocketAddress(broadcastAddress, RESPONSE_PORT); - final Instant discoveryStartTime = Instant.now(); - final Instant discoveryEndTime = discoveryStartTime.plusSeconds(INTERFACE_TIMEOUT_SEC); + final Instant discoveryEndTime = Instant.now().plusSeconds(INTERFACE_TIMEOUT_SEC); - try (MulticastSocket sendSocket = new MulticastSocket(socketAddress)) { - sendSocket.setSoTimeout(INTERFACE_TIMEOUT_SEC * 1000); - sendSocket.setReuseAddress(true); - sendSocket.setBroadcast(true); - sendSocket.setTimeToLive(2); - sendSocket.joinGroup(new InetSocketAddress(broadcastAddress, RESPONSE_PORT), intf); - - byte[] requestData = DISCOVER_REQUEST.getBytes(); - - DatagramPacket request = new DatagramPacket(requestData, requestData.length, broadcastAddress, - DISCOVERY_PORT); - sendSocket.send(request); - } + Collections.list(intf.getInetAddresses()).stream().filter(address -> address instanceof Inet4Address) + .map(address -> address.getHostAddress()).forEach(ipv4Address -> { + try (DatagramChannel channel = (DatagramChannel) DatagramChannel + .open(StandardProtocolFamily.INET) + .setOption(StandardSocketOptions.SO_REUSEADDR, true) + .setOption(StandardSocketOptions.IP_MULTICAST_TTL, 64) + .setOption(StandardSocketOptions.IP_MULTICAST_IF, intf) + .bind(new InetSocketAddress(ipv4Address, DISCOVERY_PORT)) + .configureBlocking(false)) { + logger.trace("Datagram channel bound to {}:{} on {}", ipv4Address, DISCOVERY_PORT, + intf.getDisplayName()); + channel.send(ByteBuffer.wrap(DISCOVER_REQUEST.getBytes()), + new InetSocketAddress(DISCOVERY_MULTICAST_ADDRESS, DISCOVERY_PORT)); + logger.trace("Sent request to {}:{} with content = {}", DISCOVERY_MULTICAST_ADDRESS, + DISCOVERY_PORT, DISCOVER_REQUEST); + } catch (IOException e) { + logger.debug("Network error", e); + } + }); do { try { @@ -180,7 +189,7 @@ private class StatusReceiver extends Thread { private boolean stopped = false; private @Nullable DiscoveryResultReceiver discoveryResultReceiver; - private @Nullable MulticastSocket socket; + private @Nullable DatagramSocket socket; StatusReceiver() { super("GoveeStatusReceiver"); @@ -208,13 +217,13 @@ void stopReceiving() { public void run() { while (!stopped) { try { - socket = new MulticastSocket(RESPONSE_PORT); + socket = new DatagramSocket(RESPONSE_PORT); byte[] buffer = new byte[10240]; - socket.setReuseAddress(true); + Objects.requireNonNull(socket).setReuseAddress(true); while (!stopped) { DatagramPacket packet = new DatagramPacket(buffer, buffer.length); - if (!socket.isClosed()) { - socket.receive(packet); + if (!Objects.requireNonNull(socket).isClosed()) { + Objects.requireNonNull(socket).receive(packet); } else { logger.warn("Socket was unexpectedly closed"); break; @@ -262,12 +271,13 @@ public void run() { } } } catch (IOException e) { - logger.warn("Exception when receiving status packet", e); + logger.debug("Exception when receiving status packet {}", e.getMessage()); // as we haven't received a packet we also don't know where it should have come from // hence, we don't know which thing put offline. // a way to monitor this would be to keep track in a list, which device answers we expect // and supervise an expected answer within a given time but that will make the whole // mechanism much more complicated and may be added in the future + // PS it also seems to be 'normal' to encounter errors when in device discovery mode } finally { if (socket != null) { socket.close(); From febb8971262adc9c463e4b3b31699f0bd2faf348 Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Tue, 3 Dec 2024 18:52:04 +0000 Subject: [PATCH 20/35] move asterisk in read me Signed-off-by: AndrewFG --- bundles/org.openhab.binding.govee/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.binding.govee/README.md b/bundles/org.openhab.binding.govee/README.md index f01ec0e4482a4..13c31fe4b5e3d 100644 --- a/bundles/org.openhab.binding.govee/README.md +++ b/bundles/org.openhab.binding.govee/README.md @@ -46,8 +46,8 @@ Here is a list of the supported devices (the ones marked with * have been tested - H608B String Downlights 3M - H608C String Downlights 2M - H608D String Downlights 10M -- H60A0 Ceiling Light (*) -- H60A1 Smart Ceiling Light +- H60A0 Ceiling Light +- H60A1 Smart Ceiling Light (*) - H610A Glide Lively Wall Lights - H610B Music Wall Lights - H6110 2x5M Multicolor with Alexa From 34d9731bab5cc60d95c9c3c64ec02ef81400ae2d Mon Sep 17 00:00:00 2001 From: AndrewFG Date: Wed, 4 Dec 2024 16:14:59 +0000 Subject: [PATCH 21/35] normalize thread name (see #17804) Signed-off-by: AndrewFG --- .../openhab/binding/govee/internal/CommunicationManager.java | 2 +- .../openhab/binding/govee/internal/GoveeBindingConstants.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index c82aba3736654..952dbfac23fb0 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -192,7 +192,7 @@ private class StatusReceiver extends Thread { private @Nullable DatagramSocket socket; StatusReceiver() { - super("GoveeStatusReceiver"); + super("OH-binding-" + GoveeBindingConstants.BINDING_ID + "-StatusReceiver"); } synchronized void setDiscoveryResultsReceiver(@Nullable DiscoveryResultReceiver receiver) { diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeBindingConstants.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeBindingConstants.java index e99c670b202c5..713ac70e9bd51 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeBindingConstants.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeBindingConstants.java @@ -31,7 +31,7 @@ public class GoveeBindingConstants { public static final String PRODUCT_NAME = "productName"; public static final String HW_VERSION = "wifiHardwareVersion"; public static final String SW_VERSION = "wifiSoftwareVersion"; - private static final String BINDING_ID = "govee"; + public static final String BINDING_ID = "govee"; // List of all Thing Type UIDs public static final ThingTypeUID THING_TYPE_LIGHT = new ThingTypeUID(BINDING_ID, "govee-light"); From 80ce98227e210fa63b263feecb0b329a18744d60 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sun, 8 Dec 2024 10:57:34 +0000 Subject: [PATCH 22/35] adopt reviewer suggestions Signed-off-by: Andrew Fiddian-Green --- .../govee/internal/CommunicationManager.java | 10 ++-- .../binding/govee/internal/GoveeHandler.java | 50 +++++++------------ 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index 952dbfac23fb0..2d315ae6082df 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -28,7 +28,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import java.util.Objects; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -217,13 +216,14 @@ void stopReceiving() { public void run() { while (!stopped) { try { - socket = new DatagramSocket(RESPONSE_PORT); + DatagramSocket loopSocket = new DatagramSocket(RESPONSE_PORT); + this.socket = loopSocket; byte[] buffer = new byte[10240]; - Objects.requireNonNull(socket).setReuseAddress(true); + loopSocket.setReuseAddress(true); while (!stopped) { DatagramPacket packet = new DatagramPacket(buffer, buffer.length); - if (!Objects.requireNonNull(socket).isClosed()) { - Objects.requireNonNull(socket).receive(packet); + if (!loopSocket.isClosed()) { + loopSocket.receive(packet); } else { logger.warn("Socket was unexpectedly closed"); break; diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 8e45613166a18..3fd8d152d587c 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -194,33 +194,21 @@ public void handleCommand(ChannelUID channelUID, Command commandParam) { switch (channelUID.getId()) { case CHANNEL_COLOR: if (command instanceof HSBType hsb) { - communicationTasks.add(() -> { - sendColor(hsb); - return true; - }); + communicationTasks.add(() -> sendColor(hsb)); command = hsb.getBrightness(); // fall through } if (command instanceof PercentType percent) { - communicationTasks.add(() -> { - sendBrightness(percent); - return true; - }); + communicationTasks.add(() -> sendBrightness(percent)); command = OnOffType.from(percent.intValue() > 0); // fall through } if (command instanceof OnOffType onOff) { - communicationTasks.add(() -> { - sendOnOff(onOff); - return true; - }); + communicationTasks.add(() -> sendOnOff(onOff)); } break; case CHANNEL_COLOR_TEMPERATURE: if (command instanceof PercentType percent) { - communicationTasks.add(() -> { - sendKelvin(percentToKelvin(percent)); - return true; - }); + communicationTasks.add(() -> sendKelvin(percentToKelvin(percent))); } break; @@ -231,24 +219,15 @@ public void handleCommand(ChannelUID channelUID, Command commandParam) { logger.warn("handleCommand() invalid QuantityType:{}", genericQuantity); break; } - communicationTasks.add(() -> { - sendKelvin(kelvin.intValue()); - return true; - }); + communicationTasks.add(() -> sendKelvin(kelvin.intValue())); } else if (command instanceof DecimalType kelvin) { - communicationTasks.add(() -> { - sendKelvin(kelvin.intValue()); - return true; - }); + communicationTasks.add(() -> sendKelvin(kelvin.intValue())); } break; } if (mustRefresh || !communicationTasks.isEmpty()) { - communicationTasks.add(() -> { - triggerDeviceStatusRefresh(); - return true; - }); + communicationTasks.add(() -> triggerDeviceStatusRefresh()); } if (!communicationTasks.isEmpty()) { @@ -296,52 +275,57 @@ private void runCallableTasks(List> callables) { /** * Initiate a refresh to our thing device */ - private void triggerDeviceStatusRefresh() throws IOException { + private boolean triggerDeviceStatusRefresh() throws IOException { logger.debug("triggerDeviceStatusRefresh() to {}", thing.getUID()); GenericGoveeData data = new EmptyValueQueryStatusData(); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("devStatus", data)); communicationManager.sendRequest(this, request); + return true; } /** * Send the normalized RGB color parameters. */ - public void sendColor(HSBType color) throws IOException { + public boolean sendColor(HSBType color) throws IOException { logger.debug("sendColor({}) to {}", color, thing.getUID()); int[] normalRGB = ColorUtil.hsbToRgb(new HSBType(color.getHue(), color.getSaturation(), PercentType.HUNDRED)); GenericGoveeData data = new ColorData(new Color(normalRGB[0], normalRGB[1], normalRGB[2]), 0); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("colorwc", data)); communicationManager.sendRequest(this, request); + return true; } /** * Send the brightness parameter. */ - public void sendBrightness(PercentType brightness) throws IOException { + public boolean sendBrightness(PercentType brightness) throws IOException { logger.debug("sendBrightness({}) to {}", brightness, thing.getUID()); GenericGoveeData data = new ValueIntData(brightness.intValue()); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("brightness", data)); communicationManager.sendRequest(this, request); + return true; } /** * Send the on-off parameter. */ - private void sendOnOff(OnOffType onOff) throws IOException { + private boolean sendOnOff(OnOffType onOff) throws IOException { logger.debug("sendOnOff({}) to {}", onOff, thing.getUID()); GenericGoveeData data = new ValueIntData(onOff == OnOffType.ON ? 1 : 0); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("turn", data)); communicationManager.sendRequest(this, request); + return true; } /** * Set the color temperature (Kelvin) parameter. */ - private void sendKelvin(int kelvin) throws IOException { + private boolean sendKelvin(int kelvin) throws IOException { logger.debug("sendKelvin({}) to {}", kelvin, thing.getUID()); GenericGoveeData data = new ColorData(new Color(0, 0, 0), kelvin); GenericGoveeRequest request = new GenericGoveeRequest(new GenericGoveeMsg("colorwc", data)); communicationManager.sendRequest(this, request); + return true; } /** From 8bf37f736955ff932b02bba31730dd100fd4e3f6 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sun, 8 Dec 2024 11:07:50 +0000 Subject: [PATCH 23/35] adopt reviewer suggestion Signed-off-by: Andrew Fiddian-Green --- .../binding/govee/internal/CommunicationManager.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index 2d315ae6082df..3c86420c2db96 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -69,6 +69,9 @@ public class CommunicationManager { private static final String DISCOVER_REQUEST = "{\"msg\": {\"cmd\": \"scan\", \"data\": {\"account_topic\": \"reserve\"}}}"; + private static final InetSocketAddress DISCOVERY_SOCKET_ADDRESS = new InetSocketAddress(DISCOVERY_MULTICAST_ADDRESS, + DISCOVERY_PORT); + public interface DiscoveryResultReceiver { void onResultReceived(DiscoveryResponse result); } @@ -156,8 +159,7 @@ public void runDiscoveryForInterface(NetworkInterface intf, DiscoveryResultRecei .configureBlocking(false)) { logger.trace("Datagram channel bound to {}:{} on {}", ipv4Address, DISCOVERY_PORT, intf.getDisplayName()); - channel.send(ByteBuffer.wrap(DISCOVER_REQUEST.getBytes()), - new InetSocketAddress(DISCOVERY_MULTICAST_ADDRESS, DISCOVERY_PORT)); + channel.send(ByteBuffer.wrap(DISCOVER_REQUEST.getBytes()), DISCOVERY_SOCKET_ADDRESS); logger.trace("Sent request to {}:{} with content = {}", DISCOVERY_MULTICAST_ADDRESS, DISCOVERY_PORT, DISCOVER_REQUEST); } catch (IOException e) { From 14280b430fb34b0c7a404826acdc4fb20b2db51b Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Wed, 11 Dec 2024 20:46:51 +0000 Subject: [PATCH 24/35] extensive refactoring Signed-off-by: Andrew Fiddian-Green --- .../govee/internal/CommunicationManager.java | 351 +++++++++--------- .../govee/internal/GoveeDiscoveryService.java | 50 +-- .../govee/internal/GoveeHandlerFactory.java | 4 +- 3 files changed, 203 insertions(+), 202 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index 3c86420c2db96..1c75b06609c40 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -19,15 +19,18 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.NetworkInterface; +import java.net.SocketException; import java.net.StandardProtocolFamily; import java.net.StandardSocketOptions; import java.net.UnknownHostException; import java.nio.ByteBuffer; import java.nio.channels.DatagramChannel; +import java.time.Duration; import java.time.Instant; import java.util.Collections; -import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -35,6 +38,7 @@ import org.openhab.binding.govee.internal.model.GenericGoveeRequest; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Deactivate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,39 +53,52 @@ * * @author Stefan Höhn - Initial contribution * @author Danny Baumann - Thread-Safe design refactoring + * @author Andrew Fiddian-Green - Extensive refactoring */ @NonNullByDefault @Component(service = CommunicationManager.class) -public class CommunicationManager { +public class CommunicationManager implements Runnable { private final Logger logger = LoggerFactory.getLogger(CommunicationManager.class); private final Gson gson = new Gson(); - // Holds a list of all thing handlers to send them thing updates via the receiver-Thread - private final Map thingHandlers = new HashMap<>(); - @Nullable - private StatusReceiver receiverThread; + + // list of Thing handler listeners that will receive state notifications + private final Map thingHandlerListeners = new ConcurrentHashMap<>(); + + private @Nullable GoveeDiscoveryListener discoveryListener; + private @Nullable Thread serverThread; + private @Nullable DatagramSocket serverSocket; + private final Object serverLock = new Object(); private static final String DISCOVERY_MULTICAST_ADDRESS = "239.255.255.250"; private static final int DISCOVERY_PORT = 4001; private static final int RESPONSE_PORT = 4002; private static final int REQUEST_PORT = 4003; - private static final int INTERFACE_TIMEOUT_SEC = 5; + public static final int SCAN_TIMEOUT_SEC = 5; private static final String DISCOVER_REQUEST = "{\"msg\": {\"cmd\": \"scan\", \"data\": {\"account_topic\": \"reserve\"}}}"; private static final InetSocketAddress DISCOVERY_SOCKET_ADDRESS = new InetSocketAddress(DISCOVERY_MULTICAST_ADDRESS, DISCOVERY_PORT); - public interface DiscoveryResultReceiver { - void onResultReceived(DiscoveryResponse result); + public interface GoveeDiscoveryListener { + void onDiscoveryResponse(DiscoveryResponse discoveryResponse); } @Activate public CommunicationManager() { + super(); + } + + @Deactivate + public void deactivate() { + thingHandlerListeners.clear(); + discoveryListener = null; + listenerCountDecreased(); } /** - * Get the resolved IP address from the given host name + * Get the resolved IP address from the given host name. */ private static String ipAddressFrom(String host) { try { @@ -91,202 +108,186 @@ private static String ipAddressFrom(String host) { return host; } - public void registerHandler(GoveeHandler handler) { - synchronized (thingHandlers) { - thingHandlers.put(ipAddressFrom(handler.getHostname()), handler); - if (receiverThread == null) { - receiverThread = new StatusReceiver(); - receiverThread.start(); + /** + * Call this after one or more listeners have been added. + * Starts the server thread if it is not already running. + */ + private void listenerCountIncreased() { + synchronized (serverLock) { + Thread thread = serverThread; + if ((thread == null) || thread.isInterrupted() || !thread.isAlive()) { + thread = new Thread(this, "OH-binding-" + GoveeBindingConstants.BINDING_ID + "-StatusReceiver"); + serverThread = thread; + thread.start(); } } } - public void unregisterHandler(GoveeHandler handler) { - synchronized (thingHandlers) { - thingHandlers.remove(ipAddressFrom(handler.getHostname())); - if (thingHandlers.isEmpty()) { - StatusReceiver receiver = receiverThread; - if (receiver != null) { - receiver.stopReceiving(); + /** + * Call this after one or more listeners have been removed. + * Stops the server thread when listener count reaches zero. + */ + private void listenerCountDecreased() { + synchronized (serverLock) { + if (thingHandlerListeners.isEmpty() && (discoveryListener == null)) { + Thread thread = serverThread; + DatagramSocket socket = serverSocket; + if (thread != null) { + thread.interrupt(); // set interrupt flag before closing socket } - receiverThread = null; + if (socket != null) { + socket.close(); + } + serverThread = null; + serverSocket = null; } } } - public void sendRequest(GoveeHandler handler, GenericGoveeRequest request) throws IOException { - final String hostname = handler.getHostname(); - final DatagramSocket socket = new DatagramSocket(); - socket.setReuseAddress(true); - final String message = gson.toJson(request); - final byte[] data = message.getBytes(); - final InetAddress address = InetAddress.getByName(hostname); - DatagramPacket packet = new DatagramPacket(data, data.length, address, REQUEST_PORT); - logger.trace("Sending request to {} on {} with content = {}", handler.getThing().getUID(), - address.getHostAddress(), message); - socket.send(packet); - socket.close(); + /** + * Thing handlers register themselves to receive state updates when they are initalized. + */ + public void registerHandler(GoveeHandler handler) { + thingHandlerListeners.put(ipAddressFrom(handler.getHostname()), handler); + listenerCountIncreased(); } - public void runDiscoveryForInterface(NetworkInterface intf, DiscoveryResultReceiver receiver) throws IOException { - synchronized (receiver) { - StatusReceiver localReceiver = null; - StatusReceiver activeReceiver = null; - - try { - if (receiverThread == null) { - localReceiver = new StatusReceiver(); - localReceiver.start(); - activeReceiver = localReceiver; - } else { - activeReceiver = receiverThread; - } + /** + * Thing handlers unregister themselves when they are destroyed. + */ + public void unregisterHandler(GoveeHandler handler) { + thingHandlerListeners.remove(ipAddressFrom(handler.getHostname())); + listenerCountDecreased(); + } - if (activeReceiver != null) { - activeReceiver.setDiscoveryResultsReceiver(receiver); - } + /** + * Send a unicast command request to the device. + */ + public void sendRequest(GoveeHandler handler, GenericGoveeRequest request) throws IOException { + try (DatagramSocket socket = new DatagramSocket()) { + socket.setReuseAddress(true); + String message = gson.toJson(request); + byte[] data = message.getBytes(); + String hostname = handler.getHostname(); + InetAddress address = InetAddress.getByName(hostname); + DatagramPacket packet = new DatagramPacket(data, data.length, address, REQUEST_PORT); + socket.send(packet); + logger.trace("Sent request to {} on {} with content = {}", handler.getThing().getUID(), + address.getHostAddress(), message); + } + } - final Instant discoveryEndTime = Instant.now().plusSeconds(INTERFACE_TIMEOUT_SEC); + /** + * Send discovery multicast pings on any ipv4 address bound to any network interface in the given list and + * then sleep for sufficient time until responses may have been received. + */ + public void runDiscoveryForInterfaces(List interfaces, GoveeDiscoveryListener listener) { + try { + discoveryListener = listener; + listenerCountIncreased(); + Instant sleepUntil = Instant.now().plusSeconds(SCAN_TIMEOUT_SEC); - Collections.list(intf.getInetAddresses()).stream().filter(address -> address instanceof Inet4Address) - .map(address -> address.getHostAddress()).forEach(ipv4Address -> { - try (DatagramChannel channel = (DatagramChannel) DatagramChannel - .open(StandardProtocolFamily.INET) - .setOption(StandardSocketOptions.SO_REUSEADDR, true) - .setOption(StandardSocketOptions.IP_MULTICAST_TTL, 64) - .setOption(StandardSocketOptions.IP_MULTICAST_IF, intf) - .bind(new InetSocketAddress(ipv4Address, DISCOVERY_PORT)) - .configureBlocking(false)) { - logger.trace("Datagram channel bound to {}:{} on {}", ipv4Address, DISCOVERY_PORT, - intf.getDisplayName()); - channel.send(ByteBuffer.wrap(DISCOVER_REQUEST.getBytes()), DISCOVERY_SOCKET_ADDRESS); - logger.trace("Sent request to {}:{} with content = {}", DISCOVERY_MULTICAST_ADDRESS, - DISCOVERY_PORT, DISCOVER_REQUEST); - } catch (IOException e) { - logger.debug("Network error", e); - } - }); + interfaces.parallelStream() // send on all interfaces in parallel + .forEach(interFace -> Collections.list(interFace.getInetAddresses()).stream() + .filter(address -> address instanceof Inet4Address).map(address -> address.getHostAddress()) + .forEach(ipv4Address -> sendPing(interFace, ipv4Address))); - do { - try { - receiver.wait(INTERFACE_TIMEOUT_SEC * 1000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } while (Instant.now().isBefore(discoveryEndTime)); - } finally { - if (activeReceiver != null) { - activeReceiver.setDiscoveryResultsReceiver(null); - } - if (localReceiver != null) { - localReceiver.stopReceiving(); + Duration sleepDuration = Duration.between(Instant.now(), sleepUntil); + if (!sleepDuration.isNegative()) { + try { + Thread.sleep(sleepDuration.toMillis()); + } catch (InterruptedException e) { + // just return } } + } finally { + discoveryListener = null; + listenerCountDecreased(); } } - private class StatusReceiver extends Thread { - private final Logger logger = LoggerFactory.getLogger(CommunicationManager.class); - private boolean stopped = false; - private @Nullable DiscoveryResultReceiver discoveryResultReceiver; - - private @Nullable DatagramSocket socket; - - StatusReceiver() { - super("OH-binding-" + GoveeBindingConstants.BINDING_ID + "-StatusReceiver"); - } - - synchronized void setDiscoveryResultsReceiver(@Nullable DiscoveryResultReceiver receiver) { - discoveryResultReceiver = receiver; + /** + * Send discovery ping multicast on the given network interface and ipv4 address. + */ + private void sendPing(NetworkInterface interFace, String ipv4Address) { + try (DatagramChannel channel = (DatagramChannel) DatagramChannel.open(StandardProtocolFamily.INET) + .setOption(StandardSocketOptions.SO_REUSEADDR, true) + .setOption(StandardSocketOptions.IP_MULTICAST_TTL, 64) + .setOption(StandardSocketOptions.IP_MULTICAST_IF, interFace) + .bind(new InetSocketAddress(ipv4Address, DISCOVERY_PORT)).configureBlocking(false)) { + logger.trace("Sending ping from {}:{} ({}) to {}:{} with content = {}", ipv4Address, DISCOVERY_PORT, + interFace.getDisplayName(), DISCOVERY_MULTICAST_ADDRESS, DISCOVERY_PORT, DISCOVER_REQUEST); + channel.send(ByteBuffer.wrap(DISCOVER_REQUEST.getBytes()), DISCOVERY_SOCKET_ADDRESS); + } catch (IOException e) { + logger.debug("Network error", e); } + } - void stopReceiving() { - stopped = true; - interrupt(); - if (socket != null) { - socket.close(); - } - - try { - join(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } + /** + * This is a {@link Runnable} 'run()' method which gets executed on the server thread. + */ + @Override + public synchronized void run() { + while (!Thread.currentThread().isInterrupted()) { + try (DatagramSocket socket = new DatagramSocket(RESPONSE_PORT)) { + serverSocket = socket; + socket.setReuseAddress(true); + byte[] buffer = new byte[1024]; - @Override - public void run() { - while (!stopped) { - try { - DatagramSocket loopSocket = new DatagramSocket(RESPONSE_PORT); - this.socket = loopSocket; - byte[] buffer = new byte[10240]; - loopSocket.setReuseAddress(true); - while (!stopped) { - DatagramPacket packet = new DatagramPacket(buffer, buffer.length); - if (!loopSocket.isClosed()) { - loopSocket.receive(packet); + while (!Thread.currentThread().isInterrupted()) { + DatagramPacket packet = new DatagramPacket(buffer, buffer.length); + try { + socket.receive(packet); + } catch (IOException e) { + if (Thread.currentThread().isInterrupted()) { + return; // terminate thread } else { - logger.warn("Socket was unexpectedly closed"); - break; - } - if (stopped) { - break; + logger.debug("Unexpected receive exception {}", e.getMessage()); + break; // recycle socket and retry } + } - String response = new String(packet.getData(), packet.getOffset(), packet.getLength()); - String deviceIPAddress = packet.getAddress().toString().replace("/", ""); - logger.trace("Received response from {} with content = {}", deviceIPAddress, response); + String notification = new String(packet.getData(), packet.getOffset(), packet.getLength()); + String ipAddress = packet.getAddress().toString().replace("/", ""); + logger.trace("Received notification from {} with content = {}", ipAddress, notification); - final DiscoveryResultReceiver discoveryReceiver; - synchronized (this) { - discoveryReceiver = discoveryResultReceiver; - } - if (discoveryReceiver != null) { - // We're in discovery mode: try to parse result as discovery message and signal the receiver - // if parsing was successful - try { - DiscoveryResponse result = gson.fromJson(response, DiscoveryResponse.class); - if (result != null) { - synchronized (discoveryReceiver) { - discoveryReceiver.onResultReceived(result); - discoveryReceiver.notifyAll(); - } - } - } catch (JsonParseException e) { - logger.debug( - "JsonParseException when trying to parse the response, probably a status message", - e); - } - } else { - final @Nullable GoveeHandler handler; - synchronized (thingHandlers) { - handler = thingHandlers.get(deviceIPAddress); - } - if (handler == null) { - logger.warn("Handler not found for {}", deviceIPAddress); - } else { - logger.debug("Processing response for {} on {}", handler.getThing().getUID(), - deviceIPAddress); - handler.handleIncomingStatus(response); + /* + * check if there is a discovery listener and if so try to parse the notification as a discovery + * notification and if it parsed successfully then notify the listener + */ + GoveeDiscoveryListener discoveryListener = this.discoveryListener; + if (discoveryListener != null) { + try { + DiscoveryResponse response = gson.fromJson(notification, DiscoveryResponse.class); + if ((response != null) && !thingHandlerListeners.containsKey(ipAddress)) { + logger.debug("Notifying potential new Thing discovered on {}", ipAddress); + discoveryListener.onDiscoveryResponse(response); } + continue; // prepare to receive next notification + } catch (JsonParseException e) { + logger.debug("Cannot parse as discovery notification; consider as state notification"); + // fall through: consider as state notification } } - } catch (IOException e) { - logger.debug("Exception when receiving status packet {}", e.getMessage()); - // as we haven't received a packet we also don't know where it should have come from - // hence, we don't know which thing put offline. - // a way to monitor this would be to keep track in a list, which device answers we expect - // and supervise an expected answer within a given time but that will make the whole - // mechanism much more complicated and may be added in the future - // PS it also seems to be 'normal' to encounter errors when in device discovery mode - } finally { - if (socket != null) { - socket.close(); - socket = null; + + /* + * check if there is a thing handler listener and if so try to process the notification as a + * state notification by notifying the listener + */ + GoveeHandler handler = thingHandlerListeners.get(ipAddress); + if (handler != null) { + logger.debug("Sending state notification to {} on {}", handler.getThing().getUID(), ipAddress); + handler.handleIncomingStatus(notification); + } else { + logger.warn("Missing Thing handler listener for {}", ipAddress); } - } + } // {while} + } catch (SocketException e) { + logger.debug("Unexpected socket exception {}", e.getMessage()); + } finally { + serverSocket = null; + serverThread = null; } - } + } // {while} } } diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeDiscoveryService.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeDiscoveryService.java index ef5a62b69fd6b..4e58bfcd4021e 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeDiscoveryService.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeDiscoveryService.java @@ -12,16 +12,17 @@ */ package org.openhab.binding.govee.internal; -import java.io.IOException; import java.net.NetworkInterface; import java.net.SocketException; import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Set; +import java.util.concurrent.TimeUnit; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.binding.govee.internal.CommunicationManager.GoveeDiscoveryListener; import org.openhab.binding.govee.internal.model.DiscoveryData; import org.openhab.binding.govee.internal.model.DiscoveryResponse; import org.openhab.core.config.discovery.AbstractDiscoveryService; @@ -79,17 +80,18 @@ */ @NonNullByDefault @Component(service = DiscoveryService.class, immediate = true, configurationPid = "discovery.govee") -public class GoveeDiscoveryService extends AbstractDiscoveryService { +public class GoveeDiscoveryService extends AbstractDiscoveryService implements GoveeDiscoveryListener { private final Logger logger = LoggerFactory.getLogger(GoveeDiscoveryService.class); - private CommunicationManager communicationManager; + private final CommunicationManager communicationManager; private static final Set SUPPORTED_THING_TYPES_UIDS = Set.of(GoveeBindingConstants.THING_TYPE_LIGHT); @Activate - public GoveeDiscoveryService(@Reference TranslationProvider i18nProvider, @Reference LocaleProvider localeProvider, - @Reference CommunicationManager communicationManager) { - super(SUPPORTED_THING_TYPES_UIDS, 0, false); + public GoveeDiscoveryService(final @Reference TranslationProvider i18nProvider, + final @Reference LocaleProvider localeProvider, + final @Reference CommunicationManager communicationManager) { + super(SUPPORTED_THING_TYPES_UIDS, CommunicationManager.SCAN_TIMEOUT_SEC, false); this.i18nProvider = i18nProvider; this.localeProvider = localeProvider; this.communicationManager = communicationManager; @@ -103,23 +105,9 @@ public GoveeDiscoveryService(CommunicationManager communicationManager) { @Override protected void startScan() { - logger.debug("starting Scan"); - - getLocalNetworkInterfaces().forEach(localNetworkInterface -> { - logger.debug("Discovering Govee devices on {} ...", localNetworkInterface); - try { - communicationManager.runDiscoveryForInterface(localNetworkInterface, response -> { - DiscoveryResult result = responseToResult(response); - if (result != null) { - thingDiscovered(result); - } - }); - logger.trace("After runDiscoveryForInterface"); - } catch (IOException e) { - logger.debug("Discovery with IO exception: {}", e.getMessage()); - } - logger.trace("After try"); - }); + logger.debug("Starting scan"); + scheduler.schedule(() -> communicationManager.runDiscoveryForInterfaces(getLocalNetworkInterfaces(), this), 0, + TimeUnit.MILLISECONDS); } public @Nullable DiscoveryResult responseToResult(DiscoveryResponse response) { @@ -165,11 +153,11 @@ protected void startScan() { } String hwVersion = data.wifiVersionHard(); - if (hwVersion != null) { + if (!hwVersion.isEmpty()) { builder.withProperty(GoveeBindingConstants.HW_VERSION, hwVersion); } String swVersion = data.wifiVersionSoft(); - if (swVersion != null) { + if (!swVersion.isEmpty()) { builder.withProperty(GoveeBindingConstants.SW_VERSION, swVersion); } @@ -194,4 +182,16 @@ private List getLocalNetworkInterfaces() { } return result; } + + /** + * This method is called back by the {@link CommunicationManager} when it receives a {@link DiscoveryResponse} + * notification carrying information about potential newly discovered Things. + */ + @Override + public synchronized void onDiscoveryResponse(DiscoveryResponse discoveryResponse) { + DiscoveryResult discoveryResult = responseToResult(discoveryResponse); + if (discoveryResult != null) { + thingDiscovered(discoveryResult); + } + } } diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandlerFactory.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandlerFactory.java index 63a539ff8011a..a9b5ec5e9babc 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandlerFactory.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandlerFactory.java @@ -42,8 +42,8 @@ public class GoveeHandlerFactory extends BaseThingHandlerFactory { private final GoveeStateDescriptionProvider stateDescriptionProvider; @Activate - public GoveeHandlerFactory(@Reference CommunicationManager communicationManager, - @Reference GoveeStateDescriptionProvider stateDescriptionProvider) { + public GoveeHandlerFactory(final @Reference CommunicationManager communicationManager, + final @Reference GoveeStateDescriptionProvider stateDescriptionProvider) { this.communicationManager = communicationManager; this.stateDescriptionProvider = stateDescriptionProvider; } From 7f8771bbea5f81d73c4d650f8ce6a095ebfb1d8b Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Wed, 11 Dec 2024 20:58:47 +0000 Subject: [PATCH 25/35] resequence methods for easier review Signed-off-by: Andrew Fiddian-Green --- .../govee/internal/CommunicationManager.java | 129 +++++++++--------- 1 file changed, 64 insertions(+), 65 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index 1c75b06609c40..839de2a249d8a 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -87,7 +87,6 @@ public interface GoveeDiscoveryListener { @Activate public CommunicationManager() { - super(); } @Deactivate @@ -97,53 +96,6 @@ public void deactivate() { listenerCountDecreased(); } - /** - * Get the resolved IP address from the given host name. - */ - private static String ipAddressFrom(String host) { - try { - return InetAddress.getByName(host).getHostAddress(); - } catch (UnknownHostException e) { - } - return host; - } - - /** - * Call this after one or more listeners have been added. - * Starts the server thread if it is not already running. - */ - private void listenerCountIncreased() { - synchronized (serverLock) { - Thread thread = serverThread; - if ((thread == null) || thread.isInterrupted() || !thread.isAlive()) { - thread = new Thread(this, "OH-binding-" + GoveeBindingConstants.BINDING_ID + "-StatusReceiver"); - serverThread = thread; - thread.start(); - } - } - } - - /** - * Call this after one or more listeners have been removed. - * Stops the server thread when listener count reaches zero. - */ - private void listenerCountDecreased() { - synchronized (serverLock) { - if (thingHandlerListeners.isEmpty() && (discoveryListener == null)) { - Thread thread = serverThread; - DatagramSocket socket = serverSocket; - if (thread != null) { - thread.interrupt(); // set interrupt flag before closing socket - } - if (socket != null) { - socket.close(); - } - serverThread = null; - serverSocket = null; - } - } - } - /** * Thing handlers register themselves to receive state updates when they are initalized. */ @@ -206,23 +158,6 @@ public void runDiscoveryForInterfaces(List interfaces, GoveeDi } } - /** - * Send discovery ping multicast on the given network interface and ipv4 address. - */ - private void sendPing(NetworkInterface interFace, String ipv4Address) { - try (DatagramChannel channel = (DatagramChannel) DatagramChannel.open(StandardProtocolFamily.INET) - .setOption(StandardSocketOptions.SO_REUSEADDR, true) - .setOption(StandardSocketOptions.IP_MULTICAST_TTL, 64) - .setOption(StandardSocketOptions.IP_MULTICAST_IF, interFace) - .bind(new InetSocketAddress(ipv4Address, DISCOVERY_PORT)).configureBlocking(false)) { - logger.trace("Sending ping from {}:{} ({}) to {}:{} with content = {}", ipv4Address, DISCOVERY_PORT, - interFace.getDisplayName(), DISCOVERY_MULTICAST_ADDRESS, DISCOVERY_PORT, DISCOVER_REQUEST); - channel.send(ByteBuffer.wrap(DISCOVER_REQUEST.getBytes()), DISCOVERY_SOCKET_ADDRESS); - } catch (IOException e) { - logger.debug("Network error", e); - } - } - /** * This is a {@link Runnable} 'run()' method which gets executed on the server thread. */ @@ -290,4 +225,68 @@ public synchronized void run() { } } // {while} } + + /** + * Get the resolved IP address from the given host name. + */ + private static String ipAddressFrom(String host) { + try { + return InetAddress.getByName(host).getHostAddress(); + } catch (UnknownHostException e) { + } + return host; + } + + /** + * Call this after one or more listeners have been added. + * Starts the server thread if it is not already running. + */ + private void listenerCountIncreased() { + synchronized (serverLock) { + Thread thread = serverThread; + if ((thread == null) || thread.isInterrupted() || !thread.isAlive()) { + thread = new Thread(this, "OH-binding-" + GoveeBindingConstants.BINDING_ID + "-StatusReceiver"); + serverThread = thread; + thread.start(); + } + } + } + + /** + * Call this after one or more listeners have been removed. + * Stops the server thread when listener count reaches zero. + */ + private void listenerCountDecreased() { + synchronized (serverLock) { + if (thingHandlerListeners.isEmpty() && (discoveryListener == null)) { + Thread thread = serverThread; + DatagramSocket socket = serverSocket; + if (thread != null) { + thread.interrupt(); // set interrupt flag before closing socket + } + if (socket != null) { + socket.close(); + } + serverThread = null; + serverSocket = null; + } + } + } + + /** + * Send discovery ping multicast on the given network interface and ipv4 address. + */ + private void sendPing(NetworkInterface interFace, String ipv4Address) { + try (DatagramChannel channel = (DatagramChannel) DatagramChannel.open(StandardProtocolFamily.INET) + .setOption(StandardSocketOptions.SO_REUSEADDR, true) + .setOption(StandardSocketOptions.IP_MULTICAST_TTL, 64) + .setOption(StandardSocketOptions.IP_MULTICAST_IF, interFace) + .bind(new InetSocketAddress(ipv4Address, DISCOVERY_PORT)).configureBlocking(false)) { + logger.trace("Sending ping from {}:{} ({}) to {}:{} with content = {}", ipv4Address, DISCOVERY_PORT, + interFace.getDisplayName(), DISCOVERY_MULTICAST_ADDRESS, DISCOVERY_PORT, DISCOVER_REQUEST); + channel.send(ByteBuffer.wrap(DISCOVER_REQUEST.getBytes()), DISCOVERY_SOCKET_ADDRESS); + } catch (IOException e) { + logger.debug("Network error", e); + } + } } From 14e63d87f58d7c1252650a59e4cfdc71473a1b50 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Thu, 12 Dec 2024 14:16:16 +0000 Subject: [PATCH 26/35] improve discovery Signed-off-by: Andrew Fiddian-Green --- .../govee/internal/GoveeDiscoveryService.java | 35 +++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeDiscoveryService.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeDiscoveryService.java index 4e58bfcd4021e..3d22c007f031e 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeDiscoveryService.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeDiscoveryService.java @@ -18,6 +18,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Set; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -81,9 +82,13 @@ @NonNullByDefault @Component(service = DiscoveryService.class, immediate = true, configurationPid = "discovery.govee") public class GoveeDiscoveryService extends AbstractDiscoveryService implements GoveeDiscoveryListener { + + private static final int BACKGROUND_SCAN_INTERVAL_SECONDS = 300; + private final Logger logger = LoggerFactory.getLogger(GoveeDiscoveryService.class); private final CommunicationManager communicationManager; + private @Nullable ScheduledFuture backgroundScanTask; private static final Set SUPPORTED_THING_TYPES_UIDS = Set.of(GoveeBindingConstants.THING_TYPE_LIGHT); @@ -91,7 +96,7 @@ public class GoveeDiscoveryService extends AbstractDiscoveryService implements G public GoveeDiscoveryService(final @Reference TranslationProvider i18nProvider, final @Reference LocaleProvider localeProvider, final @Reference CommunicationManager communicationManager) { - super(SUPPORTED_THING_TYPES_UIDS, CommunicationManager.SCAN_TIMEOUT_SEC, false); + super(SUPPORTED_THING_TYPES_UIDS, CommunicationManager.SCAN_TIMEOUT_SEC, true); this.i18nProvider = i18nProvider; this.localeProvider = localeProvider; this.communicationManager = communicationManager; @@ -106,8 +111,7 @@ public GoveeDiscoveryService(CommunicationManager communicationManager) { @Override protected void startScan() { logger.debug("Starting scan"); - scheduler.schedule(() -> communicationManager.runDiscoveryForInterfaces(getLocalNetworkInterfaces(), this), 0, - TimeUnit.MILLISECONDS); + scheduler.schedule(this::doDiscovery, 0, TimeUnit.MILLISECONDS); } public @Nullable DiscoveryResult responseToResult(DiscoveryResponse response) { @@ -183,6 +187,13 @@ private List getLocalNetworkInterfaces() { return result; } + /** + * Command the {@link CommunicationManager) to run the scans. + */ + private void doDiscovery() { + communicationManager.runDiscoveryForInterfaces(getLocalNetworkInterfaces(), this); + } + /** * This method is called back by the {@link CommunicationManager} when it receives a {@link DiscoveryResponse} * notification carrying information about potential newly discovered Things. @@ -194,4 +205,22 @@ public synchronized void onDiscoveryResponse(DiscoveryResponse discoveryResponse thingDiscovered(discoveryResult); } } + + @Override + protected void startBackgroundDiscovery() { + ScheduledFuture backgroundScanTask = this.backgroundScanTask; + if (backgroundScanTask == null || backgroundScanTask.isCancelled()) { + this.backgroundScanTask = scheduler.scheduleWithFixedDelay(this::doDiscovery, 0, + BACKGROUND_SCAN_INTERVAL_SECONDS, TimeUnit.SECONDS); + } + } + + @Override + protected void stopBackgroundDiscovery() { + ScheduledFuture backgroundScanTask = this.backgroundScanTask; + if (backgroundScanTask != null) { + backgroundScanTask.cancel(true); + this.backgroundScanTask = null; + } + } } From d04139ee2dd534f301c2ff8d300bdba951889c4b Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Thu, 12 Dec 2024 14:16:57 +0000 Subject: [PATCH 27/35] functional tests done Signed-off-by: Andrew Fiddian-Green --- .../govee/internal/CommunicationManager.java | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index 839de2a249d8a..5e9c0d939d219 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -164,6 +164,7 @@ public void runDiscoveryForInterfaces(List interfaces, GoveeDi @Override public synchronized void run() { while (!Thread.currentThread().isInterrupted()) { + logger.trace("Server thread started"); try (DatagramSocket socket = new DatagramSocket(RESPONSE_PORT)) { serverSocket = socket; socket.setReuseAddress(true); @@ -186,42 +187,50 @@ public synchronized void run() { String ipAddress = packet.getAddress().toString().replace("/", ""); logger.trace("Received notification from {} with content = {}", ipAddress, notification); + GoveeHandler handler = thingHandlerListeners.get(ipAddress); + boolean isStatusNotification = notification.contains("devStatus"); + GoveeDiscoveryListener discoveryListener = this.discoveryListener; + /* - * check if there is a discovery listener and if so try to parse the notification as a discovery - * notification and if it parsed successfully then notify the listener + * a Thing handler exists and we have a status notification, so notify the Thing handler + * listener */ - GoveeDiscoveryListener discoveryListener = this.discoveryListener; - if (discoveryListener != null) { + if (handler != null && isStatusNotification) { + logger.debug("Sending state notification to {} on {}", handler.getThing().getUID(), ipAddress); + handler.handleIncomingStatus(notification); + continue; + } + + /* + * a Thing handler does not exist, we do not have a status notification, but there is a + * discoveryListener, so notify the discovery listener + */ + if (handler == null && !isStatusNotification && discoveryListener != null) { try { DiscoveryResponse response = gson.fromJson(notification, DiscoveryResponse.class); - if ((response != null) && !thingHandlerListeners.containsKey(ipAddress)) { + if (response != null) { logger.debug("Notifying potential new Thing discovered on {}", ipAddress); discoveryListener.onDiscoveryResponse(response); } - continue; // prepare to receive next notification } catch (JsonParseException e) { - logger.debug("Cannot parse as discovery notification; consider as state notification"); - // fall through: consider as state notification + logger.debug("Failed to parse discovery notification", e); } + continue; } /* - * check if there is a thing handler listener and if so try to process the notification as a - * state notification by notifying the listener + * none of the above conditions apply so log it */ - GoveeHandler handler = thingHandlerListeners.get(ipAddress); - if (handler != null) { - logger.debug("Sending state notification to {} on {}", handler.getThing().getUID(), ipAddress); - handler.handleIncomingStatus(notification); - } else { - logger.warn("Missing Thing handler listener for {}", ipAddress); - } + logger.warn( + "Unrecognised notification for ipAddress:{}, handler:{}, stateNotification:{}, discoveryListener:{}", + ipAddress, handler, isStatusNotification, discoveryListener); } // {while} } catch (SocketException e) { logger.debug("Unexpected socket exception {}", e.getMessage()); } finally { serverSocket = null; serverThread = null; + logger.trace("Server thread finished"); } } // {while} } @@ -245,7 +254,7 @@ private void listenerCountIncreased() { synchronized (serverLock) { Thread thread = serverThread; if ((thread == null) || thread.isInterrupted() || !thread.isAlive()) { - thread = new Thread(this, "OH-binding-" + GoveeBindingConstants.BINDING_ID + "-StatusReceiver"); + thread = new Thread(this, "OH-binding-" + GoveeBindingConstants.BINDING_ID); serverThread = thread; thread.start(); } @@ -282,9 +291,9 @@ private void sendPing(NetworkInterface interFace, String ipv4Address) { .setOption(StandardSocketOptions.IP_MULTICAST_TTL, 64) .setOption(StandardSocketOptions.IP_MULTICAST_IF, interFace) .bind(new InetSocketAddress(ipv4Address, DISCOVERY_PORT)).configureBlocking(false)) { - logger.trace("Sending ping from {}:{} ({}) to {}:{} with content = {}", ipv4Address, DISCOVERY_PORT, - interFace.getDisplayName(), DISCOVERY_MULTICAST_ADDRESS, DISCOVERY_PORT, DISCOVER_REQUEST); channel.send(ByteBuffer.wrap(DISCOVER_REQUEST.getBytes()), DISCOVERY_SOCKET_ADDRESS); + logger.trace("Sent ping from {}:{} ({}) to {}:{} with content = {}", ipv4Address, DISCOVERY_PORT, + interFace.getDisplayName(), DISCOVERY_MULTICAST_ADDRESS, DISCOVERY_PORT, DISCOVER_REQUEST); } catch (IOException e) { logger.debug("Network error", e); } From a64ae1d263659fa6e9200b1617c82e20f955e757 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Thu, 12 Dec 2024 16:18:59 +0000 Subject: [PATCH 28/35] eliminate spurious log messages Signed-off-by: Andrew Fiddian-Green --- .../govee/internal/CommunicationManager.java | 35 ++++++++----------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index 5e9c0d939d219..a6482a9627373 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -191,38 +191,33 @@ public synchronized void run() { boolean isStatusNotification = notification.contains("devStatus"); GoveeDiscoveryListener discoveryListener = this.discoveryListener; - /* - * a Thing handler exists and we have a status notification, so notify the Thing handler - * listener - */ + // it is a status notification and there is a Thing handler listener, so notify that listener if (handler != null && isStatusNotification) { logger.debug("Sending state notification to {} on {}", handler.getThing().getUID(), ipAddress); handler.handleIncomingStatus(notification); continue; } - /* - * a Thing handler does not exist, we do not have a status notification, but there is a - * discoveryListener, so notify the discovery listener - */ - if (handler == null && !isStatusNotification && discoveryListener != null) { - try { - DiscoveryResponse response = gson.fromJson(notification, DiscoveryResponse.class); - if (response != null) { - logger.debug("Notifying potential new Thing discovered on {}", ipAddress); - discoveryListener.onDiscoveryResponse(response); + // it is a discovery notification and there is a discoveryListener, so notify that listener + if (!isStatusNotification && discoveryListener != null) { + // send discovery notification only when there is no Thing handler listener + if (handler == null) { + try { + DiscoveryResponse response = gson.fromJson(notification, DiscoveryResponse.class); + if (response != null) { + logger.debug("Notifying potential new Thing discovered on {}", ipAddress); + discoveryListener.onDiscoveryResponse(response); + } + } catch (JsonParseException e) { + logger.debug("Failed to parse discovery notification", e); } - } catch (JsonParseException e) { - logger.debug("Failed to parse discovery notification", e); } continue; } - /* - * none of the above conditions apply so log it - */ + // none of the above conditions apply just so log it logger.warn( - "Unrecognised notification for ipAddress:{}, handler:{}, stateNotification:{}, discoveryListener:{}", + "Unhandled notification for ipAddress:{}, handler:{}, stateNotification:{}, discoveryListener:{}", ipAddress, handler, isStatusNotification, discoveryListener); } // {while} } catch (SocketException e) { From 8a549034b6e5631cb22a6e79790cf2f63ff67b04 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Thu, 12 Dec 2024 23:25:41 +0000 Subject: [PATCH 29/35] restrict refresh interval to 2 seconds Signed-off-by: Andrew Fiddian-Green --- .../binding/govee/internal/GoveeHandler.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 3fd8d152d587c..636972699e57c 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -99,11 +99,12 @@ public class GoveeHandler extends BaseThingHandler { private OnOffType lastSwitch = OnOffType.OFF; private HSBType lastColor = new HSBType(); - private int lastKelvin; + private int lastKelvin; private int minKelvin; private int maxKelvin; + private static final int REFRESH_SECONDS_MIN = 2; private static final int INTER_COMMAND_DELAY_MILLISEC = 100; /** @@ -157,12 +158,16 @@ public void initialize() { stateDescriptionProvider.setMinMaxKelvin(new ChannelUID(thing.getUID(), CHANNEL_COLOR_TEMPERATURE_ABS), minKelvin, maxKelvin); + int refreshSeconds = goveeConfiguration.refreshInterval; + refreshSeconds = refreshSeconds > 0 ? Math.max(refreshSeconds, REFRESH_SECONDS_MIN) : 0; + updateStatus(ThingStatus.UNKNOWN); communicationManager.registerHandler(this); - if (triggerStatusJob == null) { + + if (triggerStatusJob == null && refreshSeconds > 0) { logger.debug("Starting refresh trigger job for thing {} ", thing.getLabel()); - triggerStatusJob = executorService.scheduleWithFixedDelay(thingRefreshSender, 100, - goveeConfiguration.refreshInterval * 1000L, TimeUnit.MILLISECONDS); + triggerStatusJob = executorService.scheduleWithFixedDelay(thingRefreshSender, 100, refreshSeconds, + TimeUnit.SECONDS); } } From 9a394707ae0e116bb1d93b04bb17af3fe896d1bd Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Thu, 12 Dec 2024 23:34:10 +0000 Subject: [PATCH 30/35] restrict refresh interval to 2 seconds #2 Signed-off-by: Andrew Fiddian-Green --- .../openhab/binding/govee/internal/GoveeHandler.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 636972699e57c..c20f8ffa0c7f5 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -158,15 +158,18 @@ public void initialize() { stateDescriptionProvider.setMinMaxKelvin(new ChannelUID(thing.getUID(), CHANNEL_COLOR_TEMPERATURE_ABS), minKelvin, maxKelvin); - int refreshSeconds = goveeConfiguration.refreshInterval; - refreshSeconds = refreshSeconds > 0 ? Math.max(refreshSeconds, REFRESH_SECONDS_MIN) : 0; + int refreshSecs = goveeConfiguration.refreshInterval; + if (refreshSecs < REFRESH_SECONDS_MIN) { + logger.warn("Config Param refreshInterval={} too low, minimum={}", refreshSecs, REFRESH_SECONDS_MIN); + refreshSecs = REFRESH_SECONDS_MIN; + } updateStatus(ThingStatus.UNKNOWN); communicationManager.registerHandler(this); - if (triggerStatusJob == null && refreshSeconds > 0) { + if (triggerStatusJob == null) { logger.debug("Starting refresh trigger job for thing {} ", thing.getLabel()); - triggerStatusJob = executorService.scheduleWithFixedDelay(thingRefreshSender, 100, refreshSeconds, + triggerStatusJob = executorService.scheduleWithFixedDelay(thingRefreshSender, 100, refreshSecs, TimeUnit.SECONDS); } } From be9f0baf8f54ab2bb7583443dbbb3febb28eb935 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Thu, 12 Dec 2024 23:56:45 +0000 Subject: [PATCH 31/35] fix config.xml Signed-off-by: Andrew Fiddian-Green --- .../src/main/resources/OH-INF/config/config.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/config/config.xml b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/config/config.xml index ba8d9a650d948..9ddf0f4c14d61 100644 --- a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/config/config.xml +++ b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/config/config.xml @@ -14,7 +14,7 @@ MAC Address of the device - + The amount of time that passes until the device is refreshed (in seconds) 2 From 9cff94e91c2acb526ce743b69025b811961d9d02 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Fri, 13 Dec 2024 00:14:40 +0000 Subject: [PATCH 32/35] increase inter command delay Signed-off-by: Andrew Fiddian-Green --- .../java/org/openhab/binding/govee/internal/GoveeHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index c20f8ffa0c7f5..717a79a91c61e 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -105,7 +105,7 @@ public class GoveeHandler extends BaseThingHandler { private int maxKelvin; private static final int REFRESH_SECONDS_MIN = 2; - private static final int INTER_COMMAND_DELAY_MILLISEC = 100; + private static final int INTER_COMMAND_DELAY_MILLISEC = 200; /** * This thing related job thingRefreshSender triggers an update to the Govee device. From b6595beceee248f528749365747ff2843790ad16 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Thu, 19 Dec 2024 13:33:32 +0000 Subject: [PATCH 33/35] improve server thread; add command pipelining Signed-off-by: Andrew Fiddian-Green --- bundles/org.openhab.binding.govee/README.md | 1 + .../govee/internal/CommunicationManager.java | 224 ++++++++++-------- .../binding/govee/internal/GoveeHandler.java | 210 ++++++++-------- .../resources/OH-INF/i18n/govee.properties | 1 + 4 files changed, 215 insertions(+), 221 deletions(-) diff --git a/bundles/org.openhab.binding.govee/README.md b/bundles/org.openhab.binding.govee/README.md index 13c31fe4b5e3d..bcd797a22f87d 100644 --- a/bundles/org.openhab.binding.govee/README.md +++ b/bundles/org.openhab.binding.govee/README.md @@ -128,6 +128,7 @@ Here is a list of the supported devices (the ones marked with * have been tested - H706A Permanent Outdoor Lights Pro 30M - H706B Permanent Outdoor Lights Pro 45M - H706C Permanent Outdoor Lights Pro 60M +- H7070 Outdoor Projector Light (*) - H7075 Outdoor Wall Light - H70B1 520 LED Curtain Lights - H70BC 400 LED Curtain Lights diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index a6482a9627373..ba5baef7d31e4 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -19,11 +19,12 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.NetworkInterface; -import java.net.SocketException; +import java.net.SocketAddress; import java.net.StandardProtocolFamily; import java.net.StandardSocketOptions; import java.net.UnknownHostException; import java.nio.ByteBuffer; +import java.nio.channels.ClosedByInterruptException; import java.nio.channels.DatagramChannel; import java.time.Duration; import java.time.Instant; @@ -46,18 +47,18 @@ import com.google.gson.JsonParseException; /** - * The {@link CommunicationManager} is a thread that handles the answers of all devices. - * Therefore it needs to apply the information to the right thing. - * - * Discovery uses the same response code, so we must not refresh the status during discovery. + * The {@link CommunicationManager} component implements a sender to send commands to Govee devices, + * and implements a thread that handles the notifications from all devices. It applies the status + * information to the right Thing. It supports both discovery and status commands and notifications + * concurrently. * * @author Stefan Höhn - Initial contribution * @author Danny Baumann - Thread-Safe design refactoring - * @author Andrew Fiddian-Green - Extensive refactoring + * @author Andrew Fiddian-Green - New threading model using java.nio channel */ @NonNullByDefault @Component(service = CommunicationManager.class) -public class CommunicationManager implements Runnable { +public class CommunicationManager { private final Logger logger = LoggerFactory.getLogger(CommunicationManager.class); private final Gson gson = new Gson(); @@ -66,8 +67,11 @@ public class CommunicationManager implements Runnable { private @Nullable GoveeDiscoveryListener discoveryListener; private @Nullable Thread serverThread; - private @Nullable DatagramSocket serverSocket; + private boolean serverStopFlag = false; + + private final Object paramsLock = new Object(); private final Object serverLock = new Object(); + private final Object senderLock = new Object(); private static final String DISCOVERY_MULTICAST_ADDRESS = "239.255.255.250"; private static final int DISCOVERY_PORT = 4001; @@ -87,21 +91,21 @@ public interface GoveeDiscoveryListener { @Activate public CommunicationManager() { + serverStart(); } @Deactivate public void deactivate() { thingHandlerListeners.clear(); discoveryListener = null; - listenerCountDecreased(); + serverStop(); } /** - * Thing handlers register themselves to receive state updates when they are initalized. + * Thing handlers register themselves to receive state updates when they are initialized. */ public void registerHandler(GoveeHandler handler) { thingHandlerListeners.put(ipAddressFrom(handler.getHostname()), handler); - listenerCountIncreased(); } /** @@ -109,34 +113,36 @@ public void registerHandler(GoveeHandler handler) { */ public void unregisterHandler(GoveeHandler handler) { thingHandlerListeners.remove(ipAddressFrom(handler.getHostname())); - listenerCountDecreased(); } /** * Send a unicast command request to the device. */ public void sendRequest(GoveeHandler handler, GenericGoveeRequest request) throws IOException { - try (DatagramSocket socket = new DatagramSocket()) { - socket.setReuseAddress(true); - String message = gson.toJson(request); - byte[] data = message.getBytes(); - String hostname = handler.getHostname(); - InetAddress address = InetAddress.getByName(hostname); - DatagramPacket packet = new DatagramPacket(data, data.length, address, REQUEST_PORT); - socket.send(packet); - logger.trace("Sent request to {} on {} with content = {}", handler.getThing().getUID(), - address.getHostAddress(), message); + serverStart(); + synchronized (senderLock) { + try (DatagramSocket socket = new DatagramSocket()) { + socket.setReuseAddress(true); + String message = gson.toJson(request); + byte[] data = message.getBytes(); + String hostname = handler.getHostname(); + InetAddress address = InetAddress.getByName(hostname); + DatagramPacket packet = new DatagramPacket(data, data.length, address, REQUEST_PORT); + socket.send(packet); + logger.trace("Sent request to {} on {} with content = {}", handler.getThing().getUID(), + address.getHostAddress(), message); + } } } /** - * Send discovery multicast pings on any ipv4 address bound to any network interface in the given list and - * then sleep for sufficient time until responses may have been received. + * Send discovery multicast pings on any ipv4 address bound to any network interface in the given + * list and then sleep for sufficient time until responses may have been received. */ public void runDiscoveryForInterfaces(List interfaces, GoveeDiscoveryListener listener) { + serverStart(); try { discoveryListener = listener; - listenerCountIncreased(); Instant sleepUntil = Instant.now().plusSeconds(SCAN_TIMEOUT_SEC); interfaces.parallelStream() // send on all interfaces in parallel @@ -154,80 +160,96 @@ public void runDiscoveryForInterfaces(List interfaces, GoveeDi } } finally { discoveryListener = null; - listenerCountDecreased(); } } /** - * This is a {@link Runnable} 'run()' method which gets executed on the server thread. + * This method gets executed on the server thread. It uses a {@link DatagramChannel} to listen on port + * 4002 and it processes any notifications received. The task runs continuously in a loop until the + * thread is externally interrupted. + * + *
  • In case of status notifications it forwards the message to the Thing handler listener.
    • + *
  • In case of discovery notifications it forwards the message to the discovery listener.
    • + *
  • If there is neither a Thing handler listener, nor a discovery listener, it logs an error.
    • */ - @Override - public synchronized void run() { - while (!Thread.currentThread().isInterrupted()) { - logger.trace("Server thread started"); - try (DatagramSocket socket = new DatagramSocket(RESPONSE_PORT)) { - serverSocket = socket; - socket.setReuseAddress(true); - byte[] buffer = new byte[1024]; + private void serverThreadTask() { + synchronized (serverLock) { + try { + logger.trace("Server thread started."); + ByteBuffer buffer = ByteBuffer.allocate(1024); - while (!Thread.currentThread().isInterrupted()) { - DatagramPacket packet = new DatagramPacket(buffer, buffer.length); - try { - socket.receive(packet); - } catch (IOException e) { - if (Thread.currentThread().isInterrupted()) { - return; // terminate thread - } else { - logger.debug("Unexpected receive exception {}", e.getMessage()); - break; // recycle socket and retry - } - } + while (!serverStopFlag) { + try (DatagramChannel channel = DatagramChannel.open() + .setOption(StandardSocketOptions.SO_REUSEADDR, true) + .bind(new InetSocketAddress(RESPONSE_PORT))) { - String notification = new String(packet.getData(), packet.getOffset(), packet.getLength()); - String ipAddress = packet.getAddress().toString().replace("/", ""); - logger.trace("Received notification from {} with content = {}", ipAddress, notification); + while (!serverStopFlag) { + String sourceIp = ""; + try { + SocketAddress socketAddress = channel.receive(buffer.clear()); + if ((socketAddress instanceof InetSocketAddress inetSocketAddress) + && (inetSocketAddress.getAddress() instanceof InetAddress inetAddress)) { + sourceIp = inetAddress.getHostAddress(); + } else { + logger.debug("Receive() - bad socketAddress={}", socketAddress); + return; + } + } catch (ClosedByInterruptException e) { + // thrown if 'Thread.interrupt()' is called during 'channel.receive()' + logger.trace( + "Receive exception=ClosedByInterruptException, isInterrupted={}, serverStopFlag={}", + Thread.currentThread().isInterrupted(), serverStopFlag); + Thread.interrupted(); // clear 'interrupted' flag + if (serverStopFlag) { + return; + } else { + break; + } + } catch (IOException e) { + logger.debug("Receive unexpected exception={}", e.getMessage()); + break; + } - GoveeHandler handler = thingHandlerListeners.get(ipAddress); - boolean isStatusNotification = notification.contains("devStatus"); - GoveeDiscoveryListener discoveryListener = this.discoveryListener; + String message = new String(buffer.array(), 0, buffer.position()); + logger.trace("Receive from sourceIp={}, message={}", sourceIp, message); - // it is a status notification and there is a Thing handler listener, so notify that listener - if (handler != null && isStatusNotification) { - logger.debug("Sending state notification to {} on {}", handler.getThing().getUID(), ipAddress); - handler.handleIncomingStatus(notification); - continue; - } + GoveeHandler handler = thingHandlerListeners.get(sourceIp); + boolean devStatus = message.contains("devStatus"); + if (handler != null && devStatus) { + logger.debug("Notifying status of thing={} on sourcecIp={}", + handler.getThing().getUID(), sourceIp); + handler.handleIncomingStatus(message); + continue; + } - // it is a discovery notification and there is a discoveryListener, so notify that listener - if (!isStatusNotification && discoveryListener != null) { - // send discovery notification only when there is no Thing handler listener - if (handler == null) { - try { - DiscoveryResponse response = gson.fromJson(notification, DiscoveryResponse.class); - if (response != null) { - logger.debug("Notifying potential new Thing discovered on {}", ipAddress); - discoveryListener.onDiscoveryResponse(response); + GoveeDiscoveryListener discoveryListener = this.discoveryListener; + if (!devStatus && discoveryListener != null) { + try { + DiscoveryResponse response = gson.fromJson(message, DiscoveryResponse.class); + if (response != null) { + logger.debug("Notifying discovery of device on sourceIp={}", sourceIp); + discoveryListener.onDiscoveryResponse(response); + } + } catch (JsonParseException e) { + logger.debug("Discovery notification parse exception={}", e.getMessage()); } - } catch (JsonParseException e) { - logger.debug("Failed to parse discovery notification", e); + continue; } + + logger.warn( + "Unhandled message with sourceIp={}, devStatus={}, handler={}, discoveryListener={}", + sourceIp, devStatus, handler, discoveryListener); } - continue; + } catch (IOException e) { + logger.debug("Datagram channel create exception={}", e.getMessage()); } - - // none of the above conditions apply just so log it - logger.warn( - "Unhandled notification for ipAddress:{}, handler:{}, stateNotification:{}, discoveryListener:{}", - ipAddress, handler, isStatusNotification, discoveryListener); - } // {while} - } catch (SocketException e) { - logger.debug("Unexpected socket exception {}", e.getMessage()); + } } finally { - serverSocket = null; serverThread = null; - logger.trace("Server thread finished"); + serverStopFlag = false; + logger.trace("Server thread terminated."); } - } // {while} + } } /** @@ -242,37 +264,29 @@ private static String ipAddressFrom(String host) { } /** - * Call this after one or more listeners have been added. * Starts the server thread if it is not already running. */ - private void listenerCountIncreased() { - synchronized (serverLock) { - Thread thread = serverThread; - if ((thread == null) || thread.isInterrupted() || !thread.isAlive()) { - thread = new Thread(this, "OH-binding-" + GoveeBindingConstants.BINDING_ID); - serverThread = thread; - thread.start(); + private void serverStart() { + synchronized (paramsLock) { + Thread serverthread = serverThread; + if (serverthread == null) { + serverthread = new Thread(this::serverThreadTask, "OH-binding-" + GoveeBindingConstants.BINDING_ID); + serverThread = serverthread; + serverStopFlag = false; + serverthread.start(); } } } /** - * Call this after one or more listeners have been removed. - * Stops the server thread when listener count reaches zero. + * Stops the server thread. */ - private void listenerCountDecreased() { - synchronized (serverLock) { - if (thingHandlerListeners.isEmpty() && (discoveryListener == null)) { - Thread thread = serverThread; - DatagramSocket socket = serverSocket; - if (thread != null) { - thread.interrupt(); // set interrupt flag before closing socket - } - if (socket != null) { - socket.close(); - } - serverThread = null; - serverSocket = null; + private void serverStop() { + synchronized (paramsLock) { + serverStopFlag = true; + Thread serverthread = serverThread; + if (serverthread != null) { + serverthread.interrupt(); } } } diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java index 717a79a91c61e..7fd81c6953aea 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java @@ -15,9 +15,9 @@ import static org.openhab.binding.govee.internal.GoveeBindingConstants.*; import java.io.IOException; +import java.time.Instant; import java.util.ArrayList; import java.util.List; -import java.util.ListIterator; import java.util.Objects; import java.util.concurrent.Callable; import java.util.concurrent.ScheduledExecutorService; @@ -79,23 +79,24 @@ * https://app-h5.govee.com/user-manual/wlan-guide * * @author Stefan Höhn - Initial contribution + * @author Andrew Fiddian-Green - Added sequential task processing */ @NonNullByDefault public class GoveeHandler extends BaseThingHandler { - /* - * Messages to be sent to the Govee devices - */ private static final Gson GSON = new Gson(); + private static final int REFRESH_SECONDS_MIN = 2; + private static final int INTER_COMMAND_DELAY_MILLISEC = 100; private final Logger logger = LoggerFactory.getLogger(GoveeHandler.class); + protected ScheduledExecutorService executorService = scheduler; - @Nullable - private ScheduledFuture triggerStatusJob; // send device status update job + private @Nullable ScheduledFuture thingTaskSenderTask; private GoveeConfiguration goveeConfiguration = new GoveeConfiguration(); private final CommunicationManager communicationManager; private final GoveeStateDescriptionProvider stateDescriptionProvider; + private final List> taskQueue = new ArrayList<>(); private OnOffType lastSwitch = OnOffType.OFF; private HSBType lastColor = new HSBType(); @@ -104,22 +105,39 @@ public class GoveeHandler extends BaseThingHandler { private int minKelvin; private int maxKelvin; - private static final int REFRESH_SECONDS_MIN = 2; - private static final int INTER_COMMAND_DELAY_MILLISEC = 200; + private int refreshIntervalSeconds; + private Instant nextRefreshDueTime = Instant.EPOCH; /** - * This thing related job thingRefreshSender triggers an update to the Govee device. - * The device sends it back to the common port and the response is - * then received by the common #refreshStatusReceiver + * This thing related job thingTaskSender sends the next queued command (if any) + * to the Govee device. If there is no queued command and a regular refresh is due then + * sends the command to trigger a status refresh. + * + * The device may send a reply to the common port and if so the response is received by + * the refresh status receiver. */ - private final Runnable thingRefreshSender = () -> { - try { - triggerDeviceStatusRefresh(); - updateStatus(ThingStatus.ONLINE); - } catch (IOException e) { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, - "@text/offline.communication-error.could-not-query-device [\"" + goveeConfiguration.hostname - + "\"]"); + private final Runnable thingTaskSender = () -> { + synchronized (taskQueue) { + if (taskQueue.isEmpty() && Instant.now().isBefore(nextRefreshDueTime)) { + return; // no queued command nor pending refresh + } + if (taskQueue.isEmpty()) { + taskQueue.add(() -> triggerDeviceStatusRefresh()); + nextRefreshDueTime = Instant.now().plusSeconds(refreshIntervalSeconds); + } else if (taskQueue.size() > 20) { + logger.info("Command task queue size:{} exceeds limit:20", taskQueue.size()); + } + try { + if (taskQueue.remove(0).call()) { + updateStatus(ThingStatus.ONLINE); + } + } catch (IndexOutOfBoundsException e) { + logger.warn("Unexpected List.remove() exception:{}", e.getMessage()); + } catch (Exception e) { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, + "@text/offline.communication-error.could-not-query-device [\"" + goveeConfiguration.hostname + + "\"]"); + } } }; @@ -153,35 +171,37 @@ public void initialize() { "@text/offline.configuration-error.invalid-color-temperature-range"); return; } + thing.setProperty(PROPERTY_COLOR_TEMPERATURE_MIN, Integer.toString(minKelvin)); thing.setProperty(PROPERTY_COLOR_TEMPERATURE_MAX, Integer.toString(maxKelvin)); stateDescriptionProvider.setMinMaxKelvin(new ChannelUID(thing.getUID(), CHANNEL_COLOR_TEMPERATURE_ABS), minKelvin, maxKelvin); - int refreshSecs = goveeConfiguration.refreshInterval; - if (refreshSecs < REFRESH_SECONDS_MIN) { - logger.warn("Config Param refreshInterval={} too low, minimum={}", refreshSecs, REFRESH_SECONDS_MIN); - refreshSecs = REFRESH_SECONDS_MIN; + refreshIntervalSeconds = goveeConfiguration.refreshInterval; + if (refreshIntervalSeconds < REFRESH_SECONDS_MIN) { + logger.warn("Config Param refreshInterval={} too low, minimum={}", refreshIntervalSeconds, + REFRESH_SECONDS_MIN); + refreshIntervalSeconds = REFRESH_SECONDS_MIN; } updateStatus(ThingStatus.UNKNOWN); communicationManager.registerHandler(this); - if (triggerStatusJob == null) { + if (thingTaskSenderTask == null) { logger.debug("Starting refresh trigger job for thing {} ", thing.getLabel()); - triggerStatusJob = executorService.scheduleWithFixedDelay(thingRefreshSender, 100, refreshSecs, - TimeUnit.SECONDS); + thingTaskSenderTask = executorService.scheduleWithFixedDelay(thingTaskSender, INTER_COMMAND_DELAY_MILLISEC, + INTER_COMMAND_DELAY_MILLISEC, TimeUnit.MILLISECONDS); } } @Override public void dispose() { super.dispose(); - - ScheduledFuture triggerStatusJobFuture = triggerStatusJob; - if (triggerStatusJobFuture != null) { - triggerStatusJobFuture.cancel(true); - triggerStatusJob = null; + taskQueue.clear(); + ScheduledFuture job = thingTaskSenderTask; + if (job != null) { + job.cancel(true); + thingTaskSenderTask = null; } communicationManager.unregisterHandler(this); } @@ -190,94 +210,52 @@ public void dispose() { public void handleCommand(ChannelUID channelUID, Command commandParam) { Command command = commandParam; - logger.debug("handleCommand({}, {})", channelUID, command); - - List> communicationTasks = new ArrayList<>(); - boolean mustRefresh = false; - - if (command instanceof RefreshType) { - // we are refreshing all channels at once, as we get all information at the same time - mustRefresh = true; - } else { - switch (channelUID.getId()) { - case CHANNEL_COLOR: - if (command instanceof HSBType hsb) { - communicationTasks.add(() -> sendColor(hsb)); - command = hsb.getBrightness(); // fall through - } - if (command instanceof PercentType percent) { - communicationTasks.add(() -> sendBrightness(percent)); - command = OnOffType.from(percent.intValue() > 0); // fall through - } - if (command instanceof OnOffType onOff) { - communicationTasks.add(() -> sendOnOff(onOff)); - } - break; - - case CHANNEL_COLOR_TEMPERATURE: - if (command instanceof PercentType percent) { - communicationTasks.add(() -> sendKelvin(percentToKelvin(percent))); - } - break; - - case CHANNEL_COLOR_TEMPERATURE_ABS: - if (command instanceof QuantityType genericQuantity) { - QuantityType kelvin = genericQuantity.toInvertibleUnit(Units.KELVIN); - if (kelvin == null) { - logger.warn("handleCommand() invalid QuantityType:{}", genericQuantity); - break; - } - communicationTasks.add(() -> sendKelvin(kelvin.intValue())); - } else if (command instanceof DecimalType kelvin) { - communicationTasks.add(() -> sendKelvin(kelvin.intValue())); - } - break; - } + synchronized (taskQueue) { + logger.debug("handleCommand({}, {})", channelUID, command); - if (mustRefresh || !communicationTasks.isEmpty()) { - communicationTasks.add(() -> triggerDeviceStatusRefresh()); - } - - if (!communicationTasks.isEmpty()) { - scheduler.submit(() -> runCallableTasks(communicationTasks)); - } - } - } + if (command instanceof RefreshType) { + taskQueue.add(() -> triggerDeviceStatusRefresh()); + } else { + switch (channelUID.getId()) { + case CHANNEL_COLOR: + if (command instanceof HSBType hsb) { + taskQueue.add(() -> sendColor(hsb)); + command = hsb.getBrightness(); // fall through + } + if (command instanceof PercentType percent) { + taskQueue.add(() -> sendBrightness(percent)); + command = OnOffType.from(percent.intValue() > 0); // fall through + } + if (command instanceof OnOffType onOff) { + taskQueue.add(() -> sendOnOff(onOff)); + taskQueue.add(() -> triggerDeviceStatusRefresh()); + } + break; - /** - * Iterate over the given list of {@link Callable} tasks and run them with a delay between each. Reason for the - * delay is that Govee lamps cannot process multiple commands in short succession. Update the thing status depending - * on whether any of the tasks encountered an exception. - * - * @param callables a list of Callable tasks (which shall not be empty) - */ - @SuppressWarnings("null") - private void runCallableTasks(List> callables) { - boolean exception = false; - ListIterator> tasks = callables.listIterator(); - while (true) { // don't check hasNext() (we do it below) - try { - tasks.next().call(); - } catch (Exception e) { - exception = true; - break; - } - if (!tasks.hasNext()) { - break; - } - try { - Thread.sleep(INTER_COMMAND_DELAY_MILLISEC); - } catch (InterruptedException e) { - return; + case CHANNEL_COLOR_TEMPERATURE: + if (command instanceof PercentType percent) { + taskQueue.add(() -> sendKelvin(percentToKelvin(percent))); + taskQueue.add(() -> triggerDeviceStatusRefresh()); + } + break; + + case CHANNEL_COLOR_TEMPERATURE_ABS: + if (command instanceof QuantityType genericQuantity) { + QuantityType kelvin = genericQuantity.toInvertibleUnit(Units.KELVIN); + if (kelvin == null) { + logger.warn("handleCommand() invalid QuantityType:{}", genericQuantity); + break; + } + taskQueue.add(() -> sendKelvin(kelvin.intValue())); + taskQueue.add(() -> triggerDeviceStatusRefresh()); + } else if (command instanceof DecimalType kelvin) { + taskQueue.add(() -> sendKelvin(kelvin.intValue())); + taskQueue.add(() -> triggerDeviceStatusRefresh()); + } + break; + } } } - if (!exception) { - updateStatus(ThingStatus.ONLINE); - } else { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, - "@text/offline.communication-error.could-not-query-device [\"" + goveeConfiguration.hostname - + "\"]"); - } } /** @@ -353,7 +331,7 @@ private static HSBType buildHSB(Color normalRgbParams, int brightnessParam, bool return new HSBType(normalColor.getHue(), normalColor.getSaturation(), brightness); } - void handleIncomingStatus(String response) { + public void handleIncomingStatus(String response) { if (response.isEmpty()) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "@text/offline.communication-error.empty-response"); diff --git a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties index fcced7de99c1d..d9c7d3a4b29bd 100644 --- a/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties +++ b/bundles/org.openhab.binding.govee/src/main/resources/OH-INF/i18n/govee.properties @@ -146,6 +146,7 @@ discovery.govee-light.H7066 = H7066 Outdoor Spot Lights discovery.govee-light.H706A = H706A Permanent Outdoor Lights Pro 30M discovery.govee-light.H706B = H706B Permanent Outdoor Lights Pro 45M discovery.govee-light.H706C = H706C Permanent Outdoor Lights Pro 60M +discovery.govee-light.H7070 = H7070 Outdoor Projector Light discovery.govee-light.H7075 = H7075 Outdoor Wall Light discovery.govee-light.H70B1 = H70B1 520 LED Curtain Lights discovery.govee-light.H70BC = H70BC 400 LED Curtain Lights From be443cfdcba9a7d76ebe3fa4e39a4fc0d3558027 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sun, 22 Dec 2024 09:43:05 +0000 Subject: [PATCH 34/35] Update README.md Co-authored-by: lsiepel Signed-off-by: Andrew Fiddian-Green --- bundles/org.openhab.binding.govee/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.openhab.binding.govee/README.md b/bundles/org.openhab.binding.govee/README.md index bcd797a22f87d..25d2c288c4675 100644 --- a/bundles/org.openhab.binding.govee/README.md +++ b/bundles/org.openhab.binding.govee/README.md @@ -141,7 +141,7 @@ Here is a list of the supported devices (the ones marked with * have been tested ## Firewall Govee devices communicate via multicast and unicast messages over the LAN. -So you must ensure that any firewall on your OpenHAB server is configured to pass the following traffic: +So you must ensure that any firewall on your openHAB server is configured to pass the following traffic: - Multicast UDP on 239.255.255.250 port 4001 - Incoming unicast UDP on port 4002 From 21b0ffd2d4a8766fa16909c53280acf09aa59167 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sun, 22 Dec 2024 12:59:25 +0000 Subject: [PATCH 35/35] adopt reviewer suggestion Signed-off-by: Andrew Fiddian-Green --- .../govee/internal/CommunicationManager.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java index ba5baef7d31e4..abecfb6ab1697 100644 --- a/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java +++ b/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java @@ -196,15 +196,10 @@ private void serverThreadTask() { } } catch (ClosedByInterruptException e) { // thrown if 'Thread.interrupt()' is called during 'channel.receive()' - logger.trace( - "Receive exception=ClosedByInterruptException, isInterrupted={}, serverStopFlag={}", + logger.debug("Receive ClosedByInterruptException, isInterrupted={}, serverStopFlag={}", Thread.currentThread().isInterrupted(), serverStopFlag); Thread.interrupted(); // clear 'interrupted' flag - if (serverStopFlag) { - return; - } else { - break; - } + break; } catch (IOException e) { logger.debug("Receive unexpected exception={}", e.getMessage()); break; @@ -239,11 +234,11 @@ private void serverThreadTask() { logger.warn( "Unhandled message with sourceIp={}, devStatus={}, handler={}, discoveryListener={}", sourceIp, devStatus, handler, discoveryListener); - } + } // end of inner while loop } catch (IOException e) { logger.debug("Datagram channel create exception={}", e.getMessage()); } - } + } // end of outer while loop } finally { serverThread = null; serverStopFlag = false;