From f6b07943f7abd25324c25f0832282af3016f7526 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Mon, 25 Jan 2021 12:00:55 +0000 Subject: [PATCH 1/7] [hue] implement PR #2144 in openhab.core Signed-off-by: Andrew Fiddian-Green --- .../internal/discovery/HueBridgeDiscoveryParticipant.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java index 278b717ac7186..b81c22372b09a 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java @@ -92,4 +92,10 @@ public Set getSupportedThingTypeUIDs() { } return null; } + + @Override + public int getRemovalGracePeriodSeconds(RemoteDevice device) { + // Hue bridges have maxAge 100 seconds, so apply a grace period of half that + return 50; + } } From 7d88bfd4a5062e899fb3ec8eb4de708548650cd4 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sat, 30 Jan 2021 14:44:53 +0000 Subject: [PATCH 2/7] [hue] add binding configuration parameter; return long Signed-off-by: Andrew Fiddian-Green --- .../hue/internal/HueBindingConstants.java | 3 ++ .../HueBridgeDiscoveryParticipant.java | 36 ++++++++++++++++--- .../main/resources/OH-INF/binding/binding.xml | 14 ++++++-- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/HueBindingConstants.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/HueBindingConstants.java index a914cf14e53f9..543f7d562866b 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/HueBindingConstants.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/HueBindingConstants.java @@ -83,6 +83,9 @@ public class HueBindingConstants { public static final String EVENT_DIMMER_SWITCH = "dimmer_switch_event"; public static final String EVENT_TAP_SWITCH = "tap_switch_event"; + // Binding configuration properties + public static final String REMOVAL_GRACE_PERIOD = "removalGracePeriod"; + // Bridge config properties public static final String HOST = "ipAddress"; public static final String PORT = "port"; diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java index b81c22372b09a..7c4258ba5d9ad 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java @@ -16,6 +16,7 @@ import static org.openhab.core.thing.Thing.PROPERTY_SERIAL_NUMBER; import java.util.Collections; +import java.util.Dictionary; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -25,13 +26,17 @@ import org.jupnp.model.meta.DeviceDetails; import org.jupnp.model.meta.ModelDetails; import org.jupnp.model.meta.RemoteDevice; +import org.openhab.binding.hue.internal.HueBindingConstants; import org.openhab.core.config.discovery.DiscoveryResult; import org.openhab.core.config.discovery.DiscoveryResultBuilder; import org.openhab.core.config.discovery.upnp.UpnpDiscoveryParticipant; import org.openhab.core.config.discovery.upnp.internal.UpnpDiscoveryService; import org.openhab.core.thing.ThingTypeUID; import org.openhab.core.thing.ThingUID; +import org.osgi.service.component.ComponentContext; +import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Modified; /** * The {@link HueBridgeDiscoveryParticipant} is responsible for discovering new and @@ -41,9 +46,33 @@ * @author Thomas Höfer - Added representation */ @NonNullByDefault -@Component(service = UpnpDiscoveryParticipant.class) +@Component(service = UpnpDiscoveryParticipant.class, configurationPid = "binding.hue") public class HueBridgeDiscoveryParticipant implements UpnpDiscoveryParticipant { + // Hue bridges have maxAge 100 seconds, so set the default grace period to half of that + private long removalGracePeriodSeconds = 50; + + @Activate + protected void activate(ComponentContext componentContext) { + activateOrModifyService(componentContext); + } + + @Modified + protected void modified(ComponentContext componentContext) { + activateOrModifyService(componentContext); + } + + private void activateOrModifyService(ComponentContext componentContext) { + Dictionary properties = componentContext.getProperties(); + Object property = properties.get(HueBindingConstants.REMOVAL_GRACE_PERIOD); + if (property != null) { + try { + removalGracePeriodSeconds = Integer.valueOf(property.toString()).longValue(); + } catch (NumberFormatException e) { + } + } + } + @Override public Set getSupportedThingTypeUIDs() { return Collections.singleton(THING_TYPE_BRIDGE); @@ -94,8 +123,7 @@ public Set getSupportedThingTypeUIDs() { } @Override - public int getRemovalGracePeriodSeconds(RemoteDevice device) { - // Hue bridges have maxAge 100 seconds, so apply a grace period of half that - return 50; + public long getRemovalGracePeriodSeconds(RemoteDevice device) { + return removalGracePeriodSeconds; } } diff --git a/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/binding/binding.xml b/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/binding/binding.xml index 90daae7d40426..49db56e52f73a 100644 --- a/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/binding/binding.xml +++ b/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/binding/binding.xml @@ -3,8 +3,16 @@ xmlns:binding="https://openhab.org/schemas/binding/v1.0.0" xsi:schemaLocation="https://openhab.org/schemas/binding/v1.0.0 https://openhab.org/schemas/binding-1.0.0.xsd"> - hue Binding - The hue Binding integrates the Philips hue system. It - allows to control hue bulbs. + Hue Binding + The Hue Binding integrates the Philips Hue system. It allows to control Hue bulbs. + + + + + Extra grace period (seconds) that UPnP discovery shall wait before removing a lost Bridge from the + Inbox. Default is 50 seconds. + 50 + + From f5535465cbec121e01f56d0a006eb0019bbf1247 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sat, 30 Jan 2021 15:37:56 +0000 Subject: [PATCH 3/7] [hue] readme Signed-off-by: Andrew Fiddian-Green --- bundles/org.openhab.binding.hue/README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bundles/org.openhab.binding.hue/README.md b/bundles/org.openhab.binding.hue/README.md index 816f77bb9376f..faebd90842aa6 100644 --- a/bundles/org.openhab.binding.hue/README.md +++ b/bundles/org.openhab.binding.hue/README.md @@ -373,3 +373,11 @@ if (receivedEvent.getEvent() == "1000.0")) { //do stuff } ``` + +### UPnP Discovery: Inbox 'Grace Period' + +The Hue Bridge can sometimes be late in sending its UPnP 'ssdp:alive' notifications even though it has not really gone offline. +This means that the Hue Bridge could be repeatedly removed from, and (re)added to, the InBox. +Which would lead to confusion in the UI, and repeated logger messages. +To prevent this, the binding tells the OpenHAB core to wait for a further period of time ('grace period') before actually removing the Bridge from the Inbox. +The 'grace period' has a default value of 50 seconds, but it can be fine tuned in the main UI via Settings | Bindings | Hue | Configure. From c08cab1a4358f122af341c23a4ef6a7fb9514a96 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Mon, 1 Feb 2021 15:47:23 +0000 Subject: [PATCH 4/7] [hue] remove configurationPid hack Signed-off-by: Andrew Fiddian-Green --- .../HueBridgeDiscoveryParticipant.java | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java index 7c4258ba5d9ad..1364704fb7e0e 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java @@ -15,6 +15,7 @@ import static org.openhab.binding.hue.internal.HueBindingConstants.*; import static org.openhab.core.thing.Thing.PROPERTY_SERIAL_NUMBER; +import java.io.IOException; import java.util.Collections; import java.util.Dictionary; import java.util.HashMap; @@ -33,10 +34,12 @@ import org.openhab.core.config.discovery.upnp.internal.UpnpDiscoveryService; import org.openhab.core.thing.ThingTypeUID; import org.openhab.core.thing.ThingUID; -import org.osgi.service.component.ComponentContext; -import org.osgi.service.component.annotations.Activate; +import org.osgi.service.cm.Configuration; +import org.osgi.service.cm.ConfigurationAdmin; import org.osgi.service.component.annotations.Component; -import org.osgi.service.component.annotations.Modified; +import org.osgi.service.component.annotations.Reference; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * The {@link HueBridgeDiscoveryParticipant} is responsible for discovering new and @@ -46,32 +49,17 @@ * @author Thomas Höfer - Added representation */ @NonNullByDefault -@Component(service = UpnpDiscoveryParticipant.class, configurationPid = "binding.hue") +@Component(service = UpnpDiscoveryParticipant.class) public class HueBridgeDiscoveryParticipant implements UpnpDiscoveryParticipant { + private final Logger logger = LoggerFactory.getLogger(HueBridgeDiscoveryParticipant.class); + // Hue bridges have maxAge 100 seconds, so set the default grace period to half of that private long removalGracePeriodSeconds = 50; - @Activate - protected void activate(ComponentContext componentContext) { - activateOrModifyService(componentContext); - } - - @Modified - protected void modified(ComponentContext componentContext) { - activateOrModifyService(componentContext); - } - - private void activateOrModifyService(ComponentContext componentContext) { - Dictionary properties = componentContext.getProperties(); - Object property = properties.get(HueBindingConstants.REMOVAL_GRACE_PERIOD); - if (property != null) { - try { - removalGracePeriodSeconds = Integer.valueOf(property.toString()).longValue(); - } catch (NumberFormatException e) { - } - } - } + @Reference + @Nullable + ConfigurationAdmin configAdmin; @Override public Set getSupportedThingTypeUIDs() { @@ -124,6 +112,19 @@ public Set getSupportedThingTypeUIDs() { @Override public long getRemovalGracePeriodSeconds(RemoteDevice device) { + if (configAdmin != null) { + try { + Configuration conf = configAdmin.getConfiguration("binding.hue"); + Dictionary properties = conf.getProperties(); + Object property = properties.get(HueBindingConstants.REMOVAL_GRACE_PERIOD); + if (property != null) { + removalGracePeriodSeconds = Integer.valueOf(property.toString()).longValue(); + } + } catch (IOException | IllegalStateException | NullPointerException | NumberFormatException e) { + // fall through to pre-initialised (default) value + } + } + logger.trace("getRemovalGracePeriodSeconds={}", removalGracePeriodSeconds); return removalGracePeriodSeconds; } } From 7082733c0f8cd5ce18dce0e5415fc8460561db5a Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sun, 7 Feb 2021 12:22:18 +0000 Subject: [PATCH 5/7] [hue] use constructor injection Signed-off-by: Andrew Fiddian-Green --- .../HueBridgeDiscoveryParticipant.java | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java index 1364704fb7e0e..f99a4ad362011 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java @@ -36,6 +36,7 @@ import org.openhab.core.thing.ThingUID; import org.osgi.service.cm.Configuration; import org.osgi.service.cm.ConfigurationAdmin; +import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; import org.slf4j.Logger; @@ -57,9 +58,12 @@ public class HueBridgeDiscoveryParticipant implements UpnpDiscoveryParticipant { // Hue bridges have maxAge 100 seconds, so set the default grace period to half of that private long removalGracePeriodSeconds = 50; - @Reference - @Nullable - ConfigurationAdmin configAdmin; + private final ConfigurationAdmin configAdmin; + + @Activate + public HueBridgeDiscoveryParticipant(final @Reference ConfigurationAdmin configAdmin) { + this.configAdmin = configAdmin; + } @Override public Set getSupportedThingTypeUIDs() { @@ -112,17 +116,15 @@ public Set getSupportedThingTypeUIDs() { @Override public long getRemovalGracePeriodSeconds(RemoteDevice device) { - if (configAdmin != null) { - try { - Configuration conf = configAdmin.getConfiguration("binding.hue"); - Dictionary properties = conf.getProperties(); - Object property = properties.get(HueBindingConstants.REMOVAL_GRACE_PERIOD); - if (property != null) { - removalGracePeriodSeconds = Integer.valueOf(property.toString()).longValue(); - } - } catch (IOException | IllegalStateException | NullPointerException | NumberFormatException e) { - // fall through to pre-initialised (default) value + try { + Configuration conf = configAdmin.getConfiguration("binding.hue"); + Dictionary properties = conf.getProperties(); + Object property = properties.get(HueBindingConstants.REMOVAL_GRACE_PERIOD); + if (property != null) { + removalGracePeriodSeconds = Integer.valueOf(property.toString()).longValue(); } + } catch (IOException | IllegalStateException | NullPointerException | NumberFormatException e) { + // fall through to pre-initialised (default) value } logger.trace("getRemovalGracePeriodSeconds={}", removalGracePeriodSeconds); return removalGracePeriodSeconds; From 710e156f98fafa2acb2f0f1b1a9320240380d28e Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sun, 7 Feb 2021 12:27:12 +0000 Subject: [PATCH 6/7] [hue] binding.xml element attributes Signed-off-by: Andrew Fiddian-Green --- .../src/main/resources/OH-INF/binding/binding.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/binding/binding.xml b/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/binding/binding.xml index 49db56e52f73a..87ae19a6aa0b8 100644 --- a/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/binding/binding.xml +++ b/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/binding/binding.xml @@ -7,7 +7,7 @@ The Hue Binding integrates the Philips Hue system. It allows to control Hue bulbs. - + Extra grace period (seconds) that UPnP discovery shall wait before removing a lost Bridge from the Inbox. Default is 50 seconds. From 52a23348a8df2bef3d3f667624816b0a35de50d7 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Tue, 9 Feb 2021 12:58:21 +0000 Subject: [PATCH 7/7] [hue] adopt reviewer suggestions Signed-off-by: Andrew Fiddian-Green --- .../hue/internal/discovery/HueBridgeDiscoveryParticipant.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java index f99a4ad362011..d5bb4ce498354 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeDiscoveryParticipant.java @@ -121,9 +121,9 @@ public long getRemovalGracePeriodSeconds(RemoteDevice device) { Dictionary properties = conf.getProperties(); Object property = properties.get(HueBindingConstants.REMOVAL_GRACE_PERIOD); if (property != null) { - removalGracePeriodSeconds = Integer.valueOf(property.toString()).longValue(); + removalGracePeriodSeconds = Long.parseLong(property.toString()); } - } catch (IOException | IllegalStateException | NullPointerException | NumberFormatException e) { + } catch (IOException | IllegalStateException | NumberFormatException e) { // fall through to pre-initialised (default) value } logger.trace("getRemovalGracePeriodSeconds={}", removalGracePeriodSeconds);