Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f4f3842b authored by Eric Schwarzenbach's avatar Eric Schwarzenbach
Browse files

Sort APs by Speed label value instead of ranking score. 

This prevents unintuitive sorting if the ranking score (somehow)
sorts differently from the speed label.

This change also fixes a broken test in AccessPointTest.java
'testSummaryString_showsSpeedLabel()'.

Bug: 63116984
Test: runtest --path
frameworks/base/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/AccessPointTest.java

Change-Id: I4a9e55950541ddbf32e01508484bd40326ef228e
(cherry picked from commit 0b431c9a)
parent 0d7ff532

packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java

+10 −29
Original line number Diff line number Diff line
@@ -177,7 +177,6 @@ public class AccessPoint implements Comparable<AccessPoint> { 


    private Object mTag;



    private int mRankingScore = Integer.MIN_VALUE;

    private int mSpeed = Speed.NONE;

    private boolean mIsScoredNetworkMetered = false;
@@ -283,7 +282,6 @@ public class AccessPoint implements Comparable<AccessPoint> { 
        this.mId = that.mId;

        this.mSpeed = that.mSpeed;

        this.mIsScoredNetworkMetered = that.mIsScoredNetworkMetered;

        this.mRankingScore = that.mRankingScore;

    }



    /**
@@ -294,7 +292,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
    *   1. Active before inactive

    *   2. Reachable before unreachable

    *   3. Saved before unsaved

    *   4. (Internal only) Network ranking score

    *   4. Network speed value

    *   5. Stronger signal before weaker signal

    *   6. SSID alphabetically

    *
@@ -315,9 +313,9 @@ public class AccessPoint implements Comparable<AccessPoint> { 
        if (isSaved() && !other.isSaved()) return -1;

        if (!isSaved() && other.isSaved()) return 1;



        // Higher scores go before lower scores

        if (getRankingScore() != other.getRankingScore()) {

            return (getRankingScore() > other.getRankingScore()) ? -1 : 1;

        // Faster speeds go before slower speeds

        if (getSpeed() != other.getSpeed()) {

            return other.getSpeed() - getSpeed();

        }



        // Sort by signal strength, bucketed by level
@@ -376,9 +374,6 @@ public class AccessPoint implements Comparable<AccessPoint> { 
            builder.append(',').append(securityToString(security, pskType));

        }

        builder.append(",level=").append(getLevel());

        if (mRankingScore != Integer.MIN_VALUE) {

            builder.append(",rankingScore=").append(mRankingScore);

        }

        if (mSpeed != Speed.NONE) {

            builder.append(",speed=").append(mSpeed);

        }
@@ -409,9 +404,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
     */

    private boolean updateScores(WifiNetworkScoreCache scoreCache) {

        int oldSpeed = mSpeed;

        int oldRankingScore = mRankingScore;

        mSpeed = Speed.NONE;

        mRankingScore = Integer.MIN_VALUE;



        if (isActive() && mInfo != null) {

            NetworkKey key = new NetworkKey(new WifiKey(
@@ -419,9 +412,6 @@ public class AccessPoint implements Comparable<AccessPoint> { 
            ScoredNetwork score = scoreCache.getScoredNetwork(key);

            if (score != null) {

                mSpeed = score.calculateBadge(mInfo.getRssi());

                if (score.hasRankingScore()) {

                    mRankingScore = score.calculateRankingScore(mInfo.getRssi());

                }

            }

        } else {

            for (ScanResult result : mScanResultCache.values()) {
@@ -429,11 +419,6 @@ public class AccessPoint implements Comparable<AccessPoint> { 
                if (score == null) {

                    continue;

                }



                if (score.hasRankingScore()) {

                    mRankingScore =

                            Math.max(mRankingScore, score.calculateRankingScore(result.level));

                }

                // TODO(sghuman): Rename calculateBadge API

                mSpeed = Math.max(mSpeed, score.calculateBadge(result.level));

            }
@@ -443,7 +428,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
            Log.i(TAG, String.format("%s: Set speed to %d", ssid, mSpeed));

        }



        return (oldSpeed != mSpeed || oldRankingScore != mRankingScore);

        return oldSpeed != mSpeed;

    }



    /**
@@ -799,12 +784,15 @@ public class AccessPoint implements Comparable<AccessPoint> { 
            }

        }



        // If Speed label is present, use the preference combination to prepend it to the summary.

        if (mSpeed != Speed.NONE) {

        // If Speed label and summary are both present, use the preference combination to combine

        // the two, else return the non-null one.

        if (getSpeedLabel() != null && summary.length() != 0) {

            return mContext.getResources().getString(

                    R.string.preference_summary_default_combination,

                    getSpeedLabel(),

                    summary.toString());

        } else if (getSpeedLabel() != null) {

            return getSpeedLabel();

        } else {

            return summary.toString();

        }
@@ -834,9 +822,6 @@ public class AccessPoint implements Comparable<AccessPoint> { 
            visibility.append(" rssi=").append(mInfo.getRssi());

            visibility.append(" ");

            visibility.append(" score=").append(mInfo.score);

            if (mRankingScore != Integer.MIN_VALUE) {

                visibility.append(" rankingScore=").append(getRankingScore());

            }

            if (mSpeed != Speed.NONE) {

                visibility.append(" speed=").append(getSpeedLabel());

            }
@@ -1141,10 +1126,6 @@ public class AccessPoint implements Comparable<AccessPoint> { 
        setRssi(AccessPoint.UNREACHABLE_RSSI);

    }



    int getRankingScore() {

        return mRankingScore;

    }



    int getSpeed() { return mSpeed;}



    @Nullable

packages/SettingsLib/src/com/android/settingslib/wifi/TestAccessPointBuilder.java

+8 −0
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import android.net.wifi.ScanResult; 
import android.net.wifi.WifiConfiguration;

import android.net.wifi.WifiInfo;

import android.os.Bundle;

import com.android.settingslib.wifi.AccessPoint.Speed;



import java.util.ArrayList;
@@ -40,6 +41,7 @@ public class TestAccessPointBuilder { 


    // set some sensible defaults

    private String mBssid = null;

    private int mSpeed = Speed.NONE;

    private int mRssi = AccessPoint.UNREACHABLE_RSSI;

    private int mNetworkId = WifiConfiguration.INVALID_NETWORK_ID;

    private String ssid = "TestSsid";
@@ -78,6 +80,7 @@ public class TestAccessPointBuilder { 
            bundle.putParcelableArrayList(AccessPoint.KEY_SCANRESULTCACHE, mScanResultCache);

        }

        bundle.putInt(AccessPoint.KEY_SECURITY, mSecurity);

        bundle.putInt(AccessPoint.KEY_SPEED, mSpeed);



        AccessPoint ap = new AccessPoint(mContext, bundle);

        ap.setRssi(mRssi);
@@ -127,6 +130,11 @@ public class TestAccessPointBuilder { 
        return this;

    }



    public TestAccessPointBuilder setSpeed(int speed) {

        mSpeed = speed;

        return this;

    }



    /**

    * Set whether the AccessPoint is reachable.

    * Side effect: if the signal level was not previously set,

packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/AccessPointTest.java

+9 −2
Original line number Diff line number Diff line
@@ -50,6 +50,7 @@ import android.text.style.TtsSpan; 


import com.android.settingslib.R;



import com.android.settingslib.wifi.AccessPoint.Speed;

import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;
@@ -145,7 +146,13 @@ public class AccessPointTest { 
        assertSortingWorks(savedAp, notSavedAp);

    }



    //TODO: add tests for mRankingScore sort order if ranking is exposed

    @Test

    public void testCompareTo_GivesHighSpeedBeforeLowSpeed() {

        AccessPoint fastAp = new TestAccessPointBuilder(mContext).setSpeed(Speed.FAST).build();

        AccessPoint slowAp = new TestAccessPointBuilder(mContext).setSpeed(Speed.SLOW).build();



        assertSortingWorks(fastAp, slowAp);

    }



    @Test

    public void testCompareTo_GivesHighLevelBeforeLowLevel() {
@@ -505,7 +512,7 @@ public class AccessPointTest { 
    }



    /**

    * Assert that the first AccessPoint appears after the second AccessPoint

    * Assert that the first AccessPoint appears before the second AccessPoint

    * once sorting has been completed.

    */

    private void assertSortingWorks(AccessPoint first, AccessPoint second) {