Skip to content

Commit

Permalink
Addressed some of the last comments (#1)
Browse files Browse the repository at this point in the history 
* Removed plugName parameter

It should not be a required parameter as it's not used to identify.
So removed it as it's also already used to set the name.

Signed-off-by: Hilbrand Bouwkamp <[email protected]>

* Simplified discovery code

Changed to use serial number for smart plug thing id as is done with the other devices.
Also some clean up of logging that were incorrect.

Signed-off-by: Hilbrand Bouwkamp <[email protected]>

* Ran spotless to conform to styling

Signed-off-by: Hilbrand Bouwkamp <[email protected]>

* Changed log message

Signed-off-by: Hilbrand Bouwkamp <[email protected]>

* Added future to cancel scheduled command

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
  • Loading branch information
@Hilbrand @andrew-schofield
Hilbrand authored and andrew-schofield committed Jul 28, 2020
1 parent f8e278c commit 4ea7361
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 87 deletions.
2 changes: 1 addition & 1 deletion ...tonwiser/src/main/java/org/openhab/binding/draytonwiser/internal/api/DraytonWiserApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ public void setSmartPlugAwayAction(final int id, final boolean awayAction) {
logger.debug("Interrupted: {}", e.getMessage(), e);
Thread.currentThread().interrupt();
} catch (final ExecutionException e) {
logger.debug("Execution interrupted: {}", e.getMessage(), e);
logger.debug("Execution Exception: {}", e.getMessage(), e);
throw new DraytonWiserApiException(e.getMessage(), e);
} catch (final RuntimeException e) {
logger.debug("Unexpected error: {}", e.getMessage(), e);
Expand Down
75 changes: 28 additions & 47 deletions ...ava/org/openhab/binding/draytonwiser/internal/discovery/DraytonWiserDiscoveryService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,37 +104,20 @@ private void onControllerAdded(final DraytonWiserDTO domainDTOProxy) {
}

private void onHotWaterAdded(final DraytonWiserDTO domainDTOProxy, final HotWaterDTO hotWater) {
final DeviceDTO device = domainDTOProxy.getExtendedDeviceProperties(hotWater.getId());
final Integer hotWaterId = hotWater.getId();
final DeviceDTO device = domainDTOProxy.getExtendedDeviceProperties(hotWaterId);

if (device != null) {
logger.debug("Hot Water discovered, serialnumber: {}", device.getSerialNumber());
final RoomDTO assignedRoom = domainDTOProxy.getRoomForDeviceId(hotWater.getId());
final String assignedRoomName = assignedRoom == null ? "" : assignedRoom.getName();
final DiscoveryResult discoveryResult = DiscoveryResultBuilder
.create(new ThingUID(THING_TYPE_HOTWATER, bridgeUID, "hotwater")).withBridge(bridgeUID)
.withLabel(assignedRoomName + " - Hot Water").withRepresentationProperty(device.getSerialNumber())
.build();

thingDiscovered(discoveryResult);
onThingWithSerialNumber("Hot Water", device, getRoomName(domainDTOProxy, hotWaterId));
}
}

private void onRoomStatAdded(final DraytonWiserDTO domainDTOProxy, final RoomStatDTO roomStat) {
final DeviceDTO device = domainDTOProxy.getExtendedDeviceProperties(roomStat.getId());
final Integer roomStatId = roomStat.getId();
final DeviceDTO device = domainDTOProxy.getExtendedDeviceProperties(roomStatId);

if (device != null) {
logger.debug("Smart Value discovered, serialnumber: {}", device.getSerialNumber());
final Map<String, Object> properties = new HashMap<>();

DraytonWiserPropertyHelper.setRoomStatProperties(device, properties);
final RoomDTO assignedRoom = domainDTOProxy.getRoomForDeviceId(roomStat.getId());
final String assignedRoomName = assignedRoom == null ? "" : assignedRoom.getName();
final DiscoveryResult discoveryResult = DiscoveryResultBuilder
.create(new ThingUID(THING_TYPE_ROOMSTAT, bridgeUID, device.getSerialNumber()))
.withProperties(properties).withBridge(bridgeUID).withLabel(assignedRoomName + " - Thermostat")
.withRepresentationProperty(device.getSerialNumber()).build();

thingDiscovered(discoveryResult);
onThingWithSerialNumber("Thermostat", device, getRoomName(domainDTOProxy, roomStatId));
}
}

Expand All @@ -152,41 +135,39 @@ private void onRoomAdded(final RoomDTO room) {
}

private void onSmartValveAdded(final DraytonWiserDTO domainDTOProxy, final SmartValveDTO smartValve) {
final DeviceDTO device = domainDTOProxy.getExtendedDeviceProperties(smartValve.getId());
final Integer smartValueId = smartValve.getId();
final DeviceDTO device = domainDTOProxy.getExtendedDeviceProperties(smartValueId);

if (device != null) {
logger.debug("Smart Value discovered, serialnumber: {}", device.getSerialNumber());
final Map<String, Object> properties = new HashMap<>();

DraytonWiserPropertyHelper.setSmartValveProperties(device, properties);
final RoomDTO assignedRoom = domainDTOProxy.getRoomForDeviceId(smartValve.getId());
final String assignedRoomName = assignedRoom == null ? "" : assignedRoom.getName();

final DiscoveryResult discoveryResult = DiscoveryResultBuilder
.create(new ThingUID(THING_TYPE_ITRV, bridgeUID, device.getSerialNumber()))
.withProperties(properties).withBridge(bridgeUID).withLabel(assignedRoomName + " - TRV")
.withRepresentationProperty(device.getSerialNumber()).build();

thingDiscovered(discoveryResult);
onThingWithSerialNumber("TRV", device, getRoomName(domainDTOProxy, smartValueId));
}
}

private void onSmartPlugAdded(final DraytonWiserDTO domainDTOProxy, final SmartPlugDTO smartPlug) {
final DeviceDTO device = domainDTOProxy.getExtendedDeviceProperties(smartPlug.getId());

if (device != null) {
logger.debug("Smart Value discovered, serialnumber: {}", device.getSerialNumber());
final Map<String, Object> properties = new HashMap<>();
onThingWithSerialNumber("Smart Plug", device, smartPlug.getName());
}
}

DraytonWiserPropertyHelper.setSmartPlugProperties(device, properties, smartPlug.getName());
final DiscoveryResult discoveryResult = DiscoveryResultBuilder
.create(new ThingUID(THING_TYPE_SMARTPLUG, bridgeUID,
smartPlug.getName().replaceAll("[^A-Za-z0-9]", "").toLowerCase()))
.withProperties(properties).withBridge(bridgeUID).withLabel(smartPlug.getName() + " - Smart Plug")
.withRepresentationProperty(device.getSerialNumber()).build();
private String getRoomName(final DraytonWiserDTO domainDTOProxy, final Integer roomId) {
final RoomDTO assignedRoom = domainDTOProxy.getRoomForDeviceId(roomId);
return assignedRoom == null ? "" : assignedRoom.getName();
}

thingDiscovered(discoveryResult);
}
private void onThingWithSerialNumber(final String deviceType, final DeviceDTO device, final String name) {
final String serialNumber = device.getSerialNumber();
logger.debug("{} discovered, serialnumber: {}", deviceType, serialNumber);
final Map<String, Object> properties = new HashMap<>();

DraytonWiserPropertyHelper.setPropertiesWithSerialNumber(device, properties);
final DiscoveryResult discoveryResult = DiscoveryResultBuilder
.create(new ThingUID(THING_TYPE_SMARTPLUG, bridgeUID, serialNumber)).withProperties(properties)
.withBridge(bridgeUID).withLabel((name.isEmpty() ? "" : (name + " - ")) + deviceType)
.withRepresentationProperty(serialNumber).build();

thingDiscovered(discoveryResult);
}

@Override
Expand Down
20 changes: 2 additions & 18 deletions ...in/java/org/openhab/binding/draytonwiser/internal/handler/DraytonWiserPropertyHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,9 @@ public static void setControllerProperties(final DeviceDTO device, final Map<Str
setGeneralDeviceProperties(device, properties);
}

public static void setRoomStatProperties(final DeviceDTO device, final Map<String, Object> properties) {
setSerialNumber(device, properties);
setGeneralDeviceProperties(device, properties);
}

public static void setSmartValveProperties(final DeviceDTO device, final Map<String, Object> properties) {
setSerialNumber(device, properties);
setGeneralDeviceProperties(device, properties);
}

public static void setSmartPlugProperties(final DeviceDTO device, final Map<String, Object> properties,
final String name) {
setSerialNumber(device, properties);
setGeneralDeviceProperties(device, properties);
properties.put("plugName", name);
}

private static void setSerialNumber(final DeviceDTO device, final Map<String, Object> properties) {
public static void setPropertiesWithSerialNumber(final DeviceDTO device, final Map<String, Object> properties) {
properties.put("serialNumber", device.getSerialNumber());
setGeneralDeviceProperties(device, properties);
}

public static <T> void setGeneralDeviceProperties(final DeviceDTO device, final Map<String, T> properties) {
Expand Down
30 changes: 25 additions & 5 deletions ...main/java/org/openhab/binding/draytonwiser/internal/handler/DraytonWiserThingHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
*/
package org.openhab.binding.draytonwiser.internal.handler;

import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;

Expand All @@ -23,7 +24,6 @@
import org.eclipse.smarthome.core.thing.ThingStatus;
import org.eclipse.smarthome.core.thing.ThingStatusDetail;
import org.eclipse.smarthome.core.thing.ThingStatusInfo;
import org.eclipse.smarthome.core.thing.ThingUID;
import org.eclipse.smarthome.core.thing.binding.BaseThingHandler;
import org.eclipse.smarthome.core.thing.util.ThingHandlerHelper;
import org.eclipse.smarthome.core.types.Command;
Expand Down Expand Up @@ -51,6 +51,7 @@ abstract class DraytonWiserThingHandler<T> extends BaseThingHandler implements D
private @Nullable DraytonWiserApi api;
private @Nullable T data;
private @Nullable DraytonWiserDTO draytonWiseDTO;
private @Nullable ScheduledFuture<?> handleCommandRefreshFuture;

protected DraytonWiserThingHandler(final Thing thing) {
super(thing);
Expand All @@ -70,24 +71,43 @@ public void initialize() {

@Override
public final void handleCommand(final ChannelUID channelUID, final Command command) {
final HeatHubHandler heatHubHandler = getHeatHubHandler();

if (heatHubHandler == null) {
return; // if null status will be updated to offline
}
if (command instanceof RefreshType) {
getHeatHubHandler().refresh();
heatHubHandler.refresh();
} else {
final DraytonWiserApi api = this.api;

if (api != null && data != null) {
handleCommand(channelUID.getId(), command);
// cancel previous refresh, but wait for it to finish, so no forced cancel
disposehandleCommandRefreshFuture(false);
// update the state after the heathub has had time to react
scheduler.schedule(() -> getHeatHubHandler().refresh(), 5, TimeUnit.SECONDS);
handleCommandRefreshFuture = scheduler.schedule(heatHubHandler::refresh, 5, TimeUnit.SECONDS);
}
}
}

private void disposehandleCommandRefreshFuture(final boolean force) {
if (handleCommandRefreshFuture != null) {
handleCommandRefreshFuture.cancel(force);
handleCommandRefreshFuture = null;
}
}

@Override
public final void dispose() {
disposehandleCommandRefreshFuture(true);
}

/**
* Performs the actual command. This method is only called when api and device cache are not null.
*
* @param channelId
* @param command
* @param channelId Channel id part of the Channel UID
* @param command the command to perform
*/
protected abstract void handleCommand(String channelId, Command command);

Expand Down
6 changes: 3 additions & 3 deletions ...wiser/src/main/java/org/openhab/binding/draytonwiser/internal/handler/HeatHubHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ private void notifyListeners(final DraytonWiserDTO domain) {
if (discoveryService != null) {
discoveryService.onRefresh(domain);
}
getThing().getThings().stream().map(Thing::getHandler).filter(handler -> handler instanceof DraytonWiserRefreshListener)
.map(DraytonWiserRefreshListener.class::cast)
.forEach(listener -> listener.onRefresh(domain));
getThing().getThings().stream().map(Thing::getHandler)
.filter(handler -> handler instanceof DraytonWiserRefreshListener)
.map(DraytonWiserRefreshListener.class::cast).forEach(listener -> listener.onRefresh(domain));
}
}
10 changes: 3 additions & 7 deletions ...iser/src/main/java/org/openhab/binding/draytonwiser/internal/handler/HotWaterHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.core.library.types.DecimalType;
import org.eclipse.smarthome.core.library.types.OnOffType;
import org.eclipse.smarthome.core.thing.ChannelUID;
import org.eclipse.smarthome.core.thing.Thing;
import org.eclipse.smarthome.core.types.Command;
import org.eclipse.smarthome.core.types.State;
Expand Down Expand Up @@ -78,20 +77,17 @@ private State getHotWaterOverride() {

private State getHotWaterDemandState() {
final List<HotWaterDTO> hotWater = getData().hotWater;
return OnOffType.from(hotWater.size() >= 1
&& "ON".equalsIgnoreCase(hotWater.get(0).getHotWaterRelayState()));
return OnOffType.from(hotWater.size() >= 1 && "ON".equalsIgnoreCase(hotWater.get(0).getHotWaterRelayState()));
}

private State getManualModeState() {
final List<HotWaterDTO> hotWater = getData().hotWater;
return OnOffType
.from(hotWater.size() >= 1 && "MANUAL".equalsIgnoreCase(hotWater.get(0).getMode()));
return OnOffType.from(hotWater.size() >= 1 && "MANUAL".equalsIgnoreCase(hotWater.get(0).getMode()));
}

private State getSetPointState() {
final List<HotWaterDTO> hotWater = getData().hotWater;
return OnOffType.from(hotWater.size() >= 1
&& "ON".equalsIgnoreCase(hotWater.get(0).getWaterHeatingState()));
return OnOffType.from(hotWater.size() >= 1 && "ON".equalsIgnoreCase(hotWater.get(0).getWaterHeatingState()));
}

private void setManualMode(final boolean manualMode) {
Expand Down
3 changes: 1 addition & 2 deletions ...tonwiser/src/main/java/org/openhab/binding/draytonwiser/internal/handler/RoomHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ private State getBoostedState() {
private State getBoostRemainingState() {
final Integer overrideTimeout = getData().getOverrideTimeoutUnixTime();
if (overrideTimeout != null && !"NONE".equalsIgnoreCase(getData().getOverrideType())) {
return new DecimalType(
(overrideTimeout - (System.currentTimeMillis() / 1000L)) / 60);
return new DecimalType((overrideTimeout - (System.currentTimeMillis() / 1000L)) / 60);
}
return DecimalType.ZERO;
}
Expand Down
4 changes: 0 additions & 4 deletions bundles/org.openhab.binding.draytonwiser/src/main/resources/ESH-INF/thing/thing-types.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,6 @@
</channels>

<config-description>
<parameter name="plugName" type="text" required="true">
<label>Smart Plug Name</label>
<description>The plug name as it appears in the Wiser app</description>
</parameter>
<parameter name="serialNumber" type="text" required="true">
<label>Serial Number</label>
<description>Device Serial Number</description>
Expand Down

0 comments on commit 4ea7361

Please sign in to comment.