Skip to content

Commit

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

Signed-off-by: Jacob Laursen <[email protected]>
  • Loading branch information
@jlaur
jlaur committed Nov 28, 2023
1 parent 1c8ce7c commit 33d8a7d
Show file tree
Hide file tree
Showing 8 changed files with 511 additions and 58 deletions.
50 changes: 47 additions & 3 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 @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
45 changes: 44 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,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;

Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -341,7 +348,43 @@ public static Resource setResource(Resource target, Resource source) {
targetTimedEffects.setDuration(duration);
}
}

return target;
}

/**
* Extract on/dimming/color fields from light and grouped light resources
* and merge them into separate resources.
*/
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(onState);
resource.setOnState(null);
}
Dimming dimming = resource.getDimming();
if (dimming != null) {
extractedResource.setDimming(dimming);
resource.setDimming(null);
}
ColorXy colorXy = resource.getColorXy();
if (colorXy != null) {
extractedResource.setColorXy(colorXy);
resource.setColorXy(null);
}
}
}
resources.removeIf(r -> r.isEmpty());
resources.addAll(extractedResources.values());

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
75 changes: 41 additions & 34 deletions ...binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2ThingHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -635,46 +635,53 @@ 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 void putResourceToCache(Resource resource) {
if (SUPPORTED_SCENE_TYPES.contains(resource.getType())) {
sceneContributorsCache.put(resource.getId(), resource);
} else {
serviceContributorsCache.put(resource.getId(), resource);
}
}

private @Nullable Resource getResourceFromCache(Resource resource) {
return SUPPORTED_SCENE_TYPES.contains(resource.getType()) //
? sceneContributorsCache.get(resource.getId())
: serviceContributorsCache.get(resource.getId());
}

/**
* Update the thing internal state depending on a full list of resources sent from the bridge. If the resourceType
* is SCENE then call updateScenes(), otherwise if the resource refers to this thing, consume it via onResource() as
Expand Down
32 changes: 18 additions & 14 deletions bundles/org.openhab.binding.hue/src/main/resources/OH-INF/thing/Clip2Thing.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,18 @@
<description>A Hue API v2 device with channels depending on its actual capabilities.</description>

<channels>
<channel id="color" typeId="system.color"/>
<channel id="color-temperature" typeId="system.color-temperature"/>
<channel id="brightness" typeId="system.brightness"/>
<channel id="switch" typeId="system.power"/>
<channel id="color" typeId="system.color">
<autoUpdatePolicy>veto</autoUpdatePolicy>
</channel>
<channel id="color-temperature" typeId="system.color-temperature">
<autoUpdatePolicy>veto</autoUpdatePolicy>
</channel>
<channel id="brightness" typeId="system.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>
Expand Down Expand Up @@ -85,20 +93,16 @@
<channel id="battery-low" typeId="system.low-battery"/>
<channel id="last-updated" typeId="last-updated-v2"/>
<channel id="dynamics" typeId="dynamics"/>
<channel id="color-temperature-abs" typeId="system.color-temperature-abs"/>
<channel id="color-xy-only" typeId="advanced-color">
<description>Set the color xy parameter of the light without changing other state parameters.</description>
</channel>
<channel id="dimming-only" typeId="advanced-brightness">
<description>Set the dimming parameter of the light without changing other state parameters.</description>
</channel>
<channel id="on-off-only" typeId="advanced-power">
<description>Set the on/off parameter of the light without changing other state parameters.</description>
<channel id="color-temperature-abs" typeId="system.color-temperature-abs">
<autoUpdatePolicy>veto</autoUpdatePolicy>
</channel>
<channel id="color-xy-only" typeId="advanced-color"/>
<channel id="dimming-only" typeId="advanced-brightness"/>
<channel id="on-off-only" typeId="advanced-power"/>
</channels>

<properties>
<property name="thingTypeVersion">1</property>
<property name="thingTypeVersion">2</property>
</properties>

<representation-property>resourceId</representation-property>
Expand Down
6 changes: 6 additions & 0 deletions bundles/org.openhab.binding.hue/src/main/resources/OH-INF/thing/channels.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,20 +240,26 @@
<channel-type id="advanced-color" advanced="true">
<item-type>Color</item-type>
<label>Color XY Only</label>
<description>Set the color xy parameter of the light without changing other state parameters.</description>
<category>ColorLight</category>
<autoUpdatePolicy>veto</autoUpdatePolicy>
</channel-type>

<channel-type id="advanced-brightness" advanced="true">
<item-type>Dimmer</item-type>
<label>Dimming Only</label>
<description>Set the dimming parameter of the light without changing other state parameters.</description>
<category>Light</category>
<state pattern="%.1f %%"/>
<autoUpdatePolicy>veto</autoUpdatePolicy>
</channel-type>

<channel-type id="advanced-power" advanced="true">
<item-type>Switch</item-type>
<label>On/Off Only</label>
<description>Set the on/off parameter of the light without changing other state parameters.</description>
<category>Switch</category>
<autoUpdatePolicy>veto</autoUpdatePolicy>
</channel-type>

<channel-type id="security-contact">
Expand Down
18 changes: 18 additions & 0 deletions bundles/org.openhab.binding.hue/src/main/resources/OH-INF/update/instructions.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,24 @@
</add-channel>
</instruction-set>

<instruction-set targetVersion="2">
<update-channel id="brightness">
<type>system:brightness</type>
</update-channel>
<update-channel id="color">
<type>system:color</type>
</update-channel>
<update-channel id="color-temperature">
<type>system:color-temperature</type>
</update-channel>
<update-channel id="color-temperature-abs">
<type>system:color-temperature-abs</type>
</update-channel>
<update-channel id="switch">
<type>system:power</type>
</update-channel>
</instruction-set>

</thing-type>

</update:update-descriptions>
Loading

0 comments on commit 33d8a7d

Please sign in to comment.