From c3b51aeafdd05de721f901a0c5e2b35e62455fa2 Mon Sep 17 00:00:00 2001
From: RockyMandayam2 <106835690+RockyMandayam2@users.noreply.github.com>
Date: Fri, 30 Sep 2022 15:23:26 -0700
Subject: [PATCH] [WIFI-10943] Deal with "auto" value for channel and fix 80p80
 representation (#92) (#93)

Co-authored-by: Jun Woo Shin <jwoos@users.noreply.github.com>
---
 .../optimizers/channel/ChannelOptimizer.java  |  5 +-
 .../openwifirrm/ucentral/UCentralUtils.java   | 23 +++++--
 .../ucentral/UCentralUtilsTest.java           | 61 +++++++++++++++++++
 3 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/src/main/java/com/facebook/openwifirrm/optimizers/channel/ChannelOptimizer.java b/src/main/java/com/facebook/openwifirrm/optimizers/channel/ChannelOptimizer.java
index 9846c25..628983c 100644
--- a/src/main/java/com/facebook/openwifirrm/optimizers/channel/ChannelOptimizer.java
+++ b/src/main/java/com/facebook/openwifirrm/optimizers/channel/ChannelOptimizer.java
@@ -234,8 +234,9 @@ public abstract class ChannelOptimizer {
 				// the difference of 8 means it is consecutive
 				int channelDiff =
 					Math.abs(vhtOperObj.channel1 - vhtOperObj.channel2);
-				// the "8080" below does not mean 8080 MHz wide, it refers to 80+80 MHz channel
-				return channelDiff == 8 ? 160 : 8080;
+				// TODO it will currently return just 80 for 80p80 - it should be dealt
+				// with properly.
+				return channelDiff == 8 ? 160 : 80;
 			} else {
 				return MIN_CHANNEL_WIDTH;
 			}
diff --git a/src/main/java/com/facebook/openwifirrm/ucentral/UCentralUtils.java b/src/main/java/com/facebook/openwifirrm/ucentral/UCentralUtils.java
index 11f10a6..8e68729 100644
--- a/src/main/java/com/facebook/openwifirrm/ucentral/UCentralUtils.java
+++ b/src/main/java/com/facebook/openwifirrm/ucentral/UCentralUtils.java
@@ -130,9 +130,24 @@ public class UCentralUtils {
 				continue;
 			}
 
-			// Compare vs. existing value
-			int currentValue = radioConfig.get(fieldName).getAsInt();
-			if (currentValue == newValue) {
+			// Compare vs. existing value.
+			// not all values are int so override those values
+			Integer currentValue = null;
+			JsonElement fieldValue = radioConfig.get(fieldName);
+			if (
+				fieldValue.isJsonPrimitive() &&
+					fieldValue.getAsJsonPrimitive().isNumber()
+			) {
+				currentValue = fieldValue.getAsInt();
+			} else {
+				logger.debug(
+					"Unable to get field '{}' as int, value was {}",
+					fieldName,
+					fieldValue.toString()
+				);
+			}
+
+			if (currentValue != null && currentValue == newValue) {
 				logger.info(
 					"Device {}: {} {} is already {}",
 					serialNumber,
@@ -150,7 +165,7 @@ public class UCentralUtils {
 					operationalBand,
 					fieldName,
 					newValue,
-					currentValue
+					currentValue != null ? currentValue : fieldValue.toString()
 				);
 				wasModified = true;
 			}
diff --git a/src/test/java/com/facebook/openwifirrm/ucentral/UCentralUtilsTest.java b/src/test/java/com/facebook/openwifirrm/ucentral/UCentralUtilsTest.java
index 27c0e48..a1717f8 100644
--- a/src/test/java/com/facebook/openwifirrm/ucentral/UCentralUtilsTest.java
+++ b/src/test/java/com/facebook/openwifirrm/ucentral/UCentralUtilsTest.java
@@ -8,7 +8,12 @@
 
 package com.facebook.openwifirrm.ucentral;
 
+import java.util.Collections;
+import java.util.Map;
+
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 
 import org.junit.jupiter.api.Test;
 
@@ -17,4 +22,60 @@ public class UCentralUtilsTest {
 	void test_placeholder() throws Exception {
 		assertEquals(3, 1 + 2);
 	}
+
+	@Test
+	void test_setRadioConfigFieldChannel() throws Exception {
+		final String serialNumber = "aaaaaaaaaaaa";
+		final int expectedChannel = 1;
+		final Map<String, Integer> newValueList = Collections
+			.singletonMap(UCentralConstants.BAND_5G, expectedChannel);
+
+		// test case where channel value is a string and not an integer
+		UCentralApConfiguration config = new UCentralApConfiguration(
+			"{\"interfaces\": [], \"radios\": [{\"band\": \"5G\", \"channel\": \"auto\"}]}"
+		);
+		boolean modified = UCentralUtils
+			.setRadioConfigField(serialNumber, config, "channel", newValueList);
+		assertTrue(modified);
+		assertEquals(
+			config.getRadioConfig(0).get("channel").getAsInt(),
+			expectedChannel
+		);
+
+		// field doesn't exist
+		config = new UCentralApConfiguration(
+			"{\"interfaces\": [], \"radios\": [{\"band\": \"5G\"}]}"
+		);
+		modified = UCentralUtils
+			.setRadioConfigField(serialNumber, config, "channel", newValueList);
+		assertTrue(modified);
+		assertEquals(
+			config.getRadioConfig(0).get("channel").getAsInt(),
+			expectedChannel
+		);
+
+		// normal field, not modified
+		config = new UCentralApConfiguration(
+			"{\"interfaces\": [], \"radios\": [{\"band\": \"5G\", \"channel\": 1}]}"
+		);
+		modified = UCentralUtils
+			.setRadioConfigField(serialNumber, config, "channel", newValueList);
+		assertFalse(modified);
+		assertEquals(
+			config.getRadioConfig(0).get("channel").getAsInt(),
+			expectedChannel
+		);
+
+		// normal field, modified
+		config = new UCentralApConfiguration(
+			"{\"interfaces\": [], \"radios\": [{\"band\": \"5G\", \"channel\": 15}]}"
+		);
+		modified = UCentralUtils
+			.setRadioConfigField(serialNumber, config, "channel", newValueList);
+		assertTrue(modified);
+		assertEquals(
+			config.getRadioConfig(0).get("channel").getAsInt(),
+			expectedChannel
+		);
+	}
 }