[ipcamera] Fix connection checks with ONVIF cameras with no snapshots (#15119)

* Added connection check via IdleStateHandler events for sent onvif requests.
Also checking connect errors and setting new states connectError or refusedError accordingly.
On connect, only one request is sent to have less parallel actions in case of reconnect, timeout.
Moved GetCapabilities call to GetSystemDateAndTime response handler part.
Added support to disable automatic polling at startup.

* Fixed call of sendOnvifRequest (missed to remove one call of requestBuilder).

---------

Signed-off-by: Thomas Burri <th@thonojato.ch>
Signed-off-by: Matthew Skinner <matt@pcmus.com>
Co-authored-by: Matthew Skinner <matt@pcmus.com>

diff --git a/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/handler/IpCameraHandler.java b/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/handler/IpCameraHandler.java
index e5f1702d8823fb74041cf9c634be5f085d469a2b..4af99acb981222e7cccee1aa033711c4e5d2d22c 100644 (file)
--- a/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/handler/IpCameraHandler.java
+++ b/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/handler/IpCameraHandler.java
@@ -521,6 +521,12 @@ public class IpCameraHandler extends BaseThingHandler {
             }
             return;// ffmpeg snapshot stream is still alive
         }
+
+        // if ONVIF cam also use connection state which is updated by regular messages to camera
+        if (thing.getThingTypeUID().getId().equals(ONVIF_THING) && snapshotUri.isEmpty() && onvifCamera.isConnected()) {
+            return;
+        }
+
         // Open a HTTP connection without sending any requests as we do not need a snapshot.
         Bootstrap localBootstrap = mainBootstrap;
         if (localBootstrap != null) {
@@ -659,7 +665,7 @@ public class IpCameraHandler extends BaseThingHandler {
                                     break;
                             }
                             ch.writeAndFlush(request);
-                        } else { // an error occured
+                        } else { // an error occurred
                             cameraCommunicationError(
                                     "Connection Timeout: Check your IP and PORT are correct and the camera can be reached.");
                         }
@@ -1417,9 +1423,16 @@ public class IpCameraHandler extends BaseThingHandler {
             return;
         }
         if (cameraConfig.getOnvifPort() > 0 && !onvifCamera.isConnected()) {
+            if (onvifCamera.isConnectError()) {
+                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Camera is not reachable");
+            } else if (onvifCamera.isRefusedError()) {
+                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                        "Camera refused connection on ONVIF ports.");
+            }
             logger.debug("About to connect to the IP Camera using the ONVIF PORT at IP:{}:{}", cameraConfig.getIp(),
                     cameraConfig.getOnvifPort());
             onvifCamera.connect(thing.getThingTypeUID().getId().equals(ONVIF_THING));
