From 2206e8c12880662524b5e6ba6468147ffb0fb1a4 Mon Sep 17 00:00:00 2001 From: clinique Date: Wed, 13 Dec 2023 17:04:38 +0100 Subject: [PATCH 1/3] Close all jobs Signed-off-by: clinique --- .../internal/handler/ApiBridgeHandler.java | 16 +++--- .../internal/handler/CommonInterface.java | 31 +++-------- .../handler/capability/CapabilityMap.java | 10 ++++ .../capability/ParentUpdateCapability.java | 52 +++++++++++++++++++ .../handler/capability/RefreshCapability.java | 11 ++-- 5 files changed, 82 insertions(+), 38 deletions(-) create mode 100644 bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java index 340059da07747..4dec67d208156 100644 --- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java @@ -293,7 +293,7 @@ public void handleCommand(ChannelUID channelUID, Command command) { logger.info("Unable to instantiate {}, expected scope {} is not active", clazz, expected); } } catch (SecurityException | ReflectiveOperationException e) { - logger.warn("Error invoking RestManager constructor for class {} : {}", clazz, e.getMessage()); + logger.warn("Error invoking RestManager constructor for class {}: {}", clazz, e.getMessage()); } } return (T) managers.get(clazz); @@ -319,7 +319,7 @@ public synchronized T executeUri(URI uri, HttpMethod method, Class clazz, request.content(inputStreamContentProvider, contentType); request.header(HttpHeader.ACCEPT, "application/json"); } - logger.trace(" -with payload : {} ", payload); + logger.trace(" -with payload: {} ", payload); } if (isLinked(requestCountChannelUID)) { @@ -331,13 +331,13 @@ public synchronized T executeUri(URI uri, HttpMethod method, Class clazz, } updateState(requestCountChannelUID, new DecimalType(requestsTimestamps.size())); } - logger.trace(" -with headers : {} ", + logger.trace(" -with headers: {} ", String.join(", ", request.getHeaders().stream().map(HttpField::toString).toList())); ContentResponse response = request.send(); Code statusCode = HttpStatus.getCode(response.getStatus()); String responseBody = new String(response.getContent(), StandardCharsets.UTF_8); - logger.trace(" -returned : code {} body {}", statusCode, responseBody); + logger.trace(" -returned: code {} body {}", statusCode, responseBody); if (statusCode == Code.OK) { return deserializer.deserialize(clazz, responseBody); @@ -347,7 +347,7 @@ public synchronized T executeUri(URI uri, HttpMethod method, Class clazz, try { exception = new NetatmoException(deserializer.deserialize(ApiError.class, responseBody)); } catch (NetatmoException e) { - exception = new NetatmoException("Error deserializing error : %s".formatted(statusCode.getMessage())); + exception = new NetatmoException("Error deserializing error: %s".formatted(statusCode.getMessage())); } throw exception; } catch (NetatmoException e) { @@ -359,10 +359,10 @@ public synchronized T executeUri(URI uri, HttpMethod method, Class clazz, } catch (InterruptedException e) { Thread.currentThread().interrupt(); updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage()); - throw new NetatmoException(String.format("%s: \"%s\"", e.getClass().getName(), e.getMessage())); + throw new NetatmoException("Request interrupted"); } catch (TimeoutException | ExecutionException e) { if (retryCount > 0) { - logger.debug("Request timedout, retry counter : {}", retryCount); + logger.debug("Request timedout, retry counter: {}", retryCount); return executeUri(uri, method, clazz, payload, contentType, retryCount - 1); } updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "@text/request-time-out"); @@ -427,7 +427,7 @@ public void identifyAllModulesAndApplyAction(BiFunction { - CommonInterface bridgeHandler = getBridgeHandler(); - if (bridgeHandler != null) { - bridgeHandler.expireData(); - } - }, 1, TimeUnit.SECONDS); + if (ModuleType.ACCOUNT.equals(getModuleType().getBridge())) { + NAThingConfiguration config = getThing().getConfiguration().as(NAThingConfiguration.class); + getCapabilities().put(new RefreshCapability(this, config.refreshInterval)); + } + getCapabilities().put(new ParentUpdateCapability(this)); } } @@ -237,20 +236,6 @@ default ModuleType getModuleType() { return ModuleType.from(getThing().getThingTypeUID()); } - default void setRefreshCapability() { - if (ModuleType.ACCOUNT.equals(getModuleType().getBridge())) { - NAThingConfiguration config = getThing().getConfiguration().as(NAThingConfiguration.class); - getCapabilities().put(new RefreshCapability(this, getScheduler(), config.refreshInterval)); - } - } - - default void removeRefreshCapability() { - Capability refreshCap = getCapabilities().remove(RefreshCapability.class); - if (refreshCap != null) { - refreshCap.dispose(); - } - } - default void commonDispose() { getCapabilities().values().forEach(Capability::dispose); } diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CapabilityMap.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CapabilityMap.java index 033d84b57cc8d..9cb6e58661ac9 100644 --- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CapabilityMap.java +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CapabilityMap.java @@ -16,6 +16,7 @@ import java.util.concurrent.ConcurrentHashMap; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; /** * {@link CapabilityMap} is a specialized Map designed to store capabilities @@ -40,4 +41,13 @@ public Optional get(Class clazz) { T cap = (T) super.get(clazz); return Optional.ofNullable(cap); } + + public void remove(Class clazz) { + @SuppressWarnings("unchecked") + @Nullable + T cap = (T) super.remove(clazz); + if (cap != null) { + cap.dispose(); + } + } } diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java new file mode 100644 index 0000000000000..43fae4af34c0b --- /dev/null +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java @@ -0,0 +1,52 @@ +/** + * Copyright (c) 2010-2023 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.netatmo.internal.handler.capability; + +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.openhab.binding.netatmo.internal.handler.CommonInterface; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * {@link ParentUpdateCapability} is the class used to request data update upon initialization of a module + * + * @author Gaël L'hopital - Initial contribution + * + */ +@NonNullByDefault +public class ParentUpdateCapability extends Capability { + private static final int DEFAULT_DELAY_S = 2; + + private final Logger logger = LoggerFactory.getLogger(ParentUpdateCapability.class); + private final ScheduledFuture job; + + public ParentUpdateCapability(CommonInterface handler) { + super(handler); + this.job = handler.getScheduler().schedule(() -> { + logger.debug("Requesting parents data refresh for Thing {}", handler.getId()); + CommonInterface bridgeHandler = handler.getBridgeHandler(); + if (bridgeHandler != null) { + bridgeHandler.expireData(); + } + }, DEFAULT_DELAY_S, TimeUnit.SECONDS); + } + + @Override + public void dispose() { + job.cancel(true); + super.dispose(); + } +} diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshCapability.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshCapability.java index 258f10748eb47..adad77b9619f5 100644 --- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshCapability.java +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshCapability.java @@ -18,7 +18,6 @@ import java.time.Instant; import java.time.ZonedDateTime; import java.util.Optional; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -42,7 +41,6 @@ public class RefreshCapability extends Capability { private static final Duration OFFLINE_INTERVAL = Duration.of(15, MINUTES); private final Logger logger = LoggerFactory.getLogger(RefreshCapability.class); - private final ScheduledExecutorService scheduler; private Duration dataValidity; private Instant dataTimeStamp = Instant.now(); @@ -50,9 +48,8 @@ public class RefreshCapability extends Capability { private Optional> refreshJob = Optional.empty(); private boolean refreshConfigured; - public RefreshCapability(CommonInterface handler, ScheduledExecutorService scheduler, int refreshInterval) { + public RefreshCapability(CommonInterface handler, int refreshInterval) { super(handler); - this.scheduler = scheduler; this.dataValidity = Duration.ofSeconds(Math.max(0, refreshInterval)); this.refreshConfigured = !probing(); freeJobAndReschedule(2); @@ -117,8 +114,8 @@ protected void updateNAThing(NAThing newData) { } private void freeJobAndReschedule(long delay) { - refreshJob.ifPresent(job -> job.cancel(false)); - refreshJob = Optional - .ofNullable(delay == 0 ? null : scheduler.schedule(() -> proceedWithUpdate(), delay, TimeUnit.SECONDS)); + refreshJob.ifPresent(job -> job.cancel(true)); + refreshJob = Optional.ofNullable(delay == 0 ? null + : handler.getScheduler().schedule(() -> proceedWithUpdate(), delay, TimeUnit.SECONDS)); } } From 71187f6d447fe51e71120da0e776c2cd4cf06bae Mon Sep 17 00:00:00 2001 From: clinique Date: Fri, 15 Dec 2023 13:57:39 +0100 Subject: [PATCH 2/3] Small code refining Signed-off-by: clinique --- .../handler/capability/CapabilityMap.java | 5 ++--- .../capability/ParentUpdateCapability.java | 17 +++++++++++------ .../handler/capability/RefreshCapability.java | 6 +++++- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CapabilityMap.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CapabilityMap.java index 9cb6e58661ac9..fe48db190c7fd 100644 --- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CapabilityMap.java +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CapabilityMap.java @@ -42,10 +42,9 @@ public Optional get(Class clazz) { return Optional.ofNullable(cap); } - public void remove(Class clazz) { - @SuppressWarnings("unchecked") + public void remove(Class clazz) { @Nullable - T cap = (T) super.remove(clazz); + Capability cap = super.remove(clazz); if (cap != null) { cap.dispose(); } diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java index 43fae4af34c0b..0cbcca446e9e8 100644 --- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java @@ -12,6 +12,7 @@ */ package org.openhab.binding.netatmo.internal.handler.capability; +import java.util.Optional; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -31,22 +32,26 @@ public class ParentUpdateCapability extends Capability { private static final int DEFAULT_DELAY_S = 2; private final Logger logger = LoggerFactory.getLogger(ParentUpdateCapability.class); - private final ScheduledFuture job; + private Optional> job = Optional.empty(); public ParentUpdateCapability(CommonInterface handler) { super(handler); - this.job = handler.getScheduler().schedule(() -> { + } + + @Override + public void initialize() { + job = Optional.of(handler.getScheduler().schedule(() -> { logger.debug("Requesting parents data refresh for Thing {}", handler.getId()); - CommonInterface bridgeHandler = handler.getBridgeHandler(); - if (bridgeHandler != null) { + if (handler.getBridgeHandler() instanceof CommonInterface bridgeHandler) { bridgeHandler.expireData(); } - }, DEFAULT_DELAY_S, TimeUnit.SECONDS); + }, DEFAULT_DELAY_S, TimeUnit.SECONDS)); } @Override public void dispose() { - job.cancel(true); + job.ifPresent(j -> j.cancel(true)); + job = Optional.empty(); super.dispose(); } } diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshCapability.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshCapability.java index adad77b9619f5..11750ec6c3fc0 100644 --- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshCapability.java +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshCapability.java @@ -51,6 +51,10 @@ public class RefreshCapability extends Capability { public RefreshCapability(CommonInterface handler, int refreshInterval) { super(handler); this.dataValidity = Duration.ofSeconds(Math.max(0, refreshInterval)); + } + + @Override + public void initialize() { this.refreshConfigured = !probing(); freeJobAndReschedule(2); } @@ -106,7 +110,7 @@ protected void updateNAThing(NAThing newData) { refreshConfigured = true; logger.debug("Data validity period identified to be {}", dataValidity); } else { - logger.debug("Data validity period not yet found - data timestamp unchanged"); + logger.debug("Data validity period not yet found, data timestamp unchanged"); } } dataTimeStamp = tsInstant; From 6870293f4c4ef189108019e56f4585b636662892 Mon Sep 17 00:00:00 2001 From: clinique Date: Fri, 15 Dec 2023 14:32:21 +0100 Subject: [PATCH 3/3] Correcting SAT error Signed-off-by: clinique --- .../internal/handler/capability/ParentUpdateCapability.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java index 0cbcca446e9e8..ce79a99deb19f 100644 --- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/ParentUpdateCapability.java @@ -41,8 +41,9 @@ public ParentUpdateCapability(CommonInterface handler) { @Override public void initialize() { job = Optional.of(handler.getScheduler().schedule(() -> { - logger.debug("Requesting parents data refresh for Thing {}", handler.getId()); - if (handler.getBridgeHandler() instanceof CommonInterface bridgeHandler) { + logger.debug("Requesting parents data update for Thing {}", handler.getId()); + CommonInterface bridgeHandler = handler.getBridgeHandler(); + if (bridgeHandler != null) { bridgeHandler.expireData(); } }, DEFAULT_DELAY_S, TimeUnit.SECONDS));