Move adjustCandidateWithUserSelection to WifiNetworkSelector
WifiNetworkSelector#setUserConnectChoice contains all of the logic for
favoring a user selected network over other visible networks present at
the last round on network selection. However, the logic for using this
data is contained in
SavedNetworkEvaluator#adjustCandidateWithUserSelection. This has the
side effect that RecommendedNetworkEvaluator or any new NetworkEvaluator
in the future can override the user's explicit intent to choose a
specific network. This change moves adjustCandidateWithUserSelection to
WifiNetworkSelector, fixing this bug and bringing all user connect
choice logic into the same class, WifiNetworkSelector.
Set a candidate ScanResult for every WifiConfiguration in
SavedNetworkEvaluator, regardless of whether it is externally scored.
This ensures that the user connect choice is respected if an externally
scored network is selected by a user over a secure saved network.
Bug: 34971941
Test: ./runtests.sh
Change-Id: I8730aee89dbee0ebb97161a56f00c026139ef5c9
diff --git a/service/java/com/android/server/wifi/SavedNetworkEvaluator.java b/service/java/com/android/server/wifi/SavedNetworkEvaluator.java
index 6ca88ba..25591f5 100644
--- a/service/java/com/android/server/wifi/SavedNetworkEvaluator.java
+++ b/service/java/com/android/server/wifi/SavedNetworkEvaluator.java
@@ -230,32 +230,6 @@
return score;
}
- private WifiConfiguration adjustCandidateWithUserSelection(WifiConfiguration candidate,
- ScanResult scanResultCandidate) {
- WifiConfiguration tempConfig = candidate;
-
- while (tempConfig.getNetworkSelectionStatus().getConnectChoice() != null) {
- String key = tempConfig.getNetworkSelectionStatus().getConnectChoice();
- tempConfig = mWifiConfigManager.getConfiguredNetwork(key);
-
- if (tempConfig != null) {
- WifiConfiguration.NetworkSelectionStatus tempStatus =
- tempConfig.getNetworkSelectionStatus();
- if (tempStatus.getCandidate() != null && tempStatus.isNetworkEnabled()) {
- scanResultCandidate = tempStatus.getCandidate();
- candidate = tempConfig;
- }
- } else {
- localLog("Connect choice: " + key + " has no corresponding saved config.");
- break;
- }
- }
- localLog("After user selection adjustment, the final candidate is:"
- + WifiNetworkSelector.toNetworkString(candidate) + " : "
- + scanResultCandidate.BSSID);
- return candidate;
- }
-
/**
* Evaluate all the networks from the scan results and return
* the WifiConfiguration of the network chosen for connection.
@@ -275,7 +249,6 @@
for (ScanDetail scanDetail : scanDetails) {
ScanResult scanResult = scanDetail.getScanResult();
int highestScoreOfScanResult = Integer.MIN_VALUE;
- int score;
int candidateIdOfScanResult = WifiConfiguration.INVALID_NETWORK_ID;
// One ScanResult can be associated with more than one networks, hence we calculate all
@@ -310,8 +283,22 @@
continue;
}
- // If the network is marked to use external scores, leave it to the
- // external score evaluator to handle it.
+ int score = calculateBssidScore(scanResult, network, currentNetwork, currentBssid,
+ scoreHistory);
+
+ // Set candidate ScanResult for all saved networks to ensure that users can
+ // override network selection. See WifiNetworkSelector#setUserConnectChoice.
+ // TODO(b/36067705): consider alternative designs to push filtering/selecting of
+ // user connect choice networks to RecommendedNetworkEvaluator.
+ if (score > status.getCandidateScore() || (score == status.getCandidateScore()
+ && status.getCandidate() != null
+ && scanResult.level > status.getCandidate().level)) {
+ mWifiConfigManager.setNetworkCandidateScanResult(
+ network.networkId, scanResult, score);
+ }
+
+ // If the network is marked to use external scores, or is an open network with
+ // curate saved open networks enabled, do not consider it for network selection.
if (network.useExternalScores) {
localLog("Network " + WifiNetworkSelector.toNetworkString(network)
+ " has external score.");
@@ -325,20 +312,10 @@
continue;
}
- score = calculateBssidScore(scanResult, network, currentNetwork, currentBssid,
- scoreHistory);
-
if (score > highestScoreOfScanResult) {
highestScoreOfScanResult = score;
candidateIdOfScanResult = network.networkId;
}
-
- if (score > status.getCandidateScore() || (score == status.getCandidateScore()
- && status.getCandidate() != null
- && scanResult.level > status.getCandidate().level)) {
- mWifiConfigManager.setNetworkCandidateScanResult(
- candidateIdOfScanResult, scanResult, score);
- }
}
if (connectableNetworks != null) {
@@ -363,11 +340,9 @@
localLog("\n" + scoreHistory.toString());
}
- if (scanResultCandidate != null) {
- return adjustCandidateWithUserSelection(candidate, scanResultCandidate);
- } else {
+ if (scanResultCandidate == null) {
localLog("did not see any good candidates.");
- return null;
}
+ return candidate;
}
}
diff --git a/service/java/com/android/server/wifi/WifiNetworkSelector.java b/service/java/com/android/server/wifi/WifiNetworkSelector.java
index 6521cc3..956fe62 100644
--- a/service/java/com/android/server/wifi/WifiNetworkSelector.java
+++ b/service/java/com/android/server/wifi/WifiNetworkSelector.java
@@ -16,6 +16,7 @@
package com.android.server.wifi;
+import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.ActivityManager;
import android.content.Context;
@@ -399,6 +400,39 @@
}
/**
+ * Overrides the {@code candidate} chosen by the {@link #mEvaluators} with the user chosen
+ * {@link WifiConfiguration} if one exists.
+ *
+ * @return the user chosen {@link WifiConfiguration} if one exists, {@code candidate} otherwise
+ */
+ private WifiConfiguration overrideCandidateWithUserConnectChoice(
+ @NonNull WifiConfiguration candidate) {
+ WifiConfiguration tempConfig = candidate;
+ ScanResult scanResultCandidate = candidate.getNetworkSelectionStatus().getCandidate();
+
+ while (tempConfig.getNetworkSelectionStatus().getConnectChoice() != null) {
+ String key = tempConfig.getNetworkSelectionStatus().getConnectChoice();
+ tempConfig = mWifiConfigManager.getConfiguredNetwork(key);
+
+ if (tempConfig != null) {
+ WifiConfiguration.NetworkSelectionStatus tempStatus =
+ tempConfig.getNetworkSelectionStatus();
+ if (tempStatus.getCandidate() != null && tempStatus.isNetworkEnabled()) {
+ scanResultCandidate = tempStatus.getCandidate();
+ candidate = tempConfig;
+ }
+ } else {
+ localLog("Connect choice: " + key + " has no corresponding saved config.");
+ break;
+ }
+ }
+ localLog("After user selection adjustment, the final candidate is:"
+ + WifiNetworkSelector.toNetworkString(candidate) + " : "
+ + scanResultCandidate.BSSID);
+ return candidate;
+ }
+
+ /**
* Enable/disable a BSSID for Network Selection
* When an association rejection event is obtained, Network Selector will disable this
* BSSID but supplicant still can try to connect to this bssid. If supplicant connect to it
@@ -519,6 +553,7 @@
}
if (selectedNetwork != null) {
+ selectedNetwork = overrideCandidateWithUserConnectChoice(selectedNetwork);
mLastNetworkSelectionTimeStamp = mClock.getElapsedSinceBootMillis();
}
diff --git a/service/tests/wifitests/src/com/android/server/wifi/SavedNetworkEvaluatorTest.java b/service/tests/wifitests/src/com/android/server/wifi/SavedNetworkEvaluatorTest.java
index dcd2b8e..a258238 100644
--- a/service/tests/wifitests/src/com/android/server/wifi/SavedNetworkEvaluatorTest.java
+++ b/service/tests/wifitests/src/com/android/server/wifi/SavedNetworkEvaluatorTest.java
@@ -219,6 +219,49 @@
assertNull(candidate);
}
+ /**
+ * Set the candidate {@link ScanResult} for all {@link WifiConfiguration}s even if they have
+ * useExternalScores set or are open networks with
+ * {@link Settings.Global.CURATE_SAVED_OPEN_NETWORKS} and
+ * {@link Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED} enabled.
+ */
+ @Test
+ public void setCandidateScanResultsForAllSavedNetworks() {
+ String[] ssids = {"\"test1\"", "\"test2\""};
+ String[] bssids = {"6c:f3:7f:ae:8c:f3", "6c:f3:7f:ae:8c:f4"};
+ int[] freqs = {5200, 5240};
+ String[] caps = {"[WPA2-EAP-CCMP][ESS]", "[ESS]"};
+ int[] levels = {mThresholdQualifiedRssi5G, mThresholdQualifiedRssi5G};
+ int[] securities = {SECURITY_PSK, SECURITY_NONE};
+
+ when(mFrameworkFacade.getIntegerSetting(mContext,
+ Settings.Global.CURATE_SAVED_OPEN_NETWORKS, 0)).thenReturn(1);
+ when(mFrameworkFacade.getIntegerSetting(mContext,
+ Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED, 0)).thenReturn(1);
+ mContentObserver.onChange(false);
+
+ ScanDetailsAndWifiConfigs scanDetailsAndConfigs =
+ WifiNetworkSelectorTestUtil.setupScanDetailsAndConfigStore(ssids, bssids,
+ freqs, caps, levels, securities, mWifiConfigManager, mClock);
+ List<ScanDetail> scanDetails = scanDetailsAndConfigs.getScanDetails();
+ WifiConfiguration useExternalScoresConfig = scanDetailsAndConfigs.getWifiConfigs()[0];
+ useExternalScoresConfig.useExternalScores = true;
+ WifiConfiguration openNetworkConfig = scanDetailsAndConfigs.getWifiConfigs()[1];
+
+ WifiConfiguration candidate = mSavedNetworkEvaluator.evaluateNetworks(scanDetails,
+ null, null, true, false, null);
+
+ assertNull(candidate);
+
+ verify(mWifiConfigManager).setNetworkCandidateScanResult(
+ eq(useExternalScoresConfig.networkId),
+ eq(scanDetails.get(0).getScanResult()),
+ anyInt());
+ verify(mWifiConfigManager).setNetworkCandidateScanResult(
+ eq(openNetworkConfig.networkId),
+ eq(scanDetails.get(1).getScanResult()),
+ anyInt());
+ }
/**
* Between two 2G networks, choose the one with stronger RSSI value if other conditions
diff --git a/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkSelectorTest.java b/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkSelectorTest.java
index 5fea90b..4a38f40 100644
--- a/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkSelectorTest.java
+++ b/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkSelectorTest.java
@@ -24,9 +24,9 @@
import android.content.Context;
import android.content.res.Resources;
-import android.net.NetworkScoreManager;
import android.net.wifi.ScanResult;
import android.net.wifi.WifiConfiguration;
+import android.net.wifi.WifiConfiguration.NetworkSelectionStatus;
import android.net.wifi.WifiInfo;
import android.os.SystemClock;
import android.test.suitebuilder.annotation.SmallTest;
@@ -38,6 +38,8 @@
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
import java.util.List;
@@ -50,10 +52,11 @@
/** Sets up test. */
@Before
public void setUp() throws Exception {
- mResource = getResource();
- mContext = getContext();
- mWifiConfigManager = getWifiConfigManager();
- mWifiInfo = getWifiInfo();
+ MockitoAnnotations.initMocks(this);
+ setupContext();
+ setupResources();
+ setupWifiConfigManager();
+ setupWifiInfo();
mWifiNetworkSelector = new WifiNetworkSelector(mContext, mWifiConfigManager, mClock);
mWifiNetworkSelector.registerNetworkEvaluator(mDummyEvaluator, 1);
@@ -81,24 +84,19 @@
*/
public class DummyNetworkEvaluator implements WifiNetworkSelector.NetworkEvaluator {
private static final String NAME = "DummyNetworkEvaluator";
- private WifiConfigManager mConfigManager;
- /**
- * Get the evaluator name.
- */
+ @Override
public String getName() {
return NAME;
}
- /**
- * Update thee evaluator.
- */
- public void update(List<ScanDetail> scanDetails) {
- }
+ @Override
+ public void update(List<ScanDetail> scanDetails) {}
/**
* Always return the first network in the scan results for connection.
*/
+ @Override
public WifiConfiguration evaluateNetworks(List<ScanDetail> scanDetails,
WifiConfiguration currentNetwork, String currentBssid, boolean connected,
boolean untrustedNetworkAllowed,
@@ -112,70 +110,51 @@
private WifiNetworkSelector mWifiNetworkSelector = null;
private DummyNetworkEvaluator mDummyEvaluator = new DummyNetworkEvaluator();
- private WifiConfigManager mWifiConfigManager = null;
- private Context mContext;
- private Resources mResource;
- private WifiInfo mWifiInfo;
- private Clock mClock = mock(Clock.class);
+ @Mock private WifiConfigManager mWifiConfigManager;
+ @Mock private Context mContext;
+ @Mock private Resources mResource;
+ @Mock private WifiInfo mWifiInfo;
+ @Mock private Clock mClock;
private int mThresholdMinimumRssi2G;
private int mThresholdMinimumRssi5G;
private int mThresholdQualifiedRssi2G;
private int mThresholdQualifiedRssi5G;
- Context getContext() {
- Context context = mock(Context.class);
- Resources resource = mock(Resources.class);
-
- when(context.getResources()).thenReturn(mResource);
- return context;
+ private void setupContext() {
+ when(mContext.getResources()).thenReturn(mResource);
}
- Resources getResource() {
- Resources resource = mock(Resources.class);
-
- when(resource.getBoolean(
+ private void setupResources() {
+ when(mResource.getBoolean(
R.bool.config_wifi_framework_enable_associated_network_selection)).thenReturn(true);
- when(resource.getInteger(
+ when(mResource.getInteger(
R.integer.config_wifi_framework_wifi_score_low_rssi_threshold_5GHz))
.thenReturn(-70);
- when(resource.getInteger(
+ when(mResource.getInteger(
R.integer.config_wifi_framework_wifi_score_low_rssi_threshold_24GHz))
.thenReturn(-73);
- when(resource.getInteger(
+ when(mResource.getInteger(
R.integer.config_wifi_framework_wifi_score_bad_rssi_threshold_5GHz))
.thenReturn(-82);
- when(resource.getInteger(
+ when(mResource.getInteger(
R.integer.config_wifi_framework_wifi_score_bad_rssi_threshold_24GHz))
.thenReturn(-85);
- return resource;
}
- NetworkScoreManager getNetworkScoreManager() {
- NetworkScoreManager networkScoreManager = mock(NetworkScoreManager.class);
-
- return networkScoreManager;
- }
-
- WifiInfo getWifiInfo() {
- WifiInfo wifiInfo = mock(WifiInfo.class);
-
+ private void setupWifiInfo() {
// simulate a disconnected state
- when(wifiInfo.is24GHz()).thenReturn(true);
- when(wifiInfo.is5GHz()).thenReturn(false);
- when(wifiInfo.getRssi()).thenReturn(-70);
- when(wifiInfo.getNetworkId()).thenReturn(WifiConfiguration.INVALID_NETWORK_ID);
- when(wifiInfo.getBSSID()).thenReturn(null);
- return wifiInfo;
+ when(mWifiInfo.is24GHz()).thenReturn(true);
+ when(mWifiInfo.is5GHz()).thenReturn(false);
+ when(mWifiInfo.getRssi()).thenReturn(-70);
+ when(mWifiInfo.getNetworkId()).thenReturn(WifiConfiguration.INVALID_NETWORK_ID);
+ when(mWifiInfo.getBSSID()).thenReturn(null);
}
- WifiConfigManager getWifiConfigManager() {
- WifiConfigManager wifiConfigManager = mock(WifiConfigManager.class);
- when(wifiConfigManager.getLastSelectedNetwork())
+ private void setupWifiConfigManager() {
+ when(mWifiConfigManager.getLastSelectedNetwork())
.thenReturn(WifiConfiguration.INVALID_NETWORK_ID);
- return wifiConfigManager;
}
-
/**
* No network selection if scan result is empty.
*
@@ -294,6 +273,7 @@
WifiConfiguration[] savedConfigs = scanDetailsAndConfigs.getWifiConfigs();
WifiConfiguration candidate = mWifiNetworkSelector.selectNetwork(scanDetails,
mWifiInfo, false, true, false);
+ WifiConfigurationTestUtil.assertConfigurationEqual(savedConfigs[0], candidate);
when(mClock.getElapsedSinceBootMillis()).thenReturn(SystemClock.elapsedRealtime()
+ WifiNetworkSelector.MINIMUM_NETWORK_SELECTION_INTERVAL_MS - 2000);
@@ -351,7 +331,6 @@
assertEquals("Expect null configuration", null, candidate);
}
-
/**
* New network selection is performed if the currently connected network
* band is 2G.
@@ -572,13 +551,109 @@
}
/**
+ * Ensures that settings the user connect choice updates the
+ * NetworkSelectionStatus#mConnectChoice for all other WifiConfigurations in range in the last
+ * round of network selection.
+ *
+ * Expected behavior: WifiConfiguration.NetworkSelectionStatus#mConnectChoice is set to
+ * test1's configkey for test2. test3's WifiConfiguration is unchanged.
+ */
+ @Test
+ public void setUserConnectChoice() {
+ String[] ssids = {"\"test1\"", "\"test2\"", "\"test3\""};
+ String[] bssids = {"6c:f3:7f:ae:8c:f3", "6c:f3:7f:ae:8c:f4", "6c:f3:7f:ae:8c:f5"};
+ int[] freqs = {2437, 5180, 5181};
+ String[] caps = {"[WPA2-EAP-CCMP][ESS]", "[WPA2-EAP-CCMP][ESS]", "[WPA2-EAP-CCMP][ESS]"};
+ int[] levels = {mThresholdMinimumRssi2G + 1, mThresholdMinimumRssi5G + 1,
+ mThresholdMinimumRssi5G + 1};
+ int[] securities = {SECURITY_PSK, SECURITY_PSK, SECURITY_PSK};
+
+ ScanDetailsAndWifiConfigs scanDetailsAndConfigs =
+ WifiNetworkSelectorTestUtil.setupScanDetailsAndConfigStore(ssids, bssids,
+ freqs, caps, levels, securities, mWifiConfigManager, mClock);
+
+ WifiConfiguration selectedWifiConfig = scanDetailsAndConfigs.getWifiConfigs()[0];
+ selectedWifiConfig.getNetworkSelectionStatus()
+ .setCandidate(scanDetailsAndConfigs.getScanDetails().get(0).getScanResult());
+ selectedWifiConfig.getNetworkSelectionStatus().setNetworkSelectionStatus(
+ NetworkSelectionStatus.NETWORK_SELECTION_PERMANENTLY_DISABLED);
+ selectedWifiConfig.getNetworkSelectionStatus().setConnectChoice("bogusKey");
+
+ WifiConfiguration configInLastNetworkSelection = scanDetailsAndConfigs.getWifiConfigs()[1];
+ configInLastNetworkSelection.getNetworkSelectionStatus()
+ .setSeenInLastQualifiedNetworkSelection(true);
+
+ WifiConfiguration configNotInLastNetworkSelection =
+ scanDetailsAndConfigs.getWifiConfigs()[2];
+
+ assertTrue(mWifiNetworkSelector.setUserConnectChoice(selectedWifiConfig.networkId));
+
+ verify(mWifiConfigManager).updateNetworkSelectionStatus(selectedWifiConfig.networkId,
+ NetworkSelectionStatus.NETWORK_SELECTION_ENABLE);
+ verify(mWifiConfigManager).clearNetworkConnectChoice(selectedWifiConfig.networkId);
+ verify(mWifiConfigManager).setNetworkConnectChoice(configInLastNetworkSelection.networkId,
+ selectedWifiConfig.configKey(), mClock.getWallClockMillis());
+ verify(mWifiConfigManager, never()).setNetworkConnectChoice(
+ configNotInLastNetworkSelection.networkId, selectedWifiConfig.configKey(),
+ mClock.getWallClockMillis());
+ }
+
+ /**
+ * If two qualified networks, test1 and test2, are in range when the user selects test2 over
+ * test1, WifiNetworkSelector will override the NetworkSelector's choice to connect to test1
+ * with test2.
+ *
+ * Expected behavior: test2 is the recommended network
+ */
+ @Test
+ public void userConnectChoiceOverridesNetworkEvaluators() {
+ String[] ssids = {"\"test1\"", "\"test2\""};
+ String[] bssids = {"6c:f3:7f:ae:8c:f3", "6c:f3:7f:ae:8c:f4"};
+ int[] freqs = {2437, 5180};
+ String[] caps = {"[WPA2-EAP-CCMP][ESS]", "[WPA2-EAP-CCMP][ESS]"};
+ int[] levels = {mThresholdMinimumRssi2G + 1, mThresholdMinimumRssi5G + 1};
+ int[] securities = {SECURITY_PSK, SECURITY_PSK};
+
+ ScanDetailsAndWifiConfigs scanDetailsAndConfigs =
+ WifiNetworkSelectorTestUtil.setupScanDetailsAndConfigStore(ssids, bssids,
+ freqs, caps, levels, securities, mWifiConfigManager, mClock);
+ List<ScanDetail> scanDetails = scanDetailsAndConfigs.getScanDetails();
+
+ // DummyEvaluator always selects the first network in the list.
+ WifiConfiguration networkSelectorChoice = scanDetailsAndConfigs.getWifiConfigs()[0];
+ networkSelectorChoice.getNetworkSelectionStatus()
+ .setSeenInLastQualifiedNetworkSelection(true);
+
+ WifiConfiguration userChoice = scanDetailsAndConfigs.getWifiConfigs()[1];
+ userChoice.getNetworkSelectionStatus()
+ .setCandidate(scanDetailsAndConfigs.getScanDetails().get(1).getScanResult());
+
+ // With no user choice set, networkSelectorChoice should be chosen.
+ WifiConfiguration candidate = mWifiNetworkSelector.selectNetwork(scanDetails,
+ mWifiInfo, false, true, false);
+
+ WifiConfigurationTestUtil.assertConfigurationEqual(networkSelectorChoice, candidate);
+
+ when(mClock.getElapsedSinceBootMillis()).thenReturn(SystemClock.elapsedRealtime()
+ + WifiNetworkSelector.MINIMUM_NETWORK_SELECTION_INTERVAL_MS + 2000);
+
+ assertTrue(mWifiNetworkSelector.setUserConnectChoice(userChoice.networkId));
+
+ // After user connect choice is set, userChoice should override networkSelectorChoice.
+ candidate = mWifiNetworkSelector.selectNetwork(scanDetails,
+ mWifiInfo, false, true, false);
+
+ WifiConfigurationTestUtil.assertConfigurationEqual(userChoice, candidate);
+ }
+
+ /**
* Wifi network selector blacklists a BSSID immediately if it's unable to handle
* new stations.
*
* Expected behavior: no network recommended by Network Selector
*/
@Test
- public void blacklistNetworkImmeidatelyIfApHasNoCapacityForNewStation() {
+ public void blacklistNetworkImmediatelyIfApHasNoCapacityForNewStation() {
String[] ssids = {"\"test1\""};
String[] bssids = {"6c:f3:7f:ae:99:f3"};
int[] freqs = {2437};
