]> git.basschouten.com Git - openhab-addons.git/commitdiff
[zoneminder] Rework discovery to not use the thing handler thread (#9600)
authorMark Hilbush <mark@hilbush.com>
Thu, 31 Dec 2020 12:16:23 +0000 (07:16 -0500)
committerGitHub <noreply@github.com>
Thu, 31 Dec 2020 12:16:23 +0000 (13:16 +0100)
Signed-off-by: Mark Hilbush <mark@hilbush.com>
bundles/org.openhab.binding.zoneminder/src/main/java/org/openhab/binding/zoneminder/internal/ZmHandlerFactory.java
bundles/org.openhab.binding.zoneminder/src/main/java/org/openhab/binding/zoneminder/internal/config/ZmBridgeConfig.java
bundles/org.openhab.binding.zoneminder/src/main/java/org/openhab/binding/zoneminder/internal/discovery/MonitorDiscoveryService.java
bundles/org.openhab.binding.zoneminder/src/main/java/org/openhab/binding/zoneminder/internal/handler/ZmBridgeHandler.java
bundles/org.openhab.binding.zoneminder/src/main/java/org/openhab/binding/zoneminder/internal/handler/ZmMonitorHandler.java
bundles/org.openhab.binding.zoneminder/src/main/resources/OH-INF/config/config.xml
bundles/org.openhab.binding.zoneminder/src/main/resources/OH-INF/thing/thing-types.xml

index 26704f29040cf07b0c89b4e6ba534dd0db6e07d1..00bf3d8cb1afd9742143cb91af1571657d2eda74 100644 (file)
@@ -37,7 +37,7 @@ import org.osgi.service.component.annotations.Reference;
  * @author Mark Hilbush - Initial contribution
  */
 @NonNullByDefault
-@Component(configurationPid = "binding.zm", service = ThingHandlerFactory.class)
+@Component(configurationPid = "binding.zoneminder", service = ThingHandlerFactory.class)
 public class ZmHandlerFactory extends BaseThingHandlerFactory {
 
     private final TimeZoneProvider timeZoneProvider;
index b1b170b114b619a22a05dc1d2a7dfc9f3199b130..7e4d374385f1f1e3f43381a6bae1cc07b62834b5 100644 (file)
@@ -50,16 +50,6 @@ public class ZmBridgeConfig {
      */
     public @Nullable Integer refreshInterval;
 
-    /**
-     * Enable/disable monitor discovery
-     */
-    public @Nullable Boolean discoveryEnabled;
-
-    /**
-     * Frequency at which the binding will try to discover monitors
-     */
-    public @Nullable Integer discoveryInterval;
-
     /**
      * Alarm duration set on monitor things when they're discovered
      */
@@ -70,6 +60,11 @@ public class ZmBridgeConfig {
      */
     public @Nullable Integer defaultImageRefreshInterval;
 
+    /**
+     * Enable/disable monitor discovery
+     */
+    public Boolean discoveryEnabled = Boolean.TRUE;
+
     /**
      * Zoneminder user name
      */
index a7429c16b3ccff9de1079d90896868d06623661f..4fc46d2fe026ddea9c5b144ff33ae8f195b498c2 100644 (file)
@@ -17,6 +17,8 @@ import static org.openhab.binding.zoneminder.internal.ZmBindingConstants.*;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
@@ -44,31 +46,38 @@ public class MonitorDiscoveryService extends AbstractDiscoveryService implements
 
     private final Logger logger = LoggerFactory.getLogger(MonitorDiscoveryService.class);
 
-    private @Nullable ZmBridgeHandler bridgeHandler;
+    private static final int DISCOVERY_INTERVAL_SECONDS = 300;
+    private static final int DISCOVERY_INITIAL_DELAY_SECONDS = 10;
+    private static final int DISCOVERY_TIMEOUT_SECONDS = 6;
+
+    private @NonNullByDefault({}) ZmBridgeHandler bridgeHandler;
+
+    private @Nullable Future<?> discoveryJob;
 
     public MonitorDiscoveryService() {
-        super(30);
+        super(SUPPORTED_MONITOR_THING_TYPES_UIDS, DISCOVERY_TIMEOUT_SECONDS, true);
     }
 
     @Override
-    public void setThingHandler(@Nullable ThingHandler handler) {
-        if (handler instanceof ZmBridgeHandler) {
-            ((ZmBridgeHandler) handler).setDiscoveryService(this);
-            this.bridgeHandler = (ZmBridgeHandler) handler;
-        }
+    public void activate() {
+        super.activate(null);
     }
 
     @Override
-    public @Nullable ThingHandler getThingHandler() {
-        return bridgeHandler;
+    public void deactivate() {
+        super.deactivate();
     }
 
     @Override
-    public void activate() {
+    public void setThingHandler(@Nullable ThingHandler handler) {
+        if (handler instanceof ZmBridgeHandler) {
+            bridgeHandler = (ZmBridgeHandler) handler;
+        }
     }
 
     @Override
-    public void deactivate() {
+    public @Nullable ThingHandler getThingHandler() {
+        return bridgeHandler;
     }
 
     @Override
@@ -77,52 +86,63 @@ public class MonitorDiscoveryService extends AbstractDiscoveryService implements
     }
 
     @Override
-    public void startBackgroundDiscovery() {
-        logger.trace("Discovery: Performing background discovery scan for {}", getBridgeUID());
-        discoverMonitors();
+    protected void startBackgroundDiscovery() {
+        Future<?> localDiscoveryJob = discoveryJob;
+        if (localDiscoveryJob == null || localDiscoveryJob.isCancelled()) {
+            logger.debug("ZoneminderDiscovery: Starting background discovery job");
+            discoveryJob = scheduler.scheduleWithFixedDelay(this::backgroundDiscoverMonitors,
+                    DISCOVERY_INITIAL_DELAY_SECONDS, DISCOVERY_INTERVAL_SECONDS, TimeUnit.SECONDS);
+        }
+    }
+
+    @Override
+    protected void stopBackgroundDiscovery() {
+        Future<?> localDiscoveryJob = discoveryJob;
+        if (localDiscoveryJob != null) {
+            logger.debug("ZoneminderDiscovery: Stopping background discovery job");
+            localDiscoveryJob.cancel(true);
+            discoveryJob = null;
+        }
     }
 
     @Override
     public void startScan() {
-        logger.debug("Discovery: Starting monitor discovery scan for {}", getBridgeUID());
+        logger.debug("ZoneminderDiscovery: Running discovery scan");
         discoverMonitors();
     }
 
-    private @Nullable ThingUID getBridgeUID() {
-        ZmBridgeHandler localBridgeHandler = bridgeHandler;
-        return localBridgeHandler != null ? localBridgeHandler.getThing().getUID() : null;
+    private void backgroundDiscoverMonitors() {
+        if (!bridgeHandler.isBackgroundDiscoveryEnabled()) {
+            return;
+        }
+        logger.debug("ZoneminderDiscovery: Running background discovery scan");
+        discoverMonitors();
     }
 
     private synchronized void discoverMonitors() {
-        ZmBridgeHandler localBridgeHandler = bridgeHandler;
-        ThingUID bridgeUID = getBridgeUID();
-        if (localBridgeHandler != null && bridgeUID != null) {
-            Integer alarmDuration = localBridgeHandler.getDefaultAlarmDuration();
-            Integer imageRefreshInterval = localBridgeHandler.getDefaultImageRefreshInterval();
-            for (Monitor monitor : localBridgeHandler.getSavedMonitors()) {
-                String id = monitor.getId();
-                String name = monitor.getName();
-                ThingUID thingUID = new ThingUID(UID_MONITOR, bridgeUID, monitor.getId());
-                Map<String, Object> properties = new HashMap<>();
-                properties.put(CONFIG_MONITOR_ID, id);
-                properties.put(CONFIG_ALARM_DURATION, alarmDuration);
-                if (imageRefreshInterval != null) {
-                    properties.put(CONFIG_IMAGE_REFRESH_INTERVAL, imageRefreshInterval);
-                }
-                thingDiscovered(createDiscoveryResult(thingUID, bridgeUID, id, name, properties));
-                logger.trace("Discovery: Monitor with id '{}' and name '{}' added to Inbox with UID '{}'",
-                        monitor.getId(), monitor.getName(), thingUID);
+        ThingUID bridgeUID = bridgeHandler.getThing().getUID();
+        Integer alarmDuration = bridgeHandler.getDefaultAlarmDuration();
+        Integer imageRefreshInterval = bridgeHandler.getDefaultImageRefreshInterval();
+        for (Monitor monitor : bridgeHandler.getSavedMonitors()) {
+            String id = monitor.getId();
+            String name = monitor.getName();
+            ThingUID thingUID = new ThingUID(UID_MONITOR, bridgeUID, monitor.getId());
+            Map<String, Object> properties = new HashMap<>();
+            properties.put(CONFIG_MONITOR_ID, id);
+            properties.put(CONFIG_ALARM_DURATION, alarmDuration);
+            if (imageRefreshInterval != null) {
+                properties.put(CONFIG_IMAGE_REFRESH_INTERVAL, imageRefreshInterval);
             }
+            thingDiscovered(createDiscoveryResult(thingUID, bridgeUID, id, name, properties));
+            logger.debug("ZoneminderDiscovery: Monitor with id '{}' and name '{}' added to Inbox with UID '{}'",
+                    monitor.getId(), monitor.getName(), thingUID);
         }
     }
 
     private DiscoveryResult createDiscoveryResult(ThingUID monitorUID, ThingUID bridgeUID, String id, String name,
             Map<String, Object> properties) {
         return DiscoveryResultBuilder.create(monitorUID).withProperties(properties).withBridge(bridgeUID)
-                .withLabel(buildLabel(name)).withRepresentationProperty(CONFIG_MONITOR_ID).build();
-    }
-
-    private String buildLabel(String name) {
-        return String.format("Zoneminder Monitor %s", name);
+                .withLabel(String.format("Zoneminder Monitor %s", name)).withRepresentationProperty(CONFIG_MONITOR_ID)
+                .build();
     }
 }
index d5e8bbc3f7d94cf9daf0f6e10847e1e973f970dd..4a3384b86ea0a701439be55da8fd3590dbfd0f96 100644 (file)
@@ -28,7 +28,6 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
-import java.util.concurrent.atomic.AtomicInteger;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
@@ -81,14 +80,8 @@ import com.google.gson.JsonSyntaxException;
 @NonNullByDefault
 public class ZmBridgeHandler extends BaseBridgeHandler {
 
-    private static final int REFRESH_INTERVAL_SECONDS = 1;
-    private static final int REFRESH_STARTUP_DELAY_SECONDS = 3;
-
-    private static final int MONITORS_INTERVAL_SECONDS = 5;
-    private static final int MONITORS_INITIAL_DELAY_SECONDS = 3;
-
-    private static final int DISCOVERY_INTERVAL_SECONDS = 300;
-    private static final int DISCOVERY_INITIAL_DELAY_SECONDS = 10;
+    private static final int MONITOR_REFRESH_INTERVAL_SECONDS = 10;
+    private static final int MONITOR_REFRESH_STARTUP_DELAY_SECONDS = 5;
 
     private static final int API_TIMEOUT_MSEC = 10000;
 
@@ -104,10 +97,6 @@ public class ZmBridgeHandler extends BaseBridgeHandler {
     private final Logger logger = LoggerFactory.getLogger(ZmBridgeHandler.class);
 
     private @Nullable Future<?> refreshMonitorsJob;
-    private final AtomicInteger monitorsCounter = new AtomicInteger();
-
-    private @Nullable MonitorDiscoveryService discoveryService;
-    private final AtomicInteger discoveryCounter = new AtomicInteger();
 
     private List<Monitor> savedMonitors = new ArrayList<>();
 
@@ -115,9 +104,8 @@ public class ZmBridgeHandler extends BaseBridgeHandler {
     private boolean useSSL;
     private @Nullable String portNumber;
     private String urlPath = DEFAULT_URL_PATH;
-    private int monitorsInterval;
-    private int discoveryInterval;
-    private boolean discoveryEnabled;
+    private int monitorRefreshInterval;
+    private boolean backgroundDiscoveryEnabled;
     private int defaultAlarmDuration;
     private @Nullable Integer defaultImageRefreshInterval;
 
@@ -144,17 +132,15 @@ public class ZmBridgeHandler extends BaseBridgeHandler {
 
         Integer value;
         value = config.refreshInterval;
-        monitorsInterval = value == null ? MONITORS_INTERVAL_SECONDS : value;
-
-        value = config.discoveryInterval;
-        discoveryInterval = value == null ? DISCOVERY_INTERVAL_SECONDS : value;
+        monitorRefreshInterval = value == null ? MONITOR_REFRESH_INTERVAL_SECONDS : value;
 
         value = config.defaultAlarmDuration;
         defaultAlarmDuration = value == null ? DEFAULT_ALARM_DURATION_SECONDS : value;
 
         defaultImageRefreshInterval = config.defaultImageRefreshInterval;
 
-        discoveryEnabled = config.discoveryEnabled == null ? false : config.discoveryEnabled.booleanValue();
+        backgroundDiscoveryEnabled = config.discoveryEnabled;
+        logger.debug("Bridge: Background discovery is {}", backgroundDiscoveryEnabled == true ? "ENABLED" : "DISABLED");
 
         host = config.host;
         useSSL = config.useSSL.booleanValue();
@@ -222,12 +208,8 @@ public class ZmBridgeHandler extends BaseBridgeHandler {
         return Collections.singleton(MonitorDiscoveryService.class);
     }
 
-    public void setDiscoveryService(MonitorDiscoveryService discoveryService) {
-        this.discoveryService = discoveryService;
-    }
-
-    public boolean isDiscoveryEnabled() {
-        return discoveryEnabled;
+    public boolean isBackgroundDiscoveryEnabled() {
+        return backgroundDiscoveryEnabled;
     }
 
     public Integer getDefaultAlarmDuration() {
@@ -571,40 +553,14 @@ public class ZmBridgeHandler extends BaseBridgeHandler {
         return false;
     }
 
-    /*
-     * The refresh job is executed every second
-     * - updates the monitor handlers every monitorsInterval seconds, and
-     * - runs the monitor discovery every discoveryInterval seconds
-     */
-    private void refresh() {
-        refreshMonitors();
-        discoverMonitors();
-    }
-
     @SuppressWarnings("null")
     private void refreshMonitors() {
-        if (monitorsCounter.getAndDecrement() == 0) {
-            monitorsCounter.set(monitorsInterval);
-            List<Monitor> monitors = getMonitors();
-            savedMonitors = monitors;
-            for (Monitor monitor : monitors) {
-                ZmMonitorHandler handler = monitorHandlers.get(monitor.getId());
-                if (handler != null) {
-                    handler.updateStatus(monitor);
-                }
-            }
-        }
-    }
-
-    private void discoverMonitors() {
-        if (isDiscoveryEnabled()) {
-            if (discoveryCounter.getAndDecrement() == 0) {
-                discoveryCounter.set(discoveryInterval);
-                MonitorDiscoveryService localDiscoveryService = discoveryService;
-                if (localDiscoveryService != null) {
-                    logger.trace("Bridge: Running monitor discovery");
-                    localDiscoveryService.startBackgroundDiscovery();
-                }
+        List<Monitor> monitors = getMonitors();
+        savedMonitors = monitors;
+        for (Monitor monitor : monitors) {
+            ZmMonitorHandler handler = monitorHandlers.get(monitor.getId());
+            if (handler != null) {
+                handler.updateStatus(monitor);
             }
         }
     }
@@ -612,10 +568,8 @@ public class ZmBridgeHandler extends BaseBridgeHandler {
     private void scheduleRefreshJob() {
         logger.debug("Bridge: Scheduling monitors refresh job");
         cancelRefreshJob();
-        monitorsCounter.set(MONITORS_INITIAL_DELAY_SECONDS);
-        discoveryCounter.set(DISCOVERY_INITIAL_DELAY_SECONDS);
-        refreshMonitorsJob = scheduler.scheduleWithFixedDelay(this::refresh, REFRESH_STARTUP_DELAY_SECONDS,
-                REFRESH_INTERVAL_SECONDS, TimeUnit.SECONDS);
+        refreshMonitorsJob = scheduler.scheduleWithFixedDelay(this::refreshMonitors,
+                MONITOR_REFRESH_STARTUP_DELAY_SECONDS, monitorRefreshInterval, TimeUnit.SECONDS);
     }
 
     private void cancelRefreshJob() {
@@ -623,6 +577,7 @@ public class ZmBridgeHandler extends BaseBridgeHandler {
         if (localRefreshThermostatsJob != null) {
             localRefreshThermostatsJob.cancel(true);
             logger.debug("Bridge: Canceling monitors refresh job");
+            refreshMonitorsJob = null;
         }
     }
 }
index 629d95f33028c9e168b4d5e3aefa43f57eaf92a1..ea41cf34edb4fbab2e1f6e542d2fa9554e0d9d39 100644 (file)
@@ -254,6 +254,7 @@ public class ZmMonitorHandler extends BaseThingHandler {
     }
 
     private void startImageRefreshJob() {
+        stopImageRefreshJob();
         Integer interval = imageRefreshIntervalSeconds;
         if (interval != null) {
             long delay = getRandomDelay(interval);
index 5f66f9d9c34122a8267ac066dc822858ce49f037..18227c978ac7e33e3d7397b3c1d79d833e12010a 100644 (file)
                <parameter-group name="auth-info">
                        <label>Authentication Information</label>
                </parameter-group>
-               <parameter name="refreshInterval" type="integer" min="2" unit="s" required="true" groupName="config-info">
-                       <label>Refresh Interval</label>
-                       <description>Interval in seconds at which monitor status is updated</description>
-                       <default>5</default>
-               </parameter>
-               <parameter name="discoveryEnabled" type="boolean" required="true" groupName="config-info">
-                       <label>Discovery Enabled</label>
-                       <description>Enable/disable automatic discovery</description>
-                       <default>true</default>
-               </parameter>
-               <parameter name="discoveryInterval" type="integer" min="60" unit="s" required="true" groupName="config-info">
-                       <label>Monitor Discovery Interval</label>
-                       <description>Specifies time in seconds in which the binding will attempt to discover monitors</description>
-                       <default>300</default>
-               </parameter>
-               <parameter name="defaultAlarmDuration" type="integer" unit="s" required="false" groupName="config-info">
-                       <label>Default Alarm Duration</label>
-                       <description>Duration in seconds after which the alarm will be turned off</description>
-                       <default>60</default>
-               </parameter>
-               <parameter name="defaultImageRefreshInterval" type="integer" unit="s" required="false"
-                       groupName="config-info">
-                       <label>Default Image Refresh Interval</label>
-                       <description>Interval in seconds at which monitor image snapshot will be updated</description>
-               </parameter>
+
+               <!-- Parameter Group url-info -->
                <parameter name="host" type="text" required="true" groupName="url-info">
                        <label>Server</label>
                        <description>ZoneMinder server name or IP address</description>
                        <description>URL path (Default is /zm. Use / if Zoneminder installed under the root directory)</description>
                        <default>/zm</default>
                </parameter>
+
+               <!-- Parameter Group config-info -->
+               <parameter name="refreshInterval" type="integer" min="2" unit="s" required="true" groupName="config-info">
+                       <label>Refresh Interval</label>
+                       <description>Interval in seconds at which monitor status is updated</description>
+                       <default>5</default>
+               </parameter>
+               <parameter name="defaultAlarmDuration" type="integer" unit="s" required="false" groupName="config-info">
+                       <label>Default Alarm Duration</label>
+                       <description>Duration in seconds after which the alarm will be turned off</description>
+                       <default>60</default>
+               </parameter>
+               <parameter name="defaultImageRefreshInterval" type="integer" unit="s" required="false"
+                       groupName="config-info">
+                       <label>Default Image Refresh Interval</label>
+                       <description>Interval in seconds at which monitor image snapshot will be updated</description>
+               </parameter>
+               <parameter name="discoveryEnabled" type="boolean" required="true" groupName="config-info">
+                       <label>Background Discovery Enabled</label>
+                       <description>Enable/disable background discovery of monitors</description>
+                       <default>true</default>
+               </parameter>
+
+               <!-- Parameter Group auth-info -->
                <parameter name="user" type="text" required="false" groupName="auth-info">
                        <label>User Name</label>
                        <description>User name (if authentication enabled in ZoneMinder)</description>
index 8dcb3451030f252828513b24451eb251f8eaebe7..a9e12479e863c2338012c7b2f249c95c95d32fea 100644 (file)
@@ -73,6 +73,9 @@
                        <channel id="eventAlarmFrames" typeId="eventAlarmFrames"/>
                        <channel id="eventLength" typeId="eventLength"/>
                </channels>
+
+               <representation-property>monitorId</representation-property>
+
                <config-description-ref uri="thing-type:zoneminder:monitor"/>
        </thing-type>