From 1b8696ef34d4e7c5fdf1da0904d808decf6e9623 Mon Sep 17 00:00:00 2001 From: David Pace Date: Sun, 18 Aug 2024 15:41:17 +0200 Subject: [PATCH] [boschshc] Update location properties when initializing things When a device is initialized, an attempt is made to look up the room name for the room id specified for the device and to store the room name in a thing property. This property is also updated if a room change is detected. The legacy property `Location` is removed if present. From now on the property `location` (with proper lower case spelling) is used. * add constants for location properties * implement location updates in abstract device handler * extend bridge handler to provide a cached list of rooms * add unit tests Signed-off-by: David Pace --- .../devices/BoschSHCBindingConstants.java | 4 ++ .../devices/BoschSHCDeviceHandler.java | 39 +++++++++++ .../devices/bridge/BridgeHandler.java | 33 ++++++++- .../discovery/ThingDiscoveryService.java | 2 +- .../AbstractBoschSHCDeviceHandlerTest.java | 69 +++++++++++++++++++ .../devices/relay/RelayHandlerTest.java | 54 +++++++++++++++ .../discovery/ThingDiscoveryServiceTest.java | 3 +- 7 files changed, 201 insertions(+), 3 deletions(-) diff --git a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/BoschSHCBindingConstants.java b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/BoschSHCBindingConstants.java index f48a18bb90606..4b803683fdeb6 100644 --- a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/BoschSHCBindingConstants.java +++ b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/BoschSHCBindingConstants.java @@ -128,4 +128,8 @@ private BoschSHCBindingConstants() { // static device/service names public static final String SERVICE_INTRUSION_DETECTION = "intrusionDetectionSystem"; + + // thing properties + public static final String PROPERTY_LOCATION_LEGACY = "Location"; + public static final String PROPERTY_LOCATION = "location"; } diff --git a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/BoschSHCDeviceHandler.java b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/BoschSHCDeviceHandler.java index 6791b19c9273d..912543900f2d2 100644 --- a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/BoschSHCDeviceHandler.java +++ b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/BoschSHCDeviceHandler.java @@ -12,6 +12,10 @@ */ package org.openhab.binding.boschshc.internal.devices; +import static org.openhab.binding.boschshc.internal.devices.BoschSHCBindingConstants.PROPERTY_LOCATION; +import static org.openhab.binding.boschshc.internal.devices.BoschSHCBindingConstants.PROPERTY_LOCATION_LEGACY; + +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; @@ -83,9 +87,44 @@ public void initialize() { * otherwise */ protected boolean processDeviceInfo(Device deviceInfo) { + try { + updateLocationPropertiesIfApplicable(deviceInfo); + } catch (Exception e) { + logger.warn("Error while updating location properties for thing {}.", getThing().getUID(), e); + } + // do not cancel thing initialization if location properties cannot be updated return true; } + private void updateLocationPropertiesIfApplicable(Device deviceInfo) throws BoschSHCException { + Map thingProperties = getThing().getProperties(); + removeLegacyLocationPropertyIfApplicable(thingProperties); + updateLocationPropertyIfApplicable(thingProperties, deviceInfo); + } + + private void updateLocationPropertyIfApplicable(Map thingProperties, Device deviceInfo) + throws BoschSHCException { + @Nullable + String roomName = getBridgeHandler().resolveRoomId(deviceInfo.roomId); + if (roomName != null) { + @Nullable + String currentLocation = thingProperties.get(PROPERTY_LOCATION); + if (currentLocation == null || !currentLocation.equals(roomName)) { + logger.debug("Updating property '{}' of thing {} to '{}'.", PROPERTY_LOCATION, getThing().getUID(), + roomName); + updateProperty(PROPERTY_LOCATION, roomName); + } + } + } + + private void removeLegacyLocationPropertyIfApplicable(Map thingProperties) { + if (thingProperties.containsKey(PROPERTY_LOCATION_LEGACY)) { + logger.debug("Removing legacy property '{}' from thing {}.", PROPERTY_LOCATION_LEGACY, getThing().getUID()); + // null value indicates that the property should be removed + updateProperty(PROPERTY_LOCATION_LEGACY, null); + } + } + /** * Attempts to obtain information about the device with the specified ID via a REST call. *

diff --git a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/BridgeHandler.java b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/BridgeHandler.java index 9575de3c9556c..56e0246e504a2 100644 --- a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/BridgeHandler.java +++ b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/BridgeHandler.java @@ -17,6 +17,7 @@ import static org.eclipse.jetty.http.HttpMethod.PUT; import java.lang.reflect.Type; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -54,6 +55,7 @@ import org.openhab.binding.boschshc.internal.serialization.GsonUtils; import org.openhab.binding.boschshc.internal.services.dto.BoschSHCServiceState; import org.openhab.binding.boschshc.internal.services.dto.JsonRestExceptionResponse; +import org.openhab.core.cache.ExpiringCache; import org.openhab.core.library.types.StringType; import org.openhab.core.thing.Bridge; import org.openhab.core.thing.Channel; @@ -88,6 +90,8 @@ public class BridgeHandler extends BaseBridgeHandler { private static final String HTTP_CLIENT_NOT_INITIALIZED = "HttpClient not initialized"; + private static final Duration ROOM_CACHE_DURATION = Duration.ofMinutes(2); + private final Logger logger = LoggerFactory.getLogger(BridgeHandler.class); /** @@ -107,13 +111,22 @@ public class BridgeHandler extends BaseBridgeHandler { /** * SHC thing/device discovery service instance. - * Registered and unregistered if service is actived/deactived. + * Registered and unregistered if service is activated/deactivated. * Used to scan for things after bridge is paired with SHC. */ private @Nullable ThingDiscoveryService thingDiscoveryService; private final ScenarioHandler scenarioHandler; + private ExpiringCache> roomCache = new ExpiringCache<>(ROOM_CACHE_DURATION, () -> { + try { + return getRooms(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return null; + } + }); + public BridgeHandler(Bridge bridge) { super(bridge); scenarioHandler = new ScenarioHandler(); @@ -437,6 +450,24 @@ public List getRooms() throws InterruptedException { } } + public @Nullable List getRoomsWithCache() { + return roomCache.getValue(); + } + + public @Nullable String resolveRoomId(@Nullable String roomId) { + if (roomId == null) { + return null; + } + + @Nullable + List rooms = getRoomsWithCache(); + if (rooms != null) { + return rooms.stream().filter(r -> r.id.equals(roomId)).map(r -> r.name).findAny().orElse(null); + } + + return null; + } + /** * Get public information from Bosch SHC. */ diff --git a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java index 6f5bb912cca2c..42ad3e2ff3d8d 100644 --- a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java +++ b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java @@ -242,7 +242,7 @@ protected void addDevice(Device device, String roomName) { discoveryResult.withBridge(thingHandler.getThing().getUID()); if (!roomName.isEmpty()) { - discoveryResult.withProperty("Location", roomName); + discoveryResult.withProperty(BoschSHCBindingConstants.PROPERTY_LOCATION, roomName); } thingDiscovered(discoveryResult.build()); diff --git a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/AbstractBoschSHCDeviceHandlerTest.java b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/AbstractBoschSHCDeviceHandlerTest.java index 70a178c35784f..70918ab88633f 100644 --- a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/AbstractBoschSHCDeviceHandlerTest.java +++ b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/AbstractBoschSHCDeviceHandlerTest.java @@ -14,14 +14,20 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import org.openhab.binding.boschshc.internal.devices.bridge.dto.Device; @@ -42,11 +48,34 @@ public abstract class AbstractBoschSHCDeviceHandlerTest extends AbstractBoschSHCHandlerTest { + protected static final String TAG_LEGACY_LOCATION_PROPERTY = "LegacyLocationProperty"; + protected static final String TAG_LOCATION_PROPERTY = "LocationProperty"; + protected static final String DEFAULT_ROOM_ID = "hz_1"; + @Override protected void configureDevice(Device device) { super.configureDevice(device); device.id = getDeviceID(); + device.roomId = DEFAULT_ROOM_ID; + } + + @Override + protected void beforeHandlerInitialization(TestInfo testInfo) { + super.beforeHandlerInitialization(testInfo); + Set tags = testInfo.getTags(); + if (tags.contains(TAG_LEGACY_LOCATION_PROPERTY) || tags.contains(TAG_LOCATION_PROPERTY)) { + Map properties = new HashMap<>(); + when(getThing().getProperties()).thenReturn(properties); + + if (tags.contains(TAG_LEGACY_LOCATION_PROPERTY)) { + properties.put(BoschSHCBindingConstants.PROPERTY_LOCATION_LEGACY, "Living Room"); + } + + if (tags.contains(TAG_LOCATION_PROPERTY)) { + when(getBridgeHandler().resolveRoomId(DEFAULT_ROOM_ID)).thenReturn("Kitchen"); + } + } } @Override @@ -80,4 +109,44 @@ void initializeHandleExceptionDuringDeviceInfoRestCall(Exception exception) argThat(status -> status.getStatus().equals(ThingStatus.OFFLINE) && status.getStatusDetail().equals(ThingStatusDetail.CONFIGURATION_ERROR))); } + + @Tag(TAG_LEGACY_LOCATION_PROPERTY) + @Test + protected void deleteLegacyLocationProperty() { + verify(getThing()).setProperty(BoschSHCBindingConstants.PROPERTY_LOCATION_LEGACY, null); + verify(getCallback()).thingUpdated(getThing()); + } + + @Tag(TAG_LOCATION_PROPERTY) + @Test + protected void locationPropertyDidNotChange() { + verify(getThing()).setProperty(BoschSHCBindingConstants.PROPERTY_LOCATION, "Kitchen"); + verify(getCallback()).thingUpdated(getThing()); + + getThing().getProperties().put(BoschSHCBindingConstants.PROPERTY_LOCATION, "Kitchen"); + + // re-initialize + getFixture().initialize(); + + verify(getThing()).setProperty(BoschSHCBindingConstants.PROPERTY_LOCATION, "Kitchen"); + verify(getCallback()).thingUpdated(getThing()); + } + + @Tag(TAG_LOCATION_PROPERTY) + @Test + protected void locationPropertyDidChange() { + verify(getThing()).setProperty(BoschSHCBindingConstants.PROPERTY_LOCATION, "Kitchen"); + verify(getCallback()).thingUpdated(getThing()); + + getThing().getProperties().put(BoschSHCBindingConstants.PROPERTY_LOCATION, "Kitchen"); + + getDevice().roomId = "hz_2"; + when(getBridgeHandler().resolveRoomId("hz_2")).thenReturn("Dining Room"); + + // re-initialize + getFixture().initialize(); + + verify(getThing()).setProperty(BoschSHCBindingConstants.PROPERTY_LOCATION, "Dining Room"); + verify(getCallback(), times(2)).thingUpdated(getThing()); + } } diff --git a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/relay/RelayHandlerTest.java b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/relay/RelayHandlerTest.java index 8524a1042236c..92f88b7751081 100644 --- a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/relay/RelayHandlerTest.java +++ b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/relay/RelayHandlerTest.java @@ -12,8 +12,11 @@ */ package org.openhab.binding.boschshc.internal.devices.relay; +import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.hamcrest.collection.IsCollectionWithSize.hasSize; +import static org.hamcrest.collection.IsMapContaining.hasKey; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; @@ -376,4 +379,55 @@ void testUpdateModePropertyIfApplicableImpulseSwitchMode() { verify(getCallback(), times(2)).thingUpdated(argThat(t -> ImpulseSwitchService.IMPULSE_SWITCH_SERVICE_NAME .equals(t.getProperties().get(RelayHandler.PROPERTY_MODE)))); } + + /** + * This has to be tested differently for the RelayHandler because the thing mock + * will be replaced by a real thing during the first initialization, which + * modifies the channels. + */ + @Test + @Tag(TAG_LEGACY_LOCATION_PROPERTY) + @Override + protected void deleteLegacyLocationProperty() { + ArgumentCaptor thingCaptor = ArgumentCaptor.forClass(Thing.class); + verify(getCallback(), times(3)).thingUpdated(thingCaptor.capture()); + List allValues = thingCaptor.getAllValues(); + assertThat(allValues, hasSize(3)); + assertThat(allValues.get(2).getProperties(), not(hasKey(BoschSHCBindingConstants.PROPERTY_LOCATION_LEGACY))); + } + + /** + * This has to be tested differently for the RelayHandler because the thing mock + * will be replaced by a real thing during the first initialization, which + * modifies the channels. + */ + @Test + @Tag(TAG_LOCATION_PROPERTY) + @Override + protected void locationPropertyDidNotChange() { + // re-initialize + getFixture().initialize(); + + verify(getCallback(), times(3)).thingUpdated( + argThat(t -> t.getProperties().get(BoschSHCBindingConstants.PROPERTY_LOCATION).equals("Kitchen"))); + } + + /** + * This has to be tested differently for the RelayHandler because the thing mock + * will be replaced by a real thing during the first initialization, which + * modifies the channels. + */ + @Test + @Tag(TAG_LOCATION_PROPERTY) + @Override + protected void locationPropertyDidChange() { + getDevice().roomId = "hz_2"; + when(getBridgeHandler().resolveRoomId("hz_2")).thenReturn("Dining Room"); + + // re-initialize + getFixture().initialize(); + + verify(getCallback(), times(4)).thingUpdated( + argThat(t -> t.getProperties().get(BoschSHCBindingConstants.PROPERTY_LOCATION).equals("Dining Room"))); + } } diff --git a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryServiceTest.java b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryServiceTest.java index cc5f5d58b4e41..2ec461306f5e7 100644 --- a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryServiceTest.java +++ b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryServiceTest.java @@ -194,7 +194,8 @@ void testAddDevice() { assertThat(result.getThingUID().getId(), is("testDevice_ID")); assertThat(result.getBridgeUID().getId(), is("testSHC")); assertThat(result.getLabel(), is("Test Name")); - assertThat(String.valueOf(result.getProperties().get("Location")), is("TestRoom")); + assertThat(String.valueOf(result.getProperties().get(BoschSHCBindingConstants.PROPERTY_LOCATION)), + is("TestRoom")); } @Test