From 8e5c2455e8c9a7d2ace2c6dbb2eb328af5628564 Mon Sep 17 00:00:00 2001 From: Jacob Laursen Date: Sat, 6 Aug 2022 08:42:59 +0200 Subject: [PATCH] [boschindego] Fix thing marked as offline when device is reachable (#13219) * Fix thing offline on invalid command Signed-off-by: Jacob Laursen * Rename exception Signed-off-by: Jacob Laursen * Go back to last thing status after temporary disruptions Signed-off-by: Jacob Laursen * Increase call frequency after device being unreachable Signed-off-by: Jacob Laursen --- .../internal/IndegoController.java | 36 +++++++++++----- .../internal/dto/response/ErrorResponse.java | 22 ++++++++++ .../IndegoInvalidCommandException.java | 14 ++++++ ...ption.java => IndegoTimeoutException.java} | 8 ++-- .../internal/handler/BoschIndegoHandler.java | 43 +++++++++++++------ 5 files changed, 97 insertions(+), 26 deletions(-) create mode 100644 bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/dto/response/ErrorResponse.java rename bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/{IndegoUnreachableException.java => IndegoTimeoutException.java} (74%) diff --git a/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/IndegoController.java b/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/IndegoController.java index eeb9cde6a87..d81bf26ca5a 100644 --- a/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/IndegoController.java +++ b/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/IndegoController.java @@ -38,6 +38,7 @@ import org.openhab.binding.boschindego.internal.dto.request.SetStateRequest; import org.openhab.binding.boschindego.internal.dto.response.AuthenticationResponse; import org.openhab.binding.boschindego.internal.dto.response.DeviceCalendarResponse; import org.openhab.binding.boschindego.internal.dto.response.DeviceStateResponse; +import org.openhab.binding.boschindego.internal.dto.response.ErrorResponse; import org.openhab.binding.boschindego.internal.dto.response.LocationWeatherResponse; import org.openhab.binding.boschindego.internal.dto.response.OperatingDataResponse; import org.openhab.binding.boschindego.internal.dto.response.PredictiveLastCuttingResponse; @@ -46,7 +47,7 @@ import org.openhab.binding.boschindego.internal.exceptions.IndegoAuthenticationE import org.openhab.binding.boschindego.internal.exceptions.IndegoException; import org.openhab.binding.boschindego.internal.exceptions.IndegoInvalidCommandException; import org.openhab.binding.boschindego.internal.exceptions.IndegoInvalidResponseException; -import org.openhab.binding.boschindego.internal.exceptions.IndegoUnreachableException; +import org.openhab.binding.boschindego.internal.exceptions.IndegoTimeoutException; import org.openhab.core.library.types.RawType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -192,11 +193,11 @@ public class IndegoController { * @param dtoClass the DTO class to which the JSON result should be deserialized * @return the deserialized DTO from the JSON response * @throws IndegoAuthenticationException if request was rejected as unauthorized - * @throws IndegoUnreachableException if device cannot be reached (gateway timeout error) + * @throws IndegoTimeoutException if device cannot be reached (gateway timeout error) * @throws IndegoException if any communication or parsing error occurred */ private T getRequestWithAuthentication(String path, Class dtoClass) - throws IndegoAuthenticationException, IndegoUnreachableException, IndegoException { + throws IndegoAuthenticationException, IndegoTimeoutException, IndegoException { if (!session.isValid()) { authenticate(); } @@ -222,11 +223,11 @@ public class IndegoController { * @param dtoClass the DTO class to which the JSON result should be deserialized * @return the deserialized DTO from the JSON response * @throws IndegoAuthenticationException if request was rejected as unauthorized - * @throws IndegoUnreachableException if device cannot be reached (gateway timeout error) + * @throws IndegoTimeoutException if device cannot be reached (gateway timeout error) * @throws IndegoException if any communication or parsing error occurred */ private T getRequest(String path, Class dtoClass) - throws IndegoAuthenticationException, IndegoUnreachableException, IndegoException { + throws IndegoAuthenticationException, IndegoTimeoutException, IndegoException { int status = 0; try { Request request = httpClient.newRequest(BASE_URL + path).method(HttpMethod.GET).header(CONTEXT_HEADER_NAME, @@ -236,21 +237,23 @@ public class IndegoController { } ContentResponse response = sendRequest(request); status = response.getStatus(); + String jsonResponse = response.getContentAsString(); + if (!jsonResponse.isEmpty()) { + logger.trace("JSON response: '{}'", jsonResponse); + } if (status == HttpStatus.UNAUTHORIZED_401) { // This will currently not happen because "WWW-Authenticate" header is missing; see below. throw new IndegoAuthenticationException("Context rejected"); } if (status == HttpStatus.GATEWAY_TIMEOUT_504) { - throw new IndegoUnreachableException("Gateway timeout"); + throw new IndegoTimeoutException("Gateway timeout"); } if (!HttpStatus.isSuccess(status)) { throw new IndegoException("The request failed with error: " + status); } - String jsonResponse = response.getContentAsString(); if (jsonResponse.isEmpty()) { throw new IndegoInvalidResponseException("No content returned", status); } - logger.trace("JSON response: '{}'", jsonResponse); @Nullable T result = gson.fromJson(jsonResponse, dtoClass); @@ -450,12 +453,25 @@ public class IndegoController { logger.trace("{} request for {} with no payload", method, BASE_URL + path); } ContentResponse response = sendRequest(request); + String jsonResponse = response.getContentAsString(); + if (!jsonResponse.isEmpty()) { + logger.trace("JSON response: '{}'", jsonResponse); + } int status = response.getStatus(); if (status == HttpStatus.UNAUTHORIZED_401) { // This will currently not happen because "WWW-Authenticate" header is missing; see below. throw new IndegoAuthenticationException("Context rejected"); } if (status == HttpStatus.INTERNAL_SERVER_ERROR_500) { + try { + ErrorResponse result = gson.fromJson(jsonResponse, ErrorResponse.class); + if (result != null) { + throw new IndegoInvalidCommandException("The request failed with HTTP error: " + status, + result.error); + } + } catch (JsonParseException e) { + // Ignore missing error code, next line will throw. + } throw new IndegoInvalidCommandException("The request failed with HTTP error: " + status); } if (!HttpStatus.isSuccess(status)) { @@ -575,11 +591,11 @@ public class IndegoController { * * @return the device state * @throws IndegoAuthenticationException if request was rejected as unauthorized - * @throws IndegoUnreachableException if device cannot be reached (gateway timeout error) + * @throws IndegoTimeoutException if device cannot be reached (gateway timeout error) * @throws IndegoException if any communication or parsing error occurred */ public OperatingDataResponse getOperatingData() - throws IndegoAuthenticationException, IndegoUnreachableException, IndegoException { + throws IndegoAuthenticationException, IndegoTimeoutException, IndegoException { return getRequestWithAuthentication(SERIAL_NUMBER_SUBPATH + this.getSerialNumber() + "/operatingData", OperatingDataResponse.class); } diff --git a/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/dto/response/ErrorResponse.java b/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/dto/response/ErrorResponse.java new file mode 100644 index 00000000000..3e88d43ce8f --- /dev/null +++ b/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/dto/response/ErrorResponse.java @@ -0,0 +1,22 @@ +/** + * Copyright (c) 2010-2022 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.boschindego.internal.dto.response; + +/** + * Response from PUT request. + * + * @author Jacob Laursen - Initial contribution + */ +public class ErrorResponse { + public int error; +} diff --git a/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/IndegoInvalidCommandException.java b/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/IndegoInvalidCommandException.java index 024a9efb1de..7a28803a091 100644 --- a/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/IndegoInvalidCommandException.java +++ b/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/IndegoInvalidCommandException.java @@ -23,8 +23,22 @@ import org.eclipse.jdt.annotation.NonNullByDefault; public class IndegoInvalidCommandException extends IndegoException { private static final long serialVersionUID = -2946398731437793113L; + private int errorCode = 0; public IndegoInvalidCommandException(String message) { super(message); } + + public IndegoInvalidCommandException(String message, int errorCode) { + super(message); + this.errorCode = errorCode; + } + + public boolean hasErrorCode() { + return errorCode != 0; + } + + public int getErrorCode() { + return errorCode; + } } diff --git a/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/IndegoUnreachableException.java b/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/IndegoTimeoutException.java similarity index 74% rename from bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/IndegoUnreachableException.java rename to bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/IndegoTimeoutException.java index 1af704f874b..1a21e66957d 100644 --- a/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/IndegoUnreachableException.java +++ b/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/exceptions/IndegoTimeoutException.java @@ -15,21 +15,21 @@ package org.openhab.binding.boschindego.internal.exceptions; import org.eclipse.jdt.annotation.NonNullByDefault; /** - * {@link IndegoUnreachableException} is thrown on gateway timeout, which + * {@link IndegoTimeoutException} is thrown on gateway timeout, which * means that Bosch services cannot connect to the device. * * @author Jacob Laursen - Initial contribution */ @NonNullByDefault -public class IndegoUnreachableException extends IndegoException { +public class IndegoTimeoutException extends IndegoException { private static final long serialVersionUID = -7952585411438042139L; - public IndegoUnreachableException(String message) { + public IndegoTimeoutException(String message) { super(message); } - public IndegoUnreachableException(String message, Throwable cause) { + public IndegoTimeoutException(String message, Throwable cause) { super(message, cause); } } diff --git a/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/handler/BoschIndegoHandler.java b/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/handler/BoschIndegoHandler.java index d4f611ace6e..9b5d9ec2f02 100644 --- a/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/handler/BoschIndegoHandler.java +++ b/bundles/org.openhab.binding.boschindego/src/main/java/org/openhab/binding/boschindego/internal/handler/BoschIndegoHandler.java @@ -35,7 +35,8 @@ import org.openhab.binding.boschindego.internal.dto.response.DeviceStateResponse import org.openhab.binding.boschindego.internal.dto.response.OperatingDataResponse; import org.openhab.binding.boschindego.internal.exceptions.IndegoAuthenticationException; import org.openhab.binding.boschindego.internal.exceptions.IndegoException; -import org.openhab.binding.boschindego.internal.exceptions.IndegoUnreachableException; +import org.openhab.binding.boschindego.internal.exceptions.IndegoInvalidCommandException; +import org.openhab.binding.boschindego.internal.exceptions.IndegoTimeoutException; import org.openhab.core.i18n.TimeZoneProvider; import org.openhab.core.library.types.DateTimeType; import org.openhab.core.library.types.DecimalType; @@ -73,6 +74,7 @@ public class BoschIndegoHandler extends BaseThingHandler { private static final Duration MAP_REFRESH_INTERVAL = Duration.ofDays(1); private static final Duration OPERATING_DATA_INACTIVE_REFRESH_INTERVAL = Duration.ofHours(6); + private static final Duration OPERATING_DATA_OFFLINE_REFRESH_INTERVAL = Duration.ofMinutes(30); private static final Duration OPERATING_DATA_ACTIVE_REFRESH_INTERVAL = Duration.ofMinutes(2); private static final Duration MAP_REFRESH_SESSION_DURATION = Duration.ofMinutes(5); private static final Duration COMMAND_STATE_REFRESH_TIMEOUT = Duration.ofSeconds(10); @@ -92,6 +94,7 @@ public class BoschIndegoHandler extends BaseThingHandler { private Instant cachedMapTimestamp = Instant.MIN; private Instant operatingDataTimestamp = Instant.MIN; private Instant mapRefreshStartedTimestamp = Instant.MIN; + private ThingStatus lastOperatingDataStatus = ThingStatus.UNINITIALIZED; private int stateInactiveRefreshIntervalSeconds; private int stateActiveRefreshIntervalSeconds; private int currentRefreshIntervalSeconds; @@ -193,16 +196,21 @@ public class BoschIndegoHandler extends BaseThingHandler { } catch (IndegoAuthenticationException e) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "@text/offline.comm-error.authentication-failure"); - } catch (IndegoUnreachableException e) { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, + } catch (IndegoTimeoutException e) { + updateStatus(lastOperatingDataStatus = ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "@text/offline.comm-error.unreachable"); + } catch (IndegoInvalidCommandException e) { + logger.warn("Invalid command: {}", e.getMessage()); + if (e.hasErrorCode()) { + updateState(ERRORCODE, new DecimalType(e.getErrorCode())); + } } catch (IndegoException e) { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage()); + logger.warn("Command failed: {}", e.getMessage()); } } private void handleRefreshCommand(String channelId) - throws IndegoAuthenticationException, IndegoUnreachableException, IndegoException { + throws IndegoAuthenticationException, IndegoTimeoutException, IndegoException { switch (channelId) { case GARDEN_MAP: // Force map refresh and fall through to state update. @@ -274,8 +282,8 @@ public class BoschIndegoHandler extends BaseThingHandler { } catch (IndegoAuthenticationException e) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "@text/offline.comm-error.authentication-failure"); - } catch (IndegoUnreachableException e) { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, + } catch (IndegoTimeoutException e) { + updateStatus(lastOperatingDataStatus = ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "@text/offline.comm-error.unreachable"); } catch (IndegoException e) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage()); @@ -322,6 +330,15 @@ public class BoschIndegoHandler extends BaseThingHandler { refreshOperatingDataConditionally( previousDeviceStatus.isCharging() || deviceStatus.isCharging() || deviceStatus.isActive()); + if (lastOperatingDataStatus == ThingStatus.ONLINE && thing.getStatus() != ThingStatus.ONLINE) { + // Revert temporary offline status caused by disruptions other than unreachable device. + updateStatus(ThingStatus.ONLINE); + } else if (lastOperatingDataStatus == ThingStatus.OFFLINE) { + // Update description to reflect why thing is still offline. + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, + "@text/offline.comm-error.unreachable"); + } + rescheduleStatePollAccordingToState(deviceStatus); } @@ -342,21 +359,23 @@ public class BoschIndegoHandler extends BaseThingHandler { } private void refreshOperatingDataConditionally(boolean isActive) - throws IndegoAuthenticationException, IndegoUnreachableException, IndegoException { + throws IndegoAuthenticationException, IndegoTimeoutException, IndegoException { // Refresh operating data only occationally or when robot is active/charging. // This will contact the robot directly through cellular network and wake it up - // when sleeping. + // when sleeping. Additionally, refresh more often after being offline to try to get + // back online as soon as possible without putting too much stress on the service. if ((isActive && operatingDataTimestamp.isBefore(Instant.now().minus(OPERATING_DATA_ACTIVE_REFRESH_INTERVAL))) + || (lastOperatingDataStatus != ThingStatus.ONLINE && operatingDataTimestamp + .isBefore(Instant.now().minus(OPERATING_DATA_OFFLINE_REFRESH_INTERVAL))) || operatingDataTimestamp.isBefore(Instant.now().minus(OPERATING_DATA_INACTIVE_REFRESH_INTERVAL))) { refreshOperatingData(); } } - private void refreshOperatingData() - throws IndegoAuthenticationException, IndegoUnreachableException, IndegoException { + private void refreshOperatingData() throws IndegoAuthenticationException, IndegoTimeoutException, IndegoException { updateOperatingData(controller.getOperatingData()); operatingDataTimestamp = Instant.now(); - updateStatus(ThingStatus.ONLINE); + updateStatus(lastOperatingDataStatus = ThingStatus.ONLINE); } private void refreshCuttingTimesWithExceptionHandling() {