From 6e4ac4bdc03079c898687855667e02f23dc3c1a2 Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Fri, 10 May 2024 14:59:04 +0800 Subject: [PATCH] Fix empty network scan result Settings use TelephonyScanManager.NetworkScanCallback to scan, and its onComplete() could happens before onResults(). We will change Settings to fix. Fix: 338986191 Test: manual - on NetworkSelectSettings Test: unit test Change-Id: If41d957f916a99eacc1becb6b460e58722a4dca7 --- .../telephony/NetworkSelectSettings.java | 92 ++++--------------- .../telephony/scan/NetworkScanRepository.kt | 42 +++++---- .../scan/NetworkScanRepositoryTest.kt | 36 +++++--- .../telephony/NetworkSelectSettingsTest.java | 14 ++- 4 files changed, 79 insertions(+), 105 deletions(-) diff --git a/src/com/android/settings/network/telephony/NetworkSelectSettings.java b/src/com/android/settings/network/telephony/NetworkSelectSettings.java index f29a5efd2d6..9455b70d091 100644 --- 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 mCellInfoList; + @NonNull + List 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 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); diff --git a/src/com/android/settings/network/telephony/scan/NetworkScanRepository.kt b/src/com/android/settings/network/telephony/scan/NetworkScanRepository.kt index dfa79cbc869..4ae5842c513 100644 --- 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) : NetworkScanResult - data object NetworkScanComplete : NetworkScanResult - data class NetworkScanError(val error: Int) : NetworkScanResult + data class NetworkScanResult( + val state: NetworkScanState, + val cellInfos: List, + ) - 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 = callbackFlow { + var state = NetworkScanState.ACTIVE + var cellInfos: List = emptyList() + val callback = object : TelephonyScanManager.NetworkScanCallback() { override fun onResults(results: List) { - 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 { diff --git 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 index 070c779b5ea..c0b918fcf5d 100644 --- 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), diff --git a/tests/unit/src/com/android/settings/network/telephony/NetworkSelectSettingsTest.java b/tests/unit/src/com/android/settings/network/telephony/NetworkSelectSettingsTest.java index a4657cee8a0..d71af84dcf8 100644 --- 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);