Fix multiple state updates (#15905)

Fixes #15700

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
This commit is contained in:
Jacob Laursen 2023-12-04 22:40:31 +01:00 committed by GitHub
parent f5065ad6e4
commit a76187fa3a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 539 additions and 11 deletions

View File

@ -105,7 +105,7 @@ public class Resource {
private @Nullable @SerializedName("relative_rotary") RelativeRotary relativeRotary; private @Nullable @SerializedName("relative_rotary") RelativeRotary relativeRotary;
private @Nullable List<ResourceReference> children; private @Nullable List<ResourceReference> children;
private @Nullable JsonElement status; private @Nullable JsonElement status;
private @Nullable @SuppressWarnings("unused") Dynamics dynamics; private @Nullable Dynamics dynamics;
private @Nullable @SerializedName("contact_report") ContactReport contactReport; private @Nullable @SerializedName("contact_report") ContactReport contactReport;
private @Nullable @SerializedName("tamper_reports") List<TamperReport> tamperReports; private @Nullable @SerializedName("tamper_reports") List<TamperReport> tamperReports;
private @Nullable String state; private @Nullable String state;
@ -121,6 +121,36 @@ public class Resource {
} }
} }
/**
* 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() { public @Nullable List<ActionEntry> getActions() {
return actions; return actions;
} }
@ -777,7 +807,7 @@ public class Resource {
return this; return this;
} }
public Resource setColorXy(ColorXy color) { public Resource setColorXy(@Nullable ColorXy color) {
this.color = color; this.color = color;
return this; return this;
} }
@ -787,7 +817,7 @@ public class Resource {
return this; return this;
} }
public Resource setDimming(Dimming dimming) { public Resource setDimming(@Nullable Dimming dimming) {
this.dimming = dimming; this.dimming = dimming;
return this; return this;
} }
@ -844,7 +874,7 @@ public class Resource {
return this; return this;
} }
public void setOnState(OnState on) { public void setOnState(@Nullable OnState on) {
this.on = on; this.on = on;
} }

View File

@ -14,8 +14,13 @@ package org.openhab.binding.hue.internal.api.dto.clip2.helper;
import java.math.BigDecimal; import java.math.BigDecimal;
import java.time.Duration; import java.time.Duration;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.Set;
import javax.measure.Unit; import javax.measure.Unit;
@ -33,6 +38,7 @@ import org.openhab.binding.hue.internal.api.dto.clip2.Resource;
import org.openhab.binding.hue.internal.api.dto.clip2.TimedEffects; 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.ActionType;
import org.openhab.binding.hue.internal.api.dto.clip2.enums.EffectType; 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.DecimalType;
import org.openhab.core.library.types.HSBType; import org.openhab.core.library.types.HSBType;
import org.openhab.core.library.types.PercentType; import org.openhab.core.library.types.PercentType;
@ -52,6 +58,8 @@ import org.openhab.core.util.ColorUtil.Gamut;
@NonNullByDefault @NonNullByDefault
public class Setters { public class Setters {
private static final Set<ResourceType> LIGHT_TYPES = Set.of(ResourceType.LIGHT, ResourceType.GROUPED_LIGHT);
/** /**
* Setter for Alert field: * Setter for Alert field:
* Use the given command value to set the target resource DTO value based on the attributes of the source resource * Use the given command value to set the target resource DTO value based on the attributes of the source resource
@ -341,7 +349,56 @@ public class Setters {
targetTimedEffects.setDuration(duration); targetTimedEffects.setDuration(duration);
} }
} }
return target; 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;
}
} }

View File

@ -37,6 +37,7 @@ import org.openhab.binding.hue.internal.api.dto.clip2.ResourceReference;
import org.openhab.binding.hue.internal.api.dto.clip2.Resources; 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.Archetype;
import org.openhab.binding.hue.internal.api.dto.clip2.enums.ResourceType; 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.config.Clip2BridgeConfig;
import org.openhab.binding.hue.internal.connection.Clip2Bridge; import org.openhab.binding.hue.internal.connection.Clip2Bridge;
import org.openhab.binding.hue.internal.connection.HueTlsTrustManagerProvider; import org.openhab.binding.hue.internal.connection.HueTlsTrustManagerProvider;
@ -528,13 +529,15 @@ public class Clip2BridgeHandler extends BaseBridgeHandler {
} }
private void onResourcesEventTask(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 -> { getThing().getThings().forEach(thing -> {
ThingHandler handler = thing.getHandler(); if (thing.getHandler() instanceof Clip2ThingHandler clip2ThingHandler) {
if (handler instanceof Clip2ThingHandler) { resources.forEach(resource -> clip2ThingHandler.onResource(resource));
resources.forEach(resource -> {
((Clip2ThingHandler) handler).onResource(resource);
});
} }
}); });
} }

View File

@ -0,0 +1,438 @@
/**
* Copyright (c) 2010-2023 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.hue.internal.clip2;
import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.openhab.binding.hue.internal.api.dto.clip2.ColorTemperature;
import org.openhab.binding.hue.internal.api.dto.clip2.Dimming;
import org.openhab.binding.hue.internal.api.dto.clip2.Effects;
import org.openhab.binding.hue.internal.api.dto.clip2.OnState;
import org.openhab.binding.hue.internal.api.dto.clip2.Resource;
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.exceptions.DTOPresentButEmptyException;
/**
* Tests for {@link Setters}.
*
* @author Jacob Laursen - Initial contribution
*/
@NonNullByDefault
public class SettersTest {
/**
* Tests merging of on state and dimming for same resource.
*
* Input:
* - Resource 1: type=light/grouped_light, sparse, id=1, on=on
* - Resource 2: type=light/grouped_light, sparse, id=1, dimming=50
*
* Expected output:
* - Resource 1: type=light/grouped_light, sparse, id=1, on=on, dimming=50
*
* @throws DTOPresentButEmptyException
*/
@ParameterizedTest
@MethodSource("provideLightResourceTypes")
void mergeLightResourcesMergeOnStateAndDimmingWhenSparseAndSameId(ResourceType resourceType)
throws DTOPresentButEmptyException {
List<Resource> resources = new ArrayList<>();
Resource resource1 = createResource(resourceType, "1");
resource1.setOnState(createOnState(true));
resources.add(resource1);
Resource resource2 = createResource(resourceType, "1");
resource2.setDimming(createDimming(50));
resources.add(resource2);
Setters.mergeLightResources(resources);
assertThat(resources.size(), is(equalTo(1)));
Resource mergedResource = resources.get(0);
assertThat(mergedResource.getId(), is(equalTo(resource1.getId())));
assertThat(mergedResource.getType(), is(equalTo(resourceType)));
assertThat(mergedResource.hasFullState(), is(false));
OnState actualOnState = mergedResource.getOnState();
assertThat(actualOnState, is(notNullValue()));
if (actualOnState != null) {
assertThat(actualOnState.isOn(), is(true));
}
Dimming actualDimming = mergedResource.getDimming();
assertThat(actualDimming, is(notNullValue()));
if (actualDimming != null) {
assertThat(actualDimming.getBrightness(), is(equalTo(50.0)));
}
}
private static Stream<Arguments> provideLightResourceTypes() {
return Stream.of(Arguments.of(ResourceType.LIGHT), Arguments.of(ResourceType.GROUPED_LIGHT));
}
/**
* Tests merging of dimming for same resource where last value wins.
*
* Input:
* - Resource 1: type=light, sparse, id=1, dimming=49
* - Resource 2: type=light, sparse, id=1, dimming=50
*
* Expected output:
* - Resource 1: type=light, sparse, id=1, dimming=50
*
* @throws DTOPresentButEmptyException
*/
@Test
void mergeLightResourcesMergeDimmingToLatestValueWhenSparseAndSameId() throws DTOPresentButEmptyException {
List<Resource> resources = new ArrayList<>();
Resource resource1 = createResource(ResourceType.LIGHT, "1");
resource1.setDimming(createDimming(49));
resources.add(resource1);
Resource resource2 = createResource(ResourceType.LIGHT, "1");
resource2.setDimming(createDimming(50));
resources.add(resource2);
Setters.mergeLightResources(resources);
assertThat(resources.size(), is(equalTo(1)));
Resource mergedResource = resources.get(0);
assertThat(mergedResource.hasFullState(), is(false));
Dimming actualDimming = mergedResource.getDimming();
assertThat(actualDimming, is(notNullValue()));
if (actualDimming != null) {
assertThat(actualDimming.getBrightness(), is(equalTo(50.0)));
}
}
/**
* Tests merging of HSB type fields while keeping resource with effects.
*
* Input:
* - Resource 1: type=light, sparse, id=1, dimming=49
* - Resource 2: type=light, sparse, id=1, on=on, dimming=50, effect=xxx
*
* Expected output:
* - Resource 1: type=light, sparse, id=1, on=on, dimming=50
* - Resource 2: type=light, sparse, id=1, effect=xxx
*
* @throws DTOPresentButEmptyException
*/
@Test
void mergeLightResourcesMergeHSBFieldsDoNotRemoveResourceWithEffect() throws DTOPresentButEmptyException {
List<Resource> resources = new ArrayList<>();
Resource resource1 = createResource(ResourceType.LIGHT, "1");
resource1.setDimming(createDimming(49));
resources.add(resource1);
Resource resource2 = createResource(ResourceType.LIGHT, "1");
resource2.setDimming(createDimming(50));
resource2.setOnState(createOnState(true));
resource2.setFixedEffects(new Effects());
resources.add(resource2);
Setters.mergeLightResources(resources);
assertThat(resources.size(), is(equalTo(2)));
Resource mergedResource = resources.get(0);
assertThat(mergedResource.hasFullState(), is(false));
OnState actualOnState = mergedResource.getOnState();
assertThat(actualOnState, is(notNullValue()));
if (actualOnState != null) {
assertThat(actualOnState.isOn(), is(true));
}
Dimming actualDimming = mergedResource.getDimming();
assertThat(actualDimming, is(notNullValue()));
if (actualDimming != null) {
assertThat(actualDimming.getBrightness(), is(equalTo(50.0)));
}
Resource effectsResource = resources.get(1);
assertThat(effectsResource.hasFullState(), is(false));
assertThat(effectsResource.getFixedEffects(), is(notNullValue()));
}
/**
* Tests leaving different resources separated.
*
* Input:
* - Resource 1: type=light, sparse, id=1, on=on
* - Resource 2: type=light, sparse, id=2, dimming=50
*
* Expected output:
* - Resource 1: type=light, sparse, id=1, on=on
* - Resource 2: type=light, sparse, id=2, dimming=50
*
* @throws DTOPresentButEmptyException
*/
@Test
void mergeLightResourcesDoNotMergeOnStateAndDimmingWhenSparseAndDifferentId() throws DTOPresentButEmptyException {
List<Resource> resources = new ArrayList<>();
Resource resource1 = createResource(ResourceType.LIGHT, "1");
resource1.setOnState(createOnState(true));
resources.add(resource1);
Resource resource2 = createResource(ResourceType.LIGHT, "2");
resource2.setDimming(createDimming(50));
resources.add(resource2);
Setters.mergeLightResources(resources);
assertThat(resources.size(), is(equalTo(2)));
Resource firstResource = resources.get(0);
OnState actualOnState = firstResource.getOnState();
assertThat(actualOnState, is(notNullValue()));
if (actualOnState != null) {
assertThat(actualOnState.isOn(), is(true));
}
assertThat(firstResource.getDimming(), is(nullValue()));
Resource secondResource = resources.get(1);
assertThat(secondResource.getOnState(), is(nullValue()));
Dimming actualDimming = secondResource.getDimming();
assertThat(actualDimming, is(notNullValue()));
if (actualDimming != null) {
assertThat(actualDimming.getBrightness(), is(equalTo(50.0)));
}
}
/**
* Tests merging of on state and dimming for same resource when full is first.
*
* Input:
* - Resource 1: type=light, full, id=1, on=on
*
* Expected output:
* - Exception thrown, full state is not supported/expected.
*
* @throws DTOPresentButEmptyException
*/
@Test
void mergeLightResourcesMergeOnStateAndDimmingWhenFullStateFirstAndSameId() throws DTOPresentButEmptyException {
List<Resource> resources = new ArrayList<>();
Resource resource = new Resource(ResourceType.LIGHT);
resource.setId("1");
resources.add(resource);
assertThrows(IllegalStateException.class, () -> Setters.mergeLightResources(resources));
}
/**
* Tests leaving resources with on state and color temperature separated.
* In this case they could be merged, but it's not needed.
*
* Input:
* - Resource 1: type=light, sparse, id=1, on=on
* - Resource 2: type=light, sparse, id=1, color temperature=370 mirek
*
* Expected output:
* - Resource 1: type=light, sparse, id=1, on=on
* - Resource 2: type=light, sparse, id=1, color temperature=370 mirek
*
* @throws DTOPresentButEmptyException
*/
@Test
void mergeLightResourcesDoNotMergeOnStateAndColorTemperatureWhenSparseAndSameId()
throws DTOPresentButEmptyException {
List<Resource> resources = new ArrayList<>();
Resource resource1 = createResource(ResourceType.LIGHT, "1");
resource1.setOnState(createOnState(true));
resources.add(resource1);
Resource resource2 = createResource(ResourceType.LIGHT, "1");
resource2.setColorTemperature(createColorTemperature(370));
resources.add(resource2);
Setters.mergeLightResources(resources);
assertThat(resources.size(), is(equalTo(2)));
Resource firstResource = resources.get(0);
OnState actualOnState = firstResource.getOnState();
assertThat(actualOnState, is(notNullValue()));
if (actualOnState != null) {
assertThat(actualOnState.isOn(), is(true));
}
assertThat(firstResource.getColorTemperature(), is(nullValue()));
Resource secondResource = resources.get(1);
assertThat(secondResource.getOnState(), is(nullValue()));
ColorTemperature actualColorTemperature = secondResource.getColorTemperature();
assertThat(actualColorTemperature, is(notNullValue()));
if (actualColorTemperature != null) {
assertThat(actualColorTemperature.getMirek(), is(equalTo(370L)));
}
}
/**
* Tests merging resources with on state/color and leaving color temperature separated.
* In this case they could be merged, but it's not needed.
*
* Input:
* - Resource 1: type=light, sparse, id=1, on=on
* - Resource 2: type=light, sparse, id=1, dimming=50, color temperature=370 mirek
*
* Expected output:
* - Resource 1: type=light, sparse, id=1, on=on, dimming=50
* - Resource 2: type=light, sparse, id=1, color temperature=370 mirek
*
* @throws DTOPresentButEmptyException
*/
@Test
void mergeLightResourcesMergeOnStateAndDimmingButNotColorTemperatureWhenSparseAndSameId()
throws DTOPresentButEmptyException {
List<Resource> resources = new ArrayList<>();
Resource resource1 = createResource(ResourceType.LIGHT, "1");
resource1.setOnState(createOnState(true));
resources.add(resource1);
Resource resource2 = createResource(ResourceType.LIGHT, "1");
resource2.setDimming(createDimming(50));
resource2.setColorTemperature(createColorTemperature(370));
resources.add(resource2);
Setters.mergeLightResources(resources);
assertThat(resources.size(), is(equalTo(2)));
Resource firstResource = resources.get(0);
OnState actualOnState = firstResource.getOnState();
assertThat(actualOnState, is(notNullValue()));
if (actualOnState != null) {
assertThat(actualOnState.isOn(), is(true));
}
Dimming actualDimming = firstResource.getDimming();
assertThat(actualDimming, is(notNullValue()));
if (actualDimming != null) {
assertThat(actualDimming.getBrightness(), is(equalTo(50.0)));
}
assertThat(firstResource.getColorTemperature(), is(nullValue()));
Resource secondResource = resources.get(1);
assertThat(secondResource.getOnState(), is(nullValue()));
assertThat(secondResource.getDimming(), is(nullValue()));
ColorTemperature actualColorTemperature = secondResource.getColorTemperature();
assertThat(actualColorTemperature, is(notNullValue()));
if (actualColorTemperature != null) {
assertThat(actualColorTemperature.getMirek(), is(equalTo(370L)));
}
}
/**
* Tests preserving resource with on state and color temperature.
*
* Input:
* - Resource 1: type=light, sparse, id=1, on=on, color temperature=370
*
* Expected output:
* - Resource 1: type=light, sparse, id=1, on=on, color temperature=370
*
* @throws DTOPresentButEmptyException
*/
@Test
void mergeLightResourcesSeparateOnStateAndColorTemperatureWhenSparseAndSameId() throws DTOPresentButEmptyException {
List<Resource> resources = new ArrayList<>();
Resource resource = createResource(ResourceType.LIGHT, "1");
resource.setOnState(createOnState(true));
resource.setColorTemperature(createColorTemperature(370));
resources.add(resource);
Setters.mergeLightResources(resources);
assertThat(resources.size(), is(equalTo(1)));
Resource firstResource = resources.get(0);
OnState actualOnState = firstResource.getOnState();
assertThat(actualOnState, is(notNullValue()));
if (actualOnState != null) {
assertThat(actualOnState.isOn(), is(true));
}
ColorTemperature actualColorTemperature = firstResource.getColorTemperature();
assertThat(actualColorTemperature, is(notNullValue()));
if (actualColorTemperature != null) {
assertThat(actualColorTemperature.getMirek(), is(equalTo(370L)));
}
}
/**
* Tests that resources that are not light or grouped_light will not throw.
*
* Input:
* - Resource 1: type=motion, sparse, id=1
*
* Expected output:
* - Resource 1: type=motion, sparse, id=1
*
* @throws DTOPresentButEmptyException
*/
@Test
void mergeLightResourcesNonLightResourceShouldNotThrow() throws DTOPresentButEmptyException {
List<Resource> resources = new ArrayList<>();
Resource resource = createResource(ResourceType.MOTION, "1");
resources.add(resource);
Setters.mergeLightResources(resources);
assertThat(resources.size(), is(equalTo(1)));
Resource firstResource = resources.get(0);
assertThat(firstResource.getType(), is(equalTo(ResourceType.MOTION)));
}
private OnState createOnState(boolean on) {
OnState onState = new OnState();
onState.setOn(on);
return onState;
}
private Dimming createDimming(double brightness) {
Dimming dimming = new Dimming();
dimming.setBrightness(brightness);
return dimming;
}
private ColorTemperature createColorTemperature(double mirek) {
ColorTemperature colorTemperature = new ColorTemperature();
colorTemperature.setMirek(mirek);
return colorTemperature;
}
private Resource createResource(ResourceType resourceType, String id) {
Resource resource = new Resource(resourceType);
resource.setId(id);
resource.markAsSparse();
return resource;
}
}