From c43b30ca476ce5d1b6a959215ddf54ff611ebeae Mon Sep 17 00:00:00 2001 From: Sundeep Ghuman Date: Wed, 6 Sep 2017 11:46:21 -0700 Subject: [PATCH] Do not pause scanning during Network state changes. This prevents UI bugs in WifiSettings where scan results would become to old upon switching networks and thus all the APs would be removed from the wifi picker. Now scanning is only paused/resumed during start/stop tracking as well as on wifi state changes between enabled/ not enabled. Also adds additional logging to scan result age in verbose logging. Bug: b/64989100 Test: runtest --path frameworks/base/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java Manual: 1. Repeated switch back and forth between different networks. 2. Ensure that AccessPoints in WifiSettings do not disappear after new connections. Change-Id: I11e424bcce799a5f3d003a955dcee004294058b7 Merged-In: I11e424bcce799a5f3d003a955dcee004294058b7 (cherry picked from commit db9b94c195f056248b1f5c3b4ef5e907c1b82028) --- .../android/settingslib/wifi/AccessPoint.java | 18 ++++-- .../android/settingslib/wifi/WifiTracker.java | 34 ++++++----- .../settingslib/wifi/AccessPointTest.java | 2 +- .../settingslib/wifi/WifiTrackerTest.java | 61 ++++++++++++++++--- 4 files changed, 86 insertions(+), 29 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java b/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java index 3fd7c392d77c8..682b85e461650 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java @@ -133,7 +133,7 @@ public class AccessPoint implements Comparable { private final Map mScoredNetworkCache = new HashMap<>(); /** Maximum age of scan results to hold onto while actively scanning. **/ - private static final long MAX_SCAN_RESULT_AGE_MILLIS = 15000; + private static final long MAX_SCAN_RESULT_AGE_MILLIS = 25000; static final String KEY_NETWORKINFO = "key_networkinfo"; static final String KEY_WIFIINFO = "key_wifiinfo"; @@ -426,6 +426,11 @@ public class AccessPoint implements Comparable { } builder.append(",metered=").append(isMetered()); + if (WifiTracker.sVerboseLogging) { + builder.append(",rssi=").append(mRssi); + builder.append(",scan cache size=").append(mScanResultCache.size()); + } + return builder.append(')').toString(); } @@ -951,6 +956,7 @@ public class AccessPoint implements Comparable { evictOldScanResults(); // TODO: sort list by RSSI or age + long nowMs = SystemClock.elapsedRealtime(); for (ScanResult result : mScanResultCache.values()) { if (result.frequency >= LOWER_FREQ_5GHZ && result.frequency <= HIGHER_FREQ_5GHZ) { @@ -961,7 +967,7 @@ public class AccessPoint implements Comparable { maxRssi5 = result.level; } if (num5 <= maxDisplayedScans) { - scans5GHz.append(verboseScanResultSummary(result, bssid)); + scans5GHz.append(verboseScanResultSummary(result, bssid, nowMs)); } } else if (result.frequency >= LOWER_FREQ_24GHZ && result.frequency <= HIGHER_FREQ_24GHZ) { @@ -972,7 +978,7 @@ public class AccessPoint implements Comparable { maxRssi24 = result.level; } if (num24 <= maxDisplayedScans) { - scans24GHz.append(verboseScanResultSummary(result, bssid)); + scans24GHz.append(verboseScanResultSummary(result, bssid, nowMs)); } } } @@ -1000,7 +1006,7 @@ public class AccessPoint implements Comparable { } @VisibleForTesting - /* package */ String verboseScanResultSummary(ScanResult result, String bssid) { + /* package */ String verboseScanResultSummary(ScanResult result, String bssid, long nowMs) { StringBuilder stringBuilder = new StringBuilder(); stringBuilder.append(" \n{").append(result.BSSID); if (result.BSSID.equals(bssid)) { @@ -1013,6 +1019,8 @@ public class AccessPoint implements Comparable { stringBuilder.append(",") .append(getSpeedLabel(speed)); } + int ageSeconds = (int) (nowMs - result.timestamp / 1000) / 1000; + stringBuilder.append(",").append(ageSeconds).append("s"); stringBuilder.append("}"); return stringBuilder.toString(); } @@ -1209,6 +1217,7 @@ public class AccessPoint implements Comparable { /** Attempt to update the AccessPoint and return true if an update occurred. */ public boolean update( @Nullable WifiConfiguration config, WifiInfo info, NetworkInfo networkInfo) { + boolean updated = false; final int oldLevel = getLevel(); if (info != null && isInfoForThisAccessPoint(config, info)) { @@ -1240,6 +1249,7 @@ public class AccessPoint implements Comparable { mAccessPointListener.onLevelChanged(this); } } + return updated; } diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java index 6895550e51081..3c664b0b01a95 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java @@ -624,7 +624,7 @@ public class WifiTracker { String prevSsid = prevAccessPoint.getSsidStr(); boolean found = false; for (AccessPoint newAccessPoint : accessPoints) { - if (newAccessPoint.getSsid() != null && newAccessPoint.getSsid() + if (newAccessPoint.getSsidStr() != null && newAccessPoint.getSsidStr() .equals(prevSsid)) { found = true; break; @@ -676,24 +676,23 @@ public class WifiTracker { private void updateNetworkInfo(NetworkInfo networkInfo) { /* sticky broadcasts can call this when wifi is disabled */ if (!mWifiManager.isWifiEnabled()) { - mMainHandler.sendEmptyMessage(MainHandler.MSG_PAUSE_SCANNING); clearAccessPointsAndConditionallyUpdate(); return; } - if (networkInfo != null && - networkInfo.getDetailedState() == DetailedState.OBTAINING_IPADDR) { - mMainHandler.sendEmptyMessage(MainHandler.MSG_PAUSE_SCANNING); - } else { - mMainHandler.sendEmptyMessage(MainHandler.MSG_RESUME_SCANNING); - } - if (networkInfo != null) { mLastNetworkInfo = networkInfo; + if (DBG()) { + Log.d(TAG, "mLastNetworkInfo set: " + mLastNetworkInfo); + } } WifiConfiguration connectionConfig = null; + mLastInfo = mWifiManager.getConnectionInfo(); + if (DBG()) { + Log.d(TAG, "mLastInfo set as: " + mLastInfo); + } if (mLastInfo != null) { connectionConfig = getWifiConfigurationForNetworkId(mLastInfo.getNetworkId(), mWifiManager.getConfiguredNetworks()); @@ -717,7 +716,6 @@ public class WifiTracker { } if (reorder) Collections.sort(mInternalAccessPoints); - if (updated) mMainHandler.sendEmptyMessage(MainHandler.MSG_ACCESS_POINT_CHANGED); } } @@ -866,6 +864,9 @@ public class WifiTracker { if (mScanner != null) { mScanner.pause(); } + synchronized (mLock) { + mStaleScanResults = true; + } break; } } @@ -928,6 +929,9 @@ public class WifiTracker { if (mScanner != null) { mScanner.pause(); } + synchronized (mLock) { + mStaleScanResults = true; + } } mMainHandler.obtainMessage(MainHandler.MSG_WIFI_STATE_CHANGED, msg.arg1, 0) .sendToTarget(); @@ -982,7 +986,7 @@ public class WifiTracker { } return; } - sendEmptyMessageDelayed(0, WIFI_RESCAN_INTERVAL_MS); + sendEmptyMessageDelayed(MSG_SCAN, WIFI_RESCAN_INTERVAL_MS); } } @@ -1075,10 +1079,12 @@ public class WifiTracker { oldAccessPoints.put(accessPoint.mId, accessPoint); } - if (DBG()) { - Log.d(TAG, "Starting to copy AP items on the MainHandler"); - } synchronized (mLock) { + if (DBG()) { + Log.d(TAG, "Starting to copy AP items on the MainHandler. Internal APs: " + + mInternalAccessPoints); + } + if (notifyListeners) { notificationMap = mAccessPointListenerAdapter.mPendingNotifications.clone(); } diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/AccessPointTest.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/AccessPointTest.java index b99584f17e88d..230b20081961b 100644 --- a/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/AccessPointTest.java +++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/AccessPointTest.java @@ -454,7 +454,7 @@ public class AccessPointTest { ap.update(mockWifiNetworkScoreCache, true /* scoringUiEnabled */, MAX_SCORE_CACHE_AGE_MILLIS); - String summary = ap.verboseScanResultSummary(scanResults.get(0), null); + String summary = ap.verboseScanResultSummary(scanResults.get(0), null, 0); assertThat(summary.contains(mContext.getString(R.string.speed_label_very_fast))).isTrue(); } diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java index df6587e5042f8..7cce648f0dfad 100644 --- a/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java +++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java @@ -105,15 +105,30 @@ public class WifiTrackerTest { private static final byte SCORE_2 = 15; private static final int BADGE_2 = AccessPoint.Speed.FAST; - private static final int CONNECTED_NETWORK_ID = 123; + // TODO(b/65594609): Convert mutable Data objects to instance variables / builder pattern + private static final int NETWORK_ID_1 = 123; private static final int CONNECTED_RSSI = -50; private static final WifiInfo CONNECTED_AP_1_INFO = new WifiInfo(); static { CONNECTED_AP_1_INFO.setSSID(WifiSsid.createFromAsciiEncoded(SSID_1)); CONNECTED_AP_1_INFO.setBSSID(BSSID_1); - CONNECTED_AP_1_INFO.setNetworkId(CONNECTED_NETWORK_ID); + CONNECTED_AP_1_INFO.setNetworkId(NETWORK_ID_1); CONNECTED_AP_1_INFO.setRssi(CONNECTED_RSSI); } + private static final WifiConfiguration CONFIGURATION_1 = new WifiConfiguration(); + static { + CONFIGURATION_1.SSID = SSID_1; + CONFIGURATION_1.BSSID = BSSID_1; + CONFIGURATION_1.networkId = NETWORK_ID_1; + } + + private static final int NETWORK_ID_2 = 2; + private static final WifiConfiguration CONFIGURATION_2 = new WifiConfiguration(); + static { + CONFIGURATION_2.SSID = SSID_2; + CONFIGURATION_2.BSSID = BSSID_2; + CONFIGURATION_2.networkId = NETWORK_ID_2; + } @Captor ArgumentCaptor mScoreCacheCaptor; @Mock private ConnectivityManager mockConnectivityManager; @@ -159,6 +174,8 @@ public class WifiTrackerTest { when(mockWifiManager.isWifiEnabled()).thenReturn(true); when(mockWifiManager.getScanResults()) .thenReturn(Arrays.asList(buildScanResult1(), buildScanResult2())); + when(mockWifiManager.getConfiguredNetworks()) + .thenReturn(Arrays.asList(CONFIGURATION_1, CONFIGURATION_2)); when(mockCurve1.lookupScore(RSSI_1)).thenReturn(SCORE_1); @@ -333,8 +350,7 @@ public class WifiTrackerTest { WifiConfiguration configuration = new WifiConfiguration(); configuration.SSID = SSID_1; configuration.BSSID = BSSID_1; - configuration.networkId = CONNECTED_NETWORK_ID; - when(mockWifiManager.getConfiguredNetworks()).thenReturn(Arrays.asList(configuration)); + configuration.networkId = NETWORK_ID_1; NetworkInfo networkInfo = new NetworkInfo( ConnectivityManager.TYPE_WIFI, 0, "Type Wifi", "subtype"); @@ -365,6 +381,24 @@ public class WifiTrackerTest { mainLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS)); } + private void switchToNetwork2(WifiTracker tracker) throws InterruptedException { + NetworkInfo networkInfo = new NetworkInfo( + ConnectivityManager.TYPE_WIFI, 0, "Type Wifi", "subtype"); + networkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTING, "connecting", "test"); + + WifiInfo info = new WifiInfo(); + info.setSSID(WifiSsid.createFromAsciiEncoded(SSID_2)); + info.setBSSID(BSSID_2); + info.setRssi(CONNECTED_RSSI); + info.setNetworkId(NETWORK_ID_2); + when(mockWifiManager.getConnectionInfo()).thenReturn(info); + + Intent intent = new Intent(WifiManager.NETWORK_STATE_CHANGED_ACTION); + intent.putExtra(WifiManager.EXTRA_NETWORK_INFO, networkInfo); + tracker.mReceiver.onReceive(mContext, intent); + waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker); + } + @Test public void testAccessPointListenerSetWhenLookingUpUsingScanResults() { ScanResult scanResult = new ScanResult(); @@ -722,12 +756,6 @@ public class WifiTrackerTest { when(mockWifiManager.getConnectionInfo()).thenReturn(CONNECTED_AP_1_INFO); - WifiConfiguration configuration = new WifiConfiguration(); - configuration.SSID = SSID_1; - configuration.BSSID = BSSID_1; - configuration.networkId = CONNECTED_NETWORK_ID; - when(mockWifiManager.getConfiguredNetworks()).thenReturn(Arrays.asList(configuration)); - NetworkInfo networkInfo = new NetworkInfo( ConnectivityManager.TYPE_WIFI, 0, "Type Wifi", "subtype"); networkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTED, "connected", "test"); @@ -881,4 +909,17 @@ public class WifiTrackerTest { assertThat(tracker.isConnected()).isFalse(); verify(mockWifiListener, times(2)).onConnectedChanged(); } + + @Test + public void updateNetworkInfoWithNewConnectedNetwork_switchesNetworks() throws Exception { + WifiTracker tracker = createTrackerWithScanResultsAndAccessPoint1Connected(); + + switchToNetwork2(tracker); + + List aps = tracker.getAccessPoints(); + assertThat(aps.get(0).getSsidStr()).isEqualTo(SSID_2); + + assertThat(aps.get(0).isReachable()).isTrue(); + assertThat(aps.get(1).isReachable()).isTrue(); + } }