From 4a8cb5fac9afdf0346cd66c6a9d9f5f3462f7639 Mon Sep 17 00:00:00 2001 From: wzbfyb <57131473+wzbfyb@users.noreply.github.com> Date: Thu, 14 Dec 2023 21:55:12 +0100 Subject: [PATCH] [opensprinkler] Make http connection more resilient (#14998) Signed-off-by: Bernhard Kreuz Signed-off-by: wzbfyb <57131473+wzbfyb@users.noreply.github.com> Co-authored-by: Hilbrand Bouwkamp Co-authored-by: Laurent Garnier --- .../README.md | 2 + .../OpenSprinklerBindingConstants.java | 2 + .../api/OpenSprinklerHttpApiV100.java | 46 +++++++++++++------ .../OpenSprinklerHttpInterfaceConfig.java | 11 +++++ .../OH-INF/i18n/opensprinkler.properties | 4 ++ .../resources/OH-INF/thing/thing-types.xml | 12 +++++ 6 files changed, 64 insertions(+), 13 deletions(-) diff --git a/bundles/org.openhab.binding.opensprinkler/README.md b/bundles/org.openhab.binding.opensprinkler/README.md index 03ca4c97af2..613463b32b7 100644 --- a/bundles/org.openhab.binding.opensprinkler/README.md +++ b/bundles/org.openhab.binding.opensprinkler/README.md @@ -28,6 +28,8 @@ Due to this method used, it is very slow at finding devices and can saturate net - port: Port the OpenSprinkler device is listening on. Usually 80. - password: Admin password of the API. Factory default is: opendoor - refresh: Number of seconds in between refreshing the Thing state with the API. +- timeout: (optional) Number of seconds to wait for a timeout when calling the OpenSprinkler HTTP API. +- retry: (optional) Number of retries on connection timeouts. - basicUsername: (optional) Only needed when the OpenSprinkler device is behind a basic auth enforcing reverse proxy. - basicPassword: (optional) Only needed when the OpenSprinkler device is behind a basic auth enforcing reverse proxy. diff --git a/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/OpenSprinklerBindingConstants.java b/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/OpenSprinklerBindingConstants.java index d16fab81453..193330eb16d 100644 --- a/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/OpenSprinklerBindingConstants.java +++ b/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/OpenSprinklerBindingConstants.java @@ -48,6 +48,8 @@ public class OpenSprinklerBindingConstants { public static final String JSON_OPTION_STATION_COUNT = "nstations"; public static final String JSON_OPTION_RESULT = "result"; public static final int DEFAULT_REFRESH_RATE = 60; + public static final int DEFAULT_TIMEOUT = 5; + public static final int DEFFAULT_RETRIES = 3; public static final int DISCOVERY_THREAD_POOL_SIZE = 15; public static final boolean DISCOVERY_DEFAULT_AUTO_DISCOVER = false; public static final int DISCOVERY_DEFAULT_TIMEOUT_RATE = 500; diff --git a/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/api/OpenSprinklerHttpApiV100.java b/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/api/OpenSprinklerHttpApiV100.java index b282523f702..afba8e3674f 100644 --- a/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/api/OpenSprinklerHttpApiV100.java +++ b/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/api/OpenSprinklerHttpApiV100.java @@ -12,7 +12,15 @@ */ package org.openhab.binding.opensprinkler.internal.api; -import static org.openhab.binding.opensprinkler.internal.OpenSprinklerBindingConstants.*; +import static org.openhab.binding.opensprinkler.internal.OpenSprinklerBindingConstants.CMD_DISABLE_MANUAL_MODE; +import static org.openhab.binding.opensprinkler.internal.OpenSprinklerBindingConstants.CMD_ENABLE_MANUAL_MODE; +import static org.openhab.binding.opensprinkler.internal.OpenSprinklerBindingConstants.CMD_OPTIONS_INFO; +import static org.openhab.binding.opensprinkler.internal.OpenSprinklerBindingConstants.CMD_PASSWORD; +import static org.openhab.binding.opensprinkler.internal.OpenSprinklerBindingConstants.CMD_STATION_INFO; +import static org.openhab.binding.opensprinkler.internal.OpenSprinklerBindingConstants.CMD_STATUS_INFO; +import static org.openhab.binding.opensprinkler.internal.OpenSprinklerBindingConstants.DEFAULT_STATION_COUNT; +import static org.openhab.binding.opensprinkler.internal.OpenSprinklerBindingConstants.HTTPS_REQUEST_URL_PREFIX; +import static org.openhab.binding.opensprinkler.internal.OpenSprinklerBindingConstants.HTTP_REQUEST_URL_PREFIX; import java.math.BigDecimal; import java.nio.charset.StandardCharsets; @@ -379,22 +387,34 @@ class OpenSprinklerHttpApiV100 implements OpenSprinklerApi { } else { location = url; } - ContentResponse response; - try { - response = withGeneralProperties(httpClient.newRequest(location)).timeout(5, TimeUnit.SECONDS) - .method(HttpMethod.GET).send(); - } catch (InterruptedException | TimeoutException | ExecutionException e) { - throw new CommunicationApiException("Request to OpenSprinkler device failed: " + e.getMessage()); + ContentResponse response = null; + int retriesLeft = Math.max(1, config.retry); + boolean connectionSuccess = false; + while (connectionSuccess == false && retriesLeft > 0) { + retriesLeft--; + try { + response = withGeneralProperties(httpClient.newRequest(location)) + .timeout(config.timeout, TimeUnit.SECONDS).method(HttpMethod.GET).send(); + connectionSuccess = true; + } catch (InterruptedException | TimeoutException | ExecutionException e) { + logger.warn("Request to OpenSprinkler device failed (retries left: {}): {}", retriesLeft, + e.getMessage()); + } } - if (response.getStatus() != HTTP_OK_CODE) { + if (connectionSuccess == false) { + throw new CommunicationApiException("Request to OpenSprinkler device failed"); + } + if (response != null && response.getStatus() != HTTP_OK_CODE) { throw new CommunicationApiException( "Error sending HTTP GET request to " + url + ". Got response code: " + response.getStatus()); + } else if (response != null) { + String content = response.getContentAsString(); + if ("{\"result\":2}".equals(content)) { + throw new UnauthorizedApiException("Unauthorized, check your password is correct"); + } + return content; } - String content = response.getContentAsString(); - if ("{\"result\":2}".equals(content)) { - throw new UnauthorizedApiException("Unauthorized, check your password is correct"); - } - return content; + return ""; } private Request withGeneralProperties(Request request) { diff --git a/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/config/OpenSprinklerHttpInterfaceConfig.java b/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/config/OpenSprinklerHttpInterfaceConfig.java index 3ba46b388b1..52811650ea2 100644 --- a/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/config/OpenSprinklerHttpInterfaceConfig.java +++ b/bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/config/OpenSprinklerHttpInterfaceConfig.java @@ -43,6 +43,17 @@ public class OpenSprinklerHttpInterfaceConfig { * Number of seconds in between refreshes from the OpenSprinkler device. */ public int refresh = 60; + + /** + * Number of seconds for connection timeouts + */ + public int timeout = 5; + + /** + * Number of retries in case of connection timeouts + */ + public int retry = 3; + /** * The basic auth username to use when the OpenSprinkler device is behind a reverse proxy with basic auth enabled. */ diff --git a/bundles/org.openhab.binding.opensprinkler/src/main/resources/OH-INF/i18n/opensprinkler.properties b/bundles/org.openhab.binding.opensprinkler/src/main/resources/OH-INF/i18n/opensprinkler.properties index 7bb8cf3eea8..7840ee4047d 100644 --- a/bundles/org.openhab.binding.opensprinkler/src/main/resources/OH-INF/i18n/opensprinkler.properties +++ b/bundles/org.openhab.binding.opensprinkler/src/main/resources/OH-INF/i18n/opensprinkler.properties @@ -26,6 +26,10 @@ thing-type.config.opensprinkler.http.port.label = Port thing-type.config.opensprinkler.http.port.description = Port of the OpenSprinkler Web API interface. thing-type.config.opensprinkler.http.refresh.label = Refresh Interval thing-type.config.opensprinkler.http.refresh.description = Specifies the refresh interval in seconds. +thing-type.config.opensprinkler.http.retry.label = Retry Count +thing-type.config.opensprinkler.http.retry.description = Specifies the number of retries on connection timeouts. +thing-type.config.opensprinkler.http.timeout.label = Timeout +thing-type.config.opensprinkler.http.timeout.description = Specifies the connection timeout in seconds. thing-type.config.opensprinkler.station.stationIndex.label = Station Index thing-type.config.opensprinkler.station.stationIndex.description = The index of the station, starting with 0, of the station. diff --git a/bundles/org.openhab.binding.opensprinkler/src/main/resources/OH-INF/thing/thing-types.xml b/bundles/org.openhab.binding.opensprinkler/src/main/resources/OH-INF/thing/thing-types.xml index bdd001917a5..4d9f3611599 100644 --- a/bundles/org.openhab.binding.opensprinkler/src/main/resources/OH-INF/thing/thing-types.xml +++ b/bundles/org.openhab.binding.opensprinkler/src/main/resources/OH-INF/thing/thing-types.xml @@ -32,6 +32,18 @@ Specifies the refresh interval in seconds. 60 + + + Specifies the number of retries on connection timeouts. + 3 + true + + + + Specifies the connection timeout in seconds. + 5 + true + Used if the OpenSprinkler device is behind a basic auth protected reverse proxy.