[ipcamera] Fix event streams crash when no source is specified. (openhab#18312)

* Fix regressions

Signed-off-by: Matthew Skinner <[email protected]>

Author: Skinah

bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/InstarHandler.java

@@ -383,9 +383,10 @@ public void alarmTriggered(String alarm) {
     public ArrayList<String> getLowPriorityRequests() {
         ArrayList<String> lowPriorityRequests = new ArrayList<>(7);
         lowPriorityRequests.add("/param.cgi?cmd=getaudioalarmattr");
-        lowPriorityRequests.add("/cgi-bin/hi3510/param.cgi?cmd=getmdattr");
-        if (ipCameraHandler.newInstarApi) {// old API cameras get a error 404 response to this
+        if (ipCameraHandler.newInstarApi) {// old and new API cameras get a error 404 if sent the wrong one
             lowPriorityRequests.add("/param.cgi?cmd=getalarmattr");
+        } else {
+            lowPriorityRequests.add("/cgi-bin/hi3510/param.cgi?cmd=getmdattr");
         }
         lowPriorityRequests.add("/param.cgi?cmd=getinfrared");
         lowPriorityRequests.add("/param.cgi?cmd=getoverlayattr&-region=1");

bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/ReolinkHandler.java

@@ -305,6 +305,7 @@ public void channelRead(@Nullable ChannelHandlerContext ctx, @Nullable Object ms
                         ipCameraHandler.setChannelState(CHANNEL_WHITE_LED, OnOffType.ON);
                     }
                     break;
+                case "/api.cgi?cmd=AudioAlarmPlay":
                 case "/cgi-bin/api.cgi?cmd=Snap":
                     break;
                 case "/api.cgi?cmd=GetAiCfg":
@@ -322,7 +323,8 @@ public void channelRead(@Nullable ChannelHandlerContext ctx, @Nullable Object ms
                     }
                     break;
                 default:
-                    if (!cutDownURL.startsWith("/cgi-bin/api.cgi?cmd=Set")) {// ignore responses from all Setxx commands
+                    if (!cutDownURL.startsWith("/cgi-bin/api.cgi?cmd=Set")
+                            || !cutDownURL.startsWith("/api.cgi?cmd=Set")) {// ignore responses from all Setxx commands
                         ipCameraHandler.logger.warn(
                                 "URL {} is not handled currently by the binding, please report this message",
                                 cutDownURL);

bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/onvif/OnvifCodec.java

@@ -115,7 +115,6 @@ public void exceptionCaught(@Nullable ChannelHandlerContext ctx, @Nullable Throw
     @Override
     public void handlerRemoved(@Nullable ChannelHandlerContext ctx) {
         if (requestType == RequestType.PullMessages) {
-            onvifConnection.lastPullMessageReceivedTimestamp = System.currentTimeMillis();
             onvifConnection.pullMessageRequests.decrementAndGet();
         }
     }

bundles/org.openhab.binding.ipcamera/src/main/java/org/openhab/binding/ipcamera/internal/onvif/OnvifConnection.java

@@ -141,10 +141,9 @@ public enum RequestType {
     private IpCameraHandler ipCameraHandler;
     // Use/skip events even if camera support them. API cameras skip, as their own methods give better results.
     private boolean usingEvents = false;
-    private int onvifEventServiceType = 0; // 0 = disabled, 1 = PullMessages, 2 = WSBaseSubscription
+    private int onvifEventServiceType = 0; // 0 = auto detect, 1 = disabled, 2 = PullMessages, 3 = WSBaseSubscription
     public AtomicInteger pullMessageRequests = new AtomicInteger();
     private long createSubscriptionTimestamp;
-    public long lastPullMessageReceivedTimestamp;
 
     // These hold the cameras PTZ position in the range that the camera uses, ie
     // mine is -1 to +1
@@ -425,13 +424,18 @@ public void processReply(RequestType requestType, String message) {
 
     private void setOnvifEventServiceType(boolean cameraSupportsPullPointSupport,
             boolean cameraSupportsSubscriptionPolicySupport) {
-        if (cameraSupportsPullPointSupport && ipCameraHandler.cameraConfig.getOnvifEventServiceType() == 0
-                || ipCameraHandler.cameraConfig.getOnvifEventServiceType() == 2) {
-            onvifEventServiceType = 1;
-        } else if (cameraSupportsSubscriptionPolicySupport
-                && ipCameraHandler.cameraConfig.getOnvifEventServiceType() == 0
-                || ipCameraHandler.cameraConfig.getOnvifEventServiceType() == 3) {
+        // 0 = auto detect, 1 = disabled, 2 = PullMessages, 3 = WSBaseSubscription
+        if (cameraSupportsPullPointSupport && ipCameraHandler.cameraConfig.getOnvifEventServiceType() == 0) {
             onvifEventServiceType = 2;
+        } else if (cameraSupportsSubscriptionPolicySupport
+                && ipCameraHandler.cameraConfig.getOnvifEventServiceType() == 0) {
+            onvifEventServiceType = 3;
+        } else if (ipCameraHandler.cameraConfig.getOnvifEventServiceType() == 0) {
+            logger.warn(
+                    "Camera at {} could not auto detect the ONVIF event method the camera supports, try setting the configuration away from auto to remove this message by forcing an option.",
+                    ipAddress);
+        } else {
+            onvifEventServiceType = ipCameraHandler.cameraConfig.getOnvifEventServiceType();
         }
     }
 
@@ -480,7 +484,7 @@ void setSubscriptionXAddr(String message) {
 
     public void createSubscription() {
         if (!getEventsSupported()) {
-            // ONVIF events are disabled or not supported.
+            logger.debug("ONVIF events are disabled or not supported for camera at {}", ipAddress);
             return;
         }
 
@@ -495,9 +499,9 @@ public void createSubscription() {
 
         // Prefer PullPoint events over WSBaseSubscription because there is no way to check if a WSBaseSubscription is
         // already registered on the camera.
-        if (onvifEventServiceType == 1) {
+        if (onvifEventServiceType == 2) {
             sendOnvifRequest(RequestType.CreatePullPointSubscription, eventXAddr);
-        } else if (onvifEventServiceType == 2) {
+        } else if (onvifEventServiceType == 3) {
             sendOnvifRequest(RequestType.Subscribe, eventXAddr);
         }
     }
@@ -507,20 +511,20 @@ public void createSubscription() {
      * needed.
      */
     public void checkAndRenewEventSubscription() {
-        if (getEventsSupported()) {
-            // If we get events via PullMessages check if a PullMessages request is running or we just received an
-            // answer in the last second. If this is not the case create a new PullMessages subscription.
-            if (onvifEventServiceType == 1 && pullMessageRequests.intValue() == 0
-                    && System.currentTimeMillis() - lastPullMessageReceivedTimestamp > 1000) {
-                logger.debug("The alarm stream was not running for camera {}, re-starting it now", ipAddress);
-                createSubscription();
-            } else if (!subscriptionXAddr.isEmpty()) {
-                // Renew the active subscription.
-                sendOnvifRequest(RequestType.Renew, subscriptionXAddr);
-            } else {
+        if (getEventsSupported() && onvifEventServiceType == 2) {
+            if (subscriptionXAddr.isEmpty()) {
                 // The camera claims to have event support, but no subscription was created yet. Try to create a new
                 // subscription.
                 createSubscription();
+            } else if (pullMessageRequests.intValue() == 0) {
+                // If we get events via PullMessages, check if a PullMessages request is running. Netty's
+                // IdleStateHandler
+                // will know if any request fails after a set time expires.
+                sendOnvifRequest(RequestType.Renew, subscriptionXAddr);
+                logger.debug("The alarm stream was not running for camera {}, re-starting it now", ipAddress);
+                sendOnvifRequest(RequestType.PullMessages, subscriptionXAddr);
+            } else {
+                sendOnvifRequest(RequestType.Renew, subscriptionXAddr);
             }
         }
     }
@@ -828,16 +832,8 @@ public void eventReceived(String eventMessage) {
             Element topicElement = (Element) notificationMessageElement.getElementsByTagName("wsnt:Topic").item(0);
             String topic = topicElement.getFirstChild().getNodeValue().replace("tns1:", "");
 
-            String sourceName = "";
-            String sourceValue = "";
             Element sourceElement = (Element) notificationMessageElement.getElementsByTagName("tt:Source").item(0);
 
-            if (sourceElement != null) {
-                Element sourceItemElement = (Element) sourceElement.getElementsByTagName("tt:SimpleItem").item(0);
-                sourceName = sourceItemElement.getAttributes().getNamedItem("Name").getNodeValue();
-                sourceValue = sourceItemElement.getAttributes().getNamedItem("Value").getNodeValue();
-            }
-
             Element dataElement = (Element) notificationMessageElement.getElementsByTagName("tt:Data").item(0);
 
             if (dataElement == null) {
@@ -850,8 +846,7 @@ public void eventReceived(String eventMessage) {
             String dataName = dataItemElement.getAttributes().getNamedItem("Name").getNodeValue();
             String dataValue = dataItemElement.getAttributes().getNamedItem("Value").getNodeValue();
 
-            logger.debug("ONVIF Event Topic: {}, Source name: {}, Source value: {}, Data name: {}, Data value: {}",
-                    topic, sourceName, sourceValue, dataName, dataValue);
+            logger.debug("ONVIF Event Topic: {}, Data name: {}, Data value: {}", topic, dataName, dataValue);
             switch (topic) {
                 case "RuleEngine/CellMotionDetector/Motion":
                     if ("true".equals(dataValue)) {
@@ -1165,7 +1160,7 @@ public boolean isConnected() {
     }
 
     public boolean getEventsSupported() {
-        return onvifEventServiceType > 0;
+        return onvifEventServiceType > 1;// 0 = auto detect, 1 = disabled, 2 = PullMessages, 3 = WSBaseSubscription
     }
 
     public void setIsConnected(boolean isConnected) {