Merge "Fix empty network scan result" into main

2024-05-10 12:03:26 +00:00
parent 544f85c262 6e4ac4bdc0
commit 59e2bce852
4 changed files with 79 additions and 105 deletions
--- a/src/com/android/settings/network/telephony/NetworkSelectSettings.java
+++ b/src/com/android/settings/network/telephony/NetworkSelectSettings.java
@@ -48,13 +48,12 @@ import com.android.internal.telephony.flags.Flags;
 import com.android.settings.R;
 import com.android.settings.dashboard.DashboardFragment;
 import com.android.settings.network.telephony.scan.NetworkScanRepository;
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanCellInfos;
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanComplete;
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanError;
 import com.android.settings.overlay.FeatureFactory;
 import com.android.settingslib.core.instrumentation.MetricsFeatureProvider;
 import com.android.settingslib.utils.ThreadUtils;

+import com.google.common.collect.ImmutableList;
+
 import kotlin.Unit;

 import java.util.ArrayList;
@@ -83,7 +82,8 @@ public class NetworkSelectSettings extends DashboardFragment {
    private View mProgressHeader;
    private Preference mStatusMessagePreference;
    @VisibleForTesting
-    List<CellInfo> mCellInfoList;
+    @NonNull
+    List<CellInfo> mCellInfoList = ImmutableList.of();
    private int mSubId = SubscriptionManager.INVALID_SUBSCRIPTION_ID;
    private TelephonyManager mTelephonyManager;
    private SatelliteManager mSatelliteManager;
@@ -96,7 +96,6 @@ public class NetworkSelectSettings extends DashboardFragment {
    private AtomicBoolean mShouldFilterOutSatellitePlmn = new AtomicBoolean();

    private NetworkScanRepository mNetworkScanRepository;
-    private boolean mUpdateScanResult = false;

    private NetworkSelectRepository mNetworkSelectRepository;

@@ -213,38 +212,16 @@ public class NetworkSelectSettings extends DashboardFragment {
    }

    private void launchNetworkScan() {
+        setProgressBarVisible(true);
        mNetworkScanRepository.launchNetworkScan(getViewLifecycleOwner(), (networkScanResult) -> {
-            if (!mUpdateScanResult) {
-                // Not update UI if not in scan mode.
-                return Unit.INSTANCE;
-            }
-            if (networkScanResult instanceof NetworkScanCellInfos networkScanCellInfos) {
-                scanResultHandler(networkScanCellInfos.getCellInfos());
-                return Unit.INSTANCE;
-            }
-            if (!isPreferenceScreenEnabled()) {
-                clearPreferenceSummary();
-                enablePreferenceScreen(true);
-            } else if (networkScanResult instanceof NetworkScanComplete
-                    && mCellInfoList == null) {
-                // In case the scan timeout before getting any results
-                addMessagePreference(R.string.empty_networks_list);
-            } else if (networkScanResult instanceof NetworkScanError) {
-                addMessagePreference(R.string.network_query_error);
+            if (isPreferenceScreenEnabled()) {
+                scanResultHandler(networkScanResult);
            }

            return Unit.INSTANCE;
        });
    }

-    @Override
-    public void onStart() {
-        super.onStart();
-
-        setProgressBarVisible(true);
-        mUpdateScanResult = true;
-    }
-
    /**
     * Update forbidden PLMNs from the USIM App
     */
@@ -268,8 +245,6 @@ public class NetworkSelectSettings extends DashboardFragment {
            return false;
        }

-        mUpdateScanResult = false;
-
        // Refresh the last selected item in case users reselect network.
        clearPreferenceSummary();
        if (mSelectedPreference != null) {
@@ -380,27 +355,19 @@ public class NetworkSelectSettings extends DashboardFragment {
        }
    }

-    @Keep
    @VisibleForTesting
-    protected void scanResultHandler(List<CellInfo> results) {
-        mCellInfoList = filterOutSatellitePlmn(results);
+    protected void scanResultHandler(NetworkScanRepository.NetworkScanResult results) {
+        mCellInfoList = filterOutSatellitePlmn(results.getCellInfos());
        Log.d(TAG, "CellInfoList: " + CellInfoUtil.cellInfoListToString(mCellInfoList));
-        if (mCellInfoList != null && mCellInfoList.size() != 0) {
-            final NetworkOperatorPreference connectedPref = updateAllPreferenceCategory();
-            if (connectedPref != null) {
-                // update selected preference instance into connected preference
-                if (mSelectedPreference != null) {
-                    mSelectedPreference = connectedPref;
-                }
-            } else if (!isPreferenceScreenEnabled()) {
-                mSelectedPreference.setSummary(R.string.network_connecting);
-            }
-            enablePreferenceScreen(true);
-        } else if (isPreferenceScreenEnabled()) {
+        updateAllPreferenceCategory();
+        NetworkScanRepository.NetworkScanState state = results.getState();
+        if (state == NetworkScanRepository.NetworkScanState.ERROR) {
+            addMessagePreference(R.string.network_query_error);
+        } else if (mCellInfoList.isEmpty()) {
            addMessagePreference(R.string.empty_networks_list);
-            // keep showing progress bar, it will be stopped when error or completed
-            setProgressBarVisible(true);
        }
+        // keep showing progress bar, it will be stopped when error or completed
+        setProgressBarVisible(state == NetworkScanRepository.NetworkScanState.ACTIVE);
    }

    @Keep
@@ -417,11 +384,8 @@ public class NetworkSelectSettings extends DashboardFragment {

    /**
     * Update the content of network operators list.
-     *
-     * @return preference which shows connected
     */
-    @Nullable
-    private NetworkOperatorPreference updateAllPreferenceCategory() {
+    private void updateAllPreferenceCategory() {
        int numberOfPreferences = mPreferenceCategory.getPreferenceCount();

        // remove unused preferences
@@ -432,7 +396,6 @@ public class NetworkSelectSettings extends DashboardFragment {
        }

        // update the content of preference
-        NetworkOperatorPreference connectedPref = null;
        for (int index = 0; index < mCellInfoList.size(); index++) {
            final CellInfo cellInfo = mCellInfoList.get(index);

@@ -457,23 +420,10 @@ public class NetworkSelectSettings extends DashboardFragment {

            if (mCellInfoList.get(index).isRegistered()) {
                pref.setSummary(R.string.network_connected);
-                connectedPref = pref;
            } else {
                pref.setSummary(null);
            }
        }
-
-        // update selected preference instance by index
-        for (int index = 0; index < mCellInfoList.size(); index++) {
-            final CellInfo cellInfo = mCellInfoList.get(index);
-
-            if ((mSelectedPreference != null) && mSelectedPreference.isSameCell(cellInfo)) {
-                mSelectedPreference = (NetworkOperatorPreference)
-                        (mPreferenceCategory.getPreference(index));
-            }
-        }
-
-        return connectedPref;
    }

    /**
@@ -524,13 +474,6 @@ public class NetworkSelectSettings extends DashboardFragment {
        }
    }

-    private boolean isProgressBarVisible() {
-        if (mProgressHeader == null) {
-            return false;
-        }
-        return (mProgressHeader.getVisibility() == View.VISIBLE);
-    }
-
    protected void setProgressBarVisible(boolean visible) {
        if (mProgressHeader != null) {
            mProgressHeader.setVisibility(visible ? View.VISIBLE : View.GONE);
@@ -538,7 +481,6 @@ public class NetworkSelectSettings extends DashboardFragment {
    }

    private void addMessagePreference(int messageId) {
-        setProgressBarVisible(false);
        mStatusMessagePreference.setTitle(messageId);
        mPreferenceCategory.removeAll();
        mPreferenceCategory.addPreference(mStatusMessagePreference);
--- a/src/com/android/settings/network/telephony/scan/NetworkScanRepository.kt
+++ b/src/com/android/settings/network/telephony/scan/NetworkScanRepository.kt
@@ -27,25 +27,29 @@ import android.telephony.TelephonyScanManager
 import android.util.Log
 import androidx.lifecycle.LifecycleOwner
 import com.android.settings.R
-import com.android.settings.network.telephony.CellInfoUtil
 import com.android.settings.network.telephony.CellInfoUtil.getNetworkTitle
+import com.android.settings.network.telephony.telephonyManager
 import com.android.settingslib.spa.framework.util.collectLatestWithLifecycle
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.asExecutor
 import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.callbackFlow
+import kotlinx.coroutines.flow.conflate
 import kotlinx.coroutines.flow.flowOn
+import kotlinx.coroutines.flow.onEach

 class NetworkScanRepository(private val context: Context, subId: Int) {
-    sealed interface NetworkScanResult
+    enum class NetworkScanState {
+        ACTIVE, COMPLETE, ERROR
+    }

-    data class NetworkScanCellInfos(val cellInfos: List<CellInfo>) : NetworkScanResult
-    data object NetworkScanComplete : NetworkScanResult
-    data class NetworkScanError(val error: Int) : NetworkScanResult
+    data class NetworkScanResult(
+        val state: NetworkScanState,
+        val cellInfos: List<CellInfo>,
+    )

-    private val telephonyManager =
-        context.getSystemService(TelephonyManager::class.java)!!.createForSubscriptionId(subId)
+    private val telephonyManager = context.telephonyManager(subId)

    /** TODO: Move this to UI layer, when UI layer migrated to Kotlin. */
    fun launchNetworkScan(lifecycleOwner: LifecycleOwner, onResult: (NetworkScanResult) -> Unit) {
@@ -65,23 +69,29 @@ class NetworkScanRepository(private val context: Context, subId: Int) {
    }

    fun networkScanFlow(): Flow<NetworkScanResult> = callbackFlow {
+        var state = NetworkScanState.ACTIVE
+        var cellInfos: List<CellInfo> = emptyList()
+
        val callback = object : TelephonyScanManager.NetworkScanCallback() {
            override fun onResults(results: List<CellInfo>) {
-                val cellInfos = results.distinctBy { CellInfoScanKey(it) }
-                trySend(NetworkScanCellInfos(cellInfos))
-                Log.d(TAG, "CellInfoList: ${CellInfoUtil.cellInfoListToString(cellInfos)}")
+                cellInfos = results.distinctBy { CellInfoScanKey(it) }
+                sendResult()
            }

            override fun onComplete() {
-                trySend(NetworkScanComplete)
-                close()
-                Log.d(TAG, "onComplete")
+                state = NetworkScanState.COMPLETE
+                sendResult()
+                // Don't call close() here since onComplete() could happens before onResults()
            }

            override fun onError(error: Int) {
-                trySend(NetworkScanError(error))
+                state = NetworkScanState.ERROR
+                sendResult()
                close()
-                Log.d(TAG, "onError: $error")
+            }
+
+            private fun sendResult() {
+                trySend(NetworkScanResult(state, cellInfos))
            }
        }

@@ -92,7 +102,7 @@ class NetworkScanRepository(private val context: Context, subId: Int) {
        )

        awaitClose { networkScan.stopScan() }
-    }.flowOn(Dispatchers.Default)
+    }.conflate().onEach { Log.d(TAG, "networkScanFlow: $it") }.flowOn(Dispatchers.Default)

    /** Create network scan for allowed network types. */
    private fun createNetworkScan(): NetworkScanRequest {
--- a/tests/spa_unit/src/com/android/settings/network/telephony/scan/NetworkScanRepositoryTest.kt
+++ b/tests/spa_unit/src/com/android/settings/network/telephony/scan/NetworkScanRepositoryTest.kt
@@ -32,9 +32,6 @@ import android.telephony.TelephonyManager.NETWORK_CLASS_BITMASK_5G
 import android.telephony.TelephonyScanManager
 import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanCellInfos
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanComplete
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanError
 import com.android.settingslib.spa.testutils.firstWithTimeoutOrNull
 import com.android.settingslib.spa.testutils.toListWithTimeout
 import com.google.common.truth.Truth.assertThat
@@ -88,7 +85,12 @@ class NetworkScanRepositoryTest {

        callback?.onResults(cellInfos)

-        assertThat(listDeferred.await()).containsExactly(NetworkScanCellInfos(cellInfos))
+        assertThat(listDeferred.await()).containsExactly(
+            NetworkScanRepository.NetworkScanResult(
+                state = NetworkScanRepository.NetworkScanState.ACTIVE,
+                cellInfos = cellInfos,
+            )
+        )
    }

    @Test
@@ -100,7 +102,12 @@ class NetworkScanRepositoryTest {

        callback?.onComplete()

-        assertThat(listDeferred.await()).containsExactly(NetworkScanComplete)
+        assertThat(listDeferred.await()).containsExactly(
+            NetworkScanRepository.NetworkScanResult(
+                state = NetworkScanRepository.NetworkScanState.COMPLETE,
+                cellInfos = emptyList(),
+            )
+        )
    }

    @Test
@@ -112,7 +119,12 @@ class NetworkScanRepositoryTest {

        callback?.onError(1)

-        assertThat(listDeferred.await()).containsExactly(NetworkScanError(1))
+        assertThat(listDeferred.await()).containsExactly(
+            NetworkScanRepository.NetworkScanResult(
+                state = NetworkScanRepository.NetworkScanState.ERROR,
+                cellInfos = emptyList(),
+            )
+        )
    }

    @Test
@@ -133,12 +145,13 @@ class NetworkScanRepositoryTest {
        callback?.onResults(cellInfos)

        assertThat(listDeferred.await()).containsExactly(
-            NetworkScanCellInfos(
-                listOf(
+            NetworkScanRepository.NetworkScanResult(
+                state = NetworkScanRepository.NetworkScanState.ACTIVE,
+                cellInfos = listOf(
                    createCellInfoLte("123", false),
                    createCellInfoLte("124", true),
                    createCellInfoGsm("123", false),
-                )
+                ),
            )
        )
    }
@@ -162,8 +175,9 @@ class NetworkScanRepositoryTest {
        callback?.onResults(cellInfos)

        assertThat(listDeferred.await()).containsExactly(
-            NetworkScanCellInfos(
-                listOf(
+            NetworkScanRepository.NetworkScanResult(
+                state = NetworkScanRepository.NetworkScanState.ACTIVE,
+                cellInfos = listOf(
                    createCellInfoLte("123", false),
                    createCellInfoLte("123", true),
                    createCellInfoLte("124", false),
--- a/tests/unit/src/com/android/settings/network/telephony/NetworkSelectSettingsTest.java
+++ b/tests/unit/src/com/android/settings/network/telephony/NetworkSelectSettingsTest.java
@@ -44,8 +44,12 @@ import androidx.preference.PreferenceManager;
 import androidx.test.annotation.UiThreadTest;
 import androidx.test.core.app.ApplicationProvider;

+import com.android.settings.network.telephony.scan.NetworkScanRepository;
+import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanResult;
 import com.android.settingslib.core.instrumentation.MetricsFeatureProvider;

+import com.google.common.collect.ImmutableList;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
@@ -163,8 +167,7 @@ public class NetworkSelectSettingsTest {
        }

        @Override
-        protected NetworkOperatorPreference
-                createNetworkOperatorPreference(CellInfo cellInfo) {
+        protected NetworkOperatorPreference createNetworkOperatorPreference(CellInfo cellInfo) {
            NetworkOperatorPreference pref = super.createNetworkOperatorPreference(cellInfo);
            if (cellInfo == mTestEnv.mCellInfo1) {
                pref.updateCell(cellInfo, mTestEnv.mCellId1);
@@ -183,9 +186,14 @@ public class NetworkSelectSettingsTest {
    @Test
    @UiThreadTest
    public void updateAllPreferenceCategory_correctOrderingPreference() {
+        NetworkScanResult result = new NetworkScanResult(
+                NetworkScanRepository.NetworkScanState.COMPLETE,
+                ImmutableList.of(mCellInfo1, mCellInfo2));
        mNetworkSelectSettings.onCreateInitialization();
        mNetworkSelectSettings.enablePreferenceScreen(true);
-        mNetworkSelectSettings.scanResultHandler(Arrays.asList(mCellInfo1, mCellInfo2));
+
+        mNetworkSelectSettings.scanResultHandler(result);
+
        assertThat(mPreferenceCategory.getPreferenceCount()).isEqualTo(2);
        final NetworkOperatorPreference preference =
                (NetworkOperatorPreference) mPreferenceCategory.getPreference(1);