Fix multiple state updates

Fixes openhab#15700

Signed-off-by: Jacob Laursen <[email protected]>

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/api/dto/clip2/ColorXy.java

@@ -29,6 +29,16 @@ public class ColorXy {
     private @Nullable PairXy xy;
     private @Nullable Gamut2 gamut;
 
+    public ColorXy() {
+    }
+
+    public ColorXy(ColorXy source) {
+        PairXy xy = source.xy;
+        this.xy = xy == null ? null : new PairXy(xy);
+        Gamut2 gamut = source.gamut;
+        this.gamut = gamut == null ? null : new Gamut2(gamut);
+    }
+
     public @Nullable Gamut getGamut() {
         Gamut2 gamut = this.gamut;
         return Objects.nonNull(gamut) ? gamut.getGamut() : null;

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/api/dto/clip2/Dimming.java

@@ -32,6 +32,17 @@ public class Dimming {
 
     public static final double DEFAULT_MINIMUM_DIMMIMG_LEVEL = 0.5f;
 
+    public Dimming() {
+    }
+
+    public Dimming(Dimming source) {
+        Double brightness = source.brightness;
+        this.brightness = brightness == null ? null : brightness.doubleValue();
+
+        Double minimumDimmingLevel = source.minimumDimmingLevel;
+        this.minimumDimmingLevel = minimumDimmingLevel == null ? null : minimumDimmingLevel.doubleValue();
+    }
+
     /**
      * @throws DTOPresentButEmptyException to indicate that the DTO is present but empty.
      */

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/api/dto/clip2/Gamut2.java

@@ -29,6 +29,18 @@ public class Gamut2 {
     private @Nullable PairXy green;
     private @Nullable PairXy blue;
 
+    public Gamut2() {
+    }
+
+    public Gamut2(Gamut2 source) {
+        PairXy red = source.red;
+        this.red = red == null ? null : new PairXy(red);
+        PairXy green = source.green;
+        this.green = green == null ? null : new PairXy(green);
+        PairXy blue = source.blue;
+        this.blue = blue == null ? null : new PairXy(blue);
+    }
+
     public @Nullable Gamut getGamut() {
         PairXy red = this.red;
         PairXy green = this.green;

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/api/dto/clip2/OnState.java

@@ -27,6 +27,13 @@
 public class OnState {
     private @Nullable Boolean on;
 
+    public OnState() {
+    }
+
+    public OnState(OnState source) {
+        on = source.on;
+    }
+
     /**
      * @throws DTOPresentButEmptyException to indicate that the DTO is present but empty.
      */

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/api/dto/clip2/PairXy.java

@@ -21,6 +21,14 @@ public class PairXy {
     private double x;
     private double y;
 
+    public PairXy() {
+    }
+
+    public PairXy(PairXy source) {
+        x = source.x;
+        y = source.y;
+    }
+
     public double[] getXY() {
         return new double[] { x, y };
     }

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/api/dto/clip2/Resource.java

@@ -121,6 +121,45 @@ public Resource(@Nullable ResourceType resourceType) {
         }
     }
 
+    /**
+     * Create an empty clone of this object.
+     * The clone will carry the same id, resource type and status.
+     * 
+     * @return cloned object
+     */
+    public Resource cloneEmpty() {
+        Resource clonedResource = new Resource(getType());
+        clonedResource.id = id;
+        clonedResource.hasSparseData = true;
+
+        JsonElement status = this.status;
+        clonedResource.status = status == null ? null : status.deepCopy();
+
+        return clonedResource;
+    }
+
+    /**
+     * Check if resource contains any state information.
+     *
+     * @return true is resource doesn't contain any state information
+     */
+    public boolean isEmpty() {
+        return metadata == null && on == null && dimming == null && colorTemperature == null && color == null
+                && alert == null && effects == null && timedEffects == null && actions == null && recall == null
+                && enabled == null && light == null && button == null && temperature == null && motion == null
+                && powerState == null && relativeRotary == null && contactReport == null && tamperReports == null
+                && status == null;
+    }
+
+    /**
+     * Check if resource contains any HSB component (on, dimming or color).
+     *
+     * @return true if resource has any HSB component
+     */
+    public boolean hasHSBComponent() {
+        return on != null || dimming != null || color != null;
+    }
+
     public @Nullable List<ActionEntry> getActions() {
         return actions;
     }
@@ -777,7 +816,7 @@ public Resource setColorTemperature(ColorTemperature colorTemperature) {
         return this;
     }
 
-    public Resource setColorXy(ColorXy color) {
+    public Resource setColorXy(@Nullable ColorXy color) {
         this.color = color;
         return this;
     }
@@ -787,7 +826,7 @@ public Resource setContactReport(ContactReport contactReport) {
         return this;
     }
 
-    public Resource setDimming(Dimming dimming) {
+    public Resource setDimming(@Nullable Dimming dimming) {
         this.dimming = dimming;
         return this;
     }
@@ -844,7 +883,7 @@ public Resource setOnOff(Command command) {
         return this;
     }
 
-    public void setOnState(OnState on) {
+    public void setOnState(@Nullable OnState on) {
         this.on = on;
     }
 
@@ -889,6 +928,11 @@ public Resource setType(ResourceType resourceType) {
         return this;
     }
 
+    public Resource setStatus(JsonElement status) {
+        this.status = status.deepCopy();
+        return this;
+    }
+
     @Override
     public String toString() {
         String id = this.id;

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/api/dto/clip2/helper/Setters.java

@@ -14,8 +14,12 @@
 
 import java.math.BigDecimal;
 import java.time.Duration;
+import java.util.Collection;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 
 import javax.measure.Unit;
 
@@ -33,6 +37,7 @@
 import org.openhab.binding.hue.internal.api.dto.clip2.TimedEffects;
 import org.openhab.binding.hue.internal.api.dto.clip2.enums.ActionType;
 import org.openhab.binding.hue.internal.api.dto.clip2.enums.EffectType;
+import org.openhab.binding.hue.internal.api.dto.clip2.enums.ResourceType;
 import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.library.types.HSBType;
 import org.openhab.core.library.types.PercentType;
@@ -52,6 +57,8 @@
 @NonNullByDefault
 public class Setters {
 
+    private static final Set<ResourceType> LIGHT_TYPES = Set.of(ResourceType.LIGHT, ResourceType.GROUPED_LIGHT);
+
     /**
      * Setter for Alert field:
      * Use the given command value to set the target resource DTO value based on the attributes of the source resource
@@ -243,14 +250,14 @@ public static Resource setResource(Resource target, Resource source) {
         OnState targetOnOff = target.getOnState();
         OnState sourceOnOff = source.getOnState();
         if (Objects.isNull(targetOnOff) && Objects.nonNull(sourceOnOff)) {
-            target.setOnState(sourceOnOff);
+            target.setOnState(new OnState(sourceOnOff));
         }
 
         // dimming
         Dimming targetDimming = target.getDimming();
         Dimming sourceDimming = source.getDimming();
         if (Objects.isNull(targetDimming) && Objects.nonNull(sourceDimming)) {
-            target.setDimming(sourceDimming);
+            target.setDimming(new Dimming(sourceDimming));
             targetDimming = target.getDimming();
         }
 
@@ -266,7 +273,7 @@ public static Resource setResource(Resource target, Resource source) {
         ColorXy targetColor = target.getColorXy();
         ColorXy sourceColor = source.getColorXy();
         if (Objects.isNull(targetColor) && Objects.nonNull(sourceColor)) {
-            target.setColorXy(sourceColor);
+            target.setColorXy(new ColorXy(sourceColor));
             targetColor = target.getColorXy();
         }
 
@@ -344,4 +351,37 @@ public static Resource setResource(Resource target, Resource source) {
 
         return target;
     }
+
+    public static Collection<Resource> mergeLightResources(Collection<Resource> resources) {
+        Map<String, Resource> extractedResources = new HashMap<>();
+        for (Resource resource : resources) {
+            if (!resource.hasFullState() && LIGHT_TYPES.contains(resource.getType()) && resource.hasHSBComponent()) {
+                String id = resource.getId();
+                Resource extractedResource = extractedResources.get(id);
+                if (extractedResource == null) {
+                    extractedResource = resource.cloneEmpty();
+                    extractedResources.put(id, extractedResource);
+                }
+                OnState onState = resource.getOnState();
+                if (onState != null) {
+                    extractedResource.setOnState(new OnState(onState));
+                    resource.setOnState(null);
+                }
+                Dimming dimming = resource.getDimming();
+                if (dimming != null) {
+                    extractedResource.setDimming(new Dimming(dimming));
+                    resource.setDimming(null);
+                }
+                ColorXy colorXy = resource.getColorXy();
+                if (colorXy != null) {
+                    extractedResource.setColorXy(new ColorXy(colorXy));
+                    resource.setColorXy(null);
+                }
+            }
+        }
+        resources.removeIf(r -> r.isEmpty());
+        resources.addAll(extractedResources.values());
+
+        return resources;
+    }
 }

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2BridgeHandler.java

@@ -37,6 +37,7 @@
 import org.openhab.binding.hue.internal.api.dto.clip2.Resources;
 import org.openhab.binding.hue.internal.api.dto.clip2.enums.Archetype;
 import org.openhab.binding.hue.internal.api.dto.clip2.enums.ResourceType;
+import org.openhab.binding.hue.internal.api.dto.clip2.helper.Setters;
 import org.openhab.binding.hue.internal.config.Clip2BridgeConfig;
 import org.openhab.binding.hue.internal.connection.Clip2Bridge;
 import org.openhab.binding.hue.internal.connection.HueTlsTrustManagerProvider;
@@ -528,13 +529,15 @@ public void onResourcesEvent(List<Resource> resources) {
     }
 
     private void onResourcesEventTask(List<Resource> resources) {
-        logger.debug("onResourcesEventTask() resource count {}", resources.size());
+        int numberOfResources = resources.size();
+        logger.debug("onResourcesEventTask() resource count {}", numberOfResources);
+        Setters.mergeLightResources(resources);
+        if (numberOfResources != resources.size()) {
+            logger.debug("onResourcesEventTask() merged to {} resources", resources.size());
+        }
         getThing().getThings().forEach(thing -> {
-            ThingHandler handler = thing.getHandler();
-            if (handler instanceof Clip2ThingHandler) {
-                resources.forEach(resource -> {
-                    ((Clip2ThingHandler) handler).onResource(resource);
-                });
+            if (thing.getHandler() instanceof Clip2ThingHandler clip2ThingHandler) {
+                resources.forEach(resource -> clip2ThingHandler.onResource(resource));
             }
         });
     }

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2ThingHandler.java

@@ -635,43 +635,50 @@ public void initialize() {
      * @param resource a Resource object containing the new state.
      */
     public void onResource(Resource resource) {
-        if (!disposing) {
-            boolean resourceConsumed = false;
-            String incomingResourceId = resource.getId();
-            if (resourceId.equals(incomingResourceId)) {
-                if (resource.hasFullState()) {
-                    thisResource = resource;
-                    if (!updatePropertiesDone) {
-                        updateProperties(resource);
-                        resourceConsumed = updatePropertiesDone;
-                    }
-                }
-                if (!updateDependenciesDone) {
-                    resourceConsumed = true;
-                    cancelTask(updateDependenciesTask, false);
-                    updateDependenciesTask = scheduler.submit(() -> updateDependencies());
-                }
-            } else if (SUPPORTED_SCENE_TYPES.contains(resource.getType())) {
-                Resource cachedScene = sceneContributorsCache.get(incomingResourceId);
-                if (Objects.nonNull(cachedScene)) {
-                    Setters.setResource(resource, cachedScene);
-                    resourceConsumed = updateChannels(resource);
-                    sceneContributorsCache.put(incomingResourceId, resource);
-                }
-            } else {
-                Resource cachedService = serviceContributorsCache.get(incomingResourceId);
-                if (Objects.nonNull(cachedService)) {
-                    Setters.setResource(resource, cachedService);
-                    resourceConsumed = updateChannels(resource);
-                    serviceContributorsCache.put(incomingResourceId, resource);
-                    if (ResourceType.LIGHT == resource.getType() && !updateLightPropertiesDone) {
-                        updateLightProperties(resource);
-                    }
+        if (disposing) {
+            return;
+        }
+        boolean resourceConsumed = false;
+        if (resourceId.equals(resource.getId())) {
+            if (resource.hasFullState()) {
+                thisResource = resource;
+                if (!updatePropertiesDone) {
+                    updateProperties(resource);
+                    resourceConsumed = updatePropertiesDone;
                 }
             }
-            if (resourceConsumed) {
-                logger.debug("{} -> onResource() consumed resource {}", resourceId, resource);
+            if (!updateDependenciesDone) {
+                resourceConsumed = true;
+                cancelTask(updateDependenciesTask, false);
+                updateDependenciesTask = scheduler.submit(() -> updateDependencies());
             }
+        } else {
+            Resource cachedResource = getResourceFromCache(resource);
+            if (cachedResource != null) {
+                Setters.setResource(resource, cachedResource);
+                resourceConsumed = updateChannels(resource);
+                putResourceToCache(resource);
+                if (ResourceType.LIGHT == resource.getType() && !updateLightPropertiesDone) {
+                    updateLightProperties(resource);
+                }
+            }
+        }
+        if (resourceConsumed) {
+            logger.debug("{} -> onResource() consumed resource {}", resourceId, resource);
+        }
+    }
+
+    private @Nullable Resource getResourceFromCache(Resource resource) {
+        return SUPPORTED_SCENE_TYPES.contains(resource.getType()) //
+                ? sceneContributorsCache.get(resource.getId())
+                : serviceContributorsCache.get(resource.getId());
+    }
+
+    private void putResourceToCache(Resource resource) {
+        if (SUPPORTED_SCENE_TYPES.contains(resource.getType())) {
+            sceneContributorsCache.put(resource.getId(), resource);
+        } else {
+            serviceContributorsCache.put(resource.getId(), resource);
         }
     }
 

bundles/org.openhab.binding.hue/src/main/resources/OH-INF/thing/Clip2Thing.xml

@@ -13,10 +13,19 @@
 		<description>A Hue API v2 device with channels depending on its actual capabilities.</description>
 
 		<channels>
-			<channel id="color" typeId="system.color"/>
+			<channel id="color" typeId="system.color">
+				<autoUpdatePolicy>veto</autoUpdatePolicy>
+			</channel>
 			<channel id="color-temperature" typeId="system.color-temperature"/>
-			<channel id="brightness" typeId="system.brightness"/>
-			<channel id="switch" typeId="system.power"/>
+			<!-- Overriding auto update policy for channel is currently not working.
+				See https://github.com/openhab/openhab-core/issues/3887
+				Therefore channel type system.brightness has been duplicated -->
+			<channel id="brightness" typeId="brightness">
+				<autoUpdatePolicy>veto</autoUpdatePolicy>
+			</channel>
+			<channel id="switch" typeId="system.power">
+				<autoUpdatePolicy>veto</autoUpdatePolicy>
+			</channel>
 			<channel id="alert" typeId="alert-v2">
 				<description>Activate the alert for the light.</description>
 			</channel>