From 6805f8461e4f2ccafa9c455f327275c74d1947b9 Mon Sep 17 00:00:00 2001 From: Matthew Skinner Date: Sat, 17 Sep 2022 18:51:55 +1000 Subject: [PATCH] [ipcamera] Fix ONVIF fails to reconnect (#13396) * Fix reconnecting issues * Cleanup logging. Signed-off-by: Matthew Skinner --- .../internal/handler/IpCameraHandler.java | 22 +++-- .../internal/onvif/OnvifConnection.java | 96 ++++++++++++------- .../resources/OH-INF/thing/thing-types.xml | 2 +- 3 files changed, 76 insertions(+), 44 deletions(-) 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 628005cd9..500f6e188 100644 --- 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 @@ -492,10 +492,17 @@ public class IpCameraHandler extends BaseThingHandler { } private void checkCameraConnection() { - if (snapshotUri.isEmpty() || snapshotPolling) { - // Already polling or camera has RTSP only and no HTTP server + if (snapshotPolling) {// Currently polling a real URL for snapshots, so camera must be online. return; + } else if (ffmpegSnapshotGeneration) {// Use RTSP stream creating snapshots to know camera is online. + Ffmpeg localSnapshot = ffmpegSnapshot; + if (localSnapshot != null && !localSnapshot.getIsAlive()) { + cameraCommunicationError("FFmpeg Snapshots Stopped: Check your camera can be reached."); + return; + } + return;// ffmpeg snapshot stream is still alive } + // Open a HTTP connection without sending any requests as we do not need a snapshot. Bootstrap localBootstrap = mainBootstrap; if (localBootstrap != null) { ChannelFuture chFuture = localBootstrap @@ -1362,6 +1369,7 @@ public class IpCameraHandler extends BaseThingHandler { if (snapshotUri.isEmpty() || "ffmpeg".equals(snapshotUri)) { snapshotIsFfmpeg(); } else { + ffmpegSnapshotGeneration = false; updateSnapshot(); } return; @@ -1374,6 +1382,7 @@ public class IpCameraHandler extends BaseThingHandler { if ("ffmpeg".equals(snapshotUri)) { snapshotIsFfmpeg(); } else if (!snapshotUri.isEmpty()) { + ffmpegSnapshotGeneration = false; updateSnapshot(); } else if (!rtspUri.isEmpty()) { snapshotIsFfmpeg(); @@ -1497,16 +1506,9 @@ public class IpCameraHandler extends BaseThingHandler { // what needs to be done every poll// switch (thing.getThingTypeUID().getId()) { case GENERIC_THING: - if (!snapshotUri.isEmpty() && !snapshotPolling) { + if (!snapshotPolling) { checkCameraConnection(); } - // RTSP stream has stopped and we need it for snapshots - if (ffmpegSnapshotGeneration) { - Ffmpeg localSnapshot = ffmpegSnapshot; - if (localSnapshot != null && !localSnapshot.getIsAlive()) { - localSnapshot.startConverting(); - } - } break; case ONVIF_THING: if (!snapshotPolling) { 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 11381896e..ed452194b 100644 --- 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 @@ -29,6 +29,7 @@ import java.util.TimeZone; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantLock; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -114,6 +115,7 @@ public class OnvifConnection { private ScheduledExecutorService threadPool = Executors.newScheduledThreadPool(2); private @Nullable Bootstrap bootstrap; private EventLoopGroup mainEventLoopGroup = new NioEventLoopGroup(2); + private ReentrantLock connecting = new ReentrantLock(); private String ipAddress = ""; private String user = ""; private String password = ""; @@ -308,7 +310,12 @@ public class OnvifConnection { } else if (message.contains("RenewResponse")) { sendOnvifRequest(requestBuilder(RequestType.PullMessages, subscriptionXAddr)); } else if (message.contains("GetSystemDateAndTimeResponse")) {// 1st to be sent. - isConnected = true; + connecting.lock(); + try { + isConnected = true; + } finally { + connecting.unlock(); + } sendOnvifRequest(requestBuilder(RequestType.GetCapabilities, deviceXAddr)); parseDateAndTime(message); logger.debug("Openhabs UTC dateTime is:{}", getUTCdateTime()); @@ -355,7 +362,8 @@ public class OnvifConnection { } else if (message.contains("GetSnapshotUriResponse")) { snapshotUri = removeIPfromUrl(Helper.fetchXML(message, ":MediaUri", ":Uri")); logger.debug("GetSnapshotUri:{}", snapshotUri); - if (ipCameraHandler.snapshotUri.isEmpty()) { + if (ipCameraHandler.snapshotUri.isEmpty() + && !"ffmpeg".equals(ipCameraHandler.cameraConfig.getSnapshotUrl())) { ipCameraHandler.snapshotUri = snapshotUri; } } else if (message.contains("GetStreamUriResponse")) { @@ -512,6 +520,7 @@ public class OnvifConnection { @SuppressWarnings("null") public void sendOnvifRequest(HttpRequest request) { if (bootstrap == null) { + mainEventLoopGroup = new NioEventLoopGroup(2); bootstrap = new Bootstrap(); bootstrap.group(mainEventLoopGroup); bootstrap.channel(NioSocketChannel.class); @@ -530,24 +539,28 @@ public class OnvifConnection { } }); } - bootstrap.connect(new InetSocketAddress(ipAddress, onvifPort)).addListener(new ChannelFutureListener() { + if (!mainEventLoopGroup.isShuttingDown()) { + bootstrap.connect(new InetSocketAddress(ipAddress, onvifPort)).addListener(new ChannelFutureListener() { - @Override - public void operationComplete(@Nullable ChannelFuture future) { - if (future == null) { - return; - } - if (future.isDone() && future.isSuccess()) { - Channel ch = future.channel(); - ch.writeAndFlush(request); - } else { // an error occured - logger.debug("Camera is not reachable on ONVIF port:{} or the port may be wrong.", onvifPort); - if (isConnected) { - disconnect(); + @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 on ONVIF port:{} or the port may be wrong.", onvifPort); + if (isConnected) { + disconnect(); + } } } - } - }); + }); + } else { + logger.debug("ONVIF message not sent as connection is shutting down"); + } } OnvifConnection getHandle() { @@ -833,42 +846,59 @@ public class OnvifConnection { } public void connect(boolean useEvents) { - if (!isConnected) { - sendOnvifRequest(requestBuilder(RequestType.GetSystemDateAndTime, deviceXAddr)); - usingEvents = useEvents; + connecting.lock(); + try { + if (!isConnected) { + logger.debug("Connecting {} to ONVIF", ipAddress); + threadPool = Executors.newScheduledThreadPool(2); + sendOnvifRequest(requestBuilder(RequestType.GetSystemDateAndTime, deviceXAddr)); + usingEvents = useEvents; + } + } finally { + connecting.unlock(); } } public boolean isConnected() { - return isConnected; + connecting.lock(); + try { + return isConnected; + } finally { + connecting.unlock(); + } } private void cleanup() { - mainEventLoopGroup.shutdownGracefully(); - isConnected = false; - if (!mainEventLoopGroup.isShutdown()) { + if (!isConnected && !mainEventLoopGroup.isShuttingDown()) { try { + mainEventLoopGroup.shutdownGracefully(); mainEventLoopGroup.awaitTermination(3, TimeUnit.SECONDS); } catch (InterruptedException e) { logger.warn("ONVIF was not cleanly shutdown, due to being interrupted"); } finally { logger.debug("Eventloop is shutdown:{}", mainEventLoopGroup.isShutdown()); bootstrap = null; + threadPool.shutdown(); } } - threadPool.shutdown(); } public void disconnect() { - if (bootstrap != null) { - if (usingEvents && isConnected && !mainEventLoopGroup.isShuttingDown()) { - // Some cameras may continue to send events even when they can't reach a server. - sendOnvifRequest(requestBuilder(RequestType.Unsubscribe, subscriptionXAddr)); + 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()) { + // Some cameras may continue to send events even when they can't reach a server. + sendOnvifRequest(requestBuilder(RequestType.Unsubscribe, subscriptionXAddr)); + } + // give time for the Unsubscribe request to be sent, shutdownGracefully will try to send it first. + threadPool.schedule(this::cleanup, 50, TimeUnit.MILLISECONDS); + } else { + cleanup(); } - // give time for the Unsubscribe request to be sent to the camera. - threadPool.schedule(this::cleanup, 50, TimeUnit.MILLISECONDS); - } else { - cleanup(); + } finally { + connecting.unlock(); } } } diff --git a/bundles/org.openhab.binding.ipcamera/src/main/resources/OH-INF/thing/thing-types.xml b/bundles/org.openhab.binding.ipcamera/src/main/resources/OH-INF/thing/thing-types.xml index d643a30a7..06ef7a3d8 100644 --- a/bundles/org.openhab.binding.ipcamera/src/main/resources/OH-INF/thing/thing-types.xml +++ b/bundles/org.openhab.binding.ipcamera/src/main/resources/OH-INF/thing/thing-types.xml @@ -1913,7 +1913,7 @@ true - + Set this to 1 if it is a stand alone camera, or to the input channel number if you use a compatible NVR.