[netatmo] Enforce API reconnect delay (#18029)

* Enable push / pull mode when webhook is set

Signed-off-by: clinique <gael@lhopital.org>
Signed-off-by: gael@lhopital.org <gael@lhopital.org>

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/data/NetatmoConstants.java

@@ -420,6 +420,8 @@ public class NetatmoConstants {
         DEVICE_NOT_FOUND,
         @SerializedName("10")
         MISSING_ARGUMENTS,
+        @SerializedName("11")
+        CONCURRENCY_LIMIT_TIMED_OUT,
         @SerializedName("13")
         OPERATION_FORBIDDEN,
         @SerializedName("19")

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/config/ApiHandlerConfiguration.java

@@ -12,6 +12,8 @@
  */
 package org.openhab.binding.netatmo.internal.config;
 
+import java.time.Duration;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
 
 /**
@@ -28,7 +30,7 @@ public class ApiHandlerConfiguration {
     public String clientSecret = "";
     public String webHookUrl = "";
     public String webHookPostfix = "";
-    public int reconnectInterval = 300;
+    private int reconnectInterval = 300;
 
     public ConfigurationLevel check() {
         if (clientId.isBlank()) {
@@ -38,4 +40,8 @@ public class ApiHandlerConfiguration {
         }
         return ConfigurationLevel.COMPLETED;
     }
+
+    public Duration getReconnectInterval() {
+        return Duration.ofSeconds(reconnectInterval);
+    }
 }

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java

@@ -20,6 +20,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
+import java.time.Duration;
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.ArrayDeque;
@@ -161,6 +162,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         }
 
         logger.debug("Connected to Netatmo API.");
+        freeConnectJob();
 
         ApiHandlerConfiguration configuration = getConfiguration();
         if (!configuration.webHookUrl.isBlank()
@@ -199,7 +201,11 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
             startAuthorizationFlow();
             return false;
         } catch (IOException e) {
-            prepareReconnection(getConfiguration().reconnectInterval, e.getMessage(), code, redirectUri);
+            String message = e.getMessage();
+            if (message == null) {
+                message = e.getClass().getName();
+            }
+            prepareReconnection(message, code, redirectUri);
             return false;
         }
 
@@ -226,29 +232,32 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         return getConfigAs(ApiHandlerConfiguration.class);
     }
 
-    private void prepareReconnection(int delay, @Nullable String message, @Nullable String code,
+    private void prepareReconnection(String message, @Nullable String code, @Nullable String redirectUri) {
+        prepareReconnection(message, getConfiguration().getReconnectInterval(), code, redirectUri);
+    }
+
+    private void prepareReconnection(String message, Duration delay, @Nullable String code,
             @Nullable String redirectUri) {
         if (!ThingStatus.OFFLINE.equals(thing.getStatus())) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, message);
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                    "Reconnection in %s: %s".formatted(delay, message));
         }
         connectApi.dispose();
         freeConnectJob();
-        connectJob = scheduler.schedule(() -> openConnection(code, redirectUri), delay, TimeUnit.SECONDS);
+        connectJob = scheduler.schedule(() -> openConnection(code, redirectUri), delay.toSeconds(), TimeUnit.SECONDS);
         logger.debug("Reconnection scheduled in {} seconds", delay);
     }
 
     private void freeConnectJob() {
-        ScheduledFuture<?> connectJob = this.connectJob;
+        if (connectJob instanceof ScheduledFuture job) {
-        if (connectJob != null) {
+            job.cancel(true);
-            connectJob.cancel(true);
         }
         this.connectJob = null;
     }
 
     private void freeGrantServlet() {
-        GrantServlet grantServlet = this.grantServlet;
+        if (grantServlet instanceof GrantServlet servlet) {
-        if (grantServlet != null) {
+            servlet.dispose();
-            grantServlet.dispose();
         }
         this.grantServlet = null;
     }
@@ -304,13 +313,17 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
 
     public synchronized <T> T executeUri(URI uri, HttpMethod method, Class<T> clazz, @Nullable String payload,
             @Nullable String contentType, int retryCount) throws NetatmoException {
+        if (connectJob != null) {
+            throw new NetatmoException("Connection pending, no other request accepted in the meantime.");
+        }
+
+        logger.debug("executeUri {}  {} ", method.toString(), uri);
+
+        Request request = httpClient.newRequest(uri).method(method).timeout(TIMEOUT_S, TimeUnit.SECONDS);
+
         try {
-            logger.debug("executeUri {}  {} ", method.toString(), uri);
-
-            Request request = httpClient.newRequest(uri).method(method).timeout(TIMEOUT_S, TimeUnit.SECONDS);
-
             if (!authenticate(null, null)) {
-                prepareReconnection(getConfiguration().reconnectInterval, "@text/status-bridge-offline", null, null);
+                prepareReconnection("@text/status-bridge-offline", null, null);
                 throw new NetatmoException("Not authenticated");
             }
             connectApi.getAuthorization().ifPresent(auth -> request.header(HttpHeader.AUTHORIZATION, auth));
@@ -359,10 +372,18 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
                             "Error deserializing error: %s".formatted(statusCode.getMessage()));
                 }
             }
-            if (statusCode == Code.TOO_MANY_REQUESTS
+            if (statusCode == Code.TOO_MANY_REQUESTS) {
-                    || exception.getStatusCode() == ServiceError.MAXIMUM_USAGE_REACHED) {
+                String message = null;
-                prepareReconnection(API_LIMIT_INTERVAL_S,
+                String delayStr = response.getHeaders().get(HttpHeader.RETRY_AFTER);
-                        "@text/maximum-usage-reached [ \"%d\" ]".formatted(API_LIMIT_INTERVAL_S), null, null);
+                int delay = delayStr != null ? Integer.valueOf(delayStr) : Integer.MAX_VALUE;
+                if (exception.getStatusCode() == ServiceError.CONCURRENCY_LIMIT_TIMED_OUT) {
+                    delay = Math.min(delay, TIMEOUT_S);
+                    message = "@text/concurrency-limit-timed-out [ \"%d\" ]";
+                } else { // ServiceError.MAXIMUM_USAGE_REACHED
+                    delay = Math.min(delay, API_LIMIT_INTERVAL_S);
+                    message = "@text/maximum-usage-reached [ \"%d\" ]";
+                }
+                prepareReconnection(message.formatted(delay), Duration.ofSeconds(delay), null, null);
             }
             throw exception;
         } catch (InterruptedException e) {
@@ -374,7 +395,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
                 logger.debug("Request error, retry counter: {}", retryCount);
                 return executeUri(uri, method, clazz, payload, contentType, retryCount - 1);
             }
-            prepareReconnection(getConfiguration().reconnectInterval, "@text/request-time-out", null, e.getMessage());
+            prepareReconnection("@text/request-time-out", null, e.getMessage());
             throw new NetatmoException("%s: \"%s\"".formatted(e.getClass().getName(), e.getMessage()));
         }
     }
@@ -463,4 +484,8 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
     public Optional<WebhookServlet> getWebHookServlet() {
         return Optional.ofNullable(webHookServlet);
     }
+
+    public @Nullable Duration getIdleTime() {
+        return connectJob instanceof ScheduledFuture job ? Duration.ofSeconds(job.getDelay(TimeUnit.SECONDS)) : null;
+    }
 }

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/AlarmEventCapability.java

@@ -55,8 +55,9 @@ public class AlarmEventCapability extends HomeSecurityThingCapability {
 
     @Override
     public List<NAObject> updateReadings() {
-        return Objects.requireNonNull(
+        return pullMode() ? Objects.requireNonNull(
                 getSecurityCapability().map(cap -> cap.getDeviceLastEvent(handler.getId(), moduleType.apiName))
-                        .map(event -> List.of((NAObject) event)).orElse(List.of()));
+                        .map(event -> List.of((NAObject) event)).orElse(List.of()))
+                : List.of();
     }
 }

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CameraCapability.java

@@ -99,8 +99,7 @@ public class CameraCapability extends HomeSecurityThingCapability {
         vpnUrl = newVpnUrl;
         if (!SdCardStatus.SD_CARD_WORKING.equals(newData.getSdStatus())) {
             statusReason = newData.getSdStatus().toString();
-        }
+        } else if (!AlimentationStatus.ALIM_CORRECT_POWER.equals(newData.getAlimStatus())) {
-        if (!AlimentationStatus.ALIM_CORRECT_POWER.equals(newData.getAlimStatus())) {
             statusReason = newData.getAlimStatus().toString();
         }
     }

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/Capability.java

@@ -105,46 +105,46 @@ public class Capability {
         statusReason = null;
     }
 
-    protected void afterNewData(@Nullable NAObject newData) {
+    protected void afterNewData(@SuppressWarnings("unused") @Nullable NAObject newData) {
         if (!properties.equals(getThing().getProperties())) {
             getThing().setProperties(properties);
         }
         firstLaunch = false;
     }
 
-    protected void updateNAThing(NAThing newData) {
+    protected void updateNAThing(@SuppressWarnings("unused") NAThing newData) {
         // do nothing by default, can be overridden by subclasses
     }
 
-    protected void updateNAMain(NAMain newData) {
+    protected void updateNAMain(@SuppressWarnings("unused") NAMain newData) {
         // do nothing by default, can be overridden by subclasses
     }
 
-    protected void updateHomeEvent(HomeEvent newData) {
+    protected void updateHomeEvent(@SuppressWarnings("unused") HomeEvent newData) {
         // do nothing by default, can be overridden by subclasses
     }
 
-    protected void updateHomeStatus(HomeStatus newData) {
+    protected void updateHomeStatus(@SuppressWarnings("unused") HomeStatus newData) {
         // do nothing by default, can be overridden by subclasses
     }
 
-    protected void updateHomeData(HomeData newData) {
+    protected void updateHomeData(@SuppressWarnings("unused") HomeData newData) {
         // do nothing by default, can be overridden by subclasses
     }
 
-    protected void updateEvent(Event newData) {
+    protected void updateEvent(@SuppressWarnings("unused") Event newData) {
         // do nothing by default, can be overridden by subclasses
     }
 
-    protected void updateWebhookEvent(WebhookEvent newData) {
+    protected void updateWebhookEvent(@SuppressWarnings("unused") WebhookEvent newData) {
         // do nothing by default, can be overridden by subclasses
     }
 
-    protected void updateNADevice(Device newData) {
+    protected void updateNADevice(@SuppressWarnings("unused") Device newData) {
         // do nothing by default, can be overridden by subclasses
     }
 
-    protected void updateErrors(NAError error) {
+    protected void updateErrors(@SuppressWarnings("unused") NAError error) {
         // do nothing by default, can be overridden by subclasses
     }
 
@@ -163,11 +163,12 @@ public class Capability {
         // do nothing by default, can be overridden by subclasses
     }
 
-    public void updateHomeStatusModule(HomeStatusModule newData) {
+    public void updateHomeStatusModule(@SuppressWarnings("unused") HomeStatusModule newData) {
         // do nothing by default, can be overridden by subclasses
     }
 
-    public void handleCommand(String channelName, Command command) {
+    public void handleCommand(@SuppressWarnings("unused") String channelName,
+            @SuppressWarnings("unused") Command command) {
         // do nothing by default, can be overridden by subclasses
     }
 

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/HomeSecurityThingCapability.java

@@ -53,8 +53,7 @@ public abstract class HomeSecurityThingCapability extends Capability {
     protected Optional<SecurityCapability> getSecurityCapability() {
         if (securityCapability == null) {
             handler.getHomeCapability(SecurityCapability.class).ifPresent(cap -> securityCapability = cap);
-            ApiBridgeHandler accountHandler = handler.getAccountHandler();
+            if (handler.getAccountHandler() instanceof ApiBridgeHandler accountHandler) {
-            if (accountHandler != null) {
                 webhookServlet = null;
                 accountHandler.getWebHookServlet().ifPresent(servlet -> {
                     webhookServlet = servlet;
@@ -74,10 +73,14 @@ public abstract class HomeSecurityThingCapability extends Capability {
 
     @Override
     public void dispose() {
-        WebhookServlet webhook = this.webhookServlet;
+        if (webhookServlet instanceof WebhookServlet webhook) {
-        if (webhook != null) {
             webhook.unregisterDataListener(handler.getId());
+            webhookServlet = null;
         }
         super.dispose();
     }
+
+    protected boolean pullMode() {
+        return webhookServlet == null;
+    }
 }

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/PersonCapability.java

@@ -106,12 +106,14 @@ public class PersonCapability extends HomeSecurityThingCapability {
     @Override
     public List<NAObject> updateReadings() {
         List<NAObject> result = new ArrayList<>();
-        getSecurityCapability().ifPresent(cap -> {
+        if (pullMode()) {
-            HomeEvent event = cap.getLastPersonEvent(handler.getId());
+            getSecurityCapability().ifPresent(cap -> {
-            if (event != null) {
+                HomeEvent event = cap.getLastPersonEvent(handler.getId());
-                result.add(event);
+                if (event != null) {
-            }
+                    result.add(event);
-        });
+                }
+            });
+        }
         return result;
     }
 }

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshAutoCapability.java

@@ -36,7 +36,7 @@ public class RefreshAutoCapability extends RefreshCapability {
 
     private final Logger logger = LoggerFactory.getLogger(RefreshAutoCapability.class);
 
-    private Instant dataTimeStamp = Instant.MIN;
+    private @Nullable Instant dataTimestamp = null;
 
     public RefreshAutoCapability(CommonInterface handler) {
         super(handler);
@@ -44,38 +44,38 @@ public class RefreshAutoCapability extends RefreshCapability {
 
     @Override
     public void expireData() {
-        dataTimeStamp = Instant.MIN;
+        dataTimestamp = null;
         super.expireData();
     }
 
     @Override
     protected Duration calcDelay() {
-        if (Instant.MIN.equals(dataTimeStamp)) {
+        Instant timestamp = dataTimestamp;
+        if (timestamp == null) {
             return PROBING_INTERVAL;
         }
 
-        Duration dataAge = Duration.between(dataTimeStamp, Instant.now());
+        Duration dataAge = Duration.between(timestamp, Instant.now());
-
         Duration delay = dataValidity.minus(dataAge);
-        if (delay.isNegative() || delay.isZero()) {
+
-            logger.debug("{} did not update data in expected time, return to probing", thingUID);
+        if (delay.isPositive()) {
-            dataTimeStamp = Instant.MIN;
+            return delay.plus(DEFAULT_DELAY);
-            return PROBING_INTERVAL;
         }
 
-        return delay.plus(DEFAULT_DELAY);
+        logger.debug("{} did not update data in expected time, return to probing", thingUID);
+        dataTimestamp = null;
+        return PROBING_INTERVAL;
     }
 
     @Override
     protected void updateNAThing(NAThing newData) {
         super.updateNAThing(newData);
-        ZonedDateTime lastSeen = newData.getLastSeen();
+        dataTimestamp = newData.getLastSeen() instanceof ZonedDateTime ls ? ls.toInstant() : null;
-        dataTimeStamp = lastSeen != null ? lastSeen.toInstant() : Instant.MIN;
     }
 
     @Override
     protected void afterNewData(@Nullable NAObject newData) {
-        properties.put("probing", Boolean.valueOf(Instant.MIN.equals(dataTimeStamp)).toString());
+        properties.put("probing", Boolean.valueOf(dataTimestamp == null).toString());
         super.afterNewData(newData);
     }
 }

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshCapability.java

@@ -21,6 +21,7 @@ import java.util.concurrent.TimeUnit;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.netatmo.internal.api.dto.NAObject;
+import org.openhab.binding.netatmo.internal.handler.ApiBridgeHandler;
 import org.openhab.binding.netatmo.internal.handler.CommonInterface;
 import org.openhab.core.thing.ThingStatus;
 import org.slf4j.Logger;
@@ -35,7 +36,7 @@ import org.slf4j.LoggerFactory;
  */
 @NonNullByDefault
 public class RefreshCapability extends Capability {
-    protected static final Duration ASAP = Duration.ofSeconds(2);
+    public static final Duration ASAP = Duration.ofSeconds(2);
     protected static final Duration OFFLINE_DELAY = Duration.ofMinutes(15);
     protected static final Duration PROBING_INTERVAL = Duration.ofMinutes(2);
 
@@ -50,7 +51,7 @@ public class RefreshCapability extends Capability {
     }
 
     public void setInterval(Duration dataValidity) {
-        if (dataValidity.isNegative() || dataValidity.isZero()) {
+        if (!dataValidity.isPositive()) {
             throw new IllegalArgumentException("refreshInterval must be positive");
         }
         this.dataValidity = dataValidity;
@@ -84,9 +85,14 @@ public class RefreshCapability extends Capability {
     private void proceedWithUpdate() {
         Duration delay;
         handler.proceedWithUpdate();
-        if (!ThingStatus.ONLINE.equals(handler.getThing().getStatus())) {
+        if (handler.getAccountHandler() instanceof ApiBridgeHandler accountHandler
+                && !ThingStatus.ONLINE.equals(accountHandler.getThing().getStatus())) {
+            delay = accountHandler.getIdleTime();
+            delay = delay != null ? delay.plus(ASAP) : OFFLINE_DELAY;
+            logger.debug("Bridge is not ONLINE, will wait for him to come-back in {}", delay);
+        } else if (!ThingStatus.ONLINE.equals(handler.getThing().getStatus())) {
             delay = OFFLINE_DELAY;
-            logger.debug("Thing '{}' is not ONLINE, using special refresh interval", thingUID);
+            logger.debug("Thing '{}' is not ONLINE, special refresh interval {} used", thingUID, delay);
         } else {
             delay = calcDelay();
         }
@@ -110,10 +116,9 @@ public class RefreshCapability extends Capability {
     }
 
     private void stopJob() {
-        ScheduledFuture<?> refreshJob = this.refreshJob;
+        if (refreshJob instanceof ScheduledFuture job) {
-        if (refreshJob != null) {
+            job.cancel(true);
-            refreshJob.cancel(true);
         }
-        this.refreshJob = null;
+        refreshJob = null;
     }
 }

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RestCapability.java

@@ -61,7 +61,7 @@ public abstract class RestCapability<T extends RestManager> extends DeviceCapabi
         return result;
     }
 
-    protected List<NAObject> updateReadings(T api) {
+    protected List<NAObject> updateReadings(@SuppressWarnings("unused") T api) {
         return List.of();
     }
 

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/channelhelper/ChannelHelper.java

@@ -84,26 +84,32 @@ public abstract class ChannelHelper {
         return result;
     }
 
+    @SuppressWarnings("unused")
     protected @Nullable State internalGetObject(String channelId, NAObject localData) {
         return null;
     }
 
+    @SuppressWarnings("unused")
     protected @Nullable State internalGetOther(String channelId) {
         return null;
     }
 
+    @SuppressWarnings("unused")
     protected @Nullable State internalGetDashboard(String channelId, Dashboard dashboard) {
         return null;
     }
 
+    @SuppressWarnings("unused")
     protected @Nullable State internalGetProperty(String channelId, NAThing naThing, Configuration config) {
         return null;
     }
 
+    @SuppressWarnings("unused")
     protected @Nullable State internalGetEvent(String channelId, Event event) {
         return null;
     }
 
+    @SuppressWarnings("unused")
     protected @Nullable State internalGetHomeEvent(String channelId, @Nullable String groupId, HomeEvent event) {
         return null;
     }

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/servlet/NetatmoServlet.java

@@ -32,18 +32,17 @@ import org.slf4j.LoggerFactory;
 @NonNullByDefault
 public abstract class NetatmoServlet extends HttpServlet {
     private static final long serialVersionUID = 5671438863935117735L;
-    private static final String BASE_PATH = "/" + BINDING_ID + "/";
 
-    private final Logger logger = LoggerFactory.getLogger(this.getClass());
+    private final Logger logger = LoggerFactory.getLogger(NetatmoServlet.class);
     private final HttpService httpService;
     private final String path;
 
     protected final ApiBridgeHandler handler;
 
     public NetatmoServlet(ApiBridgeHandler handler, HttpService httpService, String localPath) {
-        this.path = BASE_PATH + localPath + "/" + handler.getId();
+        this.path = "/" + BINDING_ID + "/" + localPath + "/" + handler.getId();
-        this.handler = handler;
         this.httpService = httpService;
+        this.handler = handler;
     }
 
     public void startListening() {

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/servlet/WebhookServlet.java

@@ -121,8 +121,7 @@ public class WebhookServlet extends NetatmoServlet {
 
     private void notifyListeners(WebhookEvent event) {
         event.getNAObjectList().forEach(id -> {
-            Capability module = dataListeners.get(id);
+            if (dataListeners.get(id) instanceof Capability module) {
-            if (module != null) {
                 logger.trace("Dispatching webhook event to {}", id);
                 module.setNewData(event);
             }

bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/i18n/netatmo.properties

@@ -500,6 +500,7 @@ data-over-limit = Data seems quite old
 request-time-out = Request timed out - will attempt reconnection later
 deserialization-unknown = Deserialization lead to an unknown code
 maximum-usage-reached = Maximum usage reached, will reconnect in {0} seconds.
+concurrency-limit-timed-out = Concurrency limited section, will retry in {0} seconds.
 homestatus-unknown-error = Unknown error
 homestatus-internal-error = Internal error
 homestatus-parser-error = Parser error