Only push updates for desired zones/venues (#80)

src/main/java/com/facebook/openwifirrm/RRMAlgorithm.java

@@ -143,6 +143,9 @@ public class RRMAlgorithm {
 	 * @param dryRun if set, do not apply changes
 	 * @param allowDefaultMode if false, "mode" argument must be present and
 	 *                         valid (returns error if invalid)
+	 * @param updateImmediately true if the method should queue the zone for
+	 * 							update and interrupt the config manager thread
+	 * 							to trigger immediate update
 	 *
 	 * @return the algorithm result, with exactly one field set ("error" upon
 	 *         failure, any others upon success)
@@ -153,7 +156,8 @@ public class RRMAlgorithm {
 		Modeler modeler,
 		String zone,
 		boolean dryRun,
-		boolean allowDefaultMode
+		boolean allowDefaultMode,
+		boolean updateImmediately
 	) {
 		AlgorithmResult result = new AlgorithmResult();
 		if (name == null || args == null) {
@@ -212,11 +216,14 @@ public class RRMAlgorithm {
 			}
 			result.channelMap = optimizer.computeChannelMap();
 			if (!dryRun) {
-				optimizer.applyConfig(
+				optimizer.updateDeviceApConfig(
 					deviceDataManager,
 					configManager,
 					result.channelMap
 				);
+				if (updateImmediately) {
+					configManager.queueZoneAndWakeUp(zone);
+				}
 			}
 		} else if (
 			name.equals(RRMAlgorithm.AlgorithmType.OptimizeTxPower.name())
@@ -270,11 +277,14 @@ public class RRMAlgorithm {
 			}
 			result.txPowerMap = optimizer.computeTxPowerMap();
 			if (!dryRun) {
-				optimizer.applyConfig(
+				optimizer.updateDeviceApConfig(
 					deviceDataManager,
 					configManager,
 					result.txPowerMap
 				);
+				if (updateImmediately) {
+					configManager.queueZoneAndWakeUp(zone);
+				}
 			}
 		} else {
 			result.error = String.format("Unknown algorithm: '%s'", name);

src/main/java/com/facebook/openwifirrm/modules/ApiServer.java

@@ -712,7 +712,8 @@ public class ApiServer implements Runnable {
 				modeler,
 				venue,
 				mock,
-				true /* allowDefaultMode */
+				true, /* allowDefaultMode */
+				true /* updateImmediately */
 			);
 			if (result.error != null) {
 				response.status(400);
@@ -918,7 +919,7 @@ public class ApiServer implements Runnable {
 				DeviceConfig networkConfig =
 					gson.fromJson(request.body(), DeviceConfig.class);
 				deviceDataManager.setDeviceNetworkConfig(networkConfig);
-				configManager.wakeUp();
+				configManager.queueAllZonesAndWakeUp();
 
 				// Revalidate data model
 				modeler.revalidate();
@@ -982,7 +983,7 @@ public class ApiServer implements Runnable {
 				DeviceConfig zoneConfig =
 					gson.fromJson(request.body(), DeviceConfig.class);
 				deviceDataManager.setDeviceZoneConfig(zone, zoneConfig);
-				configManager.wakeUp();
+				configManager.queueZoneAndWakeUp(zone);
 
 				// Revalidate data model
 				modeler.revalidate();
@@ -1045,7 +1046,10 @@ public class ApiServer implements Runnable {
 				DeviceConfig apConfig =
 					gson.fromJson(request.body(), DeviceConfig.class);
 				deviceDataManager.setDeviceApConfig(serialNumber, apConfig);
-				configManager.wakeUp();
+				// TODO enable updates to device(s), not just the entire zone
+				final String zone =
+					deviceDataManager.getDeviceZone(serialNumber);
+				configManager.queueZoneAndWakeUp(zone);
 
 				// Revalidate data model
 				modeler.revalidate();
@@ -1118,7 +1122,10 @@ public class ApiServer implements Runnable {
 						.computeIfAbsent(serialNumber, k -> new DeviceConfig())
 						.apply(apConfig);
 				});
-				configManager.wakeUp();
+				final String zone =
+					deviceDataManager.getDeviceZone(serialNumber);
+				// TODO enable updates to device(s), not just the entire zone
+				configManager.queueZoneAndWakeUp(zone);
 
 				// Revalidate data model
 				modeler.revalidate();
@@ -1261,7 +1268,8 @@ public class ApiServer implements Runnable {
 				modeler,
 				zone,
 				dryRun,
-				false /* allowDefaultMode */
+				false, /* allowDefaultMode */
+				true /* updateImmediately */
 			);
 			if (result.error != null) {
 				response.status(400);
@@ -1372,7 +1380,8 @@ public class ApiServer implements Runnable {
 				modeler,
 				zone,
 				dryRun,
-				false /* allowDefaultMode */
+				false, /* allowDefaultMode */
+				true /* updateImmediately */
 			);
 			if (result.error != null) {
 				response.status(400);

src/main/java/com/facebook/openwifirrm/modules/ConfigManager.java

@@ -10,9 +10,12 @@ package com.facebook.openwifirrm.modules;
 
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.slf4j.Logger;
@@ -63,8 +66,11 @@ public class ConfigManager implements Runnable {
 	/** Is the main thread sleeping? */
 	private final AtomicBoolean sleepingFlag = new AtomicBoolean(false);
 
-	/** Was a manual config update requested? */
-	private final AtomicBoolean eventFlag = new AtomicBoolean(false);
+	/**
+	 * Thread-safe set of zones for which manual config updates have been
+	 * requested.
+	 */
+	private Set<String> zonesToUpdate = ConcurrentHashMap.newKeySet();
 
 	/** Config listener interface. */
 	public interface ConfigListener {
@@ -180,7 +186,10 @@ public class ConfigManager implements Runnable {
 		List<String> devicesNeedingUpdate = new ArrayList<>();
 		final long CONFIG_DEBOUNCE_INTERVAL_NS =
 			params.configDebounceIntervalSec * 1_000_000_000L;
-		final boolean isEvent = eventFlag.getAndSet(false);
+		Set<String> zonesToUpdateCopy = new HashSet<>(zonesToUpdate);
+		// use removeAll() instead of clear() in case items are added between
+		// the previous line and the following line
+		zonesToUpdate.removeAll(zonesToUpdateCopy);
 		for (DeviceWithStatus device : devices) {
 			// Update config structure
 			DeviceData data = deviceDataMap.computeIfAbsent(
@@ -201,11 +210,13 @@ public class ConfigManager implements Runnable {
 			for (ConfigListener listener : configListeners.values()) {
 				listener.receiveDeviceConfig(device.serialNumber, data.config);
 			}
-
-			// Check event flag
+			// Check if there are requested updates for this zone
+			String deviceZone =
+				deviceDataManager.getDeviceZone(device.serialNumber);
+			boolean isEvent = zonesToUpdateCopy.contains(deviceZone);
 			if (params.configOnEventOnly && !isEvent) {
 				logger.debug(
-					"Skipping config for {} (event flag not set)",
+					"Skipping config for {} (zone not marked for updates)",
 					device.serialNumber
 				);
 				continue;
@@ -251,15 +262,16 @@ public class ConfigManager implements Runnable {
 			}
 		}
 
+		final boolean shouldUpdate = !zonesToUpdateCopy.isEmpty();
 		// Send config changes to devices
 		if (!params.configEnabled) {
 			logger.trace("Config changes are disabled.");
 		} else if (devicesNeedingUpdate.isEmpty()) {
 			logger.debug("No device configs to send.");
-		} else if (params.configOnEventOnly && !isEvent) {
+		} else if (params.configOnEventOnly && !shouldUpdate) {
 			// shouldn't happen
 			logger.error(
-				"ERROR!! {} device(s) queued for config update, but event flag not set",
+				"ERROR!! {} device(s) queued for config update, but no zones queued for update.",
 				devicesNeedingUpdate.size()
 			);
 		} else {
@@ -364,9 +376,38 @@ public class ConfigManager implements Runnable {
 		return (configListeners.remove(id) != null);
 	}
 
-	/** Interrupt the main thread, possibly triggering an update immediately. */
-	public void wakeUp() {
-		eventFlag.set(true);
+	/**
+	 * Mark the zone to be updated, then interrupt the main thread to possibly
+	 * trigger an update immediately.
+	 *
+	 * @param zone non-null zone (i.e., venue)
+	 */
+	public void queueZoneAndWakeUp(String zone) {
+		if (zone == null) {
+			logger.debug("Zone to queue must be a non-null String.");
+			return;
+		}
+		zonesToUpdate.add(zone);
+		wakeUp();
+	}
+
+	/**
+	 * Track all zones to be updated, then interrupt the main thread to possibly
+	 * trigger an update immediately.
+	 */
+	public void queueAllZonesAndWakeUp() {
+		/*
+		 * Note, addAll is not atomic, but that is ok. This just means that it
+		 * is possible that some zones may get updated now by the main thread
+		 * while others get updated either when the main thread is woken up or
+		 * the next time the main thread does its periodic update.
+		 */
+		zonesToUpdate.addAll(deviceDataManager.getZones());
+		wakeUp();
+	}
+
+	/** Interrupt the main thread to possibly trigger an update immediately. */
+	private void wakeUp() {
 		if (mainThread != null && mainThread.isAlive() && sleepingFlag.get()) {
 			wakeupFlag.set(true);
 			mainThread.interrupt();

src/main/java/com/facebook/openwifirrm/modules/RRMScheduler.java

@@ -8,15 +8,15 @@
 
 package com.facebook.openwifirrm.modules;
 
+import java.text.ParseException;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Properties;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.text.ParseException;
 
-import org.quartz.CronScheduleBuilder;
 import org.quartz.CronExpression;
+import org.quartz.CronScheduleBuilder;
 import org.quartz.Job;
 import org.quartz.JobBuilder;
 import org.quartz.JobDetail;
@@ -332,7 +332,8 @@ public class RRMScheduler {
 				modeler,
 				zone,
 				params.dryRun,
-				true /* allowDefaultMode */
+				true, /* allowDefaultMode */
+				false /* updateImmediately */
 			);
 			logger.info(
 				"'{}' result for zone '{}': {}",
@@ -341,5 +342,6 @@ public class RRMScheduler {
 				gson.toJson(result)
 			);
 		}
+		configManager.queueZoneAndWakeUp(zone);
 	}
 }

src/main/java/com/facebook/openwifirrm/optimizers/channel/ChannelOptimizer.java

@@ -638,14 +638,13 @@ public abstract class ChannelOptimizer {
 	public abstract Map<String, Map<String, Integer>> computeChannelMap();
 
 	/**
-	 * Program the given channel map into the AP config and notify the config
-	 * manager.
+	 * Program the given channel map into the AP config.
 	 *
 	 * @param deviceDataManager the DeviceDataManager instance
 	 * @param configManager the ConfigManager instance
 	 * @param channelMap the map of devices (by serial number) to radio to channel
 	 */
-	public void applyConfig(
+	public void updateDeviceApConfig(
 		DeviceDataManager deviceDataManager,
 		ConfigManager configManager,
 		Map<String, Map<String, Integer>> channelMap
@@ -663,8 +662,5 @@ public abstract class ChannelOptimizer {
 				deviceConfig.autoChannels = entry.getValue();
 			}
 		});
-
-		// Trigger config update now
-		configManager.wakeUp();
 	}
 }

src/main/java/com/facebook/openwifirrm/optimizers/tpc/TPC.java

@@ -151,14 +151,13 @@ public abstract class TPC {
 	public abstract Map<String, Map<String, Integer>> computeTxPowerMap();
 
 	/**
-	 * Program the given tx power map into the AP config and notify the config
-	 * manager.
+	 * Program the given tx power map into the AP config.
 	 *
 	 * @param deviceDataManager the DeviceDataManager instance
 	 * @param configManager the ConfigManager instance
 	 * @param txPowerMap the map of devices (by serial number) to radio to tx power
 	 */
-	public void applyConfig(
+	public void updateDeviceApConfig(
 		DeviceDataManager deviceDataManager,
 		ConfigManager configManager,
 		Map<String, Map<String, Integer>> txPowerMap
@@ -176,9 +175,6 @@ public abstract class TPC {
 				deviceConfig.autoTxPowers = entry.getValue();
 			}
 		});
-
-		// Trigger config update now
-		configManager.wakeUp();
 	}
 
 	/**