Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
  • Loading branch information
J-N-K committed Jun 25, 2020
1 parent fbc27d2 commit 9f58cb3
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 112 deletions.
7 changes: 4 additions & 3 deletions bundles/org.openhab.binding.amazonechocontrol/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ You will find the required serial number in settings of the device in the Alexa

### Discover Smart Home Devices

If you want to discover your smart home devices you need to activate it in the 'Amazon Account' thing. Devices from other skills can be discovered too. See section *Smart Home Devices* below for more information.
If you want to discover your smart home devices you need to activate it in the 'Amazon Account' thing.
Devices from other skills can be discovered too.
See section *Smart Home Devices* below for more information.

## Account

Expand All @@ -104,10 +106,9 @@ The configuration of your Amazon account must be done in the 'Amazon Account' de

| Configuration name | Default | Description |
|---------------------------------|---------|---------------------------------------------------------------------------------------|
| discoverSmartHome | 0 | 0...No discover, 1...Discover direct connected, 2...Discover direct and skill devices |
| discoverSmartHome | 0 | 0...No discover, 1...Discover direct connected, 2...Discover direct and Alexa skill devices, 3...Discover direct, Alexa and openHAB skill devices |
| pollingIntervalSmartHomeAlexa | 30 | Defines the time in seconds for openHAB to pull the state of the Alexa connected devices. The minimum is 10 seconds. |
| pollingIntervalSmartSkills | 120 | Defines the time in seconds for openHAB to pull the state of the over a skill connected devices. The minimum is 60 seconds. |
| discoverOpenHabSmartHomeDevices | false | Defines, if smart home devices of the openHAB skill should be discovered. This option is for development and testing purpose only. ||

#### Channels

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Copyright (c) 2010-2020 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.amazonechocontrol.internal;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.binding.amazonechocontrol.internal.handler.AccountHandler;

