[netatmo] Handling camera ping timeouts (#13487)

* Solves #13486 Signed-off-by: clinique <gael@lhopital.org>
openhab · Oct 6, 2022 · e14ce7d · e14ce7d
1 parent 2519ec7
commit e14ce7d
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 15 deletions.
diff --git a/...b.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/SecurityApi.java b/...b.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/SecurityApi.java
@@ -21,13 +21,16 @@
 import javax.ws.rs.core.UriBuilder;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.netatmo.internal.api.data.NetatmoConstants.FeatureArea;
 import org.openhab.binding.netatmo.internal.api.data.NetatmoConstants.FloodLightMode;
 import org.openhab.binding.netatmo.internal.api.dto.Home;
 import org.openhab.binding.netatmo.internal.api.dto.HomeEvent;
 import org.openhab.binding.netatmo.internal.api.dto.HomeEvent.NAEventsDataResponse;
 import org.openhab.binding.netatmo.internal.api.dto.Ping;
 import org.openhab.binding.netatmo.internal.handler.ApiBridgeHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Base class for all Security related endpoints
@@ -36,6 +39,8 @@
  */
 @NonNullByDefault
 public class SecurityApi extends RestManager {
+    private final Logger logger = LoggerFactory.getLogger(SecurityApi.class);
+
     public SecurityApi(ApiBridgeHandler apiClient) {
         super(apiClient, FeatureArea.SECURITY);
     }
@@ -91,10 +96,14 @@ public Collection<HomeEvent> getDeviceEvents(String homeId, String deviceId, Str
         throw new NetatmoException("home should not be null");
     }
 
-    public String ping(String vpnUrl) throws NetatmoException {
+    public @Nullable String ping(String vpnUrl) {
         UriBuilder uriBuilder = UriBuilder.fromUri(vpnUrl).path(PATH_COMMAND).path(SUB_PATH_PING);
-        Ping response = get(uriBuilder, Ping.class);
-        return response.getStatus();
+        try {
+            return get(uriBuilder, Ping.class).getStatus();
+        } catch (NetatmoException e) {
+            logger.debug("Pinging {} failed : {}", vpnUrl, e.getMessage());
+            return null;
+        }
     }
 
     public void changeStatus(String localCameraURL, boolean setOn) throws NetatmoException {

diff --git a/...g.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/CommonInterface.java b/...g.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/CommonInterface.java
@@ -155,7 +155,9 @@ default void setNewData(NAObject newData) {
                 finalReason = thingStatusReason;
             }
         }
-        if (!newData.isIgnoredForThingUpdate()) {
+        // Prevent turning ONLINE myself if in the meantime something turned account OFFLINE
+        ApiBridgeHandler accountHandler = getAccountHandler();
+        if (accountHandler != null && accountHandler.isConnected() && !newData.isIgnoredForThingUpdate()) {
             setThingStatus(finalReason == null ? ThingStatus.ONLINE : ThingStatus.OFFLINE, ThingStatusDetail.NONE,
                     finalReason);
         }

diff --git a/...c/main/java/org/openhab/binding/netatmo/internal/handler/capability/CameraCapability.java b/...c/main/java/org/openhab/binding/netatmo/internal/handler/capability/CameraCapability.java
@@ -49,6 +49,7 @@ public class CameraCapability extends HomeSecurityThingCapability {
     private final ChannelUID personChannelUID;
 
     protected @Nullable String localUrl;
+    protected @Nullable String vpnUrl;
 
     public CameraCapability(CommonInterface handler, NetatmoDescriptionProvider descriptionProvider,
             List<ChannelHelper> channelHelpers) {
@@ -62,12 +63,15 @@ public CameraCapability(CommonInterface handler, NetatmoDescriptionProvider desc
     @Override
     public void updateHomeStatusModule(HomeStatusModule newData) {
         super.updateHomeStatusModule(newData);
-        String vpnUrl = newData.getVpnUrl();
-        if (vpnUrl != null) {
-            localUrl = newData.isLocal() ? securityCapability.map(cap -> cap.ping(vpnUrl)).orElse(null) : null;
-            cameraHelper.setUrls(vpnUrl, localUrl);
-            eventHelper.setUrls(vpnUrl, localUrl);
+        // Per documentation vpn_url expires every 3 hours and on camera reboot. So useless to reping it if not changed
+        String newVpnUrl = newData.getVpnUrl();
+        if (newVpnUrl != null && !newVpnUrl.equals(vpnUrl)) {
+            // This will also decrease the number of requests emitted toward Netatmo API.
+            localUrl = newData.isLocal() ? securityCapability.map(cap -> cap.ping(newVpnUrl)).orElse(null) : null;
+            cameraHelper.setUrls(newVpnUrl, localUrl);
+            eventHelper.setUrls(newVpnUrl, localUrl);
         }
+        vpnUrl = newVpnUrl;
         if (!SdCardStatus.SD_CARD_WORKING.equals(newData.getSdStatus())
                 || !AlimentationStatus.ALIM_CORRECT_POWER.equals(newData.getAlimStatus())) {
             statusReason = String.format("%s, %s", newData.getSdStatus(), newData.getAlimStatus());

diff --git a/...main/java/org/openhab/binding/netatmo/internal/handler/capability/SecurityCapability.java b/...main/java/org/openhab/binding/netatmo/internal/handler/capability/SecurityCapability.java
@@ -145,12 +145,7 @@ public void setPersonAway(String personId, boolean away) {
 
     public @Nullable String ping(String vpnUrl) {
         return getApi().map(api -> {
-            try {
-                return api.ping(vpnUrl);
-            } catch (NetatmoException e) {
-                logger.warn("Error pinging camera '{}' : {}", vpnUrl, e.getMessage());
-                return null;
-            }
+            return api.ping(vpnUrl);
         }).orElse(null);
     }