Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Hilbrand Bouwkamp <[email protected]>
  • Loading branch information
@Hilbrand
Hilbrand committed Nov 27, 2019
1 parent 836e1da commit 472ee3e
Show file tree
Hide file tree
Showing 17 changed files with 120 additions and 188 deletions.
94 changes: 24 additions & 70 deletions ...thome/src/main/java/org/openhab/binding/innogysmarthome/internal/client/InnogyClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.openhab.binding.innogysmarthome.internal.client.Constants.*;

import java.io.IOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
Expand All @@ -31,6 +32,7 @@
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.util.StringContentProvider;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.smarthome.core.auth.client.oauth2.AccessTokenResponse;
Expand Down Expand Up @@ -76,7 +78,6 @@
public class InnogyClient {
private static final String BEARER = "Bearer ";
private static final String CONTENT_TYPE = "application/json";
private static final String AUTHORIZATION_HEADER = "Authorization";
private static final int HTTP_CLIENT_TIMEOUT_SECONDS = 10;

private final Logger logger = LoggerFactory.getLogger(InnogyClient.class);
Expand All @@ -91,7 +92,6 @@ public class InnogyClient {
private final HttpClient httpClient;
private @Nullable Gateway bridgeDetails;
private String configVersion = "";
private long apiCallCounter;

public InnogyClient(final OAuthClientService oAuthService, final HttpClient httpClient) {
this.oAuthService = oAuthService;
Expand Down Expand Up @@ -136,7 +136,6 @@ public void refreshStatus() throws IOException, ApiException, AuthenticationExce
* @throws ApiException
*/
private ContentResponse executeGet(final String url) throws IOException, AuthenticationException, ApiException {
apiCallCounter++;
return request(httpClient.newRequest(url).method(HttpMethod.GET));
}

Expand All @@ -152,39 +151,25 @@ private ContentResponse executeGet(final String url) throws IOException, Authent
*/
private ContentResponse executePost(final String url, final Action action)
throws IOException, AuthenticationException, ApiException {
apiCallCounter++;
return executePost(url, gson.toJson(action));
}
final String json = gson.toJson(action);
logger.debug("Action {} JSON: {}", action.getType(), json);

/**
* Executes a HTTP POST request with JSON formatted content.
*
* @param url
* @param content
* @return
* @throws IOException
* @throws AuthenticationException
* @throws ApiException
*/
private ContentResponse executePost(final String url, final String content)
throws IOException, AuthenticationException, ApiException {
apiCallCounter++;
return request(httpClient.newRequest(url).method(HttpMethod.POST)
.content(new StringContentProvider(content), CONTENT_TYPE).accept("application/json"));
.content(new StringContentProvider(json), CONTENT_TYPE).accept(CONTENT_TYPE));
}

private ContentResponse request(final Request request) throws IOException, AuthenticationException, ApiException {
final ContentResponse response;
try {
final AccessTokenResponse accessTokenResponse = getAccessTokenResponse();

response = request.header("Accept", CONTENT_TYPE)
.header(AUTHORIZATION_HEADER, BEARER + accessTokenResponse.getAccessToken())
response = request.header(HttpHeader.ACCEPT, CONTENT_TYPE)
.header(HttpHeader.AUTHORIZATION, BEARER + accessTokenResponse.getAccessToken())
.timeout(HTTP_CLIENT_TIMEOUT_SECONDS, TimeUnit.SECONDS).send();
} catch (InterruptedException | TimeoutException | ExecutionException e) {
throw new IOException(e);
}
handleResponseErrors(response);
handleResponseErrors(response, request.getURI());
return response;
}

Expand All @@ -205,26 +190,27 @@ public AccessTokenResponse getAccessTokenResponse() throws AuthenticationExcepti
* Handles errors from the {@link ContentResponse} and throws the following errors:
*
* @param response
* @param uri uri of api call made
* @throws SessionExistsException
* @throws SessionNotFoundException
* @throws ControllerOfflineException thrown, if the innogy SmartHome controller (SHC) is offline.
* @throws IOException
* @throws ApiException
* @throws AuthenticationException
*/
private void handleResponseErrors(final ContentResponse response)
private void handleResponseErrors(final ContentResponse response, final URI uri)
throws IOException, ApiException, AuthenticationException {
String content = "";

switch (response.getStatus()) {
case HttpStatus.OK_200:
logger.debug("[{}] Statuscode is OK.", apiCallCounter);
logger.debug("Statuscode is OK: [{}]", uri);
return;
case HttpStatus.SERVICE_UNAVAILABLE_503:
logger.debug("innogy service is unavailabe (503).");
throw new ServiceUnavailableException("innogy service is unavailabe (503).");
default:
logger.debug("[{}] Statuscode is NOT OK: {}", apiCallCounter, response.getStatus());
logger.debug("Statuscode {} is NOT OK: [{}]", response.getStatus(), uri);
try {
content = response.getContentAsString();
logger.trace("Response error content: {}", content);
Expand Down Expand Up @@ -274,12 +260,7 @@ private void handleResponseErrors(final ContentResponse response)
*/
public void setSwitchActuatorState(final String capabilityId, final boolean state)
throws IOException, ApiException, AuthenticationException {
final Action action = new SetStateAction(capabilityId, Capability.TYPE_SWITCHACTUATOR, state);

final String json = gson.toJson(action);
logger.debug("Action toggle JSON: {}", json);

executePost(API_URL_ACTION, action);
executePost(API_URL_ACTION, new SetStateAction(capabilityId, Capability.TYPE_SWITCHACTUATOR, state));
}

/**
Expand All @@ -292,12 +273,7 @@ public void setSwitchActuatorState(final String capabilityId, final boolean stat
*/
public void setDimmerActuatorState(final String capabilityId, final int dimLevel)
throws IOException, ApiException, AuthenticationException {
final Action action = new SetStateAction(capabilityId, Capability.TYPE_DIMMERACTUATOR, dimLevel);

final String json = gson.toJson(action);
logger.debug("Action dimm JSON: {}", json);

executePost(API_URL_ACTION, action);
executePost(API_URL_ACTION, new SetStateAction(capabilityId, Capability.TYPE_DIMMERACTUATOR, dimLevel));
}

/**
Expand All @@ -310,13 +286,8 @@ public void setDimmerActuatorState(final String capabilityId, final int dimLevel
*/
public void setRollerShutterActuatorState(final String capabilityId, final int rollerShutterLevel)
throws IOException, ApiException, AuthenticationException {
final Action action = new SetStateAction(capabilityId, Capability.TYPE_ROLLERSHUTTERACTUATOR,
rollerShutterLevel);

final String json = gson.toJson(action);
logger.debug("Action rollershutter JSON: {}", json);

executePost(API_URL_ACTION, action);
executePost(API_URL_ACTION,
new SetStateAction(capabilityId, Capability.TYPE_ROLLERSHUTTERACTUATOR, rollerShutterLevel));
}

/**
Expand All @@ -329,12 +300,7 @@ public void setRollerShutterActuatorState(final String capabilityId, final int r
*/
public void setVariableActuatorState(final String capabilityId, final boolean state)
throws IOException, ApiException, AuthenticationException {
final Action action = new SetStateAction(capabilityId, Capability.TYPE_VARIABLEACTUATOR, state);

final String json = gson.toJson(action);
logger.debug("Action toggle JSON: {}", json);

executePost(API_URL_ACTION, action);
executePost(API_URL_ACTION, new SetStateAction(capabilityId, Capability.TYPE_VARIABLEACTUATOR, state));
}

/**
Expand All @@ -347,12 +313,8 @@ public void setVariableActuatorState(final String capabilityId, final boolean st
*/
public void setPointTemperatureState(final String capabilityId, final double pointTemperature)
throws IOException, ApiException, AuthenticationException {
final Action action = new SetStateAction(capabilityId, Capability.TYPE_THERMOSTATACTUATOR, pointTemperature);

final String json = gson.toJson(action);
logger.debug("Action toggle JSON: {}", json);

executePost(API_URL_ACTION, action);
executePost(API_URL_ACTION,
new SetStateAction(capabilityId, Capability.TYPE_THERMOSTATACTUATOR, pointTemperature));
}

/**
Expand All @@ -365,13 +327,10 @@ public void setPointTemperatureState(final String capabilityId, final double poi
*/
public void setOperationMode(final String capabilityId, final boolean autoMode)
throws IOException, ApiException, AuthenticationException {
final Action action = new SetStateAction(capabilityId, Capability.TYPE_THERMOSTATACTUATOR,
autoMode ? "Auto" : "Manu");

final String json = gson.toJson(action);
logger.debug("Action toggle JSON: {}", json);

executePost(API_URL_ACTION, action);
executePost(API_URL_ACTION,
new SetStateAction(capabilityId, Capability.TYPE_THERMOSTATACTUATOR,
autoMode ? CapabilityState.STATE_VALUE_OPERATION_MODE_AUTO
: CapabilityState.STATE_VALUE_OPERATION_MODE_MANUAL));
}

/**
Expand All @@ -384,12 +343,7 @@ public void setOperationMode(final String capabilityId, final boolean autoMode)
*/
public void setAlarmActuatorState(final String capabilityId, final boolean alarmState)
throws IOException, ApiException, AuthenticationException {
final Action action = new SetStateAction(capabilityId, Capability.TYPE_ALARMACTUATOR, alarmState);

final String json = gson.toJson(action);
logger.debug("Action toggle JSON: {}", json);

executePost(API_URL_ACTION, action);
executePost(API_URL_ACTION, new SetStateAction(capabilityId, Capability.TYPE_ALARMACTUATOR, alarmState));
}

/**
Expand Down
12 changes: 0 additions & 12 deletions ...nogysmarthome/src/main/java/org/openhab/binding/innogysmarthome/internal/client/Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.time.format.DateTimeFormatter;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;

/**
* Utility class with commonly used methods.
Expand All @@ -34,15 +33,4 @@ private Util() {
public static ZonedDateTime convertZuluTimeStringToDate(String timeString) {
return ZonedDateTime.parse(timeString, DateTimeFormatter.ISO_OFFSET_DATE_TIME);
}

/**
* Compares two strings.
*
* @param string1
* @param string2
* @return true, if both strings are equal and not null
*/
public static boolean equalsIfPresent(@Nullable String string1, @Nullable String string2) {
return string1 == null ? false : string1.equals(string2);
}
}
38 changes: 20 additions & 18 deletions ...ava/org/openhab/binding/innogysmarthome/internal/client/entity/action/SetStateAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
*/
public class SetStateAction extends Action {

private static final String CONSTANT = "Constant";

/**
* Constructs a new {@link SetStateAction}.
*
Expand All @@ -32,14 +34,14 @@ public class SetStateAction extends Action {
public SetStateAction(String capabilityId, String capabilityType, boolean state) {
setType(ACTION_TYPE_SETSTATE);
setTargetCapabilityById(capabilityId);
ActionParams params = new ActionParams();
final ActionParams params = new ActionParams();

if (capabilityType.equals(Capability.TYPE_SWITCHACTUATOR)) {
params.setOnState(new BooleanActionParam("Constant", state));
} else if (capabilityType.equals(Capability.TYPE_VARIABLEACTUATOR)) {
params.setValue(new BooleanActionParam("Constant", state));
} else if (capabilityType.equals(Capability.TYPE_ALARMACTUATOR)) {
params.setOnState(new BooleanActionParam("Constant", state));
if (Capability.TYPE_SWITCHACTUATOR.equals(capabilityType)) {
params.setOnState(new BooleanActionParam(CONSTANT, state));
} else if (Capability.TYPE_VARIABLEACTUATOR.equals(capabilityType)) {
params.setValue(new BooleanActionParam(CONSTANT, state));
} else if (Capability.TYPE_ALARMACTUATOR.equals(capabilityType)) {
params.setOnState(new BooleanActionParam(CONSTANT, state));
}
setParams(params);
}
Expand All @@ -54,10 +56,10 @@ public SetStateAction(String capabilityId, String capabilityType, boolean state)
public SetStateAction(String capabilityId, String capabilityType, double newValue) {
setType(ACTION_TYPE_SETSTATE);
setTargetCapabilityById(capabilityId);
ActionParams params = new ActionParams();
final ActionParams params = new ActionParams();

if (capabilityType.equals(Capability.TYPE_THERMOSTATACTUATOR)) {
params.setPointTemperature(new DoubleActionParam("Constant", newValue));
if (Capability.TYPE_THERMOSTATACTUATOR.equals(capabilityType)) {
params.setPointTemperature(new DoubleActionParam(CONSTANT, newValue));
}
setParams(params);
}
Expand All @@ -72,12 +74,12 @@ public SetStateAction(String capabilityId, String capabilityType, double newValu
public SetStateAction(String capabilityId, String capabilityType, int newValue) {
setType(ACTION_TYPE_SETSTATE);
setTargetCapabilityById(capabilityId);
ActionParams params = new ActionParams();
final ActionParams params = new ActionParams();

if (capabilityType.equals(Capability.TYPE_DIMMERACTUATOR)) {
params.setDimLevel(new IntegerActionParam("Constant", newValue));
} else if (capabilityType.equals(Capability.TYPE_ROLLERSHUTTERACTUATOR)) {
params.setShutterLevel(new IntegerActionParam("Constant", newValue));
if (Capability.TYPE_DIMMERACTUATOR.equals(capabilityType)) {
params.setDimLevel(new IntegerActionParam(CONSTANT, newValue));
} else if (Capability.TYPE_ROLLERSHUTTERACTUATOR.equals(capabilityType)) {
params.setShutterLevel(new IntegerActionParam(CONSTANT, newValue));
}

setParams(params);
Expand All @@ -93,10 +95,10 @@ public SetStateAction(String capabilityId, String capabilityType, int newValue)
public SetStateAction(String capabilityId, String capabilityType, String newValue) {
setType(ACTION_TYPE_SETSTATE);
setTargetCapabilityById(capabilityId);
ActionParams params = new ActionParams();
final ActionParams params = new ActionParams();

if (capabilityType.equals(Capability.TYPE_THERMOSTATACTUATOR)) {
params.setOperationMode(new StringActionParam("Constant", newValue));
if (Capability.TYPE_THERMOSTATACTUATOR.equals(capabilityType)) {
params.setOperationMode(new StringActionParam(CONSTANT, newValue));
}
setParams(params);
}
Expand Down
Loading

0 comments on commit 472ee3e

Please sign in to comment.