Skip to content

Commit

Permalink
Fix multiple state updates (openhab#15905)
Browse files Browse the repository at this point in the history 
Fixes openhab#15700

Signed-off-by: Jacob Laursen <[email protected]>
Signed-off-by: Jørgen Austvik <[email protected]>
  • Loading branch information
@jlaur @austvik
jlaur authored and austvik committed Mar 27, 2024
1 parent c33ea1f commit efc7540
Show file tree
Hide file tree
Showing 4 changed files with 539 additions and 11 deletions.
38 changes: 34 additions & 4 deletions ...ab.binding.hue/src/main/java/org/openhab/binding/hue/internal/api/dto/clip2/Resource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public class Resource {
private @Nullable @SerializedName("relative_rotary") RelativeRotary relativeRotary;
private @Nullable List<ResourceReference> children;
private @Nullable JsonElement status;
private @Nullable @SuppressWarnings("unused") Dynamics dynamics;
private @Nullable Dynamics dynamics;
private @Nullable @SerializedName("contact_report") ContactReport contactReport;
private @Nullable @SerializedName("tamper_reports") List<TamperReport> tamperReports;
private @Nullable String state;
Expand All @@ -121,6 +121,36 @@ public Resource(@Nullable ResourceType resourceType) {
}
}

/**
* Check if <code>light</code> or <code>grouped_light</code> resource contains any
* relevant fields to process according to its type.
*
* As an example, {@link #colorTemperature} is relevant for a <code>light</code>
* resource because it's needed for updating the color-temperature channels.
*
* @return true is resource contains any relevant field
*/
public boolean hasAnyRelevantField() {
return switch (getType()) {
// https://developers.meethue.com/develop/hue-api-v2/api-reference/#resource_light_get
case LIGHT -> hasHSBField() || colorTemperature != null || dynamics != null || effects != null
|| timedEffects != null;
// https://developers.meethue.com/develop/hue-api-v2/api-reference/#resource_grouped_light_get
case GROUPED_LIGHT -> on != null || dimming != null || alert != null;
default -> throw new IllegalStateException(type + " is not supported by hasAnyRelevantField()");
};
}

/**
* Check if resource contains any field which is needed to represent an HSB value
* (<code>on</code>, <code>dimming</code> or <code>color</code>).
*
* @return true if resource has any HSB field
*/
public boolean hasHSBField() {
return on != null || dimming != null || color != null;
}

public @Nullable List<ActionEntry> getActions() {
return actions;
}
Expand Down Expand Up @@ -777,7 +807,7 @@ public Resource setColorTemperature(ColorTemperature colorTemperature) {
return this;
}

public Resource setColorXy(ColorXy color) {
public Resource setColorXy(@Nullable ColorXy color) {
this.color = color;
return this;
}
Expand All @@ -787,7 +817,7 @@ public Resource setContactReport(ContactReport contactReport) {
return this;
}

public Resource setDimming(Dimming dimming) {
public Resource setDimming(@Nullable Dimming dimming) {
this.dimming = dimming;
return this;
}
Expand Down Expand Up @@ -844,7 +874,7 @@ public Resource setOnOff(Command command) {
return this;
}

public void setOnState(OnState on) {
public void setOnState(@Nullable OnState on) {
this.on = on;
}

Expand Down
59 changes: 58 additions & 1 deletion ...ding.hue/src/main/java/org/openhab/binding/hue/internal/api/dto/clip2/helper/Setters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,13 @@

import java.math.BigDecimal;
import java.time.Duration;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

import javax.measure.Unit;

Expand All @@ -33,6 +38,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;
Expand All @@ -52,6 +58,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
Expand Down Expand Up @@ -341,7 +349,56 @@ public static Resource setResource(Resource target, Resource source) {
targetTimedEffects.setDuration(duration);
}
}

return target;
}

/**
* Merge on/dimming/color fields from light and grouped light resources.
* Subsequent resources will be merged into the first one.
* Full state resources are not supported by this method.
*/
public static Collection<Resource> mergeLightResources(Collection<Resource> resources) {
Map<String, Resource> resourceIndex = new HashMap<>();
Iterator<Resource> iterator = resources.iterator();
while (iterator.hasNext()) {
Resource resource = iterator.next();
String id = resource.getId();

if (resource.hasFullState()) {
throw new IllegalStateException("Resource " + id + " has full state, this is not expected");
}

Resource indexedResource = resourceIndex.get(id);
if (indexedResource == null) {
resourceIndex.put(id, resource);
continue;
}

if (!LIGHT_TYPES.contains(resource.getType()) || !resource.hasHSBField()) {
continue;
}

OnState onState = resource.getOnState();
if (onState != null) {
indexedResource.setOnState(onState);
resource.setOnState(null);
}
Dimming dimming = resource.getDimming();
if (dimming != null) {
indexedResource.setDimming(dimming);
resource.setDimming(null);
}
ColorXy colorXy = resource.getColorXy();
if (colorXy != null) {
indexedResource.setColorXy(colorXy);
resource.setColorXy(null);
}

if (!resource.hasAnyRelevantField()) {
iterator.remove();
}
}

return resources;
}
}
15 changes: 9 additions & 6 deletions ...inding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2BridgeHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
});
}
Expand Down
Loading

0 comments on commit efc7540

Please sign in to comment.