+            return;
         }
         if ("ffmpeg".equals(snapshotUri)) {
             snapshotIsFfmpeg();
@@ -1556,9 +1569,6 @@ public class IpCameraHandler extends BaseThingHandler {
                 if (!snapshotPolling) {
                     checkCameraConnection();
                 }
-                if (!onvifCamera.isConnected()) {
-                    onvifCamera.connect(true);
-                }
                 break;
             case INSTAR_THING:
                 if (!snapshotPolling) {
@@ -1758,6 +1768,9 @@ public class IpCameraHandler extends BaseThingHandler {
     }
 
     private void tryConnecting() {
+        int firstDelay = 4;
+        int normalDelay = 12; // doesn't make sense to have faster retry than CONNECT_TIMEOUT, which is 10 seconds, if
+                              // camera is off
         if (!thing.getThingTypeUID().getId().equals(GENERIC_THING)
                 && !thing.getThingTypeUID().getId().equals(DOORBIRD_THING) && cameraConfig.getOnvifPort() > 0) {
             onvifCamera = new OnvifConnection(this, cameraConfig.getIp() + ":" + cameraConfig.getOnvifPort(),
@@ -1765,8 +1778,16 @@ public class IpCameraHandler extends BaseThingHandler {
             onvifCamera.setSelectedMediaProfile(cameraConfig.getOnvifMediaProfile());
             // Only use ONVIF events if it is not an API camera.
             onvifCamera.connect(supportsOnvifEvents());
+
+            if (supportsOnvifEvents()) {
+                // it takes some time to try to retrieve the ONVIF snapshot and stream URLs and update internal members
+                // on first connect; if connection lost, doesn't make sense to poll to often
+                firstDelay = 12;
+                normalDelay = 30;
+            }
         }
-        cameraConnectionJob = threadPool.scheduleWithFixedDelay(this::pollingCameraConnection, 4, 8, TimeUnit.SECONDS);
+        cameraConnectionJob = threadPool.scheduleWithFixedDelay(this::pollingCameraConnection, firstDelay, normalDelay,
+                TimeUnit.SECONDS);
     }
 
     private boolean supportsOnvifEvents() {

diff --git a/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/onvif/OnvifCodec.java b/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/onvif/OnvifCodec.java
index 378a6b2e0418e58c594fdd02ae4f00c849b72bf9..a3f49a9ebb5ff2eadf59f7bd6daa85567d39b9a8 100644 (file)
--- a/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/onvif/OnvifCodec.java
+++ b/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/onvif/OnvifCodec.java
@@ -21,6 +21,7 @@ import io.netty.channel.ChannelDuplexHandler;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.http.HttpContent;
 import io.netty.handler.codec.http.LastHttpContent;
+import io.netty.handler.timeout.IdleStateEvent;
 import io.netty.util.CharsetUtil;
 import io.netty.util.ReferenceCountUtil;
 
@@ -58,6 +59,21 @@ public class OnvifCodec extends ChannelDuplexHandler {
         }
     }
 
+    @Override
+    public void userEventTriggered(@Nullable ChannelHandlerContext ctx, @Nullable Object evt) throws Exception {
+        if (ctx == null) {
+            return;
+        }
+        if (evt instanceof IdleStateEvent) {
+            IdleStateEvent e = (IdleStateEvent) evt;
+            logger.trace("IdleStateEvent received {}", e.state());
+            onvifConnection.setIsConnected(false);
+            ctx.close();
+        } else {
+            logger.trace("Other ONVIF netty channel event occured {}", evt);
+        }
+    }
+
     @Override
     public void exceptionCaught(@Nullable ChannelHandlerContext ctx, @Nullable Throwable cause) {
         if (ctx == null || cause == null) {

diff --git a/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/onvif/OnvifConnection.java b/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/onvif/OnvifConnection.java
index 9725dfcc6f622a6cc2e0e523af7bd473b3ea942e..70b830ab394b2f70b3e301f0753050d0d0ce94d7 100644 (file)
--- a/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/onvif/OnvifConnection.java
+++ b/bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/onvif/OnvifConnection.java
@@ -14,6 +14,7 @@ package org.openhab.binding.ipcamera.internal.onvif;
 
 import static org.openhab.binding.ipcamera.internal.IpCameraBindingConstants.*;
 
+import java.net.ConnectException;
 import java.net.InetSocketAddress;
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
@@ -50,6 +51,7 @@ import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelFutureListener;
 import io.netty.channel.ChannelInitializer;
 import io.netty.channel.ChannelOption;
+import io.netty.channel.ConnectTimeoutException;
 import io.netty.channel.EventLoopGroup;
 import io.netty.channel.nio.NioEventLoopGroup;
 import io.netty.channel.socket.SocketChannel;
@@ -127,6 +129,8 @@ public class OnvifConnection {
     private String imagingXAddr = "http://" + ipAddress + "/onvif/device_service";
     private String ptzXAddr = "http://" + ipAddress + "/onvif/ptz_service";
     private String subscriptionXAddr = "http://" + ipAddress + "/onvif/device_service";
+    private boolean connectError = false;
+    private boolean refusedError = false;
     private boolean isConnected = false;
     private int mediaProfileIndex = 0;
     private String snapshotUri = "";
@@ -310,24 +314,15 @@ public class OnvifConnection {
         } else if (message.contains("RenewResponse")) {
             sendOnvifRequest(RequestType.PullMessages, subscriptionXAddr);
         } else if (message.contains("GetSystemDateAndTimeResponse")) {// 1st to be sent.
-            connecting.lock();
-            try {
-                isConnected = true;
-            } finally {
-                connecting.unlock();
-            }
+            setIsConnected(true);
+            sendOnvifRequest(RequestType.GetCapabilities, deviceXAddr);
             parseDateAndTime(message);
             logger.debug("Openhabs UTC dateTime is:{}", getUTCdateTime());
         } else if (message.contains("GetCapabilitiesResponse")) {// 2nd to be sent.
             parseXAddr(message);
             sendOnvifRequest(RequestType.GetProfiles, mediaXAddr);
         } else if (message.contains("GetProfilesResponse")) {// 3rd to be sent.
-            connecting.lock();
-            try {
-                isConnected = true;
-            } finally {
-                connecting.unlock();
-            }
+            setIsConnected(true);
             parseProfiles(message);
             sendOnvifRequest(RequestType.GetSnapshotUri, mediaXAddr);
             sendOnvifRequest(RequestType.GetStreamUri, mediaXAddr);
@@ -562,7 +557,7 @@ public class OnvifConnection {
 
                 @Override
                 public void initChannel(SocketChannel socketChannel) throws Exception {
-                    socketChannel.pipeline().addLast("idleStateHandler", new IdleStateHandler(0, 0, 70));
+                    socketChannel.pipeline().addLast("idleStateHandler", new IdleStateHandler(20, 20, 20));
                     socketChannel.pipeline().addLast("HttpClientCodec", new HttpClientCodec());
                     socketChannel.pipeline().addLast("OnvifCodec", new OnvifCodec(getHandle()));
                 }
@@ -570,25 +565,36 @@ public class OnvifConnection {
             bootstrap = localBootstap;
         }
         if (!mainEventLoopGroup.isShuttingDown()) {
-            localBootstap.connect(new InetSocketAddress(ipAddress, extractPortFromUrl(xAddr)))
-                    .addListener(new ChannelFutureListener() {
+            bootstrap.connect(new InetSocketAddress(ipAddress, onvifPort)).addListener(new ChannelFutureListener() {
 
-                        @Override
-                        public void operationComplete(@Nullable ChannelFuture future) {
-                            if (future == null) {
-                                return;
-                            }
-                            if (future.isSuccess()) {
-                                Channel ch = future.channel();
-                                ch.writeAndFlush(request);
-                            } else { // an error occured
-                                logger.debug("Camera is not reachable when using xAddr:{}.", xAddr);
-                                if (isConnected) {
-                                    disconnect();
-                                }
+                @Override
+                public void operationComplete(@Nullable ChannelFuture future) {
+                    if (future == null) {
+                        return;
+                    }
+                    if (future.isSuccess()) {
+                        connectError = false;
+                        Channel ch = future.channel();
+                        ch.writeAndFlush(request);
+                    } else { // an error occured
+                        if (future.isDone() && !future.isCancelled()) {
+                            Throwable cause = future.cause();
+                            logger.trace("connect failed - cause {}", cause.getMessage());
+                            if (cause instanceof ConnectTimeoutException) {
+                                logger.debug("Camera is not reachable on IP {}", ipAddress);
+                                connectError = true;
+                            } else if ((cause instanceof ConnectException)
+                                    && cause.getMessage().contains("Connection refused")) {
+                                logger.debug("Camera ONVIF port {} is refused.", onvifPort);
+                                refusedError = true;
                             }
                         }
-                    });
+                        if (isConnected) {
+                            disconnect();
+                        }
+                    }
+                }
+            });
         } else {
             logger.debug("ONVIF message not sent as connection is shutting down");
         }
@@ -932,6 +938,14 @@ public class OnvifConnection {
         }
     }
 
+    public boolean isConnectError() {
+        return connectError;
+    }
+
+    public boolean isRefusedError() {
+        return refusedError;
+    }
+
     public boolean isConnected() {
         connecting.lock();
         try {
@@ -941,6 +955,17 @@ public class OnvifConnection {
         }
     }
 
+    public void setIsConnected(boolean isConnected) {
+        connecting.lock();
+        try {
+            this.isConnected = isConnected;
+            this.connectError = false;
+            this.refusedError = false;
+        } finally {
+            connecting.unlock();
+        }
+    }
+
     private void cleanup() {
         if (!isConnected && !mainEventLoopGroup.isShuttingDown()) {
             try {
@@ -959,9 +984,9 @@ public class OnvifConnection {
     public void disconnect() {
         connecting.lock();// Lock out multiple disconnect()/connect() attempts as we try to send Unsubscribe.
         try {
-            isConnected = false;// isConnected is not thread safe, connecting.lock() used as fix.
             if (bootstrap != null) {
-                if (usingEvents && !mainEventLoopGroup.isShuttingDown()) {
+                if (isConnected && usingEvents && !mainEventLoopGroup.isShuttingDown()) {
+                    // Only makes sense to send if connected
                     // Some cameras may continue to send events even when they can't reach a server.
                     sendOnvifRequest(RequestType.Unsubscribe, subscriptionXAddr);
                 }
@@ -970,6 +995,8 @@ public class OnvifConnection {
             } else {
                 cleanup();
             }
+
+            isConnected = false;// isConnected is not thread safe, connecting.lock() used as fix.
         } finally {
             connecting.unlock();
         }