From 1b65eb52b1314a67551a1d0c430eb54249ae3c98 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Mon, 9 Jan 2023 16:08:28 -0500 Subject: [PATCH 1/6] [Sb refactor] Upgrade MobileIconsInteractorTest to use testScope These tests were written before the library was available, and future CLs will need the fine-grained time manipulation provided with `TestScope` Test: MobileIconsInteractorTest Bug: 238425913 Change-Id: Ica20be32bc56f911625d5ed0e4bad90a046ebd30 --- .../interactor/MobileIconsInteractorTest.kt | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt index b82a58414db9e..721d49a13834e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt @@ -31,11 +31,12 @@ import com.android.systemui.util.CarrierConfigTracker import com.android.systemui.util.mockito.whenever import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest import kotlinx.coroutines.yield import org.junit.After import org.junit.Before @@ -43,13 +44,16 @@ import org.junit.Test import org.mockito.Mock import org.mockito.MockitoAnnotations +@OptIn(ExperimentalCoroutinesApi::class) @SmallTest class MobileIconsInteractorTest : SysuiTestCase() { private lateinit var underTest: MobileIconsInteractor private lateinit var connectionsRepository: FakeMobileConnectionsRepository private val userSetupRepository = FakeUserSetupRepository() private val mobileMappingsProxy = FakeMobileMappingsProxy() - private val scope = CoroutineScope(IMMEDIATE) + + private val testDispatcher = UnconfinedTestDispatcher() + private val testScope = TestScope(testDispatcher) @Mock private lateinit var carrierConfigTracker: CarrierConfigTracker @@ -73,7 +77,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { connectionsRepository, carrierConfigTracker, userSetupRepository, - scope + testScope.backgroundScope, ) } @@ -81,7 +85,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun filteredSubscriptions_default() = - runBlocking(IMMEDIATE) { + testScope.runTest { var latest: List? = null val job = underTest.filteredSubscriptions.onEach { latest = it }.launchIn(this) @@ -92,7 +96,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun filteredSubscriptions_nonOpportunistic_updatesWithMultipleSubs() = - runBlocking(IMMEDIATE) { + testScope.runTest { connectionsRepository.setSubscriptions(listOf(SUB_1, SUB_2)) var latest: List? = null @@ -105,7 +109,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun filteredSubscriptions_bothOpportunistic_configFalse_showsActive_3() = - runBlocking(IMMEDIATE) { + testScope.runTest { connectionsRepository.setSubscriptions(listOf(SUB_3_OPP, SUB_4_OPP)) connectionsRepository.setActiveMobileDataSubscriptionId(SUB_3_ID) whenever(carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault) @@ -122,7 +126,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun filteredSubscriptions_bothOpportunistic_configFalse_showsActive_4() = - runBlocking(IMMEDIATE) { + testScope.runTest { connectionsRepository.setSubscriptions(listOf(SUB_3_OPP, SUB_4_OPP)) connectionsRepository.setActiveMobileDataSubscriptionId(SUB_4_ID) whenever(carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault) @@ -139,7 +143,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun filteredSubscriptions_oneOpportunistic_configTrue_showsPrimary_active_1() = - runBlocking(IMMEDIATE) { + testScope.runTest { connectionsRepository.setSubscriptions(listOf(SUB_1, SUB_3_OPP)) connectionsRepository.setActiveMobileDataSubscriptionId(SUB_1_ID) whenever(carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault) @@ -157,7 +161,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun filteredSubscriptions_oneOpportunistic_configTrue_showsPrimary_nonActive_1() = - runBlocking(IMMEDIATE) { + testScope.runTest { connectionsRepository.setSubscriptions(listOf(SUB_1, SUB_3_OPP)) connectionsRepository.setActiveMobileDataSubscriptionId(SUB_3_ID) whenever(carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault) @@ -175,7 +179,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun activeDataConnection_turnedOn() = - runBlocking(IMMEDIATE) { + testScope.runTest { CONNECTION_1.setDataEnabled(true) var latest: Boolean? = null val job = @@ -188,7 +192,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun activeDataConnection_turnedOff() = - runBlocking(IMMEDIATE) { + testScope.runTest { CONNECTION_1.setDataEnabled(true) var latest: Boolean? = null val job = @@ -204,7 +208,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun activeDataConnection_invalidSubId() = - runBlocking(IMMEDIATE) { + testScope.runTest { var latest: Boolean? = null val job = underTest.activeDataConnectionHasDataEnabled.onEach { latest = it }.launchIn(this) @@ -220,7 +224,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun failedConnection_connected_validated_notFailed() = - runBlocking(IMMEDIATE) { + testScope.runTest { var latest: Boolean? = null val job = underTest.isDefaultConnectionFailed.onEach { latest = it }.launchIn(this) connectionsRepository.setMobileConnectivity(MobileConnectivityModel(true, true)) @@ -233,7 +237,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun failedConnection_notConnected_notValidated_notFailed() = - runBlocking(IMMEDIATE) { + testScope.runTest { var latest: Boolean? = null val job = underTest.isDefaultConnectionFailed.onEach { latest = it }.launchIn(this) @@ -247,7 +251,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun failedConnection_connected_notValidated_failed() = - runBlocking(IMMEDIATE) { + testScope.runTest { var latest: Boolean? = null val job = underTest.isDefaultConnectionFailed.onEach { latest = it }.launchIn(this) @@ -261,7 +265,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun alwaysShowDataRatIcon_configHasTrue() = - runBlocking(IMMEDIATE) { + testScope.runTest { var latest: Boolean? = null val job = underTest.alwaysShowDataRatIcon.onEach { latest = it }.launchIn(this) @@ -277,7 +281,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun alwaysShowDataRatIcon_configHasFalse() = - runBlocking(IMMEDIATE) { + testScope.runTest { var latest: Boolean? = null val job = underTest.alwaysShowDataRatIcon.onEach { latest = it }.launchIn(this) @@ -293,7 +297,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun alwaysUseCdmaLevel_configHasTrue() = - runBlocking(IMMEDIATE) { + testScope.runTest { var latest: Boolean? = null val job = underTest.alwaysUseCdmaLevel.onEach { latest = it }.launchIn(this) @@ -309,7 +313,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { @Test fun alwaysUseCdmaLevel_configHasFalse() = - runBlocking(IMMEDIATE) { + testScope.runTest { var latest: Boolean? = null val job = underTest.alwaysUseCdmaLevel.onEach { latest = it }.launchIn(this) @@ -324,7 +328,6 @@ class MobileIconsInteractorTest : SysuiTestCase() { } companion object { - private val IMMEDIATE = Dispatchers.Main.immediate private val tableLogBuffer = TableLogBuffer(8, "MobileIconsInteractorTest", FakeSystemClock()) From 5ceb454c8e8465ddd6df1e046f46719b3bb1702d Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Tue, 10 Jan 2023 15:38:56 -0500 Subject: [PATCH 2/6] [Sb refactor] Add the default network's connectivity to the view model Part of the criteria of whether or not to show the RAT indicator is whether or not `mobile` is a connected transport. The way the old pipeline does this is by keeping track of the default network capabilities (via `ConnectivityManager`) and letting those be known by all mobile connections. The equivalent in the new pipeline is to expose the default network capabilities via the MobileIconsInteractor, and pass it down to each individual icon interactor. Finally, the reason this is sensible to do in the new pipeline is that we can track when the data subscription changes and thus we can avoid icon flickering (see following CL) Test: MobileIconViewModelTest Test: MobileIconInteractorTest Test: MobileIconsInteractorTest Bug: 264683083 Bug: 238425913 Change-Id: Ib3a4f14ba3cb403f342090b7394d271309a3a7f5 --- .../domain/interactor/MobileIconInteractor.kt | 27 ++++++++++++-- .../interactor/MobileIconsInteractor.kt | 13 +++++++ .../ui/viewmodel/MobileIconViewModel.kt | 23 +++++++----- .../interactor/FakeMobileIconInteractor.kt | 2 ++ .../interactor/FakeMobileIconsInteractor.kt | 3 ++ .../interactor/MobileIconInteractorTest.kt | 1 + .../interactor/MobileIconsInteractorTest.kt | 26 ++++++++++++++ .../ui/viewmodel/MobileIconViewModelTest.kt | 35 +++++++++++++++++++ 8 files changed, 118 insertions(+), 12 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt index 003df2482c6e4..f85a1213e9287 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt @@ -21,6 +21,7 @@ import com.android.settingslib.SignalIcon.MobileIconGroup import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.DataConnectionState.Connected +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectivityModel import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel import com.android.systemui.statusbar.pipeline.mobile.data.model.ResolvedNetworkType import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository @@ -41,12 +42,29 @@ interface MobileIconInteractor { /** The current mobile data activity */ val activity: Flow + /** + * This bit is meant to be `true` if and only if the default network capabilities (see + * [android.net.ConnectivityManager.registerDefaultNetworkCallback]) result in a network that + * has the [android.net.NetworkCapabilities.TRANSPORT_CELLULAR] represented. + * + * Note that this differs from [isDataConnected], which is tracked by telephony and has to do + * with the state of using this mobile connection for data as opposed to just voice. It is + * possible for a mobile subscription to be connected but not be in a connected data state, and + * thus we wouldn't want to show the network type icon. + */ + val isConnected: Flow + + /** + * True when telephony tells us that the data state is CONNECTED. See + * [android.telephony.TelephonyCallback.DataConnectionStateListener] for more details. We + * consider this connection to be serving data, and thus want to show a network type icon, when + * data is connected. Other data connection states would typically cause us not to show the icon + */ + val isDataConnected: StateFlow + /** Only true if mobile is the default transport but is not validated, otherwise false */ val isDefaultConnectionFailed: StateFlow - /** True when telephony tells us that the data state is CONNECTED */ - val isDataConnected: StateFlow - /** True if we consider this connection to be in service, i.e. can make calls */ val isInService: StateFlow @@ -100,6 +118,7 @@ class MobileIconInteractorImpl( defaultSubscriptionHasDataEnabled: StateFlow, override val alwaysShowDataRatIcon: StateFlow, override val alwaysUseCdmaLevel: StateFlow, + defaultMobileConnectivity: StateFlow, defaultMobileIconMapping: StateFlow>, defaultMobileIconGroup: StateFlow, override val isDefaultConnectionFailed: StateFlow, @@ -111,6 +130,8 @@ class MobileIconInteractorImpl( override val activity = connectionInfo.mapLatest { it.dataActivityDirection } + override val isConnected: Flow = defaultMobileConnectivity.mapLatest { it.isConnected } + override val isDataEnabled: StateFlow = connectionRepository.dataEnabled override val isDefaultDataEnabled = defaultSubscriptionHasDataEnabled diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt index 83da1dd067806..96196e72b1031 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt @@ -23,6 +23,7 @@ import com.android.settingslib.SignalIcon.MobileIconGroup import com.android.settingslib.mobile.TelephonyIcons import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dagger.qualifiers.Application +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectivityModel import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionsRepository @@ -62,6 +63,14 @@ interface MobileIconsInteractor { /** True if the CDMA level should be preferred over the primary level. */ val alwaysUseCdmaLevel: StateFlow + /** + * The connectivity of the default mobile network. Note that this can differ from what is + * reported from [MobileConnectionsRepository] in some cases. E.g., when the active subscription + * changes but the groupUuid remains the same, we keep the old validation information for 2 + * seconds to avoid icon flickering. + */ + val defaultMobileNetworkConnectivity: StateFlow + /** The icon mapping from network type to [MobileIconGroup] for the default subscription */ val defaultMobileIconMapping: StateFlow> /** Fallback [MobileIconGroup] in the case where there is no icon in the mapping */ @@ -154,6 +163,9 @@ constructor( } } + override val defaultMobileNetworkConnectivity: StateFlow = + mobileConnectionsRepo.defaultMobileNetworkConnectivity + /** * Mapping from network type to [MobileIconGroup] using the config generated for the default * subscription Id. This mapping is the same for every subscription. @@ -207,6 +219,7 @@ constructor( activeDataConnectionHasDataEnabled, alwaysShowDataRatIcon, alwaysUseCdmaLevel, + defaultMobileNetworkConnectivity, defaultMobileIconMapping, defaultMobileIconGroup, isDefaultConnectionFailed, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt index a2117c7df188e..5e9356163e6e5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt @@ -102,24 +102,29 @@ constructor( .stateIn(scope, SharingStarted.WhileSubscribed(), initial) } + private val showNetworkTypeIcon: Flow = + combine( + iconInteractor.isDataConnected, + iconInteractor.isDataEnabled, + iconInteractor.isDefaultConnectionFailed, + iconInteractor.alwaysShowDataRatIcon, + iconInteractor.isConnected, + ) { dataConnected, dataEnabled, failedConnection, alwaysShow, connected -> + alwaysShow || (dataConnected && dataEnabled && !failedConnection && connected) + } + override val networkTypeIcon: Flow = combine( iconInteractor.networkTypeIconGroup, - iconInteractor.isDataConnected, - iconInteractor.isDataEnabled, - iconInteractor.isDefaultConnectionFailed, - iconInteractor.alwaysShowDataRatIcon, - ) { networkTypeIconGroup, dataConnected, dataEnabled, failedConnection, alwaysShow -> + showNetworkTypeIcon, + ) { networkTypeIconGroup, shouldShow -> val desc = if (networkTypeIconGroup.dataContentDescription != 0) ContentDescription.Resource(networkTypeIconGroup.dataContentDescription) else null val icon = Icon.Resource(networkTypeIconGroup.dataType, desc) return@combine when { - alwaysShow -> icon - !dataConnected -> null - !dataEnabled -> null - failedConnection -> null + !shouldShow -> null else -> icon } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconInteractor.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconInteractor.kt index a29146b016680..7aeaa48165aa6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconInteractor.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconInteractor.kt @@ -40,6 +40,8 @@ class FakeMobileIconInteractor( ) ) + override val isConnected = MutableStateFlow(true) + private val _iconGroup = MutableStateFlow(TelephonyIcons.THREE_G) override val networkTypeIconGroup = _iconGroup diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconsInteractor.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconsInteractor.kt index 1c0064610c52f..e57ad768ec8a9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconsInteractor.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconsInteractor.kt @@ -23,6 +23,7 @@ import android.telephony.TelephonyManager.NETWORK_TYPE_UMTS import com.android.settingslib.SignalIcon.MobileIconGroup import com.android.settingslib.mobile.TelephonyIcons import com.android.systemui.log.table.TableLogBuffer +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectivityModel import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy import kotlinx.coroutines.flow.MutableStateFlow @@ -60,6 +61,8 @@ class FakeMobileIconsInteractor( override val alwaysUseCdmaLevel = MutableStateFlow(false) + override val defaultMobileNetworkConnectivity = MutableStateFlow(MobileConnectivityModel()) + private val _defaultMobileIconMapping = MutableStateFlow(TEST_MAPPING) override val defaultMobileIconMapping = _defaultMobileIconMapping diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt index e6be7f15235b3..a774ffeb997a2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt @@ -61,6 +61,7 @@ class MobileIconInteractorTest : SysuiTestCase() { mobileIconsInteractor.activeDataConnectionHasDataEnabled, mobileIconsInteractor.alwaysShowDataRatIcon, mobileIconsInteractor.alwaysUseCdmaLevel, + mobileIconsInteractor.defaultMobileNetworkConnectivity, mobileIconsInteractor.defaultMobileIconMapping, mobileIconsInteractor.defaultMobileIconGroup, mobileIconsInteractor.isDefaultConnectionFailed, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt index 721d49a13834e..9e8ae6db449a3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt @@ -327,6 +327,32 @@ class MobileIconsInteractorTest : SysuiTestCase() { job.cancel() } + @Test + fun `default mobile connectivity - uses repo value`() = + testScope.runTest { + var latest: MobileConnectivityModel? = null + val job = + underTest.defaultMobileNetworkConnectivity.onEach { latest = it }.launchIn(this) + + var expected = MobileConnectivityModel(isConnected = true, isValidated = true) + connectionsRepository.setMobileConnectivity(expected) + assertThat(latest).isEqualTo(expected) + + expected = MobileConnectivityModel(isConnected = false, isValidated = true) + connectionsRepository.setMobileConnectivity(expected) + assertThat(latest).isEqualTo(expected) + + expected = MobileConnectivityModel(isConnected = true, isValidated = false) + connectionsRepository.setMobileConnectivity(expected) + assertThat(latest).isEqualTo(expected) + + expected = MobileConnectivityModel(isConnected = false, isValidated = false) + connectionsRepository.setMobileConnectivity(expected) + assertThat(latest).isEqualTo(expected) + + job.cancel() + } + companion object { private val tableLogBuffer = TableLogBuffer(8, "MobileIconsInteractorTest", FakeSystemClock()) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt index 2a8d42ff69975..a24e29aebc1ef 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt @@ -273,6 +273,41 @@ class MobileIconViewModelTest : SysuiTestCase() { job.cancel() } + @Test + fun `network type - alwaysShow - shown when not connected`() = + testScope.runTest { + interactor.setIconGroup(THREE_G) + interactor.isConnected.value = false + interactor.alwaysShowDataRatIcon.value = true + + var latest: Icon? = null + val job = underTest.networkTypeIcon.onEach { latest = it }.launchIn(this) + + val expected = + Icon.Resource( + THREE_G.dataType, + ContentDescription.Resource(THREE_G.dataContentDescription) + ) + assertThat(latest).isEqualTo(expected) + + job.cancel() + } + + @Test + fun `network type - not shown when not connected`() = + testScope.runTest { + interactor.setIconGroup(THREE_G) + interactor.isDataConnected.value = true + interactor.isConnected.value = false + + var latest: Icon? = null + val job = underTest.networkTypeIcon.onEach { latest = it }.launchIn(this) + + assertThat(latest).isNull() + + job.cancel() + } + @Test fun roaming() = testScope.runTest { From 7f894afe510888414cd45277ae7789ac2f7e6c17 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Tue, 10 Jan 2023 15:46:56 -0500 Subject: [PATCH 3/6] [Sb refactor] Implement 2s grace period for data switching The old mobile pipeline supported this particular use case. Given that there are 2 mobile subscriptions, A and B, then when data switches from A to B, we will consider B to be validated (see NetworkCapabilities) for up to 2 seconds under the conditions: 1. A was validated before the switch 2. A and B are both in the same group (i.e. they have the same group UUID) This CL implements the same exact criteria by monitoring for data switching in the same group and maintaining a validated state for 2 seconds for the default subscription. Test: MobileIconsInteractorTest Bug: 238425913 Change-Id: I8e798197beda562634c137377145dea510cbda4c --- .../repository/MobileConnectionsRepository.kt | 6 + .../repository/MobileRepositorySwitcher.kt | 3 + .../demo/DemoMobileConnectionsRepository.kt | 4 + .../prod/MobileConnectionsRepositoryImpl.kt | 32 +++ .../interactor/MobileIconsInteractor.kt | 42 ++- .../FakeMobileConnectionsRepository.kt | 2 + .../interactor/MobileIconsInteractorTest.kt | 254 ++++++++++++++++++ 7 files changed, 342 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt index 97b4c2cadbe58..828bb43bb05dd 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt @@ -37,6 +37,12 @@ interface MobileConnectionsRepository { /** Observable for the subscriptionId of the current mobile data connection */ val activeMobileDataSubscriptionId: StateFlow + /** + * Observable event for when the active data sim switches but the group stays the same. E.g., + * CBRS switching would trigger this + */ + val activeSubChangedInGroupEvent: Flow + /** The current connectivity status for the default mobile network connection */ val defaultMobileNetworkConnectivity: StateFlow diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcher.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcher.kt index 0c8593d60cf5a..164da3b390a72 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcher.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcher.kt @@ -124,6 +124,9 @@ constructor( realRepository.activeMobileDataSubscriptionId.value ) + override val activeSubChangedInGroupEvent: Flow = + activeRepo.flatMapLatest { it.activeSubChangedInGroupEvent } + override val defaultDataSubRatConfig: StateFlow = activeRepo .flatMapLatest { it.defaultDataSubRatConfig } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt index 22aca0a8b0d7a..31a09c6449826 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt @@ -48,6 +48,7 @@ import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted @@ -120,6 +121,9 @@ constructor( subscriptions.value.firstOrNull()?.subscriptionId ?: INVALID_SUBSCRIPTION_ID ) + // TODO(b/261029387): consider adding a demo command for this + override val activeSubChangedInGroupEvent: Flow = flowOf() + /** Demo mode doesn't currently support modifications to the mobile mappings */ override val defaultDataSubRatConfig = MutableStateFlow(MobileMappings.Config.readConfig(context)) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt index 4472e0972a0b0..29affcf381448 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt @@ -52,6 +52,7 @@ import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logInputChange import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepository +import com.android.systemui.util.kotlin.pairwiseBy import com.android.systemui.util.settings.GlobalSettings import javax.inject.Inject import kotlinx.coroutines.CoroutineDispatcher @@ -63,6 +64,8 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.merge @@ -258,6 +261,35 @@ constructor( .logInputChange(logger, "defaultMobileNetworkConnectivity") .stateIn(scope, SharingStarted.WhileSubscribed(), MobileConnectivityModel()) + /** + * Flow that tracks the active mobile data subscriptions. Emits `true` whenever the active data + * subscription Id changes but the subscription group remains the same. In these cases, we want + * to retain the previous subscription's validation status for up to 2s to avoid flickering the + * icon. + * + * TODO(b/265164432): we should probably expose all change events, not just same group + */ + @SuppressLint("MissingPermission") + override val activeSubChangedInGroupEvent = + flow { + activeMobileDataSubscriptionId.pairwiseBy { prevVal: Int, newVal: Int -> + if (!defaultMobileNetworkConnectivity.value.isValidated) { + return@pairwiseBy + } + val prevSub = subscriptionManager.getActiveSubscriptionInfo(prevVal) + val nextSub = subscriptionManager.getActiveSubscriptionInfo(newVal) + + if (prevSub == null || nextSub == null) { + return@pairwiseBy + } + + if (prevSub.groupUuid != null && prevSub.groupUuid == nextSub.groupUuid) { + emit(Unit) + } + } + } + .flowOn(bgDispatcher) + private fun isValidSubId(subId: Int): Boolean { subscriptions.value.forEach { if (it.subscriptionId == subId) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt index 96196e72b1031..b289315adb881 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt @@ -32,14 +32,17 @@ import com.android.systemui.util.CarrierConfigTracker import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.flow.transformLatest /** * Business layer logic for the set of mobile subscription icons. @@ -163,8 +166,45 @@ constructor( } } + /** + * Copied from the old pipeline. We maintain a 2s period of time where we will keep the + * validated bit from the old active network (A) while data is changing to the new one (B). + * + * This condition only applies if + * 1. A and B are in the same subscription group (e.c. for CBRS data switching) and + * 2. A was validated before the switch + * + * The goal of this is to minimize the flickering in the UI of the cellular indicator + */ + private val forcingCellularValidation = + mobileConnectionsRepo.activeSubChangedInGroupEvent + .filter { mobileConnectionsRepo.defaultMobileNetworkConnectivity.value.isValidated } + .transformLatest { + emit(true) + delay(2000) + emit(false) + } + .stateIn(scope, SharingStarted.WhileSubscribed(), false) + override val defaultMobileNetworkConnectivity: StateFlow = - mobileConnectionsRepo.defaultMobileNetworkConnectivity + combine( + mobileConnectionsRepo.defaultMobileNetworkConnectivity, + forcingCellularValidation, + ) { networkConnectivity, forceValidation -> + return@combine if (forceValidation) { + MobileConnectivityModel( + isValidated = true, + isConnected = networkConnectivity.isConnected + ) + } else { + networkConnectivity + } + } + .stateIn( + scope, + SharingStarted.WhileSubscribed(), + mobileConnectionsRepo.defaultMobileNetworkConnectivity.value + ) /** * Mapping from network type to [MobileIconGroup] using the config generated for the default diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionsRepository.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionsRepository.kt index 49d4bdc88c828..3acada492bf2e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionsRepository.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionsRepository.kt @@ -26,6 +26,7 @@ import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectivityModel import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy +import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow // TODO(b/261632894): remove this in favor of the real impl or DemoMobileConnectionsRepository @@ -56,6 +57,7 @@ class FakeMobileConnectionsRepository( private val _activeMobileDataSubscriptionId = MutableStateFlow(INVALID_SUBSCRIPTION_ID) override val activeMobileDataSubscriptionId = _activeMobileDataSubscriptionId + override val activeSubChangedInGroupEvent: MutableSharedFlow = MutableSharedFlow() private val _mobileConnectivity = MutableStateFlow(MobileConnectivityModel()) override val defaultMobileNetworkConnectivity = _mobileConnectivity diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt index 9e8ae6db449a3..1b62d5cc15b5a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt @@ -36,6 +36,7 @@ import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.runTest import kotlinx.coroutines.yield import org.junit.After @@ -353,6 +354,259 @@ class MobileIconsInteractorTest : SysuiTestCase() { job.cancel() } + @Test + fun `data switch - in same group - validated matches previous value`() = + testScope.runTest { + var latest: MobileConnectivityModel? = null + val job = + underTest.defaultMobileNetworkConnectivity.onEach { latest = it }.launchIn(this) + + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = true, + isValidated = true, + ) + ) + // Trigger a data change in the same subscription group + connectionsRepository.activeSubChangedInGroupEvent.emit(Unit) + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + + assertThat(latest) + .isEqualTo( + MobileConnectivityModel( + isConnected = false, + isValidated = true, + ) + ) + + job.cancel() + } + + @Test + fun `data switch - in same group - validated matches previous value - expires after 2s`() = + testScope.runTest { + var latest: MobileConnectivityModel? = null + val job = + underTest.defaultMobileNetworkConnectivity.onEach { latest = it }.launchIn(this) + + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = true, + isValidated = true, + ) + ) + // Trigger a data change in the same subscription group + connectionsRepository.activeSubChangedInGroupEvent.emit(Unit) + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + // After 1s, the force validation bit is still present + advanceTimeBy(1000) + assertThat(latest) + .isEqualTo( + MobileConnectivityModel( + isConnected = false, + isValidated = true, + ) + ) + + // After 2s, the force validation expires + advanceTimeBy(1001) + + assertThat(latest) + .isEqualTo( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + + job.cancel() + } + + @Test + fun `data switch - in same group - not validated - uses new value immediately`() = + testScope.runTest { + var latest: MobileConnectivityModel? = null + val job = + underTest.defaultMobileNetworkConnectivity.onEach { latest = it }.launchIn(this) + + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = true, + isValidated = false, + ) + ) + connectionsRepository.activeSubChangedInGroupEvent.emit(Unit) + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + + assertThat(latest) + .isEqualTo( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + + job.cancel() + } + + @Test + fun `data switch - lose validation - then switch happens - clears forced bit`() = + testScope.runTest { + var latest: MobileConnectivityModel? = null + val job = + underTest.defaultMobileNetworkConnectivity.onEach { latest = it }.launchIn(this) + + // GIVEN the network starts validated + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = true, + isValidated = true, + ) + ) + + // WHEN a data change happens in the same group + connectionsRepository.activeSubChangedInGroupEvent.emit(Unit) + + // WHEN the validation bit is lost + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + + // WHEN another data change happens in the same group + connectionsRepository.activeSubChangedInGroupEvent.emit(Unit) + + // THEN the forced validation bit is still removed after 2s + assertThat(latest) + .isEqualTo( + MobileConnectivityModel( + isConnected = false, + isValidated = true, + ) + ) + + advanceTimeBy(1000) + + assertThat(latest) + .isEqualTo( + MobileConnectivityModel( + isConnected = false, + isValidated = true, + ) + ) + + advanceTimeBy(1001) + + assertThat(latest) + .isEqualTo( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + + job.cancel() + } + + @Test + fun `data switch - while already forcing validation - resets clock`() = + testScope.runTest { + var latest: MobileConnectivityModel? = null + val job = + underTest.defaultMobileNetworkConnectivity.onEach { latest = it }.launchIn(this) + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = true, + isValidated = true, + ) + ) + + connectionsRepository.activeSubChangedInGroupEvent.emit(Unit) + + advanceTimeBy(1000) + + // WHEN another change in same group event happens + connectionsRepository.activeSubChangedInGroupEvent.emit(Unit) + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + + // THEN the forced validation remains for exactly 2 more seconds from now + + // 1.500s from second event + advanceTimeBy(1500) + assertThat(latest) + .isEqualTo( + MobileConnectivityModel( + isConnected = false, + isValidated = true, + ) + ) + + // 2.001s from the second event + advanceTimeBy(501) + assertThat(latest) + .isEqualTo( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + + job.cancel() + } + + @Test + fun `data switch - not in same group - uses new values`() = + testScope.runTest { + var latest: MobileConnectivityModel? = null + val job = + underTest.defaultMobileNetworkConnectivity.onEach { latest = it }.launchIn(this) + + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = true, + isValidated = true, + ) + ) + connectionsRepository.setMobileConnectivity( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + + assertThat(latest) + .isEqualTo( + MobileConnectivityModel( + isConnected = false, + isValidated = false, + ) + ) + + job.cancel() + } + companion object { private val tableLogBuffer = TableLogBuffer(8, "MobileIconsInteractorTest", FakeSystemClock()) From 08fc0a67c5774b79e09d3cfc0ff4395398ce6906 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Thu, 12 Jan 2023 15:41:49 -0500 Subject: [PATCH 4/6] Partial revert of "[SB Refactor] Remove unused `isDefaultDataSubscription` flow." Reference commit 3747a4723edec8d89c53c2ff52091680cf8f7bc6. The reverted commit removed MobileConnectionsRepository.defaultDataSubId and MobileConnectionRepository.isDefaultDataSubscription, the latter of which was truly unused and incorrectly defined. The former, however, is the one needed for NOT_DEFAULT_DATA Test: MobileConnectionsRepositoryTest Bug: 238425913 Change-Id: I0945a29d50e42ae8f0a09fd54fcfc5d4b4e4affe --- .../repository/MobileConnectionsRepository.kt | 4 +++ .../repository/MobileRepositorySwitcher.kt | 5 ++++ .../demo/DemoMobileConnectionsRepository.kt | 6 ++++ .../prod/MobileConnectionsRepositoryImpl.kt | 22 ++++++++++++-- .../FakeMobileConnectionsRepository.kt | 7 +++++ .../prod/MobileConnectionsRepositoryTest.kt | 29 +++++++++++++++++++ 6 files changed, 70 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt index 828bb43bb05dd..e0d156aa25f37 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionsRepository.kt @@ -18,6 +18,7 @@ package com.android.systemui.statusbar.pipeline.mobile.data.repository import android.provider.Settings import android.telephony.CarrierConfigManager +import android.telephony.SubscriptionManager import com.android.settingslib.SignalIcon.MobileIconGroup import com.android.settingslib.mobile.MobileMappings import com.android.settingslib.mobile.MobileMappings.Config @@ -43,6 +44,9 @@ interface MobileConnectionsRepository { */ val activeSubChangedInGroupEvent: Flow + /** Tracks [SubscriptionManager.getDefaultDataSubscriptionId] */ + val defaultDataSubId: StateFlow + /** The current connectivity status for the default mobile network connection */ val defaultMobileNetworkConnectivity: StateFlow diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcher.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcher.kt index 164da3b390a72..b93985604fb3e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcher.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcher.kt @@ -142,6 +142,11 @@ constructor( override val defaultMobileIconGroup: Flow = activeRepo.flatMapLatest { it.defaultMobileIconGroup } + override val defaultDataSubId: StateFlow = + activeRepo + .flatMapLatest { it.defaultDataSubId } + .stateIn(scope, SharingStarted.WhileSubscribed(), realRepository.defaultDataSubId.value) + override val defaultMobileNetworkConnectivity: StateFlow = activeRepo .flatMapLatest { it.defaultMobileNetworkConnectivity } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt index 31a09c6449826..1fcc727e8ad83 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt @@ -152,6 +152,9 @@ constructor( private fun Map.reverse() = entries.associateBy({ it.value }) { it.key } + // TODO(b/261029387): add a command for this value + override val defaultDataSubId = MutableStateFlow(INVALID_SUBSCRIPTION_ID) + // TODO(b/261029387): not yet supported override val defaultMobileNetworkConnectivity = MutableStateFlow(MobileConnectivityModel()) @@ -233,6 +236,9 @@ constructor( val connection = getRepoForSubId(subId) connectionRepoCache[subId]?.lastMobileState = state + // TODO(b/261029387): until we have a command, use the most recent subId + defaultDataSubId.value = subId + // This is always true here, because we split out disabled states at the data-source level connection.dataEnabled.value = true connection.networkName.value = NetworkNameModel.Derived(state.name) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt index 29affcf381448..510482dcb21e0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt @@ -35,6 +35,7 @@ import android.telephony.TelephonyCallback import android.telephony.TelephonyCallback.ActiveDataSubscriptionIdListener import android.telephony.TelephonyManager import androidx.annotation.VisibleForTesting +import com.android.internal.telephony.PhoneConstants import com.android.settingslib.SignalIcon.MobileIconGroup import com.android.settingslib.mobile.MobileMappings.Config import com.android.systemui.R @@ -61,6 +62,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.asExecutor import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged @@ -162,10 +164,24 @@ constructor( .logInputChange(logger, "onActiveDataSubscriptionIdChanged") .stateIn(scope, started = SharingStarted.WhileSubscribed(), INVALID_SUBSCRIPTION_ID) - private val defaultDataSubIdChangedEvent = + private val defaultDataSubIdChangeEvent: MutableSharedFlow = + MutableSharedFlow(extraBufferCapacity = 1) + + override val defaultDataSubId: StateFlow = broadcastDispatcher - .broadcastFlow(IntentFilter(TelephonyManager.ACTION_DEFAULT_DATA_SUBSCRIPTION_CHANGED)) + .broadcastFlow( + IntentFilter(TelephonyManager.ACTION_DEFAULT_DATA_SUBSCRIPTION_CHANGED) + ) { intent, _ -> + intent.getIntExtra(PhoneConstants.SUBSCRIPTION_KEY, INVALID_SUBSCRIPTION_ID) + } + .distinctUntilChanged() .logInputChange(logger, "ACTION_DEFAULT_DATA_SUBSCRIPTION_CHANGED") + .onEach { defaultDataSubIdChangeEvent.tryEmit(Unit) } + .stateIn( + scope, + SharingStarted.WhileSubscribed(), + SubscriptionManager.getDefaultDataSubscriptionId() + ) private val carrierConfigChangedEvent = broadcastDispatcher @@ -173,7 +189,7 @@ constructor( .logInputChange(logger, "ACTION_CARRIER_CONFIG_CHANGED") override val defaultDataSubRatConfig: StateFlow = - merge(defaultDataSubIdChangedEvent, carrierConfigChangedEvent) + merge(defaultDataSubIdChangeEvent, carrierConfigChangedEvent) .mapLatest { Config.readConfig(context) } .distinctUntilChanged() .logInputChange(logger, "defaultDataSubRatConfig") diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionsRepository.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionsRepository.kt index 3acada492bf2e..0add905e2750c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionsRepository.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionsRepository.kt @@ -59,6 +59,9 @@ class FakeMobileConnectionsRepository( override val activeMobileDataSubscriptionId = _activeMobileDataSubscriptionId override val activeSubChangedInGroupEvent: MutableSharedFlow = MutableSharedFlow() + private val _defaultDataSubId = MutableStateFlow(INVALID_SUBSCRIPTION_ID) + override val defaultDataSubId = _defaultDataSubId + private val _mobileConnectivity = MutableStateFlow(MobileConnectivityModel()) override val defaultMobileNetworkConnectivity = _mobileConnectivity @@ -83,6 +86,10 @@ class FakeMobileConnectionsRepository( _subscriptions.value = subs } + fun setDefaultDataSubId(id: Int) { + _defaultDataSubId.value = id + } + fun setMobileConnectivity(model: MobileConnectivityModel) { _mobileConnectivity.value = model } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt index 813b0ed041a75..ae390a0e29591 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt @@ -478,6 +478,35 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { job.cancel() } + @Test + fun testDefaultDataSubId_updatesOnBroadcast() = + runBlocking(IMMEDIATE) { + var latest: Int? = null + val job = underTest.defaultDataSubId.onEach { latest = it }.launchIn(this) + + fakeBroadcastDispatcher.registeredReceivers.forEach { receiver -> + receiver.onReceive( + context, + Intent(TelephonyManager.ACTION_DEFAULT_DATA_SUBSCRIPTION_CHANGED) + .putExtra(PhoneConstants.SUBSCRIPTION_KEY, SUB_2_ID) + ) + } + + assertThat(latest).isEqualTo(SUB_2_ID) + + fakeBroadcastDispatcher.registeredReceivers.forEach { receiver -> + receiver.onReceive( + context, + Intent(TelephonyManager.ACTION_DEFAULT_DATA_SUBSCRIPTION_CHANGED) + .putExtra(PhoneConstants.SUBSCRIPTION_KEY, SUB_1_ID) + ) + } + + assertThat(latest).isEqualTo(SUB_1_ID) + + job.cancel() + } + @Test fun mobileConnectivity_default() { assertThat(underTest.defaultMobileNetworkConnectivity.value) From 5c87aeada191c4b411f9ddb7d61b4c8005fd6460 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Tue, 10 Jan 2023 17:40:32 -0500 Subject: [PATCH 5/6] [Sb refactor] Support NOT_DEFAULT_DATA network type In the old pipeline (and now here), we track the default data subscription id, and any mobile network which is NOT that subscription (i.e., every non-default) gets a specific network icon group (per MobileMappings) which encodes a null RAT icon and a proper content description. This CL builds upon the revert and uses the default data subId to implement this icon group. Test: MobileIconInteractorTest Bug: 264683083 Bug: 238425913 Change-Id: I06365d1923714f7855ea4b179bd4bdc20d9d54ba --- .../domain/interactor/MobileIconInteractor.kt | 18 ++++++++++++- .../interactor/MobileIconsInteractor.kt | 6 +++++ .../interactor/FakeMobileIconsInteractor.kt | 3 +++ .../interactor/MobileIconInteractorTest.kt | 25 +++++++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt index f85a1213e9287..9cdff96dc7d97 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt @@ -18,6 +18,7 @@ package com.android.systemui.statusbar.pipeline.mobile.domain.interactor import android.telephony.CarrierConfigManager import com.android.settingslib.SignalIcon.MobileIconGroup +import com.android.settingslib.mobile.TelephonyIcons.NOT_DEFAULT_DATA import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.DataConnectionState.Connected @@ -121,6 +122,7 @@ class MobileIconInteractorImpl( defaultMobileConnectivity: StateFlow, defaultMobileIconMapping: StateFlow>, defaultMobileIconGroup: StateFlow, + defaultDataSubId: StateFlow, override val isDefaultConnectionFailed: StateFlow, connectionRepository: MobileConnectionRepository, ) : MobileIconInteractor { @@ -134,6 +136,15 @@ class MobileIconInteractorImpl( override val isDataEnabled: StateFlow = connectionRepository.dataEnabled + private val isDefault = + defaultDataSubId + .mapLatest { connectionRepository.subId == it } + .stateIn( + scope, + SharingStarted.WhileSubscribed(), + connectionRepository.subId == defaultDataSubId.value + ) + override val isDefaultDataEnabled = defaultSubscriptionHasDataEnabled override val networkName = @@ -158,7 +169,12 @@ class MobileIconInteractorImpl( connectionInfo, defaultMobileIconMapping, defaultMobileIconGroup, - ) { info, mapping, defaultGroup -> + isDefault, + ) { info, mapping, defaultGroup, isDefault -> + if (!isDefault) { + return@combine NOT_DEFAULT_DATA + } + when (info.resolvedNetworkType) { is ResolvedNetworkType.CarrierMergedNetworkType -> info.resolvedNetworkType.iconGroupOverride diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt index b289315adb881..9ae38e973bd07 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt @@ -66,6 +66,9 @@ interface MobileIconsInteractor { /** True if the CDMA level should be preferred over the primary level. */ val alwaysUseCdmaLevel: StateFlow + /** Tracks the subscriptionId set as the default for data connections */ + val defaultDataSubId: StateFlow + /** * The connectivity of the default mobile network. Note that this can differ from what is * reported from [MobileConnectionsRepository] in some cases. E.g., when the active subscription @@ -166,6 +169,8 @@ constructor( } } + override val defaultDataSubId = mobileConnectionsRepo.defaultDataSubId + /** * Copied from the old pipeline. We maintain a 2s period of time where we will keep the * validated bit from the old active network (A) while data is changing to the new one (B). @@ -262,6 +267,7 @@ constructor( defaultMobileNetworkConnectivity, defaultMobileIconMapping, defaultMobileIconGroup, + defaultDataSubId, isDefaultConnectionFailed, mobileConnectionsRepo.getRepoForSubId(subId), ) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconsInteractor.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconsInteractor.kt index e57ad768ec8a9..172755cb8d615 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconsInteractor.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconsInteractor.kt @@ -60,6 +60,7 @@ class FakeMobileIconsInteractor( override val alwaysShowDataRatIcon = MutableStateFlow(false) override val alwaysUseCdmaLevel = MutableStateFlow(false) + override val defaultDataSubId = MutableStateFlow(DEFAULT_DATA_SUB_ID) override val defaultMobileNetworkConnectivity = MutableStateFlow(MobileConnectivityModel()) @@ -80,6 +81,8 @@ class FakeMobileIconsInteractor( companion object { val DEFAULT_ICON = TelephonyIcons.G + const val DEFAULT_DATA_SUB_ID = 1 + // Use [MobileMappings] to define some simple definitions const val THREE_G = NETWORK_TYPE_GSM const val LTE = NETWORK_TYPE_LTE diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt index a774ffeb997a2..c42aba5a7dd9d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt @@ -64,6 +64,7 @@ class MobileIconInteractorTest : SysuiTestCase() { mobileIconsInteractor.defaultMobileNetworkConnectivity, mobileIconsInteractor.defaultMobileIconMapping, mobileIconsInteractor.defaultMobileIconGroup, + mobileIconsInteractor.defaultDataSubId, mobileIconsInteractor.isDefaultConnectionFailed, connectionRepository, ) @@ -289,6 +290,30 @@ class MobileIconInteractorTest : SysuiTestCase() { job.cancel() } + @Test + fun `icon group - checks default data`() = + runBlocking(IMMEDIATE) { + mobileIconsInteractor.defaultDataSubId.value = SUB_1_ID + connectionRepository.setConnectionInfo( + MobileConnectionModel( + resolvedNetworkType = DefaultNetworkType(mobileMappingsProxy.toIconKey(THREE_G)) + ), + ) + + var latest: MobileIconGroup? = null + val job = underTest.networkTypeIconGroup.onEach { latest = it }.launchIn(this) + + assertThat(latest).isEqualTo(TelephonyIcons.THREE_G) + + // Default data sub id changes to something else + mobileIconsInteractor.defaultDataSubId.value = 123 + yield() + + assertThat(latest).isEqualTo(TelephonyIcons.NOT_DEFAULT_DATA) + + job.cancel() + } + @Test fun alwaysShowDataRatIcon_matchesParent() = runBlocking(IMMEDIATE) { From 4c85530de0c72fdddce8d709e7ab761703cc01c5 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Tue, 10 Jan 2023 17:42:23 -0500 Subject: [PATCH 6/6] [Sb refactor] [demo] Use better default for demo mode DemoModeConnectionsRepository was using (isConnected = false, isValidated = false) for its default mobile connectivity model, and doesn't yet support updating those values via the command line. This CL just changes the default to be (true, true) so that the icons properly show up. Test: manual demo mode Bug: 238425913 Change-Id: I1c9ade7ed0bd3c278ed38a3b306627a2879fb83e --- .../repository/demo/DemoMobileConnectionsRepository.kt | 3 ++- .../demo/DemoMobileConnectionsRepositoryTest.kt | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt index 1fcc727e8ad83..108834521ebf0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt @@ -156,7 +156,8 @@ constructor( override val defaultDataSubId = MutableStateFlow(INVALID_SUBSCRIPTION_ID) // TODO(b/261029387): not yet supported - override val defaultMobileNetworkConnectivity = MutableStateFlow(MobileConnectivityModel()) + override val defaultMobileNetworkConnectivity = + MutableStateFlow(MobileConnectivityModel(isConnected = true, isValidated = true)) override fun getRepoForSubId(subId: Int): DemoMobileConnectionRepository { val current = connectionRepoCache[subId]?.repo diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepositoryTest.kt index 9d16b7fe52460..f12d113cfaa8f 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepositoryTest.kt @@ -89,6 +89,14 @@ class DemoMobileConnectionsRepositoryTest : SysuiTestCase() { underTest.startProcessingCommands() } + @Test + fun `connectivity - defaults to connected and validated`() = + testScope.runTest { + val connectivity = underTest.defaultMobileNetworkConnectivity.value + assertThat(connectivity.isConnected).isTrue() + assertThat(connectivity.isValidated).isTrue() + } + @Test fun `network event - create new subscription`() = testScope.runTest {