[harmonyhub] Fix reliability issues (#13702)

* Fix compiler info

Unsafe interpretation of method return type as '@NonNull' based on the receiver type 'java.util.Enumeration<java.net.@NonNull NetworkInterface>'. Type 'java.util.Enumeration<E>' doesn't seem to be designed with null type annotations in mind

* Improve robustness of job rescheduling and handler disposal

Handler tried updating thing although the handler was already disposed

Fixes #13701

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

diff --git a/bundles/org.openhab.binding.harmonyhub/src/main/java/org/openhab/binding/harmonyhub/internal/discovery/HarmonyHubDiscoveryService.java b/bundles/org.openhab.binding.harmonyhub/src/main/java/org/openhab/binding/harmonyhub/internal/discovery/HarmonyHubDiscoveryService.java
index 1a1c5f1fb95480823696faa7fceef17e8291ad01..4fcd9c2348101ff2b952875684b29c21aad71ed4 100644 (file)
--- a/bundles/org.openhab.binding.harmonyhub/src/main/java/org/openhab/binding/harmonyhub/internal/discovery/HarmonyHubDiscoveryService.java
+++ b/bundles/org.openhab.binding.harmonyhub/src/main/java/org/openhab/binding/harmonyhub/internal/discovery/HarmonyHubDiscoveryService.java
@@ -171,6 +171,7 @@ public class HarmonyHubDiscoveryService extends AbstractDiscoveryService {
             // Broadcast the message over all the network interfaces
             Enumeration<NetworkInterface> interfaces = NetworkInterface.getNetworkInterfaces();
             while (interfaces.hasMoreElements()) {
+                @Nullable
                 NetworkInterface networkInterface = interfaces.nextElement();
                 if (networkInterface.isLoopback() || !networkInterface.isUp()) {
                     continue;

diff --git a/bundles/org.openhab.binding.harmonyhub/src/main/java/org/openhab/binding/harmonyhub/internal/handler/HarmonyHubHandler.java b/bundles/org.openhab.binding.harmonyhub/src/main/java/org/openhab/binding/harmonyhub/internal/handler/HarmonyHubHandler.java
index 4974583eda5b5331a72d7b3ad2d9b7e01c6a15ff..8f317dfd5330b2ed899263534b3c7dea9c7bfbd0 100644 (file)
--- a/bundles/org.openhab.binding.harmonyhub/src/main/java/org/openhab/binding/harmonyhub/internal/handler/HarmonyHubHandler.java
+++ b/bundles/org.openhab.binding.harmonyhub/src/main/java/org/openhab/binding/harmonyhub/internal/handler/HarmonyHubHandler.java
@@ -22,7 +22,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
@@ -75,7 +75,7 @@ public class HarmonyHubHandler extends BaseBridgeHandler implements HarmonyClien
 
     private final Logger logger = LoggerFactory.getLogger(HarmonyHubHandler.class);
 
-    public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Collections.singleton(HARMONY_HUB_THING_TYPE);
+    public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Set.of(HARMONY_HUB_THING_TYPE);
 
     private static final Comparator<Activity> ACTIVITY_COMPERATOR = Comparator.comparing(Activity::getActivityOrder,
             Comparator.nullsFirst(Integer::compareTo));
@@ -84,7 +84,7 @@ public class HarmonyHubHandler extends BaseBridgeHandler implements HarmonyClien
     private static final int HEARTBEAT_INTERVAL = 30;
     // Websocket will timeout after 60 seconds, pick a sensible max under this,
     private static final int HEARTBEAT_INTERVAL_MAX = 50;
-    private List<HubStatusListener> listeners = new CopyOnWriteArrayList<>();
+    private Set<HubStatusListener> listeners = ConcurrentHashMap.newKeySet();
     private final HarmonyHubHandlerFactory factory;
     private @NonNullByDefault({}) HarmonyHubConfig config;
     private final HarmonyClient client;
@@ -184,7 +184,6 @@ public class HarmonyHubHandler extends BaseBridgeHandler implements HarmonyClien
     @Override
     public void initialize() {
         config = getConfigAs(HarmonyHubConfig.class);
-        cancelRetry();
         updateStatus(ThingStatus.UNKNOWN);
         scheduleRetry(0);
     }
@@ -294,29 +293,31 @@ public class HarmonyHubHandler extends BaseBridgeHandler implements HarmonyClien
     }
 
     private void disconnectFromHub() {
-        ScheduledFuture<?> localHeartBeatJob = heartBeatJob;
-        if (localHeartBeatJob != null && !localHeartBeatJob.isDone()) {
-            localHeartBeatJob.cancel(false);
+        ScheduledFuture<?> heartBeatJob = this.heartBeatJob;
+        if (heartBeatJob != null) {
+            heartBeatJob.cancel(true);
+            this.heartBeatJob = null;
         }
         client.disconnect();
     }
 
     private void setOfflineAndReconnect(String error) {
         disconnectFromHub();
-        scheduleRetry(RETRY_TIME);
         updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, error);
+        scheduleRetry(RETRY_TIME);
     }
 
     private void cancelRetry() {
-        ScheduledFuture<?> localRetryJob = retryJob;
-        if (localRetryJob != null && !localRetryJob.isDone()) {
-            localRetryJob.cancel(false);
+        ScheduledFuture<?> retryJob = this.retryJob;
+        if (retryJob != null) {
+            retryJob.cancel(true);
+            this.retryJob = null;
         }
     }
 
-    private synchronized void scheduleRetry(int retrySeconds) {
+    private synchronized void scheduleRetry(int delaySeconds) {
         cancelRetry();
-        retryJob = scheduler.schedule(this::connect, retrySeconds, TimeUnit.SECONDS);
+        retryJob = scheduler.schedule(this::connect, delaySeconds, TimeUnit.SECONDS);
     }
 
     private void updateState(@Nullable Activity activity) {