From 0b8700f842a0d92e73ce096e0513c5acc7876ee5 Mon Sep 17 00:00:00 2001 From: Eric Schwarzenbach Date: Tue, 25 Jul 2017 14:32:21 -0700 Subject: [PATCH] Add individual ScanResult speeds to verbose logging. Moves the individual ScanResult summary string generation out of getVisibilityStatus into its own method `verboseScanResultSummary`, to reduce duplication and to enable testing the per-ScanResult speed labeling. Bug: 63866500 Test: runtest --path frameworks/base/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/AccessPointTest.java Change-Id: I9583d142a7d50314017154703df4e1ce69fc68aa --- .../android/settingslib/wifi/AccessPoint.java | 145 +++++++++--------- .../settingslib/wifi/AccessPointTest.java | 21 ++- 2 files changed, 96 insertions(+), 70 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java b/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java index a814a9f938a62..d04ccf2665c13 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java @@ -59,7 +59,9 @@ import com.android.settingslib.R; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; +import java.util.HashMap; import java.util.Iterator; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; @@ -120,6 +122,10 @@ public class AccessPoint implements Comparable { */ private final ConcurrentHashMap mScanResultCache = new ConcurrentHashMap(32); + + /** Map of BSSIDs to speed values for individual ScanResults. */ + private final Map mScanResultScores = new HashMap<>(); + /** Maximum age of scan results to hold onto while actively scanning. **/ private static final long MAX_SCAN_RESULT_AGE_MS = 15000; @@ -280,6 +286,8 @@ public class AccessPoint implements Comparable { this.mNetworkInfo = that.mNetworkInfo; this.mScanResultCache.clear(); this.mScanResultCache.putAll(that.mScanResultCache); + this.mScanResultScores.clear(); + this.mScanResultScores.putAll(that.mScanResultScores); this.mId = that.mId; this.mSpeed = that.mSpeed; this.mIsScoredNetworkMetered = that.mIsScoredNetworkMetered; @@ -392,6 +400,7 @@ public class AccessPoint implements Comparable { */ boolean update(WifiNetworkScoreCache scoreCache, boolean scoringUiEnabled) { boolean scoreChanged = false; + mScanResultScores.clear(); if (scoringUiEnabled) { scoreChanged = updateScores(scoreCache); } @@ -407,6 +416,18 @@ public class AccessPoint implements Comparable { int oldSpeed = mSpeed; mSpeed = Speed.NONE; + for (ScanResult result : mScanResultCache.values()) { + ScoredNetwork score = scoreCache.getScoredNetwork(result); + if (score == null) { + continue; + } + + int speed = score.calculateBadge(result.level); + mScanResultScores.put(result.BSSID, speed); + mSpeed = Math.max(mSpeed, speed); + } + + // set mSpeed to the connected ScanResult if the AccessPoint is the active network if (isActive() && mInfo != null) { NetworkKey key = new NetworkKey(new WifiKey( AccessPoint.convertToQuotedString(ssid), mInfo.getBSSID())); @@ -414,15 +435,6 @@ public class AccessPoint implements Comparable { if (score != null) { mSpeed = score.calculateBadge(mInfo.getRssi()); } - } else { - for (ScanResult result : mScanResultCache.values()) { - ScoredNetwork score = scoreCache.getScoredNetwork(result); - if (score == null) { - continue; - } - // TODO(sghuman): Rename calculateBadge API - mSpeed = Math.max(mSpeed, score.calculateBadge(result.level)); - } } if(WifiTracker.sVerboseLogging) { @@ -813,8 +825,8 @@ public class AccessPoint implements Comparable { */ private String getVisibilityStatus() { StringBuilder visibility = new StringBuilder(); - StringBuilder scans24GHz = null; - StringBuilder scans5GHz = null; + StringBuilder scans24GHz = new StringBuilder(); + StringBuilder scans5GHz = new StringBuilder(); String bssid = null; long now = System.currentTimeMillis(); @@ -836,87 +848,55 @@ public class AccessPoint implements Comparable { visibility.append(String.format("rx=%.1f", mInfo.rxSuccessRate)); } - int rssi5 = WifiConfiguration.INVALID_RSSI; - int rssi24 = WifiConfiguration.INVALID_RSSI; - int num5 = 0; - int num24 = 0; + int maxRssi5 = WifiConfiguration.INVALID_RSSI; + int maxRssi24 = WifiConfiguration.INVALID_RSSI; + final int maxDisplayedScans = 4; + int num5 = 0; // number of scanned BSSID on 5GHz band + int num24 = 0; // number of scanned BSSID on 2.4Ghz band int numBlackListed = 0; - int n24 = 0; // Number scan results we included in the string - int n5 = 0; // Number scan results we included in the string evictOldScanResults(); + // TODO: sort list by RSSI or age for (ScanResult result : mScanResultCache.values()) { - if (result.frequency >= LOWER_FREQ_5GHZ && result.frequency <= HIGHER_FREQ_5GHZ) { // Strictly speaking: [4915, 5825] - // number of known BSSID on 5GHz band - num5 = num5 + 1; + num5++; + + if (result.level > maxRssi5) { + maxRssi5 = result.level; + } + if (num5 <= maxDisplayedScans) { + scans5GHz.append(verboseScanResultSummary(result, bssid)); + } } else if (result.frequency >= LOWER_FREQ_24GHZ && result.frequency <= HIGHER_FREQ_24GHZ) { // Strictly speaking: [2412, 2482] - // number of known BSSID on 2.4Ghz band - num24 = num24 + 1; - } + num24++; - - if (result.frequency >= LOWER_FREQ_5GHZ - && result.frequency <= HIGHER_FREQ_5GHZ) { - if (result.level > rssi5) { - rssi5 = result.level; + if (result.level > maxRssi24) { + maxRssi24 = result.level; } - if (n5 < 4) { - if (scans5GHz == null) scans5GHz = new StringBuilder(); - scans5GHz.append(" \n{").append(result.BSSID); - if (bssid != null && result.BSSID.equals(bssid)) scans5GHz.append("*"); - scans5GHz.append("=").append(result.frequency); - scans5GHz.append(",").append(result.level); - scans5GHz.append("}"); - n5++; - } - } else if (result.frequency >= LOWER_FREQ_24GHZ - && result.frequency <= HIGHER_FREQ_24GHZ) { - if (result.level > rssi24) { - rssi24 = result.level; - } - if (n24 < 4) { - if (scans24GHz == null) scans24GHz = new StringBuilder(); - scans24GHz.append(" \n{").append(result.BSSID); - if (bssid != null && result.BSSID.equals(bssid)) scans24GHz.append("*"); - scans24GHz.append("=").append(result.frequency); - scans24GHz.append(",").append(result.level); - scans24GHz.append("}"); - n24++; + if (num24 <= maxDisplayedScans) { + scans24GHz.append(verboseScanResultSummary(result, bssid)); } } } visibility.append(" ["); if (num24 > 0) { visibility.append("(").append(num24).append(")"); - if (n24 <= 4) { - if (scans24GHz != null) { - visibility.append(scans24GHz.toString()); - } - } else { - visibility.append("max=").append(rssi24); - if (scans24GHz != null) { - visibility.append(",").append(scans24GHz.toString()); - } + if (num24 > maxDisplayedScans) { + visibility.append("max=").append(maxRssi24).append(","); } + visibility.append(scans24GHz.toString()); } visibility.append(";"); if (num5 > 0) { visibility.append("(").append(num5).append(")"); - if (n5 <= 4) { - if (scans5GHz != null) { - visibility.append(scans5GHz.toString()); - } - } else { - visibility.append("max=").append(rssi5); - if (scans5GHz != null) { - visibility.append(",").append(scans5GHz.toString()); - } + if (num5 > maxDisplayedScans) { + visibility.append("max=").append(maxRssi5).append(","); } + visibility.append(scans5GHz.toString()); } if (numBlackListed > 0) visibility.append("!").append(numBlackListed); @@ -925,6 +905,28 @@ public class AccessPoint implements Comparable { return visibility.toString(); } + @VisibleForTesting + /* package */ String verboseScanResultSummary(ScanResult result, String bssid) { + StringBuilder stringBuilder = new StringBuilder(); + stringBuilder.append(" \n{").append(result.BSSID); + if (result.BSSID.equals(bssid)) { + stringBuilder.append("*"); + } + stringBuilder.append("=").append(result.frequency); + stringBuilder.append(",").append(result.level); + if (hasSpeed(result)) { + stringBuilder.append(",") + .append(getSpeedLabel(mScanResultScores.get(result.BSSID))); + } + stringBuilder.append("}"); + return stringBuilder.toString(); + } + + private boolean hasSpeed(ScanResult result) { + return mScanResultScores.containsKey(result.BSSID) + && mScanResultScores.get(result.BSSID) != Speed.NONE; + } + /** * Return whether this is the active connection. * For ephemeral connections (networkId is invalid), this returns false if the network is @@ -1135,7 +1137,12 @@ public class AccessPoint implements Comparable { @Nullable String getSpeedLabel() { - switch (mSpeed) { + return getSpeedLabel(mSpeed); + } + + @Nullable + private String getSpeedLabel(int speed) { + switch (speed) { case Speed.VERY_FAST: return mContext.getString(R.string.speed_label_very_fast); case Speed.FAST: 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 35c730e64ead4..9645c9407600f 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 @@ -73,6 +73,7 @@ public class AccessPointTest { public void setUp() { MockitoAnnotations.initMocks(this); mContext = InstrumentationRegistry.getTargetContext(); + WifiTracker.sVerboseLogging = false; } @Test @@ -426,7 +427,6 @@ public class AccessPointTest { ap.update(mockWifiNetworkScoreCache, true /* scoringUiEnabled */); verify(mockWifiNetworkScoreCache, times(2)).getScoredNetwork(key); - verify(mockWifiNetworkScoreCache, never()).getScoredNetwork(any(ScanResult.class)); assertThat(ap.getSpeed()).isEqualTo(AccessPoint.Speed.FAST); } @@ -443,6 +443,25 @@ public class AccessPointTest { assertThat(ap.getSummary()).isEqualTo(mContext.getString(R.string.speed_label_very_fast)); } + @Test + public void testVerboseSummaryString_showsScanResultSpeedLabel() { + WifiTracker.sVerboseLogging = true; + + Bundle bundle = new Bundle(); + ArrayList scanResults = buildScanResultCache(); + bundle.putParcelableArrayList(AccessPoint.KEY_SCANRESULTCACHE, scanResults); + AccessPoint ap = new AccessPoint(mContext, bundle); + + when(mockWifiNetworkScoreCache.getScoredNetwork(any(ScanResult.class))) + .thenReturn(buildScoredNetworkWithMockBadgeCurve()); + when(mockBadgeCurve.lookupScore(anyInt())).thenReturn((byte) AccessPoint.Speed.VERY_FAST); + + ap.update(mockWifiNetworkScoreCache, true /* scoringUiEnabled */); + String summary = ap.verboseScanResultSummary(scanResults.get(0), null); + + assertThat(summary.contains(mContext.getString(R.string.speed_label_very_fast))).isTrue(); + } + @Test public void testSummaryString_concatenatesSpeedLabel() { AccessPoint ap = createAccessPointWithScanResultCache();