From 54bdcfa08180a69b852c8f71b39d30f0061ce6ff Mon Sep 17 00:00:00 2001 From: Sundeep Ghuman Date: Wed, 8 Mar 2017 19:52:05 -0800 Subject: [PATCH] Consolidate getRssi logic to return mRssi. Changed previous getRssi method to updateeRssi in order to clarify process, and also changed logic to return current mRssi when the given AP is active. mRssi is set directly from WifiInfo for active networks. For non-active networks we return the highest Rssi for any recently seen BSSID (multiple APs with the same SSID are grouped together). Change getSeen to updateSeen for consistency, although seen is never used anywhere. Also fixes b/34889252. Bug: b/36077865 Test: runtest --path frameworks/base/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/AccessPointTest.java Change-Id: Ic52df41b8eb317e83d835c745b404007a53cf5d7 --- .../android/settingslib/wifi/AccessPoint.java | 57 +++++++++++++++---- .../android/settingslib/wifi/WifiTracker.java | 2 +- .../settingslib/wifi/AccessPointTest.java | 46 ++++++++++----- 3 files changed, 79 insertions(+), 26 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java b/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java index 0ba3f7c229419..0ce1e1e93856b 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java @@ -47,6 +47,7 @@ import android.text.TextUtils; import android.text.style.TtsSpan; import android.util.Log; +import com.android.internal.annotations.VisibleForTesting; import com.android.settingslib.R; import java.util.ArrayList; @@ -114,7 +115,7 @@ public class AccessPoint implements Comparable { public static final int SIGNAL_LEVELS = 4; - static final int UNREACHABLE_RSSI = Integer.MAX_VALUE; + static final int UNREACHABLE_RSSI = Integer.MIN_VALUE; private final Context mContext; @@ -170,8 +171,8 @@ public class AccessPoint implements Comparable { } } update(mConfig, mInfo, mNetworkInfo); - mRssi = getRssi(); - mSeen = getSeen(); + updateRssi(); + updateSeen(); mId = sLastId.incrementAndGet(); } @@ -377,19 +378,46 @@ public class AccessPoint implements Comparable { } public int getRssi() { + return mRssi; + } + + /** + * Updates {@link #mRssi}. + * + *

If the given connection is active, the existing value of {@link #mRssi} will be returned. + * If the given AccessPoint is not active, a value will be calculated from previous scan + * results, returning the best RSSI for all matching AccessPoints. If the access point is not + * connected and there are no scan results, the rssi will be set to {@link #UNREACHABLE_RSSI}. + * + *

Old scan results will be evicted from the cache when this method is invoked. + */ + private void updateRssi() { evictOldScanResults(); - int rssi = Integer.MIN_VALUE; + + if (this.isActive()) { + return; + } + + int rssi = UNREACHABLE_RSSI; for (ScanResult result : mScanResultCache.values()) { if (result.level > rssi) { rssi = result.level; } } - return rssi; + mRssi = rssi; } - public long getSeen() { + /** + * Updates {@link #mSeen} based on the scan result cache. + * + *

Old scan results will be evicted from the cache when this method is invoked. + */ + private void updateSeen() { evictOldScanResults(); + + // TODO(sghuman): Set to now if connected + long seen = 0; for (ScanResult result : mScanResultCache.values()) { if (result.timestamp > seen) { @@ -397,7 +425,7 @@ public class AccessPoint implements Comparable { } } - return seen; + mSeen = seen; } public NetworkInfo getNetworkInfo() { @@ -822,13 +850,12 @@ public class AccessPoint implements Comparable { boolean update(ScanResult result) { if (matches(result)) { + int oldLevel = getLevel(); + /* Add or update the scan result for the BSSID */ mScanResultCache.put(result.BSSID, result); - - int oldLevel = getLevel(); - int oldRssi = getRssi(); - mSeen = getSeen(); - mRssi = (getRssi() + oldRssi)/2; + updateSeen(); + updateRssi(); int newLevel = getLevel(); if (newLevel > 0 && newLevel != oldLevel && mAccessPointListener != null) { @@ -877,10 +904,16 @@ public class AccessPoint implements Comparable { } } + @VisibleForTesting void setRssi(int rssi) { mRssi = rssi; } + /** Sets the rssi to {@link #UNREACHABLE_RSSI}. */ + void setUnreachable() { + setRssi(AccessPoint.UNREACHABLE_RSSI); + } + int getRankingScore() { return mRankingScore; } diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java index 0771dd88028ef..55c886e6fb1cd 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java @@ -481,7 +481,7 @@ public class WifiTracker { } } if (!apFound) { - accessPoint.setRssi(AccessPoint.UNREACHABLE_RSSI); + accessPoint.setUnreachable(); } accessPoints.add(accessPoint); apMap.put(accessPoint.getSsidStr(), accessPoint); 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 ec0190cfa538c..e8a58c13baf0d 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 @@ -89,21 +89,10 @@ public class AccessPointTest { @Test public void testThatCopyAccessPoint_scanCacheShouldMatch() { - Bundle bundle = new Bundle(); - ArrayList scanResults = new ArrayList<>(); - for (int i = 0; i < 5; i++) { - ScanResult scanResult = new ScanResult(); - scanResult.level = i; - scanResult.BSSID = "bssid-" + i; - scanResult.timestamp = SystemClock.elapsedRealtime() * 1000; - scanResults.add(scanResult); - } - - bundle.putParcelableArrayList("key_scanresultcache", scanResults); - AccessPoint original = new AccessPoint(mContext, bundle); + AccessPoint original = createAccessPointWithScanResultCache(); assertThat(original.getRssi()).isEqualTo(4); AccessPoint copy = new AccessPoint(mContext, createWifiConfiguration()); - assertThat(copy.getRssi()).isEqualTo(Integer.MIN_VALUE); + assertThat(copy.getRssi()).isEqualTo(AccessPoint.UNREACHABLE_RSSI); copy.copyFrom(original); assertThat(original.getRssi()).isEqualTo(copy.getRssi()); } @@ -190,6 +179,37 @@ public class AccessPointTest { assertThat(points.indexOf(firstName)).isLessThan(points.indexOf(lastname)); } + @Test + public void testRssiIsSetFromScanResults() { + AccessPoint ap = createAccessPointWithScanResultCache(); + int originalRssi = ap.getRssi(); + assertThat(originalRssi).isNotEqualTo(AccessPoint.UNREACHABLE_RSSI); + } + + @Test + public void testGetRssiShouldReturnSetRssiValue() { + AccessPoint ap = createAccessPointWithScanResultCache(); + int originalRssi = ap.getRssi(); + int newRssi = originalRssi - 10; + ap.setRssi(newRssi); + assertThat(ap.getRssi()).isEqualTo(newRssi); + } + + private AccessPoint createAccessPointWithScanResultCache() { + Bundle bundle = new Bundle(); + ArrayList scanResults = new ArrayList<>(); + for (int i = 0; i < 5; i++) { + ScanResult scanResult = new ScanResult(); + scanResult.level = i; + scanResult.BSSID = "bssid-" + i; + scanResult.timestamp = SystemClock.elapsedRealtime() * 1000; + scanResults.add(scanResult); + } + + bundle.putParcelableArrayList("key_scanresultcache", scanResults); + return new AccessPoint(mContext, bundle); + } + private WifiConfiguration createWifiConfiguration() { WifiConfiguration configuration = new WifiConfiguration(); configuration.BSSID = "bssid";