[deconz] improve websocket connection process (#9298)

* improve websocket connection

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>

diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBridgeHandler.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBridgeHandler.java
index cc19c64f4ad9a3cfa80cfdff7605b72da06b510f..7955e11fed33fc72bd37084c400e897dd94933d2 100644 (file)
--- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBridgeHandler.java
+++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBridgeHandler.java
@@ -232,10 +232,11 @@ public class DeconzBridgeHandler extends BaseBridgeHandler implements WebSocketC
 
     /**
      * Starts the websocket connection.
+     *
      * {@link #requestFullState} need to be called first to obtain the websocket port.
      */
     private void startWebsocket() {
-        if (websocket.isConnected() || websocketPort == 0) {
+        if (websocket.isConnected() || websocketPort == 0 || websocketReconnect == false) {
             return;
         }
 
@@ -288,9 +289,7 @@ public class DeconzBridgeHandler extends BaseBridgeHandler implements WebSocketC
         }
         stopTimer();
         // Wait for POLL_FREQUENCY_SEC after a connection error before trying again
-        if (websocketReconnect) {
-            scheduledFuture = scheduler.schedule(this::startWebsocket, POLL_FREQUENCY_SEC, TimeUnit.SECONDS);
-        }
+        scheduledFuture = scheduler.schedule(this::startWebsocket, POLL_FREQUENCY_SEC, TimeUnit.SECONDS);
     }
 
     @Override
@@ -302,9 +301,7 @@ public class DeconzBridgeHandler extends BaseBridgeHandler implements WebSocketC
     @Override
     public void connectionLost(String reason) {
         updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, reason);
-        if (websocketReconnect) {
-            startWebsocket();
-        }
+        startWebsocket();
     }
 
     /**

diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/netutils/WebSocketConnection.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/netutils/WebSocketConnection.java
index 703276d08c8c8e425902fb2917638a9c374e95f1..03801bd011356336411ad8c764abb696b9112df0 100644 (file)
--- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/netutils/WebSocketConnection.java
+++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/netutils/WebSocketConnection.java
@@ -17,6 +17,7 @@ import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jetty.util.thread.QueuedThreadPool;
 import org.eclipse.jetty.websocket.api.Session;
 import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose;
 import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
@@ -51,29 +52,33 @@ public class WebSocketConnection {
     private final Logger logger = LoggerFactory.getLogger(WebSocketConnection.class);
 
     private final WebSocketClient client;
+    private final String socketName;
+    private final Gson gson;
+
     private final WebSocketConnectionListener connectionListener;
     private final Map<Map.Entry<ResourceType, String>, WebSocketMessageListener> listeners = new ConcurrentHashMap<>();
 
-    private final Gson gson;
-    private boolean connected = false;
+    private ConnectionState connectionState = ConnectionState.DISCONNECTED;
 
     public WebSocketConnection(WebSocketConnectionListener listener, WebSocketClient client, Gson gson) {
         this.connectionListener = listener;
         this.client = client;
         this.client.setMaxIdleTimeout(0);
         this.gson = gson;
+        this.socketName = ((QueuedThreadPool) client.getExecutor()).getName() + "$" + this.hashCode();
     }
 
     public void start(String ip) {
-        if (connected) {
+        if (connectionState == ConnectionState.CONNECTED) {
+            return;
+        } else if (connectionState == ConnectionState.CONNECTING) {
+            logger.debug("{} already connecting", socketName);
             return;
         }
         try {
             URI destUri = URI.create("ws://" + ip);
-
             client.start();
-
-            logger.debug("Connecting to: {}", destUri);
+            logger.debug("Trying to connect {} to {}", socketName, destUri);
             client.connect(this, destUri).get();
         } catch (Exception e) {
             connectionListener.connectionError(e);
@@ -82,10 +87,10 @@ public class WebSocketConnection {
 
     public void close() {
         try {
-            connected = false;
+            connectionState = ConnectionState.DISCONNECTING;
             client.stop();
         } catch (Exception e) {
-            logger.debug("Error while closing connection", e);
+            logger.debug("{} encountered an error while closing connection", socketName, e);
         }
         client.destroy();
     }
@@ -98,17 +103,18 @@ public class WebSocketConnection {
         listeners.remove(Map.entry(resourceType, sensorID));
     }
 
+    @SuppressWarnings("unused")
     @OnWebSocketConnect
     public void onConnect(Session session) {
-        connected = true;
-        logger.debug("Connect: {}", session.getRemoteAddress().getAddress());
+        connectionState = ConnectionState.CONNECTED;
+        logger.debug("{} successfully connected to {}", socketName, session.getRemoteAddress().getAddress());
         connectionListener.connectionEstablished();
     }
 
-    @SuppressWarnings("null")
+    @SuppressWarnings("null, unused")
     @OnWebSocketMessage
     public void onMessage(String message) {
-        logger.trace("Raw data received by websocket: {}", message);
+        logger.trace("Raw data received by websocket {}: {}", socketName, message);
 
         DeconzBaseMessage changedMessage = gson.fromJson(message, DeconzBaseMessage.class);
         if (changedMessage.r == ResourceType.UNKNOWN) {
@@ -137,19 +143,36 @@ public class WebSocketConnection {
         }
     }
 
+    @SuppressWarnings("unused")
     @OnWebSocketError
     public void onError(Throwable cause) {
-        connected = false;
+        connectionState = ConnectionState.DISCONNECTED;
         connectionListener.connectionError(cause);
     }
 
+    @SuppressWarnings("unused")
     @OnWebSocketClose
     public void onClose(int statusCode, String reason) {
-        connected = false;
+        connectionState = ConnectionState.DISCONNECTED;
         connectionListener.connectionLost(reason);
     }
 
+    /**
+     * check connection state (successfully connected)
+     *
+     * @return true if connected, false if connecting, disconnecting or disconnected
+     */
     public boolean isConnected() {
-        return connected;
+        return connectionState == ConnectionState.CONNECTED;
+    }
+
+    /**
+     * used internally to represent the connection state
+     */
+    private enum ConnectionState {
+        CONNECTING,
+        CONNECTED,
+        DISCONNECTING,
+        DISCONNECTED
     }
 }