From 36ed29992b28fe9e313ec2818dae1b8c7d626f32 Mon Sep 17 00:00:00 2001 From: Ahmed ElArabawy Date: Wed, 27 May 2020 08:11:48 -0700 Subject: Handle failure in softAp start gracefully In the current code, if start of a softAp fails for any reason, it can no longer start again until device boots. This is because the failure handling misses resetting the tethering state. This commit handles this by resetting the state on failure. Bug: 155695448 Test: atest FrameworksWifiTests Test: On a device that does not support 6GHz band $ adb shell cmd wifi start-softap kai_softap open -b 6 <==Fail $ adb shell cmd wifi start-softap kai_softap open -b 5 <==Succeed Change-Id: I6b573d6d7f88ae48a63f20a20f9d4af54a87eba7 --- .../com/android/server/wifi/WifiServiceImpl.java | 43 +++++++++++++++------- .../android/server/wifi/WifiServiceImplTest.java | 34 +++++++++++++++++ 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java index 0772b4c80..9d12aa000 100644 --- a/service/java/com/android/server/wifi/WifiServiceImpl.java +++ b/service/java/com/android/server/wifi/WifiServiceImpl.java @@ -879,6 +879,14 @@ public class WifiServiceImpl extends BaseWifiService { mLog.info("startSoftAp uid=%").c(Binder.getCallingUid()).flush(); + SoftApConfiguration softApConfig = null; + if (wifiConfig != null) { + softApConfig = ApConfigUtil.fromWifiConfiguration(wifiConfig); + if (softApConfig == null) { + return false; + } + } + if (!mTetheredSoftApTracker.setEnablingIfAllowed()) { mLog.err("Tethering is already active.").flush(); return false; @@ -890,17 +898,14 @@ public class WifiServiceImpl extends BaseWifiService { mLohsSoftApTracker.stopAll(); } - if (wifiConfig != null) { - SoftApConfiguration softApConfig = ApConfigUtil.fromWifiConfiguration(wifiConfig); - if (softApConfig == null) return false; - return startSoftApInternal(new SoftApModeConfiguration( - WifiManager.IFACE_IP_MODE_TETHERED, - softApConfig, mTetheredSoftApTracker.getSoftApCapability())); - } else { - return startSoftApInternal(new SoftApModeConfiguration( - WifiManager.IFACE_IP_MODE_TETHERED, null, - mTetheredSoftApTracker.getSoftApCapability())); + if (!startSoftApInternal(new SoftApModeConfiguration( + WifiManager.IFACE_IP_MODE_TETHERED, softApConfig, + mTetheredSoftApTracker.getSoftApCapability()))) { + mTetheredSoftApTracker.setFailedWhileEnabling(); + return false; } + + return true; } private boolean validateSoftApBand(int apBand) { @@ -951,11 +956,15 @@ public class WifiServiceImpl extends BaseWifiService { mLohsSoftApTracker.stopAll(); } - return startSoftApInternal(new SoftApModeConfiguration( + if (!startSoftApInternal(new SoftApModeConfiguration( WifiManager.IFACE_IP_MODE_TETHERED, softApConfig, - mTetheredSoftApTracker.getSoftApCapability())); - } + mTetheredSoftApTracker.getSoftApCapability()))) { + mTetheredSoftApTracker.setFailedWhileEnabling(); + return false; + } + return true; + } /** * Internal method to start softap mode. Callers of this method should have already checked @@ -1053,6 +1062,14 @@ public class WifiServiceImpl extends BaseWifiService { } } + public void setFailedWhileEnabling() { + synchronized (mLock) { + if (mTetheredSoftApState == WIFI_AP_STATE_ENABLING) { + mTetheredSoftApState = WIFI_AP_STATE_FAILED; + } + } + } + public List getConnectedClients() { synchronized (mLock) { return mTetheredSoftApConnectedClients; diff --git a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java index d91dffbfb..603a4d8f5 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java @@ -1323,6 +1323,24 @@ public class WifiServiceImplTest extends WifiBaseTest { eq(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK), any()); } + /** + * Verify that startSoftAp() with valid config succeeds after a failed call + */ + @Test + public void testStartSoftApWithValidConfigSucceedsAfterFailure() { + // First initiate a failed call + assertFalse(mWifiServiceImpl.startSoftAp(mApConfig)); + verify(mActiveModeWarden, never()).startSoftAp(any()); + + // Next attempt a valid config + WifiConfiguration config = createValidWifiApConfiguration(); + assertTrue(mWifiServiceImpl.startSoftAp(config)); + verify(mActiveModeWarden).startSoftAp(mSoftApModeConfigCaptor.capture()); + WifiConfigurationTestUtil.assertConfigurationEqualForSoftAp( + config, + mSoftApModeConfigCaptor.getValue().getSoftApConfiguration().toWifiConfiguration()); + } + /** * Verify caller with proper permission can call startTetheredHotspot. */ @@ -1496,6 +1514,22 @@ public class WifiServiceImplTest extends WifiBaseTest { eq(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK), any()); } + /** + * Verify a valied call to startTetheredHotspot succeeds after a failed call. + */ + @Test + public void testStartTetheredHotspotWithValidConfigSucceedsAfterFailedCall() { + // First issue an invalid call + assertFalse(mWifiServiceImpl.startTetheredHotspot( + new SoftApConfiguration.Builder().build())); + verify(mActiveModeWarden, never()).startSoftAp(any()); + + // Now attempt a successful call + assertTrue(mWifiServiceImpl.startTetheredHotspot(null)); + verify(mActiveModeWarden).startSoftAp(mSoftApModeConfigCaptor.capture()); + assertNull(mSoftApModeConfigCaptor.getValue().getSoftApConfiguration()); + } + /** * Verify caller with proper permission can call stopSoftAp. */ -- cgit v1.2.3