[boschindego] Fix thing marked as offline when device is reachable (#13219)

* Fix thing offline on invalid command

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>

* Rename exception

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>

* Go back to last thing status after temporary disruptions

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>

* Increase call frequency after device being unreachable

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>

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> T getRequestWithAuthentication(String path, Class<? extends T> 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> T getRequest(String path, Class<? extends T> 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);
     }

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;
+}

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;
+    }
 }

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);
     }
 }

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) {
+        } catch (IndegoTimeoutException e) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+            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) {
+        } catch (IndegoTimeoutException e) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+            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()
+    private void refreshOperatingData() throws IndegoAuthenticationException, IndegoTimeoutException, IndegoException {
-            throws IndegoAuthenticationException, IndegoUnreachableException, IndegoException {
         updateOperatingData(controller.getOperatingData());
         operatingDataTimestamp = Instant.now();
-        updateStatus(ThingStatus.ONLINE);
+        updateStatus(lastOperatingDataStatus = ThingStatus.ONLINE);
     }
 
     private void refreshCuttingTimesWithExceptionHandling() {