Skip to content

Commit 5871ead

Browse files
david-pacedavid-pace
authored
[boschshc] Update location properties when initializing things (openhab#17893)
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. Signed-off-by: David Pace <[email protected]>
1 parent 4ce97d5 commit 5871ead

File tree

8 files changed

+230
-3
lines changed

8 files changed

+230
-3
lines changed

‎bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/BoschSHCBindingConstants.java

+4
Original file line numberDiff line numberDiff line change
@@ -134,4 +134,8 @@ private BoschSHCBindingConstants() {
134134

135135
// static device/service names
136136
public static final String SERVICE_INTRUSION_DETECTION = "intrusionDetectionSystem";
137+
138+
// thing properties
139+
public static final String PROPERTY_LOCATION_LEGACY = "Location";
140+
public static final String PROPERTY_LOCATION = "location";
137141
}

‎bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/BoschSHCDeviceHandler.java

+37
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
*/
1313
package org.openhab.binding.boschshc.internal.devices;
1414

15+
import static org.openhab.binding.boschshc.internal.devices.BoschSHCBindingConstants.PROPERTY_LOCATION;
16+
import static org.openhab.binding.boschshc.internal.devices.BoschSHCBindingConstants.PROPERTY_LOCATION_LEGACY;
17+
18+
import java.util.Map;
1519
import java.util.concurrent.ExecutionException;
1620
import java.util.concurrent.TimeoutException;
1721

@@ -83,9 +87,42 @@ public void initialize() {
8387
* otherwise
8488
*/
8589
protected boolean processDeviceInfo(Device deviceInfo) {
90+
try {
91+
updateLocationPropertiesIfApplicable(deviceInfo);
92+
} catch (BoschSHCException e) {
93+
logger.warn("Error while updating location properties for thing {}.", getThing().getUID(), e);
94+
}
95+
// do not cancel thing initialization if location properties cannot be updated
8696
return true;
8797
}
8898

99+
private void updateLocationPropertiesIfApplicable(Device deviceInfo) throws BoschSHCException {
100+
Map<String, String> thingProperties = getThing().getProperties();
101+
removeLegacyLocationPropertyIfApplicable(thingProperties);
102+
updateLocationPropertyIfApplicable(thingProperties, deviceInfo);
103+
}
104+
105+
private void updateLocationPropertyIfApplicable(Map<String, String> thingProperties, Device deviceInfo)
106+
throws BoschSHCException {
107+
String roomName = getBridgeHandler().resolveRoomId(deviceInfo.roomId);
108+
if (roomName != null) {
109+
String currentLocation = thingProperties.get(PROPERTY_LOCATION);
110+
if (!roomName.equals(currentLocation)) {
111+
logger.debug("Updating property '{}' of thing {} to '{}'.", PROPERTY_LOCATION, getThing().getUID(),
112+
roomName);
113+
updateProperty(PROPERTY_LOCATION, roomName);
114+
}
115+
}
116+
}
117+
118+
private void removeLegacyLocationPropertyIfApplicable(Map<String, String> thingProperties) {
119+
if (thingProperties.containsKey(PROPERTY_LOCATION_LEGACY)) {
120+
logger.debug("Removing legacy property '{}' from thing {}.", PROPERTY_LOCATION_LEGACY, getThing().getUID());
121+
// null value indicates that the property should be removed
122+
updateProperty(PROPERTY_LOCATION_LEGACY, null);
123+
}
124+
}
125+
89126
/**
90127
* Attempts to obtain information about the device with the specified ID via a REST call.
91128
* <p>

‎bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/BridgeHandler.java

+32-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static org.eclipse.jetty.http.HttpMethod.PUT;
1818

1919
import java.lang.reflect.Type;
20+
import java.time.Duration;
2021
import java.util.ArrayList;
2122
import java.util.Collection;
2223
import java.util.Collections;
@@ -54,6 +55,7 @@
5455
import org.openhab.binding.boschshc.internal.serialization.GsonUtils;
5556
import org.openhab.binding.boschshc.internal.services.dto.BoschSHCServiceState;
5657
import org.openhab.binding.boschshc.internal.services.dto.JsonRestExceptionResponse;
58+
import org.openhab.core.cache.ExpiringCache;
5759
import org.openhab.core.library.types.StringType;
5860
import org.openhab.core.thing.Bridge;
5961
import org.openhab.core.thing.Channel;
@@ -88,6 +90,8 @@ public class BridgeHandler extends BaseBridgeHandler {
8890

8991
private static final String HTTP_CLIENT_NOT_INITIALIZED = "HttpClient not initialized";
9092

93+
private static final Duration ROOM_CACHE_DURATION = Duration.ofMinutes(2);
94+
9195
private final Logger logger = LoggerFactory.getLogger(BridgeHandler.class);
9296

9397
/**
@@ -107,13 +111,22 @@ public class BridgeHandler extends BaseBridgeHandler {
107111

108112
/**
109113
* SHC thing/device discovery service instance.
110-
* Registered and unregistered if service is actived/deactived.
114+
* Registered and unregistered if service is activated/deactivated.
111115
* Used to scan for things after bridge is paired with SHC.
112116
*/
113117
private @Nullable ThingDiscoveryService thingDiscoveryService;
114118

115119
private final ScenarioHandler scenarioHandler;
116120

121+
private ExpiringCache<List<Room>> roomCache = new ExpiringCache<>(ROOM_CACHE_DURATION, () -> {
122+
try {
123+
return getRooms();
124+
} catch (InterruptedException e) {
125+
Thread.currentThread().interrupt();
126+
return null;
127+
}
128+
});
129+
117130
public BridgeHandler(Bridge bridge) {
118131
super(bridge);
119132
scenarioHandler = new ScenarioHandler();
@@ -437,6 +450,24 @@ public List<Room> getRooms() throws InterruptedException {
437450
}
438451
}
439452

453+
public @Nullable List<Room> getRoomsWithCache() {
454+
return roomCache.getValue();
455+
}
456+
457+
public @Nullable String resolveRoomId(@Nullable String roomId) {
458+
if (roomId == null) {
459+
return null;
460+
}
461+
462+
@Nullable
463+
List<Room> rooms = getRoomsWithCache();
464+
if (rooms != null) {
465+
return rooms.stream().filter(r -> r.id.equals(roomId)).map(r -> r.name).findAny().orElse(null);
466+
}
467+
468+
return null;
469+
}
470+
440471
/**
441472
* Get public information from Bosch SHC.
442473
*/

‎bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ protected void addDevice(Device device, String roomName) {
243243
discoveryResult.withBridge(thingHandler.getThing().getUID());
244244

245245
if (!roomName.isEmpty()) {
246-
discoveryResult.withProperty("Location", roomName);
246+
discoveryResult.withProperty(BoschSHCBindingConstants.PROPERTY_LOCATION, roomName);
247247
}
248248
thingDiscovered(discoveryResult.build());
249249

‎bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/AbstractBoschSHCDeviceHandlerTest.java

+69
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,20 @@
1414

1515
import static org.mockito.ArgumentMatchers.any;
1616
import static org.mockito.ArgumentMatchers.argThat;
17+
import static org.mockito.Mockito.times;
1718
import static org.mockito.Mockito.verify;
1819
import static org.mockito.Mockito.when;
1920

21+
import java.util.HashMap;
22+
import java.util.Map;
23+
import java.util.Set;
2024
import java.util.concurrent.ExecutionException;
2125
import java.util.concurrent.TimeoutException;
2226

2327
import org.eclipse.jdt.annotation.NonNullByDefault;
28+
import org.junit.jupiter.api.Tag;
2429
import org.junit.jupiter.api.Test;
30+
import org.junit.jupiter.api.TestInfo;
2531
import org.junit.jupiter.params.ParameterizedTest;
2632
import org.junit.jupiter.params.provider.MethodSource;
2733
import org.openhab.binding.boschshc.internal.devices.bridge.dto.Device;
@@ -42,11 +48,34 @@
4248
public abstract class AbstractBoschSHCDeviceHandlerTest<T extends BoschSHCDeviceHandler>
4349
extends AbstractBoschSHCHandlerTest<T> {
4450

51+
protected static final String TAG_LEGACY_LOCATION_PROPERTY = "LegacyLocationProperty";
52+
protected static final String TAG_LOCATION_PROPERTY = "LocationProperty";
53+
protected static final String DEFAULT_ROOM_ID = "hz_1";
54+
4555
@Override
4656
protected void configureDevice(Device device) {
4757
super.configureDevice(device);
4858

4959
device.id = getDeviceID();
60+
device.roomId = DEFAULT_ROOM_ID;
61+
}
62+
63+
@Override
64+
protected void beforeHandlerInitialization(TestInfo testInfo) {
65+
super.beforeHandlerInitialization(testInfo);
66+
Set<String> tags = testInfo.getTags();
67+
if (tags.contains(TAG_LEGACY_LOCATION_PROPERTY) || tags.contains(TAG_LOCATION_PROPERTY)) {
68+
Map<String, String> properties = new HashMap<>();
69+
when(getThing().getProperties()).thenReturn(properties);
70+
71+
if (tags.contains(TAG_LEGACY_LOCATION_PROPERTY)) {
72+
properties.put(BoschSHCBindingConstants.PROPERTY_LOCATION_LEGACY, "Living Room");
73+
}
74+
75+
if (tags.contains(TAG_LOCATION_PROPERTY)) {
76+
when(getBridgeHandler().resolveRoomId(DEFAULT_ROOM_ID)).thenReturn("Kitchen");
77+
}
78+
}
5079
}
5180

5281
@Override
@@ -80,4 +109,44 @@ void initializeHandleExceptionDuringDeviceInfoRestCall(Exception exception)
80109
argThat(status -> status.getStatus().equals(ThingStatus.OFFLINE)
81110
&& status.getStatusDetail().equals(ThingStatusDetail.CONFIGURATION_ERROR)));
82111
}
112+
113+
@Tag(TAG_LEGACY_LOCATION_PROPERTY)
114+
@Test
115+
protected void deleteLegacyLocationProperty() {
116+
verify(getThing()).setProperty(BoschSHCBindingConstants.PROPERTY_LOCATION_LEGACY, null);
117+
verify(getCallback()).thingUpdated(getThing());
118+
}
119+
120+
@Tag(TAG_LOCATION_PROPERTY)
121+
@Test
122+
protected void locationPropertyDidNotChange() {
123+
verify(getThing()).setProperty(BoschSHCBindingConstants.PROPERTY_LOCATION, "Kitchen");
124+
verify(getCallback()).thingUpdated(getThing());
125+
126+
getThing().getProperties().put(BoschSHCBindingConstants.PROPERTY_LOCATION, "Kitchen");
127+
128+
// re-initialize
129+
getFixture().initialize();
130+
131+
verify(getThing()).setProperty(BoschSHCBindingConstants.PROPERTY_LOCATION, "Kitchen");
132+
verify(getCallback()).thingUpdated(getThing());
133+
}
134+
135+
@Tag(TAG_LOCATION_PROPERTY)
136+
@Test
137+
protected void locationPropertyDidChange() {
138+
verify(getThing()).setProperty(BoschSHCBindingConstants.PROPERTY_LOCATION, "Kitchen");
139+
verify(getCallback()).thingUpdated(getThing());
140+
141+
getThing().getProperties().put(BoschSHCBindingConstants.PROPERTY_LOCATION, "Kitchen");
142+
143+
getDevice().roomId = "hz_2";
144+
when(getBridgeHandler().resolveRoomId("hz_2")).thenReturn("Dining Room");
145+
146+
// re-initialize
147+
getFixture().initialize();
148+
149+
verify(getThing()).setProperty(BoschSHCBindingConstants.PROPERTY_LOCATION, "Dining Room");
150+
verify(getCallback(), times(2)).thingUpdated(getThing());
151+
}
83152
}

‎bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/BridgeHandlerTest.java

+31
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
*/
1313
package org.openhab.binding.boschshc.internal.devices.bridge;
1414

15+
import static org.hamcrest.CoreMatchers.is;
16+
import static org.hamcrest.CoreMatchers.nullValue;
1517
import static org.hamcrest.MatcherAssert.assertThat;
1618
import static org.hamcrest.collection.IsCollectionWithSize.hasSize;
1719
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -1126,4 +1128,33 @@ void getPublicInformation() throws InterruptedException, BoschSHCException, Exec
11261128
verify(httpClient).createRequest(any(), same(HttpMethod.GET));
11271129
verify(httpClient).sendRequest(any(), same(PublicInformation.class), any(), isNull());
11281130
}
1131+
1132+
@Test
1133+
void resolveRoomId() throws InterruptedException, TimeoutException, ExecutionException {
1134+
Request request = mock(Request.class);
1135+
when(httpClient.createRequest(any(), eq(HttpMethod.GET))).thenReturn(request);
1136+
ContentResponse contentResponse = mock(ContentResponse.class);
1137+
when(request.send()).thenReturn(contentResponse);
1138+
when(contentResponse.getStatus()).thenReturn(200);
1139+
String roomsJson = """
1140+
[
1141+
{
1142+
"@type": "room",
1143+
"id": "hz_1",
1144+
"iconId": "icon_room_living_room",
1145+
"name": "Living Room"
1146+
},
1147+
{
1148+
"@type": "room",
1149+
"id": "hz_2",
1150+
"iconId": "icon_room_dining_room",
1151+
"name": "Dining Room"
1152+
}
1153+
]
1154+
""";
1155+
when(contentResponse.getContentAsString()).thenReturn(roomsJson);
1156+
assertThat(fixture.resolveRoomId("hz_1"), is("Living Room"));
1157+
assertThat(fixture.resolveRoomId("hz_2"), is("Dining Room"));
1158+
assertThat(fixture.resolveRoomId(null), is(nullValue()));
1159+
}
11291160
}