/**
* The {@link AccountHandlerConfig} holds the configuration for the {@link AccountHandler}
*
* @author Jan N. Klug - Initial contribution
*/
@NonNullByDefault
public class AccountHandlerConfig {
public int discoverSmartHome = 0;
public int pollingIntervalSmartHomeAlexa = 60;
public int pollingIntervalSmartSkills = 120;
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
*/
@NonNullByDefault
public class AmazonEchoControlBindingConstants {

public static final String BINDING_ID = "amazonechocontrol";
public static final String BINDING_NAME = "Amazon Echo Control";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,10 @@ synchronized void setSmartHomeDevices(List<SmartHomeBaseDevice> deviceList) {
if (discoveryServiceCallback == null) {
return;
}
int shouldDiscoverSmartHomeDevice = accountHandler.getSmartHomeDevicesDiscoveryMode();
if (shouldDiscoverSmartHomeDevice == 0) {
int smartHomeDeviceDiscoveryMode = accountHandler.getSmartHomeDevicesDiscoveryMode();
if (smartHomeDeviceDiscoveryMode == 0) {
return;
}
boolean shouldDiscoverOpenHabDevices = accountHandler.shouldDiscoverOpenHABSmartHomeDevices();

for (Object smartHomeDevice : deviceList) {
ThingUID bridgeThingUID = this.accountHandler.getThing().getUID();
Expand All @@ -177,11 +176,11 @@ synchronized void setSmartHomeDevices(List<SmartHomeBaseDevice> deviceList) {
DriverIdentity driverIdentity = shd.driverIdentity;
isSkillDevice = driverIdentity != null && "SKILL".equals(driverIdentity.namespace);

if (shouldDiscoverSmartHomeDevice == 1 && isSkillDevice) {
if (smartHomeDeviceDiscoveryMode == 1 && isSkillDevice) {
// Connected through skill
continue;
}
if (!shouldDiscoverOpenHabDevices && "openHAB".equalsIgnoreCase(shd.manufacturerName)) {
if (!(smartHomeDeviceDiscoveryMode == 2) && "openHAB".equalsIgnoreCase(shd.manufacturerName)) {
// OpenHAB device
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
package org.openhab.binding.amazonechocontrol.internal.handler;

import java.io.IOException;
import java.math.BigDecimal;
import java.net.URISyntaxException;
import java.net.URLEncoder;
import java.net.UnknownHostException;
Expand All @@ -33,7 +32,6 @@
import org.apache.commons.lang.StringUtils;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.config.core.Configuration;
import org.eclipse.smarthome.core.storage.Storage;
import org.eclipse.smarthome.core.thing.Bridge;
import org.eclipse.smarthome.core.thing.ChannelUID;
Expand All @@ -46,6 +44,7 @@
import org.eclipse.smarthome.core.types.Command;
import org.eclipse.smarthome.core.types.RefreshType;
import org.eclipse.smarthome.core.types.State;
import org.openhab.binding.amazonechocontrol.internal.AccountHandlerConfig;
import org.openhab.binding.amazonechocontrol.internal.AccountServlet;
import org.openhab.binding.amazonechocontrol.internal.Connection;
import org.openhab.binding.amazonechocontrol.internal.ConnectionException;
Expand Down Expand Up @@ -97,9 +96,9 @@ public class AccountHandler extends BaseBridgeHandler implements IWebSocketComma
private @Nullable Connection connection;
private @Nullable WebSocketConnection webSocketConnection;

private final Set<EchoHandler> echoHandlers = new HashSet<>();
private final Set<EchoHandler> echoHandlers = new CopyOnWriteArraySet<>();
private final Set<SmartHomeDeviceHandler> smartHomeDeviceHandlers = new CopyOnWriteArraySet<>();
private final Set<FlashBriefingProfileHandler> flashBriefingProfileHandlers = new HashSet<>();
private final Set<FlashBriefingProfileHandler> flashBriefingProfileHandlers = new CopyOnWriteArraySet<>();

private final Object synchronizeConnection = new Object();
private Map<String, Device> jsonSerialNumberDeviceMapping = new HashMap<>();
Expand All @@ -122,6 +121,8 @@ public class AccountHandler extends BaseBridgeHandler implements IWebSocketComma
private @Nullable SmartHomeDeviceStateGroupUpdateCalculator smartHomeDeviceStateGroupUpdateCalculator;
private List<ChannelHandler> channelHandlers = new ArrayList<>();

private AccountHandlerConfig handlerConfig = new AccountHandlerConfig();

public AccountHandler(Bridge bridge, HttpService httpService, Storage<String> stateStorage, Gson gson) {
super(bridge);
this.gson = gson;
Expand All @@ -132,7 +133,7 @@ public AccountHandler(Bridge bridge, HttpService httpService, Storage<String> st

@Override
public void initialize() {
logger.debug("amazon account bridge starting...");
handlerConfig = getConfig().as(AccountHandlerConfig.class);

synchronized (synchronizeConnection) {
Connection connection = this.connection;
Expand All @@ -141,9 +142,9 @@ public void initialize() {
}
}

if (this.accountServlet == null) {
if (accountServlet == null) {
try {
this.accountServlet = new AccountServlet(httpService, this.getThing().getUID().getId(), this, gson);
accountServlet = new AccountServlet(httpService, this.getThing().getUID().getId(), this, gson);
} catch (IllegalStateException e) {
logger.warn("Failed to create account servlet", e);
}
Expand All @@ -154,21 +155,18 @@ public void initialize() {
checkLoginJob = scheduler.scheduleWithFixedDelay(this::checkLogin, 0, 60, TimeUnit.SECONDS);
checkDataJob = scheduler.scheduleWithFixedDelay(this::checkData, 4, 60, TimeUnit.SECONDS);

Configuration config = getThing().getConfiguration();
int pollingIntervalAlexa = ((BigDecimal) config.getProperties().get("pollingIntervalSmartHomeAlexa"))
.intValue();
int pollingIntervalAlexa = handlerConfig.pollingIntervalSmartHomeAlexa;
if (pollingIntervalAlexa < 10) {
pollingIntervalAlexa = 10;
}
int pollingIntervalSkills = ((BigDecimal) config.getProperties().get("pollingIntervalSmartSkills")).intValue();
int pollingIntervalSkills = handlerConfig.pollingIntervalSmartSkills;
if (pollingIntervalSkills < 60) {
pollingIntervalSkills = 60;
}
smartHomeDeviceStateGroupUpdateCalculator = new SmartHomeDeviceStateGroupUpdateCalculator(pollingIntervalAlexa,
pollingIntervalSkills);
updateSmartHomeStateJob = scheduler.scheduleWithFixedDelay(() -> updateSmartHomeState(null), 20, 10,
TimeUnit.SECONDS);
logger.debug("amazon account bridge handler started.");
}

@Override
Expand Down Expand Up @@ -200,7 +198,7 @@ public void handleCommand(ChannelUID channelUID, Command command) {
}

public List<FlashBriefingProfileHandler> getFlashBriefingProfileHandlers() {
return new ArrayList<>(this.flashBriefingProfileHandlers);
return new ArrayList<>(flashBriefingProfileHandlers);
}

public List<Device> getLastKnownDevices() {
Expand All @@ -211,17 +209,11 @@ public List<SmartHomeBaseDevice> getLastKnownSmartHomeDevices() {
return new ArrayList<>(jsonIdSmartHomeDeviceMapping.values());
}

public List<SmartHomeDeviceHandler> getSmartHomeDeviceHandlers() {
return new ArrayList<>(this.smartHomeDeviceHandlers);
}

public void addEchoHandler(EchoHandler echoHandler) {
synchronized (echoHandlers) {
if (!echoHandlers.add(echoHandler)) {
return;
}
if (echoHandlers.add(echoHandler)) {

forceCheckData();
}
forceCheckData();
}

public void addSmartHomeDeviceHandler(SmartHomeDeviceHandler smartHomeDeviceHandler) {
Expand All @@ -232,14 +224,10 @@ public void addSmartHomeDeviceHandler(SmartHomeDeviceHandler smartHomeDeviceHand

public void forceCheckData() {
if (forceCheckDataJob == null) {
forceCheckDataJob = scheduler.schedule(this::forceCheckDataHandler, 1000, TimeUnit.MILLISECONDS);
forceCheckDataJob = scheduler.schedule(this::checkData, 1000, TimeUnit.MILLISECONDS);
}
}

void forceCheckDataHandler() {
this.checkData();
}

public @Nullable Thing findThingBySerialNumber(@Nullable String deviceSerialNumber) {
EchoHandler echoHandler = findEchoHandlerBySerialNumber(deviceSerialNumber);
if (echoHandler != null) {
Expand All @@ -249,20 +237,16 @@ void forceCheckDataHandler() {
}

public @Nullable EchoHandler findEchoHandlerBySerialNumber(@Nullable String deviceSerialNumber) {
synchronized (echoHandlers) {
for (EchoHandler echoHandler : echoHandlers) {
if (StringUtils.equals(echoHandler.findSerialNumber(), deviceSerialNumber)) {
return echoHandler;
}
for (EchoHandler echoHandler : echoHandlers) {
if (deviceSerialNumber != null && deviceSerialNumber.equals(echoHandler.findSerialNumber())) {
return echoHandler;
}
}
return null;
}

public void addFlashBriefingProfileHandler(FlashBriefingProfileHandler flashBriefingProfileHandler) {
synchronized (flashBriefingProfileHandlers) {
flashBriefingProfileHandlers.add(flashBriefingProfileHandler);
}
flashBriefingProfileHandlers.add(flashBriefingProfileHandler);
Connection connection = this.connection;
if (connection != null && connection.getIsLoggedIn()) {
if (currentFlashBriefingJson.isEmpty()) {
Expand Down Expand Up @@ -292,15 +276,11 @@ public void handleRemoval() {
public void childHandlerDisposed(ThingHandler childHandler, Thing childThing) {
// check for echo handler
if (childHandler instanceof EchoHandler) {
synchronized (echoHandlers) {
echoHandlers.remove(childHandler);
}
echoHandlers.remove(childHandler);
}
// check for flash briefing profile handler
if (childHandler instanceof FlashBriefingProfileHandler) {
synchronized (flashBriefingProfileHandlers) {
flashBriefingProfileHandlers.remove(childHandler);
}
flashBriefingProfileHandlers.remove(childHandler);
}
// check for flash briefing profile handler
if (childHandler instanceof SmartHomeDeviceHandler) {
Expand Down Expand Up @@ -495,9 +475,8 @@ private void refreshNotifications(@Nullable JsonCommandPayloadPushNotificationCh
}
ZonedDateTime timeStampNow = ZonedDateTime.now();

for (EchoHandler child : echoHandlers) {
child.updateNotifications(timeStamp, timeStampNow, pushPayload, notifications);
}
echoHandlers.forEach(
echoHandler -> echoHandler.updateNotifications(timeStamp, timeStampNow, pushPayload, notifications));
}

private void refreshData() {
Expand Down Expand Up @@ -549,7 +528,6 @@ private void refreshData() {
for (EchoHandler child : echoHandlers) {
Device device = findDeviceJson(child.findSerialNumber());

@Nullable
JsonNotificationSound[] notificationSounds = null;
JsonPlaylists playlists = null;
if (device != null && currentConnection.getIsLoggedIn()) {
Expand Down Expand Up @@ -666,21 +644,20 @@ public List<Device> updateDeviceList() {

WakeWord[] wakeWords = currentConnection.getWakeWords();
// update handlers
synchronized (echoHandlers) {
for (EchoHandler child : echoHandlers) {
String serialNumber = child.findSerialNumber();
String deviceWakeWord = null;
for (WakeWord wakeWord : wakeWords) {
if (wakeWord != null) {
if (StringUtils.equals(wakeWord.deviceSerialNumber, serialNumber)) {
deviceWakeWord = wakeWord.wakeWord;
break;
}
for (EchoHandler echoHandler : echoHandlers) {
String serialNumber = echoHandler.findSerialNumber();
String deviceWakeWord = null;
for (WakeWord wakeWord : wakeWords) {
if (wakeWord != null) {
if (serialNumber != null && serialNumber.equals(wakeWord.deviceSerialNumber)) {
deviceWakeWord = wakeWord.wakeWord;
break;
}
}
child.setDeviceAndUpdateThingState(this, findDeviceJson(serialNumber), deviceWakeWord);
}
echoHandler.setDeviceAndUpdateThingState(this, findDeviceJson(serialNumber), deviceWakeWord);
}

if (devices != null) {
return devices;
}
Expand Down Expand Up @@ -713,19 +690,17 @@ public String updateFlashBriefingHandlers() {
}

private String updateFlashBriefingHandlers(Connection currentConnection) {
synchronized (flashBriefingProfileHandlers) {
if (!flashBriefingProfileHandlers.isEmpty() || currentFlashBriefingJson.isEmpty()) {
updateFlashBriefingProfiles(currentConnection);
}
boolean flashBriefingProfileFound = false;
for (FlashBriefingProfileHandler child : flashBriefingProfileHandlers) {
flashBriefingProfileFound |= child.initialize(this, currentFlashBriefingJson);
}
if (flashBriefingProfileFound) {
return "";
}
return this.currentFlashBriefingJson;
if (!flashBriefingProfileHandlers.isEmpty() || currentFlashBriefingJson.isEmpty()) {
updateFlashBriefingProfiles(currentConnection);
}
boolean flashBriefingProfileFound = false;
for (FlashBriefingProfileHandler child : flashBriefingProfileHandlers) {
flashBriefingProfileFound |= child.initialize(this, currentFlashBriefingJson);
}
if (flashBriefingProfileFound) {
return "";
}
return this.currentFlashBriefingJson;
}

public @Nullable Connection findConnection() {
Expand Down Expand Up @@ -869,26 +844,7 @@ void refreshAfterCommand() {
}

public int getSmartHomeDevicesDiscoveryMode() {
Configuration config = getThing().getConfiguration();
Object discoverSmartHomeConfig = config.getProperties().get("discoverSmartHome");
int discoverSmartHome = 0;
if (discoverSmartHomeConfig instanceof Boolean) {
discoverSmartHome = ((boolean) discoverSmartHomeConfig) ? 2 : 0;
}
if (discoverSmartHomeConfig instanceof BigDecimal) {
discoverSmartHome = ((BigDecimal) discoverSmartHomeConfig).intValue();
}
return discoverSmartHome;
}

public boolean shouldDiscoverOpenHABSmartHomeDevices() {
Configuration config = getThing().getConfiguration();
Boolean discoverOpenHabSmartHomeDevices = (Boolean) config.getProperties()
.get("discoverOpenHabSmartHomeDevices");
if (discoverOpenHabSmartHomeDevices == null) {
return false;
}
return discoverOpenHabSmartHomeDevices;
return handlerConfig.discoverSmartHome;
}

public List<SmartHomeBaseDevice> updateSmartHomeDeviceList(boolean forceUpdate) {
Expand Down
Loading

0 comments on commit 9f58cb3

Please sign in to comment.