Remove shouldDisconnect and redundant transition
This removes the redundant state transition upon
START_CONNECT request.
This also removes the shouldDisconnect parameter
because supplicant will implicitly disconnect current
connected network anyway upon a connect request.
Bug: 36535549
Test: compile, unit tests, integration test
Change-Id: Ib636eb05e9b37e54deeeb1f107dd47da7fe75ad0
diff --git a/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java b/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java
index 0fdcebe..2432db6 100644
--- a/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java
+++ b/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java
@@ -385,18 +385,12 @@
* 5. Select the new network in wpa_supplicant.
*
* @param config WifiConfiguration parameters for the provided network.
- * @param shouldDisconnect whether to trigger a disconnection or not.
* @return {@code true} if it succeeds, {@code false} otherwise
*/
- public boolean connectToNetwork(WifiConfiguration config, boolean shouldDisconnect) {
+ public boolean connectToNetwork(WifiConfiguration config) {
mFrameworkNetworkId = WifiConfiguration.INVALID_NETWORK_ID;
mCurrentNetwork = null;
- logd("connectToNetwork " + config.configKey()
- + " (shouldDisconnect " + shouldDisconnect + ")");
- if (shouldDisconnect && !disconnect()) {
- loge("Failed to trigger disconnect");
- return false;
- }
+ logd("connectToNetwork " + config.configKey());
if (!removeAllNetworks()) {
loge("Failed to remove existing networks");
return false;
@@ -430,7 +424,7 @@
if (mFrameworkNetworkId != config.networkId || mCurrentNetwork == null) {
Log.w(TAG, "Cannot roam to a different network, initiate new connection. "
+ "Current network ID: " + mFrameworkNetworkId);
- return connectToNetwork(config, false);
+ return connectToNetwork(config);
}
String bssid = config.getNetworkSelectionStatus().getNetworkSelectionBSSID();
logd("roamToNetwork" + config.configKey() + " (bssid " + bssid + ")");
diff --git a/service/java/com/android/server/wifi/WifiNative.java b/service/java/com/android/server/wifi/WifiNative.java
index 68da5fb..cae705a 100644
--- a/service/java/com/android/server/wifi/WifiNative.java
+++ b/service/java/com/android/server/wifi/WifiNative.java
@@ -673,19 +673,17 @@
/**
* Add the provided network configuration to wpa_supplicant and initiate connection to it.
* This method does the following:
- * 1. Triggers disconnect command to wpa_supplicant (if |shouldDisconnect| is true).
- * 2. Remove any existing network in wpa_supplicant.
- * 3. Add a new network to wpa_supplicant.
- * 4. Save the provided configuration to wpa_supplicant.
- * 5. Select the new network in wpa_supplicant.
- * 6. Triggers reconnect command to wpa_supplicant.
+ * 1. Remove any existing network in wpa_supplicant(This implicitly triggers disconnect).
+ * 2. Add a new network to wpa_supplicant.
+ * 3. Save the provided configuration to wpa_supplicant.
+ * 4. Select the new network in wpa_supplicant.
+ * 5. Triggers reconnect command to wpa_supplicant.
*
* @param configuration WifiConfiguration parameters for the provided network.
- * @param shouldDisconnect whether to trigger a disconnection or not.
* @return {@code true} if it succeeds, {@code false} otherwise
*/
- public boolean connectToNetwork(WifiConfiguration configuration, boolean shouldDisconnect) {
- return mSupplicantStaIfaceHal.connectToNetwork(configuration, shouldDisconnect);
+ public boolean connectToNetwork(WifiConfiguration configuration) {
+ return mSupplicantStaIfaceHal.connectToNetwork(configuration);
}
/**
diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java
index 5c5ace5..50c8f4e 100644
--- a/service/java/com/android/server/wifi/WifiStateMachine.java
+++ b/service/java/com/android/server/wifi/WifiStateMachine.java
@@ -5011,17 +5011,14 @@
reportConnectionAttemptStart(config, mTargetRoamBSSID,
WifiMetricsProto.ConnectionEvent.ROAM_UNRELATED);
- boolean shouldDisconnect = (getCurrentState() != mDisconnectedState);
- if (mWifiNative.connectToNetwork(config, shouldDisconnect)) {
+ if (mWifiNative.connectToNetwork(config)) {
lastConnectAttemptTimestamp = mClock.getWallClockMillis();
targetWificonfiguration = config;
mAutoRoaming = false;
if (isRoaming() || isLinkDebouncing()) {
transitionTo(mRoamingState);
- } else if (shouldDisconnect) {
+ } else if (getCurrentState() != mDisconnectedState) {
transitionTo(mDisconnectingState);
- } else {
- transitionTo(mDisconnectedState);
}
} else {
loge("CMD_START_CONNECT Failed to start connection to network " + config);
diff --git a/service/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java b/service/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java
index de96a7a..abf533d 100644
--- a/service/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java
+++ b/service/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java
@@ -448,30 +448,21 @@
}
/**
- * Tests connection to a specified network without triggering disconnect.
+ * Tests connection to a specified network with empty existing network.
*/
@Test
- public void testConnectWithNoDisconnectAndEmptyExistingNetworks() throws Exception {
+ public void testConnectWithEmptyExistingNetwork() throws Exception {
executeAndValidateInitializationSequence();
- executeAndValidateConnectSequence(0, false, false);
+ executeAndValidateConnectSequence(0, false);
}
/**
- * Tests connection to a specified network without triggering disconnect.
+ * Tests connection to a specified network with single existing network.
*/
@Test
- public void testConnectWithNoDisconnectAndSingleExistingNetwork() throws Exception {
+ public void testConnectWithSingleExistingNetwork() throws Exception {
executeAndValidateInitializationSequence();
- executeAndValidateConnectSequence(0, true, false);
- }
-
- /**
- * Tests connection to a specified network, with a triggered disconnect.
- */
- @Test
- public void testConnectWithDisconnectAndSingleExistingNetwork() throws Exception {
- executeAndValidateInitializationSequence();
- executeAndValidateConnectSequence(0, false, true);
+ executeAndValidateConnectSequence(0, true);
}
/**
@@ -489,7 +480,7 @@
}).when(mISupplicantStaIfaceMock).addNetwork(
any(ISupplicantStaIface.addNetworkCallback.class));
- assertFalse(mDut.connectToNetwork(new WifiConfiguration(), false));
+ assertFalse(mDut.connectToNetwork(new WifiConfiguration()));
}
/**
@@ -503,7 +494,7 @@
when(mSupplicantStaNetworkMock.saveWifiConfiguration(any(WifiConfiguration.class)))
.thenReturn(false);
- assertFalse(mDut.connectToNetwork(new WifiConfiguration(), false));
+ assertFalse(mDut.connectToNetwork(new WifiConfiguration()));
// We should have removed the existing network once before connection and once more
// on failure to save network configuration.
verify(mISupplicantStaIfaceMock, times(2)).removeNetwork(anyInt());
@@ -519,7 +510,7 @@
when(mSupplicantStaNetworkMock.select()).thenReturn(false);
- assertFalse(mDut.connectToNetwork(new WifiConfiguration(), false));
+ assertFalse(mDut.connectToNetwork(new WifiConfiguration()));
}
/**
@@ -547,7 +538,7 @@
public void testRoamFailureDueToBssidSet() throws Exception {
executeAndValidateInitializationSequence();
int connectedNetworkId = 5;
- executeAndValidateConnectSequence(connectedNetworkId, false, false);
+ executeAndValidateConnectSequence(connectedNetworkId, false);
when(mSupplicantStaNetworkMock.setBssid(anyString())).thenReturn(false);
WifiConfiguration roamingConfig = new WifiConfiguration();
@@ -586,7 +577,7 @@
public void testRoamFailureDueToReassociate() throws Exception {
executeAndValidateInitializationSequence();
int connectedNetworkId = 5;
- executeAndValidateConnectSequence(connectedNetworkId, false, false);
+ executeAndValidateConnectSequence(connectedNetworkId, false);
doAnswer(new MockAnswerUtil.AnswerWithArguments() {
public SupplicantStatus answer() throws RemoteException {
@@ -613,7 +604,7 @@
// Return null when not connected to the network.
assertTrue(mDut.getCurrentNetworkWpsNfcConfigurationToken() == null);
verify(mSupplicantStaNetworkMock, never()).getWpsNfcConfigurationToken();
- executeAndValidateConnectSequence(4, false, false);
+ executeAndValidateConnectSequence(4, false);
assertEquals(token, mDut.getCurrentNetworkWpsNfcConfigurationToken());
verify(mSupplicantStaNetworkMock).getWpsNfcConfigurationToken();
}
@@ -630,7 +621,7 @@
// Fail when not connected to a network.
assertFalse(mDut.setCurrentNetworkBssid(bssidStr));
verify(mSupplicantStaNetworkMock, never()).setBssid(eq(bssidStr));
- executeAndValidateConnectSequence(4, false, false);
+ executeAndValidateConnectSequence(4, false);
assertTrue(mDut.setCurrentNetworkBssid(bssidStr));
verify(mSupplicantStaNetworkMock).setBssid(eq(bssidStr));
}
@@ -648,7 +639,7 @@
// Fail when not connected to a network.
assertFalse(mDut.sendCurrentNetworkEapIdentityResponse(identity));
verify(mSupplicantStaNetworkMock, never()).sendNetworkEapIdentityResponse(eq(identity));
- executeAndValidateConnectSequence(4, false, false);
+ executeAndValidateConnectSequence(4, false);
assertTrue(mDut.sendCurrentNetworkEapIdentityResponse(identity));
verify(mSupplicantStaNetworkMock).sendNetworkEapIdentityResponse(eq(identity));
}
@@ -666,7 +657,7 @@
// Fail when not connected to a network.
assertFalse(mDut.sendCurrentNetworkEapSimGsmAuthResponse(params));
verify(mSupplicantStaNetworkMock, never()).sendNetworkEapSimGsmAuthResponse(eq(params));
- executeAndValidateConnectSequence(4, false, false);
+ executeAndValidateConnectSequence(4, false);
assertTrue(mDut.sendCurrentNetworkEapSimGsmAuthResponse(params));
verify(mSupplicantStaNetworkMock).sendNetworkEapSimGsmAuthResponse(eq(params));
}
@@ -684,7 +675,7 @@
// Fail when not connected to a network.
assertFalse(mDut.sendCurrentNetworkEapSimUmtsAuthResponse(params));
verify(mSupplicantStaNetworkMock, never()).sendNetworkEapSimUmtsAuthResponse(eq(params));
- executeAndValidateConnectSequence(4, false, false);
+ executeAndValidateConnectSequence(4, false);
assertTrue(mDut.sendCurrentNetworkEapSimUmtsAuthResponse(params));
verify(mSupplicantStaNetworkMock).sendNetworkEapSimUmtsAuthResponse(eq(params));
}
@@ -702,7 +693,7 @@
// Fail when not connected to a network.
assertFalse(mDut.sendCurrentNetworkEapSimUmtsAutsResponse(params));
verify(mSupplicantStaNetworkMock, never()).sendNetworkEapSimUmtsAutsResponse(eq(params));
- executeAndValidateConnectSequence(4, false, false);
+ executeAndValidateConnectSequence(4, false);
assertTrue(mDut.sendCurrentNetworkEapSimUmtsAutsResponse(params));
verify(mSupplicantStaNetworkMock).sendNetworkEapSimUmtsAutsResponse(eq(params));
}
@@ -870,7 +861,7 @@
public void testStateChangeToAssociatedCallback() throws Exception {
executeAndValidateInitializationSequence();
int frameworkNetworkId = 6;
- executeAndValidateConnectSequence(frameworkNetworkId, false, false);
+ executeAndValidateConnectSequence(frameworkNetworkId, false);
assertNotNull(mISupplicantStaIfaceCallback);
mISupplicantStaIfaceCallback.onStateChanged(
@@ -890,7 +881,7 @@
public void testStateChangeToCompletedCallback() throws Exception {
executeAndValidateInitializationSequence();
int frameworkNetworkId = 6;
- executeAndValidateConnectSequence(frameworkNetworkId, false, false);
+ executeAndValidateConnectSequence(frameworkNetworkId, false);
assertNotNull(mISupplicantStaIfaceCallback);
mISupplicantStaIfaceCallback.onStateChanged(
@@ -1427,11 +1418,7 @@
* Helper function to validate the connect sequence.
*/
private void validateConnectSequence(
- final boolean haveExistingNetwork, boolean shouldDisconnect, int numNetworkAdditions)
- throws Exception {
- if (shouldDisconnect) {
- verify(mISupplicantStaIfaceMock).disconnect();
- }
+ final boolean haveExistingNetwork, int numNetworkAdditions) throws Exception {
if (haveExistingNetwork) {
verify(mISupplicantStaIfaceMock).removeNetwork(anyInt());
}
@@ -1447,16 +1434,14 @@
*
* @param newFrameworkNetworkId Framework Network Id of the new network to connect.
* @param haveExistingNetwork Removes the existing network.
- * @param shouldDisconnect Should trigger disconnect before connecting.
*/
private void executeAndValidateConnectSequence(
- final int newFrameworkNetworkId, final boolean haveExistingNetwork,
- boolean shouldDisconnect) throws Exception {
+ final int newFrameworkNetworkId, final boolean haveExistingNetwork) throws Exception {
setupMocksForConnectSequence(haveExistingNetwork);
WifiConfiguration config = new WifiConfiguration();
config.networkId = newFrameworkNetworkId;
- assertTrue(mDut.connectToNetwork(config, shouldDisconnect));
- validateConnectSequence(haveExistingNetwork, shouldDisconnect, 1);
+ assertTrue(mDut.connectToNetwork(config));
+ validateConnectSequence(haveExistingNetwork, 1);
}
/**
@@ -1485,7 +1470,7 @@
} else {
roamNetworkId = connectedNetworkId + 1;
}
- executeAndValidateConnectSequence(connectedNetworkId, false, true);
+ executeAndValidateConnectSequence(connectedNetworkId, false);
setupMocksForRoamSequence(roamBssid);
WifiConfiguration roamingConfig = new WifiConfiguration();
@@ -1494,7 +1479,7 @@
assertTrue(mDut.roamToNetwork(roamingConfig));
if (!sameNetwork) {
- validateConnectSequence(false, false, 2);
+ validateConnectSequence(false, 2);
verify(mSupplicantStaNetworkMock, never()).setBssid(anyString());
verify(mISupplicantStaIfaceMock, never()).reassociate();
} else {