‎bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/relay/RelayHandlerTest.java

+54
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@
1212
*/
1313
package org.openhab.binding.boschshc.internal.devices.relay;
1414

15+
import static org.hamcrest.CoreMatchers.not;
1516
import static org.hamcrest.MatcherAssert.assertThat;
1617
import static org.hamcrest.Matchers.is;
18+
import static org.hamcrest.collection.IsCollectionWithSize.hasSize;
19+
import static org.hamcrest.collection.IsMapContaining.hasKey;
1720
import static org.mockito.ArgumentMatchers.any;
1821
import static org.mockito.ArgumentMatchers.argThat;
1922
import static org.mockito.ArgumentMatchers.eq;
@@ -376,4 +379,55 @@ void testUpdateModePropertyIfApplicableImpulseSwitchMode() {
376379
verify(getCallback(), times(2)).thingUpdated(argThat(t -> ImpulseSwitchService.IMPULSE_SWITCH_SERVICE_NAME
377380
.equals(t.getProperties().get(RelayHandler.PROPERTY_MODE))));
378381
}
382+
383+
/**
384+
* This has to be tested differently for the RelayHandler because the thing mock
385+
* will be replaced by a real thing during the first initialization, which
386+
* modifies the channels.
387+
*/
388+
@Test
389+
@Tag(TAG_LEGACY_LOCATION_PROPERTY)
390+
@Override
391+
protected void deleteLegacyLocationProperty() {
392+
ArgumentCaptor<Thing> thingCaptor = ArgumentCaptor.forClass(Thing.class);
393+
verify(getCallback(), times(3)).thingUpdated(thingCaptor.capture());
394+
List<Thing> allValues = thingCaptor.getAllValues();
395+
assertThat(allValues, hasSize(3));
396+
assertThat(allValues.get(2).getProperties(), not(hasKey(BoschSHCBindingConstants.PROPERTY_LOCATION_LEGACY)));
397+
}
398+
399+
/**
400+
* This has to be tested differently for the RelayHandler because the thing mock
401+
* will be replaced by a real thing during the first initialization, which
402+
* modifies the channels.
403+
*/
404+
@Test
405+
@Tag(TAG_LOCATION_PROPERTY)
406+
@Override
407+
protected void locationPropertyDidNotChange() {
408+
// re-initialize
409+
getFixture().initialize();
410+
411+
verify(getCallback(), times(3)).thingUpdated(
412+
argThat(t -> t.getProperties().get(BoschSHCBindingConstants.PROPERTY_LOCATION).equals("Kitchen")));
413+
}
414+
415+
/**
416+
* This has to be tested differently for the RelayHandler because the thing mock
417+
* will be replaced by a real thing during the first initialization, which
418+
* modifies the channels.
419+
*/
420+
@Test
421+
@Tag(TAG_LOCATION_PROPERTY)
422+
@Override
423+
protected void locationPropertyDidChange() {
424+
getDevice().roomId = "hz_2";
425+
when(getBridgeHandler().resolveRoomId("hz_2")).thenReturn("Dining Room");
426+
427+
// re-initialize
428+
getFixture().initialize();
429+
430+
verify(getCallback(), times(4)).thingUpdated(
431+
argThat(t -> t.getProperties().get(BoschSHCBindingConstants.PROPERTY_LOCATION).equals("Dining Room")));
432+
}
379433
}

‎bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryServiceTest.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,8 @@ void testAddDevice() {
194194
assertThat(result.getThingUID().getId(), is("testDevice_ID"));
195195
assertThat(result.getBridgeUID().getId(), is("testSHC"));
196196
assertThat(result.getLabel(), is("Test Name"));
197-
assertThat(String.valueOf(result.getProperties().get("Location")), is("TestRoom"));
197+
assertThat(String.valueOf(result.getProperties().get(BoschSHCBindingConstants.PROPERTY_LOCATION)),
198+
is("TestRoom"));
198199
}
199200

200201
@Test

0 commit comments

Comments
 (0)