Add option to update the remote device cache (#104)

* Add option to update the remote device cache

* Make LocalDevice implement AutoClosable

* Add test for expirationCheck

* Fix case where iAmReceived newDevice is same instance

Move callback outside synchronized
Make RemoteDeviceDiscoverer implement AutoClosable
Add tests for startRemoteDeviceDiscovery method

* Add Javadoc, fix imports

* Update README.md

* Add flatten-maven-plugin

README.md

@@ -61,6 +61,7 @@ Releases
 - Allow NULL values for daily schedule, exception schedule and schedule default
 - Fix scheduling issues when TimeValue sequences are not in chronological order
 - Fix schedule object using incorrect time format to trigger next update
+- Add new CacheUpdate option to the startRemoteDeviceDiscovery() method of LocalDevice 
 
 *Version 6.0.0*
 - fix DeviceObjectTest.timeSynchronization test to pass

pom.xml

@@ -70,6 +70,32 @@
 				<artifactId>maven-site-plugin</artifactId>
 				<version>3.8.2</version>
 			</plugin>
+			<plugin>
+				<groupId>org.codehaus.mojo</groupId>
+				<artifactId>flatten-maven-plugin</artifactId>
+				<version>1.6.0</version>
+				<configuration>
+					<updatePomFile>true</updatePomFile>
+					<flattenMode>resolveCiFriendliesOnly</flattenMode>
+					<outputDirectory>${project.build.directory}</outputDirectory>
+				</configuration>
+				<executions>
+					<execution>
+						<id>flatten</id>
+						<phase>process-resources</phase>
+						<goals>
+							<goal>flatten</goal>
+						</goals>
+					</execution>
+					<execution>
+						<id>flatten.clean</id>
+						<phase>clean</phase>
+						<goals>
+							<goal>clean</goal>
+						</goals>
+					</execution>
+				</executions>
+			</plugin>
 		</plugins>
 	</build>
 	<dependencies>

src/main/java/com/serotonin/bacnet4j/LocalDevice.java

@@ -45,6 +45,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
@@ -107,7 +108,7 @@
  * - default character string encoding
  * - persistence of recipient lists in notification forwarder object
  */
-public class LocalDevice {
+public class LocalDevice implements AutoCloseable {
     static final Logger LOG = LoggerFactory.getLogger(LocalDevice.class);
     public static final String VERSION = "6.0.0";
 
@@ -841,34 +842,72 @@ public RemoteDevice getRemoteDeviceBlocking(final int instanceNumber, final long
         return rd;
     }
 
+    @Override
+    public synchronized void close() throws Exception {
+        if (initialized) {
+            terminate();
+        }
+    }
+
+    public enum CacheUpdate {
+        /**
+         * Always update the remote device cache, even if the existing entry has not expired.
+         */
+        ALWAYS,
+        /**
+         * Never update the remote device cache, even if the existing entry has expired.
+         */
+        NEVER,
+        /**
+         * Only update the remote device cache if the existing entry has expired.
+         */
+        IF_EXPIRED
+    }
+
     public RemoteDeviceDiscoverer startRemoteDeviceDiscovery() {
-        return startRemoteDeviceDiscovery(null);
+        return startRemoteDeviceDiscovery(CacheUpdate.NEVER, null);
+    }
+
+    public RemoteDeviceDiscoverer startRemoteDeviceDiscovery(final Consumer<RemoteDevice> callback) {
+        return startRemoteDeviceDiscovery(CacheUpdate.NEVER, callback);
     }
 
     /**
      * Creates and starts a remote device discovery. Discovered devices are added to the cache as they are found. The
      * returned discoverer must be stopped by the caller.
      *
-     * @param callback
-     *            optional client callback
+     * @param cacheUpdate controls if the remote device cache should be updated
+     * @param callback optional client callback
      * @return the discoverer, which must be stopped by the caller
      */
-    public RemoteDeviceDiscoverer startRemoteDeviceDiscovery(final Consumer<RemoteDevice> callback) {
-        final Consumer<RemoteDevice> cachingCallback = (d) -> {
+    public RemoteDeviceDiscoverer startRemoteDeviceDiscovery(CacheUpdate cacheUpdate, final Consumer<RemoteDevice> callback) {
+        final RemoteDeviceDiscoverer discoverer = new RemoteDeviceDiscoverer(this, discoveredDevice -> {
             // Cache the device.
-            remoteDeviceCache.putEntity(d.getInstanceNumber(), d, cachePolicies.getDevicePolicy(d.getInstanceNumber()));
+            remoteDeviceCache.putEntity(discoveredDevice.getInstanceNumber(), discoveredDevice,
+                    cachePolicies.getDevicePolicy(discoveredDevice.getInstanceNumber()));
 
             // Call the given callback
-            if (callback != null)
-                callback.accept(d);
-        };
-
-        final RemoteDeviceDiscoverer discoverer = new RemoteDeviceDiscoverer(this, cachingCallback);
+            if (callback != null) {
+                callback.accept(discoveredDevice);
+            }
+        }, getExpirationCheck(cacheUpdate));
         discoverer.start();
-
         return discoverer;
     }
 
+    private Predicate<RemoteDevice> getExpirationCheck(CacheUpdate cacheUpdate) {
+        switch (cacheUpdate) {
+            case ALWAYS:
+                return d -> true;
+            case NEVER:
+                return d -> false;
+            case IF_EXPIRED:
+                return d -> remoteDeviceCache.getCachedEntity(d.getInstanceNumber()) == null;
+            default:
+                throw new IllegalArgumentException("Unknown value: " + cacheUpdate);
+        }
+    }
+
     /**
      * Updates the remote device with the given number with the given address, but only if the
      * remote device is cached. Otherwise, nothing happens.

src/main/java/com/serotonin/bacnet4j/util/RemoteDeviceDiscoverer.java

@@ -29,8 +29,11 @@
 package com.serotonin.bacnet4j.util;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.function.Consumer;
+import java.util.function.Predicate;
 
 import com.serotonin.bacnet4j.LocalDevice;
 import com.serotonin.bacnet4j.RemoteDevice;
@@ -47,50 +50,58 @@
  *
  * @author Matthew
  */
-public class RemoteDeviceDiscoverer {
+public class RemoteDeviceDiscoverer implements AutoCloseable {
     private final LocalDevice localDevice;
     private final Consumer<RemoteDevice> callback;
 
     private DeviceEventAdapter adapter;
-    private final List<RemoteDevice> allDevices = new ArrayList<>();
+    private final Map<Integer, RemoteDevice> allDevices = new HashMap<>();
     private final List<RemoteDevice> latestDevices = new ArrayList<>();
+    private final Predicate<RemoteDevice> expirationCheck;
 
     public RemoteDeviceDiscoverer(final LocalDevice localDevice) {
         this(localDevice, null);
     }
 
     public RemoteDeviceDiscoverer(final LocalDevice localDevice, final Consumer<RemoteDevice> callback) {
+        this(localDevice, callback, remoteDevice -> false);
+    }
+
+    public RemoteDeviceDiscoverer(final LocalDevice localDevice, final Consumer<RemoteDevice> callback,
+                                  final Predicate<RemoteDevice> expirationCheck) {
         this.localDevice = localDevice;
         this.callback = callback;
+        this.expirationCheck = expirationCheck;
     }
 
-    public void start() {
-        adapter = new DeviceEventAdapter() {
+    public synchronized void start() {
+        if (adapter != null) {
+            throw new IllegalStateException("Already started");
+        }
+        this.adapter = new DeviceEventAdapter() {
             @Override
-            public void iAmReceived(final RemoteDevice d) {
+            public void iAmReceived(final RemoteDevice newDevice) {
+                BooleanHolder updated = new BooleanHolder();
                 synchronized (allDevices) {
                     // Check if we already know about this device.
-                    boolean found = false;
-                    for (final RemoteDevice known : allDevices) {
-                        if (d.getInstanceNumber() == known.getInstanceNumber()) {
-                            found = true;
-                            break;
+                    allDevices.compute(newDevice.getInstanceNumber(), (k, existing) -> {
+                        if (existing == null || expirationCheck.test(existing)) {
+                            updated.value = true;
+                            return newDevice;
                         }
-                    }
-
-                    if (!found) {
-                        // Add to all devices
-                        allDevices.add(d);
+                        return existing;
+                    });
 
+                    if (updated.value) {
                         // Add to latest devices
-                        latestDevices.add(d);
-
-                        // Notify the callback
-                        if (callback != null) {
-                            callback.accept(d);
-                        }
+                        latestDevices.add(newDevice);
                     }
                 }
+
+                // Notify the callback
+                if (updated.value && callback != null) {
+                    callback.accept(newDevice);
+                }
             }
         };
 
@@ -101,17 +112,20 @@ public void iAmReceived(final RemoteDevice d) {
         localDevice.sendGlobalBroadcast(new WhoIsRequest());
     }
 
-    public void stop() {
-        // Unregister as a listener
-        localDevice.getEventHandler().removeListener(adapter);
+    public synchronized void stop() {
+        if (adapter != null) {
+            // Unregister as a listener
+            localDevice.getEventHandler().removeListener(adapter);
+            this.adapter = null;
+        }
     }
 
     /**
      * Returns all devices discovered by this discoverer so far.
      */
     public List<RemoteDevice> getRemoteDevices() {
         synchronized (allDevices) {
-            return new ArrayList<>(allDevices);
+            return new ArrayList<>(allDevices.values());
         }
     }
 
@@ -125,4 +139,13 @@ public List<RemoteDevice> getLatestRemoteDevices() {
             return result;
         }
     }
+
+    @Override
+    public void close() {
+        stop();
+    }
+
+    private static class BooleanHolder {
+        private boolean value = false;
+    }
 }